[PATCH] D59533: [X86] Add __crc32b/__crc32w/__crc32d/__crc32q intrinsics to match gcc and icc.

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: spatel, RKSimon.
Herald added a project: clang.
craig.topper updated this revision to Diff 191251.
craig.topper added a comment.

Add the test file that I forgot to git add before running arcanist


gcc has these intrinsics in ia32intrin.h as well. And icc implements them
though they aren't documented in the Intel Intrinsics Guide.


Repository:
  rC Clang

https://reviews.llvm.org/D59533

Files:
  lib/Headers/ia32intrin.h
  test/CodeGen/x86-crc-builtins.c


Index: test/CodeGen/x86-crc-builtins.c
===
--- /dev/null
+++ test/CodeGen/x86-crc-builtins.c
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin 
-target-feature +sse4.2 -emit-llvm -o - -Wall -Werror | FileCheck %s 
--check-prefixes=CHECK,CHECK64
+// RUN: %clang_cc1 -ffreestanding %s -triple=i686-apple-darwin -target-feature 
+sse4.2 -emit-llvm -o - -Wall -Werror | FileCheck %s
+
+#include 
+
+unsigned int test__crc32b(unsigned int CRC, unsigned char V) {
+// CHECK-LABEL: test__crc32b
+// CHECK: call i32 @llvm.x86.sse42.crc32.32.8(i32 %{{.*}}, i8 %{{.*}})
+  return __crc32b(CRC, V);
+}
+
+unsigned int test__crc32w(unsigned int CRC, unsigned short V) {
+// CHECK-LABEL: test__crc32w
+// CHECK: call i32 @llvm.x86.sse42.crc32.32.16(i32 %{{.*}}, i16 %{{.*}})
+  return __crc32w(CRC, V);
+}
+
+unsigned int test__crc32d(unsigned int CRC, unsigned int V) {
+// CHECK-LABEL: test__crc32d
+// CHECK: call i32 @llvm.x86.sse42.crc32.32.32(i32 %{{.*}}, i32 %{{.*}})
+  return __crc32d(CRC, V);
+}
+
+#ifdef __x86_64__
+unsigned long long test__crc32q(unsigned long long CRC, unsigned long long V) {
+// CHECK64-LABEL: test__crc32q
+// CHECK64: call i64 @llvm.x86.sse42.crc32.64.64(i64 %{{.*}}, i64 %{{.*}})
+  return __crc32q(CRC, V);
+}
+#endif
Index: lib/Headers/ia32intrin.h
===
--- lib/Headers/ia32intrin.h
+++ lib/Headers/ia32intrin.h
@@ -55,6 +55,32 @@
 }
 #endif /* !__x86_64__ */
 
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, 
__target__("sse4.2")))
+__crc32b(unsigned int __C, unsigned char __D)
+{
+  return __builtin_ia32_crc32qi(__C, __D);
+}
+
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, 
__target__("sse4.2")))
+__crc32w(unsigned int __C, unsigned short __D)
+{
+  return __builtin_ia32_crc32hi(__C, __D);
+}
+
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, 
__target__("sse4.2")))
+__crc32d(unsigned int __C, unsigned int __D)
+{
+  return __builtin_ia32_crc32si(__C, __D);
+}
+
+#ifdef __x86_64__
+static __inline__ unsigned long long __attribute__((__always_inline__, 
__nodebug__, __target__("sse4.2")))
+__crc32q(unsigned long long __C, unsigned long long __D)
+{
+  return __builtin_ia32_crc32di(__C, __D);
+}
+#endif /* __x86_64__ */
+
 static __inline__ unsigned long long __attribute__((__always_inline__, 
__nodebug__))
 __rdpmc(int __A) {
   return __builtin_ia32_rdpmc(__A);


Index: test/CodeGen/x86-crc-builtins.c
===
--- /dev/null
+++ test/CodeGen/x86-crc-builtins.c
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin -target-feature +sse4.2 -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes=CHECK,CHECK64
+// RUN: %clang_cc1 -ffreestanding %s -triple=i686-apple-darwin -target-feature +sse4.2 -emit-llvm -o - -Wall -Werror | FileCheck %s
+
+#include 
+
+unsigned int test__crc32b(unsigned int CRC, unsigned char V) {
+// CHECK-LABEL: test__crc32b
+// CHECK: call i32 @llvm.x86.sse42.crc32.32.8(i32 %{{.*}}, i8 %{{.*}})
+  return __crc32b(CRC, V);
+}
+
+unsigned int test__crc32w(unsigned int CRC, unsigned short V) {
+// CHECK-LABEL: test__crc32w
+// CHECK: call i32 @llvm.x86.sse42.crc32.32.16(i32 %{{.*}}, i16 %{{.*}})
+  return __crc32w(CRC, V);
+}
+
+unsigned int test__crc32d(unsigned int CRC, unsigned int V) {
+// CHECK-LABEL: test__crc32d
+// CHECK: call i32 @llvm.x86.sse42.crc32.32.32(i32 %{{.*}}, i32 %{{.*}})
+  return __crc32d(CRC, V);
+}
+
+#ifdef __x86_64__
+unsigned long long test__crc32q(unsigned long long CRC, unsigned long long V) {
+// CHECK64-LABEL: test__crc32q
+// CHECK64: call i64 @llvm.x86.sse42.crc32.64.64(i64 %{{.*}}, i64 %{{.*}})
+  return __crc32q(CRC, V);
+}
+#endif
Index: lib/Headers/ia32intrin.h
===
--- lib/Headers/ia32intrin.h
+++ lib/Headers/ia32intrin.h
@@ -55,6 +55,32 @@
 }
 #endif /* !__x86_64__ */
 
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, __target__("sse4.2")))
+__crc32b(unsigned int __C, unsigned char __D)
+{
+  return __builtin_ia32_crc32qi(__C, __D);
+}
+
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, __target__("sse4.2")))
+__crc32w(unsigned int __C, unsigned short __D)
+{
+  return 

[PATCH] D59533: [X86] Add __crc32b/__crc32w/__crc32d/__crc32q intrinsics to match gcc and icc.

[Craig Topper via Phabricator via cfe-commits]

craig.topper updated this revision to Diff 191251.
craig.topper added a comment.

Add the test file that I forgot to git add before running arcanist


Repository:
  rC Clang

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

https://reviews.llvm.org/D59533

Files:
  lib/Headers/ia32intrin.h
  test/CodeGen/x86-crc-builtins.c


Index: test/CodeGen/x86-crc-builtins.c
===
--- /dev/null
+++ test/CodeGen/x86-crc-builtins.c
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin 
-target-feature +sse4.2 -emit-llvm -o - -Wall -Werror | FileCheck %s 
--check-prefixes=CHECK,CHECK64
+// RUN: %clang_cc1 -ffreestanding %s -triple=i686-apple-darwin -target-feature 
+sse4.2 -emit-llvm -o - -Wall -Werror | FileCheck %s
+
+#include 
+
+unsigned int test__crc32b(unsigned int CRC, unsigned char V) {
+// CHECK-LABEL: test__crc32b
+// CHECK: call i32 @llvm.x86.sse42.crc32.32.8(i32 %{{.*}}, i8 %{{.*}})
+  return __crc32b(CRC, V);
+}
+
+unsigned int test__crc32w(unsigned int CRC, unsigned short V) {
+// CHECK-LABEL: test__crc32w
+// CHECK: call i32 @llvm.x86.sse42.crc32.32.16(i32 %{{.*}}, i16 %{{.*}})
+  return __crc32w(CRC, V);
+}
+
+unsigned int test__crc32d(unsigned int CRC, unsigned int V) {
+// CHECK-LABEL: test__crc32d
+// CHECK: call i32 @llvm.x86.sse42.crc32.32.32(i32 %{{.*}}, i32 %{{.*}})
+  return __crc32d(CRC, V);
+}
+
+#ifdef __x86_64__
+unsigned long long test__crc32q(unsigned long long CRC, unsigned long long V) {
+// CHECK64-LABEL: test__crc32q
+// CHECK64: call i64 @llvm.x86.sse42.crc32.64.64(i64 %{{.*}}, i64 %{{.*}})
+  return __crc32q(CRC, V);
+}
+#endif
Index: lib/Headers/ia32intrin.h
===
--- lib/Headers/ia32intrin.h
+++ lib/Headers/ia32intrin.h
@@ -55,6 +55,32 @@
 }
 #endif /* !__x86_64__ */
 
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, 
__target__("sse4.2")))
+__crc32b(unsigned int __C, unsigned char __D)
+{
+  return __builtin_ia32_crc32qi(__C, __D);
+}
+
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, 
__target__("sse4.2")))
+__crc32w(unsigned int __C, unsigned short __D)
+{
+  return __builtin_ia32_crc32hi(__C, __D);
+}
+
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, 
__target__("sse4.2")))
+__crc32d(unsigned int __C, unsigned int __D)
+{
+  return __builtin_ia32_crc32si(__C, __D);
+}
+
+#ifdef __x86_64__
+static __inline__ unsigned long long __attribute__((__always_inline__, 
__nodebug__, __target__("sse4.2")))
+__crc32q(unsigned long long __C, unsigned long long __D)
+{
+  return __builtin_ia32_crc32di(__C, __D);
+}
+#endif /* __x86_64__ */
+
 static __inline__ unsigned long long __attribute__((__always_inline__, 
__nodebug__))
 __rdpmc(int __A) {
   return __builtin_ia32_rdpmc(__A);


Index: test/CodeGen/x86-crc-builtins.c
===
--- /dev/null
+++ test/CodeGen/x86-crc-builtins.c
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin -target-feature +sse4.2 -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefixes=CHECK,CHECK64
+// RUN: %clang_cc1 -ffreestanding %s -triple=i686-apple-darwin -target-feature +sse4.2 -emit-llvm -o - -Wall -Werror | FileCheck %s
+
+#include 
+
+unsigned int test__crc32b(unsigned int CRC, unsigned char V) {
+// CHECK-LABEL: test__crc32b
+// CHECK: call i32 @llvm.x86.sse42.crc32.32.8(i32 %{{.*}}, i8 %{{.*}})
+  return __crc32b(CRC, V);
+}
+
+unsigned int test__crc32w(unsigned int CRC, unsigned short V) {
+// CHECK-LABEL: test__crc32w
+// CHECK: call i32 @llvm.x86.sse42.crc32.32.16(i32 %{{.*}}, i16 %{{.*}})
+  return __crc32w(CRC, V);
+}
+
+unsigned int test__crc32d(unsigned int CRC, unsigned int V) {
+// CHECK-LABEL: test__crc32d
+// CHECK: call i32 @llvm.x86.sse42.crc32.32.32(i32 %{{.*}}, i32 %{{.*}})
+  return __crc32d(CRC, V);
+}
+
+#ifdef __x86_64__
+unsigned long long test__crc32q(unsigned long long CRC, unsigned long long V) {
+// CHECK64-LABEL: test__crc32q
+// CHECK64: call i64 @llvm.x86.sse42.crc32.64.64(i64 %{{.*}}, i64 %{{.*}})
+  return __crc32q(CRC, V);
+}
+#endif
Index: lib/Headers/ia32intrin.h
===
--- lib/Headers/ia32intrin.h
+++ lib/Headers/ia32intrin.h
@@ -55,6 +55,32 @@
 }
 #endif /* !__x86_64__ */
 
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, __target__("sse4.2")))
+__crc32b(unsigned int __C, unsigned char __D)
+{
+  return __builtin_ia32_crc32qi(__C, __D);
+}
+
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, __target__("sse4.2")))
+__crc32w(unsigned int __C, unsigned short __D)
+{
+  return __builtin_ia32_crc32hi(__C, __D);
+}
+
+static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, __target__("sse4.2")))
+__crc32d(unsigned int __C, unsigned int __D

[PATCH] D59415: Do not resolve directory junctions for `-fdiagnostics-absolute-paths` on Windows.

[Igor Kudrin via Phabricator via cfe-commits]

ikudrin marked an inline comment as done.
ikudrin added inline comments.



Comment at: test/Frontend/absolute-paths-windows.test:4
+// RUN: mkdir -p %t.dir\real
+// RUN: cmd /c mklink /j %t.dir\junc %t.dir\real
+// RUN: echo "wrong code" > %t.dir\real\foo.cpp

ikudrin wrote:
> hans wrote:
> > This requires (a recent version of) Win 10 or later to run without running 
> > as Admin, right?
> As far as I know, creating a directory junction, in contrast to a symbolic 
> link, does not require escalated privileges. But, honestly, I do not have any 
> old system to validate that.
We checked that on Windows 7. It does not require rights elevation, too.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59415



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

[PATCH] D57662: [clang-tidy] Parallelize clang-tidy-diff.py

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added a comment.

One more gentle ping.


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

https://reviews.llvm.org/D57662



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

[PATCH] D59449: [clang-tidy] Integrate clang-tidy-diff.py machinery into run-clang-tidy.py

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis added a comment.

Any comments?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59449



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

[PATCH] D59485: [ASTImporter] Allow adding a import strategy to the ASTImporter

[Raphael Isemann via Phabricator via cfe-commits]

teemperor updated this revision to Diff 191257.
teemperor retitled this revision from "[ASTImporter] Allow adding a shim to the 
ASTImporter" to "[ASTImporter] Allow adding a import strategy to the 
ASTImporter".
teemperor edited the summary of this revision.
teemperor added a comment.

Thanks for the quick review! I changed the code to also update the internal 
ASTImporter state like with a normal decl.

Changes:

- Renamed from Shim -> Strategy
- Now using the ImportedDecls map and also updating the internal ASTImporter 
state.


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

https://reviews.llvm.org/D59485

Files:
  clang/include/clang/AST/ASTImporter.h
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -316,11 +316,14 @@
 std::unique_ptr Unit;
 TranslationUnitDecl *TUDecl = nullptr;
 std::unique_ptr Importer;
-TU(StringRef Code, StringRef FileName, ArgVector Args)
+ImportStrategy *Strategy;
+TU(StringRef Code, StringRef FileName, ArgVector Args,
+   ImportStrategy *Strategy = nullptr)
 : Code(Code), FileName(FileName),
   Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
  this->FileName)),
-  TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {
+  TUDecl(Unit->getASTContext().getTranslationUnitDecl()),
+  Strategy(Strategy) {
   Unit->enableSourceFileDiagnostics();
 }
 
@@ -331,6 +334,7 @@
 new ASTImporter(ToAST->getASTContext(), ToAST->getFileManager(),
 Unit->getASTContext(), Unit->getFileManager(),
 false, &LookupTable));
+Importer->setStrategy(Strategy);
   }
   assert(&ToAST->getASTContext() == &Importer->getToContext());
   createVirtualFileIfNeeded(ToAST, FileName, Code);
@@ -401,11 +405,12 @@
   // Must not be called more than once within the same test.
   std::tuple
   getImportedDecl(StringRef FromSrcCode, Language FromLang, StringRef ToSrcCode,
-  Language ToLang, StringRef Identifier = DeclToImportID) {
+  Language ToLang, StringRef Identifier = DeclToImportID,
+  ImportStrategy *Strategy = nullptr) {
 ArgVector FromArgs = getArgVectorForLanguage(FromLang),
   ToArgs = getArgVectorForLanguage(ToLang);
 
-FromTUs.emplace_back(FromSrcCode, InputFileName, FromArgs);
+FromTUs.emplace_back(FromSrcCode, InputFileName, FromArgs, Strategy);
 TU &FromTU = FromTUs.back();
 
 assert(!ToAST);
@@ -455,6 +460,12 @@
 return ToAST->getASTContext().getTranslationUnitDecl();
   }
 
+  ASTImporter &getImporter(Decl *From, Language ToLang) {
+lazyInitToAST(ToLang, "", OutputFileName);
+TU *FromTU = findFromTU(From);
+return *FromTU->Importer;
+  }
+
   // Import the given Decl into the ToCtx.
   // May be called several times in a given test.
   // The different instances of the param From may have different ASTContext.
@@ -544,6 +555,81 @@
   EXPECT_THAT(RedeclsD1, ::testing::ContainerEq(RedeclsD2));
 }
 
+namespace {
+class RedirectStrategy : public ImportStrategy {
+  llvm::Optional Import(ASTImporter &Importer, Decl *FromD) override {
+auto *ND = dyn_cast(FromD);
+if (!ND)
+  return {};
+if (ND->getName() != "shouldNotBeImported")
+  return {};
+for (Decl *D : Importer.getToContext().getTranslationUnitDecl()->decls())
+  if (auto ND = dyn_cast(D))
+if (ND->getName() == "realDecl")
+  return ND;
+return {};
+  }
+};
+} // namespace
+
+struct ImportStrategyTest : ASTImporterOptionSpecificTestBase {};
+
+// Test that the ImportStrategy can intercept an import call.
+TEST_P(ImportStrategyTest, InterceptImport) {
+  RedirectStrategy Strategy;
+  Decl *From, *To;
+  std::tie(From, To) = getImportedDecl("class shouldNotBeImported {};",
+   Lang_CXX, "class realDecl {};", Lang_CXX,
+   "shouldNotBeImported", &Strategy);
+  auto *Imported = cast(To);
+  EXPECT_EQ(Imported->getQualifiedNameAsString(), "realDecl");
+
+  // Make sure the Strategy prevented the importing of the decl.
+  auto *ToTU = Imported->getTranslationUnitDecl();
+  auto Pattern = functionDecl(hasName("shouldNotBeImported"));
+  unsigned count =
+  DeclCounterWithPredicate().match(ToTU, Pattern);
+  EXPECT_EQ(0U, count);
+}
+
+// Test that when we indirectly import a declaration the Strategy still works.
+// Also tests that the ImportStrategy allows the import process to continue when
+// we try to import a declaration that we ignore in the Strategy.
+TEST_P(ImportStrategyTest, InterceptIndirectImport) {
+  RedirectStrategy Strategy;
+  Decl *From, *To;
+  std::tie(Fr

[PATCH] D59487: [Tooling] Add more scope specifiers until spelling is not ambiguous.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: lib/Tooling/Core/Lookup.cpp:165
+if (UnspelledScopes.empty()) {
+  Disambiguated = "::" + Disambiguated;
+} else {

ioeric wrote:
> kadircet wrote:
> > maybe return or break afterwards?
> I also struggled a bit here. But I decided to let `IsAmbiguousSpelling` 
> handle it because it seemed a bit more natural. Otherwise, we would somewhat 
> duplicate the logic that spelling with leading "::" is not ambiguous.
ok sgtm


Repository:
  rC Clang

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

https://reviews.llvm.org/D59487



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

[PATCH] D59440: add steps to preprocess file and reduce command line args

[serge via Phabricator via cfe-commits]

serge-sans-paille added inline comments.



Comment at: clang/utils/creduce-clang-crash.py:45
+  result = []
+  for i, arg in enumerate(cmd):
+if arg.startswith('$'):

Useless enumerate here. You could even use a list comprehension here

```
return ' '.join(arg if arg.startswith('$') else pipes.quote(arg) for arg in cmd)
```



Comment at: clang/utils/creduce-clang-crash.py:161
+  for arg in args:
+if (any(arg.startswith(a) for a in opts_startswith)):
+  continue

useless parenthesis around test

could be a list comprehension

```
result = [arg for arg in args if all(not arg.startswith(a) for a in 
opts_startswith)]
```


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

https://reviews.llvm.org/D59440



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

[PATCH] D59485: [ASTImporter] Allow adding a import strategy to the ASTImporter

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

It looks better now.
One "problem" is that now there is the strategy and there is the `Imported` 
function. It would be possible to unify these into a strategy that contains 
maybe a "import" and a "post-import" callback. (But this requires big changes 
in the `ASTImporter` user code.)




Comment at: clang/include/clang/AST/ASTImporter.h:154
 
+/// The Shim that should rewrite the import calls of this ASTImporter, or
+/// a nullptr of this ASTImporter has no shim.

'Shim' should be replaced with 'strategy' (and the nullptr means "no 
user-specified strategy" or "default strategy").


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

https://reviews.llvm.org/D59485



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

[PATCH] D40181: [libcxx] Allow to set locale on Windows.

[Andrey Khalyavin via Phabricator via cfe-commits]

halyavin added a comment.

Yes, I considered it. But Microsoft documentation is not clear whether it will 
work or not for restoring inconsistent locales. The documentation shows how to 
save inconsistent locales with LC_ALL but doesn't provide an example for 
restoring them.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D40181



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

[PATCH] D59481: [clangd] Count number of references while merging RefSlabs inside FileIndex

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked 2 inline comments as done.
kadircet added a comment.

In D59481#1432881 , @ioeric wrote:

> I'm not sure if FileIndex is the correct layer to make decision about how 
> References is calculated. Currently, we have two use cases in clangd 1) one 
> slab per TU and 2) one slab per file. It seems to me that we would want 
> different strategies for the two use cases, so it might make sense to make 
> this configurable in FileIndex.  And in one case, we have `References` ~= # 
> of TUs, and in the other case, we would have `References` ~= # of files.


`References` ~= # of TUs case has nothing to do with FileIndex actually, it 
only happens if references are counted beforehand(which is currently only 
happening in clangd-indexer).
FileIndex has two ways to handle duplicates during an index merge:

- `pickone` which doesn't count references at all (this is the oneslab per TU 
case)
- `merge` which counts references as number of files this symbol has occured 
(this is the oneslab per File case)

> This can over-count `References`  in the second case. It might be fine as an 
> approximation (if there is no better solution), but we should definitely 
> document it (e.g. in `BackgroundIndex`).

I don't follow why this can over-count, FileIndex keeps only one RefSlab per 
each file, so we can't over-count? Did you mean it will be more than #TUs ?




Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:50
+// FileIndex keeps only oneslab per file.
+// This contradicts with behavior inside clangd-indexer, it only counts
+// translation units since it processes each header multiple times.

ioeric wrote:
> this is a bit of a code smell. FileIndex is not supposed to know anything 
> about clangd-indexer etc.
I've actually just left the comment for review so that we can decide what to do 
about discrepancy(with my reasoning). It is not like FileIndex is somehow tied 
to clangd-indexer now?



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:170
   I.first->second = mergeSymbol(I.first->second, Sym);
+// We re-count number of references while merging refs from scratch.
+I.first->getSecond().References = 0;

ioeric wrote:
> note that references might not always exist. SymbolCollector can be set up to 
> not collect references.
I thought we wouldn't make any promises about `References` in such a case, do 
we?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59481



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

[clang-tools-extra] r356445 - [clangd] Add support for type hierarchy (super types only for now)

[Kadir Cetinkaya via cfe-commits]

Author: kadircet
Date: Tue Mar 19 02:27:04 2019
New Revision: 356445

URL: http://llvm.org/viewvc/llvm-project?rev=356445&view=rev
Log:
[clangd] Add support for type hierarchy (super types only for now)

Summary:
Patch by Nathan Ridge(@nridge)!

This is an LSP extension proposed here:
https://github.com/Microsoft/vscode-languageserver-node/pull/426

An example client implementation can be found here:
https://github.com/theia-ide/theia/pull/3802

Reviewers: kadircet, sammccall

Reviewed By: kadircet

Subscribers: jdoerfert, sammccall, cfe-commits, mgorny, dschaefer, simark, 
ilya-biryukov, ioeric, MaskRay, jkorous, arphaman, kadircet

Tags: #clang

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

Added:
clang-tools-extra/trunk/test/clangd/type-hierarchy.test
clang-tools-extra/trunk/unittests/clangd/TypeHierarchyTests.cpp
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/FindSymbols.cpp
clang-tools-extra/trunk/clangd/FindSymbols.h
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/clangd/XRefs.cpp
clang-tools-extra/trunk/clangd/XRefs.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.h
clang-tools-extra/trunk/test/clangd/initialize-params.test
clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
clang-tools-extra/trunk/unittests/clangd/Matchers.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=356445&r1=356444&r2=356445&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Tue Mar 19 02:27:04 2019
@@ -368,6 +368,7 @@ void ClangdLSPServer::onInitialize(const
   {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND,
ExecuteCommandParams::CLANGD_APPLY_TWEAK}},
  }},
+{"typeHierarchyProvider", true},
 );
 }
 
@@ -806,6 +807,13 @@ void ClangdLSPServer::onHover(const Text
 std::move(Reply));
 }
 
+void ClangdLSPServer::onTypeHierarchy(
+const TypeHierarchyParams &Params,
+Callback> Reply) {
+  Server->typeHierarchy(Params.textDocument.uri.file(), Params.position,
+Params.resolve, Params.direction, std::move(Reply));
+}
+
 void ClangdLSPServer::applyConfiguration(
 const ConfigurationSettings &Settings) {
   // Per-file update to the compilation database.
@@ -885,6 +893,7 @@ ClangdLSPServer::ClangdLSPServer(class T
   MsgHandler->bind("workspace/didChangeWatchedFiles", 
&ClangdLSPServer::onFileEvent);
   MsgHandler->bind("workspace/didChangeConfiguration", 
&ClangdLSPServer::onChangeConfiguration);
   MsgHandler->bind("textDocument/symbolInfo", &ClangdLSPServer::onSymbolInfo);
+  MsgHandler->bind("textDocument/typeHierarchy", 
&ClangdLSPServer::onTypeHierarchy);
   // clang-format on
 }
 

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=356445&r1=356444&r2=356445&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Tue Mar 19 02:27:04 2019
@@ -93,6 +93,8 @@ private:
   void onRename(const RenameParams &, Callback);
   void onHover(const TextDocumentPositionParams &,
Callback>);
+  void onTypeHierarchy(const TypeHierarchyParams &,
+   Callback>);
   void onChangeConfiguration(const DidChangeConfigurationParams &);
   void onSymbolInfo(const TextDocumentPositionParams &,
 Callback>);

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=356445&r1=356444&r2=356445&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Tue Mar 19 02:27:04 2019
@@ -362,9 +362,8 @@ void ClangdServer::enumerateTweaks(PathR
 
 void ClangdServer::applyTweak(PathRef File, Range Sel, StringRef TweakID,
   Callback CB) {
-  auto Action = [Sel](decltype(CB) CB, std::string File,
-std::string TweakID,
-Expected InpAST) {
+  auto Action = [Sel](decltype(CB) CB, std::string File, std::string TweakID,
+  Expected InpAST) {
 if (!InpA

[PATCH] D56370: [clangd] Add support for type hierarchy (super types only for now)

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356445: [clangd] Add support for type hierarchy (super types 
only for now) (authored by kadircet, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D56370?vs=190755&id=191263#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D56370

Files:
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
  clang-tools-extra/trunk/clangd/ClangdLSPServer.h
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.h
  clang-tools-extra/trunk/clangd/FindSymbols.cpp
  clang-tools-extra/trunk/clangd/FindSymbols.h
  clang-tools-extra/trunk/clangd/Protocol.cpp
  clang-tools-extra/trunk/clangd/Protocol.h
  clang-tools-extra/trunk/clangd/XRefs.cpp
  clang-tools-extra/trunk/clangd/XRefs.h
  clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
  clang-tools-extra/trunk/clangd/index/SymbolCollector.h
  clang-tools-extra/trunk/test/clangd/initialize-params.test
  clang-tools-extra/trunk/test/clangd/type-hierarchy.test
  clang-tools-extra/trunk/unittests/clangd/CMakeLists.txt
  clang-tools-extra/trunk/unittests/clangd/Matchers.h
  clang-tools-extra/trunk/unittests/clangd/TypeHierarchyTests.cpp

Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
@@ -368,6 +368,7 @@
   {ExecuteCommandParams::CLANGD_APPLY_FIX_COMMAND,
ExecuteCommandParams::CLANGD_APPLY_TWEAK}},
  }},
+{"typeHierarchyProvider", true},
 );
 }
 
@@ -806,6 +807,13 @@
 std::move(Reply));
 }
 
