date:20190811

[PATCH] D65993: [NFC][clang] Adding argument based Phase list filtering to getComplicationPhases

2019-08-11 Thread Puyan Lotfi via Phabricator via cfe-commits

plotfi marked 4 inline comments as done.
plotfi added inline comments.



Comment at: clang/include/clang/Driver/Types.h:107
+llvm::opt::DerivedArgList &DAL, ID Id,
+llvm::SmallVectorImpl &Phases);
 

compnerd wrote:
> This really makes things confusing, perhaps renaming `getCompilationPhases` 
> to `getCompletePhaseList` or something might make it less confusing?  
> Although, I suppose that you do have follow up patches to improve this.
I'm mainly going by the original name. But I can certainly do this in a 
followup patch. Actually I'd prefer to not do it here, and address this in a 
patch where I remove the other getFinalPhase method from the Driver class.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65993/new/

https://reviews.llvm.org/D65993



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r368528 - Properly handle reference initialization when detecting gsl::Pointer initialization chains

2019-08-11 Thread Gabor Horvath via cfe-commits

Author: xazax
Date: Sun Aug 11 01:05:28 2019
New Revision: 368528

URL: http://llvm.org/viewvc/llvm-project?rev=368528&view=rev
Log:
Properly handle reference initialization when detecting gsl::Pointer 
initialization chains

Modified:
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368528&r1=368527&r2=368528&view=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Sun Aug 11 01:05:28 2019
@@ -7079,8 +7079,12 @@ static SourceRange nextPathEntryRange(co
 }
 
 static bool pathOnlyInitializesGslPointer(IndirectLocalPath &Path) {
-  return !Path.empty() &&
- Path.back().Kind == IndirectLocalPathEntry::GslPointerInit;
+  for (auto It = Path.rbegin(), End = Path.rend(); It != End; ++It) {
+if (It->Kind == IndirectLocalPathEntry::VarInit)
+  continue;
+return It->Kind == IndirectLocalPathEntry::GslPointerInit;
+  }
+  return false;
 }
 
 void Sema::checkInitializerLifetime(const InitializedEntity &Entity,
@@ -7109,8 +7113,8 @@ void Sema::checkInitializerLifetime(cons
 // a local or temporary owner or the address of a local variable/param. We
 // do not want to follow the references when returning a pointer 
originating
 // from a local owner to avoid the following false positive:
-//   int &p = *localOwner;
-//   someContainer.add(std::move(localOwner));
+//   int &p = *localUniquePtr;
+//   someContainer.add(std::move(localUniquePtr));
 //   return p;
 if (!IsTempGslOwner && pathOnlyInitializesGslPointer(Path) &&
 !(IsLocalGslOwner && !pathContainsInit(Path)))
@@ -7217,7 +7221,7 @@ void Sema::checkInitializerLifetime(cons
 if (pathContainsInit(Path))
   return false;
 
-// Suppress false positives for code like the below:
+// Suppress false positives for code like the one below:
 //   Ctor(unique_ptr up) : member(*up), member2(move(up)) {}
 if (IsLocalGslOwner && pathOnlyInitializesGslPointer(Path))
   return false;

Modified: cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp?rev=368528&r1=368527&r2=368528&view=diff
==
--- cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp (original)
+++ cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp Sun Aug 11 01:05:28 
2019
@@ -130,7 +130,7 @@ typename remove_reference::type &&mov
 template 
 struct basic_iterator {
   basic_iterator operator++();
-  T& operator*();
+  T& operator*() const;
 };
 
 template
@@ -227,8 +227,16 @@ const char *trackThroughMultiplePointer(
 }
 
 struct X {
-  X(std::unique_ptr up) : pointee(*up), pointer(std::move(up)) {}
-
+  X(std::unique_ptr up) :
+pointee(*up), pointee2(up.get()), pointer(std::move(up)) {}
   int &pointee;
+  int *pointee2;
   std::unique_ptr pointer;
 };
+
+std::vector::iterator getIt();
+
+const int &handleGslPtrInitsThroughReference(const std::vector &v) {
+  const auto &it = getIt(); // Ok, it is lifetime extended.
+  return *it;
+}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66059: [clang-format] Expand AllowShortBlocksOnASingleLine for WebKit

2019-08-11 Thread MyDeveloperDay via Phabricator via cfe-commits

MyDeveloperDay added a comment.

LGTM


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66059/new/

https://reviews.llvm.org/D66059



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66035: [WebAssembly] WIP: Add support for reference types

2019-08-11 Thread Heejin Ahn via Phabricator via cfe-commits

aheejin added a comment.

Thank you! We also have been floating some ideas about supporting reference 
types as types in another address space, but haven't actually started 
implementing it AFAIK. I like the general direction. Let's wait for other 
people's comments too.

- Are you planning to land this at some point (after adding some tests and 
such), or is this purely for discussions for the direction? If it's the latter, 
maybe I don't need to comment in the code in too much detail.. But anyway I 
did, so please ignore them if you are not actually planning to land this.
- Even if not to the extent to cover the whole CL, I think a few test cases 
still would be helpful for discussions to see how reference types can be 
represented in .ll files and printed in .s files.
- There are many places that add `anyref` but not `funcref`.
- Out of curiousity, is Julia planning to use reference types? If so, for which 
concept?
- I know it is not finished, but please clang-format if you are gonna land this 
later.
- Maybe you've already seen this, but if we end up landing this, we should 
update the linking spec 
 in the 
tool convention, because we add a new relocation type.




Comment at: lld/wasm/WriterUtils.cpp:186
+  case ValType::FUNCREF:
+return "func";
+  case ValType::ANYREF:

Why not `funcref`?



Comment at: llvm/include/llvm/BinaryFormat/WasmRelocs.def:18
 WASM_RELOC(R_WASM_TABLE_INDEX_REL_SLEB, 12)
+WASM_RELOC(R_WASM_TABLE_INDEX_LEB,  13)

I guess this is for the index to distinguish tables, not elements within the 
table, right? The name is not very distinguishable with 
`R_WASM_TABLE_INDEX_SLEB` and `R_WASM_TABLE_INDEX_I32` above, so I guess we 
need a new name or something



Comment at: llvm/include/llvm/MC/MCExpr.h:296
 VK_WASM_TYPEINDEX, // Reference to a symbol's type (signature)
+VK_WASM_TABLEINDEX,// Reference to a table
 VK_WASM_MBREL, // Memory address relative to memory base

Do we need this? Other symbol kinds (global, function, and events) don't have 
corresponding `VK_WSM_` entries here. `VK_WASM_TYPEINDEX` is left for some 
other reason (D58472).



Comment at: llvm/lib/MC/WasmObjectWriter.cpp:1233
+Import.Kind = wasm::WASM_EXTERNAL_TABLE;
+Import.Table.ElemType = wasm::WASM_TYPE_ANYREF;
+Imports.push_back(Import);

Can't this be other reference types, like `FUNCREF`?



Comment at: llvm/lib/Object/WasmObjectFile.cpp:823
+  if (!isValidTableSymbol(Reloc.Index))
+return make_error("Bad relocation event index",
+  object_error::parse_failed);

event -> table



Comment at: llvm/lib/Object/WasmObjectFile.cpp:998
+if (Tables.back().ElemType != wasm::WASM_TYPE_FUNCREF &&
+// TODO: Only allow anyref here when reference-types is enabled?
+Tables.back().ElemType != wasm::WASM_TYPE_ANYREF) {

Why should we only allow anyref?



Comment at: 
llvm/lib/Target/WebAssembly/MCTargetDesc/WebAssemblyMCTargetDesc.h:51
   OPERAND_GLOBAL,
+  /// Global index.
+  OPERAND_TABLE,

Global -> Table



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp:1125
+OperandFlags = WebAssemblyII::MO_TABLE_INDEX;
+  }
+

It looks it assumes at this point we are not PIC. Maybe we can assert in `if 
(isPositionIndependent()) {` above that nonzero address space is not supported?



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.h:54
+  MVT::getIntegerVT(DL.getPointerSizeInBits(AS));
+  }
+

How are we gonna represent other reference types, such as `funcref` or `exnref`?



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrInfo.td:148
+let OperandType = "OPERAND_TABLE" in
+def table_op : Operand;
+

Not sure what this means. Isn't this the type of table itself, not that of the 
elements?



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrTable.td:5
+ (ins table_op: $table, I32:$offset),
+ (outs), (ins table_op: $table),
+ [], !strconcat(Name, "\t$dst, ${offset}(${table})"),

- Nit: For these two lines: We usually don't seem to have a whitespace after 
`:` for *.td files
- Is the reason for using `offset` instead of `index` here that you plan to use 
named operand table methods?



Comment at: llvm/lib/Target/WebAssembly/WebAssemblyInstrTable.td:10
+
+defm TABLE_GET : WebAssemblyTableGet;
+

I guess the name should be `anyref.table.get` to follow the wasm backend 
convention (even though we don't have separate instructions). And because 
"table.get" part is the const

r368534 - Properly detect temporary gsl::Owners through reference initialization chains.

2019-08-11 Thread Gabor Horvath via cfe-commits

Author: xazax
Date: Sun Aug 11 07:39:42 2019
New Revision: 368534

URL: http://llvm.org/viewvc/llvm-project?rev=368534&view=rev
Log:
Properly detect temporary gsl::Owners through reference initialization chains.

Modified:
cfe/trunk/lib/Sema/SemaInit.cpp
cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp

Modified: cfe/trunk/lib/Sema/SemaInit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368534&r1=368533&r2=368534&view=diff
==
--- cfe/trunk/lib/Sema/SemaInit.cpp (original)
+++ cfe/trunk/lib/Sema/SemaInit.cpp Sun Aug 11 07:39:42 2019
@@ -7104,7 +7104,8 @@ void Sema::checkInitializerLifetime(cons
 SourceLocation DiagLoc = DiagRange.getBegin();
 
 auto *MTE = dyn_cast(L);
-bool IsTempGslOwner = MTE && isRecordWithAttr(MTE->getType());
+bool IsTempGslOwner = MTE && !MTE->getExtendingDecl() &&
+  isRecordWithAttr(MTE->getType());
 bool IsLocalGslOwner =
 isa(L) && isRecordWithAttr(L->getType());
 

Modified: cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp?rev=368534&r1=368533&r2=368534&view=diff
==
--- cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp (original)
+++ cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp Sun Aug 11 07:39:42 
2019
@@ -141,7 +141,7 @@ struct vector {
   typedef basic_iterator iterator;
   iterator begin();
   iterator end();
-  T *data();
+  const T *data() const;
   T &at(int n);
 };
 
@@ -235,8 +235,14 @@ struct X {
 };
 
 std::vector::iterator getIt();
+std::vector getVec();
 
-const int &handleGslPtrInitsThroughReference(const std::vector &v) {
+const int &handleGslPtrInitsThroughReference() {
   const auto &it = getIt(); // Ok, it is lifetime extended.
   return *it;
 }
+
+void handleGslPtrInitsThroughReference2() {
+  const std::vector &v = getVec();
+  const int *val = v.data(); // Ok, it is lifetime extended.
+}


___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D25845: [CUDA] Ignore implicit target attributes during function template instantiation.

2019-08-11 Thread Simon Pilgrim via Phabricator via cfe-commits

RKSimon added inline comments.
Herald added subscribers: llvm-commits, sanjoy.google, bixia.
Herald added a project: LLVM.



Comment at: cfe/trunk/lib/Sema/SemaDecl.cpp:8416
+// in the CheckFunctionTemplateSpecialization() call below.
+if (getLangOpts().CUDA & !isFunctionTemplateSpecialization)
+  maybeAddCUDAHostDeviceAttrs(NewFD, Previous);

@tra Sorry for touching such an old ticket - but cppcheck is warning that we 
are using a boolean result in a bitwise expression - I'm assuming this should 
be:
```
if (getLangOpts().CUDA && !isFunctionTemplateSpecialization)
```


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D25845/new/

https://reviews.llvm.org/D25845



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-11 Thread Joel E. Denny via Phabricator via cfe-commits

jdenny updated this revision to Diff 214559.
jdenny marked 3 inline comments as done.
jdenny edited the summary of this revision.
jdenny set the repository for this revision to rG LLVM Github Monorepo.
jdenny added a comment.

In D65835#1624477 , @ABataev wrote:

> Try something like `target parallel firstprivate (a) map(a)`. Currently it 
> will create a firstprivate copy of the variable a in target context thought 
> it is not required at all. It may lead to increased register pressure and 
> performance degradation.


Thanks.  The only combination that appears to be affected is `firstprivate` and 
`map` for scalar types.  I had only tried arrays and structs earlier, but 
they're not affected. If I had looked a little more closely at the test case 
this patch already introduced, I would have noticed that `int` is affected.  
The problematic analysis is in sema, where there was an apparent assumption 
that `firstprivate` wouldn't appear with `map` due to the previous restriction. 
 This update fixes that.

For my previous update, I meant to point out that I introduced a fixme into the 
test suite.  See the phabricator summary for details.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65835/new/

https://reviews.llvm.org/D65835

Files:
  clang/include/clang/Sema/ScopeInfo.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/Sema.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaOpenMP.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_lastprivate_messages.cpp
  
clang/test/OpenMP/target_teams_distribute_parallel_for_simd_lastprivate_messages.cpp
  clang/test/OpenMP/target_teams_distribute_simd_firstprivate_messages.cpp
  clang/test/OpenMP/target_teams_distribute_simd_lastprivate_messages.cpp
  clang/test/OpenMP/target_teams_distribute_simd_private_messages.cpp
  clang/test/OpenMP/target_teams_map_codegen.cpp
  clang/test/OpenMP/target_teams_map_messages.cpp

Index: clang/test/OpenMP/target_teams_map_messages.cpp
===
--- clang/test/OpenMP/target_teams_map_messages.cpp
+++ clang/test/OpenMP/target_teams_map_messages.cpp
@@ -1,7 +1,10 @@
-// RUN: %clang_cc1 -verify -fopenmp -ferror-limit 200 %s -Wno-openmp-target -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp -ferror-limit 200 %s -Wno-openmp-target -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-version=40 -fopenmp -ferror-limit 200 %s -Wno-openmp-target -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-version=45 -fopenmp -ferror-limit 200 %s -Wno-openmp-target -Wuninitialized
+// RUN: %clang_cc1 -verify=expected -fopenmp-version=50 -fopenmp -ferror-limit 200 %s -Wno-openmp-target -Wuninitialized
 
-// RUN: %clang_cc1 -verify -fopenmp-simd -ferror-limit 200 %s -Wno-openmp-target -Wuninitialized
-// RUN: %clang_cc1 -DCCODE -verify -fopenmp -ferror-limit 200 -x c %s -Wno-openmp-target -Wuninitialized
+// RUN: %clang_cc1 -verify=expected,le45 -fopenmp-simd -ferror-limit 200 %s -Wno-openmp-target -Wuninitialized
+// RUN: %clang_cc1 -DCCODE -verify=expected,le45 -fopenmp -ferror-limit 200 -x c %s -Wno-openmp-target -Wuninitialized
 #ifdef CCODE
 void foo(int arg) {
   const int n = 0;
@@ -536,10 +539,10 @@
 #pragma omp target teams map(tofrom j) // expected-error {{expected ',' or ')' in 'map' clause}}
   foo();
 
-#pragma omp target teams private(j) map(j) // expected-error {{private variable cannot be in a map clause in '#pragma omp target teams' directive}}  expected-note {{defined as private}}
+#pragma omp target teams private(j) map(j) // le45-error {{private variable cannot be in a map clause in '#pragma omp target teams' directive}}  le45-note {{defined as private}}
   {}
 
-#pragma omp target teams firstprivate(j) map(j)  // expected-error {{firstprivate variable cannot be in a map clause in '#pragma omp target teams' directive}} expected-note {{defined as firstprivate}}
+#pragma omp target teams firstprivate(j) map(j) // le45-error {{firstprivate variable cannot be in a map clause in '#pragma omp target teams' directive}} le45-note {{defined as firstprivate}}
   {}
 
 #pragma omp target teams map(m)
Index: clang/test/OpenMP/target_teams_map_codegen.cpp
===
--- /dev/null
+++ clang/test/OpenMP/target_teams_map_codegen.cpp
@@ -0,0 +1,64 @@
+// Test host codegen.
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=50 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -o - | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -std=c++11 -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -std=c++11 -include-pch %t -verify %s -emit

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-11 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added a comment.

In D65835#1624610 , @jdenny wrote:

> In D65835#1624477 , @ABataev wrote:
>
> > Try something like `target parallel firstprivate (a) map(a)`. Currently it 
> > will create a firstprivate copy of the variable a in target context thought 
> > it is not required at all. It may lead to increased register pressure and 
> > performance degradation.
>
>
> Thanks.  The only combination that appears to be affected is `firstprivate` 
> and `map` for scalar types.  I had only tried arrays and structs earlier, but 
> they're not affected. If I had looked a little more closely at the test case 
> this patch already introduced, I would have noticed that `int` is affected.  
> The problematic analysis is in sema, where there was an apparent assumption 
> that `firstprivate` wouldn't appear with `map` due to the previous 
> restriction.  This update fixes that.
>
> For my previous update, I meant to point out that I introduced a fixme into 
> the test suite.  See the phabricator summary for details.


This is wrong. It affects all possible combinations and not only fof scalar 
types, all of them are affected.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65835/new/

https://reviews.llvm.org/D65835



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-11 Thread Joel E. Denny via Phabricator via cfe-commits

jdenny added a comment.

In D65835#1624615 , @ABataev wrote:

> This is wrong. It affects all possible combinations and not only fof scalar 
> types, all of them are affected.


Are you saying the patch isn't sufficient because other types need to be fixed 
too?  Can you give an example?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65835/new/

https://reviews.llvm.org/D65835



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-11 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added a comment.

In D65835#1624616 , @jdenny wrote:

> In D65835#1624615 , @ABataev wrote:
>
> > This is wrong. It affects all possible combinations and not only fof scalar 
> > types, all of them are affected.
>
>
> Are you saying the patch isn't sufficient because other types need to be 
> fixed too?  Can you give an example?


Target teams private map will produce extra private in target context, other 
constructs either.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65835/new/

https://reviews.llvm.org/D65835



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-11 Thread Joel E. Denny via Phabricator via cfe-commits

jdenny added a comment.

In D65835#1624617 , @ABataev wrote:

> Target teams private map will produce extra private in target context, other 
> constructs either.


Here's what I tried:

  int a;
  #pragma omp target teams private(a) map(a)
;

The same code is generated as for:

  int a;
  #pragma omp target map(a)
  #pragma omp teams private(a)
;

I haven't debugged it yet, but it seems orthogonal to whether you have a 
combined directive, which is what this patch is about.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65835/new/

https://reviews.llvm.org/D65835



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65149: [Format] Add test demonstrating PR42722

2019-08-11 Thread Brian Gesiak via Phabricator via cfe-commits

modocache added inline comments.



Comment at: clang/unittests/Format/FormatTest.cpp:5184
+   "c++;\n"
+   "d++\n"
+   "  });\n"

MyDeveloperDay wrote:
> modocache wrote:
> > This is a passing test that demonstrates that the indentation of `c++` and 
> > `d++` here is wacky.
> in what way do you think its incorrect? change the test to show what it 
> should be...I think its better to commit a fix and a test at the same time 
> then its easier for the reviewer and others to understand what you are 
> driving at.
The linked bug report, https://bugs.llvm.org/show_bug.cgi?id=42722, describes 
what I think the indentation of these two lines should be. I apologize, I 
should have included the actual code snippet in the commit message here. I am 
curious if you think the indentation in that bug report is desirable, though, 
so please let me know!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65149/new/

https://reviews.llvm.org/D65149



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r368539 - [clang-format] Expand AllowShortBlocksOnASingleLine for WebKit

2019-08-11 Thread Owen Pan via cfe-commits

Author: owenpan
Date: Sun Aug 11 10:48:36 2019
New Revision: 368539

URL: http://llvm.org/viewvc/llvm-project?rev=368539&view=rev
Log:
[clang-format] Expand AllowShortBlocksOnASingleLine for WebKit

See PR40840

Differential Revision: https://reviews.llvm.org/D66059

Modified:
cfe/trunk/docs/ClangFormatStyleOptions.rst
cfe/trunk/include/clang/Format/Format.h
cfe/trunk/lib/Format/Format.cpp
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/docs/ClangFormatStyleOptions.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ClangFormatStyleOptions.rst?rev=368539&r1=368538&r2=368539&view=diff
==
--- cfe/trunk/docs/ClangFormatStyleOptions.rst (original)
+++ cfe/trunk/docs/ClangFormatStyleOptions.rst Sun Aug 11 10:48:36 2019
@@ -344,10 +344,42 @@ the configuration (without a prefix: ``A
 int d,
 int e);
 
-**AllowShortBlocksOnASingleLine** (``bool``)
-  Allows contracting simple braced statements to a single line.
+**AllowShortBlocksOnASingleLine** (``ShortBlockStyle``)
+  Dependent on the value, ``while (true) { continue; }`` can be put on a
+  single line.
+
+  Possible values:
+
+  * ``SBS_Never`` (in configuration: ``Never``)
+Never merge blocks into a single line.
+
+.. code-block:: c++
+
+  while (true) {
+  }
+  while (true) {
+continue;
+  }
+
+  * ``SBS_Empty`` (in configuration: ``Empty``)
+Only merge empty blocks.
+
+.. code-block:: c++
+
+  while (true) {}
+  while (true) {
+continue;
+  }
+
+  * ``SBS_Always`` (in configuration: ``Always``)
+Always merge short blocks into a single line.
+
+.. code-block:: c++
+
+  while (true) {}
+  while (true) { continue; }
+
 
-  E.g., this allows ``if (a) { return; }`` to be put on a single line.
 
 **AllowShortCaseLabelsOnASingleLine** (``bool``)
   If ``true``, short case labels will be contracted to a single line.

Modified: cfe/trunk/include/clang/Format/Format.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Format/Format.h?rev=368539&r1=368538&r2=368539&view=diff
==
--- cfe/trunk/include/clang/Format/Format.h (original)
+++ cfe/trunk/include/clang/Format/Format.h Sun Aug 11 10:48:36 2019
@@ -216,10 +216,37 @@ struct FormatStyle {
   /// \endcode
   bool AllowAllParametersOfDeclarationOnNextLine;
 
-  /// Allows contracting simple braced statements to a single line.
-  ///
-  /// E.g., this allows ``if (a) { return; }`` to be put on a single line.
-  bool AllowShortBlocksOnASingleLine;
+  /// Different styles for merging short blocks containing at most one
+  /// statement.
+  enum ShortBlockStyle {
+/// Never merge blocks into a single line.
+/// \code
+///   while (true) {
+///   }
+///   while (true) {
+/// continue;
+///   }
+/// \endcode
+SBS_Never,
+/// Only merge empty blocks.
+/// \code
+///   while (true) {}
+///   while (true) {
+/// continue;
+///   }
+/// \endcode
+SBS_Empty,
+/// Always merge short blocks into a single line.
+/// \code
+///   while (true) {}
+///   while (true) { continue; }
+/// \endcode
+SBS_Always,
+  };
+
+  /// Dependent on the value, ``while (true) { continue; }`` can be put on a
+  /// single line.
+  ShortBlockStyle AllowShortBlocksOnASingleLine;
 
   /// If ``true``, short case labels will be contracted to a single line.
   /// \code

Modified: cfe/trunk/lib/Format/Format.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=368539&r1=368538&r2=368539&view=diff
==
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Sun Aug 11 10:48:36 2019
@@ -95,6 +95,16 @@ template <> struct ScalarEnumerationTrai
   }
 };
 
+template <> struct ScalarEnumerationTraits {
+  static void enumeration(IO &IO, FormatStyle::ShortBlockStyle &Value) {
+IO.enumCase(Value, "Never", FormatStyle::SBS_Never);
+IO.enumCase(Value, "false", FormatStyle::SBS_Never);
+IO.enumCase(Value, "Always", FormatStyle::SBS_Always);
+IO.enumCase(Value, "true", FormatStyle::SBS_Always);
+IO.enumCase(Value, "Empty", FormatStyle::SBS_Empty);
+  }
+};
+
 template <> struct ScalarEnumerationTraits {
   static void enumeration(IO &IO, FormatStyle::ShortFunctionStyle &Value) {
 IO.enumCase(Value, "None", FormatStyle::SFS_None);
@@ -674,7 +684,7 @@ FormatStyle getLLVMStyle(FormatStyle::La
   LLVMStyle.AllowAllConstructorInitializersOnNextLine = true;
   LLVMStyle.AllowAllParametersOfDeclarationOnNextLine = true;
   LLVMStyle.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_All;
-  LLVMStyle.AllowShortBloc

[PATCH] D66059: [clang-format] Expand AllowShortBlocksOnASingleLine for WebKit

2019-08-11 Thread Owen Pan via Phabricator via cfe-commits

This revision was automatically updated to reflect the committed changes.
Closed by commit rL368539: [clang-format] Expand AllowShortBlocksOnASingleLine 
for WebKit (authored by owenpan, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D66059?vs=214547&id=214560#toc

Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66059/new/

https://reviews.llvm.org/D66059

Files:
  cfe/trunk/docs/ClangFormatStyleOptions.rst
  cfe/trunk/include/clang/Format/Format.h
  cfe/trunk/lib/Format/Format.cpp
  cfe/trunk/lib/Format/TokenAnnotator.cpp
  cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -561,7 +561,8 @@
 TEST_F(FormatTest, FormatShortBracedStatements) {
   FormatStyle AllowSimpleBracedStatements = getLLVMStyle();
   AllowSimpleBracedStatements.ColumnLimit = 40;
-  AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine = true;
+  AllowSimpleBracedStatements.AllowShortBlocksOnASingleLine =
+  FormatStyle::SBS_Always;
 
   AllowSimpleBracedStatements.AllowShortIfStatementsOnASingleLine =
   FormatStyle::SIS_WithoutElse;
@@ -714,7 +715,7 @@
 
 TEST_F(FormatTest, ShortBlocksInMacrosDontMergeWithCodeAfterMacro) {
   FormatStyle Style = getLLVMStyleWithColumns(60);
-  Style.AllowShortBlocksOnASingleLine = true;
+  Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Always;
   Style.AllowShortIfStatementsOnASingleLine = FormatStyle::SIS_WithoutElse;
   Style.BreakBeforeBraces = FormatStyle::BS_Allman;
   EXPECT_EQ("#define A  \\\n"
@@ -1163,7 +1164,7 @@
 
   FormatStyle Style = getLLVMStyle();
   Style.IndentCaseLabels = true;
-  Style.AllowShortBlocksOnASingleLine = false;
+  Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Never;
   Style.BreakBeforeBraces = FormatStyle::BS_Custom;
   Style.BraceWrapping.AfterCaseLabel = true;
   Style.BraceWrapping.AfterControlStatement = true;
@@ -3692,10 +3693,10 @@
   EXPECT_EQ("{}", format("{}"));
   verifyFormat("enum E {};");
   verifyFormat("enum E {}");
-  EXPECT_EQ("void f() { }", format("void f() {}", getWebKitStyle()));
   FormatStyle Style = getLLVMStyle();
-  Style.AllowShortBlocksOnASingleLine = true;
   Style.SpaceInEmptyBlock = true;
+  EXPECT_EQ("void f() { }", format("void f() {}", Style));
+  Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;
   EXPECT_EQ("while (true) { }", format("while (true) {}", Style));
 }
 
@@ -11745,7 +11746,6 @@
   CHECK_PARSE_BOOL(AllowAllArgumentsOnNextLine);
   CHECK_PARSE_BOOL(AllowAllConstructorInitializersOnNextLine);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
-  CHECK_PARSE_BOOL(AllowShortBlocksOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortCaseLabelsOnASingleLine);
   CHECK_PARSE_BOOL(AllowShortLoopsOnASingleLine);
   CHECK_PARSE_BOOL(BinPackArguments);
@@ -11920,6 +11920,19 @@
   CHECK_PARSE("UseTab: false", UseTab, FormatStyle::UT_Never);
   CHECK_PARSE("UseTab: true", UseTab, FormatStyle::UT_Always);
 
+  Style.AllowShortBlocksOnASingleLine = FormatStyle::SBS_Empty;
+  CHECK_PARSE("AllowShortBlocksOnASingleLine: Never",
+  AllowShortBlocksOnASingleLine, FormatStyle::SBS_Never);
+  CHECK_PARSE("AllowShortBlocksOnASingleLine: Empty",
+  AllowShortBlocksOnASingleLine, FormatStyle::SBS_Empty);
+  CHECK_PARSE("AllowShortBlocksOnASingleLine: Always",
+  AllowShortBlocksOnASingleLine, FormatStyle::SBS_Always);
+  // For backward compatibility:
+  CHECK_PARSE("AllowShortBlocksOnASingleLine: false",
+  AllowShortBlocksOnASingleLine, FormatStyle::SBS_Never);
+  CHECK_PARSE("AllowShortBlocksOnASingleLine: true",
+  AllowShortBlocksOnASingleLine, FormatStyle::SBS_Always);
+
   Style.AllowShortFunctionsOnASingleLine = FormatStyle::SFS_Inline;
   CHECK_PARSE("AllowShortFunctionsOnASingleLine: None",
   AllowShortFunctionsOnASingleLine, FormatStyle::SFS_None);
@@ -12527,6 +12540,14 @@
   // Allow functions on a single line.
   verifyFormat("void f() { return; }", Style);
 
+  // Allow empty blocks on a single line and insert a space in empty blocks.
+  EXPECT_EQ("void f() { }", format("void f() {}", Style));
+  EXPECT_EQ("while (true) { }", format("while (true) {}", Style));
+  // However, don't merge non-empty short loops.
+  EXPECT_EQ("while (true) {\n"
+"continue;\n"
+"}", format("while (true) { continue; }", Style));
+
   // Constructor initializers are formatted one per line with the "," on the
   // new line.
   verifyFormat("Constructor()\n"
@@ -13033,7 +13054,7 @@
 
 TEST_F(FormatTest, FormatsBlocks) {
   FormatStyle ShortBlocks = getLLVMStyle();
-  ShortBlocks.AllowShortBlocks

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-11 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added a comment.

In D65835#1624619 , @jdenny wrote:

> In D65835#1624617 , @ABataev wrote:
>
> > Target teams private map will produce extra private in target context, 
> > other constructs either.
>
>
> Here's what I tried:
>
>   int a;
>   #pragma omp target teams private(a) map(a)
> ;
>
>
> The same code is generated as for:
>
>   int a;
>   #pragma omp target map(a)
>   #pragma omp teams private(a)
> ;
>
>
> I haven't debugged it yet, but it seems orthogonal to whether you have a 
> combined directive, which is what this patch is about.


Did you check the mapping flags, generated during host codegen? They must be 
the same. With private clause it may generate just map(alloc) instead of 
map(tofrom).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65835/new/

https://reviews.llvm.org/D65835



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-11 Thread Joel E. Denny via Phabricator via cfe-commits

jdenny added a comment.

In D65835#1624624 , @ABataev wrote:

> In D65835#1624619 , @jdenny wrote:
>
> > In D65835#1624617 , @ABataev wrote:
> >
> > > Target teams private map will produce extra private in target context, 
> > > other constructs either.
> >
> >
> > Here's what I tried:
> >
> >   int a;
> >   #pragma omp target teams private(a) map(a)
> > ;
> >
> >
> > The same code is generated as for:
> >
> >   int a;
> >   #pragma omp target map(a)
> >   #pragma omp teams private(a)
> > ;
> >
> >
> > I haven't debugged it yet, but it seems orthogonal to whether you have a 
> > combined directive, which is what this patch is about.
>
>
> Did you check the mapping flags, generated during host codegen? They must be 
> the same. With private clause it may generate just map(alloc) instead of 
> map(tofrom).


I diffed the `.ll` files for combined vs. separate constructs.  The only 
difference is the file ID.  `@.offload_maptypes` isn't generated in either (but 
it is if I replace `private` with `firstprivate`).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65835/new/

https://reviews.llvm.org/D65835



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r368446 - More warnings regarding gsl::Pointer and gsl::Owner attributes

2019-08-11 Thread Gábor Horváth via cfe-commits

This should be fixed now.

On Sat, Aug 10, 2019, 8:08 PM Gábor Horváth  wrote:

> You are right!
>
> I will look into this tomorrow. If you think this is urgent feel free to
> revert the commits.
>
> Cheers,
> Gabor
>
> On Sat, Aug 10, 2019, 6:41 PM Nico Weber  wrote:
>
>> Hi Gabor,
>>
>> this makes clang warn on this program:
>>
>> $ cat test.cc
>> #include 
>> #include 
>>
>> struct S {
>>   bool operator==(const S &rhs) const;
>> };
>>
>> const S &f(const std::vector &v) {
>>   const auto &it = std::find(v.rbegin(), v.rend(), S());
>>   return *it;
>> }
>>
>>
>> $ out/gn/bin/clang -c test.cc -isysroot $(xcrun -show-sdk-path) -isystem
>> libcxx/include/ -Wall
>> test.cc:10:11: warning: returning reference to local temporary object
>> [-Wreturn-stack-address]
>>   return *it;
>>   ^~
>> test.cc:9:15: note: binding reference variable 'it' here
>>   const auto &it = std::find(v.rbegin(), v.rend(), S());
>>   ^
>> 1 warning generated.
>>
>>
>> Is the warning correct here? The `const auto &it` is lifetime-extended to
>> the end of the block, and *it should return a reference to an element in
>> the vector. Since the vector's passed in, this should be fine. Or am I
>> missing something?
>>
>> Thanks,
>> Nico
>>
>> On Fri, Aug 9, 2019 at 11:15 AM Gabor Horvath via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: xazax
>>> Date: Fri Aug  9 08:16:35 2019
>>> New Revision: 368446
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=368446&view=rev
>>> Log:
>>> More warnings regarding gsl::Pointer and gsl::Owner attributes
>>>
>>> Differential Revision: https://reviews.llvm.org/D65120
>>>
>>> Modified:
>>> cfe/trunk/lib/Sema/SemaInit.cpp
>>> cfe/trunk/test/Analysis/inner-pointer.cpp
>>> cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
>>>
>>> Modified: cfe/trunk/lib/Sema/SemaInit.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368446&r1=368445&r2=368446&view=diff
>>>
>>> ==
>>> --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
>>> +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Aug  9 08:16:35 2019
>>> @@ -6564,6 +6564,25 @@ template  static bool isReco
>>>return false;
>>>  }
>>>
>>> +static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
>>> +  if (auto *Conv = dyn_cast_or_null(Callee))
>>> +if (isRecordWithAttr(Conv->getConversionType()))
>>> +  return true;
>>> +  if (!Callee->getParent()->isInStdNamespace() ||
>>> !Callee->getIdentifier())
>>> +return false;
>>> +  if (!isRecordWithAttr(Callee->getThisObjectType()) &&
>>> +  !isRecordWithAttr(Callee->getThisObjectType()))
>>> +return false;
>>> +  if (!isRecordWithAttr(Callee->getReturnType()) &&
>>> +  !Callee->getReturnType()->isPointerType())
>>> +return false;
>>> +  return llvm::StringSwitch(Callee->getName())
>>> +  .Cases("begin", "rbegin", "cbegin", "crbegin", true)
>>> +  .Cases("end", "rend", "cend", "crend", true)
>>> +  .Cases("c_str", "data", "get", true)
>>> +  .Default(false);
>>> +}
>>> +
>>>  static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr *Call,
>>>  LocalVisitor Visit) {
>>>auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
>>> @@ -6577,10 +6596,9 @@ static void handleGslAnnotatedTypes(Indi
>>>};
>>>
>>>if (auto *MCE = dyn_cast(Call)) {
>>> -const FunctionDecl *Callee = MCE->getDirectCallee();
>>> -if (auto *Conv = dyn_cast_or_null(Callee))
>>> -  if (isRecordWithAttr(Conv->getConversionType()))
>>> -VisitPointerArg(Callee, MCE->getImplicitObjectArgument());
>>> +const auto *MD =
>>> cast_or_null(MCE->getDirectCallee());
>>> +if (MD && shouldTrackImplicitObjectArg(MD))
>>> +  VisitPointerArg(MD, MCE->getImplicitObjectArgument());
>>>  return;
>>>}
>>>
>>>
>>> Modified: cfe/trunk/test/Analysis/inner-pointer.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inner-pointer.cpp?rev=368446&r1=368445&r2=368446&view=diff
>>>
>>> ==
>>> --- cfe/trunk/test/Analysis/inner-pointer.cpp (original)
>>> +++ cfe/trunk/test/Analysis/inner-pointer.cpp Fri Aug  9 08:16:35 2019
>>> @@ -1,4 +1,5 @@
>>> -// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer \
>>> +// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer   \
>>> +// RUN:   -Wno-dangling -Wno-dangling-field -Wno-return-stack-address \
>>>  // RUN:   %s -analyzer-output=text -verify
>>>
>>>  #include "Inputs/system-header-simulator-cxx.h"
>>>
>>> Modified: cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp?rev=368446&r1=368445&r2=368446&view=diff
>>>
>>> ===

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-11 Thread Alexey Bataev via Phabricator via cfe-commits

ABataev added a comment.

In D65835#1624629 , @jdenny wrote:

> In D65835#1624624 , @ABataev wrote:
>
> > In D65835#1624619 , @jdenny wrote:
> >
> > > In D65835#1624617 , @ABataev 
> > > wrote:
> > >
> > > > Target teams private map will produce extra private in target context, 
> > > > other constructs either.
> > >
> > >
> > > Here's what I tried:
> > >
> > >   int a;
> > >   #pragma omp target teams private(a) map(a)
> > > ;
> > >
> > >
> > > The same code is generated as for:
> > >
> > >   int a;
> > >   #pragma omp target map(a)
> > >   #pragma omp teams private(a)
> > > ;
> > >
> > >
> > > I haven't debugged it yet, but it seems orthogonal to whether you have a 
> > > combined directive, which is what this patch is about.
> >
> >
> > Did you check the mapping flags, generated during host codegen? They must 
> > be the same. With private clause it may generate just map(alloc) instead of 
> > map(tofrom).
>
>
> I diffed the `.ll` files for combined vs. separate constructs.  The only 
> difference is the file ID.  `@.offload_maptypes` isn't generated in either 
> (but it is if I replace `private` with `firstprivate`).


Maptypes array must be generated in all cases, check the host codegen.
Also, test codegen with the different kinds of maptypes, not only to from,  but 
also alloc, to, from, etc. Yoh will see the difference in many cases and 2ith 
many kinds of types, not only scalars.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65835/new/

https://reviews.llvm.org/D65835



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65935: [ASTImporter] Import ctor initializers after setting flags.

2019-08-11 Thread Aleksei Sidorin via Phabricator via cfe-commits

a_sidorin added a comment.

Hello Balazs,
Do I understand correctly that it was unset 
`ToFunction->setLexicalDeclContext(LexicalDC);` that caused lookup problems?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65935/new/

https://reviews.llvm.org/D65935



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

2019-08-11 Thread Kristóf Umann via Phabricator via cfe-commits

Szelethus added reviewers: gamesh411, NoQ.
Szelethus added subscribers: gamesh411, NoQ.
Szelethus added a comment.

+ @gamesh411 as he took over this checker before I commited it on his behalf, 
+@NoQ because he is far more knowledgeable about this part of the analyzer.




Comment at: 
clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:121-122
   // Check if any of the enum values possibly match.
   bool PossibleValueMatch = llvm::any_of(
   DeclValues, ConstraintBasedEQEvaluator(C, *ValueToCast));
 

So this is where the assertion comes from, and will eventually lead to 
`SValBuilder::evalEQ`, which calls `SValBuilder::evalBinOp`, where this will 
fire on line 427:
```
assert(op == BO_Add);
```
Seems like this happens because `unused`'s value in your testcase will be 
retrieved as a `Loc`, while the values in the enum are (correctly) `NonLoc`, 
and `SValBuilder::evalBinOp` thinks this is some sort of pointer arithmetic (`5 
+ ptr` etc).

How about instead of checking for LValueToRValue cast, we check whether 
`ValueToCast` is `Loc`, and bail out if so? That sounds like a more general 
solution, but I didn't sit atop of this for hours.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66014/new/

https://reviews.llvm.org/D66014



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65999: [ASTImporter] Import additional flags for functions.

2019-08-11 Thread Aleksei Sidorin via Phabricator via cfe-commits

a_sidorin added a comment.

Hello Balazs,
The patch looks good in general.




Comment at: clang/unittests/AST/ASTImporterTest.cpp:5239
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportOfDefaultImplicitFunctions) {
+  // Test that import of implicit functions works and the functions

balazske wrote:
> martong wrote:
> > I don't exactly see how this test is related.
> I do not remember exactly why this test was added but probably the problem is 
> structural equivalence related: The flags are not imported correctly for the 
> first time, and at the second import structural match fails and a new Decl is 
> created instead of returning the existing one. This test fails if the change 
> is not applied.
Should we consider isExplicitlyDefaulted() when computing structural 
equivalence?



Comment at: clang/unittests/AST/ASTImporterTest.cpp:5242
+  // are merged into one chain.
+  auto GetDeclToImport = [this](const char *File) {
+Decl *FromTU = getTuDecl(

`const char *` -> `StringRef`?



Comment at: clang/unittests/AST/ASTImporterTest.cpp:5246
+struct X { };
+void f() { X x1, x2; x1 = x2; X *x3 = new X; delete x3; }
+)",

Can we remove the function body or reduce it to 'X x'?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65999/new/

https://reviews.llvm.org/D65999



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

2019-08-11 Thread Joel E. Denny via Phabricator via cfe-commits

jdenny added a comment.

In D65835#1624659 , @ABataev wrote:

> In D65835#1624629 , @jdenny wrote:
>
> > In D65835#1624624 , @ABataev wrote:
> >
> > > In D65835#1624619 , @jdenny 
> > > wrote:
> > >
> > > > In D65835#1624617 , @ABataev 
> > > > wrote:
> > > >
> > > > > Target teams private map will produce extra private in target 
> > > > > context, other constructs either.
> > > >
> > > >
> > > > Here's what I tried:
> > > >
> > > >   int a;
> > > >   #pragma omp target teams private(a) map(a)
> > > > ;
> > > >
> > > >
> > > > The same code is generated as for:
> > > >
> > > >   int a;
> > > >   #pragma omp target map(a)
> > > >   #pragma omp teams private(a)
> > > > ;
> > > >
> > > >
> > > > I haven't debugged it yet, but it seems orthogonal to whether you have 
> > > > a combined directive, which is what this patch is about.
> > >
> > >
> > > Did you check the mapping flags, generated during host codegen? They must 
> > > be the same. With private clause it may generate just map(alloc) instead 
> > > of map(tofrom).
> >
> >
> > I diffed the `.ll` files for combined vs. separate constructs.  The only 
> > difference is the file ID.  `@.offload_maptypes` isn't generated in either 
> > (but it is if I replace `private` with `firstprivate`).
>
>
> Maptypes array must be generated in all cases, check the host codegen.
>  Also, test codegen with the different kinds of maptypes, not only to from,  
> but also alloc, to, from, etc. Yoh will see the difference in many cases and 
> 2ith many kinds of types, not only scalars.


I'll work on another patch for the bugs where directives are separate, and I'll 
return to this patch if more fixes are then needed to support combined 
directives consistently.  Thanks.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65835/new/

https://reviews.llvm.org/D65835



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65269: [ASTImporter] Fix for import of friend class template with definition.

2019-08-11 Thread Aleksei Sidorin via Phabricator via cfe-commits

a_sidorin accepted this revision.
a_sidorin added a comment.
This revision is now accepted and ready to land.

LGTM, thanks!


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65269/new/

https://reviews.llvm.org/D65269



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65373: [clangd] Update features table in the docs with links to LSP extension proposals

2019-08-11 Thread Nathan Ridge via Phabricator via cfe-commits

nridge added a comment.

Review ping :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65373/new/

https://reviews.llvm.org/D65373



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r368446 - More warnings regarding gsl::Pointer and gsl::Owner attributes

2019-08-11 Thread Nico Weber via cfe-commits

Confirmed, after r368534 the Chromium builds looks good again. (r368528
without r368534 regressed something else, but r368534 fixed things. I'm
assuming you don't need a repro for the breakage in r368528
without r368534.)

Your warnings even found a bug:
https://bugs.chromium.org/p/openscreen/issues/detail?id=63 Thanks! :)

On Sun, Aug 11, 2019 at 3:18 PM Gábor Horváth  wrote:

> This should be fixed now.
>
> On Sat, Aug 10, 2019, 8:08 PM Gábor Horváth  wrote:
>
>> You are right!
>>
>> I will look into this tomorrow. If you think this is urgent feel free to
>> revert the commits.
>>
>> Cheers,
>> Gabor
>>
>> On Sat, Aug 10, 2019, 6:41 PM Nico Weber  wrote:
>>
>>> Hi Gabor,
>>>
>>> this makes clang warn on this program:
>>>
>>> $ cat test.cc
>>> #include 
>>> #include 
>>>
>>> struct S {
>>>   bool operator==(const S &rhs) const;
>>> };
>>>
>>> const S &f(const std::vector &v) {
>>>   const auto &it = std::find(v.rbegin(), v.rend(), S());
>>>   return *it;
>>> }
>>>
>>>
>>> $ out/gn/bin/clang -c test.cc -isysroot $(xcrun -show-sdk-path) -isystem
>>> libcxx/include/ -Wall
>>> test.cc:10:11: warning: returning reference to local temporary object
>>> [-Wreturn-stack-address]
>>>   return *it;
>>>   ^~
>>> test.cc:9:15: note: binding reference variable 'it' here
>>>   const auto &it = std::find(v.rbegin(), v.rend(), S());
>>>   ^
>>> 1 warning generated.
>>>
>>>
>>> Is the warning correct here? The `const auto &it` is lifetime-extended
>>> to the end of the block, and *it should return a reference to an element in
>>> the vector. Since the vector's passed in, this should be fine. Or am I
>>> missing something?
>>>
>>> Thanks,
>>> Nico
>>>
>>> On Fri, Aug 9, 2019 at 11:15 AM Gabor Horvath via cfe-commits <
>>> cfe-commits@lists.llvm.org> wrote:
>>>
 Author: xazax
 Date: Fri Aug  9 08:16:35 2019
 New Revision: 368446

 URL: http://llvm.org/viewvc/llvm-project?rev=368446&view=rev
 Log:
 More warnings regarding gsl::Pointer and gsl::Owner attributes

 Differential Revision: https://reviews.llvm.org/D65120

 Modified:
 cfe/trunk/lib/Sema/SemaInit.cpp
 cfe/trunk/test/Analysis/inner-pointer.cpp
 cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp

 Modified: cfe/trunk/lib/Sema/SemaInit.cpp
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368446&r1=368445&r2=368446&view=diff

 ==
 --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
 +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Aug  9 08:16:35 2019
 @@ -6564,6 +6564,25 @@ template  static bool isReco
return false;
  }

 +static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee) {
 +  if (auto *Conv = dyn_cast_or_null(Callee))
 +if (isRecordWithAttr(Conv->getConversionType()))
 +  return true;
 +  if (!Callee->getParent()->isInStdNamespace() ||
 !Callee->getIdentifier())
 +return false;
 +  if (!isRecordWithAttr(Callee->getThisObjectType()) &&
 +  !isRecordWithAttr(Callee->getThisObjectType()))
 +return false;
 +  if (!isRecordWithAttr(Callee->getReturnType()) &&
 +  !Callee->getReturnType()->isPointerType())
 +return false;
 +  return llvm::StringSwitch(Callee->getName())
 +  .Cases("begin", "rbegin", "cbegin", "crbegin", true)
 +  .Cases("end", "rend", "cend", "crend", true)
 +  .Cases("c_str", "data", "get", true)
 +  .Default(false);
 +}
 +
  static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr
 *Call,
  LocalVisitor Visit) {
auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
 @@ -6577,10 +6596,9 @@ static void handleGslAnnotatedTypes(Indi
};

if (auto *MCE = dyn_cast(Call)) {
 -const FunctionDecl *Callee = MCE->getDirectCallee();
 -if (auto *Conv = dyn_cast_or_null(Callee))
 -  if (isRecordWithAttr(Conv->getConversionType()))
 -VisitPointerArg(Callee, MCE->getImplicitObjectArgument());
 +const auto *MD =
 cast_or_null(MCE->getDirectCallee());
 +if (MD && shouldTrackImplicitObjectArg(MD))
 +  VisitPointerArg(MD, MCE->getImplicitObjectArgument());
  return;
}


 Modified: cfe/trunk/test/Analysis/inner-pointer.cpp
 URL:
 http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inner-pointer.cpp?rev=368446&r1=368445&r2=368446&view=diff

 ==
 --- cfe/trunk/test/Analysis/inner-pointer.cpp (original)
 +++ cfe/trunk/test/Analysis/inner-pointer.cpp Fri Aug  9 08:16:35 2019
 @@ -1,4 +1,5 @@
 -// RUN: %clang_analyze_cc1 -analyzer-checker=cplusplus.InnerPointer \
 +// RUN: %

Re: r368446 - More warnings regarding gsl::Pointer and gsl::Owner attributes

2019-08-11 Thread Gábor Horváth via cfe-commits

Great news!

Thanks for all the feedback and patience! :)

On Sun, Aug 11, 2019, 4:35 PM Nico Weber  wrote:

> Confirmed, after r368534 the Chromium builds looks good again. (r368528
> without r368534 regressed something else, but r368534 fixed things. I'm
> assuming you don't need a repro for the breakage in r368528
> without r368534.)
>
> Your warnings even found a bug:
> https://bugs.chromium.org/p/openscreen/issues/detail?id=63 Thanks! :)
>
> On Sun, Aug 11, 2019 at 3:18 PM Gábor Horváth  wrote:
>
>> This should be fixed now.
>>
>> On Sat, Aug 10, 2019, 8:08 PM Gábor Horváth  wrote:
>>
>>> You are right!
>>>
>>> I will look into this tomorrow. If you think this is urgent feel free to
>>> revert the commits.
>>>
>>> Cheers,
>>> Gabor
>>>
>>> On Sat, Aug 10, 2019, 6:41 PM Nico Weber  wrote:
>>>
 Hi Gabor,

 this makes clang warn on this program:

 $ cat test.cc
 #include 
 #include 

 struct S {
   bool operator==(const S &rhs) const;
 };

 const S &f(const std::vector &v) {
   const auto &it = std::find(v.rbegin(), v.rend(), S());
   return *it;
 }


 $ out/gn/bin/clang -c test.cc -isysroot $(xcrun -show-sdk-path)
 -isystem libcxx/include/ -Wall
 test.cc:10:11: warning: returning reference to local temporary object
 [-Wreturn-stack-address]
   return *it;
   ^~
 test.cc:9:15: note: binding reference variable 'it' here
   const auto &it = std::find(v.rbegin(), v.rend(), S());
   ^
 1 warning generated.


 Is the warning correct here? The `const auto &it` is lifetime-extended
 to the end of the block, and *it should return a reference to an element in
 the vector. Since the vector's passed in, this should be fine. Or am I
 missing something?

 Thanks,
 Nico

 On Fri, Aug 9, 2019 at 11:15 AM Gabor Horvath via cfe-commits <
 cfe-commits@lists.llvm.org> wrote:

> Author: xazax
> Date: Fri Aug  9 08:16:35 2019
> New Revision: 368446
>
> URL: http://llvm.org/viewvc/llvm-project?rev=368446&view=rev
> Log:
> More warnings regarding gsl::Pointer and gsl::Owner attributes
>
> Differential Revision: https://reviews.llvm.org/D65120
>
> Modified:
> cfe/trunk/lib/Sema/SemaInit.cpp
> cfe/trunk/test/Analysis/inner-pointer.cpp
> cfe/trunk/test/Sema/warn-lifetime-analysis-nocfg.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaInit.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaInit.cpp?rev=368446&r1=368445&r2=368446&view=diff
>
> ==
> --- cfe/trunk/lib/Sema/SemaInit.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaInit.cpp Fri Aug  9 08:16:35 2019
> @@ -6564,6 +6564,25 @@ template  static bool isReco
>return false;
>  }
>
> +static bool shouldTrackImplicitObjectArg(const CXXMethodDecl *Callee)
> {
> +  if (auto *Conv = dyn_cast_or_null(Callee))
> +if (isRecordWithAttr(Conv->getConversionType()))
> +  return true;
> +  if (!Callee->getParent()->isInStdNamespace() ||
> !Callee->getIdentifier())
> +return false;
> +  if (!isRecordWithAttr(Callee->getThisObjectType()) &&
> +  !isRecordWithAttr(Callee->getThisObjectType()))
> +return false;
> +  if (!isRecordWithAttr(Callee->getReturnType()) &&
> +  !Callee->getReturnType()->isPointerType())
> +return false;
> +  return llvm::StringSwitch(Callee->getName())
> +  .Cases("begin", "rbegin", "cbegin", "crbegin", true)
> +  .Cases("end", "rend", "cend", "crend", true)
> +  .Cases("c_str", "data", "get", true)
> +  .Default(false);
> +}
> +
>  static void handleGslAnnotatedTypes(IndirectLocalPath &Path, Expr
> *Call,
>  LocalVisitor Visit) {
>auto VisitPointerArg = [&](const Decl *D, Expr *Arg) {
> @@ -6577,10 +6596,9 @@ static void handleGslAnnotatedTypes(Indi
>};
>
>if (auto *MCE = dyn_cast(Call)) {
> -const FunctionDecl *Callee = MCE->getDirectCallee();
> -if (auto *Conv = dyn_cast_or_null(Callee))
> -  if (isRecordWithAttr(Conv->getConversionType()))
> -VisitPointerArg(Callee, MCE->getImplicitObjectArgument());
> +const auto *MD =
> cast_or_null(MCE->getDirectCallee());
> +if (MD && shouldTrackImplicitObjectArg(MD))
> +  VisitPointerArg(MD, MCE->getImplicitObjectArgument());
>  return;
>}
>
>
> Modified: cfe/trunk/test/Analysis/inner-pointer.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/inner-pointer.cpp?rev=368446&r1=368445&r2=368446&view=diff
>
> ===

r368543 - [X86] Support -march=tigerlake

2019-08-11 Thread Pengfei Wang via cfe-commits

Author: pengfei
Date: Sun Aug 11 18:29:46 2019
New Revision: 368543

URL: http://llvm.org/viewvc/llvm-project?rev=368543&view=rev
Log:
[X86] Support -march=tigerlake

Support -march=tigerlake for x86.
Compare with Icelake Client, It include 4 more new features ,they are
avx512vp2intersect, movdiri, movdir64b, shstk.

Patch by Xiang Zhang (xiangzhangllvm)

Differential Revision: https://reviews.llvm.org/D65840

Modified:
cfe/trunk/include/clang/Basic/X86Target.def
cfe/trunk/lib/Basic/Targets/X86.cpp
cfe/trunk/test/Driver/x86-march.c
cfe/trunk/test/Misc/target-invalid-cpu-note.c
cfe/trunk/test/Preprocessor/predefined-arch-macros.c

Modified: cfe/trunk/include/clang/Basic/X86Target.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/X86Target.def?rev=368543&r1=368542&r2=368543&view=diff
==
--- cfe/trunk/include/clang/Basic/X86Target.def (original)
+++ cfe/trunk/include/clang/Basic/X86Target.def Sun Aug 11 18:29:46 2019
@@ -173,6 +173,10 @@ PROC(IcelakeClient, "icelake-client", PR
 /// Icelake server microarchitecture based processors.
 PROC(IcelakeServer, "icelake-server", PROC_64_BIT)
 
+/// \name Tigerlake Server
+/// Tigerlake Server microarchitecture based processors.
+PROC(Tigerlake, "tigerlake", PROC_64_BIT)
+
 /// \name Knights Landing
 /// Knights Landing processor.
 PROC_WITH_FEAT(KNL, "knl", PROC_64_BIT, FEATURE_AVX512F)
@@ -297,6 +301,7 @@ FEATURE(FEATURE_VPCLMULQDQ)
 FEATURE(FEATURE_AVX512VNNI)
 FEATURE(FEATURE_AVX512BITALG)
 FEATURE(FEATURE_AVX512BF16)
+FEATURE(FEATURE_AVX512VP2INTERSECT)
 
 
 // FIXME: When commented out features are supported in LLVM, enable them here.

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=368543&r1=368542&r2=368543&view=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Sun Aug 11 18:29:46 2019
@@ -157,11 +157,20 @@ bool X86TargetInfo::initFeatureMap(
 // SkylakeServer cores inherits all SKL features, except SGX
 goto SkylakeCommon;
 
+  case CK_Tigerlake:
+setFeatureEnabledImpl(Features, "avx512vp2intersect", true);
+setFeatureEnabledImpl(Features, "movdiri", true);
+setFeatureEnabledImpl(Features, "movdir64b", true);
+setFeatureEnabledImpl(Features, "shstk", true);
+// Tigerlake cores inherits IcelakeClient, except pconfig and wbnoinvd
+goto IcelakeCommon;
+
   case CK_IcelakeServer:
 setFeatureEnabledImpl(Features, "pconfig", true);
 setFeatureEnabledImpl(Features, "wbnoinvd", true);
 LLVM_FALLTHROUGH;
   case CK_IcelakeClient:
+IcelakeCommon:
 setFeatureEnabledImpl(Features, "vaes", true);
 setFeatureEnabledImpl(Features, "gfni", true);
 setFeatureEnabledImpl(Features, "vpclmulqdq", true);
@@ -1000,6 +1009,7 @@ void X86TargetInfo::getTargetDefines(con
   case CK_Cannonlake:
   case CK_IcelakeClient:
   case CK_IcelakeServer:
+  case CK_Tigerlake:
 // FIXME: Historically, we defined this legacy name, it would be nice to
 // remove it at some point. We've never exposed fine-grained names for
 // recent primary x86 CPUs, and we should keep it that way.

Modified: cfe/trunk/test/Driver/x86-march.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/x86-march.c?rev=368543&r1=368542&r2=368543&view=diff
==
--- cfe/trunk/test/Driver/x86-march.c (original)
+++ cfe/trunk/test/Driver/x86-march.c Sun Aug 11 18:29:46 2019
@@ -76,6 +76,10 @@
 // RUN:   | FileCheck %s -check-prefix=icelake-server
 // icelake-server: "-target-cpu" "icelake-server"
 //
+// RUN: %clang -target x86_64-unknown-unknown -c -### %s -march=tigerlake 2>&1 
\
+// RUN:   | FileCheck %s -check-prefix=tigerlake
+// tigerlake: "-target-cpu" "tigerlake"
+//
 // RUN: %clang -target x86_64-unknown-unknown -c -### %s -march=lakemont 2>&1 \
 // RUN:   | FileCheck %s -check-prefix=lakemont
 // lakemont: "-target-cpu" "lakemont"

Modified: cfe/trunk/test/Misc/target-invalid-cpu-note.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Misc/target-invalid-cpu-note.c?rev=368543&r1=368542&r2=368543&view=diff
==
--- cfe/trunk/test/Misc/target-invalid-cpu-note.c (original)
+++ cfe/trunk/test/Misc/target-invalid-cpu-note.c Sun Aug 11 18:29:46 2019
@@ -16,7 +16,7 @@
 // X86-SAME: nocona, core2, penryn, bonnell, atom, silvermont, slm, goldmont, 
goldmont-plus, tremont,
 // X86-SAME: nehalem, corei7, westmere, sandybridge, corei7-avx, ivybridge,
 // X86-SAME: core-avx-i, haswell, core-avx2, broadwell, skylake, 
skylake-avx512,
-// X86-SAME: skx, cascadelake, cooperlake, cannonlake, icelake-client, 
icelake-server, knl, knm, lakemont, k6, k6-2, k6-3,
+// X86-SAME: skx, casc

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

2019-08-11 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added inline comments.



Comment at: 
clang/lib/StaticAnalyzer/Checkers/EnumCastOutOfRangeChecker.cpp:94-98
+  // If cast is implicit LValueToRValue, no conversion is taking place,
+  // and therefore no range check is needed.  Don't analyze further.
+  if (CE->getCastKind() == CK_LValueToRValue)
+return;
+

If you look at the list of cast kinds, you'll be shocked to see ridiculously 
weird cornercases. Even though lvalue-to-rvalue cast definitely stands out 
(because it's the only one that has very little to do with actual casting), i'd 
still be much more comfortable if we *whitelist* the casts that we check, 
rather than blacklist them.

Can you take a look at the list and find which casts are we actually talking 
about and hardcode them here?



Comment at: clang/test/Analysis/enum-cast-out-of-range.c:27-34
+enum unscoped_unspecified_t unused;
+void unusedExpr() {
+// following line is not something that EnumCastOutOfRangeChecker should 
evaluate.  checker should either ignore this line
+// or process it without producing any warnings.  However, compilation 
will (and should) still generate a warning having 
+// nothing to do with this checker.
+unused; // expected-warning {{expression result unused}}
+}

I guess this covers D33672#1537287!

That said, there's one thing about this test that i don't understand. Why does 
this AST contain an implicit lvalue-to-rvalue cast at all? This looks like a 
(most likely otherwise harmless) bug in the AST; if i understand correctly, 
this should be a freestanding `DeclRefExpr`.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66014/new/

https://reviews.llvm.org/D66014



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-11 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added a comment.

My idea here was that this new feature isn't going to be user-facing. We aren't 
promising to support all combinations of 
enabled-disabled-silenced-dependent-registercheckerhacks, but instead use the 
new feature when we know it'll do exactly what we want. It is going to be up to 
the user-facing UI to decide how to use this feature, but not up to the 
end-users who simply want to silence diagnostics.

> Here is a problem with your patch: How would you go about silencing 
> diagnostics for `osx.cocoa.RetainCount`? I suppose 
> `-analyzer-silence-checker=osx.cocoa.RetainCount`. The problem however, that 
> the checker tag associated with it refers to `osx.cocoa.RetainCountBase` 
> under the hood, so you'll need to silence that instead. From that point on, 
> any other checker that suffers from the same issue is also silenced, that was 
> not the intent.

Hmm, sounds like we need to hack up a fix for the checker tag on the bug node, 
so that it was appropriate to the presumed (as opposed to actual) checker 
name(?)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66042/new/

https://reviews.llvm.org/D66042



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66067: Push lambda scope earlier when transforming lambda expression

2019-08-11 Thread Nicholas Allegra via Phabricator via cfe-commits

comex created this revision.
comex added reviewers: rsmith, faisalv, Mordante.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When first parsing a lambda expression, `BuildDeclRefExpr` is called on the 
parameter declarations with the lambda scope already pushed.  But when 
transforming a lambda expression as part of instantiating an outer template, 
`BuildDeclRefExpr` is called again without the scope pushed.  This causes 
`tryCaptureVariable` to get confused when a lambda parameter references an 
earlier parameter, e.g.:

  [](auto c, int x = sizeof(decltype(c))) {}

Fix this by moving up the call to `PushLambdaScope` in 
`TreeTransform::TransformLambdaExpr` to match 
`Parser::ParseLambdaExpressionAfterIntroducer`.

(Note: I do not have commit access.)


Repository:
  rC Clang

https://reviews.llvm.org/D66067

Files:
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaTemplate/default-arguments-cxx0x.cpp


Index: clang/test/SemaTemplate/default-arguments-cxx0x.cpp
===
--- clang/test/SemaTemplate/default-arguments-cxx0x.cpp
+++ clang/test/SemaTemplate/default-arguments-cxx0x.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++14 -verify %s
 // expected-no-diagnostics
 
 // Test default template arguments for function templates.
@@ -114,3 +114,13 @@
 S _a{};
   };
 }
+
+namespace lambda {
+template 
+void bar() {
+  (void) [](auto c, int x = sizeof(decltype(c))) {};
+}
+void foo() {
+  bar();
+}
+} // namespace lambda
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -11318,6 +11318,10 @@
 }
   }
 
+  LambdaScopeInfo *LSI = getSema().PushLambdaScope();
+  Sema::FunctionScopeRAII FuncScopeCleanup(getSema());
+  LSI->GLTemplateParameterList = TPL;
+
   // Transform the template parameters, and add them to the current
   // instantiation scope. The null case is handled correctly.
   auto TPL = getDerived().TransformTemplateParameterList(
@@ -11348,10 +11352,6 @@
 NewCallOpType);
   }
 
-  LambdaScopeInfo *LSI = getSema().PushLambdaScope();
-  Sema::FunctionScopeRAII FuncScopeCleanup(getSema());
-  LSI->GLTemplateParameterList = TPL;
-
   // Create the local class that will describe the lambda.
   CXXRecordDecl *OldClass = E->getLambdaClass();
   CXXRecordDecl *Class


Index: clang/test/SemaTemplate/default-arguments-cxx0x.cpp
===
--- clang/test/SemaTemplate/default-arguments-cxx0x.cpp
+++ clang/test/SemaTemplate/default-arguments-cxx0x.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++14 -verify %s
 // expected-no-diagnostics
 
 // Test default template arguments for function templates.
@@ -114,3 +114,13 @@
 S _a{};
   };
 }
+
+namespace lambda {
+template 
+void bar() {
+  (void) [](auto c, int x = sizeof(decltype(c))) {};
+}
+void foo() {
+  bar();
+}
+} // namespace lambda
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -11318,6 +11318,10 @@
 }
   }
 
+  LambdaScopeInfo *LSI = getSema().PushLambdaScope();
+  Sema::FunctionScopeRAII FuncScopeCleanup(getSema());
+  LSI->GLTemplateParameterList = TPL;
+
   // Transform the template parameters, and add them to the current
   // instantiation scope. The null case is handled correctly.
   auto TPL = getDerived().TransformTemplateParameterList(
@@ -11348,10 +11352,6 @@
 NewCallOpType);
   }
 
-  LambdaScopeInfo *LSI = getSema().PushLambdaScope();
-  Sema::FunctionScopeRAII FuncScopeCleanup(getSema());
-  LSI->GLTemplateParameterList = TPL;
-
   // Create the local class that will describe the lambda.
   CXXRecordDecl *OldClass = E->getLambdaClass();
   CXXRecordDecl *Class
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-11 Thread Csaba Dabis via Phabricator via cfe-commits

Charusso added a comment.

In D66042#1624684 , @NoQ wrote:

> My idea here was that this new feature isn't going to be user-facing. We 
> aren't promising to support all combinations of 
> enabled-disabled-silenced-dependent-registercheckerhacks, but instead use the 
> new feature when we know it'll do exactly what we want. It is going to be up 
> to the user-facing UI to decide how to use this feature, but not up to the 
> end-users who simply want to silence diagnostics.
>
> > Here is a problem with your patch: How would you go about silencing 
> > diagnostics for `osx.cocoa.RetainCount`? I suppose 
> > `-analyzer-silence-checker=osx.cocoa.RetainCount`. The problem however, 
> > that the checker tag associated with it refers to 
> > `osx.cocoa.RetainCountBase` under the hood, so you'll need to silence that 
> > instead. From that point on, any other checker that suffers from the same 
> > issue is also silenced, that was not the intent.
>
> Hmm, sounds like we need to hack up a fix for the checker tag on the bug 
> node, so that it was appropriate to the presumed (as opposed to actual) 
> checker name(?)


`StringRef("osx.cocoa.RetainCountBase").startswith("osx.cocoa.RetainCount")` is 
true, so there is no real issue until we manage the prefixes well. I assume 
that the user who knows how to disable/silence a checker, knows where to read 
how to disable/silence it. At least with scan-build there will not be pitfalls 
with disabling the core modeling.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66042/new/

https://reviews.llvm.org/D66042



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66067: Push lambda scope earlier when transforming lambda expression

2019-08-11 Thread Nicholas Allegra via Phabricator via cfe-commits

comex updated this revision to Diff 214572.
comex added a comment.

Oops, I forgot to re-run `git diff` after fixing the patch.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66067/new/

https://reviews.llvm.org/D66067

Files:
  clang/lib/Sema/TreeTransform.h
  clang/test/SemaTemplate/default-arguments-cxx0x.cpp


Index: clang/test/SemaTemplate/default-arguments-cxx0x.cpp
===
--- clang/test/SemaTemplate/default-arguments-cxx0x.cpp
+++ clang/test/SemaTemplate/default-arguments-cxx0x.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++14 -verify %s
 // expected-no-diagnostics
 
 // Test default template arguments for function templates.
@@ -114,3 +114,13 @@
 S _a{};
   };
 }
+
+namespace lambda {
+template 
+void bar() {
+  (void) [](auto c, int x = sizeof(decltype(c))) {};
+}
+void foo() {
+  bar();
+}
+} // namespace lambda
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -11318,10 +11318,14 @@
 }
   }
 