+void ClangdLSPServer::onTypeHierarchy(
+const TypeHierarchyParams &Params,
+Callback> Reply) {
+  Server->typeHierarchy(Params.textDocument.uri.file(), Params.position,
+Params.resolve, Params.direction, std::move(Reply));
+}
+
 void ClangdLSPServer::applyConfiguration(
 const ConfigurationSettings &Settings) {
   // Per-file update to the compilation database.
@@ -885,6 +893,7 @@
   MsgHandler->bind("workspace/didChangeWatchedFiles", &ClangdLSPServer::onFileEvent);
   MsgHandler->bind("workspace/didChangeConfiguration", &ClangdLSPServer::onChangeConfiguration);
   MsgHandler->bind("textDocument/symbolInfo", &ClangdLSPServer::onSymbolInfo);
+  MsgHandler->bind("textDocument/typeHierarchy", &ClangdLSPServer::onTypeHierarchy);
   // clang-format on
 }
 
Index: clang-tools-extra/trunk/clangd/FindSymbols.h
===
--- clang-tools-extra/trunk/clangd/FindSymbols.h
+++ clang-tools-extra/trunk/clangd/FindSymbols.h
@@ -21,9 +21,10 @@
 class SymbolIndex;
 
 /// Searches for the symbols matching \p Query. The syntax of \p Query can be
-/// the non-qualified name or fully qualified of a symbol. For example, "vector"
-/// will match the symbol std::vector and "std::vector" would also match it.
-/// Direct children of scopes (namepaces, etc) can be listed with a trailing
+/// the non-qualified name or fully qualified of a symbol. For example,
+/// "vector" will match the symbol std::vector and "std::vector" would also
+/// match it. Direct children of scopes (namepaces, etc) can be listed with a
+/// trailing
 /// "::". For example, "std::" will list all children of the std namespace and
 /// "::" alone will list all children of the global namespace.
 /// \p Limit limits the number of results returned (0 means no limit).
Index: clang-tools-extra/trunk/clangd/Protocol.cpp
===
--- clang-tools-extra/trunk/clangd/Protocol.cpp
+++ clang-tools-extra/trunk/clangd/Protocol.cpp
@@ -211,6 +211,61 @@
   }
 }
 
+SymbolKind indexSymbolKindToSymbolKind(index::SymbolKind Kind) {
+  switch (Kind) {
+  case index::SymbolKind::Unknown:
+return SymbolKind::Variable;
+  case index::SymbolKind::Module:
+return SymbolKind::Module;
+  case index::SymbolKind::Namespace:
+return SymbolKind::Namespace;
+  case index::SymbolKind::NamespaceAlias:
+return SymbolKind::Namespace;
+  case index::SymbolKind::Macro:
+return SymbolKind::String;
+  case index::SymbolKind::Enum:
+return SymbolKind::Enum;
+  case index::SymbolKind::Struct:
+return SymbolKind::Struct;
+  case index::SymbolKind::Class:
+return SymbolKind::Class;
+  case index::SymbolKind::Protocol:
+return SymbolKind::Interface;
+  case index::SymbolKind::Extension:
+return SymbolKind::Interface;
+  case index::SymbolKind::Union:
+return SymbolKind::Class;
+  case index::SymbolKind::TypeAlias:
+return SymbolKind::Class;
+  case index::SymbolKind::Function:
+return SymbolKind::Function;
+  case index::Sy

[PATCH] D59481: [clangd] Count number of references while merging RefSlabs inside FileIndex

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

> I don't follow why this can over-count, FileIndex keeps only one RefSlab per 
> each file, so we can't over-count? Did you mean it will be more than #TUs ?

Yes. This is how `Symbol::References` is described in its documentation. If we 
want to change/expand the semantic, we need to make it clear in the 
documentation as well.




Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:170
   I.first->second = mergeSymbol(I.first->second, Sym);
+// We re-count number of references while merging refs from scratch.
+I.first->getSecond().References = 0;

kadircet wrote:
> ioeric wrote:
> > note that references might not always exist. SymbolCollector can be set up 
> > to not collect references.
> I thought we wouldn't make any promises about `References` in such a case, do 
> we?
Is this documented somewhere?

`References` and `RefSlab` have been two independent things. `References` 
existed before `RefSlab`. Now might be a good time to unify them, but I think 
we should think about how this is reflected in their definitions (documentation 
etc) and other producers and consumers of the structures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59481



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

[PATCH] D59457: [analyzer][NFC] Use capital variable names, move methods out-of-line, rename some in CheckerRegistry

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

In D59457#1433180 , 
@baloghadamsoftware wrote:

> Please rename the patch. Its name does not really express its content.


Good point, sorry about that.

In D59457#1433816 , @xazax.hun wrote:

> I did not check the patch yet but wanted to point out that we might not want 
> to rush about renaming all the variables until the community decides on the 
> coding guideline, see https://reviews.llvm.org/D59251


I wonder why this wasn't posted on cfe-dev. In any case, most of the variables 
are capitalized, there still is a consistency argument. Refactoring all the 
dependent patches would also be a pain in the buttocks for little gain.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59457



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

[PATCH] D59300: [clangd] Tune the fuzzy-matching algorithm

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Hi Jan,

Sure! And sorry for posting these metrics for a while (we had other patches 
mentioning them) without proper explanation.

We simulate a bunch of completions at random points in random files from our 
internal codebase.  We assume the desired completion item is the one written in 
the code.
Intuitively, the higher it's ranked the better. In an attempt to measure this, 
we compute the following metrics:

- MRR 
- `Top-N` - percentage of completions where the searched element is among the 
first `n` items.

We also independently calculate those metrics for interesting groups of 
completions:

- `OVERALL`. All completions.
- `INITIALISMS`. Completions with query (what the user typed) matching first 
characters of each segment in the desired completion item, e.g. `SI` or `SIC` 
for `SomeInterestingClass`.
- `EXPLICIT_MEMBER_ACCESS`. Desired completion item is a class member and the 
completion is in a member access expression, e.g. `vector().^push_back()`.
- `WANT_LOCAL`. Desired completion item is in the same file as the completion 
itself.
- `CROSS_NAMESPACE`. Simulated completion removes the namespace prefix, in 
addition to the identifier, e.g. we expect to complete `std::vector` not just 
`vector`.
- `WITH EXPECTED_TYPE`. Only completions in a context where expected type is 
available, e.g. `int* a = ^`.

For each of the picked positions in a file, we try to complete a prefix of the 
desired completion item of length up to `5` and the full identifier (except 
initialisms, more on them below).
E.g. for the following source code:

  int test() {
std::vector vec;
vec.^push_back(10); // say, simulation runs here
  }

We would try run simulation for the following completions: `vec.^`, `vec.p^`, 
`vec.pu^`, `vec.pus^`, `vec.push^` and `vec.push_^`.
You can see the breakdown of the metrics for each of the prefix lengths in each 
of the completion groups.
Individual metrics for a fixed length of the prefix are written in the `Filter 
length 0-5` sections.
We also try completion with the full identifier (e.g. `vec.push_back^`), 
metrics for those are written in the `Full identifiers` section.
Aggregated metrics for all completions in a group are written in the `All 
measurements` section.

The "initialisms" groups is special, for those we use first chars of the 
segments inside the desired completion item rather than the prefix, e.g. 
`vec.p^`, `vec.pb^`.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59300



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

[PATCH] D59485: [ASTImporter] Allow adding a import strategy to the ASTImporter

[Raphael Isemann via Phabricator via cfe-commits]

teemperor planned changes to this revision.
teemperor added a comment.

I think we could also refactor the strategy into a `PreImport` call. That way 
we don't break all the user-code out there and it seems less intrusive to the 
ASTImporter code. I'll update the patch.


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

https://reviews.llvm.org/D59485



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

[PATCH] D59300: [clangd] Tune the fuzzy-matching algorithm

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

@ioeric is the author of these completion metrics and evaluation tools. Eric, 
please feel free to correct me if I got something wrong or missed something.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59300



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

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 191265.
kadircet added a comment.

- Only surface diagnostics with level fatal or error


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/Compiler.h
  clangd/Diagnostics.cpp
  clangd/tool/ClangdMain.cpp
  unittests/clangd/DiagnosticsTests.cpp

Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -9,6 +9,7 @@
 #include "Annotations.h"
 #include "ClangdUnit.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
@@ -73,7 +74,6 @@
   return true;
 }
 
-
 // Helper function to make tests shorter.
 Position pos(int line, int character) {
   Position Res;
@@ -603,7 +603,163 @@
   "Add include \"x.h\" for symbol a::X");
 }
 