+  LambdaScopeInfo *LSI = getSema().PushLambdaScope();
+  Sema::FunctionScopeRAII FuncScopeCleanup(getSema());
+
   // Transform the template parameters, and add them to the current
   // instantiation scope. The null case is handled correctly.
   auto TPL = getDerived().TransformTemplateParameterList(
   E->getTemplateParameterList());
+  LSI->GLTemplateParameterList = TPL;
 
   // Transform the type of the original lambda's call operator.
   // The transformation MUST be done in the CurrentInstantiationScope since
@@ -11348,10 +11352,6 @@
 NewCallOpType);
   }
 
-  LambdaScopeInfo *LSI = getSema().PushLambdaScope();
-  Sema::FunctionScopeRAII FuncScopeCleanup(getSema());
-  LSI->GLTemplateParameterList = TPL;
-
   // Create the local class that will describe the lambda.
   CXXRecordDecl *OldClass = E->getLambdaClass();
   CXXRecordDecl *Class


Index: clang/test/SemaTemplate/default-arguments-cxx0x.cpp
===
--- clang/test/SemaTemplate/default-arguments-cxx0x.cpp
+++ clang/test/SemaTemplate/default-arguments-cxx0x.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -std=c++11 -verify %s
+// RUN: %clang_cc1 -fsyntax-only -std=c++14 -verify %s
 // expected-no-diagnostics
 
 // Test default template arguments for function templates.
@@ -114,3 +114,13 @@
 S _a{};
   };
 }
+
+namespace lambda {
+template 
+void bar() {
+  (void) [](auto c, int x = sizeof(decltype(c))) {};
+}
+void foo() {
+  bar();
+}
+} // namespace lambda
Index: clang/lib/Sema/TreeTransform.h
===
--- clang/lib/Sema/TreeTransform.h
+++ clang/lib/Sema/TreeTransform.h
@@ -11318,10 +11318,14 @@
 }
   }
 
+  LambdaScopeInfo *LSI = getSema().PushLambdaScope();
+  Sema::FunctionScopeRAII FuncScopeCleanup(getSema());
+
   // Transform the template parameters, and add them to the current
   // instantiation scope. The null case is handled correctly.
   auto TPL = getDerived().TransformTemplateParameterList(
   E->getTemplateParameterList());
+  LSI->GLTemplateParameterList = TPL;
 
   // Transform the type of the original lambda's call operator.
   // The transformation MUST be done in the CurrentInstantiationScope since
@@ -11348,10 +11352,6 @@
 NewCallOpType);
   }
 
-  LambdaScopeInfo *LSI = getSema().PushLambdaScope();
-  Sema::FunctionScopeRAII FuncScopeCleanup(getSema());
-  LSI->GLTemplateParameterList = TPL;
-
   // Create the local class that will describe the lambda.
   CXXRecordDecl *OldClass = E->getLambdaClass();
   CXXRecordDecl *Class
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