+ParsedAST
+build(const std::string &MainFile, const llvm::StringMap &Files,
+  llvm::Optional LimitDiagsOutsideMainFile = llvm::None) {
+  std::vector Cmd = {"clang", MainFile.c_str()};
+  ParseInputs Inputs;
+  Inputs.CompileCommand.Filename = MainFile;
+  Inputs.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
+  Inputs.CompileCommand.Directory = testRoot();
+  Inputs.Contents = Files.lookup(MainFile);
+  Inputs.FS = buildTestFS(Files);
+  Inputs.Opts = ParseOptions();
+  Inputs.Opts.LimitDiagsOutsideMainFile = LimitDiagsOutsideMainFile;
+  auto PCHs = std::make_shared();
+  auto CI = buildCompilerInvocation(Inputs);
+  assert(CI && "Failed to build compilation invocation.");
+  auto Preamble =
+  buildPreamble(MainFile, *CI,
+/*OldPreamble=*/nullptr,
+/*OldCompileCommand=*/Inputs.CompileCommand, Inputs, PCHs,
+/*StoreInMemory=*/true, /*PreambleCallback=*/nullptr);
+  auto AST = buildAST(MainFile, createInvocationFromCommandLine(Cmd), Inputs,
+  Preamble, PCHs);
+  if (!AST.hasValue()) {
+ADD_FAILURE() << "Failed to build code:\n" << Files.lookup(MainFile);
+llvm_unreachable("Failed to build TestTU!");
+  }
+  return std::move(*AST);
+}
+
+TEST(DiagsInHeaders, DiagInsideHeader) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+void foo() {})cpp");
+  std::string MainFile = testPath("main.cc");
+  llvm::StringMap Files = {{MainFile, Main.code()},
+{testPath("a.h"), "no_type_spec;"}};
+  auto FS = buildTestFS(Files);
+
+  EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+  UnorderedElementsAre(Diag(Main.range(),
+"Error in header: C++ requires a type "
+"specifier for all declarations")));
+}
+
+TEST(DiagsInHeaders, DiagInTransitiveInclude) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+void foo() {})cpp");
+  std::string MainFile = testPath("main.cc");
+  llvm::StringMap Files = {{MainFile, Main.code()},
+{testPath("a.h"), "#include \"b.h\""},
+{testPath("b.h"), "no_type_spec;"}};
+  auto FS = buildTestFS(Files);
+
+  EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+  UnorderedElementsAre(Diag(Main.range(),
+"Error in header: C++ requires a type "
+"specifier for all declarations")));
+}
+
+TEST(DiagsInHeaders, DiagInMultipleHeaders) {
+  Annotations Main(R"cpp(
+#include $a[["a.h"]]
+#include $b[["b.h"]]
+void foo() {})cpp");
+  std::string MainFile = testPath("main.cc");
+  llvm::StringMap Files = {{MainFile, Main.code()},
+{testPath("a.h"), "no_type_spec;"},
+{testPath("b.h"), "no_type_spec;"}};
+  auto FS = buildTestFS(Files);
+
+  EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range("a"), "Error in header: C++ requires a type "
+"specifier for all declarations"),
+  Diag(Main.range("b"), "Error in header: C++ requires a type "
+"specifier for all declarations")));
+}
+
+TEST(DiagsInHeaders, PreferDirectIncludeLocation) {
+  Annotations Main(R"cpp(
+#include "a.h"
+#include [["b.h"]]
+void foo() {})cpp");
+  std::string MainFile = testPath("main.cc");
+  llvm::StringMap Files = {
+  {MainFile, Main.code()},
+  {testPath("a.h"), "#include \"b.h\"\n"},
+  {testPath("b.h"), "#ifndef X\n#define X\nno_type_spec;\n#endif"}};
+  auto FS = buildTestFS(Files);
+
+  EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+  Unor

r356446 - [Tooling] Add more scope specifiers until spelling is not ambiguous.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Tue Mar 19 03:12:15 2019
New Revision: 356446

URL: http://llvm.org/viewvc/llvm-project?rev=356446&view=rev
Log:
[Tooling] Add more scope specifiers until spelling is not ambiguous.

Summary:
Previously, when the renamed spelling is ambiguous, we simply use the
full-qualfied name (with leading "::"). This patch makes it try adding
additional specifiers one at a time until name is no longer ambiguous,
which allows us to find better disambuguated spelling.

Reviewers: kadircet, gribozavr

Subscribers: jdoerfert, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Tooling/Core/Lookup.cpp
cfe/trunk/unittests/Tooling/LookupTest.cpp

Modified: cfe/trunk/lib/Tooling/Core/Lookup.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Core/Lookup.cpp?rev=356446&r1=356445&r2=356446&view=diff
==
--- cfe/trunk/lib/Tooling/Core/Lookup.cpp (original)
+++ cfe/trunk/lib/Tooling/Core/Lookup.cpp Tue Mar 19 03:12:15 2019
@@ -14,6 +14,7 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclarationName.h"
+#include "llvm/ADT/SmallVector.h"
 using namespace clang;
 using namespace clang::tooling;
 
@@ -114,35 +115,60 @@ static bool isFullyQualified(const Neste
   return false;
 }
 
-// Returns true if spelling symbol \p QName as \p Spelling in \p UseContext is
-// ambiguous. For example, if QName is "::y::bar" and the spelling is "y::bar"
-// in `UseContext` "a" that contains a nested namespace "a::y", then "y::bar"
-// can be resolved to ::a::y::bar, which can cause compile error.
+// Adds more scope specifier to the spelled name until the spelling is not
+// ambiguous. A spelling is ambiguous if the resolution of the symbol is
+// ambiguous. For example, if QName is "::y::bar", the spelling is "y::bar", 
and
+// context contains a nested namespace "a::y", then "y::bar" can be resolved to
+// ::a::y::bar in the context, which can cause compile error.
 // FIXME: consider using namespaces.
-static bool isAmbiguousNameInScope(StringRef Spelling, StringRef QName,
-   const DeclContext &UseContext) {
+static std::string disambiguateSpellingInScope(StringRef Spelling,
+   StringRef QName,
+   const DeclContext &UseContext) {
   assert(QName.startswith("::"));
+  assert(QName.endswith(Spelling));
   if (Spelling.startswith("::"))
-return false;
+return Spelling;
 
-  // Lookup the first component of Spelling in all enclosing namespaces and
-  // check if there is any existing symbols with the same name but in different
-  // scope.
-  StringRef Head = Spelling.split("::").first;
+  auto UnspelledSpecifier = QName.drop_back(Spelling.size());
+  llvm::SmallVector UnspelledScopes;
+  UnspelledSpecifier.split(UnspelledScopes, "::", /*MaxSplit=*/-1,
+   /*KeepEmpty=*/false);
 
-  llvm::SmallVector UseNamespaces =
+  llvm::SmallVector EnclosingNamespaces =
   getAllNamedNamespaces(&UseContext);
   auto &AST = UseContext.getParentASTContext();
   StringRef TrimmedQName = QName.substr(2);
-  for (const auto *NS : UseNamespaces) {
-auto LookupRes = NS->lookup(DeclarationName(&AST.Idents.get(Head)));
-if (!LookupRes.empty()) {
-  for (const NamedDecl *Res : LookupRes)
-if (!TrimmedQName.startswith(Res->getQualifiedNameAsString()))
-  return true;
+
+  auto IsAmbiguousSpelling = [&EnclosingNamespaces, &AST, &TrimmedQName](
+ const llvm::StringRef CurSpelling) {
+if (CurSpelling.startswith("::"))
+  return false;
+// Lookup the first component of Spelling in all enclosing namespaces
+// and check if there is any existing symbols with the same name but in
+// different scope.
+StringRef Head = CurSpelling.split("::").first;
+for (const auto *NS : EnclosingNamespaces) {
+  auto LookupRes = NS->lookup(DeclarationName(&AST.Idents.get(Head)));
+  if (!LookupRes.empty()) {
+for (const NamedDecl *Res : LookupRes)
+  if (!TrimmedQName.startswith(Res->getQualifiedNameAsString()))
+return true;
+  }
+}
+return false;
+  };
+
+  // Add more qualifiers until the spelling is not ambiguous.
+  std::string Disambiguated = Spelling;
+  while (IsAmbiguousSpelling(Disambiguated)) {
+if (UnspelledScopes.empty()) {
+  Disambiguated = "::" + Disambiguated;
+} else {
+  Disambiguated = (UnspelledScopes.back() + "::" + Disambiguated).str();
+  UnspelledScopes.pop_back();
 }
   }
-  return false;
+  return Disambiguated;
 }
 
 std::string tooling::replaceNestedName(const NestedNameSpecifier *Use,
@@ -179,12 +205,6 @@ std::string tooling::replaceNestedName(c
   // specific).
   StringRef Suggested = getBestNamespaceSubstr(UseContext, ReplacementString,

[PATCH] D59487: [Tooling] Add more scope specifiers until spelling is not ambiguous.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
ioeric marked an inline comment as done.
Closed by commit rL356446: [Tooling] Add more scope specifiers until spelling 
is not ambiguous. (authored by ioeric, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59487

Files:
  cfe/trunk/lib/Tooling/Core/Lookup.cpp
  cfe/trunk/unittests/Tooling/LookupTest.cpp

Index: cfe/trunk/lib/Tooling/Core/Lookup.cpp
===
--- cfe/trunk/lib/Tooling/Core/Lookup.cpp
+++ cfe/trunk/lib/Tooling/Core/Lookup.cpp
@@ -14,6 +14,7 @@
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclarationName.h"
+#include "llvm/ADT/SmallVector.h"
 using namespace clang;
 using namespace clang::tooling;
 
@@ -114,35 +115,60 @@
   return false;
 }
 
-// Returns true if spelling symbol \p QName as \p Spelling in \p UseContext is
-// ambiguous. For example, if QName is "::y::bar" and the spelling is "y::bar"
-// in `UseContext` "a" that contains a nested namespace "a::y", then "y::bar"
-// can be resolved to ::a::y::bar, which can cause compile error.
+// Adds more scope specifier to the spelled name until the spelling is not
+// ambiguous. A spelling is ambiguous if the resolution of the symbol is
+// ambiguous. For example, if QName is "::y::bar", the spelling is "y::bar", and
+// context contains a nested namespace "a::y", then "y::bar" can be resolved to
+// ::a::y::bar in the context, which can cause compile error.
 // FIXME: consider using namespaces.
-static bool isAmbiguousNameInScope(StringRef Spelling, StringRef QName,
-   const DeclContext &UseContext) {
+static std::string disambiguateSpellingInScope(StringRef Spelling,
+   StringRef QName,
+   const DeclContext &UseContext) {
   assert(QName.startswith("::"));
+  assert(QName.endswith(Spelling));
   if (Spelling.startswith("::"))
-return false;
+return Spelling;
 
-  // Lookup the first component of Spelling in all enclosing namespaces and
-  // check if there is any existing symbols with the same name but in different
-  // scope.
-  StringRef Head = Spelling.split("::").first;
+  auto UnspelledSpecifier = QName.drop_back(Spelling.size());
+  llvm::SmallVector UnspelledScopes;
+  UnspelledSpecifier.split(UnspelledScopes, "::", /*MaxSplit=*/-1,
+   /*KeepEmpty=*/false);
 
-  llvm::SmallVector UseNamespaces =
+  llvm::SmallVector EnclosingNamespaces =
   getAllNamedNamespaces(&UseContext);
   auto &AST = UseContext.getParentASTContext();
   StringRef TrimmedQName = QName.substr(2);
-  for (const auto *NS : UseNamespaces) {
-auto LookupRes = NS->lookup(DeclarationName(&AST.Idents.get(Head)));
-if (!LookupRes.empty()) {
-  for (const NamedDecl *Res : LookupRes)
-if (!TrimmedQName.startswith(Res->getQualifiedNameAsString()))
-  return true;
+
+  auto IsAmbiguousSpelling = [&EnclosingNamespaces, &AST, &TrimmedQName](
+ const llvm::StringRef CurSpelling) {
+if (CurSpelling.startswith("::"))
+  return false;
+// Lookup the first component of Spelling in all enclosing namespaces
+// and check if there is any existing symbols with the same name but in
+// different scope.
+StringRef Head = CurSpelling.split("::").first;
+for (const auto *NS : EnclosingNamespaces) {
+  auto LookupRes = NS->lookup(DeclarationName(&AST.Idents.get(Head)));
+  if (!LookupRes.empty()) {
+for (const NamedDecl *Res : LookupRes)
+  if (!TrimmedQName.startswith(Res->getQualifiedNameAsString()))
+return true;
+  }
+}
+return false;
+  };
+
+  // Add more qualifiers until the spelling is not ambiguous.
+  std::string Disambiguated = Spelling;
+  while (IsAmbiguousSpelling(Disambiguated)) {
+if (UnspelledScopes.empty()) {
+  Disambiguated = "::" + Disambiguated;
+} else {
+  Disambiguated = (UnspelledScopes.back() + "::" + Disambiguated).str();
+  UnspelledScopes.pop_back();
 }
   }
-  return false;
+  return Disambiguated;
 }
 
 std::string tooling::replaceNestedName(const NestedNameSpecifier *Use,
@@ -179,12 +205,6 @@
   // specific).
   StringRef Suggested = getBestNamespaceSubstr(UseContext, ReplacementString,
isFullyQualified(Use));
-  // Use the fully qualified name if the suggested name is ambiguous.
-  // FIXME: consider re-shortening the name until the name is not ambiguous. We
-  // are not doing this because ambiguity is pretty bad and we should not try to
-  // be clever in handling such cases. Making this noticeable to users seems to
-  // be a better option.
-  return isAmbiguousNameInSc

[PATCH] D59354: [clangd] Print arguments in template specializations

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.

LGTM with a few NITs.
Thanks for fixing this!




Comment at: clang-tools-extra/clangd/AST.cpp:86
+auto TL = Cls->getTypeAsWritten()->getTypeLoc();
+if (auto STL = TL.getAs()) {
+  llvm::SmallVector ArgLocs;

NIT: maybe inline `TL`?
```
if (auto STL = 
Cls->getTypeAsWritten()->getTypeLoc().getAs()) {
  /// ...
}
```



Comment at: clang-tools-extra/clangd/AST.cpp:89
+  ArgLocs.reserve(STL.getNumArgs());
+  for (unsigned I = 0; I < STL.getNumArgs(); I++)
+ArgLocs.push_back(STL.getArgLoc(I));

Tiny NIT: [[ https://llvm.org/docs/CodingStandards.html#prefer-preincrement | 
prefer preincrement ]]



Comment at: clang/lib/AST/TypePrinter.cpp:1635
 
+static void printArgument(const TemplateArgument &A, const PrintingPolicy &PP,
+  llvm::raw_ostream &OS) {

NIT: consider also adding a test in the clang code.
Some developers don't build `tools-extra`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59354



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

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In D59302#1431274 , @kadircet wrote:

> The optional part is rather limiting number of diagnostics that is going to 
> be surfaced. So if you have thousands of errors inside preamble only first 
> 100 of them will appear inside main file by default.
>  But looking at it again, currently there is no way for user to say "do not 
> limit number of diagnostics" :D They can only change this limit to different 
> values.


Let's pick a number and hard-code it. We'll get consistent behavior in all 
clients of clangd, I can hardly think of any existing client that would want to 
surface everything to the user.
FWIW, `100` seem too much to my taste, since we're only showing errors. Would 
`10` be a better limit?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302



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

[PATCH] D59527: [clang-format] Don't insert break between JS template string and tag identifier

[Martin Probst via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59527



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

[PATCH] D59481: [clangd] Count number of references while merging RefSlabs inside FileIndex

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked an inline comment as done.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:170
   I.first->second = mergeSymbol(I.first->second, Sym);
+// We re-count number of references while merging refs from scratch.
+I.first->getSecond().References = 0;

ioeric wrote:
> kadircet wrote:
> > ioeric wrote:
> > > note that references might not always exist. SymbolCollector can be set 
> > > up to not collect references.
> > I thought we wouldn't make any promises about `References` in such a case, 
> > do we?
> Is this documented somewhere?
> 
> `References` and `RefSlab` have been two independent things. `References` 
> existed before `RefSlab`. Now might be a good time to unify them, but I think 
> we should think about how this is reflected in their definitions 
> (documentation etc) and other producers and consumers of the structures.
Both of them are only produced by SymbolCollector, which has an option 
`CountReferences` with a comment saying `// Populate the Symbol.References 
field.`.
Unfortunately that's not what it does, instead it always sets 
`Symol.References` field to `1` for every symbol collected during indexing of a 
single TU.
Then it is up to the consumers to merge those `1`s. So even though we say:
```
/// The number of translation units that reference this symbol from their main
/// file. This number is only meaningful if aggregated in an index.```
in the comments of `Symbol::References` it is totally up-to-the consumer who 
performs the merging.

The option controls whether Symbol Occurences should be collected or not.

I see two possible options:
 - Un-tie SymbolCollector from Symbol::References and rather define it as:
```
/// This field is for the convenience of an aggregated symbol index
```
 - Rather change Index structure's(consumers of this information) to store(and 
build) this information internally if they plan to make use of it ?

WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59481



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

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/ClangdUnit.cpp:256
+// directive inside main file.
+// If a header is transitively included in multiple direct includes of main, we
+// choose the first one.

We should find a way to point into an exact `include` directive (source 
locations have this information).




Comment at: clangd/ClangdUnit.cpp:406
+  for (const Diag &D : ASTDiags.take()) {
+if (!mentionsMainFile(D) && Opts.LimitDiagsOutsideMainFile &&
+++DiagsOutsideMainFile > *Opts.LimitDiagsOutsideMainFile)

Could we limit per-include-directive instead?
The limit to avoid creating too verbose error messages, rather than reporting 
too many messages, right?



Comment at: clangd/ClangdUnit.cpp:438
+  D.File = MainInput.getFile();
+  D.Message = "Error in header: " + D.Message;
+}

We should provide the include stack, similar to how compiler does it.
Otherwise it would be really hard to figure out the exact problem the 
diagnostic is pointing at.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302



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

[PATCH] D59485: [ASTImporter] Allow adding a import strategy to the ASTImporter

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

Another observation: The `Import` function of the strategy now has no way to 
return an error. An even better version of it would be to include the 
possibility of import error (with `ImportError`, or other error type). Or the 
"PreImport" function could indicate if the Decl is handled by the strategy, and 
`Import` is called only if yes and `Import` can return an `Expected`. 
(The PostImport is called for every successfully imported Decl.)


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

https://reviews.llvm.org/D59485



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

[PATCH] D59485: [ASTImporter] Allow adding a import strategy to the ASTImporter

[Balázs Kéri via Phabricator via cfe-commits]

balazske added a comment.

By the way if the `Import` of the strategy uses recursive import of other 
things this can cause same problems as it was in `ASTImporter` before the 
`GetImportedOrCreateDecl` was introduced. So this should be avoided or 
something similar to `GetImportedOrCreateDecl` must be performed, maybe that 
function can be made public somehow.

But if this strategy is used only for special cases like in the test case 
(replace a specific Decl with something other) probably implementing this 
functionality in another way is a more simple solution.


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

https://reviews.llvm.org/D59485



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

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: JonasToth, aaron.ballman, alexfh, 
michaelplatings.
MyDeveloperDay added a project: clang-tools-extra.
Herald added subscribers: jdoerfert, xazax.hun.

While testing clang-tidy for D59251: [Documentation] Proposal for plan to 
change variable names  I found that a lambda 
capture could get incorrectly get transformed by readability-identifier-naming 
when run with -fix

The following code:

  bool foo() {
bool Columns=false;
auto ptr=[&] {
return Columns;
  }();
return true;
  }

would get transformed to  (replace `[&]` with `[columns]`

  bool foo() {
bool columns=false;
auto ptr=[columns] {
return columns;
  }();
return true;
  }

This is caused by the presence of a declrefexpr in the LambdaExpr, seeming 
having the location of the [&] (line 3 column 13), but also having the Name 
"Columns"

| -DeclRefExpr  'bool' lvalue Var 0x55f0145f1c80 'Columns' 'bool' |
|



  LambdaExpr  '(lambda at line:3:12)'
|-CXXRecordDecl  col:12 implicit class definition
| |-DefinitionData lambda pass_in_registers 
trivially_copyable can_const_default_init
| | |-DefaultConstructor
| | |-CopyConstructor simple trivial has_const_param 
needs_implicit implicit_has_const_param
| | |-MoveConstructor exists simple trivial needs_implicit
| | |-CopyAssignment trivial has_const_param needs_implicit 
implicit_has_const_param
| | |-MoveAssignment
| | `-Destructor simple irrelevant trivial
| |-FieldDecl  col:18 implicit 'bool &'
| |-CXXMethodDecl  line:3:12 used 
operator() 'auto () const -> bool' inline
| | `-CompoundStmt 
| |   `-ReturnStmt 
| | `-ImplicitCastExpr  'bool' 
| |   `-DeclRefExpr  'bool' lvalue Var 
0x55f0145f1c80 'Columns' 'bool'
| `-CXXDestructorDecl  col:12 implicit 
referenced ~ 'void () noexcept' inline default trivial
|-DeclRefExpr  'bool' lvalue Var 0x55f0145f1c80 
'Columns' 'bool'
`-CompoundStmt 
  `-ReturnStmt 
`-ImplicitCastExpr  'bool' 
  `-DeclRefExpr  'bool' lvalue Var 
0x55f0145f1c80 'Columns' 'bool'

The following code tries to detect the DeclRefExpr in the LambdaExpr, and not 
add this to the usage map

This issue is logged as https://bugs.llvm.org/show_bug.cgi?id=41119

I have retested this against lib/Clang/Format and this issue is resolved.

  // Don't use this Format, if the difference between the longest and shortest
  // element in a column exceeds a threshold to avoid excessive spaces.
  if ([&] {
for (unsigned i = 0; i < columns - 1; ++i)
  if (format.ColumnSizes[i] - minSizeInColumn[i] > 10)
return true;
return false;
  }())
continue;




https://reviews.llvm.org/D59540

Files:
  clang-tidy/readability/IdentifierNamingCheck.cpp
  test/clang-tidy/readability-identifier-naming.cpp


Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -501,3 +501,13 @@
 // CHECK-FIXES: {{^}}int * const lc_PointerB = nullptr;{{$}}
 }
 
+
+bool Foo() {
+  bool Columns=false;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local 
variable 'Columns'
+// CHECK-FIXES: {{^}}  bool columns=false;
+  auto ptr=[&]{return Columns;}();
+// CHECK-FIXES: {{^}}  auto ptr=[&]{return columns;}();
+  return Columns;
+// CHECK-FIXES: {{^}}  return columns;
+}
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -787,10 +787,22 @@
   }
 
   if (const auto *DeclRef = Result.Nodes.getNodeAs("declRef")) {
-SourceRange Range = DeclRef->getNameInfo().getSourceRange();
-addUsage(NamingCheckFailures, DeclRef->getDecl(), Range,
- Result.SourceManager);
-return;
+const auto &Parents = Result.Context->getParents(*DeclRef);
+bool LambdaDeclRef = false;
+
+if (!Parents.empty()) {
+  const Stmt *ST = Parents[0].get();
+
+  if (ST && isa(ST))
+LambdaDeclRef = true;
+}
+
+if (!LambdaDeclRef) {
+  SourceRange Range = DeclRef->getNameInfo().getSourceRange();
+  addUsage(NamingCheckFailures, DeclRef->getDecl(), Range,
+   Result.SourceManager);
+  return;
+}
   }
 
   if (const auto *Decl = Result.Nodes.getNodeAs("decl")) {


Index: test/clang-tidy/readability-identifier-na

r356447 - [clang-format] [JS] Don't break between template string and tag

[Martin Probst via cfe-commits]

Author: mprobst
Date: Tue Mar 19 04:15:52 2019
New Revision: 356447

URL: http://llvm.org/viewvc/llvm-project?rev=356447&view=rev
Log:
[clang-format] [JS] Don't break between template string and tag

Before:
const x = veryLongIdentifier
`hello`;
After:
const x =
veryLongIdentifier`hello`;

While it's allowed to have the template string and tag identifier
separated by a line break, currently the clang-format output is not
stable when a break is forced. Additionally, disallowing a line break
makes it clear that the identifier is actually a tag for a template
string.

Patch originally by mitchellwills (thanks!).

Modified:
cfe/trunk/lib/Format/TokenAnnotator.cpp
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=356447&r1=356446&r2=356447&view=diff
==
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Tue Mar 19 04:15:52 2019
@@ -3171,6 +3171,11 @@ bool TokenAnnotator::canBreakBefore(cons
   return false; // must not break in "module foo { ...}"
 if (Right.is(TT_TemplateString) && Right.closesScope())
   return false;
+// Don't split tagged template literal so there is a break between the tag
+// identifier and template string.
+if (Left.is(tok::identifier) && Right.is(TT_TemplateString)) {
+  return false;
+}
 if (Left.is(TT_TemplateString) && Left.opensScope())
   return true;
   }

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=356447&r1=356446&r2=356447&view=diff
==
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Tue Mar 19 04:15:52 2019
@@ -1962,6 +1962,12 @@ TEST_F(FormatTestJS, NestedTemplateStrin
 TEST_F(FormatTestJS, TaggedTemplateStrings) {
   verifyFormat("var x = html``;");
   verifyFormat("yield `hello`;");
+  verifyFormat("var f = {\n"
+   "  param: longTagName`This is a ${\n"
+   "'really'} long line`\n"
+   "};",
+   "var f = {param: longTagName`This is a ${'really'} long 
line`};",
+   getGoogleJSStyleWithColumns(40));
 }
 
 TEST_F(FormatTestJS, CastSyntax) {


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

[PATCH] D59527: [clang-format] Don't insert break between JS template string and tag identifier

[Martin Probst via Phabricator via cfe-commits]

mprobst closed this revision.
mprobst added a comment.

Landed as r356447.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59527



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

[PATCH] D59516: [analyzer] Make GenericTaintChecker configurable

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus requested changes to this revision.
Szelethus added a comment.
This revision now requires changes to proceed.

I'm very much guilty of doing functional and refactoring changes within the 
same patch, but I think working on `GenericTaintChecker` AND in the same patch 
doing (seemingly unrelated) function name changes in `ProgramState` might be 
overkill -- Could you please divide this patch into smaller parts please?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59516



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

[PATCH] D59492: [OpenCL] Allow variadic macros as Clang feature

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D59492#1433796 , @arsenm wrote:

> Should it be downgraded to a warning about an extension instead of just 
> removing it?


What would you suggest to put in a warning message? Clang normally doesn't warn 
about extensions...


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

https://reviews.llvm.org/D59492



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

[PATCH] D59292: [clang-format] messes up indentation when using JavaScript private fields and methods

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added subscribers: mitchellwills, mprobst.
MyDeveloperDay added a comment.

ping @mprobst,@mitchellwills  noticed you just reviewed another clang-format-js 
commit, wondered if you were interested in reviewing this for me?, finding it 
hard to get any review traction.


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

https://reviews.llvm.org/D59292



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

[PATCH] D59292: [clang-format] messes up indentation when using JavaScript private fields and methods

[Martin Probst via Phabricator via cfe-commits]

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

Actually I'll fix the nits.




Comment at: clang/docs/ReleaseNotes.rst:174
 
-- ...
+- Add support for correct indenting of private fields and methods in 
Javascript.
 

Honestly, I think we don't really do release notes for clang-format. Revert 
this, or I'll drop it when committing your change.



Comment at: clang/lib/Format/FormatToken.h:66
   TYPE(JsTypeOptionalQuestion) 
\
+  TYPE(JsPrivateIdentifier)
\
   TYPE(LambdaArrow)
\

nit: please keep alpha sorted, i.e. before Js[T]


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

https://reviews.llvm.org/D59292



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

[PATCH] D59486: [OpenCL] Improve testing of default header in C++ mode

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia marked an inline comment as done.
Anastasia added inline comments.



Comment at: test/Headers/opencl-c-header.cl:57-65
 char f(char x) {
-#if __OPENCL_C_VERSION__ != CL_VERSION_2_0
+#if !defined(__OPENCL_CPP_VERSION__) && (__OPENCL_C_VERSION__ != 
CL_VERSION_2_0)
   return convert_char_rte(x);
 
 #else //__OPENCL_C_VERSION__
   ndrange_t t;
   return ctz(x);

bader wrote:
> This test looks strange.
> It checks that convert_char_rte is compiled if OpenCL version is not 2.0, but 
> AFAIK it's not deprecated in OpenCL C 2.0, so I don't know why test is 
> written in this way (i.e. ifdef-else-endif).
> 
> I think checks should look like this:
> ```
> char f(char x) {
> // Check check OpenCL C 2.0 and OpenCL C++ functionality 
> #if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ == 
> CL_VERSION_2_0)
>   ndrange_t t;
>   x = ctz(x);
> #endif
>   return convert_char_rte(x);
> }
> ```
> 
> Probably it's better to fix separately.
Ok, I think the idea was just to have 2 unique outputs to be checked that we 
can detect CL2.0 or earlier. But I guess we can do the same with the structure 
you are suggesting too. I don't mind changing it.

I have to say the header testing is so ad-hoc at the moment and I would like to 
improve it but because it's so costly we probably need this to be done 
conditionally. I was thinking to activate it with an extra flag in cmake. 
Anyway, this is something for the future.


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

https://reviews.llvm.org/D59486



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

[PATCH] D55170: [clang-format]: Add NonEmptyParentheses spacing option

[MyDeveloperDay via Phabricator via cfe-commits]

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

LGTM

(landed here too https://github.com/mydeveloperday/clang-experimental/releases)


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

https://reviews.llvm.org/D55170



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

r356449 - [clang-format] [JS] handle private members.

[Martin Probst via cfe-commits]

Author: mprobst
Date: Tue Mar 19 05:28:41 2019
New Revision: 356449

URL: http://llvm.org/viewvc/llvm-project?rev=356449&view=rev
Log:
[clang-format] [JS] handle private members.

Addresses PR40999 https://bugs.llvm.org/show_bug.cgi?id=40999

Private fields and methods in JavaScript would get incorrectly indented
(it sees them as preprocessor directives and hence left aligns them)

In this revision `#identifier` tokens `tok::hash->tok::identifier` are
merged into a single new token `tok::identifier` with the `#` contained
inside the TokenText.

Before:

```
class Example {
  pub = 1;

  static pub2 = "foo";
  static #priv2 = "bar";

  method() { this.#priv = 5; }

  static staticMethod() {
switch (this.#priv) {
case '1':
  break;
}
  }

  this.#privateMethod(); // infinite loop
}

static #staticPrivateMethod() {}
}
```

After this fix the code will be correctly indented

```
class Example {
  pub = 1;
  #priv = 2;

  static pub2 = "foo";
  static #priv2 = "bar";

  method() { this.#priv = 5; }

  static staticMethod() {
switch (this.#priv) {
case '1':
  #priv = 3;
  break;
}
  }

  #privateMethod() {
this.#privateMethod(); // infinite loop
  }

  static #staticPrivateMethod() {}
}
```

NOTE: There might be some JavaScript code out there which uses the C
processor to preprocess .js files
http://www.nongnu.org/espresso/js-cpp.html. It's not clear how this
revision or even private fields and methods would interact.

Patch originally by MyDeveloperDays (thanks!).

Modified:
cfe/trunk/lib/Format/FormatToken.h
cfe/trunk/lib/Format/FormatTokenLexer.cpp
cfe/trunk/lib/Format/FormatTokenLexer.h
cfe/trunk/unittests/Format/FormatTestJS.cpp

Modified: cfe/trunk/lib/Format/FormatToken.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatToken.h?rev=356449&r1=356448&r2=356449&view=diff
==
--- cfe/trunk/lib/Format/FormatToken.h (original)
+++ cfe/trunk/lib/Format/FormatToken.h Tue Mar 19 05:28:41 2019
@@ -60,6 +60,7 @@ namespace format {
   TYPE(JsExponentiationEqual)  
\
   TYPE(JsFatArrow) 
\
   TYPE(JsNonNullAssertion) 
\
+  TYPE(JsPrivateIdentifier)
\
   TYPE(JsTypeColon)
\
   TYPE(JsTypeOperator) 
\
   TYPE(JsTypeOptionalQuestion) 
\

Modified: cfe/trunk/lib/Format/FormatTokenLexer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatTokenLexer.cpp?rev=356449&r1=356448&r2=356449&view=diff
==
--- cfe/trunk/lib/Format/FormatTokenLexer.cpp (original)
+++ cfe/trunk/lib/Format/FormatTokenLexer.cpp Tue Mar 19 05:28:41 2019
@@ -95,6 +95,8 @@ void FormatTokenLexer::tryMergePreviousT
   Tokens.back()->Tok.setKind(tok::starequal);
   return;
 }
+if (tryMergeJSPrivateIdentifier())
+  return;
   }
 
   if (Style.Language == FormatStyle::LK_Java) {
@@ -120,6 +122,25 @@ bool FormatTokenLexer::tryMergeNSStringL
   Tokens.erase(Tokens.end() - 1);
   return true;
 }
+
+bool FormatTokenLexer::tryMergeJSPrivateIdentifier() {
+  // Merges #idenfier into a single identifier with the text #identifier
+  // but the token tok::identifier.
+  if (Tokens.size() < 2)
+return false;
+  auto &Hash = *(Tokens.end() - 2);
+  auto &Identifier = *(Tokens.end() - 1);
+  if (!Hash->is(tok::hash) || !Identifier->is(tok::identifier))
+return false;
+  Hash->Tok.setKind(tok::identifier);
+  Hash->TokenText =
+  StringRef(Hash->TokenText.begin(),
+Identifier->TokenText.end() - Hash->TokenText.begin());
+  Hash->ColumnWidth += Identifier->ColumnWidth;
+  Hash->Type = TT_JsPrivateIdentifier;
+  Tokens.erase(Tokens.end() - 1);
+  return true;
+}
 
 bool FormatTokenLexer::tryMergeLessLess() {
   // Merge X,less,less,Y into X,lessless,Y unless X or Y is less.

Modified: cfe/trunk/lib/Format/FormatTokenLexer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/FormatTokenLexer.h?rev=356449&r1=356448&r2=356449&view=diff
==
--- cfe/trunk/lib/Format/FormatTokenLexer.h (original)
+++ cfe/trunk/lib/Format/FormatTokenLexer.h Tue Mar 19 05:28:41 2019
@@ -48,6 +48,7 @@ private:
 
   bool tryMergeLessLess();
   bool tryMergeNSStringLiteral();
+  bool tryMergeJSPrivateIdentifier();
 
   bool tryMergeTokens(ArrayRef Kinds, TokenType NewType);
 

Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=356449&r1=356448&r2=356449&view=diff
=

[PATCH] D59292: [clang-format] messes up indentation when using JavaScript private fields and methods

[Martin Probst via Phabricator via cfe-commits]

mprobst closed this revision.
mprobst added a comment.

Landed in r356449, thanks!


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

https://reviews.llvm.org/D59292



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

[PATCH] D59485: [ASTImporter] Allow adding a import strategy to the ASTImporter

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> We can't reliable import templates with the ASTImporter, so actually 
> reconstructing the template in the debug info AST and then importing it 
> doesn't work.

Could you please elaborate how the import of templates fails in ASTImporter? Is 
it because the AST you build from the DWARF is incomplete? Or is there a lookup 
problem? Is there an assertion in one of the CodeGen functions once the import 
is finished?
If we could fix that error in the ASTImporter then other clients of it (like 
CTU) could benefit from the fix too. Perhaps, the best would be to have a test 
which reproduces the template import related errors, maybe a lit test with 
clang-import-test?

If the crux of the problem is because of the incomplete AST from DWARF then 
probably we cannot fix that here in clang::ASTImporter. However, if that is not 
the case then I think we should try to fix the error here instead of 
duplicating lookup and import logic in LLDB. (As for the lookup problems, quite 
recently I just  have discovered, that lookup during expression evaluation in 
LLDB does not work properly, I'll communicate this in a separate email/patch.)

> It would also be much slower.

Could you please explain why it would be slower?


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

https://reviews.llvm.org/D59485



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

r356450 - [OpenCL] Improved testing of default header.

[Anastasia Stulova via cfe-commits]

Author: stulova
Date: Tue Mar 19 06:04:17 2019
New Revision: 356450

URL: http://llvm.org/viewvc/llvm-project?rev=356450&view=rev
Log:
[OpenCL] Improved testing of default header.

Improved some checks and moved testing of the default header
in C++ mode into the Headers folder.

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


Modified:
cfe/trunk/test/Driver/include-default-header.cl
cfe/trunk/test/Headers/opencl-c-header.cl
cfe/trunk/test/SemaOpenCL/builtin.cl
cfe/trunk/test/SemaOpenCL/extensions.cl

Modified: cfe/trunk/test/Driver/include-default-header.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/include-default-header.cl?rev=356450&r1=356449&r2=356450&view=diff
==
--- cfe/trunk/test/Driver/include-default-header.cl (original)
+++ cfe/trunk/test/Driver/include-default-header.cl Tue Mar 19 06:04:17 2019
@@ -1,5 +1,6 @@
-// RUN: %clang -save-temps -x cl -Xclang -cl-std=CL2.0 -Xclang 
-finclude-default-header -emit-llvm -S -### %s
-// CHECK-NOT: finclude-default-header
+// RUN: %clang -save-temps -x cl -Xclang -cl-std=CL2.0 -Xclang 
-finclude-default-header -emit-llvm -S -### %s 2>&1 | FileCheck %s
+
+// CHECK-LABEL: finclude-default-header
 // Make sure we don't pass -finclude-default-header to any commands other than 
the driver.
 
 void test() {}

Modified: cfe/trunk/test/Headers/opencl-c-header.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/opencl-c-header.cl?rev=356450&r1=356449&r2=356450&view=diff
==
--- cfe/trunk/test/Headers/opencl-c-header.cl (original)
+++ cfe/trunk/test/Headers/opencl-c-header.cl Tue Mar 19 06:04:17 2019
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem 
../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -verify | FileCheck %s
-// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem 
../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -verify -cl-std=CL1.1| 
FileCheck %s
-// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem 
../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -verify -cl-std=CL1.2| 
FileCheck %s
+// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem 
../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -verify -cl-std=CL1.1 
| FileCheck %s
+// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem 
../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -verify -cl-std=CL1.2 
| FileCheck %s
+// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem 
../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -verify -cl-std=c++ | 
FileCheck %s --check-prefix=CHECK20
 
 // Test including the default header as a module.
 // The module should be compiled only once and loaded from cache afterwards.
@@ -54,7 +55,7 @@
 // CHECK20: _Z3ctzc
 // CHECK20-NOT: _Z16convert_char_rtec
 char f(char x) {
-#if __OPENCL_C_VERSION__ != CL_VERSION_2_0
+#if !defined(__OPENCL_CPP_VERSION__) && (__OPENCL_C_VERSION__ != 
CL_VERSION_2_0)
   return convert_char_rte(x);
 
 #else //__OPENCL_C_VERSION__
@@ -67,7 +68,7 @@ char f(char x) {
 // from OpenCL 2.0 onwards.
 
 // CHECK20: _Z12write_imagef14ocl_image3d_wo
-#if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
 void test_image3dwo(write_only image3d_t img) {
   write_imagef(img, (0), (0.0f));
 }
@@ -75,7 +76,7 @@ void test_image3dwo(write_only image3d_t
 
 // Verify that non-builtin cl_intel_planar_yuv extension is defined from
 // OpenCL 1.2 onwards.
-#if (__OPENCL_C_VERSION__ >= CL_VERSION_1_2)
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_1_2)
 // expected-no-diagnostics
 #ifndef cl_intel_planar_yuv
 #error "Missing cl_intel_planar_yuv define"

Modified: cfe/trunk/test/SemaOpenCL/builtin.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/builtin.cl?rev=356450&r1=356449&r2=356450&view=diff
==
--- cfe/trunk/test/SemaOpenCL/builtin.cl (original)
+++ cfe/trunk/test/SemaOpenCL/builtin.cl Tue Mar 19 06:04:17 2019
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL1.2
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=c++
 
 // expected-no-diagnostics
 

Modified: cfe/trunk/test/SemaOpenCL/extensions.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaOpenCL/extensions.cl?rev=356450&r1=356449&r2=356450&view=diff
==
--- cfe/trunk/test/SemaOpenCL/extensions.cl (original)
+++ cfe/trunk/test/SemaOpenCL/extensions.cl Tue Mar 19 06:04:17 2019
@@ -28,7 +28,7 @@
 // enabled by default with -cl-std=CL2.0).
 //
 // RUN: %clang_cc1 %s -triple amdgcn-unknown-unknown -verify -pedantic 
-fsynt

[PATCH] D59486: [OpenCL] Improve testing of default header in C++ mode

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC356450: [OpenCL] Improved testing of default header. 
(authored by stulova, committed by ).
Herald added a project: clang.

Repository:
  rC Clang

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

https://reviews.llvm.org/D59486

Files:
  test/Driver/include-default-header.cl
  test/Headers/opencl-c-header.cl
  test/SemaOpenCL/builtin.cl
  test/SemaOpenCL/extensions.cl


Index: test/Headers/opencl-c-header.cl
===
--- test/Headers/opencl-c-header.cl
+++ test/Headers/opencl-c-header.cl
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem 
../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -verify | FileCheck %s
-// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem 
../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -verify -cl-std=CL1.1| 
FileCheck %s
-// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem 
../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -verify -cl-std=CL1.2| 
FileCheck %s
+// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem 
../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -verify -cl-std=CL1.1 
| FileCheck %s
+// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem 
../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -verify -cl-std=CL1.2 
| FileCheck %s
+// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem 
../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -verify -cl-std=c++ | 
FileCheck %s --check-prefix=CHECK20
 
 // Test including the default header as a module.
 // The module should be compiled only once and loaded from cache afterwards.
@@ -54,7 +55,7 @@
 // CHECK20: _Z3ctzc
 // CHECK20-NOT: _Z16convert_char_rtec
 char f(char x) {
-#if __OPENCL_C_VERSION__ != CL_VERSION_2_0
+#if !defined(__OPENCL_CPP_VERSION__) && (__OPENCL_C_VERSION__ != 
CL_VERSION_2_0)
   return convert_char_rte(x);
 
 #else //__OPENCL_C_VERSION__
@@ -67,7 +68,7 @@
 // from OpenCL 2.0 onwards.
 
 // CHECK20: _Z12write_imagef14ocl_image3d_wo
-#if __OPENCL_C_VERSION__ >= CL_VERSION_2_0
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_2_0)
 void test_image3dwo(write_only image3d_t img) {
   write_imagef(img, (0), (0.0f));
 }
@@ -75,7 +76,7 @@
 
 // Verify that non-builtin cl_intel_planar_yuv extension is defined from
 // OpenCL 1.2 onwards.
-#if (__OPENCL_C_VERSION__ >= CL_VERSION_1_2)
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ >= CL_VERSION_1_2)
 // expected-no-diagnostics
 #ifndef cl_intel_planar_yuv
 #error "Missing cl_intel_planar_yuv define"
Index: test/SemaOpenCL/builtin.cl
===
--- test/SemaOpenCL/builtin.cl
+++ test/SemaOpenCL/builtin.cl
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=CL1.2
+// RUN: %clang_cc1 %s -verify -pedantic -fsyntax-only -cl-std=c++
 
 // expected-no-diagnostics
 
Index: test/SemaOpenCL/extensions.cl
===
--- test/SemaOpenCL/extensions.cl
+++ test/SemaOpenCL/extensions.cl
@@ -28,7 +28,7 @@
 // enabled by default with -cl-std=CL2.0).
 //
 // RUN: %clang_cc1 %s -triple amdgcn-unknown-unknown -verify -pedantic 
-fsyntax-only -cl-std=CL2.0 -finclude-default-header
-// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic 
-fsyntax-only -cl-std=c++ -finclude-default-header
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -verify -pedantic 
-fsyntax-only -cl-std=c++
 
 #ifdef _OPENCL_H_
 // expected-no-diagnostics
Index: test/Driver/include-default-header.cl
===
--- test/Driver/include-default-header.cl
+++ test/Driver/include-default-header.cl
@@ -1,5 +1,6 @@
-// RUN: %clang -save-temps -x cl -Xclang -cl-std=CL2.0 -Xclang 
-finclude-default-header -emit-llvm -S -### %s
-// CHECK-NOT: finclude-default-header
+// RUN: %clang -save-temps -x cl -Xclang -cl-std=CL2.0 -Xclang 
-finclude-default-header -emit-llvm -S -### %s 2>&1 | FileCheck %s
+
+// CHECK-LABEL: finclude-default-header
 // Make sure we don't pass -finclude-default-header to any commands other than 
the driver.
 
 void test() {}


Index: test/Headers/opencl-c-header.cl
===
--- test/Headers/opencl-c-header.cl
+++ test/Headers/opencl-c-header.cl
@@ -1,6 +1,7 @@
 // RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem ../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -verify | FileCheck %s
-// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem ../../lib/Headers -include opencl-c.h -emit-llvm -o - %s -verify -cl-std=CL1.1| FileCheck %s
-// RUN: %clang_cc1 -O0 -triple spir-unknown-unknown -internal-isystem ../../lib/Headers -include op

[PATCH] D59459: [analyzer][NFC] Prefer binary searches in CheckerRegistry

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware requested changes to this revision.
baloghadamsoftware added inline comments.
This revision now requires changes to proceed.



Comment at: lib/StaticAnalyzer/Frontend/CheckerRegistry.cpp:70
+  Collection, Info,
+  FullNameLT{});
+}

Please note that `llvm::lower_bound()` uses `std::lower_bound()` which returns 
an iterator pointing to the first element that is not less than the sought 
value, or last if no such element is found. So it is mandatory to check the 
return value whether it is equal to sought value or greater. In latter case you 
should return `Collection.end()` to keep up compatibility with `find_if`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59459



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

r356452 - [ASTImporter] Fix redecl failures of ClassTemplateSpec

[Gabor Marton via cfe-commits]

Author: martong
Date: Tue Mar 19 06:34:10 2019
New Revision: 356452

URL: http://llvm.org/viewvc/llvm-project?rev=356452&view=rev
Log:
[ASTImporter] Fix redecl failures of ClassTemplateSpec

Summary:
Redecl chains of class template specializations are not handled well
currently. We want to handle them similarly to functions, i.e. try to
keep the structure of the original AST as much as possible. The aim is
to not squash a prototype with a definition, rather we create both and
put them in a redecl chain.

Reviewers: a_sidorin, shafik, a.sidorin

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, jdoerfert, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=356452&r1=356451&r2=356452&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Tue Mar 19 06:34:10 2019
@@ -5052,17 +5052,6 @@ ExpectedDecl ASTNodeImporter::VisitClass
 
 ExpectedDecl ASTNodeImporter::VisitClassTemplateSpecializationDecl(
   ClassTemplateSpecializationDecl *D) {
-  // If this record has a definition in the translation unit we're coming from,
-  // but this particular declaration is not that definition, import the
-  // definition and map to that.
-  TagDecl *Definition = D->getDefinition();
-  if (Definition && Definition != D) {
-if (ExpectedDecl ImportedDefOrErr = import(Definition))
-  return Importer.MapImported(D, *ImportedDefOrErr);
-else
-  return ImportedDefOrErr.takeError();
-  }
-
   ClassTemplateDecl *ClassTemplate;
   if (Error Err = importInto(ClassTemplate, D->getSpecializedTemplate()))
 return std::move(Err);
@@ -5080,154 +5069,141 @@ ExpectedDecl ASTNodeImporter::VisitClass
 
   // Try to find an existing specialization with these template arguments.
   void *InsertPos = nullptr;
-  ClassTemplateSpecializationDecl *D2 = nullptr;
+  ClassTemplateSpecializationDecl *PrevDecl = nullptr;
   ClassTemplatePartialSpecializationDecl *PartialSpec =
 dyn_cast(D);
   if (PartialSpec)
-D2 = ClassTemplate->findPartialSpecialization(TemplateArgs, InsertPos);
+PrevDecl =
+ClassTemplate->findPartialSpecialization(TemplateArgs, InsertPos);
   else
-D2 = ClassTemplate->findSpecialization(TemplateArgs, InsertPos);
-  ClassTemplateSpecializationDecl * const PrevDecl = D2;
-  RecordDecl *FoundDef = D2 ? D2->getDefinition() : nullptr;
-  if (FoundDef) {
-if (!D->isCompleteDefinition()) {
-  // The "From" translation unit only had a forward declaration; call it
-  // the same declaration.
-  // TODO Handle the redecl chain properly!
-  return Importer.MapImported(D, FoundDef);
-}
-
-if (IsStructuralMatch(D, FoundDef)) {
-
-  Importer.MapImported(D, FoundDef);
+PrevDecl = ClassTemplate->findSpecialization(TemplateArgs, InsertPos);
 
-  // Import those those default field initializers which have been
-  // instantiated in the "From" context, but not in the "To" context.
-  for (auto *FromField : D->fields()) {
-auto ToOrErr = import(FromField);
-if (!ToOrErr)
-  // FIXME: return the error?
-  consumeError(ToOrErr.takeError());
+  if (PrevDecl) {
+if (IsStructuralMatch(D, PrevDecl)) {
+  if (D->isThisDeclarationADefinition() && PrevDecl->getDefinition()) {
+Importer.MapImported(D, PrevDecl->getDefinition());
+// Import those default field initializers which have been
+// instantiated in the "From" context, but not in the "To" context.
+for (auto *FromField : D->fields())
+  Importer.Import(FromField);
+
+// Import those methods which have been instantiated in the
+// "From" context, but not in the "To" context.
+for (CXXMethodDecl *FromM : D->methods())
+  Importer.Import(FromM);
+
+// TODO Import instantiated default arguments.
+// TODO Import instantiated exception specifications.
+//
+// Generally, ASTCommon.h/DeclUpdateKind enum gives a very good hint
+// what else could be fused during an AST merge.
+return PrevDecl;
   }
+} else { // ODR violation.
+  // FIXME HandleNameConflict
+  return nullptr;
+}
+  }
 
-  // Import those methods which have been instantiated in the
-  // "From" context, but not in the "To" context.
-  for (CXXMethodDecl *FromM : D->methods()) {
-auto ToOrErr = import(FromM);
-if (!ToOrErr)
-  // FIXME: return the error?
-  consumeError(ToOrErr.takeError());
-  }
+  // Import the location of this declaration.
+  ExpectedSLoc BeginLocOrErr = import(D->getBeginLoc());
+  if (!BeginLocOrErr)
+

[PATCH] D58673: [ASTImporter] Fix redecl failures of ClassTemplateSpec

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356452: [ASTImporter] Fix redecl failures of 
ClassTemplateSpec (authored by martong, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58673?vs=189131&id=191285#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58673

Files:
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/unittests/AST/ASTImporterTest.cpp

Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -5052,17 +5052,6 @@
 
 ExpectedDecl ASTNodeImporter::VisitClassTemplateSpecializationDecl(
   ClassTemplateSpecializationDecl *D) {
-  // If this record has a definition in the translation unit we're coming from,
-  // but this particular declaration is not that definition, import the
-  // definition and map to that.
-  TagDecl *Definition = D->getDefinition();
-  if (Definition && Definition != D) {
-if (ExpectedDecl ImportedDefOrErr = import(Definition))
-  return Importer.MapImported(D, *ImportedDefOrErr);
-else
-  return ImportedDefOrErr.takeError();
-  }
-
   ClassTemplateDecl *ClassTemplate;
   if (Error Err = importInto(ClassTemplate, D->getSpecializedTemplate()))
 return std::move(Err);
@@ -5080,154 +5069,141 @@
 
   // Try to find an existing specialization with these template arguments.
   void *InsertPos = nullptr;
-  ClassTemplateSpecializationDecl *D2 = nullptr;
+  ClassTemplateSpecializationDecl *PrevDecl = nullptr;
   ClassTemplatePartialSpecializationDecl *PartialSpec =
 dyn_cast(D);
   if (PartialSpec)
-D2 = ClassTemplate->findPartialSpecialization(TemplateArgs, InsertPos);
+PrevDecl =
+ClassTemplate->findPartialSpecialization(TemplateArgs, InsertPos);
   else
-D2 = ClassTemplate->findSpecialization(TemplateArgs, InsertPos);
-  ClassTemplateSpecializationDecl * const PrevDecl = D2;
-  RecordDecl *FoundDef = D2 ? D2->getDefinition() : nullptr;
-  if (FoundDef) {
-if (!D->isCompleteDefinition()) {
-  // The "From" translation unit only had a forward declaration; call it
-  // the same declaration.
-  // TODO Handle the redecl chain properly!
-  return Importer.MapImported(D, FoundDef);
-}
-
-if (IsStructuralMatch(D, FoundDef)) {
-
-  Importer.MapImported(D, FoundDef);
-
-  // Import those those default field initializers which have been
-  // instantiated in the "From" context, but not in the "To" context.
-  for (auto *FromField : D->fields()) {
-auto ToOrErr = import(FromField);
-if (!ToOrErr)
-  // FIXME: return the error?
-  consumeError(ToOrErr.takeError());
-  }
+PrevDecl = ClassTemplate->findSpecialization(TemplateArgs, InsertPos);
 
-  // Import those methods which have been instantiated in the
-  // "From" context, but not in the "To" context.
-  for (CXXMethodDecl *FromM : D->methods()) {
-auto ToOrErr = import(FromM);
-if (!ToOrErr)
-  // FIXME: return the error?
-  consumeError(ToOrErr.takeError());
+  if (PrevDecl) {
+if (IsStructuralMatch(D, PrevDecl)) {
+  if (D->isThisDeclarationADefinition() && PrevDecl->getDefinition()) {
+Importer.MapImported(D, PrevDecl->getDefinition());
+// Import those default field initializers which have been
+// instantiated in the "From" context, but not in the "To" context.
+for (auto *FromField : D->fields())
+  Importer.Import(FromField);
+
+// Import those methods which have been instantiated in the
+// "From" context, but not in the "To" context.
+for (CXXMethodDecl *FromM : D->methods())
+  Importer.Import(FromM);
+
+// TODO Import instantiated default arguments.
+// TODO Import instantiated exception specifications.
+//
+// Generally, ASTCommon.h/DeclUpdateKind enum gives a very good hint
+// what else could be fused during an AST merge.
+return PrevDecl;
   }
+} else { // ODR violation.
+  // FIXME HandleNameConflict
+  return nullptr;
+}
+  }
 
-  // TODO Import instantiated default arguments.
-  // TODO Import instantiated exception specifications.
-  //
-  // Generally, ASTCommon.h/DeclUpdateKind enum gives a very good hint what
-  // else could be fused during an AST merge.
-
-  return FoundDef;
-}
-  } else { // We either couldn't find any previous specialization in the "To"
-   // context,  or we found one but without definition.  Let's create a
-   // new specialization and register that at the class template.
+  // Import the location of this declaration.
+  ExpectedSLoc BeginLocOrErr = impo

[PATCH] D58668: [ASTImporter] Fix redecl failures of FunctionTemplateSpec

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 191286.
martong added a comment.

- Rebase to master


Repository:
  rC Clang

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

https://reviews.llvm.org/D58668

Files:
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -3978,6 +3978,45 @@
   std::string getDefinition() { return TypeParam::Definition; }
   BindableMatcher getPattern() const { return TypeParam().getPattern(); }
 
+  void CheckPreviousDecl(Decl *Prev, Decl *Current) {
+ASSERT_NE(Prev, Current);
+ASSERT_EQ(&Prev->getASTContext(), &Current->getASTContext());
+EXPECT_EQ(Prev->getCanonicalDecl(), Current->getCanonicalDecl());
+
+// Templates.
+if (auto *PrevT = dyn_cast(Prev)) {
+  EXPECT_EQ(Current->getPreviousDecl(), Prev);
+  auto *CurrentT = cast(Current);
+  ASSERT_TRUE(PrevT->getTemplatedDecl());
+  ASSERT_TRUE(CurrentT->getTemplatedDecl());
+  EXPECT_EQ(CurrentT->getTemplatedDecl()->getPreviousDecl(),
+PrevT->getTemplatedDecl());
+  return;
+}
+
+// Specializations.
+if (auto *PrevF = dyn_cast(Prev)) {
+  if (PrevF->getTemplatedKind() ==
+  FunctionDecl::TK_FunctionTemplateSpecialization) {
+// There may be a hidden fwd spec decl before a spec decl.
+// In that case the previous visible decl can be reached through that
+// invisible one.
+EXPECT_THAT(Prev, testing::AnyOf(
+  Current->getPreviousDecl(),
+  Current->getPreviousDecl()->getPreviousDecl()));
+auto *ToTU = Prev->getTranslationUnitDecl();
+auto *TemplateD = FirstDeclMatcher().match(
+ToTU, functionTemplateDecl());
+auto *FirstSpecD = *(TemplateD->spec_begin());
+EXPECT_EQ(FirstSpecD->getCanonicalDecl(), PrevF->getCanonicalDecl());
+return;
+  }
+}
+
+// The rest: Classes, Functions, etc.
+EXPECT_EQ(Current->getPreviousDecl(), Prev);
+  }
+
   void
   TypedTest_PrototypeShouldBeImportedAsAPrototypeWhenThereIsNoDefinition() {
 Decl *FromTU = getTuDecl(getPrototype(), Lang_CXX);
@@ -4030,14 +4069,8 @@
 EXPECT_TRUE(Imported1 == To1);
 EXPECT_FALSE(To0->isThisDeclarationADefinition());
 EXPECT_FALSE(To1->isThisDeclarationADefinition());
-EXPECT_EQ(To1->getPreviousDecl(), To0);
-if (auto *ToT0 = dyn_cast(To0)) {
-  auto *ToT1 = cast(To1);
-  ASSERT_TRUE(ToT0->getTemplatedDecl());
-  ASSERT_TRUE(ToT1->getTemplatedDecl());
-  EXPECT_EQ(ToT1->getTemplatedDecl()->getPreviousDecl(),
-ToT0->getTemplatedDecl());
-}
+
+CheckPreviousDecl(To0, To1);
   }
 
   void TypedTest_ImportDefinitionAfterImportedPrototype() {
@@ -4059,14 +4092,8 @@
 EXPECT_TRUE(ImportedDef == ToDef);
 EXPECT_FALSE(ToProto->isThisDeclarationADefinition());
 EXPECT_TRUE(ToDef->isThisDeclarationADefinition());
-EXPECT_EQ(ToDef->getPreviousDecl(), ToProto);
-if (auto *ToProtoT = dyn_cast(ToProto)) {
-  auto *ToDefT = cast(ToDef);
-  ASSERT_TRUE(ToProtoT->getTemplatedDecl());
-  ASSERT_TRUE(ToDefT->getTemplatedDecl());
-  EXPECT_EQ(ToDefT->getTemplatedDecl()->getPreviousDecl(),
-ToProtoT->getTemplatedDecl());
-}
+
+CheckPreviousDecl(ToProto, ToDef);
   }
 
   void TypedTest_ImportPrototypeAfterImportedDefinition() {
@@ -4088,14 +4115,8 @@
 EXPECT_TRUE(ImportedProto == ToProto);
 EXPECT_TRUE(ToDef->isThisDeclarationADefinition());
 EXPECT_FALSE(ToProto->isThisDeclarationADefinition());
-EXPECT_EQ(ToProto->getPreviousDecl(), ToDef);
-if (auto *ToDefT = dyn_cast(ToDef)) {
-  auto *ToProtoT = cast(ToProto);
-  ASSERT_TRUE(ToDefT->getTemplatedDecl());
-  ASSERT_TRUE(ToProtoT->getTemplatedDecl());
-  EXPECT_EQ(ToProtoT->getTemplatedDecl()->getPreviousDecl(),
-ToDefT->getTemplatedDecl());
-}
+
+CheckPreviousDecl(ToDef, ToProto);
   }
 
   void TypedTest_ImportPrototypes() {
@@ -4117,27 +4138,8 @@
 EXPECT_TRUE(Imported1 == To1);
 EXPECT_FALSE(To0->isThisDeclarationADefinition());
 EXPECT_FALSE(To1->isThisDeclarationADefinition());
-EXPECT_EQ(To1->getPreviousDecl(), To0);
-if (auto *ToT0 = dyn_cast(To0)) {
-  auto *ToT1 = cast(To1);
-  ASSERT_TRUE(ToT0->getTemplatedDecl());
-  ASSERT_TRUE(ToT1->getTemplatedDecl());
-  EXPECT_EQ(ToT1->getTemplatedDecl()->getPreviousDecl(),
-ToT0->getTemplatedDecl());
-}
-// Extra check for specializations.
-// FIXME Add this check to other tests too (possibly factor out into a
-// function), when they start to pass.
-if (auto *From0F = dyn_cast(From0)) {
-  auto *To0F = cast(To0);
-  if (From0F->getTemplatedKind() ==
-  FunctionDecl::TK_FunctionTempla

r356455 - [ASTImporter] Fix redecl failures of FunctionTemplateSpec

[Gabor Marton via cfe-commits]

Author: martong
Date: Tue Mar 19 07:04:50 2019
New Revision: 356455

URL: http://llvm.org/viewvc/llvm-project?rev=356455&view=rev
Log:
[ASTImporter] Fix redecl failures of FunctionTemplateSpec

Summary:
Redecl chains of function template specializations are not handled well
currently. We want to handle them similarly to functions, i.e. try to
keep the structure of the original AST as much as possible. The aim is
to not squash a prototype with a definition, rather we create both and
put them in a redecl chain.

Reviewers: a_sidorin, shafik, a.sidorin

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=356455&r1=356454&r2=356455&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Tue Mar 19 07:04:50 2019
@@ -290,6 +290,16 @@ namespace clang {
 ToD->setImplicit();
 }
 
+// Check if we have found an existing definition.  Returns with that
+// definition if yes, otherwise returns null.
+Decl *FindAndMapDefinition(FunctionDecl *D, FunctionDecl *FoundFunction) {
+  const FunctionDecl *Definition = nullptr;
+  if (D->doesThisDeclarationHaveABody() &&
+  FoundFunction->hasBody(Definition))
+return Importer.MapImported(D, const_cast(Definition));
+  return nullptr;
+}
+
   public:
 explicit ASTNodeImporter(ASTImporter &Importer) : Importer(Importer) {}
 
@@ -2973,8 +2983,8 @@ ExpectedDecl ASTNodeImporter::VisitFunct
 if (!FoundFunctionOrErr)
   return FoundFunctionOrErr.takeError();
 if (FunctionDecl *FoundFunction = *FoundFunctionOrErr) {
-  if (D->doesThisDeclarationHaveABody() && FoundFunction->hasBody())
-return Importer.MapImported(D, FoundFunction);
+  if (Decl *Def = FindAndMapDefinition(D, FoundFunction))
+return Def;
   FoundByLookup = FoundFunction;
 }
   }
@@ -2993,11 +3003,8 @@ ExpectedDecl ASTNodeImporter::VisitFunct
   continue;
 
 if (IsStructuralMatch(D, FoundFunction)) {
-  const FunctionDecl *Definition = nullptr;
-  if (D->doesThisDeclarationHaveABody() &&
-  FoundFunction->hasBody(Definition))
-return Importer.MapImported(D,
-const_cast(Definition));
+  if (Decl *Def = FindAndMapDefinition(D, FoundFunction))
+return Def;
   FoundByLookup = FoundFunction;
   break;
 }

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=356455&r1=356454&r2=356455&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Tue Mar 19 07:04:50 2019
@@ -3978,6 +3978,45 @@ struct RedeclChain : ASTImporterOptionSp
   std::string getDefinition() { return TypeParam::Definition; }
   BindableMatcher getPattern() const { return TypeParam().getPattern(); }
 
+  void CheckPreviousDecl(Decl *Prev, Decl *Current) {
+ASSERT_NE(Prev, Current);
+ASSERT_EQ(&Prev->getASTContext(), &Current->getASTContext());
+EXPECT_EQ(Prev->getCanonicalDecl(), Current->getCanonicalDecl());
+
+// Templates.
+if (auto *PrevT = dyn_cast(Prev)) {
+  EXPECT_EQ(Current->getPreviousDecl(), Prev);
+  auto *CurrentT = cast(Current);
+  ASSERT_TRUE(PrevT->getTemplatedDecl());
+  ASSERT_TRUE(CurrentT->getTemplatedDecl());
+  EXPECT_EQ(CurrentT->getTemplatedDecl()->getPreviousDecl(),
+PrevT->getTemplatedDecl());
+  return;
+}
+
+// Specializations.
+if (auto *PrevF = dyn_cast(Prev)) {
+  if (PrevF->getTemplatedKind() ==
+  FunctionDecl::TK_FunctionTemplateSpecialization) {
+// There may be a hidden fwd spec decl before a spec decl.
+// In that case the previous visible decl can be reached through that
+// invisible one.
+EXPECT_THAT(Prev, testing::AnyOf(
+  Current->getPreviousDecl(),
+  Current->getPreviousDecl()->getPreviousDecl()));
+auto *ToTU = Prev->getTranslationUnitDecl();
+auto *TemplateD = FirstDeclMatcher().match(
+ToTU, functionTemplateDecl());
+auto *FirstSpecD = *(TemplateD->spec_begin());
+EXPECT_EQ(FirstSpecD->getCanonicalDecl(), PrevF->getCanonicalDecl());
+return;
+  }
+}
+
+// The rest: Classes, Functions, etc.
+EXPECT_EQ(Current->getPreviousDecl(), Prev);
+  }
+
   void
   TypedTest_PrototypeShouldBeImportedAsAPrototype

[PATCH] D58668: [ASTImporter] Fix redecl failures of FunctionTemplateSpec

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356455: [ASTImporter] Fix redecl failures of 
FunctionTemplateSpec (authored by martong, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58668

Files:
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/unittests/AST/ASTImporterTest.cpp

Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -290,6 +290,16 @@
 ToD->setImplicit();
 }
 
+// Check if we have found an existing definition.  Returns with that
+// definition if yes, otherwise returns null.
+Decl *FindAndMapDefinition(FunctionDecl *D, FunctionDecl *FoundFunction) {
+  const FunctionDecl *Definition = nullptr;
+  if (D->doesThisDeclarationHaveABody() &&
+  FoundFunction->hasBody(Definition))
+return Importer.MapImported(D, const_cast(Definition));
+  return nullptr;
+}
+
   public:
 explicit ASTNodeImporter(ASTImporter &Importer) : Importer(Importer) {}
 
@@ -2973,8 +2983,8 @@
 if (!FoundFunctionOrErr)
   return FoundFunctionOrErr.takeError();
 if (FunctionDecl *FoundFunction = *FoundFunctionOrErr) {
-  if (D->doesThisDeclarationHaveABody() && FoundFunction->hasBody())
-return Importer.MapImported(D, FoundFunction);
+  if (Decl *Def = FindAndMapDefinition(D, FoundFunction))
+return Def;
   FoundByLookup = FoundFunction;
 }
   }
@@ -2993,11 +3003,8 @@
   continue;
 
 if (IsStructuralMatch(D, FoundFunction)) {
-  const FunctionDecl *Definition = nullptr;
-  if (D->doesThisDeclarationHaveABody() &&
-  FoundFunction->hasBody(Definition))
-return Importer.MapImported(D,
-const_cast(Definition));
+  if (Decl *Def = FindAndMapDefinition(D, FoundFunction))
+return Def;
   FoundByLookup = FoundFunction;
   break;
 }
Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -3978,6 +3978,45 @@
   std::string getDefinition() { return TypeParam::Definition; }
   BindableMatcher getPattern() const { return TypeParam().getPattern(); }
 
+  void CheckPreviousDecl(Decl *Prev, Decl *Current) {
+ASSERT_NE(Prev, Current);
+ASSERT_EQ(&Prev->getASTContext(), &Current->getASTContext());
+EXPECT_EQ(Prev->getCanonicalDecl(), Current->getCanonicalDecl());
+
+// Templates.
+if (auto *PrevT = dyn_cast(Prev)) {
+  EXPECT_EQ(Current->getPreviousDecl(), Prev);
+  auto *CurrentT = cast(Current);
+  ASSERT_TRUE(PrevT->getTemplatedDecl());
+  ASSERT_TRUE(CurrentT->getTemplatedDecl());
+  EXPECT_EQ(CurrentT->getTemplatedDecl()->getPreviousDecl(),
+PrevT->getTemplatedDecl());
+  return;
+}
+
+// Specializations.
+if (auto *PrevF = dyn_cast(Prev)) {
+  if (PrevF->getTemplatedKind() ==
+  FunctionDecl::TK_FunctionTemplateSpecialization) {
+// There may be a hidden fwd spec decl before a spec decl.
+// In that case the previous visible decl can be reached through that
+// invisible one.
+EXPECT_THAT(Prev, testing::AnyOf(
+  Current->getPreviousDecl(),
+  Current->getPreviousDecl()->getPreviousDecl()));
+auto *ToTU = Prev->getTranslationUnitDecl();
+auto *TemplateD = FirstDeclMatcher().match(
+ToTU, functionTemplateDecl());
+auto *FirstSpecD = *(TemplateD->spec_begin());
+EXPECT_EQ(FirstSpecD->getCanonicalDecl(), PrevF->getCanonicalDecl());
+return;
+  }
+}
+
+// The rest: Classes, Functions, etc.
+EXPECT_EQ(Current->getPreviousDecl(), Prev);
+  }
+
   void
   TypedTest_PrototypeShouldBeImportedAsAPrototypeWhenThereIsNoDefinition() {
 Decl *FromTU = getTuDecl(getPrototype(), Lang_CXX);
@@ -4030,14 +4069,8 @@
 EXPECT_TRUE(Imported1 == To1);
 EXPECT_FALSE(To0->isThisDeclarationADefinition());
 EXPECT_FALSE(To1->isThisDeclarationADefinition());
-EXPECT_EQ(To1->getPreviousDecl(), To0);
-if (auto *ToT0 = dyn_cast(To0)) {
-  auto *ToT1 = cast(To1);
-  ASSERT_TRUE(ToT0->getTemplatedDecl());
-  ASSERT_TRUE(ToT1->getTemplatedDecl());
-  EXPECT_EQ(ToT1->getTemplatedDecl()->getPreviousDecl(),
-ToT0->getTemplatedDecl());
-}
+
+CheckPreviousDecl(To0, To1);
   }
 
   void TypedTest_ImportDefinitionAfterImportedPrototype() {
@@ -4059,14 +4092,8 @@
 EXPECT_TRUE(ImportedDef == ToDef);
 EXPECT_FALSE(ToProto->isThisDeclarationADefini

[PATCH] D59544: [OpenCL] Minor improvements in default header testing

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia created this revision.
Anastasia added a reviewer: bader.
Herald added subscribers: ebevhan, yaxunl.

Suggested in https://reviews.llvm.org/D59486


https://reviews.llvm.org/D59544

Files:
  test/Headers/opencl-c-header.cl


Index: test/Headers/opencl-c-header.cl
===
--- test/Headers/opencl-c-header.cl
+++ test/Headers/opencl-c-header.cl
@@ -53,15 +53,14 @@
 // CHECK: _Z16convert_char_rtec
 // CHECK-NOT: _Z3ctzc
 // CHECK20: _Z3ctzc
-// CHECK20-NOT: _Z16convert_char_rtec
+// CHECK20: _Z16convert_char_rtec
 char f(char x) {
-#if !defined(__OPENCL_CPP_VERSION__) && (__OPENCL_C_VERSION__ != 
CL_VERSION_2_0)
-  return convert_char_rte(x);
-
-#else //__OPENCL_C_VERSION__
+// Check functionality from OpenCL 2.0 onwards
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ == CL_VERSION_2_0)
   ndrange_t t;
-  return ctz(x);
+  x = ctz(x);
 #endif //__OPENCL_C_VERSION__
+  return convert_char_rte(x);
 }
 
 // Verify that a builtin using a write_only image3d_t type is available


Index: test/Headers/opencl-c-header.cl
===
--- test/Headers/opencl-c-header.cl
+++ test/Headers/opencl-c-header.cl
@@ -53,15 +53,14 @@
 // CHECK: _Z16convert_char_rtec
 // CHECK-NOT: _Z3ctzc
 // CHECK20: _Z3ctzc
-// CHECK20-NOT: _Z16convert_char_rtec
+// CHECK20: _Z16convert_char_rtec
 char f(char x) {
-#if !defined(__OPENCL_CPP_VERSION__) && (__OPENCL_C_VERSION__ != CL_VERSION_2_0)
-  return convert_char_rte(x);
-
-#else //__OPENCL_C_VERSION__
+// Check functionality from OpenCL 2.0 onwards
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ == CL_VERSION_2_0)
   ndrange_t t;
-  return ctz(x);
+  x = ctz(x);
 #endif //__OPENCL_C_VERSION__
+  return convert_char_rte(x);
 }
 
 // Verify that a builtin using a write_only image3d_t type is available
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59544: [OpenCL] Minor improvements in default header testing

[Alexey Bader via Phabricator via cfe-commits]

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

LGTM. Thanks!


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

https://reviews.llvm.org/D59544



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

[PATCH] D59492: [OpenCL] Allow variadic macros as Clang feature

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D59492#1434636 , @Anastasia wrote:

> In D59492#1433796 , @arsenm wrote:
>
> > Should it be downgraded to a warning about an extension instead of just 
> > removing it?
>
>
> What would you suggest to put in a warning message? Clang normally doesn't 
> warn about extensions...


Isn't that what -pedantic is for?


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

https://reviews.llvm.org/D59492



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

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

What happens on `[=]() ...`, direct capture `[&Columns]()...` and 
`[Columns]()...`?


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

https://reviews.llvm.org/D59540



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

[PATCH] D59466: [clang-tidy] openmp-exception-escape - a new check

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

Great work! Thank you! I only have minor comment: did you consider moving the 
refactoring of `ExceptionAnalyzer` into a separate (prerequisite) patch?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59466



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

[PATCH] D59520: [WebAssembly] Address review comments on r352930

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I love functional changes that remove code and add tests -- thank you for these!

> Removes errnoneous use of diag::err_alias_is_definition, which turned out to 
> be ineffective anyway since functions can be defined later in the translation 
> unit and avoid detection.

If you need to keep the prohibition that these attributes not be applied to 
functions with definitions, there are ways to accomplish that (we merge 
declarations and can decide what to do at that point if we see a declaration 
that is a definition). Given that, do you still want to lift this restriction?




Comment at: test/Sema/attr-wasm.c:3
+
+void name_a() {}
+

Was this intended to be used somewhere? Probably can just be removed if not.



Comment at: test/Sema/attr-wasm.c:15
+
+void module_a() {}
+

Same here.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59520



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

Re: r352930 - [WebAssembly] Add an import_field function attribute

[Aaron Ballman via cfe-commits]

On Mon, Mar 18, 2019 at 7:08 PM Dan Gohman  wrote:
>
>
>
> On Fri, Mar 15, 2019 at 10:55 AM Aaron Ballman  wrote:
>>
>>
>> Ping.
>
>
> I apologize for the extraordinarily delays here. I've now posted a patch to 
> address the review comments here:
>
> https://reviews.llvm.org/D59520

Delays happen to all of us; thank you for addressing the comments -- I
appreciate it!

~Aaron

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

[PATCH] D59466: [clang-tidy] openmp-exception-escape - a new check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D59466#1434841 , 
@baloghadamsoftware wrote:

> Great work! Thank you! I only have minor comment: did you consider moving the 
> refactoring of `ExceptionAnalyzer` into a separate (prerequisite) patch?


Yes, absolutely, that will be separate patch.
I just wanted to finally post the check to show how it **finally** (*sigh*) 
comes together in the end.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59466



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

[PATCH] D59546: [clang-format] structured binding in range for detected as Objective C

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: djasper, klimek, JonasToth, reuk.
MyDeveloperDay added a project: clang-tools-extra.

Sometime after 6.0.0 and the current trunk 9.0.0 the following code would be 
considered as objective C and not C++

Reported by: https://twitter.com/mattgodbolt/status/1096188576503644160

$ clang-format.exe test.h
Configuration file(s) do(es) not support Objective-C: 
C:\clang\build\.clang-format

- test.h --

  #include 
  #include 
  
  std::vector> C;
  
  void foo()
  {
 for (auto && [A,B] : C)
 {
 std::string D = A + B;
 }
  }

The following code fixes this issue of incorrect detection


https://reviews.llvm.org/D59546

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12924,6 +12924,9 @@
 guessLanguage("foo.h", "[[abusing clang:fallthrough] bar];"));
   EXPECT_EQ(FormatStyle::LK_Cpp,
 guessLanguage("foo.h", "[[using gsl: suppress(\"type\")]];"));
+  EXPECT_EQ(
+  FormatStyle::LK_Cpp,
+  guessLanguage("foo.h", "for (auto &&[endpoint, stream] : streams_)"));
   EXPECT_EQ(
   FormatStyle::LK_Cpp,
   guessLanguage("foo.h",
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -410,7 +410,10 @@
  Parent->isUnaryOperator() ||
  // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen.
  Parent->isOneOf(TT_ObjCForIn, TT_CastRParen) ||
- getBinOpPrecedence(Parent->Tok.getKind(), true, true) > 
prec::Unknown);
+ // for (auto && [A,B] : C)  && structure binding seen as 
ObjCMethodExpr
+ (Parent->isNot(tok::ampamp) &&
+  getBinOpPrecedence(Parent->Tok.getKind(), true, true) >
+  prec::Unknown));
 bool ColonFound = false;
 
 unsigned BindingIncrease = 1;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12924,6 +12924,9 @@
 guessLanguage("foo.h", "[[abusing clang:fallthrough] bar];"));
   EXPECT_EQ(FormatStyle::LK_Cpp,
 guessLanguage("foo.h", "[[using gsl: suppress(\"type\")]];"));
+  EXPECT_EQ(
+  FormatStyle::LK_Cpp,
+  guessLanguage("foo.h", "for (auto &&[endpoint, stream] : streams_)"));
   EXPECT_EQ(
   FormatStyle::LK_Cpp,
   guessLanguage("foo.h",
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -410,7 +410,10 @@
  Parent->isUnaryOperator() ||
  // FIXME(bug 36976): ObjC return types shouldn't use TT_CastRParen.
  Parent->isOneOf(TT_ObjCForIn, TT_CastRParen) ||
- getBinOpPrecedence(Parent->Tok.getKind(), true, true) > prec::Unknown);
+ // for (auto && [A,B] : C)  && structure binding seen as ObjCMethodExpr
+ (Parent->isNot(tok::ampamp) &&
+  getBinOpPrecedence(Parent->Tok.getKind(), true, true) >
+  prec::Unknown));
 bool ColonFound = false;
 
 unsigned BindingIncrease = 1;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57896: Variable names rule

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added subscribers: chandlerc, Charusso.
Charusso added a comment.

In D57896#1406812 , @lattner wrote:

> I can understand Zach's position here, but LLDB has historically never 
> conformed to the general LLVM naming or other conventions due to its 
> heritage.  It should not be taken as precedent that the rest of the project 
> should follow.
>
> In any case, I think that it is best for this discussion to take place on the 
> llvm-dev list where it is likely to get the most visibility.  Would you mind 
> moving comments and discussions there?


Hey! Random Clang developer is here after this topic became a little-bit dead 
as not everyone subbed to LLVM dev-list. I think the best solution for every 
difficult question is to let the users decide their own future among all the 
projects. I would announce polls (https://reviews.llvm.org/vote/) and announce 
them on every dev-list.

I do not see any better solution to decide if we would code like `DRE`, `VD` 
versus `expr`, `decl` as @lattner would code. And I am not sure if everyone 
happy with `this_new_styling` as @chandlerc and @zturner would code. E.g. I am 
not happy because I know my if-statements would take two lines of code instead 
of one and it would be super-duper-mega ugly and difficult to read. Here is an 
example:

  static Optional
  getConcreteIntegerValue(const Expr *CondVarExpr, const ExplodedNode *N) {
  //...
if (const auto *DRE = dyn_cast_or_null(CondVarExpr)) {
  if (const auto *VD = dyn_cast_or_null(DRE->getDecl())) {
  //...
  }

would be:

  static Optional 
  |
  getConcreteIntegerValue(const Expr *cond_var_expr, const ExplodedNode *node) 
{  |
  //... 
  |
if (const auto *decl_ref_expr = 
dyn_cast_or_null(cond_var_expr)) {
  if (const auto *var_decl = 
dyn_cast_or_null(decl_ref_expr->getDecl())) {
  //... 
  |
  } whoops 
column-81 ~^

Hungarian notation on members and globals are cool idea. However, the notation 
is made without the `_` part, so I think `mMember` is better than `m_member` as 
we used to 80-column standard and it is waste of space and hurts your 
C-developer eyes. I would recommend `b` prefix to booleans as Unreal Engine 4 
styling is used to do that (`bIsCoolStyle`) and it is handy. It is useful 
because booleans usually has multiple prefixes: `has, have, is` and you would 
list all the booleans together in autocompletion. Yes, there is a problem: if 
the notation is not capital like the pure Hungarian notation then it is 
problematic to list and we are back to the `BIsCapitalLetter` and `MMember` 
CamelCase-world where we started (except one project). I think @lattner could 
say if it is useful as all the Apple projects based on those notations and 
could be annoying.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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

r356458 - Ensure that const variables declared at namespace scope correctly have external linkage when marked as dllexport and targeting the MSVC ABI.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Tue Mar 19 07:53:52 2019
New Revision: 356458

URL: http://llvm.org/viewvc/llvm-project?rev=356458&view=rev
Log:
Ensure that const variables declared at namespace scope correctly have external 
linkage when marked as dllexport and targeting the MSVC ABI.

Patch thanks to Zahira Ammarguellat.

Added:
cfe/trunk/test/CodeGen/dllexport-1.c
cfe/trunk/test/Sema/dllexport-1.cpp
cfe/trunk/test/Sema/dllexport-2.cpp
Modified:
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/test/SemaCXX/dllexport.cpp
cfe/trunk/test/SemaCXX/dllimport.cpp

Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=356458&r1=356457&r2=356458&view=diff
==
--- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Mar 19 07:53:52 2019
@@ -5964,10 +5964,24 @@ static void checkAttributesAfterMerging(
   }
 
   if (const InheritableAttr *Attr = getDLLAttr(&ND)) {
+auto *VD = dyn_cast(&ND);
+bool IsAnonymousNS = false;
+bool IsMicrosoft = S.Context.getTargetInfo().getCXXABI().isMicrosoft();
+if (VD) {
+  const NamespaceDecl *NS = dyn_cast(VD->getDeclContext());
+  while (NS && !IsAnonymousNS) {
+IsAnonymousNS = NS->isAnonymousNamespace();
+NS = dyn_cast(NS->getParent());
+  }
+}
 // dll attributes require external linkage. Static locals may have external
 // linkage but still cannot be explicitly imported or exported.
-auto *VD = dyn_cast(&ND);
-if (!ND.isExternallyVisible() || (VD && VD->isStaticLocal())) {
+// In Microsoft mode, a variable defined in anonymous namespace must have
+// external linkage in order to be exported.
+bool AnonNSInMicrosoftMode = IsAnonymousNS && IsMicrosoft;
+if ((ND.isExternallyVisible() && AnonNSInMicrosoftMode) ||
+(!AnonNSInMicrosoftMode &&
+ (!ND.isExternallyVisible() || (VD && VD->isStaticLocal() {
   S.Diag(ND.getLocation(), diag::err_attribute_dll_not_extern)
 << &ND << Attr;
   ND.setInvalidDecl();
@@ -11376,6 +11390,14 @@ void Sema::AddInitializerToDecl(Decl *Re
 !isTemplateInstantiation(VDecl->getTemplateSpecializationKind()))
   Diag(VDecl->getLocation(), diag::warn_extern_init);
 
+// In Microsoft C++ mode, a const variable defined in namespace scope has
+// external linkage by default if the variable is declared with
+// __declspec(dllexport).
+if (Context.getTargetInfo().getCXXABI().isMicrosoft() &&
+getLangOpts().CPlusPlus && VDecl->getType().isConstQualified() &&
+VDecl->hasAttr() && VDecl->getDefinition())
+  VDecl->setStorageClass(SC_Extern);
+
 // C99 6.7.8p4. All file scoped initializers need to be constant.
 if (!getLangOpts().CPlusPlus && !VDecl->isInvalidDecl())
   CheckForConstantInitializer(Init, DclT);

Added: cfe/trunk/test/CodeGen/dllexport-1.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/dllexport-1.c?rev=356458&view=auto
==
--- cfe/trunk/test/CodeGen/dllexport-1.c (added)
+++ cfe/trunk/test/CodeGen/dllexport-1.c Tue Mar 19 07:53:52 2019
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -emit-llvm -fms-extensions 
-Wno-ignored-attributes -Wno-extern-initializer -o - %s | FileCheck %s 
-check-prefix CHECK-LNX
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -emit-llvm -fms-extensions 
-o - -DMSVC %s | FileCheck %s -check-prefix CHECK-MSVC
+
+// Export const variable.
+
+// CHECK-MSVC: @x = dso_local dllexport constant i32 3, align 4
+// CHECK-LNX: @x = constant i32 3, align 4
+
+// CHECK-MSVC: @z = dso_local constant i32 4, align 4
+// CHECK-LNX: @z = constant i32 4, align 4
+
+// CHECK-MSVC: @y = common dso_local dllexport global i32 0, align 4
+// CHECK-LNX: @y = common global i32 0, align 4
+
+__declspec(dllexport) int const x = 3;
+__declspec(dllexport) const int y;
+
+// expected-warning@+1 {{'extern' variable has an initializer}}
+extern int const z = 4;
+
+int main() {
+  int a = x + y + z;
+  return a;
+}

Added: cfe/trunk/test/Sema/dllexport-1.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/dllexport-1.cpp?rev=356458&view=auto
==
--- cfe/trunk/test/Sema/dllexport-1.cpp (added)
+++ cfe/trunk/test/Sema/dllexport-1.cpp Tue Mar 19 07:53:52 2019
@@ -0,0 +1,33 @@
+// RUN: %clang_cc1 -triple x86_64-pc-linux-gnu -fsyntax-only -fms-extensions 
-verify %s
+// RUN: %clang_cc1 -triple x86_64-pc-windows-msvc -fsyntax-only 
-fms-extensions -verify %s  -DMSVC
+
+// Export const variable initialization.
+
+#ifdef MSVC
+// expected-no-diagnostics
+#endif
+
+#ifndef MSVC
+// expected-warning@+2 {{__declspec attribute 'dllexport' is not supported}}
+#endif
+__declspec(dllexport) int const x = 3;
+
+namespace {
+nam

[PATCH] D45978: dllexport const variables must have external linkage.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman closed this revision.
aaron.ballman added a comment.

> Would you mind committing the changes please? Thanks.

Happy to do so! I've committed in r356458.


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

https://reviews.llvm.org/D45978



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

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay planned changes to this revision.
MyDeveloperDay added a comment.

In D59540#1434837 , @JonasToth wrote:

> What happens on `[=]() ...`, direct capture `[&Columns]()...` and 
> `[Columns]()...`?


Thanks for the review... in answer to your question.. not great!


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

https://reviews.llvm.org/D59540



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

[PATCH] D59461: [analyzer] Fix an assertion failure if plugins added dependencies

[Balogh, Ádám via Phabricator via cfe-commits]

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

This one seems straightforward.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59461



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

[PATCH] D54978: Move the SMT API to LLVM

[Mikhail Ramalho via Phabricator via cfe-commits]

mikhail.ramalho updated this revision to Diff 191305.
mikhail.ramalho added a comment.

Fix copy-and-paste error.


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

https://reviews.llvm.org/D54978

Files:
  clang/CMakeLists.txt
  clang/cmake/modules/FindZ3.cmake
  clang/include/clang/Config/config.h.cmake
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTAPI.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h
  clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp
  clang/lib/StaticAnalyzer/Core/CMakeLists.txt
  clang/lib/StaticAnalyzer/Core/SMTConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/Z3ConstraintManager.cpp
  clang/test/CMakeLists.txt
  clang/test/lit.site.cfg.py.in
  llvm/CMakeLists.txt
  llvm/cmake/modules/FindZ3.cmake
  llvm/cmake/modules/LLVMConfig.cmake.in
  llvm/include/llvm/Config/config.h.cmake
  llvm/include/llvm/Support/SMTAPI.h
  llvm/lib/Support/CMakeLists.txt
  llvm/lib/Support/Z3Solver.cpp

Index: llvm/lib/Support/Z3Solver.cpp
===
--- llvm/lib/Support/Z3Solver.cpp
+++ llvm/lib/Support/Z3Solver.cpp
@@ -1,4 +1,4 @@
-//== Z3ConstraintManager.cpp *- C++ -*--==//
+//== Z3Solver.cpp ---*- C++ -*--==//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -6,18 +6,14 @@
 //
 //===--===//
 
-#include "clang/Basic/TargetInfo.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/SMTConstraintManager.h"
-#include "clang/StaticAnalyzer/Core/PathSensitive/SMTConv.h"
+#include "llvm/ADT/Twine.h"
+#include "llvm/Config/config.h"
+#include "llvm/Support/SMTAPI.h"
+#include 
 
-#include "clang/Config/config.h"
+using namespace llvm;
 
-using namespace clang;
-using namespace ento;
-
-#if CLANG_ANALYZER_WITH_Z3
+#if LLVM_WITH_Z3
 
 #include 
 
@@ -818,18 +814,13 @@
 
 #endif
 
-SMTSolverRef clang::ento::CreateZ3Solver() {
-#if CLANG_ANALYZER_WITH_Z3
+llvm::SMTSolverRef llvm::CreateZ3Solver() {
+#if LLVM_WITH_Z3
   return llvm::make_unique();
 #else
   llvm::report_fatal_error("Clang was not compiled with Z3 support, rebuild "
-   "with -DCLANG_ANALYZER_ENABLE_Z3_SOLVER=ON",
+   "with -DLLVM_ENABLE_Z3_SOLVER=ON",
false);
   return nullptr;
 #endif
 }
-
-std::unique_ptr
-ento::CreateZ3ConstraintManager(ProgramStateManager &StMgr, SubEngine *Eng) {
-  return llvm::make_unique(Eng, StMgr.getSValBuilder());
-}
Index: llvm/lib/Support/CMakeLists.txt
===
--- llvm/lib/Support/CMakeLists.txt
+++ llvm/lib/Support/CMakeLists.txt
@@ -44,6 +44,13 @@
   set (delayload_flags delayimp -delayload:shell32.dll -delayload:ole32.dll)
 endif()
 
+# Link Z3 if the user wants to build it.
+if(LLVM_WITH_Z3)
+  set(Z3_LINK_FILES ${Z3_LIBRARIES})
+else()
+  set(Z3_LINK_FILES "")
+endif()
+
 add_llvm_library(LLVMSupport
   AArch64TargetParser.cpp
   ARMTargetParser.cpp
@@ -152,6 +159,7 @@
   regfree.c
   regstrlcpy.c
   xxhash.cpp
+  Z3Solver.cpp
 
 # System
   Atomic.cpp
@@ -177,7 +185,14 @@
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/ADT
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Support
   ${Backtrace_INCLUDE_DIRS}
-  LINK_LIBS ${system_libs} ${delayload_flags}
+  LINK_LIBS ${system_libs} ${delayload_flags} ${Z3_LINK_FILES}
   )
 
 set_property(TARGET LLVMSupport PROPERTY LLVM_SYSTEM_LIBS "${system_libs}")
+
+if(LLVM_WITH_Z3)
+  target_include_directories(LLVMSupport SYSTEM
+PRIVATE
+${Z3_INCLUDE_DIR}
+)
+endif()
Index: llvm/include/llvm/Support/SMTAPI.h
===
--- llvm/include/llvm/Support/SMTAPI.h
+++ llvm/include/llvm/Support/SMTAPI.h
@@ -11,15 +11,16 @@
 //
 //===--===//
 
-#ifndef LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SMTSOLVER_H
-#define LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SMTSOLVER_H
+#ifndef LLVM_SUPPORT_SMTAPI_H
+#define LLVM_SUPPORT_SMTAPI_H
 
-#include "clang/Basic/TargetInfo.h"
+#include "llvm/ADT/APFloat.h"
 #include "llvm/ADT/APSInt.h"
 #include "llvm/ADT/FoldingSet.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
 
-namespace clang {
-namespace ento {
+namespace llvm {
 
 /// Generic base class for SMT sorts
 class SMTSort {
@@ -399,7 +400,6 @@
 /// Convenience method to create and Z3Solver object
 SMTSolverRef CreateZ3Solver();
 
-} // namespace ento
-} // namespace clang
+} // namespace llvm
 
 #endif
Index: llvm/include/llvm/Config/config.h.cmake
===

[PATCH] D54978: Move the SMT API to LLVM

[Mikhail Ramalho via Phabricator via cfe-commits]

mikhail.ramalho marked an inline comment as done.
mikhail.ramalho added a comment.

Fixed.


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

https://reviews.llvm.org/D54978



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

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 191311.
MyDeveloperDay added a comment.

Minor modification to improve the `[=]` case

`[&Columns]` and `[Columns]` are not yet fixed and will not be correctly 
renamed to `[&columns]` and `[columns]`

But at least they are left unaltered


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

https://reviews.llvm.org/D59540

Files:
  clang-tidy/readability/IdentifierNamingCheck.cpp
  test/clang-tidy/readability-identifier-naming.cpp

Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -501,3 +501,43 @@
 // CHECK-FIXES: {{^}}int * const lc_PointerB = nullptr;{{$}}
 }
 
+
+bool Foo() {
+  bool Columns=false;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'Columns'
+// CHECK-FIXES: {{^}}  bool columns=false;
+  auto ptr=[&]{return Columns;}();
+// CHECK-FIXES: {{^}}  auto ptr=[&]{return columns;}();
+  return Columns;
+// CHECK-FIXES: {{^}}  return columns;
+}
+
+bool Foo1() {
+  bool Columns=false;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'Columns'
+// CHECK-FIXES: {{^}}  bool columns=false;
+  auto ptr=[=]{return Columns;}();
+// CHECK-FIXES: {{^}}  auto ptr=[=]{return columns;}();
+  return Columns;
+// CHECK-FIXES: {{^}}  return columns;
+}
+
+bool Foo2() {
+  bool Columns=false;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'Columns'
+// CHECK-FIXES: {{^}}  bool columns=false;
+  auto ptr=[&Columns]{return Columns;}();
+// XXX_CHECK-FIXES: {{^}}  auto ptr=[&columns]{return columns;}();
+  return Columns;
+// CHECK-FIXES: {{^}}  return columns;
+}
+
+bool Foo3() {
+  bool Columns=false;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'Columns'
+// CHECK-FIXES: {{^}}  bool columns=false;
+  auto ptr=[Columns]{return Columns;}();
+// XXX_CHECK-FIXES: {{^}}  auto ptr=[columns]{return columns;}();
+  return Columns;
+// CHECK-FIXES: {{^}}  return columns;
+}
Index: clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -396,7 +396,7 @@
 
   if (isa(D) && NamingStyles[SK_ObjcIvar])
 return SK_ObjcIvar;
-  
+
   if (isa(D) && NamingStyles[SK_Typedef])
 return SK_Typedef;
 
@@ -506,7 +506,7 @@
   return SK_ParameterPack;
 
 if (!Type.isNull() && Type.getTypePtr()->isAnyPointerType() && NamingStyles[SK_PointerParameter])
-return SK_PointerParameter;
+  return SK_PointerParameter;
 
 if (NamingStyles[SK_Parameter])
   return SK_Parameter;
@@ -557,7 +557,7 @@
 
 if (Decl->isStaticLocal() && NamingStyles[SK_StaticVariable])
   return SK_StaticVariable;
- 
+
 if (Decl->isLocalVarDecl() && Type.getTypePtr()->isAnyPointerType() && NamingStyles[SK_LocalPointer])
   return SK_LocalPointer;
 
@@ -787,10 +787,28 @@
   }
 
   if (const auto *DeclRef = Result.Nodes.getNodeAs("declRef")) {
-SourceRange Range = DeclRef->getNameInfo().getSourceRange();
-addUsage(NamingCheckFailures, DeclRef->getDecl(), Range,
- Result.SourceManager);
-return;
+const auto &Parents = Result.Context->getParents(*DeclRef);
+bool LambdaDeclRef = false;
+
+if (!Parents.empty()) {
+  const Stmt *ST = Parents[0].get();
+  // Step over the ImplicitCastExpr
+  if (ST && isa(ST)) {
+const auto &CastParents = Result.Context->getParents(*ST);
+if (!CastParents.empty())
+  ST = CastParents[0].get();
+  }
+
+  if (ST && isa(ST))
+LambdaDeclRef = true;
+}
+
+if (!LambdaDeclRef) {
+  SourceRange Range = DeclRef->getNameInfo().getSourceRange();
+  addUsage(NamingCheckFailures, DeclRef->getDecl(), Range,
+   Result.SourceManager);
+  return;
+}
   }
 
   if (const auto *Decl = Result.Nodes.getNodeAs("decl")) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

Thanks for fixing this! Could you expand the test a bit? See the inline comment.




Comment at: test/clang-tidy/readability-identifier-naming.cpp:506
+bool Foo() {
+  bool Columns=false;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local 
variable 'Columns'

If the formatting is not critical for the logic of the test, please 
clang-format the new test code.



Comment at: test/clang-tidy/readability-identifier-naming.cpp:509
+// CHECK-FIXES: {{^}}  bool columns=false;
+  auto ptr=[&]{return Columns;}();
+// CHECK-FIXES: {{^}}  auto ptr=[&]{return columns;}();

Please add more tests with
  1) by value automatic captures
  2) manual captures by value
  3) manual captures by reference
  4) nested lambdas capturing the same variable

A bit more nested code inside the lambda would also be interesting (where the 
use of the variable would be wrapped in a couple of compound statements).


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

https://reviews.llvm.org/D59540



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

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 191323.
kadircet marked 2 inline comments as done.
kadircet added a comment.

- Show include stack in diagnostic message
- Point to exact include location


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/Compiler.h
  clangd/Diagnostics.cpp
  clangd/Diagnostics.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/DiagnosticsTests.cpp

Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -9,6 +9,7 @@
 #include "Annotations.h"
 #include "ClangdUnit.h"
 #include "SourceCode.h"
+#include "TestFS.h"
 #include "TestIndex.h"
 #include "TestTU.h"
 #include "index/MemIndex.h"
@@ -73,7 +74,6 @@
   return true;
 }
 
-
 // Helper function to make tests shorter.
 Position pos(int line, int character) {
   Position Res;
@@ -603,7 +603,174 @@
   "Add include \"x.h\" for symbol a::X");
 }
 
+ParsedAST
+build(const std::string &MainFile, const llvm::StringMap &Files,
+  llvm::Optional LimitDiagsOutsideMainFile = llvm::None) {
+  std::vector Cmd = {"clang", MainFile.c_str()};
+  ParseInputs Inputs;
+  Inputs.CompileCommand.Filename = MainFile;
+  Inputs.CompileCommand.CommandLine = {Cmd.begin(), Cmd.end()};
+  Inputs.CompileCommand.Directory = testRoot();
+  Inputs.Contents = Files.lookup(MainFile);
+  Inputs.FS = buildTestFS(Files);
+  Inputs.Opts = ParseOptions();
+  Inputs.Opts.LimitDiagsOutsideMainFile = LimitDiagsOutsideMainFile;
+  auto PCHs = std::make_shared();
+  auto CI = buildCompilerInvocation(Inputs);
+  assert(CI && "Failed to build compilation invocation.");
+  auto Preamble =
+  buildPreamble(MainFile, *CI,
+/*OldPreamble=*/nullptr,
+/*OldCompileCommand=*/Inputs.CompileCommand, Inputs, PCHs,
+/*StoreInMemory=*/true, /*PreambleCallback=*/nullptr);
+  auto AST = buildAST(MainFile, createInvocationFromCommandLine(Cmd), Inputs,
+  Preamble, PCHs);
+  if (!AST.hasValue()) {
+ADD_FAILURE() << "Failed to build code:\n" << Files.lookup(MainFile);
+llvm_unreachable("Failed to build TestTU!");
+  }
+  return std::move(*AST);
+}
+
+TEST(DiagsInHeaders, DiagInsideHeader) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+void foo() {})cpp");
+  std::string MainFile = testPath("main.cc");
+  llvm::StringMap Files = {{MainFile, Main.code()},
+{testPath("a.h"), "no_type_spec;"}};
+  auto FS = buildTestFS(Files);
+
+  EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(), "/clangd-test/a.h:1:1: C++ requires a "
+ "type specifier for all declarations")));
+}
+
+TEST(DiagsInHeaders, DiagInTransitiveInclude) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+void foo() {})cpp");
+  std::string MainFile = testPath("main.cc");
+  llvm::StringMap Files = {{MainFile, Main.code()},
+{testPath("a.h"), "#include \"b.h\""},
+{testPath("b.h"), "no_type_spec;"}};
+  auto FS = buildTestFS(Files);
+
+  EXPECT_THAT(build(MainFile, Files).getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range(),
+   "In file included from: "
+   "/clangd-test/a.h:1:10:\n/clangd-test/b.h:1:1: C++ "
+   "requires a type specifier for all declarations")));
+}
+
+TEST(DiagsInHeaders, DiagInMultipleHeaders) {
+  Annotations Main(R"cpp(
+#include $a[["a.h"]]
+#include $b[["b.h"]]
+void foo() {})cpp");
+  std::string MainFile = testPath("main.cc");
+  llvm::StringMap Files = {{MainFile, Main.code()},
+{testPath("a.h"), "no_type_spec;"},
+{testPath("b.h"), "no_type_spec;"}};
+  auto FS = buildTestFS(Files);
+
+  EXPECT_THAT(
+  build(MainFile, Files).getDiagnostics(),
+  UnorderedElementsAre(
+  Diag(Main.range("a"), "/clangd-test/a.h:1:1: C++ requires a type "
+"specifier for all declarations"),
+  Diag(Main.range("b"), "/clangd-test/b.h:1:1: C++ requires a type "
+"specifier for all declarations")));
+}
+
+TEST(DiagsInHeaders, PreferExpansionLocation) {
+  Annotations Main(R"cpp(
+#include [["a.h"]]
+#include "b.h"
+void foo() {})cpp");
+  std::string MainFile = testPath("main.cc");
+  llvm::StringMap Files = {
+  {MainFile, Main.code()},
+  {testPath("a.h"), "#include \"b.h\"\n"},
+  {testPath("b.h"), "#ifndef X\n#define X\nno_type_spec;\n#endif"}};

[PATCH] D57662: [clang-tidy] Parallelize clang-tidy-diff.py

[Alexander Kornienko via Phabricator via cfe-commits]

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

LG with a nit




Comment at: clang-tidy/tool/clang-tidy-diff.py:90
+  # the top level key 'Diagnostics' in the output yaml files
+  mergekey="Diagnostics"
+  merged=[]

nit: Spaces around operators, please


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

https://reviews.llvm.org/D57662



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

Re: r356222 - [analyzer] Support C++17 aggregates with bases without constructors.

[Alexander Kornienko via cfe-commits]

A reduced test case:
$ cat test-RegionStoreManager__bindStruct.cc
struct a {};
class b : a {};
b c() { b d{c()}; }
$ ./clang-tidy -checks="-*,clang-analyzer*"
test-RegionStoreManager__bindStruct.cc -- -std=c++17
assert.h assertion failed at
tools/clang/lib/StaticAnalyzer/Core/RegionStore.cpp:2362 in (anonymous
namespace)::RegionBindingsRef (anonym
ous namespace)::RegionStoreManager::bindStruct(RegionBindingsConstRef,
const clang::ento::TypedValueRegion *, clang::ento::SVal):
CRD->isAggregate() && "Non-aggregates are constructed with a constructor!"
@ 0x559908170326  __assert_fail
@ 0x5599068d4854  (anonymous
namespace)::RegionStoreManager::bindStruct()
@ 0x5599068c93c8  (anonymous namespace)::RegionStoreManager::Bind()
@ 0x5599068b409f  clang::ento::ProgramState::bindLoc()
@ 0x559906865935
clang::ento::ExprEngine::processPointerEscapedOnBind()
@ 0x55990685d4b3  clang::ento::ExprEngine::evalBind()
@ 0x559906872a43  clang::ento::ExprEngine::VisitDeclStmt()
@ 0x55990685c16f  clang::ento::ExprEngine::Visit()
@ 0x559906858b1f  clang::ento::ExprEngine::ProcessStmt()
@ 0x559906858808  clang::ento::ExprEngine::processCFGElement()
@ 0x55990684cb65  clang::ento::CoreEngine::HandlePostStmt()
@ 0x55990684bf5c  clang::ento::CoreEngine::ExecuteWorkList()
@ 0x5599065b635b  (anonymous
namespace)::AnalysisConsumer::HandleCode()
@ 0x5599065a0135  (anonymous
namespace)::AnalysisConsumer::HandleTranslationUnit()
@ 0x559906bb7cbc  clang::MultiplexConsumer::HandleTranslationUnit()
@ 0x559906d226d4  clang::ParseAST()
@ 0x559906b98a83  clang::FrontendAction::Execute()
@ 0x559906b31cd1  clang::CompilerInstance::ExecuteAction()
@ 0x559906a9cf61
clang::tooling::FrontendActionFactory::runInvocation()
@ 0x55990620cc07
clang::tidy::runClangTidy()::ActionFactory::runInvocation()
@ 0x559906a9ccca  clang::tooling::ToolInvocation::runInvocation()
@ 0x559906a9c646  clang::tooling::ToolInvocation::run()
@ 0x559906a9ef22  clang::tooling::ClangTool::run()
@ 0x559906207ecf  clang::tidy::runClangTidy()
@ 0x559902d47c45  main

On Tue, Mar 19, 2019 at 1:14 AM Alexander Kornienko 
wrote:

> On Fri, Mar 15, 2019 at 1:21 AM Artem Dergachev via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> Author: dergachev
>> Date: Thu Mar 14 17:22:59 2019
>> New Revision: 356222
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=356222&view=rev
>> Log:
>> [analyzer] Support C++17 aggregates with bases without constructors.
>>
>> RegionStore now knows how to bind a nonloc::CompoundVal that represents
>> the
>> value of an aggregate initializer when it has its initial segment of
>> sub-values
>> correspond to base classes.
>>
>> Additionally, fixes the crash from pr40022.
>>
>> Differential Revision: https://reviews.llvm.org/D59054
>>
>> Modified:
>> cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
>> cfe/trunk/test/Analysis/array-struct-region.cpp
>>
>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=356222&r1=356221&r2=356222&view=diff
>>
>> ==
>> --- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
>> +++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Thu Mar 14 17:22:59
>> 2019
>> @@ -2334,12 +2334,57 @@ RegionBindingsRef RegionStoreManager::bi
>>if (V.isUnknown() || !V.getAs())
>>  return bindAggregate(B, R, UnknownVal());
>>
>> +  // The raw CompoundVal is essentially a symbolic InitListExpr: an
>> (immutable)
>> +  // list of other values. It appears pretty much only when there's an
>> actual
>> +  // initializer list expression in the program, and the analyzer tries
>> to
>> +  // unwrap it as soon as possible.
>> +  // This code is where such unwrap happens: when the compound value is
>> put into
>> +  // the object that it was supposed to initialize (it's an
>> *initializer* list,
>> +  // after all), instead of binding the whole value to the whole object,
>> we bind
>> +  // sub-values to sub-objects. Sub-values may themselves be compound
>> values,
>> +  // and in this case the procedure becomes recursive.
>> +  // FIXME: The annoying part about compound values is that they don't
>> carry
>> +  // any sort of information about which value corresponds to which
>> sub-object.
>> +  // It's simply a list of values in the middle of nowhere; we expect to
>> match
>> +  // them to sub-objects, essentially, "by index": first value binds to
>> +  // the first field, second value binds to the second field, etc.
>> +  // It would have been much safer to organize non-lazy compound values
>> as
>> +  // a mapping from fields/bases to values.
>>const nonloc::CompoundVal& CV = V.castAs();
>>nonloc::CompoundVal::iterator VI = CV.begin(), VE

Re: r356222 - [analyzer] Support C++17 aggregates with bases without constructors.

[Alexander Kornienko via cfe-commits]

Filed this as https://bugs.llvm.org/show_bug.cgi?id=41142. Any hope for a
prompt fix here?

On Tue, Mar 19, 2019 at 5:34 PM Alexander Kornienko 
wrote:

> A reduced test case:
> $ cat test-RegionStoreManager__bindStruct.cc
> struct a {};
> class b : a {};
> b c() { b d{c()}; }
> $ ./clang-tidy -checks="-*,clang-analyzer*"
> test-RegionStoreManager__bindStruct.cc -- -std=c++17
> assert.h assertion failed at
> tools/clang/lib/StaticAnalyzer/Core/RegionStore.cpp:2362 in (anonymous
> namespace)::RegionBindingsRef (anonym
> ous namespace)::RegionStoreManager::bindStruct(RegionBindingsConstRef,
> const clang::ento::TypedValueRegion *, clang::ento::SVal):
> CRD->isAggregate() && "Non-aggregates are constructed with a constructor!"
> @ 0x559908170326  __assert_fail
> @ 0x5599068d4854  (anonymous
> namespace)::RegionStoreManager::bindStruct()
> @ 0x5599068c93c8  (anonymous namespace)::RegionStoreManager::Bind()
> @ 0x5599068b409f  clang::ento::ProgramState::bindLoc()
> @ 0x559906865935
> clang::ento::ExprEngine::processPointerEscapedOnBind()
> @ 0x55990685d4b3  clang::ento::ExprEngine::evalBind()
> @ 0x559906872a43  clang::ento::ExprEngine::VisitDeclStmt()
> @ 0x55990685c16f  clang::ento::ExprEngine::Visit()
> @ 0x559906858b1f  clang::ento::ExprEngine::ProcessStmt()
> @ 0x559906858808  clang::ento::ExprEngine::processCFGElement()
> @ 0x55990684cb65  clang::ento::CoreEngine::HandlePostStmt()
> @ 0x55990684bf5c  clang::ento::CoreEngine::ExecuteWorkList()
> @ 0x5599065b635b  (anonymous
> namespace)::AnalysisConsumer::HandleCode()
> @ 0x5599065a0135  (anonymous
> namespace)::AnalysisConsumer::HandleTranslationUnit()
> @ 0x559906bb7cbc  clang::MultiplexConsumer::HandleTranslationUnit()
> @ 0x559906d226d4  clang::ParseAST()
> @ 0x559906b98a83  clang::FrontendAction::Execute()
> @ 0x559906b31cd1  clang::CompilerInstance::ExecuteAction()
> @ 0x559906a9cf61
> clang::tooling::FrontendActionFactory::runInvocation()
> @ 0x55990620cc07
> clang::tidy::runClangTidy()::ActionFactory::runInvocation()
> @ 0x559906a9ccca  clang::tooling::ToolInvocation::runInvocation()
> @ 0x559906a9c646  clang::tooling::ToolInvocation::run()
> @ 0x559906a9ef22  clang::tooling::ClangTool::run()
> @ 0x559906207ecf  clang::tidy::runClangTidy()
> @ 0x559902d47c45  main
>
> On Tue, Mar 19, 2019 at 1:14 AM Alexander Kornienko 
> wrote:
>
>> On Fri, Mar 15, 2019 at 1:21 AM Artem Dergachev via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>>
>>> Author: dergachev
>>> Date: Thu Mar 14 17:22:59 2019
>>> New Revision: 356222
>>>
>>> URL: http://llvm.org/viewvc/llvm-project?rev=356222&view=rev
>>> Log:
>>> [analyzer] Support C++17 aggregates with bases without constructors.
>>>
>>> RegionStore now knows how to bind a nonloc::CompoundVal that represents
>>> the
>>> value of an aggregate initializer when it has its initial segment of
>>> sub-values
>>> correspond to base classes.
>>>
>>> Additionally, fixes the crash from pr40022.
>>>
>>> Differential Revision: https://reviews.llvm.org/D59054
>>>
>>> Modified:
>>> cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
>>> cfe/trunk/test/Analysis/array-struct-region.cpp
>>>
>>> Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
>>> URL:
>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=356222&r1=356221&r2=356222&view=diff
>>>
>>> ==
>>> --- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp (original)
>>> +++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Thu Mar 14
>>> 17:22:59 2019
>>> @@ -2334,12 +2334,57 @@ RegionBindingsRef RegionStoreManager::bi
>>>if (V.isUnknown() || !V.getAs())
>>>  return bindAggregate(B, R, UnknownVal());
>>>
>>> +  // The raw CompoundVal is essentially a symbolic InitListExpr: an
>>> (immutable)
>>> +  // list of other values. It appears pretty much only when there's an
>>> actual
>>> +  // initializer list expression in the program, and the analyzer tries
>>> to
>>> +  // unwrap it as soon as possible.
>>> +  // This code is where such unwrap happens: when the compound value is
>>> put into
>>> +  // the object that it was supposed to initialize (it's an
>>> *initializer* list,
>>> +  // after all), instead of binding the whole value to the whole
>>> object, we bind
>>> +  // sub-values to sub-objects. Sub-values may themselves be compound
>>> values,
>>> +  // and in this case the procedure becomes recursive.
>>> +  // FIXME: The annoying part about compound values is that they don't
>>> carry
>>> +  // any sort of information about which value corresponds to which
>>> sub-object.
>>> +  // It's simply a list of values in the middle of nowhere; we expect
>>> to match
>>> +  // them to sub-objects, essentially, "by index": first value binds to

r356472 - [OPENMP] Codegen for local variables with the allocate pragma.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Mar 19 09:41:16 2019
New Revision: 356472

URL: http://llvm.org/viewvc/llvm-project?rev=356472&view=rev
Log:
[OPENMP] Codegen for local variables with the allocate pragma.

Added initial codegen for the local variables with the #pragma omp
allocate directive. Instead of allocating the variables on the stack,
__kmpc_alloc|__kmpc_free functions are used for memory (de-)allocation.

Added:
cfe/trunk/test/OpenMP/allocate_codegen.cpp
  - copied, changed from r356458, 
cfe/trunk/test/OpenMP/allocate_allocator_ast_print.cpp
Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/test/OpenMP/allocate_allocator_ast_print.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=356472&r1=356471&r2=356472&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Tue Mar 19 09:41:16 2019
@@ -667,6 +667,10 @@ enum OpenMPRTLFunction {
   // Call to void *__kmpc_task_reduction_get_th_data(int gtid, void *tg, void
   // *d);
   OMPRTL__kmpc_task_reduction_get_th_data,
+  // Call to void *__kmpc_alloc(int gtid, size_t sz, const omp_allocator_t 
*al);
+  OMPRTL__kmpc_alloc,
+  // Call to void __kmpc_free(int gtid, void *ptr, const omp_allocator_t *al);
+  OMPRTL__kmpc_free,
 
   //
   // Offloading related calls
@@ -2195,6 +2199,26 @@ llvm::FunctionCallee CGOpenMPRuntime::cr
 FnTy, /*Name=*/"__kmpc_task_reduction_get_th_data");
 break;
   }
+  case OMPRTL__kmpc_alloc: {
+// Build to void *__kmpc_alloc(int gtid, size_t sz, const omp_allocator_t
+// *al);
+// omp_allocator_t type is void *.
+llvm::Type *TypeParams[] = {CGM.IntTy, CGM.SizeTy, CGM.VoidPtrPtrTy};
+auto *FnTy =
+llvm::FunctionType::get(CGM.VoidPtrTy, TypeParams, /*isVarArg=*/false);
+RTLFn = CGM.CreateRuntimeFunction(FnTy, /*Name=*/"__kmpc_alloc");
+break;
+  }
+  case OMPRTL__kmpc_free: {
+// Build to void __kmpc_free(int gtid, void *ptr, const omp_allocator_t
+// *al);
+// omp_allocator_t type is void *.
+llvm::Type *TypeParams[] = {CGM.IntTy, CGM.VoidPtrTy, CGM.VoidPtrPtrTy};
+auto *FnTy =
+llvm::FunctionType::get(CGM.VoidTy, TypeParams, /*isVarArg=*/false);
+RTLFn = CGM.CreateRuntimeFunction(FnTy, /*Name=*/"__kmpc_free");
+break;
+  }
   case OMPRTL__kmpc_push_target_tripcount: {
 // Build void __kmpc_push_target_tripcount(int64_t device_id, kmp_uint64
 // size);
@@ -9669,8 +9693,81 @@ Address CGOpenMPRuntime::getParameterAdd
   return CGF.GetAddrOfLocalVar(NativeParam);
 }
 
+namespace {
+/// Cleanup action for allocate support.
+class OMPAllocateCleanupTy final : public EHScopeStack::Cleanup {
+public:
+  static const int CleanupArgs = 3;
+
+private:
+  llvm::FunctionCallee RTLFn;
+  llvm::Value *Args[CleanupArgs];
+
+public:
+  OMPAllocateCleanupTy(llvm::FunctionCallee RTLFn,
+   ArrayRef CallArgs)
+  : RTLFn(RTLFn) {
+assert(CallArgs.size() == CleanupArgs &&
+   "Size of arguments does not match.");
+std::copy(CallArgs.begin(), CallArgs.end(), std::begin(Args));
+  }
+  void Emit(CodeGenFunction &CGF, Flags /*flags*/) override {
+if (!CGF.HaveInsertPoint())
+  return;
+CGF.EmitRuntimeCall(RTLFn, Args);
+  }
+};
+} // namespace
+
 Address CGOpenMPRuntime::getAddressOfLocalVariable(CodeGenFunction &CGF,
const VarDecl *VD) {
+  const VarDecl *CVD = VD->getCanonicalDecl();
+  if (!CVD->hasAttr())
+return Address::invalid();
+  for (const Attr *A: CVD->getAttrs()) {
+if (const auto *AA = dyn_cast(A)) {
+  auto &Elem = OpenMPLocThreadIDMap.FindAndConstruct(CGF.CurFn);
+  if (!Elem.second.ServiceInsertPt)
+setLocThreadIdInsertPt(CGF);
+  CGBuilderTy::InsertPointGuard IPG(CGF.Builder);
+  CGF.Builder.SetInsertPoint(Elem.second.ServiceInsertPt);
+  llvm::Value *Size;
+  CharUnits Align = CGM.getContext().getDeclAlign(CVD);
+  if (CVD->getType()->isVariablyModifiedType()) {
+Size = CGF.getTypeSize(CVD->getType());
+Align = CGM.getContext().getTypeAlignInChars(CVD->getType());
+  } else {
+CharUnits Sz = CGM.getContext().getTypeSizeInChars(CVD->getType());
+Align = CGM.getContext().getDeclAlign(CVD);
+Size = CGM.getSize(Sz.alignTo(Align));
+  }
+  llvm::Value *ThreadID = getThreadID(CGF, CVD->getBeginLoc());
+  llvm::Value *Allocator;
+  if (const Expr *AllocExpr = AA->getAllocator()) {
+Allocator = CGF.EmitScalarExpr(AA->getAllocator());
+  } else {
+// Default allocator in libomp is nullptr.
+Allocator = llvm::ConstantPointerNull::get(CGM.VoidPtrPtrTy);
+  }
+  llvm::Value *Ar

[PATCH] D59449: [clang-tidy] Integrate clang-tidy-diff.py machinery into run-clang-tidy.py

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added a comment.

How is this functionality different from the clang-tidy-diff.py script with the 
-j option being added in the other patch?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59449



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

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:790
   if (const auto *DeclRef = Result.Nodes.getNodeAs("declRef")) {
-SourceRange Range = DeclRef->getNameInfo().getSourceRange();
-addUsage(NamingCheckFailures, DeclRef->getDecl(), Range,
- Result.SourceManager);
-return;
+const auto &Parents = Result.Context->getParents(*DeclRef);
+bool LambdaDeclRef = false;

Type is not obvious, so auto should not be used.



Comment at: clang-tidy/readability/IdentifierNamingCheck.cpp:797
+  if (ST && isa(ST)) {
+const auto &CastParents = Result.Context->getParents(*ST);
+if (!CastParents.empty())

Type is not obvious, so auto should not be used.


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

https://reviews.llvm.org/D59540



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

r356475 - [Sema] Adjust addr space of reference operand in compound assignment

[Anastasia Stulova via cfe-commits]

Author: stulova
Date: Tue Mar 19 09:50:21 2019
New Revision: 356475

URL: http://llvm.org/viewvc/llvm-project?rev=356475&view=rev
Log:
[Sema] Adjust addr space of reference operand in compound assignment

When we create overloads for the builtin compound assignment operators
we need to preserve address space for the reference operand taking it
from the argument that is passed in.

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


Modified:
cfe/trunk/lib/Sema/SemaOverload.cpp
cfe/trunk/test/CodeGenOpenCLCXX/addrspace-operators.cl

Modified: cfe/trunk/lib/Sema/SemaOverload.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=356475&r1=356474&r2=356475&view=diff
==
--- cfe/trunk/lib/Sema/SemaOverload.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOverload.cpp Tue Mar 19 09:50:21 2019
@@ -8513,17 +8513,16 @@ public:
Right < LastPromotedArithmeticType; ++Right) {
 QualType ParamTypes[2];
 ParamTypes[1] = ArithmeticTypes[Right];
-
+auto LeftBaseTy = AdjustAddressSpaceForBuiltinOperandType(
+S, ArithmeticTypes[Left], Args[0]);
 // Add this built-in operator as a candidate (VQ is empty).
-ParamTypes[0] =
-  S.Context.getLValueReferenceType(ArithmeticTypes[Left]);
+ParamTypes[0] = S.Context.getLValueReferenceType(LeftBaseTy);
 S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
   /*IsAssigmentOperator=*/isEqualOp);
 
 // Add this built-in operator as a candidate (VQ is 'volatile').
 if (VisibleTypeConversionsQuals.hasVolatile()) {
-  ParamTypes[0] =
-S.Context.getVolatileType(ArithmeticTypes[Left]);
+  ParamTypes[0] = S.Context.getVolatileType(LeftBaseTy);
   ParamTypes[0] = S.Context.getLValueReferenceType(ParamTypes[0]);
   S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
 /*IsAssigmentOperator=*/isEqualOp);
@@ -8579,15 +8578,14 @@ public:
Right < LastPromotedIntegralType; ++Right) {
 QualType ParamTypes[2];
 ParamTypes[1] = ArithmeticTypes[Right];
-
+auto LeftBaseTy = AdjustAddressSpaceForBuiltinOperandType(
+S, ArithmeticTypes[Left], Args[0]);
 // Add this built-in operator as a candidate (VQ is empty).
-ParamTypes[0] = S.Context.getLValueReferenceType(
-AdjustAddressSpaceForBuiltinOperandType(S, ArithmeticTypes[Left],
-Args[0]));
+ParamTypes[0] = S.Context.getLValueReferenceType(LeftBaseTy);
 S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
 if (VisibleTypeConversionsQuals.hasVolatile()) {
   // Add this built-in operator as a candidate (VQ is 'volatile').
-  ParamTypes[0] = ArithmeticTypes[Left];
+  ParamTypes[0] = LeftBaseTy;
   ParamTypes[0] = S.Context.getVolatileType(ParamTypes[0]);
   ParamTypes[0] = S.Context.getLValueReferenceType(ParamTypes[0]);
   S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);

Modified: cfe/trunk/test/CodeGenOpenCLCXX/addrspace-operators.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenOpenCLCXX/addrspace-operators.cl?rev=356475&r1=356474&r2=356475&view=diff
==
--- cfe/trunk/test/CodeGenOpenCLCXX/addrspace-operators.cl (original)
+++ cfe/trunk/test/CodeGenOpenCLCXX/addrspace-operators.cl Tue Mar 19 09:50:21 
2019
@@ -14,6 +14,8 @@ public:
 };
 
 __global E globE;
+volatile __global int globVI;
+__global int globI;
 //CHECK-LABEL: define spir_func void @_Z3barv()
 void bar() {
   C c;
@@ -25,13 +27,18 @@ void bar() {
   c.OrAssign(a);
 
   E e;
-  // CHECK: store i32 1, i32* %e
+  //CHECK: store i32 1, i32* %e
   e = b;
-  // CHECK: store i32 0, i32 addrspace(1)* @globE
+  //CHECK: store i32 0, i32 addrspace(1)* @globE
   globE = a;
-  // FIXME: Sema fails here because it thinks the types are incompatible.
-  //e = b;
-  //globE = a;
+  //CHECK: store i32 %or, i32 addrspace(1)* @globI
+  globI |= b;
+  //CHECK: store i32 %add, i32 addrspace(1)* @globI
+  globI += a;
+  //CHECK: store volatile i32 %and, i32 addrspace(1)* @globVI
+  globVI &= b;
+  //CHECK: store volatile i32 %sub, i32 addrspace(1)* @globVI
+  globVI -= a;
 }
 
 //CHECK: define linkonce_odr void @_ZNU3AS41C6AssignE1E(%class.C addrspace(4)* 
%this, i32 %e)


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

[PATCH] D59367: [Sema] Adjust address space of operands in remaining builtin operators

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC356475: [Sema] Adjust addr space of reference operand in 
compound assignment (authored by stulova, committed by ).
Herald added a subscriber: kristina.
Herald added a project: clang.

Repository:
  rC Clang

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

https://reviews.llvm.org/D59367

Files:
  lib/Sema/SemaOverload.cpp
  test/CodeGenOpenCLCXX/addrspace-operators.cl


Index: test/CodeGenOpenCLCXX/addrspace-operators.cl
===
--- test/CodeGenOpenCLCXX/addrspace-operators.cl
+++ test/CodeGenOpenCLCXX/addrspace-operators.cl
@@ -14,6 +14,8 @@
 };
 
 __global E globE;
+volatile __global int globVI;
+__global int globI;
 //CHECK-LABEL: define spir_func void @_Z3barv()
 void bar() {
   C c;
@@ -25,13 +27,18 @@
   c.OrAssign(a);
 
   E e;
-  // CHECK: store i32 1, i32* %e
+  //CHECK: store i32 1, i32* %e
   e = b;
-  // CHECK: store i32 0, i32 addrspace(1)* @globE
+  //CHECK: store i32 0, i32 addrspace(1)* @globE
   globE = a;
-  // FIXME: Sema fails here because it thinks the types are incompatible.
-  //e = b;
-  //globE = a;
+  //CHECK: store i32 %or, i32 addrspace(1)* @globI
+  globI |= b;
+  //CHECK: store i32 %add, i32 addrspace(1)* @globI
+  globI += a;
+  //CHECK: store volatile i32 %and, i32 addrspace(1)* @globVI
+  globVI &= b;
+  //CHECK: store volatile i32 %sub, i32 addrspace(1)* @globVI
+  globVI -= a;
 }
 
 //CHECK: define linkonce_odr void @_ZNU3AS41C6AssignE1E(%class.C addrspace(4)* 
%this, i32 %e)
Index: lib/Sema/SemaOverload.cpp
===
--- lib/Sema/SemaOverload.cpp
+++ lib/Sema/SemaOverload.cpp
@@ -8513,17 +8513,16 @@
Right < LastPromotedArithmeticType; ++Right) {
 QualType ParamTypes[2];
 ParamTypes[1] = ArithmeticTypes[Right];
-
+auto LeftBaseTy = AdjustAddressSpaceForBuiltinOperandType(
+S, ArithmeticTypes[Left], Args[0]);
 // Add this built-in operator as a candidate (VQ is empty).
-ParamTypes[0] =
-  S.Context.getLValueReferenceType(ArithmeticTypes[Left]);
+ParamTypes[0] = S.Context.getLValueReferenceType(LeftBaseTy);
 S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
   /*IsAssigmentOperator=*/isEqualOp);
 
 // Add this built-in operator as a candidate (VQ is 'volatile').
 if (VisibleTypeConversionsQuals.hasVolatile()) {
-  ParamTypes[0] =
-S.Context.getVolatileType(ArithmeticTypes[Left]);
+  ParamTypes[0] = S.Context.getVolatileType(LeftBaseTy);
   ParamTypes[0] = S.Context.getLValueReferenceType(ParamTypes[0]);
   S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet,
 /*IsAssigmentOperator=*/isEqualOp);
@@ -8579,15 +8578,14 @@
Right < LastPromotedIntegralType; ++Right) {
 QualType ParamTypes[2];
 ParamTypes[1] = ArithmeticTypes[Right];
-
+auto LeftBaseTy = AdjustAddressSpaceForBuiltinOperandType(
+S, ArithmeticTypes[Left], Args[0]);
 // Add this built-in operator as a candidate (VQ is empty).
-ParamTypes[0] = S.Context.getLValueReferenceType(
-AdjustAddressSpaceForBuiltinOperandType(S, ArithmeticTypes[Left],
-Args[0]));
+ParamTypes[0] = S.Context.getLValueReferenceType(LeftBaseTy);
 S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);
 if (VisibleTypeConversionsQuals.hasVolatile()) {
   // Add this built-in operator as a candidate (VQ is 'volatile').
-  ParamTypes[0] = ArithmeticTypes[Left];
+  ParamTypes[0] = LeftBaseTy;
   ParamTypes[0] = S.Context.getVolatileType(ParamTypes[0]);
   ParamTypes[0] = S.Context.getLValueReferenceType(ParamTypes[0]);
   S.AddBuiltinCandidate(ParamTypes, Args, CandidateSet);


Index: test/CodeGenOpenCLCXX/addrspace-operators.cl
===
--- test/CodeGenOpenCLCXX/addrspace-operators.cl
+++ test/CodeGenOpenCLCXX/addrspace-operators.cl
@@ -14,6 +14,8 @@
 };
 
 __global E globE;
+volatile __global int globVI;
+__global int globI;
 //CHECK-LABEL: define spir_func void @_Z3barv()
 void bar() {
   C c;
@@ -25,13 +27,18 @@
   c.OrAssign(a);
 
   E e;
-  // CHECK: store i32 1, i32* %e
+  //CHECK: store i32 1, i32* %e
   e = b;
-  // CHECK: store i32 0, i32 addrspace(1)* @globE
+  //CHECK: store i32 0, i32 addrspace(1)* @globE
   globE = a;
-  // FIXME: Sema fails here because it thinks the types are incompatible.
-  //e = b;
-  //globE = a;
+  //CHECK: store i32 %or, i32 addrspace(1)* @globI
+  globI |= b;
+  //CHECK: store i32 %add, i32 addrspace(1)* @globI
+  globI += a;
+  //CHECK: store volatile i32 %and, i32 addrspace(1)* @globVI
+  globVI &= b;
+  //CHECK: store vo

[PATCH] D59367: [Sema] Adjust address space of operands in remaining builtin operators

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D59367#1434353 , @rjmccall wrote:

> This patch LGTM.
>
> You might consider adding tests for weird cases like class types with 
> conversion operators to reference types.  To a certain extent that sort of 
> thing is unimplementable because the space of address spaces is infinite, so 
> we can't fall back on adding a candidate for every possible address space; 
> but even if we don't plan on addressing it, we can test our current behavior 
> to check that we don't do something unexpected (like crash or give a 
> nonsensical diagnostic).


Thanks for the suggestion. I will look into this!


Repository:
  rC Clang

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

https://reviews.llvm.org/D59367



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

[PATCH] D59360: [clang-tidy] Fix more false positives for bugprone-string-integer-assignment

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp:115
+Expr::EvalResult EvalResult;
+if (!E->EvaluateAsInt(EvalResult, Ctx, Expr::SE_AllowSideEffects))
+  return false;

courbet wrote:
> alexfh wrote:
> > I believe you should also check (or assert) that `E` is not 
> > instantiation-dependent before running the evaluator.
> Interesting. AFAICT if I don't check that , I end up warning in the cases 
> when the instantiation does result in a too large constant, which is what we 
> want (the user should add a cast if they want to silence this). 
IIUC, expression evaluation is just not supposed to be used on 
instantiation-dependent expressions. I've recently fixed a related crash 
(https://reviews.llvm.org/rL355401). I guess, there's a similar possibility 
here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59360



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

r356479 - [OpenCL] Minor improvements in default header testing

[Anastasia Stulova via cfe-commits]

Author: stulova
Date: Tue Mar 19 10:09:06 2019
New Revision: 356479

URL: http://llvm.org/viewvc/llvm-project?rev=356479&view=rev
Log:
[OpenCL] Minor improvements in default header testing

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


Modified:
cfe/trunk/test/Headers/opencl-c-header.cl

Modified: cfe/trunk/test/Headers/opencl-c-header.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Headers/opencl-c-header.cl?rev=356479&r1=356478&r2=356479&view=diff
==
--- cfe/trunk/test/Headers/opencl-c-header.cl (original)
+++ cfe/trunk/test/Headers/opencl-c-header.cl Tue Mar 19 10:09:06 2019
@@ -53,15 +53,14 @@
 // CHECK: _Z16convert_char_rtec
 // CHECK-NOT: _Z3ctzc
 // CHECK20: _Z3ctzc
-// CHECK20-NOT: _Z16convert_char_rtec
+// CHECK20: _Z16convert_char_rtec
 char f(char x) {
-#if !defined(__OPENCL_CPP_VERSION__) && (__OPENCL_C_VERSION__ != 
CL_VERSION_2_0)
-  return convert_char_rte(x);
-
-#else //__OPENCL_C_VERSION__
+// Check functionality from OpenCL 2.0 onwards
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ == CL_VERSION_2_0)
   ndrange_t t;
-  return ctz(x);
+  x = ctz(x);
 #endif //__OPENCL_C_VERSION__
+  return convert_char_rte(x);
 }
 
 // Verify that a builtin using a write_only image3d_t type is available


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

r356480 - [OPENMP]Remove unused parameter, NFC.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Mar 19 10:09:52 2019
New Revision: 356480

URL: http://llvm.org/viewvc/llvm-project?rev=356480&view=rev
Log:
[OPENMP]Remove unused parameter, NFC.

Parameter CodeGenModule &CGM is not required for CGOpenMPRuntime member
functions, since class holds the reference to the CGM.

Modified:
cfe/trunk/lib/CodeGen/CGDecl.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h

Modified: cfe/trunk/lib/CodeGen/CGDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDecl.cpp?rev=356480&r1=356479&r2=356480&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDecl.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDecl.cpp Tue Mar 19 10:09:52 2019
@@ -2574,5 +2574,5 @@ void CodeGenModule::EmitOMPDeclareMapper
 }
 
 void CodeGenModule::EmitOMPRequiresDecl(const OMPRequiresDecl *D) {
-  getOpenMPRuntime().checkArchForUnifiedAddressing(*this, D);
+  getOpenMPRuntime().checkArchForUnifiedAddressing(D);
 }

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=356480&r1=356479&r2=356480&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Tue Mar 19 10:09:52 2019
@@ -5499,9 +5499,9 @@ static void emitReductionCombiner(CodeGe
 }
 
 llvm::Function *CGOpenMPRuntime::emitReductionFunction(
-CodeGenModule &CGM, SourceLocation Loc, llvm::Type *ArgsType,
-ArrayRef Privates, ArrayRef LHSExprs,
-ArrayRef RHSExprs, ArrayRef ReductionOps) {
+SourceLocation Loc, llvm::Type *ArgsType, ArrayRef Privates,
+ArrayRef LHSExprs, ArrayRef RHSExprs,
+ArrayRef ReductionOps) {
   ASTContext &C = CGM.getContext();
 
   // void reduction_func(void *LHSArg, void *RHSArg);
@@ -5712,8 +5712,8 @@ void CGOpenMPRuntime::emitReduction(Code
 
   // 2. Emit reduce_func().
   llvm::Function *ReductionFn = emitReductionFunction(
-  CGM, Loc, CGF.ConvertTypeForMem(ReductionArrayTy)->getPointerTo(),
-  Privates, LHSExprs, RHSExprs, ReductionOps);
+  Loc, CGF.ConvertTypeForMem(ReductionArrayTy)->getPointerTo(), Privates,
+  LHSExprs, RHSExprs, ReductionOps);
 
   // 3. Create static kmp_critical_name lock = { 0 };
   std::string Name = getName({"reduction"});

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h?rev=356480&r1=356479&r2=356480&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h Tue Mar 19 10:09:52 2019
@@ -1229,7 +1229,7 @@ public:
   /// \param RHSExprs List of RHS in \a ReductionOps reduction operations.
   /// \param ReductionOps List of reduction operations in form 'LHS binop RHS'
   /// or 'operator binop(LHS, RHS)'.
-  llvm::Function *emitReductionFunction(CodeGenModule &CGM, SourceLocation Loc,
+  llvm::Function *emitReductionFunction(SourceLocation Loc,
 llvm::Type *ArgsType,
 ArrayRef Privates,
 ArrayRef LHSExprs,
@@ -1597,8 +1597,7 @@ public:
 
   /// Perform check on requires decl to ensure that target architecture
   /// supports unified addressing
-  virtual void checkArchForUnifiedAddressing(CodeGenModule &CGM,
- const OMPRequiresDecl *D) const {}
+  virtual void checkArchForUnifiedAddressing(const OMPRequiresDecl *D) const {}
 };
 
 /// Class supports emissionof SIMD-only code.

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=356480&r1=356479&r2=356480&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Tue Mar 19 10:09:52 2019
@@ -4316,8 +4316,8 @@ void CGOpenMPRuntimeNVPTX::emitReduction
   llvm::Value *RL = CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(
   ReductionList.getPointer(), CGF.VoidPtrTy);
   llvm::Function *ReductionFn = emitReductionFunction(
-  CGM, Loc, CGF.ConvertTypeForMem(ReductionArrayTy)->getPointerTo(),
-  Privates, LHSExprs, RHSExprs, ReductionOps);
+  Loc, CGF.ConvertTypeForMem(ReductionArrayTy)->getPointerTo(), Privates,
+  LHSExprs, RHSExprs, ReductionOps);
   llvm::Value *ReductionArrayTySize = CGF.getTypeSize(ReductionArrayTy);
   llvm::Function *ShuffleAndReduceFn = emitShuffleAndReduceFunction(
   CGM, Privates, ReductionArrayTy, ReductionFn, Loc);
@@ -4861

[PATCH] D59544: [OpenCL] Minor improvements in default header testing

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356479: [OpenCL] Minor improvements in default header 
testing (authored by stulova, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59544?vs=191289&id=191341#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59544

Files:
  cfe/trunk/test/Headers/opencl-c-header.cl


Index: cfe/trunk/test/Headers/opencl-c-header.cl
===
--- cfe/trunk/test/Headers/opencl-c-header.cl
+++ cfe/trunk/test/Headers/opencl-c-header.cl
@@ -53,15 +53,14 @@
 // CHECK: _Z16convert_char_rtec
 // CHECK-NOT: _Z3ctzc
 // CHECK20: _Z3ctzc
-// CHECK20-NOT: _Z16convert_char_rtec
+// CHECK20: _Z16convert_char_rtec
 char f(char x) {
-#if !defined(__OPENCL_CPP_VERSION__) && (__OPENCL_C_VERSION__ != 
CL_VERSION_2_0)
-  return convert_char_rte(x);
-
-#else //__OPENCL_C_VERSION__
+// Check functionality from OpenCL 2.0 onwards
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ == CL_VERSION_2_0)
   ndrange_t t;
-  return ctz(x);
+  x = ctz(x);
 #endif //__OPENCL_C_VERSION__
+  return convert_char_rte(x);
 }
 
 // Verify that a builtin using a write_only image3d_t type is available


Index: cfe/trunk/test/Headers/opencl-c-header.cl
===
--- cfe/trunk/test/Headers/opencl-c-header.cl
+++ cfe/trunk/test/Headers/opencl-c-header.cl
@@ -53,15 +53,14 @@
 // CHECK: _Z16convert_char_rtec
 // CHECK-NOT: _Z3ctzc
 // CHECK20: _Z3ctzc
-// CHECK20-NOT: _Z16convert_char_rtec
+// CHECK20: _Z16convert_char_rtec
 char f(char x) {
-#if !defined(__OPENCL_CPP_VERSION__) && (__OPENCL_C_VERSION__ != CL_VERSION_2_0)
-  return convert_char_rte(x);
-
-#else //__OPENCL_C_VERSION__
+// Check functionality from OpenCL 2.0 onwards
+#if defined(__OPENCL_CPP_VERSION__) || (__OPENCL_C_VERSION__ == CL_VERSION_2_0)
   ndrange_t t;
-  return ctz(x);
+  x = ctz(x);
 #endif //__OPENCL_C_VERSION__
+  return convert_char_rte(x);
 }
 
 // Verify that a builtin using a write_only image3d_t type is available
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: r356222 - [analyzer] Support C++17 aggregates with bases without constructors.

[Artem Dergachev via cfe-commits]

Hi,

I'll try to fix this ASAP!

It sounds as if i don't have nearly enough C++17 codebases for testing, 
are there any obvious open-source projects that you could recommend? I'd 
love to try to reincarnate 
http://green.lab.llvm.org/green/view/Experimental/job/StaticAnalyzerBenchmarks/ 
with those.


On 3/19/19 9:37 AM, Alexander Kornienko wrote:
Filed this as https://bugs.llvm.org/show_bug.cgi?id=41142. Any hope 
for a prompt fix here?


On Tue, Mar 19, 2019 at 5:34 PM Alexander Kornienko > wrote:


A reduced test case:
$ cat test-RegionStoreManager__bindStruct.cc
struct a {};
class b : a {};
b c() { b d{c()}; }
$ ./clang-tidy -checks="-*,clang-analyzer*"
test-RegionStoreManager__bindStruct.cc -- -std=c++17
assert.h assertion failed at
tools/clang/lib/StaticAnalyzer/Core/RegionStore.cpp:2362 in
(anonymous namespace)::RegionBindingsRef (anonym
ous
namespace)::RegionStoreManager::bindStruct(RegionBindingsConstRef,
const clang::ento::TypedValueRegion *, clang::ento::SVal):
CRD->isAggregate() && "Non-aggregates are constructed with a
constructor!"
    @     0x559908170326  __assert_fail
    @     0x5599068d4854  (anonymous
namespace)::RegionStoreManager::bindStruct()
    @     0x5599068c93c8  (anonymous
namespace)::RegionStoreManager::Bind()
    @     0x5599068b409f clang::ento::ProgramState::bindLoc()
    @     0x559906865935
clang::ento::ExprEngine::processPointerEscapedOnBind()
    @     0x55990685d4b3 clang::ento::ExprEngine::evalBind()
    @     0x559906872a43 clang::ento::ExprEngine::VisitDeclStmt()
    @     0x55990685c16f clang::ento::ExprEngine::Visit()
    @     0x559906858b1f clang::ento::ExprEngine::ProcessStmt()
    @     0x559906858808 clang::ento::ExprEngine::processCFGElement()
    @     0x55990684cb65 clang::ento::CoreEngine::HandlePostStmt()
    @     0x55990684bf5c clang::ento::CoreEngine::ExecuteWorkList()
    @     0x5599065b635b  (anonymous
namespace)::AnalysisConsumer::HandleCode()
    @     0x5599065a0135  (anonymous
namespace)::AnalysisConsumer::HandleTranslationUnit()
    @     0x559906bb7cbc
clang::MultiplexConsumer::HandleTranslationUnit()
    @     0x559906d226d4  clang::ParseAST()
    @     0x559906b98a83 clang::FrontendAction::Execute()
    @     0x559906b31cd1 clang::CompilerInstance::ExecuteAction()
    @     0x559906a9cf61
clang::tooling::FrontendActionFactory::runInvocation()
    @     0x55990620cc07
clang::tidy::runClangTidy()::ActionFactory::runInvocation()
    @     0x559906a9ccca
clang::tooling::ToolInvocation::runInvocation()
    @     0x559906a9c646 clang::tooling::ToolInvocation::run()
    @     0x559906a9ef22 clang::tooling::ClangTool::run()
    @     0x559906207ecf  clang::tidy::runClangTidy()
    @     0x559902d47c45  main

On Tue, Mar 19, 2019 at 1:14 AM Alexander Kornienko
mailto:ale...@google.com>> wrote:

On Fri, Mar 15, 2019 at 1:21 AM Artem Dergachev via
cfe-commits mailto:cfe-commits@lists.llvm.org>> wrote:

Author: dergachev
Date: Thu Mar 14 17:22:59 2019
New Revision: 356222

URL: http://llvm.org/viewvc/llvm-project?rev=356222&view=rev
Log:
[analyzer] Support C++17 aggregates with bases without
constructors.

RegionStore now knows how to bind a nonloc::CompoundVal
that represents the
value of an aggregate initializer when it has its initial
segment of sub-values
correspond to base classes.

Additionally, fixes the crash from pr40022.

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
cfe/trunk/test/Analysis/array-struct-region.cpp

Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
URL:

http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=356222&r1=356221&r2=356222&view=diff

==
--- cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
(original)
+++ cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp Thu
Mar 14 17:22:59 2019
@@ -2334,12 +2334,57 @@ RegionBindingsRef
RegionStoreManager::bi
   if (V.isUnknown() || !V.getAs())
     return bindAggregate(B, R, UnknownVal());

+  // The raw CompoundVal is essentially a symbolic
InitListExpr: an (immutable)
+  // list of other values. It appears pretty much only
when there's an actual
+  // initializer list expression in the program, and the
analyzer tries to
+  // 

[PATCH] D59302: [clangd] Surface diagnostics from headers inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet planned changes to this revision.
kadircet added a comment.

- Limit per include directive
- Use hardcoded value for limit


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302



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

Re: r356222 - [analyzer] Support C++17 aggregates with bases without constructors.

[Alexander Kornienko via cfe-commits]

On Tue, Mar 19, 2019 at 6:24 PM Artem Dergachev  wrote:

> Hi,
>
> I'll try to fix this ASAP!
>

Thanks!

> It sounds as if i don't have nearly enough C++17 codebases for testing,
> are there any obvious open-source projects that you could recommend?


No, but this particular example comes from C++11 code that just has subtly
different semantics in C++17. I guess, just adding -std=c++17 on existing
code (LLVM, for example ;) could help uncover some of the issues. I
believe, we can also continue providing reports in case CSA fails
assertions on our code.


> I'd
> love to try to reincarnate
>
> http://green.lab.llvm.org/green/view/Experimental/job/StaticAnalyzerBenchmarks/
> with those.
>
> On 3/19/19 9:37 AM, Alexander Kornienko wrote:
> > Filed this as https://bugs.llvm.org/show_bug.cgi?id=41142. Any hope
> > for a prompt fix here?
> >
> > On Tue, Mar 19, 2019 at 5:34 PM Alexander Kornienko  > > wrote:
> >
> > A reduced test case:
> > $ cat test-RegionStoreManager__bindStruct.cc
> > struct a {};
> > class b : a {};
> > b c() { b d{c()}; }
> > $ ./clang-tidy -checks="-*,clang-analyzer*"
> > test-RegionStoreManager__bindStruct.cc -- -std=c++17
> > assert.h assertion failed at
> > tools/clang/lib/StaticAnalyzer/Core/RegionStore.cpp:2362 in
> > (anonymous namespace)::RegionBindingsRef (anonym
> > ous
> > namespace)::RegionStoreManager::bindStruct(RegionBindingsConstRef,
> > const clang::ento::TypedValueRegion *, clang::ento::SVal):
> > CRD->isAggregate() && "Non-aggregates are constructed with a
> > constructor!"
> > @ 0x559908170326  __assert_fail
> > @ 0x5599068d4854  (anonymous
> > namespace)::RegionStoreManager::bindStruct()
> > @ 0x5599068c93c8  (anonymous
> > namespace)::RegionStoreManager::Bind()
> > @ 0x5599068b409f clang::ento::ProgramState::bindLoc()
> > @ 0x559906865935
> > clang::ento::ExprEngine::processPointerEscapedOnBind()
> > @ 0x55990685d4b3 clang::ento::ExprEngine::evalBind()
> > @ 0x559906872a43 clang::ento::ExprEngine::VisitDeclStmt()
> > @ 0x55990685c16f clang::ento::ExprEngine::Visit()
> > @ 0x559906858b1f clang::ento::ExprEngine::ProcessStmt()
> > @ 0x559906858808 clang::ento::ExprEngine::processCFGElement()
> > @ 0x55990684cb65 clang::ento::CoreEngine::HandlePostStmt()
> > @ 0x55990684bf5c clang::ento::CoreEngine::ExecuteWorkList()
> > @ 0x5599065b635b  (anonymous
> > namespace)::AnalysisConsumer::HandleCode()
> > @ 0x5599065a0135  (anonymous
> > namespace)::AnalysisConsumer::HandleTranslationUnit()
> > @ 0x559906bb7cbc
> > clang::MultiplexConsumer::HandleTranslationUnit()
> > @ 0x559906d226d4  clang::ParseAST()
> > @ 0x559906b98a83 clang::FrontendAction::Execute()
> > @ 0x559906b31cd1 clang::CompilerInstance::ExecuteAction()
> > @ 0x559906a9cf61
> > clang::tooling::FrontendActionFactory::runInvocation()
> > @ 0x55990620cc07
> > clang::tidy::runClangTidy()::ActionFactory::runInvocation()
> > @ 0x559906a9ccca
> > clang::tooling::ToolInvocation::runInvocation()
> > @ 0x559906a9c646 clang::tooling::ToolInvocation::run()
> > @ 0x559906a9ef22 clang::tooling::ClangTool::run()
> > @ 0x559906207ecf  clang::tidy::runClangTidy()
> > @ 0x559902d47c45  main
> >
> > On Tue, Mar 19, 2019 at 1:14 AM Alexander Kornienko
> > mailto:ale...@google.com>> wrote:
> >
> > On Fri, Mar 15, 2019 at 1:21 AM Artem Dergachev via
> > cfe-commits  > > wrote:
> >
> > Author: dergachev
> > Date: Thu Mar 14 17:22:59 2019
> > New Revision: 356222
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=356222&view=rev
> > Log:
> > [analyzer] Support C++17 aggregates with bases without
> > constructors.
> >
> > RegionStore now knows how to bind a nonloc::CompoundVal
> > that represents the
> > value of an aggregate initializer when it has its initial
> > segment of sub-values
> > correspond to base classes.
> >
> > Additionally, fixes the crash from pr40022.
> >
> > Differential Revision: https://reviews.llvm.org/D59054
> >
> > Modified:
> > cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
> > cfe/trunk/test/Analysis/array-struct-region.cpp
> >
> > Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
> > URL:
> >
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=356222&r1=356221&r2=356222&view=diff
> >
>  
> 

[PATCH] D59376: [LibTooling] Add Transformer, a library for source-to-source transformations.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:54
+/// boolean expression language for constructing filters.
+class MatchFilter {
+public:

ilya-biryukov wrote:
> Intuitively, it feels that any filtering should be possible at the level of 
> the AST matchers. Is that not the case?
> Could you provide some motivating examples where AST matchers cannot be used 
> to nail down the matching nodes and we need `MatchFilter`? 
> 
> Please note I have limited experience with AST matchers, so there might be 
> some obvious things that I'm missing.
Good point. The examples I have would actually be perfectly suited to a 
matcher.  That said, there is not matcher support for a simple predicate of 
this form, along the lines of gtest's `Truly(predicate)`. I'll remove this and 
separately try to add something like `Truly` to the matchers.



Comment at: clang/include/clang/Tooling/Refactoring/Transformer.h:135
+// \endcode
+class RewriteRule {
+public:

ilya-biryukov wrote:
> Maybe consider separating the fluent API to build the rewrite rule from the 
> rewrite rule itself?
> 
> Not opposed to having the fluent builder API, this does look nice and seems 
> to nicely align with the matcher APIs.
> However, it is somewhat hard to figure out what can `RewriteRule` do 
> **after** it was built when looking at the code right now.
> ```
> class RewriteRule {
> public:
>   RewriteRule(DynTypedMatcher, TextGenerator Replacement, TextGenerator 
> Explanation);
> 
>   DynTypedMatcher matcher() const;
>   Expected replacement() const;
>   Expected explanation() const;
> };
> 
> struct RewriteRuleBuilder { // Having a better name than 'Builder' would be 
> nice.
>   RewriteRule finish() &&; // produce the final RewriteRule.
> 
>   template 
>   RewriteRuleBuilder &change(const TypedNodeId &Target,
>   NodePart Part = NodePart::Node) &;
>   RewriteRuleBuilder &replaceWith(TextGenerator Replacement) &;
>   RewriteRuleBuilder &because(TextGenerator Explanation) &;
> private:
>   RewriteRule RuleInProgress;
> };
> RewriteRuleBuilder makeRewriteRule();
> ```
I see your point, but do you think it might be enough to improve the comments 
on the class? My concern with a builder is the mental burden on the user of 
another concept (the builder) and the need for an extra `.build()` everywhere. 
To a lesser extent, I also don't love the cost of an extra copy, although I 
doubt it matters and I suppose we could support moves in the build method.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59376



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

Re: r356222 - [analyzer] Support C++17 aggregates with bases without constructors.

[Richard Smith via cfe-commits]

On Tue, 19 Mar 2019 at 10:24, Artem Dergachev via cfe-commits
 wrote:
>
> Hi,
>
> I'll try to fix this ASAP!

It sounds like there might be a missing check for
InitListExpr::isTransparent somewhere. (A transparent InitListExpr
should be treated as equivalent to its one and only subexpression.)
Either that, or the static analyzer isn't aware that an object of
class type can be initialized directly from a function call, not via a
constructor.

> It sounds as if i don't have nearly enough C++17 codebases for testing,
> are there any obvious open-source projects that you could recommend? I'd
> love to try to reincarnate
> http://green.lab.llvm.org/green/view/Experimental/job/StaticAnalyzerBenchmarks/
> with those.
>
> On 3/19/19 9:37 AM, Alexander Kornienko wrote:
> > Filed this as https://bugs.llvm.org/show_bug.cgi?id=41142. Any hope
> > for a prompt fix here?
> >
> > On Tue, Mar 19, 2019 at 5:34 PM Alexander Kornienko  > > wrote:
> >
> > A reduced test case:
> > $ cat test-RegionStoreManager__bindStruct.cc
> > struct a {};
> > class b : a {};
> > b c() { b d{c()}; }
> > $ ./clang-tidy -checks="-*,clang-analyzer*"
> > test-RegionStoreManager__bindStruct.cc -- -std=c++17
> > assert.h assertion failed at
> > tools/clang/lib/StaticAnalyzer/Core/RegionStore.cpp:2362 in
> > (anonymous namespace)::RegionBindingsRef (anonym
> > ous
> > namespace)::RegionStoreManager::bindStruct(RegionBindingsConstRef,
> > const clang::ento::TypedValueRegion *, clang::ento::SVal):
> > CRD->isAggregate() && "Non-aggregates are constructed with a
> > constructor!"
> > @ 0x559908170326  __assert_fail
> > @ 0x5599068d4854  (anonymous
> > namespace)::RegionStoreManager::bindStruct()
> > @ 0x5599068c93c8  (anonymous
> > namespace)::RegionStoreManager::Bind()
> > @ 0x5599068b409f clang::ento::ProgramState::bindLoc()
> > @ 0x559906865935
> > clang::ento::ExprEngine::processPointerEscapedOnBind()
> > @ 0x55990685d4b3 clang::ento::ExprEngine::evalBind()
> > @ 0x559906872a43 clang::ento::ExprEngine::VisitDeclStmt()
> > @ 0x55990685c16f clang::ento::ExprEngine::Visit()
> > @ 0x559906858b1f clang::ento::ExprEngine::ProcessStmt()
> > @ 0x559906858808 clang::ento::ExprEngine::processCFGElement()
> > @ 0x55990684cb65 clang::ento::CoreEngine::HandlePostStmt()
> > @ 0x55990684bf5c clang::ento::CoreEngine::ExecuteWorkList()
> > @ 0x5599065b635b  (anonymous
> > namespace)::AnalysisConsumer::HandleCode()
> > @ 0x5599065a0135  (anonymous
> > namespace)::AnalysisConsumer::HandleTranslationUnit()
> > @ 0x559906bb7cbc
> > clang::MultiplexConsumer::HandleTranslationUnit()
> > @ 0x559906d226d4  clang::ParseAST()
> > @ 0x559906b98a83 clang::FrontendAction::Execute()
> > @ 0x559906b31cd1 clang::CompilerInstance::ExecuteAction()
> > @ 0x559906a9cf61
> > clang::tooling::FrontendActionFactory::runInvocation()
> > @ 0x55990620cc07
> > clang::tidy::runClangTidy()::ActionFactory::runInvocation()
> > @ 0x559906a9ccca
> > clang::tooling::ToolInvocation::runInvocation()
> > @ 0x559906a9c646 clang::tooling::ToolInvocation::run()
> > @ 0x559906a9ef22 clang::tooling::ClangTool::run()
> > @ 0x559906207ecf  clang::tidy::runClangTidy()
> > @ 0x559902d47c45  main
> >
> > On Tue, Mar 19, 2019 at 1:14 AM Alexander Kornienko
> > mailto:ale...@google.com>> wrote:
> >
> > On Fri, Mar 15, 2019 at 1:21 AM Artem Dergachev via
> > cfe-commits  > > wrote:
> >
> > Author: dergachev
> > Date: Thu Mar 14 17:22:59 2019
> > New Revision: 356222
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=356222&view=rev
> > Log:
> > [analyzer] Support C++17 aggregates with bases without
> > constructors.
> >
> > RegionStore now knows how to bind a nonloc::CompoundVal
> > that represents the
> > value of an aggregate initializer when it has its initial
> > segment of sub-values
> > correspond to base classes.
> >
> > Additionally, fixes the crash from pr40022.
> >
> > Differential Revision: https://reviews.llvm.org/D59054
> >
> > Modified:
> > cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
> > cfe/trunk/test/Analysis/array-struct-region.cpp
> >
> > Modified: cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp
> > URL:
> > 
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp?rev=356222&r1=356221&r2=356222&view=diff
> > 
> > =

[PATCH] D59388: Basic: Return a reference from FileManager::getVirtualFileSystem, NFC

[Jan Korous via Phabricator via cfe-commits]

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

In D59388#1433233 , @dexonsmith wrote:

> Yes, it's safe.  The reference count is "intrusive", meaning it's stored in 
> the object itself (via inheritance from `RefCountedBase`).  As a result, all 
> the instances of `IntrusiveRefCntPtr` that reference at the same object will 
> implicitly share their count.


I missed that `llvm::vfs::FileSystem` inherits from `ThreadSafeRefCountedBase`.

LGTM.


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

https://reviews.llvm.org/D59388



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

[PATCH] D58514: Avoid needlessly copying blocks that initialize or are assigned to local auto variables to the heap

[Hao Wu via Phabricator via cfe-commits]

wuhao5 added a comment.



> Okay, so really just a block self-reference.  We could really just add a 
> feature for that that would avoid both the complexity and the expense of the 
> self-capture dance.

Is there a plan to cover this case? or is it a legitimate use case that Clang 
should handle?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58514



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

r356496 - [OPENMP]Check that global vars require predefined allocator.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Mar 19 11:39:11 2019
New Revision: 356496

URL: http://llvm.org/viewvc/llvm-project?rev=356496&view=rev
Log:
[OPENMP]Check that global vars require predefined allocator.

According to OpenMP, 2.11.3 allocate Directive, Restrictions, C / C++,
if a list item has a static storage type, the allocator expression in
  the allocator clause must be a constant expression that evaluates to
  one of the predefined memory allocator values. Added check for this
  restriction.

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/allocate_allocator_messages.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=356496&r1=356495&r2=356496&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Mar 19 11:39:11 
2019
@@ -9141,6 +9141,11 @@ def err_omp_invalid_map_this_expr : Erro
   "invalid 'this' expression on 'map' clause">;
 def err_implied_omp_allocator_handle_t_not_found : Error<
   "omp_allocator_handle_t type not found; include ">;
+def err_omp_expected_predefined_allocator : Error<
+  "expected one of the predefined allocators for the variables with the static 
"
+  "storage: 'omp_default_mem_alloc', 'omp_large_cap_mem_alloc', "
+  "'omp_const_mem_alloc', 'omp_high_bw_mem_alloc', 'omp_low_lat_mem_alloc', "
+  "'omp_cgroup_mem_alloc', 'omp_pteam_mem_alloc' or 'omp_thread_mem_alloc'">;
 } // end of OpenMP category
 
 let CategoryName = "Related Result Type Issue" in {

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=356496&r1=356495&r2=356496&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Tue Mar 19 11:39:11 2019
@@ -2216,6 +2216,43 @@ Sema::DeclGroupPtrTy Sema::ActOnOpenMPAl
 if (isa(VD))
   continue;
 
+// OpenMP, 2.11.3 allocate Directive, Restrictions, C / C++
+// If a list item has a static storage type, the allocator expression in 
the
+// allocator clause must be a constant expression that evaluates to one of
+// the predefined memory allocator values.
+if (Allocator && VD->hasGlobalStorage()) {
+  bool IsPredefinedAllocator = false;
+  if (const auto *DRE =
+  dyn_cast(Allocator->IgnoreParenImpCasts())) {
+if (DRE->getType().isConstant(getASTContext())) {
+  DeclarationName DN = DRE->getDecl()->getDeclName();
+  if (DN.isIdentifier()) {
+StringRef PredefinedAllocators[] = {
+"omp_default_mem_alloc", "omp_large_cap_mem_alloc",
+"omp_const_mem_alloc",   "omp_high_bw_mem_alloc",
+"omp_low_lat_mem_alloc", "omp_cgroup_mem_alloc",
+"omp_pteam_mem_alloc",   "omp_thread_mem_alloc",
+};
+IsPredefinedAllocator =
+llvm::any_of(PredefinedAllocators, [&DN](StringRef S) {
+  return DN.getAsIdentifierInfo()->isStr(S);
+});
+  }
+}
+  }
+  if (!IsPredefinedAllocator) {
+Diag(Allocator->getExprLoc(),
+ diag::err_omp_expected_predefined_allocator)
+<< Allocator->getSourceRange();
+bool IsDecl = VD->isThisDeclarationADefinition(Context) ==
+  VarDecl::DeclarationOnly;
+Diag(VD->getLocation(),
+ IsDecl ? diag::note_previous_decl : diag::note_defined_here)
+<< VD;
+continue;
+  }
+}
+
 Vars.push_back(RefExpr);
 Attr *A = OMPAllocateDeclAttr::CreateImplicit(Context, Allocator,
   DE->getSourceRange());

Modified: cfe/trunk/test/OpenMP/allocate_allocator_messages.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/allocate_allocator_messages.cpp?rev=356496&r1=356495&r2=356496&view=diff
==
--- cfe/trunk/test/OpenMP/allocate_allocator_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/allocate_allocator_messages.cpp Tue Mar 19 11:39:11 
2019
@@ -20,8 +20,10 @@ struct St1{
  int a;
  static int b;
 #pragma omp allocate(b) allocator(sss) // expected-error {{initializing 
'omp_allocator_handle_t' (aka 'void *') with an expression of incompatible type 
'int'}}
-} d;
+} d; // expected-note 2 {{'d' defined here}}
 
+// expected-error@+1 {{expected one of the predefined allocators for the 
variables with the static storage: 'omp_default_mem_alloc', 
'omp_large_cap_mem_alloc', 'omp_const_mem_alloc', 'omp_high_bw_mem_alloc', 
'omp_low_lat_mem_alloc', 'omp_cgroup_mem_alloc', 'omp_p

r356497 - Fix unused variable warning. NFCI.

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Tue Mar 19 11:39:46 2019
New Revision: 356497

URL: http://llvm.org/viewvc/llvm-project?rev=356497&view=rev
Log:
Fix unused variable warning. NFCI.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=356497&r1=356496&r2=356497&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Tue Mar 19 11:39:46 2019
@@ -9744,7 +9744,7 @@ Address CGOpenMPRuntime::getAddressOfLoc
   llvm::Value *ThreadID = getThreadID(CGF, CVD->getBeginLoc());
   llvm::Value *Allocator;
   if (const Expr *AllocExpr = AA->getAllocator()) {
-Allocator = CGF.EmitScalarExpr(AA->getAllocator());
+Allocator = CGF.EmitScalarExpr(AllocExpr);
   } else {
 // Default allocator in libomp is nullptr.
 Allocator = llvm::ConstantPointerNull::get(CGM.VoidPtrPtrTy);


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

[PATCH] D57896: Variable names rule

[Chris Lattner via Phabricator via cfe-commits]

lattner added a comment.

In D57896#1434877 , @Charusso wrote:

> static Optional
>  getConcreteIntegerValue(const Expr *CondVarExpr, const ExplodedNode *N) {
>  //...
>
>   if (const auto *DRE = dyn_cast_or_null(CondVarExpr)) {
> if (const auto *VD = dyn_cast_or_null(DRE->getDecl())) {
>
> //...
>  }
>
>   would be:
>  
>
>
> static Optional 
>   |
>  getConcreteIntegerValue(const Expr *cond_var_expr, const ExplodedNode *node) 
> {  |
>  //...
>|
>
>   if (const auto *decl_ref_expr = 
> dyn_cast_or_null(cond_var_expr)) {
> if (const auto *var_decl = 
> dyn_cast_or_null(decl_ref_expr->getDecl())) {
>
> //... 
>   |
>  } whoops 
> column-81 ~^
>
>   Hungarian notation on members and globals are cool idea. However, the 
> notation is made without the `_` part, so I think `mMember` is better than 
> `m_member` as we used to 80-column standard and it is waste of space and 
> hurts your C-developer eyes. I would recommend `b` prefix to booleans as 
> Unreal Engine 4 styling is used to do that (`bIsCoolStyle`) and it is handy. 
> It is useful because booleans usually has multiple prefixes: `has, have, is` 
> and you would list all the booleans together in autocompletion. Yes, there is 
> a problem: if the notation is not capital like the pure Hungarian notation 
> then it is problematic to list and we are back to the `BIsCapitalLetter` and 
> `MMember` CamelCase-world where we started (except one project). I think 
> @lattner could say if it is useful as all the Apple projects based on those 
> notations and could be annoying.


FWIW, my suggestion is *not* to expand names like DRE to decl_ref_expr, I agree 
that doesn't add clarity to the code.  Two possibilities: "dre", or "decl" 
which is what I would write today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 191365.
MyDeveloperDay added a comment.

Address review comments
This may not be a more satisfactory solution  but it at least now covers the 
other cases raised by @JonasToth 
Add more tests around this area.


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

https://reviews.llvm.org/D59540

Files:
  clang-tidy/readability/IdentifierNamingCheck.cpp
  test/clang-tidy/readability-identifier-naming.cpp

Index: test/clang-tidy/readability-identifier-naming.cpp
===
--- test/clang-tidy/readability-identifier-naming.cpp
+++ test/clang-tidy/readability-identifier-naming.cpp
@@ -501,3 +501,159 @@
 // CHECK-FIXES: {{^}}int * const lc_PointerB = nullptr;{{$}}
 }
 
+
+bool LambdaCaptureTest() {
+  bool Columns=false;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'Columns'
+// CHECK-FIXES: {{^}}  bool columns=false;
+  auto ptr = [&]{
+return Columns;
+// CHECK-FIXES: {{^}}return columns;
+  }();
+
+  return Columns;
+// CHECK-FIXES: {{^}}  return columns;
+}
+
+bool LambdaCaptureTest1() {
+  bool Columns=false;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'Columns'
+// CHECK-FIXES: {{^}}  bool columns=false;
+  auto ptr = [=]{
+return Columns;
+  // CHECK-FIXES: {{^}}return columns;
+  }();
+
+  return Columns;
+// CHECK-FIXES: {{^}}  return columns;
+}
+
+bool LambdaCaptureTest2() {
+  bool Columns = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'Columns'
+// CHECK-FIXES: {{^}}  bool columns = false;
+  auto ptr = [&Columns]{
+// CHECK-FIXES: {{^}}  auto ptr = [&columns]{
+return Columns;
+// CHECK-FIXES: {{^}}return columns;
+  }();
+  return Columns;
+// CHECK-FIXES: {{^}}  return columns;
+}
+
+bool LambdaCaptureTest3() {
+  bool Columns = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'Columns'
+// CHECK-FIXES: {{^}}  bool columns = false;
+  auto ptr = [Columns]{
+// CHECK-FIXES: {{^}}  auto ptr = [columns]{
+return Columns;
+// CHECK-FIXES: {{^}}return columns;
+  }();
+  return Columns;
+// CHECK-FIXES: {{^}}  return columns;
+}
+
+bool LambdaCaptureTest4() {
+  bool Columns = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'Columns'
+// CHECK-FIXES: {{^}}  bool columns = false;
+  auto ptr = [&]{
+if (true)
+  return Columns;
+// CHECK-FIXES: {{^}}  return columns;
+  }();
+  return Columns;
+// CHECK-FIXES: {{^}}  return columns;
+}
+
+bool LambdaCaptureTest5() {
+  bool Columns = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'Columns'
+// CHECK-FIXES: {{^}}  bool columns = false;
+  auto ptr = [](bool Columns) {
+// CHECK-MESSAGES: :[[@LINE-1]]:22: warning: invalid case style for parameter 'Columns'
+// CHECK-FIXES: {{^}}  auto ptr = [](bool a_columns) {
+return Columns;
+// CHECK-FIXES: {{^}}return a_columns;
+  };
+  return Columns;
+// CHECK-FIXES: {{^}}  return columns;
+}
+
+bool LambdaCaptureTest6() {
+  bool Columns = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'Columns'
+// CHECK-FIXES: {{^}}  bool columns = false;
+  auto ptr = [Columns]() ->bool {
+// CHECK-FIXES: {{^}}  auto ptr = [columns]() ->bool {
+  [=]() -> bool {
+  return Columns;
+// CHECK-FIXES: {{^}}  return columns;
+};
+return Columns;
+// CHECK-FIXES: {{^}}return columns;
+  };
+  return Columns;
+// CHECK-FIXES: {{^}}  return columns;
+}
+
+bool LambdaCaptureTest7() {
+  int  a;
+  bool Columns = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'Columns'
+// CHECK-FIXES: {{^}}  bool columns = false;
+  auto ptr = [&Columns, a]{
+// CHECK-FIXES: {{^}}  auto ptr = [&columns, a]{
+return Columns;
+// CHECK-FIXES: {{^}}return columns;
+  }();
+  return Columns;
+// CHECK-FIXES: {{^}}  return columns;
+}
+
+bool LambdaCaptureTest8() {
+  int a;
+  bool Columns = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'Columns'
+// CHECK-FIXES: {{^}}  bool columns = false;
+  auto ptr = [a, Columns]{
+// CHECK-FIXES: {{^}}  auto ptr = [a, columns]{
+return Columns;
+// CHECK-FIXES: {{^}}return columns;
+  }();
+  return Columns;
+// CHECK-FIXES: {{^}}  return columns;
+}
+
+bool LambdaCaptureTest9() {
+  int Rows;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for local variable 'Rows'
+// CHECK-FIXES: {{^}}  int rows;
+  bool Columns = false;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local variable 'Columns'
+// CHECK-FIXES: {{^}}  bool columns = false;
+  auto ptr = [Rows, Columns]{
+// CHECK-FIXES: {{^}}  auto ptr = [rows, columns]{
+return Columns;
+// CHECK-FIXES: {{^}}return columns;
+  }();
+

[PATCH] D59540: [clang-tidy] [PR41119] readability-identifier-naming incorrectly fixes lambda capture

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay marked 5 inline comments as done.
MyDeveloperDay added inline comments.



Comment at: test/clang-tidy/readability-identifier-naming.cpp:509
+// CHECK-FIXES: {{^}}  bool columns=false;
+  auto ptr=[&]{return Columns;}();
+// CHECK-FIXES: {{^}}  auto ptr=[&]{return columns;}();

alexfh wrote:
> Please add more tests with
>   1) by value automatic captures
>   2) manual captures by value
>   3) manual captures by reference
>   4) nested lambdas capturing the same variable
> 
> A bit more nested code inside the lambda would also be interesting (where the 
> use of the variable would be wrapped in a couple of compound statements).
Think i've covered the cases you wanted here, if you can think of another drop 
the code into a comment and I'll add it.


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

https://reviews.llvm.org/D59540



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

[PATCH] D57896: Variable names rule

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

In D57896#1435245 , @lattner wrote:

> FWIW, my suggestion is *not* to expand names like DRE to decl_ref_expr, I 
> agree that doesn't add clarity to the code.  Two possibilities: "dre", or 
> "decl" which is what I would write today.


I totally agree with you, wherever I can I write that out for clarification. 
Please note that in my example there is two `Expr` and that is why I pointed 
out we need acronyms so we cannot really use `expr` and acronyms usually 
capital, that is why we went back to the default CamelCase standard. It was a 
little brainstorming and ping for you guys because I believe you would put 
those polls out and create a better code-base.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57896



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

[PATCH] D57965: Clean up ObjCPropertyDecl printing

[David Goldman via Phabricator via cfe-commits]

dgoldman added a comment.

friendly ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D57965



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

[PATCH] D56924: Special case ObjCPropertyDecl for printing

[David Goldman via Phabricator via cfe-commits]

dgoldman added a comment.

ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D56924



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

[PATCH] D59492: [OpenCL] Allow variadic macros as Clang feature

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D59492#1434822 , @arsenm wrote:

> In D59492#1434636 , @Anastasia wrote:
>
> > In D59492#1433796 , @arsenm wrote:
> >
> > > Should it be downgraded to a warning about an extension instead of just 
> > > removing it?
> >
> >
> > What would you suggest to put in a warning message? Clang normally doesn't 
> > warn about extensions...
>
>
> Isn't that what -pedantic is for?


Yes, indeed I think it's a better approach now since it's still in the spec...


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

https://reviews.llvm.org/D59492



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

[PATCH] D59492: [OpenCL] Allow variadic macros as Clang feature

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 191366.
Anastasia added a comment.

Instead of removing the diagnostic completely change into a warning in pedantic 
mode.


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

https://reviews.llvm.org/D59492

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Lex/PPDirectives.cpp
  test/Misc/warning-flags.c
  test/Preprocessor/macro_variadic.cl


Index: test/Preprocessor/macro_variadic.cl
===
--- test/Preprocessor/macro_variadic.cl
+++ test/Preprocessor/macro_variadic.cl
@@ -1,3 +1,20 @@
-// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify %s -cl-std=CL1.2
+// RUN: %clang_cc1 -verify %s -pedantic -DPEDANTIC -cl-std=CL1.2
 
-#define X(...) 1 // expected-error {{variadic macros not supported in OpenCL}}
+
+#define NO_VAR_FUNC(...)  5
+#define VAR_FUNC(...) func(__VA_ARGS__);
+#define VAR_PRINTF(str, ...) printf(str, __VA_ARGS__);
+#ifdef PEDANTIC
+// expected-warning@-4{{variadic macros not supported in OpenCL}}
+// expected-warning@-4{{variadic macros not supported in OpenCL}}
+// expected-warning@-4{{variadic macros not supported in OpenCL}}
+#endif
+
+int printf(__constant const char *st, ...);
+
+void foo() {
+  NO_VAR_FUNC(1, 2, 3);
+  VAR_FUNC(1, 2, 3); //expected-error{{implicit declaration of function 'func' 
is invalid in OpenCL}}
+  VAR_PRINTF("%i", 1);
+}
Index: test/Misc/warning-flags.c
===
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -96,4 +96,4 @@
 
 The list of warnings in -Wpedantic should NEVER grow.
 
-CHECK: Number in -Wpedantic (not covered by other -W flags): 27
+CHECK: Number in -Wpedantic (not covered by other -W flags): 28
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -2235,8 +2235,7 @@
 
   // OpenCL v1.2 s6.9.e: variadic macros are not supported.
   if (LangOpts.OpenCL) {
-Diag(Tok, diag::err_pp_opencl_variadic_macros);
-return true;
+Diag(Tok, diag::ext_pp_opencl_variadic_macros);
   }
 
   // Lex the token after the identifier.
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -393,8 +393,8 @@
 def note_macro_here : Note<"macro %0 defined here">;
 def note_macro_expansion_here : Note<"expansion of macro %0 requested here">;
 
-def err_pp_opencl_variadic_macros :
-  Error<"variadic macros not supported in OpenCL">;
+def ext_pp_opencl_variadic_macros : Extension<
+  "variadic macros not supported in OpenCL">;
 
 def err_pp_invalid_directive : Error<"invalid preprocessing directive">;
 def err_pp_directive_required : Error<


Index: test/Preprocessor/macro_variadic.cl
===
--- test/Preprocessor/macro_variadic.cl
+++ test/Preprocessor/macro_variadic.cl
@@ -1,3 +1,20 @@
-// RUN: %clang_cc1 -verify %s
+// RUN: %clang_cc1 -verify %s -cl-std=CL1.2
+// RUN: %clang_cc1 -verify %s -pedantic -DPEDANTIC -cl-std=CL1.2
 
-#define X(...) 1 // expected-error {{variadic macros not supported in OpenCL}}
+
+#define NO_VAR_FUNC(...)  5
+#define VAR_FUNC(...) func(__VA_ARGS__);
+#define VAR_PRINTF(str, ...) printf(str, __VA_ARGS__);
+#ifdef PEDANTIC
+// expected-warning@-4{{variadic macros not supported in OpenCL}}
+// expected-warning@-4{{variadic macros not supported in OpenCL}}
+// expected-warning@-4{{variadic macros not supported in OpenCL}}
+#endif
+
+int printf(__constant const char *st, ...);
+
+void foo() {
+  NO_VAR_FUNC(1, 2, 3);
+  VAR_FUNC(1, 2, 3); //expected-error{{implicit declaration of function 'func' is invalid in OpenCL}}
+  VAR_PRINTF("%i", 1);
+}
Index: test/Misc/warning-flags.c
===
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -96,4 +96,4 @@
 
 The list of warnings in -Wpedantic should NEVER grow.
 
-CHECK: Number in -Wpedantic (not covered by other -W flags): 27
+CHECK: Number in -Wpedantic (not covered by other -W flags): 28
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -2235,8 +2235,7 @@
 
   // OpenCL v1.2 s6.9.e: variadic macros are not supported.
   if (LangOpts.OpenCL) {
-Diag(Tok, diag::err_pp_opencl_variadic_macros);
-return true;
+Diag(Tok, diag::ext_pp_opencl_variadic_macros);
   }
 
   // Lex the token after the identifier.
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -393,8 +393,8 @@
 def n

[PATCH] D58514: Avoid needlessly copying blocks that initialize or are assigned to local auto variables to the heap

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D58514#1435228 , @wuhao5 wrote:

> > Okay, so really just a block self-reference.  We could really just add a 
> > feature for that that would avoid both the complexity and the expense of 
> > the self-capture dance.
>
> Is there a plan to cover this case? or is it a legitimate use case that Clang 
> should handle?


You are currently relying on something that ARC doesn't guarantee, so the 
client code should be fixed to explicitly copy the block.  I think we would be 
happy to consider a proposal in the long run to allow blocks to self-reference 
more easily, which will effectively bypass the problem.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58514



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

[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

[Borsik Gábor via Phabricator via cfe-commits]

boga95 created this revision.
boga95 added reviewers: Szelethus, xazax.hun, dkrupp, NoQ.
Herald added subscribers: cfe-commits, Charusso, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet, baloghadamsoftware, whisperity.
Herald added a project: clang.

Parse the yaml configuration file and store it in static variables. The user 
can define taint propagation rules, custom sink, and filter functions. E.g:

  # A list of source/propagation function
  Propagations:
# int x = mySource1(); // x is tainted
- Name: mySource1
  DstArgs:  [4294967294] # Index for return value
  
# int x;
# mySource2(&x); // x is tainted
- Name: mySource2
  DstArgs:  [0]
  
# int x, y;
# myScanf("%d %d", &x, &y); // x and y are tainted
- Name: myScanf
  VarType:  Dst
  VarIndex: 1
  
# int x; // x is tainted
# int y;
# myPropagator(x, &y); // y is tainted
- Name: myPropagator
  SrcArgs:  [0]
  DstArgs:  [1]
  
# const unsigned size = 100;
# char buf[size];
# int x, y;
# int n = mySprintf(buf, size, "%d %d", x, y); // If size, x or y is tainted
# // the return value and the buf will be tainted
- Name: mySnprintf
  SrcArgs:  [1]
  DstArgs:  [0, 4294967294]
  VarType:  Src
  VarIndex: 3
  
  # A list of filter functions
  Filters:
# int x; // x is tainted
# myFilter(&x); // x is not tainted anymore
- Name: myFilter
  Args: [0]
  
  # A list of sink functions
  Sinks:
# int x, y; // x and y are tainted
# mySink(x, 0, 1); // It will warn
# mySink(0, 1, y); // It will warn
# mySink(0, x, 1); // It won't warn
- Name: mySink
  Args: [0, 2]


Repository:
  rC Clang

https://reviews.llvm.org/D59555

Files:
  lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp

Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -13,14 +13,16 @@
 // aggressively, even if the involved symbols are under constrained.
 //
 //===--===//
-#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/AST/Attr.h"
 #include "clang/Basic/Builtins.h"
+#include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/CheckerManager.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/ProgramStateTrait.h"
+#include "llvm/ADT/StringMap.h"
+#include "llvm/Support/YAMLTraits.h"
 #include 
 #include 
 #include 
@@ -41,11 +43,38 @@
 
   void checkPreStmt(const CallExpr *CE, CheckerContext &C) const;
 
-private:
+  using ArgVector = SmallVector;
+
+  enum class VariadicType { None, Src, Dst };
+
+  /// The ``TaintConfiguration`` is used to parse configuration file.
+  struct TaintConfiguration {
+using NameArgsPair = std::pair;
+
+struct Propagation {
+  std::string Name;
+  ArgVector SrcArgs;
+  ArgVector DstArgs;
+  VariadicType VarType;
+  unsigned VarIndex;
+};
+
+std::vector Propagations;
+std::vector Filters;
+std::vector Sinks;
+  };
+
+  /// Get and read the config file.
+  static void getConfiguration(StringRef ConfigFile);
+
+  /// Parse the config.
+  static void parseConfiguration(TaintConfiguration &&Config);
+
   static const unsigned InvalidArgIndex = UINT_MAX;
   /// Denotes the return vale.
   static const unsigned ReturnValueIndex = UINT_MAX - 1;
 
+private:
   mutable std::unique_ptr BT;
   void initBugType() const {
 if (!BT)
@@ -91,8 +120,6 @@
   bool generateReportIfTainted(const Expr *E, const char Msg[],
CheckerContext &C) const;
 
-  using ArgVector = SmallVector;
-
   /// A struct used to specify taint propagation rules for a function.
   ///
   /// If any of the possible taint source arguments is tainted, all of the
@@ -103,8 +130,6 @@
   /// ReturnValueIndex is added to the dst list, the return value will be
   /// tainted.
   struct TaintPropagationRule {
-enum class VariadicType { None, Src, Dst };
-
 using PropagationFuncType = bool (*)(bool IsTainted, const CallExpr *,
  CheckerContext &C);
 
@@ -125,8 +150,7 @@
 : VariadicIndex(InvalidArgIndex), VarType(VariadicType::None),
   PropagationFunc(nullptr) {}
 
-TaintPropagationRule(std::initializer_list &&Src,
- std::initializer_list &&Dst,
+TaintPropagationRule(ArgVector &&Src, ArgVector &&Dst,
  VariadicType Var = VariadicType::None,
  unsigned VarIndex = InvalidArgIndex,
  PropagationFu

r356507 - Move options to separate checks that do not need to immediately follow the previous option. NFCI

[Douglas Yung via cfe-commits]

Author: dyung
Date: Tue Mar 19 12:34:15 2019
New Revision: 356507

URL: http://llvm.org/viewvc/llvm-project?rev=356507&view=rev
Log:
Move options to separate checks that do not need to immediately follow the 
previous option. NFCI

Modified:
cfe/trunk/test/Driver/hip-toolchain-mllvm.hip

Modified: cfe/trunk/test/Driver/hip-toolchain-mllvm.hip
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hip-toolchain-mllvm.hip?rev=356507&r1=356506&r2=356507&view=diff
==
--- cfe/trunk/test/Driver/hip-toolchain-mllvm.hip (original)
+++ cfe/trunk/test/Driver/hip-toolchain-mllvm.hip Tue Mar 19 12:34:15 2019
@@ -8,7 +8,8 @@
 // RUN:   %s 2>&1 | FileCheck %s
 
 // CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
-// CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu" "-emit-llvm-bc"
+// CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
+// CHECK-SAME: "-emit-llvm-bc"
 // CHECK-SAME: {{.*}} "-target-cpu" "gfx803"
 // CHECK-SAME: {{.*}} "-mllvm" "-amdgpu-function-calls=0" {{.*}}
 
@@ -22,7 +23,8 @@
 // CHECK-SAME: "-amdgpu-function-calls=0" "-o" {{".*-gfx803-.*o"}}
 
 // CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
-// CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu" "-emit-llvm-bc"
+// CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu"
+// CHECK-SAME: "-emit-llvm-bc"
 // CHECK-SAME: {{.*}} "-target-cpu" "gfx900"
 // CHECK-SAME: {{.*}} "-mllvm" "-amdgpu-function-calls=0" {{.*}}
 


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