2019-08-11 Thread Artem Dergachev via Phabricator via cfe-commits

NoQ added a comment.

In D66042#1624697 , @Charusso wrote:

> `StringRef("osx.cocoa.RetainCountBase").startswith("osx.cocoa.RetainCount")` 
> is true, so there is no real issue until we manage the prefixes well. I 
> assume that the user who knows how to disable/silence a checker, knows where 
> to read how to disable/silence it. At least with scan-build there will not be 
> pitfalls with disabling the core modeling.


Nice, but i suspect that `osx.OSObjectRetainCount` is still screwed :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D66042/new/

https://reviews.llvm.org/D66042



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D66068: cmake: Make building clang-shlib optional

2019-08-11 Thread Jan Vesely via Phabricator via cfe-commits

jvesely created this revision.
jvesely added a reviewer: beanz.
Herald added a subscriber: mgorny.
Herald added a project: clang.

It takes ~5min to link, add an option to avoid that.


Repository:
  rC Clang

https://reviews.llvm.org/D66068

Files:
  CMakeLists.txt
  tools/CMakeLists.txt


Index: tools/CMakeLists.txt
===
--- tools/CMakeLists.txt
+++ tools/CMakeLists.txt
@@ -14,7 +14,7 @@
 
 add_clang_subdirectory(clang-rename)
 add_clang_subdirectory(clang-refactor)
-if(UNIX)
+if(UNIX AND CLANG_ENABLE_SHLIB)
   add_clang_subdirectory(clang-shlib)
 endif()
 
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -451,6 +451,10 @@
 option(CLANG_BUILD_TOOLS
   "Build the Clang tools. If OFF, just generate build targets." ON)
 
+if (UNIX)
+  option(CLANG_ENABLE_SHLIB "Build combined clang dynamic library." ON)
+endif()
+
 option(CLANG_ENABLE_ARCMT "Build ARCMT." ON)
 option(CLANG_ENABLE_STATIC_ANALYZER "Build static analyzer." ON)
 


Index: tools/CMakeLists.txt
===
--- tools/CMakeLists.txt
+++ tools/CMakeLists.txt
@@ -14,7 +14,7 @@
 
 add_clang_subdirectory(clang-rename)
 add_clang_subdirectory(clang-refactor)
-if(UNIX)
+if(UNIX AND CLANG_ENABLE_SHLIB)
   add_clang_subdirectory(clang-shlib)
 endif()
 
Index: CMakeLists.txt
===
--- CMakeLists.txt
+++ CMakeLists.txt
@@ -451,6 +451,10 @@
 option(CLANG_BUILD_TOOLS
   "Build the Clang tools. If OFF, just generate build targets." ON)
 
+if (UNIX)
+  option(CLANG_ENABLE_SHLIB "Build combined clang dynamic library." ON)
+endif()
+
 option(CLANG_ENABLE_ARCMT "Build ARCMT." ON)
 option(CLANG_ENABLE_STATIC_ANALYZER "Build static analyzer." ON)
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65935: [ASTImporter] Import ctor initializers after setting flags.

2019-08-11 Thread Balázs Kéri via Phabricator via cfe-commits

balazske added a comment.

In this case the problem was that some of the flags of the already created and 
inserted `ToFunction` are not initialized. During the import of some "ctor 
initializers" this non-complete ToFunction may be accessed somehow (by 
structural equivalence or other code) and an assert comes because invalid 
value. (Maybe in the test code the second constructor is imported as "ctor 
initializer" during the import of the first.)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D65935/new/

https://reviews.llvm.org/D65935



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D65993: [NFC][clang] Adding argument based Phase list filtering to getComplicationPhases

r368528 - Properly handle reference initialization when detecting gsl::Pointer initialization chains

[PATCH] D66059: [clang-format] Expand AllowShortBlocksOnASingleLine for WebKit

[PATCH] D66035: [WebAssembly] WIP: Add support for reference types

r368534 - Properly detect temporary gsl::Owners through reference initialization chains.

[PATCH] D25845: [CUDA] Ignore implicit target attributes during function template instantiation.

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

[PATCH] D65149: [Format] Add test demonstrating PR42722

r368539 - [clang-format] Expand AllowShortBlocksOnASingleLine for WebKit

[PATCH] D66059: [clang-format] Expand AllowShortBlocksOnASingleLine for WebKit

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

Re: r368446 - More warnings regarding gsl::Pointer and gsl::Owner attributes

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

[PATCH] D65935: [ASTImporter] Import ctor initializers after setting flags.

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

[PATCH] D65999: [ASTImporter] Import additional flags for functions.

[PATCH] D65835: [OpenMP] Permit map with DSA on combined directive

[PATCH] D65269: [ASTImporter] Fix for import of friend class template with definition.

[PATCH] D65373: [clangd] Update features table in the docs with links to LSP extension proposals

Re: r368446 - More warnings regarding gsl::Pointer and gsl::Owner attributes

Re: r368446 - More warnings regarding gsl::Pointer and gsl::Owner attributes

r368543 - [X86] Support -march=tigerlake

[PATCH] D66014: [analyzer] Avoid unnecessary enum range check on LValueToRValue casts

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

[PATCH] D66067: Push lambda scope earlier when transforming lambda expression

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

[PATCH] D66067: Push lambda scope earlier when transforming lambda expression

[PATCH] D66042: [analyzer] Analysis: "Disable" core checkers

[PATCH] D66068: cmake: Make building clang-shlib optional

[PATCH] D65935: [ASTImporter] Import ctor initializers after setting flags.

35 matches

Site Navigation

Mail list logo

Footer information