[PATCH] D59682: [X86] Add BSR/BSF/BSWAP intrinsics to ia32intrin.h to match gcc.

[Craig Topper via Phabricator via cfe-commits]

craig.topper created this revision.
craig.topper added reviewers: RKSimon, spatel.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

These are all implemented by icc as well.

I made bit_scan_forward/reverse forward to the __bsfd/__bsrq since we also have
__bsfq/__bsrq.

Note, when lzcnt is enabled the bsr intrinsics generates lzcnt+xor instead of 
bsr.


Repository:
  rC Clang

https://reviews.llvm.org/D59682

Files:
  lib/Headers/ia32intrin.h
  lib/Headers/immintrin.h
  test/CodeGen/bitscan-builtins.c
  test/CodeGen/x86-bswap.c

Index: test/CodeGen/x86-bswap.c
===
--- /dev/null
+++ test/CodeGen/x86-bswap.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-apple-darwin -emit-llvm -o - | FileCheck %s
+
+#include 
+
+int test__bswapd(int X) {
+// CHECK-LABEL: @test__bswapd
+// CHECK: call i32 @llvm.bswap.i32
+  return __bswapd(X);
+}
+
+int test_bswap(int X) {
+// CHECK-LABEL: @test_bswap
+// CHECK: call i32 @llvm.bswap.i32
+  return _bswap(X);
+}
+
+long test__bswapq(long long X) {
+// CHECK-LABEL: @test__bswapq
+// CHECK: call i64 @llvm.bswap.i64
+  return __bswapq(X);
+}
+
+long test_bswap64(long long X) {
+// CHECK-LABEL: @test_bswap64
+// CHECK: call i64 @llvm.bswap.i64
+  return _bswap64(X);
+}
+
+
Index: test/CodeGen/bitscan-builtins.c
===
--- test/CodeGen/bitscan-builtins.c
+++ test/CodeGen/bitscan-builtins.c
@@ -3,7 +3,7 @@
 // PR33722
 // RUN: %clang_cc1 -ffreestanding -triple x86_64-unknown-unknown -fms-extensions -fms-compatibility-version=19.00 -emit-llvm -o - %s | FileCheck %s
 
-#include 
+#include 
 
 int test_bit_scan_forward(int a) {
   return _bit_scan_forward(a);
@@ -18,3 +18,30 @@
 // CHECK:  %[[sub:.*]] = sub nsw i32 31, %[[call]]
 // CHECK: ret i32 %[[sub]]
 }
+
+int test__bsfd(int X) {
+// CHECK: @test__bsfd
+// CHECK: %[[call:.*]] = call i32 @llvm.cttz.i32(
+  return __bsfd(X);
+}
+
+int test__bsfq(long long X) {
+// CHECK: @test__bsfq
+// CHECK: %[[call:.*]] = call i64 @llvm.cttz.i64(
+  return __bsfq(X);
+}
+
+int test__bsrd(int X) {
+// CHECK: @test__bsrd
+// CHECK:  %[[call:.*]] = call i32 @llvm.ctlz.i32(
+// CHECK:  %[[sub:.*]] = sub nsw i32 31, %[[call]]
+  return __bsrd(X);
+}
+
+int test__bsrq(long long X) {
+// CHECK: @test__bsrq
+// CHECK:  %[[call:.*]] = call i64 @llvm.ctlz.i64(
+// CHECK:  %[[cast:.*]] = trunc i64 %[[call]] to i32
+// CHECK:  %[[sub:.*]] = sub nsw i32 63, %[[cast]]
+  return __bsrq(X);
+}
Index: lib/Headers/immintrin.h
===
--- lib/Headers/immintrin.h
+++ lib/Headers/immintrin.h
@@ -241,18 +241,6 @@
 #endif
 #endif /* __RDRND__ */
 
-/* __bit_scan_forward */
-static __inline__ int __attribute__((__always_inline__, __nodebug__))
-_bit_scan_forward(int __A) {
-  return __builtin_ctz(__A);
-}
-
-/* __bit_scan_reverse */
-static __inline__ int __attribute__((__always_inline__, __nodebug__))
-_bit_scan_reverse(int __A) {
-  return 31 - __builtin_clz(__A);
-}
-
 #if !defined(_MSC_VER) || __has_feature(modules) || defined(__FSGSBASE__)
 #ifdef __x86_64__
 static __inline__ unsigned int __attribute__((__always_inline__, __nodebug__, __target__("fsgsbase")))
Index: lib/Headers/ia32intrin.h
===
--- lib/Headers/ia32intrin.h
+++ lib/Headers/ia32intrin.h
@@ -28,6 +28,44 @@
 #ifndef __IA32INTRIN_H
 #define __IA32INTRIN_H
 
+static __inline__ int __attribute__((__always_inline__, __nodebug__))
+__bsfd(int __A) {
+  return __builtin_ctz(__A);
+}
+
+static __inline__ int __attribute__((__always_inline__, __nodebug__))
+__bsrd(int __A) {
+  return 31 - __builtin_clz(__A);
+}
+
+static __inline__ int __attribute__((__always_inline__, __nodebug__))
+__bswapd(int __A) {
+  return __builtin_bswap32(__A);
+}
+
+#define _bswap(A) __bswapd((A))
+#define _bit_scan_forward(A) __bsfd((A))
+#define _bit_scan_reverse(A) __bsrd((A))
+
+#ifdef __x86_64__
+static __inline__ int __attribute__((__always_inline__, __nodebug__))
+__bsfq(long long __A) {
+  return __builtin_ctzll(__A);
+}
+
+static __inline__ int __attribute__((__always_inline__, __nodebug__))
+__bsrq(long long __A) {
+  return 63 - __builtin_clzll(__A);
+}
+
+static __inline__ long long __attribute__((__always_inline__, __nodebug__))
+__bswapq(long long __A) {
+  return __builtin_bswap64(__A);
+}
+
+#define _bswap64(A) __bswapq((A))
+#endif
+
 /** Counts the number of bits in the source operand having a value of 1.
  *
  *  \headerfile 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59279: [Analyzer] Checker for non-determinism caused by iteration of unordered container of pointers

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

Szelethus added a comment.

In D59279#1438514 , @mgrang wrote:

> Although, as you rightly pointed out that //ordered// sets of pointers are as 
> non-deterministic as //unordered// ones.


What if you store pointers to the elements of an array? In that case, it should 
be deterministic.

  ---
  |1|2|3|5|6|
  ---
   ^ ^ 
   p q

In the above example, pointer `p`'s memory address will always be less then 
`q`'s, so in an ordered set, iterating over them would be deterministic. When 
hashing algorithms come into the picture, than it wouldn't be :^)


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

https://reviews.llvm.org/D59279



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

[PATCH] D17407: [Sema] PR25755 Fix crash when initializing out-of-order struct references

[Don Hinton via Phabricator via cfe-commits]

hintonda updated this revision to Diff 191828.
hintonda added a comment.
Herald added a subscriber: jdoerfert.
Herald added a project: clang.

- Initial checkin from original D17407 .
- Refactored code and simplify tests based on review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17407

Files:
  clang/lib/Sema/SemaInit.cpp
  clang/test/SemaCXX/cxx0x-initializer-constructor.cpp


Index: clang/test/SemaCXX/cxx0x-initializer-constructor.cpp
===
--- clang/test/SemaCXX/cxx0x-initializer-constructor.cpp
+++ clang/test/SemaCXX/cxx0x-initializer-constructor.cpp
@@ -409,3 +409,21 @@
 [0] = 1, [2] = 3
   }; // expected-error {{ambiguous}} expected-note {{in implicit 
initialization of array element 1}}
 }
+
+namespace PR23514 {
+struct Q {  // expected-note-re 4{{candidate constructor (the implicit 
{{(copy|move)}} constructor) not viable: requires 1 argument, but 0 were 
provided}}
+  Q(int) {} // expected-note 2{{candidate constructor not viable: requires 1 
argument, but 0 were provided}}
+};
+struct A {
+  Q x, y; // expected-note 2{{in implicit initialization of field 'x' with 
omitted initializer}}
+};
+struct B {
+  int i;
+  A a;
+  int k;
+};
+B w = {.a.y = 0, 0, .a.x = 10};
+B x = {.a.y = 0, 0}; // expected-error {{no matching constructor for 
initialization of 'PR23514::Q'}}
+B y = {.a = {.y = 0, .x = 0}, 0};
+B z = {.a = {.y = 0}, 0}; // expected-error {{no matching constructor for 
initialization of 'PR23514::Q'}}
+} // namespace PR23514
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -2000,6 +2000,9 @@
 !IList->isIdiomaticZeroInitializer(SemaRef.getLangOpts());
   bool HasDesignatedInit = false;
 
+  unsigned FieldSize = std::distance(RD->field_begin(), RD->field_end());
+  llvm::BitVector SeenFields(FieldSize);
+
   while (Index < IList->getNumInits()) {
 Expr *Init = IList->getInit(Index);
 SourceLocation InitLoc = Init->getBeginLoc();
@@ -2032,6 +2035,10 @@
 }
   }
 
+  if (!hadError)
+SeenFields.set(Field == FieldEnd ? FieldSize - 1
+ : Field->getFieldIndex() - 1);
+
   InitializedSomething = true;
 
   // Disable check for missing fields when designators are used.
@@ -2045,6 +2052,8 @@
   break;
 }
 
+SeenFields.set(Field->getFieldIndex());
+
 // We've already initialized a member of a union. We're done.
 if (InitializedSomething && DeclType->isUnionType())
   break;
@@ -2111,12 +2120,15 @@
 }
   }
 
-  // Check that any remaining fields can be value-initialized.
-  if (VerifyOnly && Field != FieldEnd && !DeclType->isUnionType() &&
-  !Field->getType()->isIncompleteArrayType()) {
-// FIXME: Should check for holes left by designated initializers too.
-for (; Field != FieldEnd && !hadError; ++Field) {
-  if (!Field->isUnnamedBitfield() && !Field->hasInClassInitializer())
+  // Check that any unseen fields can be value-initialized.
+  if (VerifyOnly && TopLevelObject && !DeclType->isUnionType() &&
+  (Field != FieldEnd || HasDesignatedInit)) {
+Field = RD->field_begin();
+for (unsigned i = 0; i < FieldSize && !hadError; ++i, ++Field) {
+  if (SeenFields[i])
+continue;
+  if(!Field->isUnnamedBitfield() && !Field->hasInClassInitializer() &&
+  !Field->getType()->isIncompleteArrayType())
 CheckEmptyInitializable(
 InitializedEntity::InitializeMember(*Field, &Entity),
 IList->getEndLoc());


Index: clang/test/SemaCXX/cxx0x-initializer-constructor.cpp
===
--- clang/test/SemaCXX/cxx0x-initializer-constructor.cpp
+++ clang/test/SemaCXX/cxx0x-initializer-constructor.cpp
@@ -409,3 +409,21 @@
 [0] = 1, [2] = 3
   }; // expected-error {{ambiguous}} expected-note {{in implicit initialization of array element 1}}
 }
+
+namespace PR23514 {
+struct Q {  // expected-note-re 4{{candidate constructor (the implicit {{(copy|move)}} constructor) not viable: requires 1 argument, but 0 were provided}}
+  Q(int) {} // expected-note 2{{candidate constructor not viable: requires 1 argument, but 0 were provided}}
+};
+struct A {
+  Q x, y; // expected-note 2{{in implicit initialization of field 'x' with omitted initializer}}
+};
+struct B {
+  int i;
+  A a;
+  int k;
+};
+B w = {.a.y = 0, 0, .a.x = 10};
+B x = {.a.y = 0, 0}; // expected-error {{no matching constructor for initialization of 'PR23514::Q'}}
+B y = {.a = {.y = 0, .x = 0}, 0};
+B z = {.a = {.y = 0}, 0}; // expected-error {{no matching constructor for initialization of 'PR23514::Q'}}
+} // namespace PR23514
Index: clang/lib/Sema/SemaInit.cpp
=

[PATCH] D17407: [Sema] PR25755 Handle out of order designated initializers

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

Please check also https://bugs.llvm.org/show_bug.cgi?id=40030


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17407



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

[PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

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

balazske added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:7738
+Expected ASTImporter::ImportInternal(Decl *FromD) {
+  // Import the declaration.
+  ASTNodeImporter Importer(*this);

Comment can be better: Import it using ASTNodeImporter.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:581
+struct RedirectingImporter : public ASTImporter {
+  using ASTImporter::ASTImporter;
+  // Returns a ImporterConstructor that constructs this class.

Is this `using` needed?



Comment at: clang/unittests/AST/ASTImporterTest.cpp:583
+  // Returns a ImporterConstructor that constructs this class.
+  static ASTImporterOptionSpecificTestBase::ImporterConstructor
+  getConstructor() {

Is it possible to make this a static variable instead of function?



Comment at: clang/unittests/AST/ASTImporterTest.cpp:588
+  bool MinimalImport, ASTImporterLookupTable *LookupTabl) {
+  return static_cast(
+  new RedirectingImporter(ToContext, ToFileManager, FromContext,

The `static_cast` should not be needed.


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] D17407: [Sema] PR25755 Handle out of order designated initializers

[Don Hinton via Phabricator via cfe-commits]

hintonda added a comment.

In D17407#1439029 , @xbolva00 wrote:

> Please check also https://bugs.llvm.org/show_bug.cgi?id=40030


Thanks for pointing that out.  I was mainly handling asserts, but will add the 
restrictions as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17407



___
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 added inline comments.



Comment at: clangd/Diagnostics.cpp:396
+  for (llvm::StringRef Inc : IncludeStack)
+OS << "In file included from: " << Inc << ":\n";
+}

ilya-biryukov wrote:
> NIT: could we reuse the function from clang that does the same thing?
> 
> The code here is pretty simple, though, so writing it here is fine. Just 
> wanted to make sure we considered this option and found it's easier to redo 
> this work ourselves.
there is `TextDiagnostic` which prints the desired output expect the fact that 
it also prints the first line saying the header was included in main file. 
Therefore, I didn't make use of it since we decided that was not very useful 
for our use case. And it requires inheriting from `DiagnosticRenderer` which 
was requiring too much boiler plate code just to get this functionality so 
instead I've chosen doing it like this.

Can fallback to `TextDiagnostic` if you believe showing `Included in 
mainfile.cc:x:y:` at the beginning of the diagnostic won't be annoying.



Comment at: unittests/clangd/DiagnosticsTests.cpp:779
 } // namespace clangd
 } // namespace clang

ilya-biryukov wrote:
> Could we add a test for reaching the limit?
> Could we mention that some diagnostics were dropped in that case? Something 
> like:
> ```
> In file included from a.h:20:1:
> error: unresolved identifier 'foo'.
> 
> And 10 more diagnostics...
> ```
There is already one, see `LimitDiagsOutsideMainFile`


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] D59302: [clangd] Surface diagnostics from headers inside main file

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 191831.
kadircet marked 8 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59302

Files:
  clangd/ClangdUnit.cpp
  clangd/Diagnostics.cpp
  clangd/Diagnostics.h
  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,183 @@
   "Add include \"x.h\" for symbol a::X");
 }
 
+ParsedAST build(const std::string &MainFile,
+const llvm::StringMap &Files) {
+  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();
+  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"}};
+  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-

[PATCH] D59683: [Tooling] Avoid working-dir races in AllTUsToolExecutor

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: ioeric.
Herald added a subscriber: jdoerfert.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D59683

Files:
  clang/lib/Tooling/AllTUsExecution.cpp


Index: clang/lib/Tooling/AllTUsExecution.cpp
===
--- clang/lib/Tooling/AllTUsExecution.cpp
+++ clang/lib/Tooling/AllTUsExecution.cpp
@@ -9,6 +9,7 @@
 #include "clang/Tooling/AllTUsExecution.h"
 #include "clang/Tooling/ToolExecutorPluginRegistry.h"
 #include "llvm/Support/ThreadPool.h"
+#include "llvm/Support/VirtualFileSystem.h"
 
 namespace clang {
 namespace tooling {
@@ -114,25 +115,22 @@
   {
 llvm::ThreadPool Pool(ThreadCount == 0 ? llvm::hardware_concurrency()
: ThreadCount);
-llvm::SmallString<128> InitialWorkingDir;
-if (auto EC = llvm::sys::fs::current_path(InitialWorkingDir)) {
-  InitialWorkingDir = "";
-  llvm::errs() << "Error while getting current working directory: "
-   << EC.message() << "\n";
-}
 for (std::string File : Files) {
   Pool.async(
   [&](std::string Path) {
 Log("[" + std::to_string(Count()) + "/" + TotalNumStr +
 "] Processing file " + Path);
-ClangTool Tool(Compilations, {Path});
+// Each thread gets an indepent copy of a VFS to allow different
+// concurrent working directories.
+IntrusiveRefCntPtr FS =
+llvm::vfs::createPhysicalFileSystem().release();
+ClangTool Tool(Compilations, {Path},
+   std::make_shared(), FS);
 Tool.appendArgumentsAdjuster(Action.second);
 Tool.appendArgumentsAdjuster(getDefaultArgumentsAdjusters());
 for (const auto &FileAndContent : OverlayFiles)
   Tool.mapVirtualFile(FileAndContent.first(),
   FileAndContent.second);
-// Do not restore working dir from multiple threads to avoid races.
-Tool.setRestoreWorkingDir(false);
 if (Tool.run(Action.first.get()))
   AppendError(llvm::Twine("Failed to run action on ") + Path +
   "\n");
@@ -141,11 +139,6 @@
 }
 // Make sure all tasks have finished before resetting the working 
directory.
 Pool.wait();
-if (!InitialWorkingDir.empty()) {
-  if (auto EC = llvm::sys::fs::set_current_path(InitialWorkingDir))
-llvm::errs() << "Error while restoring working directory: "
- << EC.message() << "\n";
-}
   }
 
   if (!ErrorMsg.empty())


Index: clang/lib/Tooling/AllTUsExecution.cpp
===
--- clang/lib/Tooling/AllTUsExecution.cpp
+++ clang/lib/Tooling/AllTUsExecution.cpp
@@ -9,6 +9,7 @@
 #include "clang/Tooling/AllTUsExecution.h"
 #include "clang/Tooling/ToolExecutorPluginRegistry.h"
 #include "llvm/Support/ThreadPool.h"
+#include "llvm/Support/VirtualFileSystem.h"
 
 namespace clang {
 namespace tooling {
@@ -114,25 +115,22 @@
   {
 llvm::ThreadPool Pool(ThreadCount == 0 ? llvm::hardware_concurrency()
: ThreadCount);
-llvm::SmallString<128> InitialWorkingDir;
-if (auto EC = llvm::sys::fs::current_path(InitialWorkingDir)) {
-  InitialWorkingDir = "";
-  llvm::errs() << "Error while getting current working directory: "
-   << EC.message() << "\n";
-}
 for (std::string File : Files) {
   Pool.async(
   [&](std::string Path) {
 Log("[" + std::to_string(Count()) + "/" + TotalNumStr +
 "] Processing file " + Path);
-ClangTool Tool(Compilations, {Path});
+// Each thread gets an indepent copy of a VFS to allow different
+// concurrent working directories.
+IntrusiveRefCntPtr FS =
+llvm::vfs::createPhysicalFileSystem().release();
+ClangTool Tool(Compilations, {Path},
+   std::make_shared(), FS);
 Tool.appendArgumentsAdjuster(Action.second);
 Tool.appendArgumentsAdjuster(getDefaultArgumentsAdjusters());
 for (const auto &FileAndContent : OverlayFiles)
   Tool.mapVirtualFile(FileAndContent.first(),
   FileAndContent.second);
-// Do not restore working dir from multiple threads to avoid races.
-Tool.setRestoreWorkingDir(false);
 if (Tool.run(Action.first.get()))
   AppendError(llvm::Twine("Failed to run action on ") + Path +
   "\n");
@@ -141,11 +139,6 @@
 }
 // Make sure all tasks have finished before resetting the working directory.
 Pool.wait();
-if (!InitialWorkingDir.empty()) {
-  if (auto EC = llvm::sys::fs::set_current

[PATCH] D59683: [Tooling] Avoid working-dir races in AllTUsToolExecutor

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov planned changes to this revision.
ilya-biryukov added a comment.

This breaks lots of stuff, need to figure out why this happens :-(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59683



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

[PATCH] D59639: [clangd] Print template arguments helper

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/unittests/clangd/ASTUtilsTests.cpp:1
+#include "AST.h"
+#include "Annotations.h"

NIT: add a licence header



Comment at: clang-tools-extra/unittests/clangd/CMakeLists.txt:13
   Annotations.cpp
+  ASTUtilsTests.cpp
   BackgroundIndexTests.cpp

NIT: maybe call it `PrintASTTests`?
To avoid creating a dump of test for all the stuff we are too lazy to classify 
properly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59639



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

[PATCH] D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Hi Shafik,

Thank you for looking into this. This is indeed a bug, because whenever a we 
encounter an unnamed EnumDecl in the "from" context then we return with a 
nameconflict error.
However, I think we should fix this differently.
First of all, currently HandleNameConflict just returns the parameter `Name` it 
received. This may be unnamed in some cases and thus converts to true. So I 
think HandleNameConflict should be changed that it would return a default 
initialized DeclarationName; so once we have anything in the ConflictingDecls 
then we return with the NameConflict error:

  DeclarationName ASTImporter::HandleNameConflict(DeclarationName Name,
  DeclContext *DC,
  unsigned IDNS,
  NamedDecl **Decls,
  unsigned NumDecls) {
  -  return Name;
  +  return DeclarationName();
  }

And most importantly, I think we should push into the ConflictingDecls only if 
the structural match indicates a non-match:

if (auto *FoundEnum = dyn_cast(FoundDecl)) {
  if (isStructuralMatch(D, FoundEnum, true))
return Importer.MapImported(D, FoundEnum);
  +   ConflictingDecls.push_back(FoundDecl);
}
  -
  - ConflictingDecls.push_back(FoundDecl);

I am aware of similar errors like this with other AST nodes. We had a patch in 
our fork to fix that in January 
(https://github.com/Ericsson/clang/pull/569/files) I am going to prepare a 
patch from that cause I see now this affects you guys in LLDB too.


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

https://reviews.llvm.org/D59665



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

[PATCH] D59684: [clang-format] [PR41187] moves Java import statements to the wrong location if code contains statements that start with the word import

[MyDeveloperDay via Phabricator via cfe-commits]

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

Import sorting of java file, incorrectly move import statement to after a 
function beginning with the word import.

Make 1 character change to regular expression to ensure there is always at 
least one space/tab after the word import

Previously clang-format --style="LLVM" would format

  import X;
  
  class C {
void m() {
  importFile();
}
  }

as

  class C {
void m() {
  importFile();
  import X;
}
  }


https://reviews.llvm.org/D59684

Files:
  clang/lib/Format/Format.cpp
  clang/unittests/Format/SortImportsTestJava.cpp


Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -262,6 +262,21 @@
  "import org.a;"));
 }
 
+TEST_F(SortImportsTestJava, ImportNamedFunction) {
+  EXPECT_EQ("import X;\n"
+"class C {\n"
+"  void m() {\n"
+"importFile();\n"
+"  }\n"
+"}\n",
+sort("import X;\n"
+ "class C {\n"
+ "  void m() {\n"
+ "importFile();\n"
+ "  }\n"
+ "}\n"));
+}
+
 TEST_F(SortImportsTestJava, NoReplacementsForValidImports) {
   // Identical #includes have led to a failure with an unstable sort.
   std::string Code = "import org.a;\n"
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1960,7 +1960,7 @@
 namespace {
 
 const char JavaImportRegexPattern[] =
-"^[\t ]*import[\t ]*(static[\t ]*)?([^\t ]*)[\t ]*;";
+"^[\t ]*import[\t ]+(static[\t ]*)?([^\t ]*)[\t ]*;";
 
 } // anonymous namespace
 


Index: clang/unittests/Format/SortImportsTestJava.cpp
===
--- clang/unittests/Format/SortImportsTestJava.cpp
+++ clang/unittests/Format/SortImportsTestJava.cpp
@@ -262,6 +262,21 @@
  "import org.a;"));
 }
 
+TEST_F(SortImportsTestJava, ImportNamedFunction) {
+  EXPECT_EQ("import X;\n"
+"class C {\n"
+"  void m() {\n"
+"importFile();\n"
+"  }\n"
+"}\n",
+sort("import X;\n"
+ "class C {\n"
+ "  void m() {\n"
+ "importFile();\n"
+ "  }\n"
+ "}\n"));
+}
+
 TEST_F(SortImportsTestJava, NoReplacementsForValidImports) {
   // Identical #includes have led to a failure with an unstable sort.
   std::string Code = "import org.a;\n"
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1960,7 +1960,7 @@
 namespace {
 
 const char JavaImportRegexPattern[] =
-"^[\t ]*import[\t ]*(static[\t ]*)?([^\t ]*)[\t ]*;";
+"^[\t ]*import[\t ]+(static[\t ]*)?([^\t ]*)[\t ]*;";
 
 } // anonymous namespace
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59639: [clangd] Print template arguments helper

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/AST.cpp:26
+llvm::Optional>
+getTemplateSpecializationArgLocs(const NamedDecl &ND) {
+  if (auto *Func = llvm::dyn_cast(&ND)) {

Eugene.Zelenko wrote:
> Functions should be static, not in anonymous namespace. See LLVM Coding 
> Guidelines.
We tend to put internal functions to anonymous namespace quite a lot in clangd.
While technically a small violation of the LLVM style guide, this aligns with 
the rest of the code and we don't consider that to be a problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59639



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

[PATCH] D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D59665#1438911 , @a_sidorin wrote:

> Hi Shafik,
>
> Honestly, I was always wondering what does HandleNameConflict actually do. 
> Its implementation in ASTImporter is trivial and I don't see any of its 
> overrides in LLDB code too. Why do we check its result to be non-empty is a 
> question to me as well. And the more I look at it (and the bug you pointed in 
> it), the more I think there is something wrong with it. Maybe it is better to 
> just remove it at all? I hope LLDB developers have more knowledge about it. 
> Shafik, Davide @davide , do you know its actual purpose?


As to my best knowlege, HandleNameConflict is not overridden in LLDB (@davide , 
@shafik please correct my if I am wrong).
However, I would not remove it, I'd rather fix the problems we have around it.
I think via HandleNameConflict we could implement different strategies about 
how to handle ODR errors and perhaps that was the original intention to use it 
for. With CTU (and probably with LLDB too) this could be really helpful: 
Recently I have discovered that during the CTU analysis of Xerces there are 
real ODR errors on a typedef, but it would be nice if we could just keep on 
going with the inlining of a certain function. So, I see three possible 
different strategies to handle name conflicts:

1. Conservative. In case of name conflict propagate the error. (The function 
will not be inlined because of a typedef ODR error.)
2. Liberal. In case of name conflict create a new node with the same name and 
do not propagate the error. (This may give erroneous results if a lookup is 
performed, but most SA checkers does not do lookup)
3. Rename. In case of name conflict create a new node with a different name 
(e.g. with a prefix of the TU's name). Do not propagate the error. (This would 
result the most sane AST, but this is perhaps the most difficult to implement, 
plus a mapping between old and new names has to be given.)


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

https://reviews.llvm.org/D59665



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

[PATCH] D59683: [Tooling] Avoid working-dir races in AllTUsToolExecutor

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

False alarm, I was running tests on top of a previously broken revision.
Everything seems to work just fine.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59683



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

[PATCH] D59683: [Tooling] Avoid working-dir races in AllTUsToolExecutor

[Eric Liu via Phabricator via cfe-commits]

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

Neat!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59683



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

[PATCH] D59615: [AArch64] When creating SISD intrinsic calls widen scalar args into a zero vectors, not undef

[Tim Northover via Phabricator via cfe-commits]

t.p.northover added a comment.

Did you look into a scalar variant of the intrinsic call instead? These 
instructions have non-vector variants (e.g. `sqadd s0, s0, s0`), and that's 
actually why the intrinsics exist in the first place. It'd be a shame to always 
require this extra work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59615



___
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 updated this revision to Diff 191844.
kadircet marked 11 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59481

Files:
  clang-tools-extra/clangd/index/Background.h
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/FileIndex.h
  clang-tools-extra/unittests/clangd/BackgroundIndexTests.cpp
  clang-tools-extra/unittests/clangd/FileIndexTests.cpp

Index: clang-tools-extra/unittests/clangd/FileIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/FileIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/FileIndexTests.cpp
@@ -46,6 +46,9 @@
   return llvm::StringRef(arg.Definition.FileURI) == U;
 }
 MATCHER_P(QName, N, "") { return (arg.Scope + arg.Name).str() == N; }
+MATCHER_P(NumReferences, N, "") {
+  return arg.References == static_cast(N);
+}
 
 namespace clang {
 namespace clangd {
@@ -59,6 +62,7 @@
   Symbol Sym;
   Sym.ID = SymbolID(ID);
   Sym.Name = ID;
+  Sym.References = 1;
   return Sym;
 }
 
@@ -417,6 +421,31 @@
   EXPECT_THAT(getRefs(Index, Foo.ID), RefsAre({RefRange(Main.range())}));
 }
 
+TEST(FileSymbolsTest, CountReferencesNoRefs) {
+  FileSymbols FS;
+  FS.update("f1", numSlab(1, 3), nullptr);
+  FS.update("f2", numSlab(1, 3), nullptr);
+  FS.update("f3", numSlab(1, 3), nullptr);
+  EXPECT_THAT(
+  runFuzzyFind(*FS.buildIndex(IndexType::Light, DuplicateHandling::Merge),
+   ""),
+  UnorderedElementsAre(AllOf(QName("1"), NumReferences(3)),
+   AllOf(QName("2"), NumReferences(3)),
+   AllOf(QName("3"), NumReferences(3;
+}
+
+TEST(FileSymbolsTest, CountReferencesWithRefs) {
+  FileSymbols FS;
+  FS.update("f1", numSlab(1, 3), refSlab(SymbolID("1"), "f1.cpp"));
+  FS.update("f2", numSlab(1, 3), refSlab(SymbolID("2"), "f2.cpp"));
+  FS.update("f3", numSlab(1, 3), refSlab(SymbolID("3"), "f3.cpp"));
+  EXPECT_THAT(
+  runFuzzyFind(*FS.buildIndex(IndexType::Light, DuplicateHandling::Merge),
+   ""),
+  UnorderedElementsAre(AllOf(QName("1"), NumReferences(1)),
+   AllOf(QName("2"), NumReferences(1)),
+   AllOf(QName("3"), NumReferences(1;
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/unittests/clangd/BackgroundIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/BackgroundIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/BackgroundIndexTests.cpp
@@ -32,6 +32,7 @@
   return !arg.IsTU && !arg.URI.empty() && arg.Digest == FileDigest{{0}} &&
  arg.DirectIncludes.empty();
 }
+MATCHER_P(NumReferences, N, "") { return arg.References == N; }
 
 class MemoryShardStorage : public BackgroundIndexStorage {
   mutable std::mutex StorageMu;
@@ -127,20 +128,25 @@
   CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
 
   ASSERT_TRUE(Idx.blockUntilIdleForTest());
-  EXPECT_THAT(
-  runFuzzyFind(Idx, ""),
-  UnorderedElementsAre(Named("common"), Named("A_CC"), Named("g"),
-   AllOf(Named("f_b"), Declared(), Not(Defined();
+  EXPECT_THAT(runFuzzyFind(Idx, ""),
+  UnorderedElementsAre(AllOf(Named("common"), NumReferences(2U)),
+   AllOf(Named("A_CC"), NumReferences(1U)),
+   AllOf(Named("g"), NumReferences(0U)),
+   AllOf(Named("f_b"), Declared(),
+ Not(Defined()), NumReferences(1U;
 
   Cmd.Filename = testPath("root/B.cc");
   Cmd.CommandLine = {"clang++", Cmd.Filename};
-  CDB.setCompileCommand(testPath("root/A.cc"), Cmd);
+  CDB.setCompileCommand(testPath("root/B.cc"), Cmd);
 
   ASSERT_TRUE(Idx.blockUntilIdleForTest());
   // B_CC is dropped as we don't collect symbols from A.h in this compilation.
   EXPECT_THAT(runFuzzyFind(Idx, ""),
-  UnorderedElementsAre(Named("common"), Named("A_CC"), Named("g"),
-   AllOf(Named("f_b"), Declared(), Defined(;
+  UnorderedElementsAre(AllOf(Named("common"), NumReferences(3U)),
+   AllOf(Named("A_CC"), NumReferences(1U)),
+   AllOf(Named("g"), NumReferences(0U)),
+   AllOf(Named("f_b"), Declared(), Defined(),
+ NumReferences(2U;
 
   auto Syms = runFuzzyFind(Idx, "common");
   EXPECT_THAT(Syms, UnorderedElementsAre(Named("common")));
Index: clang-tools-extra/clangd/index/FileIndex.h
===
--- clang-tools-extra/clangd/index/FileIndex.h
+++ clang-tools-extra/clangd/index/FileIndex.h
@@ -56,6 +56,10 @@
 ///
 /// 

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

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:46
+auto It = MergedSyms->find(Sym.first);
+assert(It != MergedSyms->end() && "Reference to unknown symbol!");
+// Note that we increment References per each file mentioning the

ioeric wrote:
> I think we can trigger this assertion with an incomplete background index. 
> For example, we've seen references of a symbol in some files but we've not 
> parsed the file that contains the decls.
You are right, thanks for pointing this out!



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:
> kadircet wrote:
> > 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?
> the comment is useful. I just think it's in the wrong place. If we define the 
> reference counting behavior for `FileSymbols`, `FileSymbols`wouldn't need to 
> know about clangd-indexer or background-index. This comment can then live in 
> background index instead.
Moved into index/background.h



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:189
   AllSymbols.push_back(&Sym);
+mergeRefs(RefSlabs, RefsStorage, AllRefs);
 break;

ioeric wrote:
> any reason not to count references for `PickOne`?
Because PickOne is not populating SymsStorage and is merely passing along 
pointers to SymbolSlabs. Counting references in that case would imply also 
making a new copy per each unique symbol. 

Do you think it is worth it?


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] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> @martong It's not related to lookup or anything like that, it's just that we 
> don't have enough information in our debug info AST to allow people to use 
> meta programming. The custom logic we have in LLDB looks into the std C++ 
> module to fill out these gaps in the AST by hand when they are used in an 
> expression.
> 
> The remark about the alternative being slow just means that fixing all the 
> templates we have in our debug info AST seems like a costly approach. I'm 
> also not sure how well it would work, as I never tried mixing a C++ module in 
> an ASTContext that isn't currently being parsed by Clang.

Well, I still don't understand how LLDB synthesis the AST. 
So you have a C++ module in a .pcm file. This means you could create an AST 
from that with ASTReader from it's .clang_ast section, right? In that case the 
AST should be complete with all type information. If this was the case then I 
don't see why it is not possible to use clang::ASTImporter to import templates 
and specializations, since we do exactly that in CTU.

Or do you guys create the AST from the debug info, from the .debug_info section 
of .pcm module file? And this is why AST is incomplete? I've got this from 
https://www.youtube.com/watch?v=EgkZ8PTNSHQ&list=PL85Cf-ok7HpppFWlp2wX_-A1dkyFVlaEB&index=2&t=5s
If this is the case, then comes my naiive quiestion: Why don't you use the 
ASTReader?


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

r356742 - [ARM] Add Cortex-M35P Support

[Luke Cheeseman via cfe-commits]

Author: lukecheeseman
Date: Fri Mar 22 03:58:15 2019
New Revision: 356742

URL: http://llvm.org/viewvc/llvm-project?rev=356742&view=rev
Log:
[ARM] Add Cortex-M35P Support

- Add clang frontend testing for Cortex-M35P

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


Modified:
cfe/trunk/test/Driver/arm-cortex-cpus.c

Modified: cfe/trunk/test/Driver/arm-cortex-cpus.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/arm-cortex-cpus.c?rev=356742&r1=356741&r2=356742&view=diff
==
--- cfe/trunk/test/Driver/arm-cortex-cpus.c (original)
+++ cfe/trunk/test/Driver/arm-cortex-cpus.c Fri Mar 22 03:58:15 2019
@@ -822,8 +822,10 @@
 // RUN: %clang -target arm -mcpu=cortex-m23 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CPUV8MBASE %s
 // CHECK-CPUV8MBASE:  "-cc1"{{.*}} "-triple" "thumbv8m.base-
 
-// RUN: %clang -target arm -mcpu=cortex-m33 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CPUV8MMAIN %s
-// CHECK-CPUV8MMAIN:  "-cc1"{{.*}} "-triple" "thumbv8m.main-
+// RUN: %clang -target arm -mcpu=cortex-m33 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CORTEX-M33 %s
+// RUN: %clang -target arm -mcpu=cortex-m35p -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CORTEX-M35P %s
+// CHECK-CORTEX-M33:  "-cc1"{{.*}} "-triple" "thumbv8m.main-{{.*}} 
"-target-cpu" "cortex-m33"
+// CHECK-CORTEX-M35P:  "-cc1"{{.*}} "-triple" "thumbv8m.main-{{.*}} 
"-target-cpu" "cortex-m35p"
 
 // == Check whether -mcpu accepts mixed-case values.
 // RUN: %clang -target arm-linux-gnueabi -mcpu=Cortex-a5 -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-CASE-INSENSITIVE-CPUV7A %s


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

[PATCH] D57765: [ARM] Add Cortex-M35P Support

[Luke Cheeseman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356742: [ARM] Add Cortex-M35P Support (authored by 
LukeCheeseman, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57765?vs=188364&id=191852#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57765

Files:
  cfe/trunk/test/Driver/arm-cortex-cpus.c


Index: cfe/trunk/test/Driver/arm-cortex-cpus.c
===
--- cfe/trunk/test/Driver/arm-cortex-cpus.c
+++ cfe/trunk/test/Driver/arm-cortex-cpus.c
@@ -822,8 +822,10 @@
 // RUN: %clang -target arm -mcpu=cortex-m23 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CPUV8MBASE %s
 // CHECK-CPUV8MBASE:  "-cc1"{{.*}} "-triple" "thumbv8m.base-
 
-// RUN: %clang -target arm -mcpu=cortex-m33 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CPUV8MMAIN %s
-// CHECK-CPUV8MMAIN:  "-cc1"{{.*}} "-triple" "thumbv8m.main-
+// RUN: %clang -target arm -mcpu=cortex-m33 -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CORTEX-M33 %s
+// RUN: %clang -target arm -mcpu=cortex-m35p -### -c %s 2>&1 | FileCheck 
-check-prefix=CHECK-CORTEX-M35P %s
+// CHECK-CORTEX-M33:  "-cc1"{{.*}} "-triple" "thumbv8m.main-{{.*}} 
"-target-cpu" "cortex-m33"
+// CHECK-CORTEX-M35P:  "-cc1"{{.*}} "-triple" "thumbv8m.main-{{.*}} 
"-target-cpu" "cortex-m35p"
 
 // == Check whether -mcpu accepts mixed-case values.
 // RUN: %clang -target arm-linux-gnueabi -mcpu=Cortex-a5 -### -c %s 2>&1 | 
FileCheck -check-prefix=CHECK-CASE-INSENSITIVE-CPUV7A %s


Index: cfe/trunk/test/Driver/arm-cortex-cpus.c
===
--- cfe/trunk/test/Driver/arm-cortex-cpus.c
+++ cfe/trunk/test/Driver/arm-cortex-cpus.c
@@ -822,8 +822,10 @@
 // RUN: %clang -target arm -mcpu=cortex-m23 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV8MBASE %s
 // CHECK-CPUV8MBASE:  "-cc1"{{.*}} "-triple" "thumbv8m.base-
 
-// RUN: %clang -target arm -mcpu=cortex-m33 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CPUV8MMAIN %s
-// CHECK-CPUV8MMAIN:  "-cc1"{{.*}} "-triple" "thumbv8m.main-
+// RUN: %clang -target arm -mcpu=cortex-m33 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CORTEX-M33 %s
+// RUN: %clang -target arm -mcpu=cortex-m35p -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CORTEX-M35P %s
+// CHECK-CORTEX-M33:  "-cc1"{{.*}} "-triple" "thumbv8m.main-{{.*}} "-target-cpu" "cortex-m33"
+// CHECK-CORTEX-M35P:  "-cc1"{{.*}} "-triple" "thumbv8m.main-{{.*}} "-target-cpu" "cortex-m35p"
 
 // == Check whether -mcpu accepts mixed-case values.
 // RUN: %clang -target arm-linux-gnueabi -mcpu=Cortex-a5 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-CASE-INSENSITIVE-CPUV7A %s
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r356743 - [Tooling] Avoid working-dir races in AllTUsToolExecutor

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Fri Mar 22 04:01:13 2019
New Revision: 356743

URL: http://llvm.org/viewvc/llvm-project?rev=356743&view=rev
Log:
[Tooling] Avoid working-dir races in AllTUsToolExecutor

Reviewers: ioeric

Reviewed By: ioeric

Subscribers: jdoerfert, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/Tooling/AllTUsExecution.cpp

Modified: cfe/trunk/lib/Tooling/AllTUsExecution.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/AllTUsExecution.cpp?rev=356743&r1=356742&r2=356743&view=diff
==
--- cfe/trunk/lib/Tooling/AllTUsExecution.cpp (original)
+++ cfe/trunk/lib/Tooling/AllTUsExecution.cpp Fri Mar 22 04:01:13 2019
@@ -9,6 +9,7 @@
 #include "clang/Tooling/AllTUsExecution.h"
 #include "clang/Tooling/ToolExecutorPluginRegistry.h"
 #include "llvm/Support/ThreadPool.h"
+#include "llvm/Support/VirtualFileSystem.h"
 
 namespace clang {
 namespace tooling {
@@ -114,25 +115,22 @@ llvm::Error AllTUsToolExecutor::execute(
   {
 llvm::ThreadPool Pool(ThreadCount == 0 ? llvm::hardware_concurrency()
: ThreadCount);
-llvm::SmallString<128> InitialWorkingDir;
-if (auto EC = llvm::sys::fs::current_path(InitialWorkingDir)) {
-  InitialWorkingDir = "";
-  llvm::errs() << "Error while getting current working directory: "
-   << EC.message() << "\n";
-}
 for (std::string File : Files) {
   Pool.async(
   [&](std::string Path) {
 Log("[" + std::to_string(Count()) + "/" + TotalNumStr +
 "] Processing file " + Path);
-ClangTool Tool(Compilations, {Path});
+// Each thread gets an indepent copy of a VFS to allow different
+// concurrent working directories.
+IntrusiveRefCntPtr FS =
+llvm::vfs::createPhysicalFileSystem().release();
+ClangTool Tool(Compilations, {Path},
+   std::make_shared(), FS);
 Tool.appendArgumentsAdjuster(Action.second);
 Tool.appendArgumentsAdjuster(getDefaultArgumentsAdjusters());
 for (const auto &FileAndContent : OverlayFiles)
   Tool.mapVirtualFile(FileAndContent.first(),
   FileAndContent.second);
-// Do not restore working dir from multiple threads to avoid races.
-Tool.setRestoreWorkingDir(false);
 if (Tool.run(Action.first.get()))
   AppendError(llvm::Twine("Failed to run action on ") + Path +
   "\n");
@@ -141,11 +139,6 @@ llvm::Error AllTUsToolExecutor::execute(
 }
 // Make sure all tasks have finished before resetting the working 
directory.
 Pool.wait();
-if (!InitialWorkingDir.empty()) {
-  if (auto EC = llvm::sys::fs::set_current_path(InitialWorkingDir))
-llvm::errs() << "Error while restoring working directory: "
- << EC.message() << "\n";
-}
   }
 
   if (!ErrorMsg.empty())


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

[PATCH] D59683: [Tooling] Avoid working-dir races in AllTUsToolExecutor

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL356743: [Tooling] Avoid working-dir races in 
AllTUsToolExecutor (authored by ibiryukov, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59683?vs=191832&id=191853#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59683

Files:
  cfe/trunk/lib/Tooling/AllTUsExecution.cpp


Index: cfe/trunk/lib/Tooling/AllTUsExecution.cpp
===
--- cfe/trunk/lib/Tooling/AllTUsExecution.cpp
+++ cfe/trunk/lib/Tooling/AllTUsExecution.cpp
@@ -9,6 +9,7 @@
 #include "clang/Tooling/AllTUsExecution.h"
 #include "clang/Tooling/ToolExecutorPluginRegistry.h"
 #include "llvm/Support/ThreadPool.h"
+#include "llvm/Support/VirtualFileSystem.h"
 
 namespace clang {
 namespace tooling {
@@ -114,25 +115,22 @@
   {
 llvm::ThreadPool Pool(ThreadCount == 0 ? llvm::hardware_concurrency()
: ThreadCount);
-llvm::SmallString<128> InitialWorkingDir;
-if (auto EC = llvm::sys::fs::current_path(InitialWorkingDir)) {
-  InitialWorkingDir = "";
-  llvm::errs() << "Error while getting current working directory: "
-   << EC.message() << "\n";
-}
 for (std::string File : Files) {
   Pool.async(
   [&](std::string Path) {
 Log("[" + std::to_string(Count()) + "/" + TotalNumStr +
 "] Processing file " + Path);
-ClangTool Tool(Compilations, {Path});
+// Each thread gets an indepent copy of a VFS to allow different
+// concurrent working directories.
+IntrusiveRefCntPtr FS =
+llvm::vfs::createPhysicalFileSystem().release();
+ClangTool Tool(Compilations, {Path},
+   std::make_shared(), FS);
 Tool.appendArgumentsAdjuster(Action.second);
 Tool.appendArgumentsAdjuster(getDefaultArgumentsAdjusters());
 for (const auto &FileAndContent : OverlayFiles)
   Tool.mapVirtualFile(FileAndContent.first(),
   FileAndContent.second);
-// Do not restore working dir from multiple threads to avoid races.
-Tool.setRestoreWorkingDir(false);
 if (Tool.run(Action.first.get()))
   AppendError(llvm::Twine("Failed to run action on ") + Path +
   "\n");
@@ -141,11 +139,6 @@
 }
 // Make sure all tasks have finished before resetting the working 
directory.
 Pool.wait();
-if (!InitialWorkingDir.empty()) {
-  if (auto EC = llvm::sys::fs::set_current_path(InitialWorkingDir))
-llvm::errs() << "Error while restoring working directory: "
- << EC.message() << "\n";
-}
   }
 
   if (!ErrorMsg.empty())


Index: cfe/trunk/lib/Tooling/AllTUsExecution.cpp
===
--- cfe/trunk/lib/Tooling/AllTUsExecution.cpp
+++ cfe/trunk/lib/Tooling/AllTUsExecution.cpp
@@ -9,6 +9,7 @@
 #include "clang/Tooling/AllTUsExecution.h"
 #include "clang/Tooling/ToolExecutorPluginRegistry.h"
 #include "llvm/Support/ThreadPool.h"
+#include "llvm/Support/VirtualFileSystem.h"
 
 namespace clang {
 namespace tooling {
@@ -114,25 +115,22 @@
   {
 llvm::ThreadPool Pool(ThreadCount == 0 ? llvm::hardware_concurrency()
: ThreadCount);
-llvm::SmallString<128> InitialWorkingDir;
-if (auto EC = llvm::sys::fs::current_path(InitialWorkingDir)) {
-  InitialWorkingDir = "";
-  llvm::errs() << "Error while getting current working directory: "
-   << EC.message() << "\n";
-}
 for (std::string File : Files) {
   Pool.async(
   [&](std::string Path) {
 Log("[" + std::to_string(Count()) + "/" + TotalNumStr +
 "] Processing file " + Path);
-ClangTool Tool(Compilations, {Path});
+// Each thread gets an indepent copy of a VFS to allow different
+// concurrent working directories.
+IntrusiveRefCntPtr FS =
+llvm::vfs::createPhysicalFileSystem().release();
+ClangTool Tool(Compilations, {Path},
+   std::make_shared(), FS);
 Tool.appendArgumentsAdjuster(Action.second);
 Tool.appendArgumentsAdjuster(getDefaultArgumentsAdjusters());
 for (const auto &FileAndContent : OverlayFiles)
   Tool.mapVirtualFile(FileAndContent.first(),
   FileAndContent.second);
-// Do not restore working dir from multiple threads to avoid races.
-Tool.setRestoreWorkingDir(false);
 if (Tool.run(Action.first.get()))
   AppendError(llvm::Twine(

[PATCH] D59639: [clangd] Print template arguments helper

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet marked 15 inline comments as done.
kadircet added inline comments.



Comment at: clang-tools-extra/clangd/AST.cpp:88
 static const TemplateArgumentList *
 getTemplateSpecializationArgs(const NamedDecl &ND) {
   if (auto *Func = llvm::dyn_cast(&ND))

ioeric wrote:
> can we unify this with `getTemplateSpecializationArgLocs` somehow? 
> 
> it seems that args as written would also be favorable here (see FIXME in line 
> 112)
See D59641



Comment at: clang-tools-extra/clangd/AST.cpp:133
+printTemplateArgumentList(OS, *Args, Policy);
+  else if (auto *Cls = llvm::dyn_cast(&ND)) {
+if (const TypeSourceInfo *TSI = Cls->getTypeAsWritten()) {

ioeric wrote:
> why isn't this handled in `getTemplateSpecializationArgLocs`? Add a comment?
it is mentioned in `getTemplateSpecializationArgLocs`, would you rather move 
the comment here?



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

ioeric wrote:
> could you add clang tests for these changes?
We've also discussed this in the original comment and decided infrastructure 
necessary was too overwhelming. First we need to invoke compiler and get the 
AST, then write a Visitor to fetch relevant decls and only after that call 
printTemplateArguments ...



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

ioeric wrote:
> It's unclear to me what the new behavior is with changes in this file. Could 
> you add comment?
> 
> It might make sense to split the clang change into a separate patch and get 
> folks who are more familiar to take a look.
it just prevents erasure from TypeLoc to Type when printing the argument, so 
that we can do a fine-grained printing if Location is already there.



Comment at: clang/lib/AST/TypePrinter.cpp:1643
+  const auto &Kind = A.getArgument().getKind();
+  assert(Kind != TemplateArgument::Null &&
+ "TemplateArgumentKind can not be null!");

ioeric wrote:
> why? 
> 
> ```
> /// Represents an empty template argument, e.g., one that has not
> /// been deduced.
> ```
> It seems legitimate. 
> 
> Since this hot code path, we would want to avoid hard assertion if possible.
Sure, letting TemplateArgument::print handle that


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59639



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

[PATCH] D59639: [clangd] Print template arguments helper

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 191857.
kadircet marked an inline comment as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59639

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/unittests/clangd/CMakeLists.txt
  clang-tools-extra/unittests/clangd/PrintASTTests.cpp
  clang/lib/AST/TypePrinter.cpp

Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1632,6 +1632,19 @@
   return A.getArgument();
 }
 
+static void printArgument(const TemplateArgument &A, const PrintingPolicy &PP,
+  llvm::raw_ostream &OS) {
+  A.print(PP, OS);
+}
+
+static void printArgument(const TemplateArgumentLoc &A,
+  const PrintingPolicy &PP, llvm::raw_ostream &OS) {
+  const TemplateArgument::ArgKind &Kind = A.getArgument().getKind();
+  if (Kind == TemplateArgument::ArgKind::Type)
+return A.getTypeSourceInfo()->getType().print(OS, PP);
+  return A.getArgument().print(PP, OS);
+}
+
 template
 static void printTo(raw_ostream &OS, ArrayRef Args,
 const PrintingPolicy &Policy, bool SkipBrackets) {
@@ -1653,7 +1666,7 @@
 } else {
   if (!FirstArg)
 OS << Comma;
-  Argument.print(Policy, ArgOS);
+  printArgument(Arg, Policy, ArgOS);
 }
 StringRef ArgString = ArgOS.str();
 
Index: clang-tools-extra/unittests/clangd/PrintASTTests.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clangd/PrintASTTests.cpp
@@ -0,0 +1,94 @@
+//===--- ASTUtilsTests.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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "AST.h"
+#include "Annotations.h"
+#include "Protocol.h"
+#include "SourceCode.h"
+#include "TestTU.h"
+#include "clang/AST/RecursiveASTVisitor.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest-param-test.h"
+#include "gtest/gtest.h"
+#include "gtest/internal/gtest-param-util-generated.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using testing::ElementsAreArray;
+
+struct Case {
+  const char *AnnotatedCode;
+  std::vector Expected;
+};
+class ASTUtils : public testing::Test,
+ public ::testing::WithParamInterface {};
+
+TEST_P(ASTUtils, PrintTemplateArgs) {
+  auto Pair = GetParam();
+  Annotations Test(Pair.AnnotatedCode);
+  auto AST = TestTU::withCode(Test.code()).build();
+  struct Visitor : RecursiveASTVisitor {
+Visitor(std::vector Points) : Points(std::move(Points)) {}
+bool VisitNamedDecl(const NamedDecl *ND) {
+  auto Pos = sourceLocToPosition(ND->getASTContext().getSourceManager(),
+ ND->getLocation());
+  if (Pos != Points[TemplateArgs.size()])
+return true;
+  TemplateArgs.push_back(printTemplateSpecializationArgs(*ND));
+  return true;
+}
+std::vector TemplateArgs;
+const std::vector Points;
+  };
+  Visitor V(Test.points());
+  V.TraverseDecl(AST.getASTContext().getTranslationUnitDecl());
+  EXPECT_THAT(V.TemplateArgs, ElementsAreArray(Pair.Expected));
+}
+
+INSTANTIATE_TEST_CASE_P(ASTUtilsTests, ASTUtils,
+testing::ValuesIn(std::vector({
+{
+R"cpp(
+  template  class Bar {};
+  template <> class ^Bar {};)cpp",
+{""}},
+{
+R"cpp(
+  template  class Bar {};
+  template  class Z, int Q>
+  struct Foo {};
+  template struct ^Foo;
+  template 
+  struct ^Foo {};)cpp",
+{"", ""}},
+{
+R"cpp(
+  template  void Foz() {};
+  template <> void ^Foz<3, 5, 8>() {};)cpp",
+{"<3, 5, 8>"}},
+{
+R"cpp(
+  template  class Bar {};
+  template  class ...>
+  class Aux {};
+  template <> class ^Aux {};
+  template  T>
+ 

[PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor updated this revision to Diff 191859.
teemperor marked 4 inline comments as done.
teemperor added a comment.

- Addressed feedback.


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
@@ -304,6 +304,14 @@
   const char *const InputFileName = "input.cc";
   const char *const OutputFileName = "output.cc";
 
+public:
+  /// Allocates an ASTImporter (or one of its subclasses).
+  typedef std::function
+  ImporterConstructor;
+
+private:
   // Buffer for the To context, must live in the test scope.
   std::string ToCode;
 
@@ -316,22 +324,37 @@
 std::unique_ptr Unit;
 TranslationUnitDecl *TUDecl = nullptr;
 std::unique_ptr Importer;
-TU(StringRef Code, StringRef FileName, ArgVector Args)
+// The lambda that constructs the ASTImporter we use in this test.
+ImporterConstructor Creator;
+TU(StringRef Code, StringRef FileName, ArgVector Args,
+   ImporterConstructor C = ImporterConstructor())
 : Code(Code), FileName(FileName),
   Unit(tooling::buildASTFromCodeWithArgs(this->Code, Args,
  this->FileName)),
-  TUDecl(Unit->getASTContext().getTranslationUnitDecl()) {
+  TUDecl(Unit->getASTContext().getTranslationUnitDecl()), Creator(C) {
   Unit->enableSourceFileDiagnostics();
+
+  // If the test doesn't need a specific ASTImporter, we just create a
+  // normal ASTImporter with it.
+  if (!Creator)
+Creator = [](ASTContext &ToContext, FileManager &ToFileManager,
+ ASTContext &FromContext, FileManager &FromFileManager,
+ bool MinimalImport, ASTImporterLookupTable *LookupTable) {
+  return new ASTImporter(ToContext, ToFileManager, FromContext,
+ FromFileManager, MinimalImport, LookupTable);
+};
+}
+
+void setImporter(std::unique_ptr I) {
+  Importer = std::move(I);
 }
 
 void lazyInitImporter(ASTImporterLookupTable &LookupTable, ASTUnit *ToAST) {
   assert(ToAST);
-  if (!Importer) {
-Importer.reset(
-new ASTImporter(ToAST->getASTContext(), ToAST->getFileManager(),
-Unit->getASTContext(), Unit->getFileManager(),
-false, &LookupTable));
-  }
+  if (!Importer)
+Importer.reset(Creator(ToAST->getASTContext(), ToAST->getFileManager(),
+   Unit->getASTContext(), Unit->getFileManager(),
+   false, &LookupTable));
   assert(&ToAST->getASTContext() == &Importer->getToContext());
   createVirtualFileIfNeeded(ToAST, FileName, Code);
 }
@@ -401,11 +424,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,
+  ImporterConstructor Creator = ImporterConstructor()) {
 ArgVector FromArgs = getArgVectorForLanguage(FromLang),
   ToArgs = getArgVectorForLanguage(ToLang);
 
-FromTUs.emplace_back(FromSrcCode, InputFileName, FromArgs);
+FromTUs.emplace_back(FromSrcCode, InputFileName, FromArgs, Creator);
 TU &FromTU = FromTUs.back();
 
 assert(!ToAST);
@@ -455,6 +479,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 +574,75 @@
   EXPECT_THAT(RedeclsD1, ::testing::ContainerEq(RedeclsD2));
 }
 
+struct CustomImporter : ASTImporterOptionSpecificTestBase {};
+
+namespace {
+struct RedirectingImporter : public ASTImporter {
+  using ASTImporter::ASTImporter;
+  // Returns a ImporterConstructor that constructs this class.
+  static ASTImporterOptionSpecificTestBase::ImporterConstructor Constructor;
+
+protected:
+  llvm::Expected ImportInternal(Decl *FromD) override {
+auto *ND = dyn_cast(FromD);
+if (!ND)
+  return ASTImporter::ImportInternal(FromD);
+if (ND->getName() != "shouldNotBeImported")
+  return ASTImporter::ImportInternal(FromD);
+for (Decl *D : getToContext().getTranslationUnitDecl()->decls()) {
+  if (auto ND = dyn_cast(D))
+if (N

[PATCH] D59640: [clangd] Add TemplateArgumentList into Symbol

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D59640#1438248 , @ioeric wrote:

> should we update YAML?


I suppose we are only keeping it alive for the sake of tests, but that seems 
like an enough reason updating that as well.




Comment at: clang-tools-extra/clangd/index/SymbolCollector.cpp:526
   S.ID = std::move(ID);
+  std::string TemplateArgumentList = printTemplateArgsAsWritten(ND);
+  S.TemplateArgumentList = TemplateArgumentList;

ioeric wrote:
> put this near `ReturnType` initialization.
tried to move it to closest place I can get, can't put after this if since 
template specialization params are not indexed for code completion, we'll 
simply end up dropping those.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59640



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

[PATCH] D59640: [clangd] Add TemplateArgumentList into Symbol

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 191860.
kadircet marked 3 inline comments as done.
kadircet added a comment.

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59640

Files:
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/index/Symbol.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/YAMLSerialization.cpp
  clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp

Index: clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
===
--- clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
+++ clang-tools-extra/unittests/clangd/SymbolCollectorTests.cpp
@@ -51,6 +51,9 @@
   return (arg.Name + arg.CompletionSnippetSuffix).str() == S;
 }
 MATCHER_P(QName, Name, "") { return (arg.Scope + arg.Name).str() == Name; }
+MATCHER_P(TemplateArgs, TemplArgs, "") {
+  return arg.TemplateSpecializationArgs == TemplArgs;
+}
 MATCHER_P(DeclURI, P, "") {
   return StringRef(arg.CanonicalDeclaration.FileURI) == P;
 }
@@ -413,6 +416,71 @@
 ForCodeCompletion(false;
 }
 
+TEST_F(SymbolCollectorTest, TemplateArgs) {
+  Annotations Header(R"(
+template  class $barclasstemp[[Bar]] {};
+template  class Z, int Q>
+struct [[Tmpl]] { T $xdecl[[x]] = 0; };
+
+// template-template, non-type and type full spec
+template <> struct $specdecl[[Tmpl]] {};
+
+// template-template, non-type and type partial spec
+template  struct $partspecdecl[[Tmpl]] {};
+// instantiation
+extern template struct Tmpl;
+// instantiation
+template struct Tmpl;
+
+template  class $fooclasstemp[[Foo]] {};
+// parameter-packs full spec
+template<> class $parampack[[Foo]], int, double> {};
+// parameter-packs partial spec
+template class $parampackpartial[[Foo]] {};
+
+template  class $bazclasstemp[[Baz]] {};
+// non-type parameter-packs full spec
+template<> class $parampacknontype[[Baz]]<3, 5, 8> {};
+// non-type parameter-packs partial spec
+template class $parampacknontypepartial[[Baz]] {};
+
+template  class ...> class $fozclasstemp[[Foz]] {};
+// template-template parameter-packs full spec
+template<> class $parampacktempltempl[[Foz]] {};
+// template-template parameter-packs partial spec
+template class T>
+class $parampacktempltemplpartial[[Foz]] {};
+  )");
+  runSymbolCollector(Header.code(), /*Main=*/"");
+  EXPECT_THAT(
+  Symbols,
+  AllOf(
+  Contains(AllOf(QName("Tmpl"), TemplateArgs(""),
+ DeclRange(Header.range("specdecl")),
+ ForCodeCompletion(false))),
+  Contains(AllOf(QName("Tmpl"), TemplateArgs(""),
+ DeclRange(Header.range("partspecdecl")),
+ ForCodeCompletion(false))),
+  Contains(AllOf(QName("Foo"), TemplateArgs(", int, double>"),
+ DeclRange(Header.range("parampack")),
+ ForCodeCompletion(false))),
+  Contains(AllOf(QName("Foo"), TemplateArgs(""),
+ DeclRange(Header.range("parampackpartial")),
+ ForCodeCompletion(false))),
+  Contains(AllOf(QName("Baz"), TemplateArgs("<3, 5, 8>"),
+ DeclRange(Header.range("parampacknontype")),
+ ForCodeCompletion(false))),
+  Contains(AllOf(QName("Baz"), TemplateArgs(""),
+ DeclRange(Header.range("parampacknontypepartial")),
+ ForCodeCompletion(false))),
+  Contains(AllOf(QName("Foz"), TemplateArgs(""),
+ DeclRange(Header.range("parampacktempltempl")),
+ ForCodeCompletion(false))),
+  Contains(AllOf(QName("Foz"), TemplateArgs(""),
+ DeclRange(Header.range("parampacktempltemplpartial")),
+ ForCodeCompletion(false);
+}
+
 TEST_F(SymbolCollectorTest, ObjCSymbols) {
   const std::string Header = R"(
 @interface Person
Index: clang-tools-extra/clangd/index/YAMLSerialization.cpp
===
--- clang-tools-extra/clangd/index/YAMLSerialization.cpp
+++ clang-tools-extra/clangd/index/YAMLSerialization.cpp
@@ -193,6 +193,8 @@
 IO.mapOptional("Origin", NSymbolOrigin->Origin);
 IO.mapOptional("Flags", NSymbolFlag->Flag);
 IO.mapOptional("Signature", Sym.Signature);
+IO.mapOptional("TemplateSpecializationArgs",
+   Sym.TemplateSpecializationArgs);
 IO.mapOptional("CompletionSnippetSuffix", Sym.CompletionSnippetSuffix);
 IO.mapOptional("Documentation", Sym.Documentation);
 IO.mapOptional("ReturnType", Sym.ReturnType);
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker updated this revision to Diff 191862.
Tyker added a comment.

handled codegen for if, while, for and do/while, it will generate a 
@llvm.expect before the condition based on the attribute
i changed slithly the semantic

  if (...) {  //error
  [[likely]] ...
  }
  
  if (...) [[likely]]  { // ok
   ...
  }

and added tests for AST, Semantic and codegen


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

https://reviews.llvm.org/D59467

Files:
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Sema/SemaStmtAttr.cpp
  clang/test/AST/ast-dump-attr.cpp
  clang/test/CodeGenCXX/cxx2a-likelihood-attr.cpp
  clang/test/SemaCXX/cxx2a-likelihood-attr.cpp

Index: clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,51 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++2a
+
+int f(int i) {
+  if (i == 1) [[unlikely]]
+{
+  return 0;
+}
+  else if (i == 2) [[likely]]
+return 1;
+  return 3;
+}
+
+[[likely]] typedef int n1; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+typedef int [[likely]] n2; // expected-error {{'likely' attribute cannot be applied to types}}
+typedef int n3 [[likely]]; // expected-error {{'likely' attribute cannot be applied to a declaration}}
+
+enum [[likely]] E { // expected-error {{'likely' attribute cannot be applied to a declaration}}
+  One
+};
+
+[[likely]] // expected-error {{'likely' attribute cannot be applied to a declaration}}
+
+void test(int i) {
+  [[likely]] // expected-error {{'likely' can only appear after a selection or iteration statement}}
+if (1) [[likely, likely]] {
+  // expected-warning@-1 {{was already marked likely}}
+  // expected-note@-2 {{previous attribute is here}}
+  [[unlikely]] return ; // expected-error {{'unlikely' can only appear after a selection or iteration statement}}
+}
+  else [[unlikely]] if (1) {
+  while (1) [[likely]] {
+  // switch (i) { switch support isn't implemented yet
+  //   [[likely]] case 1:
+  // default: [[likely]]
+  //   return ;
+  // }
+}
+  for (;;) [[likely, unlikely]]
+  // expected-error@-1 {{unlikely and likely are mutually exclusive}}
+  // expected-note@-2 {{previous attribute is here}}
+[[likely]] return ;
+  // expected-warning@-1 {{was already marked likely}}
+  // expected-note@-5 {{previous attribute is here}}
+}
+  try { // expected-error {{cannot use 'try' with exceptions disabled}}
+[[likely]]; // expected-error {{'likely' can only appear after a selection or iteration statement}}
+  } catch (int) {
+  [[likely]] test: // expected-error {{'likely' attribute cannot be applied to a declaration}}
+  [[unlikely]] return ; // expected-error {{'unlikely' can only appear after a selection or iteration statement}}
+  }
+}
\ No newline at end of file
Index: clang/test/CodeGenCXX/cxx2a-likelihood-attr.cpp
===
--- /dev/null
+++ clang/test/CodeGenCXX/cxx2a-likelihood-attr.cpp
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -std=c++2a -cc1 -emit-llvm -disable-llvm-passes -O3 %s -o - -triple %itanium_abi_triple | FileCheck %s
+
+int test(int i) {
+  if (i == 0) {
+i = i + 1;
+  } else [[likely]]
+return 1;
+  // CHECK: %expval = call i1 @llvm.expect.i1(i1 %cmp, i1 false)
+  while (i == 1) [[unlikely]] {
+return 2;
+  }
+  // CHECK: %[[expval:.*]] = call i1 @llvm.expect.i1(i1 %[[cmp:.*]], i1 false)
+  for (;i == 4;) [[unlikely]] {
+return 2;
+  }
+  // CHECK: %[[expval:.*]] = call i1 @llvm.expect.i1(i1 %[[cmp:.*]], i1 false)
+  do [[likely]] {
+return 2;
+  } while (i == 3);
+  // CHECK: %[[expval:.*]] = call i1 @llvm.expect.i1(i1 %[[cmp:.*]], i1 true)
+  return 0;
+}
Index: clang/test/AST/ast-dump-attr.cpp
===
--- clang/test/AST/ast-dump-attr.cpp
+++ clang/test/AST/ast-dump-attr.cpp
@@ -1,5 +1,39 @@
 // RUN: %clang_cc1 -triple x86_64-pc-linux -std=c++11 -Wno-deprecated-declarations -ast-dump -ast-dump-filter Test %s | FileCheck --strict-whitespace %s
 
+int TestLikelyAttributeIf(int i) {
+  if (i == 1) [[likely]] {
+return 0;
+  } else if (i == 2) [[unlikely]]
+return 1;
+  return 2;
+}
+// CHECK:  IfStmt
+// CHECK:  AttributedStmt
+// CHECK-NEXT: LikelihoodAttr 0x{{[^ ]*}}  likely
+// CHECK:  IfStmt
+// CHECK:  AttributedStmt
+// CHECK-NEXT: LikelihoodAttr 0x{{[^ ]*}}  unlikely
+
+int TestLikelyAttributeLoops(int i) {
+  while (i == 1) [[likely]] {
+return 0;
+  }
+  for (;;) [[unlikely]]
+do [[likely]] {
+  return 1;
+} while (i == 2);
+  re

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang-tidy/utils/ExceptionAnalyzer.cpp:208
+template <>
+void ExceptionAnalyzer::analyze(const FunctionDecl *Func,
+ExceptionInfo &ExceptionList) {

I'd suggest to make it a non-template and call it also `analyzeImpl()` and 
return `ExceptionInfo by` value.



Comment at: clang-tidy/utils/ExceptionAnalyzer.cpp:226
+ExceptionAnalyzer::ExceptionInfo
+ExceptionAnalyzer::analyzeBoilerplate(const T *Node) {
+  ExceptionInfo ExceptionList;

JonasToth wrote:
> lebedev.ri wrote:
> > Please bikeshed on the name. I don't think this one is good.
> Hmm, `analyzeGeneric`, `analyzeGeneral`, `abstractAnalysis`, 
> `analyzeAbstract`, something good in these?
> 
> Given its private its not too important either ;)
I'd suggest to simplify by changing `analyzeBoilerplate()` into a non-template, 
into this specifically:

```
ExceptionAnalyzer::ExceptionInfo 
ExceptionAnalyzer::filterIgnoredExceptions(ExceptionInfo ExceptionList) {
if (ExceptionList.getBehaviour() == State::NotThrowing ||
  ExceptionList.getBehaviour() == State::Unknown)
return ExceptionList;

  // Remove all ignored exceptions from the list of exceptions that can be
  // thrown.
  ExceptionList.filterIgnoredExceptions(IgnoredExceptions, IgnoreBadAlloc);

  return ExceptionList;
}
```

And then call it in `analyze()`:

```
ExceptionAnalyzer::ExceptionInfo
ExceptionAnalyzer::analyze(const FunctionDecl *Func) {
  return filterIgnoredExceptions(analyzeImpl(Func));
}
```


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59650



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

[PATCH] D59692: [ASTImporter] Fix name conflict handling

[Gabor Marton via Phabricator via cfe-commits]

martong created this revision.
martong added reviewers: a_sidorin, shafik, teemperor.
Herald added subscribers: cfe-commits, jdoerfert, gamesh411, Szelethus, dkrupp, 
rnkovacs.
Herald added a reviewer: a.sidorin.
Herald added a project: clang.
martong added a parent revision: D55049: Changed every use of 
ASTImporter::Import to Import_New.

There are numorous flaws about the name conflict handling, this patch
attempts fixes them. Changes in details:

- HandleNameConflict return with a false DeclarationName

Hitherto we effectively never returned with a NameConflict error, even
if the preceding StructuralMatch indicated a conflict.
Because we just simply returned with the parameter `Name` in
HandleNameConflict and that name is almost always `true` when converted to
`bool`.

- Add tests which indicate wrong NameConflict handling

- Add to ConflictingDecls only if decl kind is different

Note, we might not indicate an ODR error when there is an existing record decl
and a enum is imported with same name.  But there are other cases. E.g. think
about the case when we import a FunctionTemplateDecl with name f and we found a
simple FunctionDecl with name f. They overload.  Or in case of a
ClassTemplateDecl and CXXRecordDecl, the CXXRecordDecl could be the 'templated'
class, so it would be false to report error.  So I think we should report a
name conflict error only when we are 100% sure of that.  That is why I think it
should be a general pattern to report the error only if the kind is the same.

- Fix failing ctu test with EnumConstandDecl

In ctu-main.c we have the enum class 'A' which brings in the enum
constant 'x' with value 0 into the global namespace.
In ctu-other.c we had the enum class 'B' which brought in the same name
('x') as an enum constant but with a different enum value (42). This is clearly
an ODR violation in the global namespace. The solution was to rename the
second enum constant.

- Fix lldb test failures

Remove the call of the unused and vexing GetOriginalDecl(). This information is
already maintained in the ImportedDecls member.


Repository:
  rC Clang

https://reviews.llvm.org/D59692

Files:
  include/clang/AST/ASTImporter.h
  lib/AST/ASTImporter.cpp
  test/Analysis/Inputs/ctu-other.c
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -1964,7 +1964,7 @@
   auto *ToTU = ToAST->getASTContext().getTranslationUnitDecl();
   // We expect one (ODR) warning during the import.
   EXPECT_EQ(1u, ToTU->getASTContext().getDiagnostics().getNumWarnings());
-  EXPECT_EQ(2u,
+  EXPECT_EQ(1u,
 DeclCounter().match(ToTU, recordDecl(hasName("X";
 }
 
@@ -2674,6 +2674,64 @@
 functionDecl(hasName("f"), hasDescendant(declRefExpr()));
 }
 
+struct ImportFunctionTemplates : ASTImporterOptionSpecificTestBase {};
+
+TEST_P(ImportFunctionTemplates,
+   ImportFunctionWhenThereIsAFunTemplateWithSameName) {
+  getToTuDecl(
+  R"(
+  template 
+  void foo(T) {}
+  void foo();
+  )",
+  Lang_CXX);
+  Decl *FromTU = getTuDecl("void foo();", Lang_CXX);
+  auto *FromD = FirstDeclMatcher().match(
+  FromTU, functionDecl(hasName("foo")));
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportConstructorWhenThereIsAFunTemplateWithSameName) {
+  auto Code =
+  R"(
+  struct Foo {
+template 
+Foo(T) {}
+Foo();
+  };
+  )";
+  getToTuDecl(Code, Lang_CXX);
+  Decl *FromTU = getTuDecl(Code, Lang_CXX);
+  auto *FromD =
+  LastDeclMatcher().match(FromTU, cxxConstructorDecl());
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
+TEST_P(ImportFunctionTemplates,
+   ImportOperatorWhenThereIsAFunTemplateWithSameName) {
+  getToTuDecl(
+  R"(
+  template 
+  void operator<(T,T) {}
+  struct X{};
+  void operator<(X, X);
+  )",
+  Lang_CXX);
+  Decl *FromTU = getTuDecl(
+  R"(
+  struct X{};
+  void operator<(X, X);
+  )",
+  Lang_CXX);
+  auto *FromD = LastDeclMatcher().match(
+  FromTU, functionDecl(hasOverloadedOperatorName("<")));
+  auto *ImportedD = Import(FromD, Lang_CXX);
+  EXPECT_TRUE(ImportedD);
+}
+
 struct ImportFriendFunctions : ImportFunctions {};
 
 TEST_P(ImportFriendFunctions, ImportFriendFunctionRedeclChainProto) {
@@ -5534,6 +5592,9 @@
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctions,
 DefaultTestValuesForRunOptions, );
 
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionTemplates,
+DefaultTestValuesForRunOptions, );
+
 INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFriendFunctionTemplates,
 DefaultTestValuesForRunOptions, );
 
Index: test/Analysis/Inputs/ctu-other.c
==

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

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:53
+  Finder->addMatcher(ompExecutableDirective(
+ unless(isStandaloneDirective()),
+ hasStructuredBlock(stmt().bind("structured-block")))

Do we need the `unless`?  `hasStructuredBlock()` just won't match standalone 
directives.



Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:65
+  Result.Nodes.getNodeAs("structured-block");
+  assert(StructuredBlock && "Expected to get soe OpenMP Structured Block.");
+

soe => some



Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:69
+  utils::ExceptionAnalyzer::State::Throwing)
+return; // No exceptions appear to escape out of the structured block.
+

appear to => have been proven to



Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:72
+  // FIXME: We should provide more information about the exact location where
+  // the exception is thrown, maybe the full path the exception escapes
+

+1



Comment at: docs/clang-tidy/checks/openmp-exception-escape.rst:21
+
+.. option:: IgnoredExceptions
+

`IgnoredExceptionTypes`?



Comment at: test/clang-tidy/bugprone-exception-escape-openmp.cpp:16
+;
+  // FIXME: this really should be caught by bugprone-exception-escape.
+  // https://bugs.llvm.org/show_bug.cgi?id=41102

Shouldn't the function be marked with `noexcept` for 
`bugprone-exception-escape` to catch it?  It does not know about OpenMP.

Are you suggesting that `bugprone-exception-escape` should subsume your new 
OpenMP-specific check?


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] D55049: Changed every use of ASTImporter::Import to Import_New

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.
Herald added a reviewer: martong.
Herald added a project: clang.



Comment at: lib/AST/ASTImporter.cpp:7767
+  if (!ToDCOrErr)
+return ToDCOrErr.takeError();
+  auto *ToDC = cast(*ToDCOrErr);

Actually, this patch is not merely a `Import` -> `Import_New` substitution. As 
this line shows, the error will be propagated correctly. However, in the old 
version we return with a nullptr DC which may be referenced later. This is the 
reason, why https://reviews.llvm.org/D59692 breaks in the unittests and thus 
depends on this patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55049



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

[PATCH] D55049: Changed every use of ASTImporter::Import to Import_New

[Gabor Marton via Phabricator via cfe-commits]

martong added a reviewer: a_sidorin.
martong added a comment.
Herald added a subscriber: rnkovacs.

Ping


Repository:
  rC Clang

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

https://reviews.llvm.org/D55049



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

[PATCH] D53757: [ASTImporter] Changed use of Import to Import_New in ASTNodeImporter.

[Gabor Marton via Phabricator via cfe-commits]

martong added a reviewer: a_sidorin.
martong added a subscriber: a_sidorin.
martong added a comment.

@shafik, @a_sidorin Ping. Could you please take a look?
Guys, this and its child patch are very important patches, because without it 
the error handling of ASTImporter is not completed. This means we may encounter 
a nullptr DeclContext in the middle of the import process, etc...


Repository:
  rC Clang

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

https://reviews.llvm.org/D53757



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

[PATCH] D59407: [clangd] Add RelationSlab

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.



Comment at: clang-tools-extra/clangd/index/Relation.h:1
+//===--- Ref.h ---*- 
C++-*-===//
+//

gribozavr wrote:
> "--- Relation.h "
Not done?



Comment at: clang-tools-extra/clangd/index/Relation.h:41
+public:
+  // Key.Symbol is Key.Kind of every symbol in Value.
+  // For example, if Key.Kind == SymbolRole::RelationChildOf,

gribozavr wrote:
> Three slashes for doc comments, please.
Not done?



Comment at: clang-tools-extra/clangd/index/Relation.h:45
+  // in Value are the base classes of Key.Symbol).
+  struct Relation {
+RelationKey Key;

nridge wrote:
> gribozavr wrote:
> > Lift it up into the `clang::clangd` namespace?  (like `Symbol` and `Ref`)
> This comment made me realize that I haven't addressed your previous comment 
> properly: I haven't changed `RelationSlab::value_type` from 
> `std::pair>` to `Relation`.
> 
> I tried to make that change this time, and ran into a problem:
> 
> In the rest of the subtypes patch (D58880), one of the things I do is extend 
> the `MemIndex` constructor so that, in addition to taking a symbol range and 
> a ref range, it takes a relation range.
> 
> That constructor assumes that the elements of that range have members of some 
> name - either `first` and `second` (as currently in D58880), or `Key` and 
> `Value`.
> 
> However, that constructor has two call sites. In `MemIndex::build()`, we pass 
> in the slabs themselves as the ranges. So, if we make this change, the field 
> names for that call site will be `Key` and `Value`. However, for the second 
> call site in `FileSymbols::buildIndex()`, the ranges that are passed in are 
> `DenseMap`s, and therefore their elements' field names are necessarily 
> `first` and `second`. The same constructor cannot therefore accept both 
> ranges.
> 
> How do you propose we address this?
> 
>  * Scrap `struct Relation`, and keep `value_type` as `std::pair llvm::ArrayRef>`?
>  * Keep `struct Relation`, but make its fields named `first` and `second`?
>  * Split the constructor of `MemIndex` into two constructors, to accomodate 
> both sets of field names?
>  * Something else?
I guess we should scrap it then.  Kadir, WDYT?



Comment at: clang-tools-extra/clangd/index/Relation.h:68
+
+  // RelationSlab::Builder is a mutable container that can 'freeze' to
+  // RelationSlab.

gribozavr wrote:
> No need to repeat the type name being documented.  "A mutable container that 
> can ..."
Not done?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407



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

[PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr accepted this revision.
gribozavr marked an inline comment as done.
gribozavr added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:166
+Preprocessor->Lex(CurrentToken);
+}
+

Haha, so the test that I asked to add did catch a bug -- just not the one I 
expected :)



Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.h:44
+  /// \brief Returns the preprocessor that provides callbacks for contents of
+  /// modular headers.
+  ///

"... callbacks for the whole translation unit, including the main file, textual 
headers, and modular headers."

Sorry, I wrote the comment before I fully understood what this preprocessor 
does.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59528



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

[PATCH] D59665: Call to HandleNameConflict in VisitEnumDecl mistakeningly using Name instead of SearchName

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> I am aware of similar errors like this with other AST nodes. We had a patch 
> in our fork to fix that in January 
> (https://github.com/Ericsson/clang/pull/569/files) I am going to prepare a 
> patch from that cause I see now this affects you guys in LLDB too.

Just created that patch:
https://reviews.llvm.org/D59692

Unfortunately, this depends on other patches which are already in the queue, 
please check the stack.


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

https://reviews.llvm.org/D59665



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

[PATCH] D59682: [X86] Add BSR/BSF/BSWAP intrinsics to ia32intrin.h to match gcc.

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added inline comments.



Comment at: lib/Headers/ia32intrin.h:31
 
+static __inline__ int __attribute__((__always_inline__, __nodebug__))
+__bsfd(int __A) {

Ideally we'd have doxygen comments.



Comment at: test/CodeGen/bitscan-builtins.c:24
+// CHECK: @test__bsfd
+// CHECK: %[[call:.*]] = call i32 @llvm.cttz.i32(
+  return __bsfd(X);

Should we be testing the is_zero_undef flag is true?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59682



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

[PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh marked 3 inline comments as done.
alexfh added inline comments.



Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp:166
+Preprocessor->Lex(CurrentToken);
+}
+

gribozavr wrote:
> Haha, so the test that I asked to add did catch a bug -- just not the one I 
> expected :)
Nope, the test passes without this. I'm not even sure this override is changing 
any observable behavior of PPCallbacks. I hope to find this out by running 
other checks on real code with modules enabled.



Comment at: clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.h:44
+  /// \brief Returns the preprocessor that provides callbacks for contents of
+  /// modular headers.
+  ///

gribozavr wrote:
> "... callbacks for the whole translation unit, including the main file, 
> textual headers, and modular headers."
> 
> Sorry, I wrote the comment before I fully understood what this preprocessor 
> does.
Indeed. Thanks for the text.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59528



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

[PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

[Raphael Isemann via Phabricator via cfe-commits]

teemperor added a comment.

> Well, I still don't understand how LLDB synthesis the AST. 
>  So you have a C++ module in a .pcm file. This means you could create an AST 
> from that with ASTReader from it's .clang_ast section, right? In that case 
> the AST should be complete with all type information. If this was the case 
> then I don't see why it is not possible to use clang::ASTImporter to import 
> templates and specializations, since we do exactly that in CTU.
> 
> Or do you guys create the AST from the debug info, from the .debug_info 
> section of .pcm module file? And this is why AST is incomplete? I've got this 
> from 
> https://youtu.be/EgkZ8PTNSHQ?list=PL85Cf-ok7HpppFWlp2wX_-A1dkyFVlaEB&t=1565
>  If this is the case, then comes my naiive quiestion: Why don't you use the 
> ASTReader?

There are no C++ modules involved in our use case beside the generic `std` 
module. No user-written code is read from a module and we don't have any PCM 
file beside the `std` module we build for the expression evaluator.

We use the ASTReader to directly read the `std` module contents into the 
expression evaluation ASTContext, but this doesn't give us the decls for the 
instantiations the user has made (e.g. `std::vector`). We only have these 
user instantiations in the 'normal' debug info where we have a rather minimal 
AST (again, no modules are not involved in building this debug info AST). The 
`InternalImport` in the LLDB patch just stitches the module AST and the debug 
info AST together when we import something that we also have (in better quality 
from the C++ module) in the target ASTContext.




Comment at: clang/unittests/AST/ASTImporterTest.cpp:581
+struct RedirectingImporter : public ASTImporter {
+  using ASTImporter::ASTImporter;
+  // Returns a ImporterConstructor that constructs this class.

balazske wrote:
> Is this `using` needed?
Yeah, otherwise we have to provide our own constructor that essentially just 
forwards to the ASTImporter constructor.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:588
+  bool MinimalImport, ASTImporterLookupTable *LookupTabl) {
+  return static_cast(
+  new RedirectingImporter(ToContext, ToFileManager, FromContext,

balazske wrote:
> The `static_cast` should not be needed.
Thanks!


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] D59466: [clang-tidy] openmp-exception-escape - a new check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 191870.
lebedev.ri marked 6 inline comments as done.
lebedev.ri added a comment.

Address some nits.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59466

Files:
  clang-tidy/openmp/CMakeLists.txt
  clang-tidy/openmp/ExceptionEscapeCheck.cpp
  clang-tidy/openmp/ExceptionEscapeCheck.h
  clang-tidy/openmp/OpenMPTidyModule.cpp
  clang-tidy/utils/ExceptionAnalyzer.cpp
  clang-tidy/utils/ExceptionAnalyzer.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/openmp-exception-escape.rst
  test/clang-tidy/bugprone-exception-escape-openmp.cpp
  test/clang-tidy/openmp-exception-escape.cpp

Index: test/clang-tidy/openmp-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/openmp-exception-escape.cpp
@@ -0,0 +1,126 @@
+// RUN: %check_clang_tidy %s openmp-exception-escape %t -- -extra-arg=-fopenmp=libomp -extra-arg=-fexceptions -config="{CheckOptions: [{key: openmp-exception-escape.IgnoredExceptions, value: 'ignored'}]}" --
+
+int thrower() {
+  throw 42;
+}
+
+class ignored {};
+namespace std {
+class bad_alloc {};
+} // namespace std
+
+void parallel() {
+#pragma omp parallel
+  thrower();
+  // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: An exception thrown inside of the OpenMP 'parallel' region is not caught in that same region.
+}
+
+void ignore() {
+#pragma omp parallel
+  throw ignored();
+}
+
+void standalone_directive() {
+#pragma omp taskwait
+  throw ignored(); // not structured block
+}
+
+void ignore_alloc() {
+#pragma omp parallel
+  throw std::bad_alloc();
+}
+
+void parallel_caught() {
+#pragma omp parallel
+  {
+try {
+  thrower();
+} catch (...) {
+}
+  }
+}
+
+void for_header(const int a) {
+  // Only the body of the loop counts.
+#pragma omp for
+  for (int i = 0; i < thrower(); i++)
+;
+}
+
+void forloop(const int a) {
+#pragma omp for
+  for (int i = 0; i < a; i++)
+thrower();
+  // CHECK-MESSAGES: :[[@LINE-3]]:9: warning: An exception thrown inside of the OpenMP 'for' region is not caught in that same region.
+}
+
+void parallel_forloop(const int a) {
+#pragma omp parallel
+  {
+#pragma omp for
+for (int i = 0; i < a; i++)
+  thrower();
+thrower();
+// CHECK-MESSAGES: :[[@LINE-6]]:9: warning: An exception thrown inside of the OpenMP 'parallel' region is not caught in that same region.
+// CHECK-MESSAGES: :[[@LINE-5]]:9: warning: An exception thrown inside of the OpenMP 'for' region is not caught in that same region.
+  }
+}
+
+void parallel_forloop_caught(const int a) {
+#pragma omp parallel
+  {
+#pragma omp for
+for (int i = 0; i < a; i++) {
+  try {
+thrower();
+  } catch (...) {
+  }
+}
+thrower();
+// CHECK-MESSAGES: :[[@LINE-10]]:9: warning: An exception thrown inside of the OpenMP 'parallel' region is not caught in that same region.
+  }
+}
+
+void parallel_caught_forloop(const int a) {
+#pragma omp parallel
+  {
+#pragma omp for
+for (int i = 0; i < a; i++)
+  thrower();
+try {
+  thrower();
+} catch (...) {
+}
+// CHECK-MESSAGES: :[[@LINE-7]]:9: warning: An exception thrown inside of the OpenMP 'for' region is not caught in that same region.
+  }
+}
+
+void parallel_outercaught_forloop(const int a) {
+#pragma omp parallel
+  {
+try {
+#pragma omp for
+  for (int i = 0; i < a; i++)
+thrower();
+  thrower();
+} catch (...) {
+}
+// CHECK-MESSAGES: :[[@LINE-6]]:9: warning: An exception thrown inside of the OpenMP 'for' region is not caught in that same region.
+  }
+}
+
+void parallel_outercaught_forloop_caught(const int a) {
+#pragma omp parallel
+  {
+try {
+#pragma omp for
+  for (int i = 0; i < a; i++) {
+try {
+  thrower();
+} catch (...) {
+}
+  }
+} catch (...) {
+}
+  }
+}
Index: test/clang-tidy/bugprone-exception-escape-openmp.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-exception-escape-openmp.cpp
@@ -0,0 +1,29 @@
+// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-fopenmp=libomp -extra-arg=-fexceptions --
+
+int thrower() {
+  throw 42;
+}
+
+void ok_parallel() {
+#pragma omp parallel
+  thrower();
+}
+
+void bad_for_header_XFAIL(const int a) noexcept {
+#pragma omp for
+  for (int i = 0; i < thrower(); i++)
+;
+  // FIXME: this really should be caught by bugprone-exception-escape.
+  // https://bugs.llvm.org/show_bug.cgi?id=41102
+}
+
+void ok_forloop(const int a) {
+#pragma omp for
+  for (int i = 0; i < a; i++)
+thrower();
+}
+
+void some_exception_just_so_that_check_clang_tidy_shuts_up() noexcept {
+  thrower();
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: an exception may be thrown in function 'some_exception_just_so_that_check_clang_tidy_shuts_up' which should not

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:53
+  Finder->addMatcher(ompExecutableDirective(
+ unless(isStandaloneDirective()),
+ hasStructuredBlock(stmt().bind("structured-block")))

gribozavr wrote:
> Do we need the `unless`?  `hasStructuredBlock()` just won't match standalone 
> directives.
*need*? no.
But it makes zero sense semantically to look for structured block
without first establishing that it has one.
Sure, we now check that twice (here, and in `hasStructuredBlock()`), but that 
is up to optimizer.

The fact that `hasStructuredBlock()` workarounds the assert in
`getStructuredBlock()` is only to avoid clang-query crashes,
it is spelled as much in the docs.



Comment at: docs/clang-tidy/checks/openmp-exception-escape.rst:21
+
+.. option:: IgnoredExceptions
+

gribozavr wrote:
> `IgnoredExceptionTypes`?
What do you mean?
In the check it's `Options.get("IgnoredExceptions", "")`.
That is identical to the check in bugprone.



Comment at: test/clang-tidy/bugprone-exception-escape-openmp.cpp:16
+;
+  // FIXME: this really should be caught by bugprone-exception-escape.
+  // https://bugs.llvm.org/show_bug.cgi?id=41102

gribozavr wrote:
> Shouldn't the function be marked with `noexcept` for 
> `bugprone-exception-escape` to catch it?  It does not know about OpenMP.
> 
> Are you suggesting that `bugprone-exception-escape` should subsume your new 
> OpenMP-specific check?
> Shouldn't the function be marked with noexcept for bugprone-exception-escape 
> to catch it?

Right. I meant `noexcept` (or did i drop it and forgot to put it back?).
But that changed nothing here, this is still an XFAIL.

> It does not know about OpenMP.
> Are you suggesting that bugprone-exception-escape should subsume your new 
> OpenMP-specific check?

Only the `;` is the structured-block here, so `thrower()` call *should* be 
caught by `bugprone-exception-escape`.


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] D59650: [NFC] ExceptionEscapeCheck: small refactoring

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 191868.
lebedev.ri marked 3 inline comments as done.
lebedev.ri added a comment.

Address some nits.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59650

Files:
  clang-tidy/utils/ExceptionAnalyzer.cpp
  clang-tidy/utils/ExceptionAnalyzer.h


Index: clang-tidy/utils/ExceptionAnalyzer.h
===
--- clang-tidy/utils/ExceptionAnalyzer.h
+++ clang-tidy/utils/ExceptionAnalyzer.h
@@ -138,10 +138,15 @@
   throwsException(const Stmt *St, const ExceptionInfo::Throwables &Caught,
   llvm::SmallSet &CallStack);
 
+  ExceptionInfo analyzeImpl(const FunctionDecl *Func);
+
+  template  ExceptionInfo analyzeDispatch(const T *Node);
+
   bool IgnoreBadAlloc = true;
   llvm::StringSet<> IgnoredExceptions;
   std::map FunctionCache;
 };
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
Index: clang-tidy/utils/ExceptionAnalyzer.cpp
===
--- clang-tidy/utils/ExceptionAnalyzer.cpp
+++ clang-tidy/utils/ExceptionAnalyzer.cpp
@@ -205,7 +205,7 @@
 }
 
 ExceptionAnalyzer::ExceptionInfo
-ExceptionAnalyzer::analyze(const FunctionDecl *Func) {
+ExceptionAnalyzer::analyzeImpl(const FunctionDecl *Func) {
   ExceptionInfo ExceptionList;
 
   // Check if the function has already been analyzed and reuse that result.
@@ -221,6 +221,14 @@
   } else
 ExceptionList = FunctionCache[Func];
 
+  return ExceptionList;
+}
+
+template 
+ExceptionAnalyzer::ExceptionInfo
+ExceptionAnalyzer::analyzeDispatch(const T *Node) {
+  ExceptionInfo ExceptionList = analyzeImpl(Node);
+
   if (ExceptionList.getBehaviour() == State::NotThrowing ||
   ExceptionList.getBehaviour() == State::Unknown)
 return ExceptionList;
@@ -231,6 +239,12 @@
 
   return ExceptionList;
 }
+
+ExceptionAnalyzer::ExceptionInfo
+ExceptionAnalyzer::analyze(const FunctionDecl *Func) {
+  return analyzeDispatch(Func);
+}
+
 } // namespace utils
 } // namespace tidy
 


Index: clang-tidy/utils/ExceptionAnalyzer.h
===
--- clang-tidy/utils/ExceptionAnalyzer.h
+++ clang-tidy/utils/ExceptionAnalyzer.h
@@ -138,10 +138,15 @@
   throwsException(const Stmt *St, const ExceptionInfo::Throwables &Caught,
   llvm::SmallSet &CallStack);
 
+  ExceptionInfo analyzeImpl(const FunctionDecl *Func);
+
+  template  ExceptionInfo analyzeDispatch(const T *Node);
+
   bool IgnoreBadAlloc = true;
   llvm::StringSet<> IgnoredExceptions;
   std::map FunctionCache;
 };
+
 } // namespace utils
 } // namespace tidy
 } // namespace clang
Index: clang-tidy/utils/ExceptionAnalyzer.cpp
===
--- clang-tidy/utils/ExceptionAnalyzer.cpp
+++ clang-tidy/utils/ExceptionAnalyzer.cpp
@@ -205,7 +205,7 @@
 }
 
 ExceptionAnalyzer::ExceptionInfo
-ExceptionAnalyzer::analyze(const FunctionDecl *Func) {
+ExceptionAnalyzer::analyzeImpl(const FunctionDecl *Func) {
   ExceptionInfo ExceptionList;
 
   // Check if the function has already been analyzed and reuse that result.
@@ -221,6 +221,14 @@
   } else
 ExceptionList = FunctionCache[Func];
 
+  return ExceptionList;
+}
+
+template 
+ExceptionAnalyzer::ExceptionInfo
+ExceptionAnalyzer::analyzeDispatch(const T *Node) {
+  ExceptionInfo ExceptionList = analyzeImpl(Node);
+
   if (ExceptionList.getBehaviour() == State::NotThrowing ||
   ExceptionList.getBehaviour() == State::Unknown)
 return ExceptionList;
@@ -231,6 +239,12 @@
 
   return ExceptionList;
 }
+
+ExceptionAnalyzer::ExceptionInfo
+ExceptionAnalyzer::analyze(const FunctionDecl *Func) {
+  return analyzeDispatch(Func);
+}
+
 } // namespace utils
 } // namespace tidy
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D57113: [clang-tidy] openmp-use-default-none - a new check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri updated this revision to Diff 191869.
lebedev.ri added a comment.

Address some nits.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57113

Files:
  clang-tidy/openmp/CMakeLists.txt
  clang-tidy/openmp/OpenMPTidyModule.cpp
  clang-tidy/openmp/UseDefaultNoneCheck.cpp
  clang-tidy/openmp/UseDefaultNoneCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/openmp-use-default-none.rst
  test/clang-tidy/openmp-use-default-none.cpp

Index: test/clang-tidy/openmp-use-default-none.cpp
===
--- /dev/null
+++ test/clang-tidy/openmp-use-default-none.cpp
@@ -0,0 +1,160 @@
+// RUN: %check_clang_tidy %s openmp-use-default-none %t -- -- -x c++ -fopenmp=libomp -fopenmp-version=40
+// RUN: %check_clang_tidy %s openmp-use-default-none %t -- -- -x c   -fopenmp=libomp -fopenmp-version=40
+
+////
+// Null cases.
+////
+
+// 'for' directive can not have 'default' clause, no diagnostics.
+void n0(const int a) {
+#pragma omp for
+  for (int b = 0; b < a; b++)
+;
+}
+
+////
+// Single-directive positive cases.
+////
+
+// 'parallel' directive.
+
+// 'parallel' directive can have 'default' clause, but said clause is not
+// specified, diagnosed.
+void p0_0() {
+#pragma omp parallel
+  ;
+  // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'parallel' does not specify 'default' clause. Consider specifying 'default(none)' clause.
+}
+
+// 'parallel' directive can have 'default' clause, and said clause is specified,
+// with 'none' kind, all good.
+void p0_1() {
+#pragma omp parallel default(none)
+  ;
+}
+
+// 'parallel' directive can have 'default' clause, and said clause is specified,
+// but with 'shared' kind, which is not 'none', diagnose.
+void p0_2() {
+#pragma omp parallel default(shared)
+  ;
+  // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'parallel' specifies 'default(shared)' clause. Consider using 'default(none)' clause instead.
+  // CHECK-NOTES: :[[@LINE-3]]:22: note: Existing 'default' clause is specified here.
+}
+
+// 'task' directive.
+
+// 'task' directive can have 'default' clause, but said clause is not
+// specified, diagnosed.
+void p1_0() {
+#pragma omp task
+  ;
+  // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'task' does not specify 'default' clause. Consider specifying 'default(none)' clause.
+}
+
+// 'task' directive can have 'default' clause, and said clause is specified,
+// with 'none' kind, all good.
+void p1_1() {
+#pragma omp task default(none)
+  ;
+}
+
+// 'task' directive can have 'default' clause, and said clause is specified,
+// but with 'shared' kind, which is not 'none', diagnose.
+void p1_2() {
+#pragma omp task default(shared)
+  ;
+  // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'task' specifies 'default(shared)' clause. Consider using 'default(none)' clause instead.
+  // CHECK-NOTES: :[[@LINE-3]]:18: note: Existing 'default' clause is specified here.
+}
+
+// 'teams' directive. (has to be inside of 'target' directive)
+
+// 'teams' directive can have 'default' clause, but said clause is not
+// specified, diagnosed.
+void p2_0() {
+#pragma omp target
+#pragma omp teams
+  ;
+  // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'teams' does not specify 'default' clause. Consider specifying 'default(none)' clause.
+}
+
+// 'teams' directive can have 'default' clause, and said clause is specified,
+// with 'none' kind, all good.
+void p2_1() {
+#pragma omp target
+#pragma omp teams default(none)
+  ;
+}
+
+// 'teams' directive can have 'default' clause, and said clause is specified,
+// but with 'shared' kind, which is not 'none', diagnose.
+void p2_2() {
+#pragma omp target
+#pragma omp teams default(shared)
+  ;
+  // CHECK-NOTES: :[[@LINE-2]]:9: warning: OpenMP directive 'teams' specifies 'default(shared)' clause. Consider using 'default(none)' clause instead.
+  // CHECK-NOTES: :[[@LINE-3]]:19: note: Existing 'default' clause is specified here.
+}
+
+// 'taskloop' directive.
+
+// 'taskloop' directive can have 'default' clause, but said clause is not
+// specified, diagnosed.
+void p3_0(const int a) {
+#pragma omp taskloop
+  for (int b = 0; b < a; b++)
+;
+  // CHECK-NOTES: :[[@LINE-3]]:9: warning: OpenMP directive 'taskloop' does not specify 'default' clause. Consider specifying 'default(none)' clause.
+}
+
+// 'taskloop' directive can have 'default' clause, and said clause is specified,
+// with 'none' kind, all good.
+void p3_1(const int a) {
+#pragma omp taskloop default(none) shared(a)
+  for (int b = 0; b < a; b++)
+;
+}
+
+// '

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/utils/ExceptionAnalyzer.cpp:226
+ExceptionAnalyzer::ExceptionInfo
+ExceptionAnalyzer::analyzeBoilerplate(const T *Node) {
+  ExceptionInfo ExceptionList;

gribozavr wrote:
> JonasToth wrote:
> > lebedev.ri wrote:
> > > Please bikeshed on the name. I don't think this one is good.
> > Hmm, `analyzeGeneric`, `analyzeGeneral`, `abstractAnalysis`, 
> > `analyzeAbstract`, something good in these?
> > 
> > Given its private its not too important either ;)
> I'd suggest to simplify by changing `analyzeBoilerplate()` into a 
> non-template, into this specifically:
> 
> ```
> ExceptionAnalyzer::ExceptionInfo 
> ExceptionAnalyzer::filterIgnoredExceptions(ExceptionInfo ExceptionList) {
> if (ExceptionList.getBehaviour() == State::NotThrowing ||
>   ExceptionList.getBehaviour() == State::Unknown)
> return ExceptionList;
> 
>   // Remove all ignored exceptions from the list of exceptions that can be
>   // thrown.
>   ExceptionList.filterIgnoredExceptions(IgnoredExceptions, IgnoreBadAlloc);
> 
>   return ExceptionList;
> }
> ```
> 
> And then call it in `analyze()`:
> 
> ```
> ExceptionAnalyzer::ExceptionInfo
> ExceptionAnalyzer::analyze(const FunctionDecl *Func) {
>   return filterIgnoredExceptions(analyzeImpl(Func));
> }
> ```
Hmm not really.
I intentionally did all this to maximally complicate any possibility of 
accidentally doing
something different given diferent entry point (`Stmt` vs `FunctionDecl`).
Refactoring it that way, via `filterIgnoredExceptions()` increases that risk 
back.
(accidentally omit that intermediate function, and ...)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59650



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

[PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh updated this revision to Diff 191871.
alexfh marked an inline comment as done.
alexfh added a comment.

- - Update a comment according to the review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59528

Files:
  clang-tools-extra/clang-tidy/CMakeLists.txt
  clang-tools-extra/clang-tidy/ClangTidy.cpp
  clang-tools-extra/clang-tidy/ClangTidy.h
  clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
  clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.h
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/test/CMakeLists.txt
  
clang-tools-extra/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/a.h
  
clang-tools-extra/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/b.h
  
clang-tools-extra/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/c.h
  
clang-tools-extra/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/module.modulemap
  clang-tools-extra/test/clang-tidy/expand-modular-headers-ppcallbacks.cpp

Index: clang-tools-extra/test/clang-tidy/expand-modular-headers-ppcallbacks.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/expand-modular-headers-ppcallbacks.cpp
@@ -0,0 +1,35 @@
+// Sanity-check. Run without modules:
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cp %S/Inputs/expand-modular-headers-ppcallbacks/* %t/
+// RUN: %check_clang_tidy %s readability-identifier-naming %t/without-modules -- \
+// RUN:   -config="CheckOptions: [{ \
+// RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
+// RUN:   -header-filter=.* \
+// RUN:   -- -x c++ -std=c++11 -I%t/
+//
+// Run clang-tidy on a file with modular includes:
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cp %S/Inputs/expand-modular-headers-ppcallbacks/* %t/
+// RUN: %check_clang_tidy %s readability-identifier-naming %t/with-modules -- \
+// RUN:   -config="CheckOptions: [{ \
+// RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
+// RUN:   -header-filter=.* \
+// RUN:   -- -x c++ -std=c++11 -I%t/ \
+// RUN:   -fmodules -fimplicit-modules -fno-implicit-module-maps \
+// RUN:   -fmodule-map-file=%t/module.modulemap \
+// RUN:   -fmodules-cache-path=%t/module-cache/
+#include "c.h"
+
+// CHECK-MESSAGES: a.h:1:9: warning: invalid case style for macro definition 'a' [readability-identifier-naming]
+// CHECK-MESSAGES: a.h:1:9: note: FIX-IT applied suggested code changes
+// CHECK-MESSAGES: b.h:2:9: warning: invalid case style for macro definition 'b'
+// CHECK-MESSAGES: b.h:2:9: note: FIX-IT applied suggested code changes
+// CHECK-MESSAGES: c.h:2:9: warning: invalid case style for macro definition 'c'
+// CHECK-MESSAGES: c.h:2:9: note: FIX-IT applied suggested code changes
+
+#define m
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for macro definition 'm'
+// CHECK-MESSAGES: :[[@LINE-2]]:9: note: FIX-IT applied suggested code changes
Index: clang-tools-extra/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/module.modulemap
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/module.modulemap
@@ -0,0 +1,3 @@
+module a { header "a.h" export * }
+module b { header "b.h" export * use a }
+module c { header "c.h" export * use b }
Index: clang-tools-extra/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/c.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/c.h
@@ -0,0 +1,2 @@
+#include "b.h"
+#define c
Index: clang-tools-extra/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/b.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/b.h
@@ -0,0 +1,2 @@
+#include "a.h"
+#define b
Index: clang-tools-extra/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/a.h
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/a.h
@@ -0,0 +1 @@
+#define a
Index: clang-tools-extra/test/CMakeLists.txt
===
--- clang-tools-extra/test/CMakeLists.txt
+++ clang-tools-extra/test/CMakeLists.txt
@@ -62,6 +62,8 @@
   clang-resource-headers
 
   clang-tidy
+  # Clang-tidy tests need clang for building modules.
+  clang
 )
 
 if(CLANGD_BUILD_XPC_SUPPORT)
Index: clang-tools-extra/clang-tidy/tool/ClangTidyMain.cpp
===
--- clang-

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler updated this revision to Diff 191872.
rdwampler added a comment.

Rebased on master.


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

https://reviews.llvm.org/D57687

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/FormatToken.h
  clang/lib/Format/TokenAnnotator.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -12239,6 +12239,43 @@
   "> {\n"
   "  //\n"
   "});");
+
+  FormatStyle DoNotMerge = getLLVMStyle();
+  DoNotMerge.AllowShortLambdasOnASingleLine = FormatStyle::SLS_None;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() { return b; };", DoNotMerge);
+  verifyFormat("auto c = []() {\n"
+   "};",
+   " auto c = []() {};", DoNotMerge);
+
+  FormatStyle MergeEmptyOnly = getLLVMStyle();
+  MergeEmptyOnly.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Empty;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() {\n"
+   "  return b;\n"
+   " };",
+   MergeEmptyOnly);
+  verifyFormat("auto c = []() {};",
+   "auto c = []() {\n"
+   "};",
+   MergeEmptyOnly);
+
+  FormatStyle MergeInline = getLLVMStyle();
+  MergeInline.AllowShortLambdasOnASingleLine = FormatStyle::SLS_Inline;
+  verifyFormat("auto c = []() {\n"
+   "  return b;\n"
+   "};",
+   "auto c = []() { return b; };", MergeInline);
+  verifyFormat("function([]() { return b; })", "function([]() { return b; })",
+   MergeInline);
+  verifyFormat("function([]() { return b; }, a)",
+   "function([]() { return b; }, a)", MergeInline);
+  verifyFormat("function(a, []() { return b; })",
+   "function(a, []() { return b; })", MergeInline);
 }
 
 TEST_F(FormatTest, EmptyLinesInLambdas) {
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1475,6 +1475,7 @@
   return true;
 }
   }
+  FormatTok->Type = TT_LambdaLBrace;
   LSquare.Type = TT_LambdaLSquare;
   parseChildBlock();
   return true;
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -1191,11 +1191,11 @@
 
 // Reset token type in case we have already looked at it and then
 // recovered from an error (e.g. failure to find the matching >).
-if (!CurrentToken->isOneOf(TT_LambdaLSquare, TT_ForEachMacro,
-   TT_FunctionLBrace, TT_ImplicitStringLiteral,
-   TT_InlineASMBrace, TT_JsFatArrow, TT_LambdaArrow,
-   TT_OverloadedOperator, TT_RegexLiteral,
-   TT_TemplateString, TT_ObjCStringLiteral))
+if (!CurrentToken->isOneOf(
+TT_LambdaLSquare, TT_LambdaLBrace, TT_ForEachMacro,
+TT_FunctionLBrace, TT_ImplicitStringLiteral, TT_InlineASMBrace,
+TT_JsFatArrow, TT_LambdaArrow, TT_OverloadedOperator,
+TT_RegexLiteral, TT_TemplateString, TT_ObjCStringLiteral))
   CurrentToken->Type = TT_Unknown;
 CurrentToken->Role.reset();
 CurrentToken->MatchingParen = nullptr;
@@ -2896,7 +2896,7 @@
 // Returns 'true' if 'Tok' is a brace we'd want to break before in Allman style.
 static bool isAllmanBrace(const FormatToken &Tok) {
   return Tok.is(tok::l_brace) && Tok.BlockKind == BK_Block &&
- !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral);
+ !Tok.isOneOf(TT_ObjCBlockLBrace, TT_LambdaLBrace, TT_DictLiteral);
 }
 
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
@@ -3024,6 +3024,19 @@
   if (Left.is(TT_ObjCBlockLBrace) && !Style.AllowShortBlocksOnASingleLine)
 return true;
 
+  if (Left.is(TT_LambdaLBrace)) {
+if (Left.MatchingParen && Left.MatchingParen->Next &&
+Left.MatchingParen->Next->isOneOf(tok::comma, tok::r_paren) &&
+Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline)
+  return false;
+
+if (Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_None ||
+Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Inline ||
+(!Left.Children.empty() &&
+ Style.AllowShortLambdasOnASingleLine == FormatStyle::SLS_Empty))
+  return true;
+  }
+
   // Put multiple C# attributes on a new line.
   if (Style

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

Additionally I think that you need to:

- add tests for templates
- handle the ternary operator

Also, as per the coding guidelines, you need to fix:

- spelling of variables, eg `hint` -> `Hint`.
- run `clang-format` on your patch.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8164
+def err_must_appear_after_branch : Error<
+  "%0 can only appear after a selection or iteration statement">;
+def warn_attribute_already_present : Warning<

I don't think that this is right. I don't see this restriction in the 
specification. A warning should definitely be emitted when the attribute is 
ignored, but I believe that an error is inappropriate.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8170
+def err_contradictory_attribute : Warning<
+  "%0 contradicing with previous attribute">;
+

This should be `warn_contradictory_attribute`, and I am not seeing tests for 
this.



Comment at: clang/lib/CodeGen/CodeGenFunction.h:4022
+
+  static BranchHint getBranchHint(const Stmt* S, bool Invert = false);
+

Doc ?



Comment at: clang/lib/Parse/ParseStmt.cpp:1309
+}
+
 // Pop the 'else' scope if needed.

I don't think that this should be done here. The natural place to me seems to 
be in `ActOnIfStmt`. Additionally I think that you should wrap this in a static 
helper function.



Comment at: clang/lib/Sema/SemaStmtAttr.cpp:232
   for (const auto *I : Attrs) {
+
+if (isa(I)) {

A comment indicating what this is doing ?


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

https://reviews.llvm.org/D59467



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

r356749 - [AST] OMPStructuredBlockTest: avoid using multiline string literals in macros

[Roman Lebedev via cfe-commits]

Author: lebedevri
Date: Fri Mar 22 06:40:36 2019
New Revision: 356749

URL: http://llvm.org/viewvc/llvm-project?rev=356749&view=rev
Log:
[AST] OMPStructuredBlockTest: avoid using multiline string literals in macros

That is what i have been doing elsewhere in these tests, maybe that's it?

Maybe this helps with failing builds:
http://lab.llvm.org:8011/builders/clang-cmake-aarch64-quick/builds/17921
http://lab.llvm.org:8011/builders/clang-cmake-aarch64-global-isel/builds/10248

Modified:
cfe/trunk/unittests/AST/OMPStructuredBlockTest.cpp

Modified: cfe/trunk/unittests/AST/OMPStructuredBlockTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/OMPStructuredBlockTest.cpp?rev=356749&r1=356748&r2=356749&view=diff
==
--- cfe/trunk/unittests/AST/OMPStructuredBlockTest.cpp (original)
+++ cfe/trunk/unittests/AST/OMPStructuredBlockTest.cpp Fri Mar 22 06:40:36 2019
@@ -102,11 +102,12 @@ void test() {
 #pragma omp cancel parallel
 }
 })";
-  ASSERT_TRUE(
-  PrintedOMPStmtMatches(Source, OMPInnermostStructuredBlockMatcher(), R"({
+  const char *Expected = R"({
 #pragma omp cancel parallel
 }
-)"));
+)";
+  ASSERT_TRUE(PrintedOMPStmtMatches(
+  Source, OMPInnermostStructuredBlockMatcher(), Expected));
   ASSERT_TRUE(PrintedOMPStmtMatches(Source, OMPStandaloneDirectiveMatcher(),
 "#pragma omp cancel parallel\n"));
 }
@@ -117,14 +118,15 @@ TEST(OMPStructuredBlock, TestCancellatio
 void test() {
 #pragma omp parallel
 {
-#pragma omp cancellation point parallel
+#pragma omp cancellation point parallel
 }
 })";
-  ASSERT_TRUE(
-  PrintedOMPStmtMatches(Source, OMPInnermostStructuredBlockMatcher(), R"({
+  const char *Expected = R"({
 #pragma omp cancellation point parallel
 }
-)"));
+)";
+  ASSERT_TRUE(PrintedOMPStmtMatches(
+  Source, OMPInnermostStructuredBlockMatcher(), Expected));
   ASSERT_TRUE(
   PrintedOMPStmtMatches(Source, OMPStandaloneDirectiveMatcher(),
 "#pragma omp cancellation point parallel\n"));


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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added inline comments.



Comment at: clang/lib/Sema/SemaStmtAttr.cpp:74
+ ControlScope->getFlags() & Scope::FnTryCatchScope)
+  S.Diag(A.getLoc(), diag::err_must_appear_after_branch) << A.getName();
+  }

I think that you need to test for each of the above.


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

https://reviews.llvm.org/D59467



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

[clang-tools-extra] r356750 - [clang-tidy] Expand modular headers for PPCallbacks

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Fri Mar 22 06:42:48 2019
New Revision: 356750

URL: http://llvm.org/viewvc/llvm-project?rev=356750&view=rev
Log:
[clang-tidy] Expand modular headers for PPCallbacks

Summary:
Add a way to expand modular headers for PPCallbacks. Checks can opt-in for this
expansion by overriding the new registerPPCallbacks virtual method and
registering their PPCallbacks in the preprocessor created for this specific
purpose.

Use module expansion in the readability-identifier-naming check

Reviewers: gribozavr, usaxena95, sammccall

Reviewed By: gribozavr

Subscribers: nemanjai, mgorny, xazax.hun, kbarton, jdoerfert, cfe-commits

Tags: #clang, #clang-tools-extra

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

Added:
clang-tools-extra/trunk/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
clang-tools-extra/trunk/clang-tidy/ExpandModularHeadersPPCallbacks.h

clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/

clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/a.h

clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/b.h

clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/c.h

clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/module.modulemap

clang-tools-extra/trunk/test/clang-tidy/expand-modular-headers-ppcallbacks.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
clang-tools-extra/trunk/clang-tidy/ClangTidy.h
clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.h
clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp
clang-tools-extra/trunk/test/CMakeLists.txt

Modified: clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/CMakeLists.txt?rev=356750&r1=356749&r2=356750&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/CMakeLists.txt Fri Mar 22 06:42:48 2019
@@ -8,6 +8,7 @@ add_clang_library(clangTidy
   ClangTidyDiagnosticConsumer.cpp
   ClangTidyOptions.cpp
   ClangTidyProfiling.cpp
+  ExpandModularHeadersPPCallbacks.cpp
 
   DEPENDS
   ClangSACheckers

Modified: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp?rev=356750&r1=356749&r2=356750&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp Fri Mar 22 06:42:48 2019
@@ -18,6 +18,7 @@
 #include "ClangTidyDiagnosticConsumer.h"
 #include "ClangTidyModuleRegistry.h"
 #include "ClangTidyProfiling.h"
+#include "ExpandModularHeadersPPCallbacks.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
@@ -290,8 +291,10 @@ private:
 } // namespace
 
 ClangTidyASTConsumerFactory::ClangTidyASTConsumerFactory(
-ClangTidyContext &Context)
-: Context(Context), CheckFactories(new ClangTidyCheckFactories) {
+ClangTidyContext &Context,
+IntrusiveRefCntPtr OverlayFS)
+: Context(Context), OverlayFS(OverlayFS),
+  CheckFactories(new ClangTidyCheckFactories) {
   for (ClangTidyModuleRegistry::iterator I = ClangTidyModuleRegistry::begin(),
  E = ClangTidyModuleRegistry::end();
I != E; ++I) {
@@ -351,7 +354,8 @@ ClangTidyASTConsumerFactory::CreateASTCo
 clang::CompilerInstance &Compiler, StringRef File) {
   // FIXME: Move this to a separate method, so that CreateASTConsumer doesn't
   // modify Compiler.
-  Context.setSourceManager(&Compiler.getSourceManager());
+  SourceManager *SM = &Compiler.getSourceManager();
+  Context.setSourceManager(SM);
   Context.setCurrentFile(File);
   Context.setASTContext(&Compiler.getASTContext());
 
@@ -377,9 +381,20 @@ ClangTidyASTConsumerFactory::CreateASTCo
   std::unique_ptr Finder(
   new ast_matchers::MatchFinder(std::move(FinderOptions)));
 
+  Preprocessor *PP = &Compiler.getPreprocessor();
+  Preprocessor *ModuleExpanderPP = PP;
+
+  if (Context.getLangOpts().Modules && OverlayFS != nullptr) {
+auto ModuleExpander = llvm::make_unique(
+&Compiler, OverlayFS);
+ModuleExpanderPP = ModuleExpander->getPreprocessor();
+PP->addPPCallbacks(std::move(ModuleExpander));
+  }
+
   for (auto &Check : Checks) {
 Check->registerMatchers(&*Finder);
 Check->registerPPCallbacks(Compiler);
+Check->registerPPCallbacks(*SM, PP, ModuleExpanderPP);
   }
 
   std::vector> Consumers;
@@ -505,7 +520,7 @@ std::vector
 runClangTidy(clang::tidy::ClangTidyContext &Context,
   

[PATCH] D59528: [clang-tidy] Expand modular headers for PPCallbacks

[Alexander Kornienko via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
alexfh marked an inline comment as done.
Closed by commit rL356750: [clang-tidy] Expand modular headers for PPCallbacks 
(authored by alexfh, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D59528?vs=191871&id=191873#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D59528

Files:
  clang-tools-extra/trunk/clang-tidy/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
  clang-tools-extra/trunk/clang-tidy/ClangTidy.h
  clang-tools-extra/trunk/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
  clang-tools-extra/trunk/clang-tidy/ExpandModularHeadersPPCallbacks.h
  clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/trunk/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/trunk/clang-tidy/tool/ClangTidyMain.cpp
  clang-tools-extra/trunk/test/CMakeLists.txt
  
clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/a.h
  
clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/b.h
  
clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/c.h
  
clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/module.modulemap
  clang-tools-extra/trunk/test/clang-tidy/expand-modular-headers-ppcallbacks.cpp

Index: clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/c.h
===
--- clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/c.h
+++ clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/c.h
@@ -0,0 +1,2 @@
+#include "b.h"
+#define c
Index: clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/module.modulemap
===
--- clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/module.modulemap
+++ clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/module.modulemap
@@ -0,0 +1,3 @@
+module a { header "a.h" export * }
+module b { header "b.h" export * use a }
+module c { header "c.h" export * use b }
Index: clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/b.h
===
--- clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/b.h
+++ clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/b.h
@@ -0,0 +1,2 @@
+#include "a.h"
+#define b
Index: clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/a.h
===
--- clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/a.h
+++ clang-tools-extra/trunk/test/clang-tidy/Inputs/expand-modular-headers-ppcallbacks/a.h
@@ -0,0 +1 @@
+#define a
Index: clang-tools-extra/trunk/test/clang-tidy/expand-modular-headers-ppcallbacks.cpp
===
--- clang-tools-extra/trunk/test/clang-tidy/expand-modular-headers-ppcallbacks.cpp
+++ clang-tools-extra/trunk/test/clang-tidy/expand-modular-headers-ppcallbacks.cpp
@@ -0,0 +1,35 @@
+// Sanity-check. Run without modules:
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cp %S/Inputs/expand-modular-headers-ppcallbacks/* %t/
+// RUN: %check_clang_tidy %s readability-identifier-naming %t/without-modules -- \
+// RUN:   -config="CheckOptions: [{ \
+// RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
+// RUN:   -header-filter=.* \
+// RUN:   -- -x c++ -std=c++11 -I%t/
+//
+// Run clang-tidy on a file with modular includes:
+//
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: cp %S/Inputs/expand-modular-headers-ppcallbacks/* %t/
+// RUN: %check_clang_tidy %s readability-identifier-naming %t/with-modules -- \
+// RUN:   -config="CheckOptions: [{ \
+// RUN:  key: readability-identifier-naming.MacroDefinitionCase, value: UPPER_CASE }]" \
+// RUN:   -header-filter=.* \
+// RUN:   -- -x c++ -std=c++11 -I%t/ \
+// RUN:   -fmodules -fimplicit-modules -fno-implicit-module-maps \
+// RUN:   -fmodule-map-file=%t/module.modulemap \
+// RUN:   -fmodules-cache-path=%t/module-cache/
+#include "c.h"
+
+// CHECK-MESSAGES: a.h:1:9: warning: invalid case style for macro definition 'a' [readability-identifier-naming]
+// CHECK-MESSAGES: a.h:1:9: note: FIX-IT applied suggested code changes
+// CHECK-MESSAGES: b.h:2:9: warning: invalid case style for macro definition 'b'
+// CHECK-MESSAGES: b.h:2:9: note: FIX-IT applied suggested code changes
+// CHECK-MESSAGES: c.h:2:9: warning: invalid case style for macro definition 'c'
+// CHECK-MESS

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D59076#1438479 , @riccibruno wrote:

> This is causing https://bugs.llvm.org/show_bug.cgi?id=41171.


@modocache Could you take a look please ? Nested scopes in a catch statement 
are completely broken because of this patch.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59076



___
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

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr accepted this revision.
gribozavr marked an inline comment as done.
gribozavr added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:53
+  Finder->addMatcher(ompExecutableDirective(
+ unless(isStandaloneDirective()),
+ hasStructuredBlock(stmt().bind("structured-block")))

lebedev.ri wrote:
> gribozavr wrote:
> > Do we need the `unless`?  `hasStructuredBlock()` just won't match 
> > standalone directives.
> *need*? no.
> But it makes zero sense semantically to look for structured block
> without first establishing that it has one.
> Sure, we now check that twice (here, and in `hasStructuredBlock()`), but that 
> is up to optimizer.
> 
> The fact that `hasStructuredBlock()` workarounds the assert in
> `getStructuredBlock()` is only to avoid clang-query crashes,
> it is spelled as much in the docs.
> But it makes zero sense semantically to look for structured block without 
> first establishing that it has one.

IDK, in my mind it makes sense.  "Does a standalone directive have a structured 
block?  No." is a coherent logical chain.



Comment at: test/clang-tidy/bugprone-exception-escape-openmp.cpp:16
+;
+  // FIXME: this really should be caught by bugprone-exception-escape.
+  // https://bugs.llvm.org/show_bug.cgi?id=41102

lebedev.ri wrote:
> gribozavr wrote:
> > Shouldn't the function be marked with `noexcept` for 
> > `bugprone-exception-escape` to catch it?  It does not know about OpenMP.
> > 
> > Are you suggesting that `bugprone-exception-escape` should subsume your new 
> > OpenMP-specific check?
> > Shouldn't the function be marked with noexcept for 
> > bugprone-exception-escape to catch it?
> 
> Right. I meant `noexcept` (or did i drop it and forgot to put it back?).
> But that changed nothing here, this is still an XFAIL.
> 
> > It does not know about OpenMP.
> > Are you suggesting that bugprone-exception-escape should subsume your new 
> > OpenMP-specific check?
> 
> Only the `;` is the structured-block here, so `thrower()` call *should* be 
> caught by `bugprone-exception-escape`.
Ah, I see.


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] D59639: [clangd] Print template arguments helper

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: clang-tools-extra/clangd/AST.cpp:26
+llvm::Optional>
+getTemplateSpecializationArgLocs(const NamedDecl &ND) {
+  if (auto *Func = llvm::dyn_cast(&ND)) {

ilya-biryukov wrote:
> Eugene.Zelenko wrote:
> > Functions should be static, not in anonymous namespace. See LLVM Coding 
> > Guidelines.
> We tend to put internal functions to anonymous namespace quite a lot in 
> clangd.
> While technically a small violation of the LLVM style guide, this aligns with 
> the rest of the code and we don't consider that to be a problem.
I don't think it's reasonable to have one style per project. Even if clangd has 
such historical code, it'll be good idea to change it to confirm to common 
guidelines. Evolution of LLDB formatting style is good example.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59639



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

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr accepted this revision.
gribozavr marked an inline comment as done.
gribozavr added inline comments.
This revision is now accepted and ready to land.



Comment at: clang-tidy/utils/ExceptionAnalyzer.cpp:226
+ExceptionAnalyzer::ExceptionInfo
+ExceptionAnalyzer::analyzeBoilerplate(const T *Node) {
+  ExceptionInfo ExceptionList;

lebedev.ri wrote:
> gribozavr wrote:
> > JonasToth wrote:
> > > lebedev.ri wrote:
> > > > Please bikeshed on the name. I don't think this one is good.
> > > Hmm, `analyzeGeneric`, `analyzeGeneral`, `abstractAnalysis`, 
> > > `analyzeAbstract`, something good in these?
> > > 
> > > Given its private its not too important either ;)
> > I'd suggest to simplify by changing `analyzeBoilerplate()` into a 
> > non-template, into this specifically:
> > 
> > ```
> > ExceptionAnalyzer::ExceptionInfo 
> > ExceptionAnalyzer::filterIgnoredExceptions(ExceptionInfo ExceptionList) {
> > if (ExceptionList.getBehaviour() == State::NotThrowing ||
> >   ExceptionList.getBehaviour() == State::Unknown)
> > return ExceptionList;
> > 
> >   // Remove all ignored exceptions from the list of exceptions that can be
> >   // thrown.
> >   ExceptionList.filterIgnoredExceptions(IgnoredExceptions, IgnoreBadAlloc);
> > 
> >   return ExceptionList;
> > }
> > ```
> > 
> > And then call it in `analyze()`:
> > 
> > ```
> > ExceptionAnalyzer::ExceptionInfo
> > ExceptionAnalyzer::analyze(const FunctionDecl *Func) {
> >   return filterIgnoredExceptions(analyzeImpl(Func));
> > }
> > ```
> Hmm not really.
> I intentionally did all this to maximally complicate any possibility of 
> accidentally doing
> something different given diferent entry point (`Stmt` vs `FunctionDecl`).
> Refactoring it that way, via `filterIgnoredExceptions()` increases that risk 
> back.
> (accidentally omit that intermediate function, and ...)
> (accidentally omit that intermediate function, and ...)

... and tests should catch it.  No big drama.

Anyway, it is not as important.  I do think however that complicating the code 
this way is not worth the benefit.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59650



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

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added a comment.

@gribozavr thank you for the review!

@baloghadamsoftware any comments?




Comment at: clang-tidy/utils/ExceptionAnalyzer.cpp:226
+ExceptionAnalyzer::ExceptionInfo
+ExceptionAnalyzer::analyzeBoilerplate(const T *Node) {
+  ExceptionInfo ExceptionList;

gribozavr wrote:
> lebedev.ri wrote:
> > gribozavr wrote:
> > > JonasToth wrote:
> > > > lebedev.ri wrote:
> > > > > Please bikeshed on the name. I don't think this one is good.
> > > > Hmm, `analyzeGeneric`, `analyzeGeneral`, `abstractAnalysis`, 
> > > > `analyzeAbstract`, something good in these?
> > > > 
> > > > Given its private its not too important either ;)
> > > I'd suggest to simplify by changing `analyzeBoilerplate()` into a 
> > > non-template, into this specifically:
> > > 
> > > ```
> > > ExceptionAnalyzer::ExceptionInfo 
> > > ExceptionAnalyzer::filterIgnoredExceptions(ExceptionInfo ExceptionList) {
> > > if (ExceptionList.getBehaviour() == State::NotThrowing ||
> > >   ExceptionList.getBehaviour() == State::Unknown)
> > > return ExceptionList;
> > > 
> > >   // Remove all ignored exceptions from the list of exceptions that can be
> > >   // thrown.
> > >   ExceptionList.filterIgnoredExceptions(IgnoredExceptions, 
> > > IgnoreBadAlloc);
> > > 
> > >   return ExceptionList;
> > > }
> > > ```
> > > 
> > > And then call it in `analyze()`:
> > > 
> > > ```
> > > ExceptionAnalyzer::ExceptionInfo
> > > ExceptionAnalyzer::analyze(const FunctionDecl *Func) {
> > >   return filterIgnoredExceptions(analyzeImpl(Func));
> > > }
> > > ```
> > Hmm not really.
> > I intentionally did all this to maximally complicate any possibility of 
> > accidentally doing
> > something different given diferent entry point (`Stmt` vs `FunctionDecl`).
> > Refactoring it that way, via `filterIgnoredExceptions()` increases that 
> > risk back.
> > (accidentally omit that intermediate function, and ...)
> > (accidentally omit that intermediate function, and ...)
> 
> ... and tests should catch it.  No big drama.
> 
> Anyway, it is not as important.  I do think however that complicating the 
> code this way is not worth the benefit.
That is kind of the point. The test would catch it if they would already exist.
If a new entry point is being added, the tests wouldn't be there yet.
This enforces that every entry point behaves the same way.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59650



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

Hmm, it seems that an attribute is not allowed by the grammar in the 
`expression` or `assignment-expression` of a `conditional-expression`. Was that 
intended when `[[likely]]` was added ?


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

https://reviews.llvm.org/D59467



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a reviewer: rsmith.
aaron.ballman added a comment.

Adding Richard for design strategy discussion.




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8164
+def err_must_appear_after_branch : Error<
+  "%0 can only appear after a selection or iteration statement">;
+def warn_attribute_already_present : Warning<

riccibruno wrote:
> I don't think that this is right. I don't see this restriction in the 
> specification. A warning should definitely be emitted when the attribute is 
> ignored, but I believe that an error is inappropriate.
> I don't think that this is right. I don't see this restriction in the 
> specification. A warning should definitely be emitted when the attribute is 
> ignored, but I believe that an error is inappropriate.

Agreed.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8165
+  "%0 can only appear after a selection or iteration statement">;
+def warn_attribute_already_present : Warning<
+  "was already marked %0">;

This diagnostic isn't needed -- `warn_duplicate_attribute_exact` will cover it.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8167
+  "was already marked %0">;
+def err_mutuably_exclusive_likelihood : Error<
+  "%0 and %1 are mutually exclusive">;

This diagnostic is not needed -- `err_attributes_are_not_compatible` will cover 
it.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:8170
+def err_contradictory_attribute : Warning<
+  "%0 contradicing with previous attribute">;
+

riccibruno wrote:
> This should be `warn_contradictory_attribute`, and I am not seeing tests for 
> this.
This note also isn't needed. You should use `note_conflicting_attribute` 
instead.



Comment at: clang/lib/Sema/SemaStmtAttr.cpp:64-75
+  if (CurScope) {
+Scope* ControlScope = CurScope->getParent();
+if (!ControlScope ||
+!(ControlScope->getFlags() & Scope::ControlScope ||
+  ControlScope->getFlags() & Scope::BreakScope) ||
+CurScope->getFlags() & Scope::CompoundStmtScope ||
+ ControlScope->getFlags() & Scope::SEHExceptScope ||

This is incorrect -- nothing in the standard requires the attribute to appear 
after a branch statement. I'm not even 100% convinced that this should map 
directly to `__builtin_expect` in all cases (though it certainly would for 
conditional branches), because that doesn't help with things like catch 
handlers or labels.



Comment at: clang/test/SemaCXX/cxx2a-likelihood-attr.cpp:52
+}
\ No newline at end of file


Be sure to add the newline, please.


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

https://reviews.llvm.org/D59467



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D59467#1439473 , @riccibruno wrote:

> Hmm, it seems that an attribute is not allowed by the grammar in the 
> `expression` or `assignment-expression` of a `conditional-expression`. Was 
> that intended when `[[likely]]` was added ?


Attributes do not appertain to expressions, so yes, that was intentional when 
we added `likely`. (You can have an attribute applied to an expression 
statement, but that's at a level that doesn't seem too useful for these 
attributes.)


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

https://reviews.llvm.org/D59467



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Bruno Ricci via Phabricator via cfe-commits]

riccibruno added a comment.

In D59467#1439485 , @aaron.ballman 
wrote:

> In D59467#1439473 , @riccibruno 
> wrote:
>
> > Hmm, it seems that an attribute is not allowed by the grammar in the 
> > `expression` or `assignment-expression` of a `conditional-expression`. Was 
> > that intended when `[[likely]]` was added ?
>
>
> Attributes do not appertain to expressions, so yes, that was intentional when 
> we added `likely`. (You can have an attribute applied to an expression 
> statement, but that's at a level that doesn't seem too useful for these 
> attributes.)


Good to know, thanks !


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

https://reviews.llvm.org/D59467



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

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Manuel Klimek via Phabricator via cfe-commits]

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

LG


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

https://reviews.llvm.org/D57687



___
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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:32
+  llvm::StringSet<> IgnoredExceptions;
+  StringRef(RawIgnoredExceptions).split(IgnoredExceptionsVec, ",", -1, false);
+  IgnoredExceptions.insert(IgnoredExceptionsVec.begin(),

Do you need to trim any of the split strings?



Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:48
+return;
+  // Similarly, if C++ Exceptions are not enabled, nothing to do.
+  if (!getLangOpts().CPlusPlus || !getLangOpts().CXXExceptions)

Do you have to worry about SEH exceptions in C for this functionality?



Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:72
+  // FIXME: We should provide more information about the exact location where
+  // the exception is thrown, maybe the full path the exception escapes
+

Missing a full stop at the end of the comment.



Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:75-76
+  diag(Directive->getBeginLoc(),
+   "An exception thrown inside of the OpenMP '%0' region is not caught in "
+   "that same region.")
+  << getOpenMPDirectiveName(Directive->getDirectiveKind());

Clang-tidy diagnostics are not complete sentences -- please make this horribly 
ungrammatical. ;-)



Comment at: docs/clang-tidy/checks/openmp-exception-escape.rst:9
+
+As per the OpenMP specification, structured block is an executable statement,
+possibly compound, with a single entry at the top and a single exit at the

, structured block -> , a structured block



Comment at: docs/clang-tidy/checks/openmp-exception-escape.rst:10-11
+As per the OpenMP specification, structured block is an executable statement,
+possibly compound, with a single entry at the top and a single exit at the
+bottom. Which means, ``throw`` may not be used to to 'exit' out of the
+structured block. If an exception is not caught in the same structured block

Does this mean setjmp/longjmp out of the block is also a dangerous activity? 
What about gotos?



Comment at: docs/clang-tidy/checks/openmp-exception-escape.rst:13
+structured block. If an exception is not caught in the same structured block
+it was thrown in, the behaviour is undefined / implementation defined,
+the program will likely terminate.

Which is it -- undefined or implementation-defined?



Comment at: docs/clang-tidy/checks/openmp-exception-escape.rst:23
+
+   Comma separated list containing type names which are not counted as thrown
+   exceptions in the check. Default value is an empty string.

Comma separated -> Comma-separated


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] D57113: [clang-tidy] openmp-use-default-none - a new check

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/openmp/UseDefaultNoneCheck.cpp:47-48
+diag(Directive->getBeginLoc(),
+ "OpenMP directive '%0' specifies 'default(%1)' clause. Consider using 
"
+ "'default(none)' clause instead.")
+<< getOpenMPDirectiveName(Directive->getDirectiveKind())

Make the diagnostic ungrammatical.



Comment at: clang-tidy/openmp/UseDefaultNoneCheck.cpp:52
+ Clause->getDefaultKind());
+diag(Clause->getBeginLoc(), "Existing 'default' clause is specified here.",
+ DiagnosticIDs::Note);

Same with notes.



Comment at: clang-tidy/openmp/UseDefaultNoneCheck.cpp:58-59
+  diag(Directive->getBeginLoc(),
+   "OpenMP directive '%0' does not specify 'default' clause. Consider "
+   "specifying 'default(none)' clause.")
+  << getOpenMPDirectiveName(Directive->getDirectiveKind());

Here too.



Comment at: docs/clang-tidy/checks/openmp-use-default-none.rst:19
+
+.. code-block:: c++
+

This is a *lot* of example text -- are you sure all of it adds value, or can 
some of it be removed?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57113



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

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang-tidy/utils/ExceptionAnalyzer.cpp:226
+ExceptionAnalyzer::ExceptionInfo
+ExceptionAnalyzer::analyzeBoilerplate(const T *Node) {
+  ExceptionInfo ExceptionList;

lebedev.ri wrote:
> gribozavr wrote:
> > lebedev.ri wrote:
> > > gribozavr wrote:
> > > > JonasToth wrote:
> > > > > lebedev.ri wrote:
> > > > > > Please bikeshed on the name. I don't think this one is good.
> > > > > Hmm, `analyzeGeneric`, `analyzeGeneral`, `abstractAnalysis`, 
> > > > > `analyzeAbstract`, something good in these?
> > > > > 
> > > > > Given its private its not too important either ;)
> > > > I'd suggest to simplify by changing `analyzeBoilerplate()` into a 
> > > > non-template, into this specifically:
> > > > 
> > > > ```
> > > > ExceptionAnalyzer::ExceptionInfo 
> > > > ExceptionAnalyzer::filterIgnoredExceptions(ExceptionInfo ExceptionList) 
> > > > {
> > > > if (ExceptionList.getBehaviour() == State::NotThrowing ||
> > > >   ExceptionList.getBehaviour() == State::Unknown)
> > > > return ExceptionList;
> > > > 
> > > >   // Remove all ignored exceptions from the list of exceptions that can 
> > > > be
> > > >   // thrown.
> > > >   ExceptionList.filterIgnoredExceptions(IgnoredExceptions, 
> > > > IgnoreBadAlloc);
> > > > 
> > > >   return ExceptionList;
> > > > }
> > > > ```
> > > > 
> > > > And then call it in `analyze()`:
> > > > 
> > > > ```
> > > > ExceptionAnalyzer::ExceptionInfo
> > > > ExceptionAnalyzer::analyze(const FunctionDecl *Func) {
> > > >   return filterIgnoredExceptions(analyzeImpl(Func));
> > > > }
> > > > ```
> > > Hmm not really.
> > > I intentionally did all this to maximally complicate any possibility of 
> > > accidentally doing
> > > something different given diferent entry point (`Stmt` vs `FunctionDecl`).
> > > Refactoring it that way, via `filterIgnoredExceptions()` increases that 
> > > risk back.
> > > (accidentally omit that intermediate function, and ...)
> > > (accidentally omit that intermediate function, and ...)
> > 
> > ... and tests should catch it.  No big drama.
> > 
> > Anyway, it is not as important.  I do think however that complicating the 
> > code this way is not worth the benefit.
> That is kind of the point. The test would catch it if they would already 
> exist.
> If a new entry point is being added, the tests wouldn't be there yet.
> This enforces that every entry point behaves the same way.
> This enforces that every entry point behaves the same way.

Not really -- the new entry point can skip calling `analyzeDispatch` (and roll 
its own analysis) just like it can skip calling `filterIgnoredException()`.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59650



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

[PATCH] D59650: [NFC] ExceptionEscapeCheck: small refactoring

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri marked an inline comment as done.
lebedev.ri added inline comments.



Comment at: clang-tidy/utils/ExceptionAnalyzer.cpp:226
+ExceptionAnalyzer::ExceptionInfo
+ExceptionAnalyzer::analyzeBoilerplate(const T *Node) {
+  ExceptionInfo ExceptionList;

gribozavr wrote:
> lebedev.ri wrote:
> > gribozavr wrote:
> > > lebedev.ri wrote:
> > > > gribozavr wrote:
> > > > > JonasToth wrote:
> > > > > > lebedev.ri wrote:
> > > > > > > Please bikeshed on the name. I don't think this one is good.
> > > > > > Hmm, `analyzeGeneric`, `analyzeGeneral`, `abstractAnalysis`, 
> > > > > > `analyzeAbstract`, something good in these?
> > > > > > 
> > > > > > Given its private its not too important either ;)
> > > > > I'd suggest to simplify by changing `analyzeBoilerplate()` into a 
> > > > > non-template, into this specifically:
> > > > > 
> > > > > ```
> > > > > ExceptionAnalyzer::ExceptionInfo 
> > > > > ExceptionAnalyzer::filterIgnoredExceptions(ExceptionInfo 
> > > > > ExceptionList) {
> > > > > if (ExceptionList.getBehaviour() == State::NotThrowing ||
> > > > >   ExceptionList.getBehaviour() == State::Unknown)
> > > > > return ExceptionList;
> > > > > 
> > > > >   // Remove all ignored exceptions from the list of exceptions that 
> > > > > can be
> > > > >   // thrown.
> > > > >   ExceptionList.filterIgnoredExceptions(IgnoredExceptions, 
> > > > > IgnoreBadAlloc);
> > > > > 
> > > > >   return ExceptionList;
> > > > > }
> > > > > ```
> > > > > 
> > > > > And then call it in `analyze()`:
> > > > > 
> > > > > ```
> > > > > ExceptionAnalyzer::ExceptionInfo
> > > > > ExceptionAnalyzer::analyze(const FunctionDecl *Func) {
> > > > >   return filterIgnoredExceptions(analyzeImpl(Func));
> > > > > }
> > > > > ```
> > > > Hmm not really.
> > > > I intentionally did all this to maximally complicate any possibility of 
> > > > accidentally doing
> > > > something different given diferent entry point (`Stmt` vs 
> > > > `FunctionDecl`).
> > > > Refactoring it that way, via `filterIgnoredExceptions()` increases that 
> > > > risk back.
> > > > (accidentally omit that intermediate function, and ...)
> > > > (accidentally omit that intermediate function, and ...)
> > > 
> > > ... and tests should catch it.  No big drama.
> > > 
> > > Anyway, it is not as important.  I do think however that complicating the 
> > > code this way is not worth the benefit.
> > That is kind of the point. The test would catch it if they would already 
> > exist.
> > If a new entry point is being added, the tests wouldn't be there yet.
> > This enforces that every entry point behaves the same way.
> > This enforces that every entry point behaves the same way.
> 
> Not really -- the new entry point can skip calling `analyzeDispatch` (and 
> roll its own analysis) just like it can skip calling 
> `filterIgnoredException()`.
Yep. I'm just hoping that that would look more out-of-place/be more noticeable 
than omitting some intermediate call.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59650



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

r356752 - [OPENMP]Emit error message for allocate directive without allocator

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Mar 22 07:41:39 2019
New Revision: 356752

URL: http://llvm.org/viewvc/llvm-project?rev=356752&view=rev
Log:
[OPENMP]Emit error message for allocate directive without allocator
clause in target region.

According to the OpenMP 5.0, 2.11.3 allocate Directive, Restrictions,
allocate directives that appear in a target region must specify an
allocator clause unless a requires directive with the dynamic_allocators
clause is present in the same compilation unit.

Added:
cfe/trunk/test/OpenMP/nvptx_allocate_messages.cpp
  - copied, changed from r356749, 
cfe/trunk/test/OpenMP/nvptx_allocate_codegen.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/nvptx_allocate_codegen.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=356752&r1=356751&r2=356752&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Mar 22 07:41:39 
2019
@@ -9152,6 +9152,9 @@ def warn_omp_used_different_allocator :
   InGroup;
 def note_omp_previous_allocator : Note<
   "previous allocator is specified here">;
+def err_expected_allocator_clause : Error<"expected an 'allocator' clause "
+  "inside of the target region; provide an 'allocator' clause or use 
'requires'"
+  " directive with the 'dynamic_allocators' clause">;
 } // 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=356752&r1=356751&r2=356752&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Fri Mar 22 07:41:39 2019
@@ -2243,8 +2243,12 @@ Sema::DeclGroupPtrTy Sema::ActOnOpenMPAl
 ArrayRef Clauses, DeclContext *Owner) {
   assert(Clauses.size() <= 1 && "Expected at most one clause.");
   Expr *Allocator = nullptr;
-  if (!Clauses.empty())
+  if (Clauses.empty()) {
+if (LangOpts.OpenMPIsDevice)
+  targetDiag(Loc, diag::err_expected_allocator_clause);
+  } else {
 Allocator = cast(Clauses.back())->getAllocator();
+  }
   OMPAllocateDeclAttr::AllocatorTypeTy AllocatorKind =
   getAllocatorKind(*this, DSAStack, Allocator);
   SmallVector Vars;

Modified: cfe/trunk/test/OpenMP/nvptx_allocate_codegen.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_allocate_codegen.cpp?rev=356752&r1=356751&r2=356752&view=diff
==
--- cfe/trunk/test/OpenMP/nvptx_allocate_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/nvptx_allocate_codegen.cpp Fri Mar 22 07:41:39 2019
@@ -64,7 +64,7 @@ int main () {
 #pragma omp allocate(a) allocator(omp_thread_mem_alloc)
   a=2;
   double b = 3;
-#pragma omp allocate(b)
+#pragma omp allocate(b) allocator(omp_default_mem_alloc)
   return (foo());
 }
 

Copied: cfe/trunk/test/OpenMP/nvptx_allocate_messages.cpp (from r356749, 
cfe/trunk/test/OpenMP/nvptx_allocate_codegen.cpp)
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_allocate_messages.cpp?p2=cfe/trunk/test/OpenMP/nvptx_allocate_messages.cpp&p1=cfe/trunk/test/OpenMP/nvptx_allocate_codegen.cpp&r1=356749&r2=356752&rev=356752&view=diff
==
--- cfe/trunk/test/OpenMP/nvptx_allocate_codegen.cpp (original)
+++ cfe/trunk/test/OpenMP/nvptx_allocate_messages.cpp Fri Mar 22 07:41:39 2019
@@ -1,10 +1,21 @@
 // RUN: %clang_cc1 -verify -fopenmp -triple x86_64-apple-darwin10.6.0 
-fopenmp-targets=nvptx64-nvidia-cuda  -emit-llvm-bc -o %t-host.bc %s
-// RUN: %clang_cc1 -verify -fopenmp -triple nvptx64-nvidia-cuda 
-fopenmp-targets=nvptx64-nvidia-cuda -emit-llvm %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-host.bc -o - | FileCheck %s
+// RUN: %clang_cc1 -verify -DDEVICE -fopenmp -triple nvptx64-nvidia-cuda 
-fopenmp-targets=nvptx64-nvidia-cuda -fsyntax-only %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-host.bc
+#ifndef DEVICE
 // expected-no-diagnostics
+#endif // DEVICE
 
 #ifndef HEADER
 #define HEADER
 
+int bar() {
+  int res = 0;
+#ifdef DEVICE
+// expected-error@+2 {{expected an 'allocator' clause inside of the target 
region; provide an 'allocator' clause or use 'requires' directive with the 
'dynamic_allocators' clause}}
+#endif // DEVICE
+#pragma omp allocate(res)
+  return 0;
+}
+
 #pragma omp declare target
 typedef void **omp_allocator_handle_t;
 extern const omp_allocator_handle_t omp_default_mem_alloc;
@@ -16,14 +27,6 @@ extern const omp_allocator_handle_t omp_
 extern const omp_allocator_handle_t omp_pteam_mem_alloc;
 extern const om

[PATCH] D59407: [clangd] Add RelationSlab

[Nathan Ridge via Phabricator via cfe-commits]

nridge marked 2 inline comments as done.
nridge added inline comments.



Comment at: clang-tools-extra/clangd/index/Relation.h:1
+//===--- Ref.h ---*- 
C++-*-===//
+//

gribozavr wrote:
> gribozavr wrote:
> > "--- Relation.h "
> Not done?
(Sorry, I have these comments addressed locally, was just waiting for the 
resolution of the remaining issue before uploading.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407



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

[clang-tools-extra] r356756 - [clang-tidy] Fix a compiler warning.

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Fri Mar 22 08:07:18 2019
New Revision: 356756

URL: http://llvm.org/viewvc/llvm-project?rev=356756&view=rev
Log:
[clang-tidy] Fix a compiler warning.

Rename the Preprocessor field to fix the

  declaration of ‘std::unique_ptr 
clang::tooling::ExpandModularHeadersPPCallbacks::Preprocessor’ changes the 
meaning of ‘Preprocessor’ from ‘class clang::Preprocessor’ [-fpermissive]

warning.

Modified:
clang-tools-extra/trunk/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
clang-tools-extra/trunk/clang-tidy/ExpandModularHeadersPPCallbacks.h

Modified: clang-tools-extra/trunk/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ExpandModularHeadersPPCallbacks.cpp?rev=356756&r1=356755&r2=356756&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/ExpandModularHeadersPPCallbacks.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/ExpandModularHeadersPPCallbacks.cpp Fri 
Mar 22 08:07:18 2019
@@ -78,12 +78,12 @@ ExpandModularHeadersPPCallbacks::ExpandM
   auto PO = std::make_shared();
   *PO = Compiler.getPreprocessorOpts();
 
-  Preprocessor = llvm::make_unique(
-  PO, Diags, LangOpts, Sources, *HeaderInfo, ModuleLoader,
-  /*IILookup=*/nullptr,
-  /*OwnsHeaderSearch=*/false);
-  Preprocessor->Initialize(Compiler.getTarget(), Compiler.getAuxTarget());
-  InitializePreprocessor(*Preprocessor, *PO, Compiler.getPCHContainerReader(),
+  PP = llvm::make_unique(PO, Diags, LangOpts, Sources,
+  *HeaderInfo, ModuleLoader,
+  /*IILookup=*/nullptr,
+  /*OwnsHeaderSearch=*/false);
+  PP->Initialize(Compiler.getTarget(), Compiler.getAuxTarget());
+  InitializePreprocessor(*PP, *PO, Compiler.getPCHContainerReader(),
  Compiler.getFrontendOpts());
   ApplyHeaderSearchOptions(*HeaderInfo, *HSO, LangOpts,
Compiler.getTarget().getTriple());
@@ -92,7 +92,7 @@ ExpandModularHeadersPPCallbacks::ExpandM
 ExpandModularHeadersPPCallbacks::~ExpandModularHeadersPPCallbacks() = default;
 
 Preprocessor *ExpandModularHeadersPPCallbacks::getPreprocessor() const {
-  return Preprocessor.get();
+  return PP.get();
 }
 
 void ExpandModularHeadersPPCallbacks::handleModuleFile(
@@ -129,11 +129,11 @@ void ExpandModularHeadersPPCallbacks::pa
 
   if (!StartedLexing) {
 StartedLexing = true;
-Preprocessor->Lex(CurrentToken);
+PP->Lex(CurrentToken);
   }
   while (!CurrentToken.is(tok::eof) &&
  Sources.isBeforeInTranslationUnit(CurrentToken.getLocation(), Loc)) {
-Preprocessor->Lex(CurrentToken);
+PP->Lex(CurrentToken);
   }
 }
 
@@ -142,7 +142,7 @@ void ExpandModularHeadersPPCallbacks::Fi
 SrcMgr::CharacteristicKind FileType, FileID PrevFID = FileID()) {
   if (!EnteredMainFile) {
 EnteredMainFile = true;
-Preprocessor->EnterMainSourceFile();
+PP->EnterMainSourceFile();
   }
 }
 
@@ -162,7 +162,7 @@ void ExpandModularHeadersPPCallbacks::In
 
 void ExpandModularHeadersPPCallbacks::EndOfMainFile() {
   while (!CurrentToken.is(tok::eof))
-Preprocessor->Lex(CurrentToken);
+PP->Lex(CurrentToken);
 }
 
 // Handle all other callbacks.

Modified: clang-tools-extra/trunk/clang-tidy/ExpandModularHeadersPPCallbacks.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/ExpandModularHeadersPPCallbacks.h?rev=356756&r1=356755&r2=356756&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/ExpandModularHeadersPPCallbacks.h 
(original)
+++ clang-tools-extra/trunk/clang-tidy/ExpandModularHeadersPPCallbacks.h Fri 
Mar 22 08:07:18 2019
@@ -125,7 +125,7 @@ private:
   TrivialModuleLoader ModuleLoader;
 
   std::unique_ptr HeaderInfo;
-  std::unique_ptr Preprocessor;
+  std::unique_ptr PP;
   bool EnteredMainFile = false;
   bool StartedLexing = false;
   Token CurrentToken;


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

[PATCH] D57687: [clang-format] Add style option AllowShortLambdasOnASingleLine

[Ronald Wampler via Phabricator via cfe-commits]

rdwampler added a comment.

I don't have commit rights, can someone land this for me or would it be better 
to try and get commit access?


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

https://reviews.llvm.org/D57687



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

r356758 - [OPENMP]Allow no allocator clause in target regions with requires

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Mar 22 08:25:12 2019
New Revision: 356758

URL: http://llvm.org/viewvc/llvm-project?rev=356758&view=rev
Log:
[OPENMP]Allow no allocator clause in target regions with requires
dynamic_allocators.

According to the OpenMP 5.0, 2.11.3 allocate Directive, Restrictions,
allocate directives that appear in a target region must specify an
allocator clause unless a requires directive with the dynamic_allocators
clause is present in the same compilation unit. Patch adds a check for a
presence of the requires directive with the dynamic_allocators clause.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp
cfe/trunk/test/OpenMP/nvptx_allocate_messages.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=356758&r1=356757&r2=356758&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Fri Mar 22 08:25:12 2019
@@ -422,6 +422,16 @@ public:
 RequiresDecls.push_back(RD);
   }
 
+  /// Checks if the defined 'requires' directive has specified type of clause.
+  template 
+  bool hasRequiresDeclWithClause() {
+return llvm::any_of(RequiresDecls, [](const OMPRequiresDecl *D) {
+  return llvm::any_of(D->clauselists(), [](const OMPClause *C) {
+return isa(C);
+  });
+});
+  }
+
   /// Checks for a duplicate clause amongst previously declared requires
   /// directives
   bool hasDuplicateRequiresClause(ArrayRef ClauseList) const {
@@ -2244,7 +2254,8 @@ Sema::DeclGroupPtrTy Sema::ActOnOpenMPAl
   assert(Clauses.size() <= 1 && "Expected at most one clause.");
   Expr *Allocator = nullptr;
   if (Clauses.empty()) {
-if (LangOpts.OpenMPIsDevice)
+if (LangOpts.OpenMPIsDevice &&
+!DSAStack->hasRequiresDeclWithClause())
   targetDiag(Loc, diag::err_expected_allocator_clause);
   } else {
 Allocator = cast(Clauses.back())->getAllocator();

Modified: cfe/trunk/test/OpenMP/nvptx_allocate_messages.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/OpenMP/nvptx_allocate_messages.cpp?rev=356758&r1=356757&r2=356758&view=diff
==
--- cfe/trunk/test/OpenMP/nvptx_allocate_messages.cpp (original)
+++ cfe/trunk/test/OpenMP/nvptx_allocate_messages.cpp Fri Mar 22 08:25:12 2019
@@ -1,17 +1,22 @@
 // RUN: %clang_cc1 -verify -fopenmp -triple x86_64-apple-darwin10.6.0 
-fopenmp-targets=nvptx64-nvidia-cuda  -emit-llvm-bc -o %t-host.bc %s
 // RUN: %clang_cc1 -verify -DDEVICE -fopenmp -triple nvptx64-nvidia-cuda 
-fopenmp-targets=nvptx64-nvidia-cuda -fsyntax-only %s -fopenmp-is-device 
-fopenmp-host-ir-file-path %t-host.bc
-#ifndef DEVICE
+// RUN: %clang_cc1 -verify -DDEVICE -DREQUIRES -fopenmp -triple 
nvptx64-nvidia-cuda -fopenmp-targets=nvptx64-nvidia-cuda -fsyntax-only %s 
-fopenmp-is-device -fopenmp-host-ir-file-path %t-host.bc
+#if !defined(DEVICE) || defined(REQUIRES)
 // expected-no-diagnostics
 #endif // DEVICE
 
 #ifndef HEADER
 #define HEADER
 
+#if defined(REQUIRES) && defined(DEVICE)
+#pragma omp requires dynamic_allocators
+#endif // REQUIRES && DEVICE
+
 int bar() {
   int res = 0;
-#ifdef DEVICE
+#if defined(DEVICE) && !defined(REQUIRES)
 // expected-error@+2 {{expected an 'allocator' clause inside of the target 
region; provide an 'allocator' clause or use 'requires' directive with the 
'dynamic_allocators' clause}}
-#endif // DEVICE
+#endif // DEVICE && !REQUIRES
 #pragma omp allocate(res)
   return 0;
 }
@@ -65,13 +70,13 @@ int main () {
 #pragma omp allocate(a) allocator(omp_thread_mem_alloc)
   a=2;
   double b = 3;
-#ifdef DEVICE
+#if defined(DEVICE) && !defined(REQUIRES)
 // expected-error@+2 {{expected an 'allocator' clause inside of the target 
region; provide an 'allocator' clause or use 'requires' directive with the 
'dynamic_allocators' clause}}
-#endif // DEVICE
+#endif // DEVICE && !REQUIRES
 #pragma omp allocate(b)
-#ifdef DEVICE
+#if defined(DEVICE) && !defined(REQUIRES)
 // expected-note@+2 {{called by 'main'}}
-#endif // DEVICE
+#endif // DEVICE && !REQUIRES
   return (foo() + bar());
 }
 


___
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

[Clement Courbet via Phabricator via cfe-commits]

courbet marked 2 inline comments as done.
courbet 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;

alexfh wrote:
> 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.
Thanks for the pointer.


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

[PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

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

balazske added inline comments.



Comment at: clang/unittests/AST/ASTImporterTest.cpp:583
+  // Returns a ImporterConstructor that constructs this class.
+  static ASTImporterOptionSpecificTestBase::ImporterConstructor
+  getConstructor() {

balazske wrote:
> Is it possible to make this a static variable instead of function?
Another small thing: The comment above is now not updated ("Returns").



Comment at: clang/unittests/AST/ASTImporterTest.cpp:348
+
+void setImporter(std::unique_ptr I) {
+  Importer = std::move(I);

This function is not needed?



Comment at: clang/unittests/AST/ASTImporterTest.cpp:482
 
+  ASTImporter &getImporter(Decl *From, Language ToLang) {
+lazyInitToAST(ToLang, "", OutputFileName);

This is unused, can be removed.


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] Add an ImportInternal method to allow customizing Import behavior.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D59485#1439390 , @teemperor wrote:

> > Well, I still don't understand how LLDB synthesis the AST. 
> >  So you have a C++ module in a .pcm file. This means you could create an 
> > AST from that with ASTReader from it's .clang_ast section, right? In that 
> > case the AST should be complete with all type information. If this was the 
> > case then I don't see why it is not possible to use clang::ASTImporter to 
> > import templates and specializations, since we do exactly that in CTU.
> > 
> > Or do you guys create the AST from the debug info, from the .debug_info 
> > section of .pcm module file? And this is why AST is incomplete? I've got 
> > this from 
> > https://youtu.be/EgkZ8PTNSHQ?list=PL85Cf-ok7HpppFWlp2wX_-A1dkyFVlaEB&t=1565
> >  If this is the case, then comes my naiive quiestion: Why don't you use the 
> > ASTReader?
>
> There are no C++ modules involved in our use case beside the generic `std` 
> module. No user-written code is read from a module and we don't have any PCM 
> file beside the `std` module we build for the expression evaluator.
>
> We use the ASTReader to directly read the `std` module contents into the 
> expression evaluation ASTContext, but this doesn't give us the decls for the 
> instantiations the user has made (e.g. `std::vector`). We only have 
> these user instantiations in the 'normal' debug info where we have a rather 
> minimal AST (again, no modules are not involved in building this debug info 
> AST). The `InternalImport` in the LLDB patch just stitches the module AST and 
> the debug info AST together when we import something that we also have (in 
> better quality from the C++ module) in the target ASTContext.


Thank you for the explanation.
Now I understand you directly create specializations from the std module and 
intercept the import to avoid importing broken specializations from the debug 
info, this makes sense.


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] D59360: [clang-tidy] Fix more false positives for bugprone-string-integer-assignment

[Clement Courbet via Phabricator via cfe-commits]

courbet updated this revision to Diff 191885.
courbet added a comment.

Ignore template contexts and add a test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59360

Files:
  clang-tools-extra/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
  clang-tools-extra/test/clang-tidy/bugprone-string-integer-assignment.cpp

Index: clang-tools-extra/test/clang-tidy/bugprone-string-integer-assignment.cpp
===
--- clang-tools-extra/test/clang-tidy/bugprone-string-integer-assignment.cpp
+++ clang-tools-extra/test/clang-tidy/bugprone-string-integer-assignment.cpp
@@ -7,6 +7,8 @@
   basic_string& operator=(basic_string);
   basic_string& operator+=(T);
   basic_string& operator+=(basic_string);
+  const T &operator[](int i) const;
+  T &operator[](int i);
 };
 
 typedef basic_string string;
@@ -21,10 +23,13 @@
 
 typedef int MyArcaneChar;
 
+constexpr char kCharConstant = 'a';
+
 int main() {
   std::string s;
   std::wstring ws;
   int x = 5;
+  const char c = 'c';
 
   s = 6;
 // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an integer is interpreted as a character code when assigning {{.*}} [bugprone-string-integer-assignment]
@@ -58,12 +63,47 @@
 
   s += toupper(x);
   s += tolower(x);
-  s += std::tolower(x);
+  s += (std::tolower(x));
+
+  s += c & s[1];
+  s += c ^ s[1];
+  s += c | s[1];
+
+  s[x] += 1;
+  s += s[x];
+  as += as[x];
 
   // Likely character expressions.
   s += x & 0xff;
   s += 0xff & x;
+  s += x % 26;
+  s += 26 % x;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: an integer is interpreted as a chara
+  // CHECK-FIXES: {{^}}  s += std::to_string(26 % x);{{$}}
+  s += c | 0x80;
+  s += c | 0x8000;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: an integer is interpreted as a chara
+  // CHECK-FIXES: {{^}}  s += std::to_string(c | 0x8000);{{$}}
+  as += c | 0x8000;
 
   s += 'a' + (x % 26);
+  s += kCharConstant + (x % 26);
+  s += 'a' + (s[x] & 0xf);
   s += (x % 10) + 'b';
+
+  s += x > 255 ? c : x;
+  s += x > 255 ? 12 : x;
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: an integer is interpreted as a chara
+  // CHECK-FIXES: {{^}}  s += std::to_string(x > 255 ? 12 : x);{{$}}
+}
+
+namespace instantiation_dependent_exprs {
+template
+struct S {
+  static constexpr T t = 0x8000;
+  std::string s;
+  void f(char c) { s += c | static_cast(t); }
+};
+
+template S;
 }
Index: clang-tools-extra/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
@@ -45,38 +45,100 @@
   this);
 }
 
-static bool isLikelyCharExpression(const Expr *Argument,
-   const ASTContext &Ctx) {
-  const auto *BinOp = dyn_cast(Argument);
-  if (!BinOp)
+class CharExpressionDetector {
+public:
+  CharExpressionDetector(QualType CharType, const ASTContext &Ctx)
+  : CharType(CharType), Ctx(Ctx) {}
+
+  bool isLikelyCharExpression(const Expr *E) const {
+if (isCharTyped(E))
+  return true;
+
+if (const auto *BinOp = dyn_cast(E)) {
+  const auto *LHS = BinOp->getLHS()->IgnoreParenImpCasts();
+  const auto *RHS = BinOp->getRHS()->IgnoreParenImpCasts();
+  // Handle both directions, e.g. `'a' + (i % 26)` and `(i % 26) + 'a'`.
+  if (BinOp->isAdditiveOp() || BinOp->isBitwiseOp())
+return handleBinaryOp(BinOp->getOpcode(), LHS, RHS) ||
+   handleBinaryOp(BinOp->getOpcode(), RHS, LHS);
+  // Except in the case of '%'.
+  if (BinOp->getOpcode() == BO_Rem)
+return handleBinaryOp(BinOp->getOpcode(), LHS, RHS);
+  return false;
+}
+
+// Ternary where at least one branch is a likely char expression, e.g.
+//i < 265 ? i : ' '
+if (const auto *CondOp = dyn_cast(E))
+  return isLikelyCharExpression(
+ CondOp->getFalseExpr()->IgnoreParenImpCasts()) ||
+ isLikelyCharExpression(
+ CondOp->getTrueExpr()->IgnoreParenImpCasts());
 return false;
-  const auto *LHS = BinOp->getLHS()->IgnoreParenImpCasts();
-  const auto *RHS = BinOp->getRHS()->IgnoreParenImpCasts();
-  //  & , mask is a compile time constant.
-  Expr::EvalResult RHSVal;
-  if (BinOp->getOpcode() == BO_And &&
-  (RHS->EvaluateAsInt(RHSVal, Ctx, Expr::SE_AllowSideEffects) ||
-   LHS->EvaluateAsInt(RHSVal, Ctx, Expr::SE_AllowSideEffects)))
-return true;
-  //  + ( % ), where  is a char literal.
-  const auto IsCharPlusModExpr = [](const Expr *L, const Expr *R) {
-const auto *ROp = dyn_cast(R);
-return ROp && ROp->getOpcode() == BO_Rem && isa(L);
-  };
-  if (BinOp->getOpcode() == BO_Add) {
-if (IsCharPlusModExpr(LHS, RHS) || IsCharPlusModExpr(RHS, LHS))
+  }
+
+private:
+  bool handleBinaryOp(clang::BinaryOperatorKind Opcode, con

[PATCH] D59407: [clangd] Add RelationSlab

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 191886.
nridge added a comment.

Scrapped 'struct Relation' and addressed other comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59407

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/Relation.cpp
  clang-tools-extra/clangd/index/Relation.h

Index: clang-tools-extra/clangd/index/Relation.h
===
--- /dev/null
+++ clang-tools-extra/clangd/index/Relation.h
@@ -0,0 +1,120 @@
+//===--- Relation.h --*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_RELATION_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_RELATION_H
+
+#include "SymbolID.h"
+#include "SymbolLocation.h"
+#include "clang/Index/IndexSymbol.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/Support/StringSaver.h"
+#include "llvm/Support/raw_ostream.h"
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+
+struct RelationKey {
+  SymbolID Symbol;
+  index::SymbolRole Kind;
+
+  bool operator==(const RelationKey &Other) const {
+return Symbol == Other.Symbol && Kind == Other.Kind;
+  }
+
+private:
+  friend llvm::hash_code hash_value(const RelationKey &Key) {
+return llvm::hash_combine(Key.Symbol, static_cast(Key.Kind));
+  }
+};
+
+class RelationSlab {
+public:
+  /// The interpretation of a pair (Key, Value) is:
+  /// " is  of every symbol in ".
+  /// For example, if Key.Kind == SymbolRole::RelationChildOf,
+  /// then Key.Symbol is the child of every symbol in Value (i.e. the symbols
+  /// in Value are the base classes of Key.Symbol).
+  using value_type = std::pair>;
+  using const_iterator = std::vector::const_iterator;
+  using iterator = const_iterator;
+
+  RelationSlab() = default;
+  RelationSlab(RelationSlab &&Slab) = default;
+  RelationSlab &operator=(RelationSlab &&RHS) = default;
+
+  const_iterator begin() const { return Relations.begin(); }
+  const_iterator end() const { return Relations.end(); }
+  size_t size() const { return Relations.size(); }
+  size_t numRelations() const { return NumRelations; }
+  bool empty() const { return Relations.empty(); }
+
+  size_t bytes() const {
+return sizeof(*this) + Arena.getTotalMemory() +
+   sizeof(value_type) * Relations.capacity();
+  }
+
+  /// A mutable container that can 'freeze' to RelationSlab.
+  class Builder {
+  public:
+Builder() {}
+/// Adds a relation to the slab.
+void insert(const RelationKey &Key, const SymbolID &S);
+/// Consumes the builder to finalize the slab.
+RelationSlab build() &&;
+
+  private:
+llvm::BumpPtrAllocator Arena;
+llvm::DenseMap> Relations;
+  };
+
+private:
+  RelationSlab(std::vector Relations, llvm::BumpPtrAllocator Arena,
+   size_t NumRelations)
+  : Arena(std::move(Arena)), Relations(std::move(Relations)),
+NumRelations(NumRelations) {}
+
+  llvm::BumpPtrAllocator Arena;
+  std::vector Relations;
+  // Number of all relations.
+  size_t NumRelations = 0;
+};
+
+} // namespace clangd
+} // namespace clang
+
+namespace llvm {
+
+// Support RelationKeys as DenseMap keys.
+template <> struct DenseMapInfo {
+  static inline clang::clangd::RelationKey getEmptyKey() {
+return {DenseMapInfo::getEmptyKey(),
+clang::index::SymbolRole::RelationChildOf};
+  }
+
+  static inline clang::clangd::RelationKey getTombstoneKey() {
+return {DenseMapInfo::getTombstoneKey(),
+clang::index::SymbolRole::RelationChildOf};
+  }
+
+  static unsigned getHashValue(const clang::clangd::RelationKey &Key) {
+return hash_value(Key);
+  }
+
+  static bool isEqual(const clang::clangd::RelationKey &LHS,
+  const clang::clangd::RelationKey &RHS) {
+return LHS == RHS;
+  }
+};
+
+} // namespace llvm
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_RELATION_H
Index: clang-tools-extra/clangd/index/Relation.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/index/Relation.cpp
@@ -0,0 +1,35 @@
+//===--- Relation.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.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Relation.h"
+
+namespace clang {
+namespace clangd {
+
+void RelationSlab::Builder::insert(const

r356759 - [OPENMP]Add missing comment, NFC.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Fri Mar 22 08:32:02 2019
New Revision: 356759

URL: http://llvm.org/viewvc/llvm-project?rev=356759&view=rev
Log:
[OPENMP]Add missing comment, NFC.

Modified:
cfe/trunk/lib/Sema/SemaOpenMP.cpp

Modified: cfe/trunk/lib/Sema/SemaOpenMP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOpenMP.cpp?rev=356759&r1=356758&r2=356759&view=diff
==
--- cfe/trunk/lib/Sema/SemaOpenMP.cpp (original)
+++ cfe/trunk/lib/Sema/SemaOpenMP.cpp Fri Mar 22 08:32:02 2019
@@ -2254,6 +2254,10 @@ Sema::DeclGroupPtrTy Sema::ActOnOpenMPAl
   assert(Clauses.size() <= 1 && "Expected at most one clause.");
   Expr *Allocator = nullptr;
   if (Clauses.empty()) {
+// OpenMP 5.0, 2.11.3 allocate Directive, Restrictions.
+// allocate directives that appear in a target region must specify an
+// allocator clause unless a requires directive with the dynamic_allocators
+// clause is present in the same compilation unit.
 if (LangOpts.OpenMPIsDevice &&
 !DSAStack->hasRequiresDeclWithClause())
   targetDiag(Loc, diag::err_expected_allocator_clause);


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

[PATCH] D59639: [clangd] Print template arguments helper

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-tools-extra/clangd/AST.cpp:149
+} else {
+  // FIXME: Fix cases when getTypeAsWritten returns null, e.g. friend 
decls.
+  printTemplateArgumentList(OS, Cls->getTemplateArgs().asArray(), Policy);

I'm not quite sure if I understand this FIXME. In this `else` branch, we are 
handling this case. Do you mean this is not a proper fix and future work is 
needed here? Could you elaborate?

One thing that's worth commenting is why we could use 
`Cls->getTemplateArgs().asArray()` when `Cls->getTypeAsWritten` is null. It's 
not trivial from the code.



Comment at: clang-tools-extra/clangd/AST.cpp:88
 static const TemplateArgumentList *
 getTemplateSpecializationArgs(const NamedDecl &ND) {
   if (auto *Func = llvm::dyn_cast(&ND))

kadircet wrote:
> ioeric wrote:
> > can we unify this with `getTemplateSpecializationArgLocs` somehow? 
> > 
> > it seems that args as written would also be favorable here (see FIXME in 
> > line 112)
> See D59641
thought?



Comment at: clang-tools-extra/clangd/AST.cpp:133
+printTemplateArgumentList(OS, *Args, Policy);
+  else if (auto *Cls = llvm::dyn_cast(&ND)) {
+if (const TypeSourceInfo *TSI = Cls->getTypeAsWritten()) {

kadircet wrote:
> ioeric wrote:
> > why isn't this handled in `getTemplateSpecializationArgLocs`? Add a comment?
> it is mentioned in `getTemplateSpecializationArgLocs`, would you rather move 
> the comment here?
I think it would be clearer if we do something like:

```
if (auto *Cls = llvm::dyn_cast(&ND)) {
  // handle this specially because ...
} else {
  // use getTemplateSpecializationArgLocs to handle rest of cases.
}

```



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

kadircet wrote:
> ioeric wrote:
> > could you add clang tests for these changes?
> We've also discussed this in the original comment and decided infrastructure 
> necessary was too overwhelming. First we need to invoke compiler and get the 
> AST, then write a Visitor to fetch relevant decls and only after that call 
> printTemplateArguments ...
No test at all is concerning... I think there are lit tests for AST printing in 
test/AST/. Is it possible to have some of those?



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

kadircet wrote:
> ioeric wrote:
> > It's unclear to me what the new behavior is with changes in this file. 
> > Could you add comment?
> > 
> > It might make sense to split the clang change into a separate patch and get 
> > folks who are more familiar to take a look.
> it just prevents erasure from TypeLoc to Type when printing the argument, so 
> that we can do a fine-grained printing if Location is already there.
could you add this into comment?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59639



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Gauthier via Phabricator via cfe-commits]

Tyker added a comment.

about the semantic issue. we will need for diagnostics purposes to detect 
attribute in branches during the semantic phase. so where and how can we store 
this information in the AST so that the CodeGen doesn't have to look through 
branches again for this attributes ?


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

https://reviews.llvm.org/D59467



___
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 updated this revision to Diff 191892.
lebedev.ri marked 15 inline comments as done.
lebedev.ri added a comment.

Addressed some nits.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D59466

Files:
  clang-tidy/openmp/CMakeLists.txt
  clang-tidy/openmp/ExceptionEscapeCheck.cpp
  clang-tidy/openmp/ExceptionEscapeCheck.h
  clang-tidy/openmp/OpenMPTidyModule.cpp
  clang-tidy/utils/ExceptionAnalyzer.cpp
  clang-tidy/utils/ExceptionAnalyzer.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/list.rst
  docs/clang-tidy/checks/openmp-exception-escape.rst
  test/clang-tidy/bugprone-exception-escape-openmp.cpp
  test/clang-tidy/openmp-exception-escape.cpp

Index: test/clang-tidy/openmp-exception-escape.cpp
===
--- /dev/null
+++ test/clang-tidy/openmp-exception-escape.cpp
@@ -0,0 +1,132 @@
+// RUN: %check_clang_tidy %s openmp-exception-escape %t -- -extra-arg=-fopenmp=libomp -extra-arg=-fexceptions -config="{CheckOptions: [{key: openmp-exception-escape.IgnoredExceptions, value: 'ignored, ignored2'}]}" --
+
+int thrower() {
+  throw 42;
+}
+
+class ignored {};
+class ignored2 {};
+namespace std {
+class bad_alloc {};
+} // namespace std
+
+void parallel() {
+#pragma omp parallel
+  thrower();
+  // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: an exception thrown inside of the OpenMP 'parallel' region is not caught in that same region.
+}
+
+void ignore() {
+#pragma omp parallel
+  throw ignored();
+}
+
+void ignore2() {
+#pragma omp parallel
+  throw ignored2();
+}
+
+void standalone_directive() {
+#pragma omp taskwait
+  throw ignored(); // not structured block
+}
+
+void ignore_alloc() {
+#pragma omp parallel
+  throw std::bad_alloc();
+}
+
+void parallel_caught() {
+#pragma omp parallel
+  {
+try {
+  thrower();
+} catch (...) {
+}
+  }
+}
+
+void for_header(const int a) {
+  // Only the body of the loop counts.
+#pragma omp for
+  for (int i = 0; i < thrower(); i++)
+;
+}
+
+void forloop(const int a) {
+#pragma omp for
+  for (int i = 0; i < a; i++)
+thrower();
+  // CHECK-MESSAGES: :[[@LINE-3]]:9: warning: an exception thrown inside of the OpenMP 'for' region is not caught in that same region.
+}
+
+void parallel_forloop(const int a) {
+#pragma omp parallel
+  {
+#pragma omp for
+for (int i = 0; i < a; i++)
+  thrower();
+thrower();
+// CHECK-MESSAGES: :[[@LINE-6]]:9: warning: an exception thrown inside of the OpenMP 'parallel' region is not caught in that same region.
+// CHECK-MESSAGES: :[[@LINE-5]]:9: warning: an exception thrown inside of the OpenMP 'for' region is not caught in that same region.
+  }
+}
+
+void parallel_forloop_caught(const int a) {
+#pragma omp parallel
+  {
+#pragma omp for
+for (int i = 0; i < a; i++) {
+  try {
+thrower();
+  } catch (...) {
+  }
+}
+thrower();
+// CHECK-MESSAGES: :[[@LINE-10]]:9: warning: an exception thrown inside of the OpenMP 'parallel' region is not caught in that same region.
+  }
+}
+
+void parallel_caught_forloop(const int a) {
+#pragma omp parallel
+  {
+#pragma omp for
+for (int i = 0; i < a; i++)
+  thrower();
+try {
+  thrower();
+} catch (...) {
+}
+// CHECK-MESSAGES: :[[@LINE-7]]:9: warning: an exception thrown inside of the OpenMP 'for' region is not caught in that same region.
+  }
+}
+
+void parallel_outercaught_forloop(const int a) {
+#pragma omp parallel
+  {
+try {
+#pragma omp for
+  for (int i = 0; i < a; i++)
+thrower();
+  thrower();
+} catch (...) {
+}
+// CHECK-MESSAGES: :[[@LINE-6]]:9: warning: an exception thrown inside of the OpenMP 'for' region is not caught in that same region.
+  }
+}
+
+void parallel_outercaught_forloop_caught(const int a) {
+#pragma omp parallel
+  {
+try {
+#pragma omp for
+  for (int i = 0; i < a; i++) {
+try {
+  thrower();
+} catch (...) {
+}
+  }
+} catch (...) {
+}
+  }
+}
Index: test/clang-tidy/bugprone-exception-escape-openmp.cpp
===
--- /dev/null
+++ test/clang-tidy/bugprone-exception-escape-openmp.cpp
@@ -0,0 +1,29 @@
+// RUN: %check_clang_tidy %s bugprone-exception-escape %t -- -extra-arg=-fopenmp=libomp -extra-arg=-fexceptions --
+
+int thrower() {
+  throw 42;
+}
+
+void ok_parallel() {
+#pragma omp parallel
+  thrower();
+}
+
+void bad_for_header_XFAIL(const int a) noexcept {
+#pragma omp for
+  for (int i = 0; i < thrower(); i++)
+;
+  // FIXME: this really should be caught by bugprone-exception-escape.
+  // https://bugs.llvm.org/show_bug.cgi?id=41102
+}
+
+void ok_forloop(const int a) {
+#pragma omp for
+  for (int i = 0; i < a; i++)
+thrower();
+}
+
+void some_exception_just_so_that_check_clang_tidy_shuts_up() noexcept {
+  thrower();
+}
+// CHECK-MESSAGES: :[[@LINE-3]]:6: warning: an exception

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Manuel Klimek via Phabricator via cfe-commits]

klimek added inline comments.



Comment at: lib/Format/WhitespaceManager.cpp:482
+  // containing any matching token to be aligned and located after such token.
+  auto AlignCurrentSequence = [&] {
+if (StartOfSequence > 0 && StartOfSequence < EndOfSequence) {

Ok, sorry, but you went one step further here than I imagined :)
The idea would be to unlinline this (and only this :D) and call it 
AligneMacroSequence.


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

https://reviews.llvm.org/D28462



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

[PATCH] D28462: clang-format: Add new style option AlignConsecutiveMacros

[Nick Renieris via Phabricator via cfe-commits]

VelocityRa updated this revision to Diff 191891.
VelocityRa added a comment.

Right, how's that?


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

https://reviews.llvm.org/D28462

Files:
  docs/ClangFormatStyleOptions.rst
  include/clang/Format/Format.h
  lib/Format/Format.cpp
  lib/Format/WhitespaceManager.cpp
  lib/Format/WhitespaceManager.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -9544,8 +9544,104 @@
NoSpaceStyle);
 }
 
+TEST_F(FormatTest, AlignConsecutiveMacros) {
+  FormatStyle Style = getLLVMStyle();
+  Style.AlignConsecutiveAssignments = true;
+  Style.AlignConsecutiveDeclarations = true;
+  Style.AlignConsecutiveMacros = false;
+
+  verifyFormat("#define a 3\n"
+   "#define  4\n"
+   "#define ccc (5)",
+   Style);
+
+  verifyFormat("#define f(x) (x * x)\n"
+   "#define fff(x, y, z) (x * y + z)\n"
+   "#define (x, y) (x - y)",
+   Style);
+
+  verifyFormat("#define foo(x, y) (x + y)\n"
+   "#define bar (5, 6)(2 + 2)",
+   Style);
+
+  verifyFormat("#define a 3\n"
+   "#define  4\n"
+   "#define ccc (5)\n"
+   "#define f(x) (x * x)\n"
+   "#define fff(x, y, z) (x * y + z)\n"
+   "#define (x, y) (x - y)",
+   Style);
+
+  Style.AlignConsecutiveMacros = true;
+  verifyFormat("#define a3\n"
+   "#define  4\n"
+   "#define ccc  (5)",
+   Style);
+
+  verifyFormat("#define f(x) (x * x)\n"
+   "#define fff(x, y, z) (x * y + z)\n"
+   "#define (x, y)   (x - y)",
+   Style);
+
+  verifyFormat("#define foo(x, y) (x + y)\n"
+   "#define bar   (5, 6)(2 + 2)",
+   Style);
+
+  verifyFormat("#define a3\n"
+   "#define  4\n"
+   "#define ccc  (5)\n"
+   "#define f(x) (x * x)\n"
+   "#define fff(x, y, z) (x * y + z)\n"
+   "#define (x, y)   (x - y)",
+   Style);
+
+  verifyFormat("#define a 5\n"
+   "#define foo(x, y) (x + y)\n"
+   "#define CCC   (6)\n"
+   "auto lambda = []() {\n"
+   "  auto  ii = 0;\n"
+   "  float j  = 0;\n"
+   "  return 0;\n"
+   "};\n"
+   "int   i  = 0;\n"
+   "float i2 = 0;\n"
+   "auto  v  = type{\n"
+   "i = 1,   //\n"
+   "(i = 2), //\n"
+   "i = 3//\n"
+   "};",
+   Style);
+
+  Style.AlignConsecutiveMacros = false;
+  Style.ColumnLimit = 20;
+
+  verifyFormat("#define a  \\\n"
+   "  \"aa\"\n"
+   "#define D  \\\n"
+   "  \"aa\" \\\n"
+   "  \"ccdde\"\n"
+   "#define B  \\\n"
+   "  \"Q\"  \\\n"
+   "  \"F\"  \\\n"
+   "  \"\"\n",
+   Style);
+
+  Style.AlignConsecutiveMacros = true;
+  verifyFormat("#define a  \\\n"
+   "  \"aa\"\n"
+   "#define D  \\\n"
+   "  \"aa\" \\\n"
+   "  \"ccdde\"\n"
+   "#define B  \\\n"
+   "  \"Q\"  \\\n"
+   "  \"F\"  \\\n"
+   "  \"\"\n",
+   Style);
+}
+
 TEST_F(FormatTest, AlignConsecutiveAssignments) {
   FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveMacros = true;
   Alignment.AlignConsecutiveAssignments = false;
   verifyFormat("int a = 5;\n"
"int oneTwoThree = 123;",
@@ -9735,6 +9831,7 @@
 
 TEST_F(FormatTest, AlignConsecutiveDeclarations) {
   FormatStyle Alignment = getLLVMStyle();
+  Alignment.AlignConsecutiveMacros = true;
   Alignment.AlignConsecutiveDeclarations = false;
   verifyFormat("float const a = 5;\n"
"int oneTwoThree = 123;",
@@ -10856,6 +10953,7 @@
   Style.Language = FormatStyle::LK_Cpp;
   CHECK_PARSE_BOOL(AlignOperands);
   CHECK_PARSE_BOOL(AlignTrailingComments);
+  CHECK_PARSE_BOOL(AlignConsecutiveMacros);
   CHECK_PARSE_BOOL(AlignConsecutiveAssignments);
   CHECK_PARSE_BOOL(AlignConsecutiveDeclarations);
   CHECK_PARSE_BOOL(AllowAllParametersOfDeclarationOnNextLine);
Index: lib/Format/WhitespaceManager.h
===
--- lib/Format/WhitespaceManager.h
+++ lib/Format/WhitespaceManager.h
@@ -169,6 +169,9 @@
   /// \c EscapedNewlineColumn for the first tokens or token parts in a line.
   void calculateLineBreakI

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

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added inline comments.



Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:53
+  Finder->addMatcher(ompExecutableDirective(
+ unless(isStandaloneDirective()),
+ hasStructuredBlock(stmt().bind("structured-block")))

gribozavr wrote:
> lebedev.ri wrote:
> > gribozavr wrote:
> > > Do we need the `unless`?  `hasStructuredBlock()` just won't match 
> > > standalone directives.
> > *need*? no.
> > But it makes zero sense semantically to look for structured block
> > without first establishing that it has one.
> > Sure, we now check that twice (here, and in `hasStructuredBlock()`), but 
> > that is up to optimizer.
> > 
> > The fact that `hasStructuredBlock()` workarounds the assert in
> > `getStructuredBlock()` is only to avoid clang-query crashes,
> > it is spelled as much in the docs.
> > But it makes zero sense semantically to look for structured block without 
> > first establishing that it has one.
> 
> IDK, in my mind it makes sense.  "Does a standalone directive have a 
> structured block?  No." is a coherent logical chain.
What i'm saying is, yes, it won't match, but i think this reads better from 
OpenMP standpoint.



Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:32
+  llvm::StringSet<> IgnoredExceptions;
+  StringRef(RawIgnoredExceptions).split(IgnoredExceptionsVec, ",", -1, false);
+  IgnoredExceptions.insert(IgnoredExceptionsVec.begin(),

aaron.ballman wrote:
> Do you need to trim any of the split strings?
Hm, i guess i might aswell..



Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:48
+return;
+  // Similarly, if C++ Exceptions are not enabled, nothing to do.
+  if (!getLangOpts().CPlusPlus || !getLangOpts().CXXExceptions)

aaron.ballman wrote:
> Do you have to worry about SEH exceptions in C for this functionality?
I don't have a clue about SEH to be honest.
(This check comes form the `bugprone-exception-escape` check)
Likely the `ExceptionAnalyzer` would need to be upgraded first.



Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:75-76
+  diag(Directive->getBeginLoc(),
+   "An exception thrown inside of the OpenMP '%0' region is not caught in "
+   "that same region.")
+  << getOpenMPDirectiveName(Directive->getDirectiveKind());

aaron.ballman wrote:
> Clang-tidy diagnostics are not complete sentences -- please make this 
> horribly ungrammatical. ;-)
Is `s/An/an` sufficient? :)
I'm not sure what i can drop here without loosing context.



Comment at: docs/clang-tidy/checks/openmp-exception-escape.rst:10-11
+As per the OpenMP specification, structured block is an executable statement,
+possibly compound, with a single entry at the top and a single exit at the
+bottom. Which means, ``throw`` may not be used to to 'exit' out of the
+structured block. If an exception is not caught in the same structured block

aaron.ballman wrote:
> Does this mean setjmp/longjmp out of the block is also a dangerous activity? 
> What about gotos?
From D59214:

https://www.openmp.org/wp-content/uploads/OpenMP-API-Specification-5.0.pdf, 
page 3:
```
structured block

For C/C++, an executable statement, possibly compound, with a single entry at 
the
top and a single exit at the bottom, or an OpenMP construct.

COMMENT: See Section 2.1 on page 38 for restrictions on structured
blocks.
```
```
2.1 Directive Format

Some executable directives include a structured block. A structured block:
• may contain infinite loops where the point of exit is never reached;
• may halt due to an IEEE exception;
• may contain calls to exit(), _Exit(), quick_exit(), abort() or functions with 
a
_Noreturn specifier (in C) or a noreturn attribute (in C/C++);
• may be an expression statement, iteration statement, selection statement, or 
try block, provided
that the corresponding compound statement obtained by enclosing it in { and } 
would be a
structured block; and

Restrictions
Restrictions to structured blocks are as follows:
• Entry to a structured block must not be the result of a branch.
• The point of exit cannot be a branch out of the structured block.
C / C++
• The point of entry to a structured block must not be a call to setjmp().
• longjmp() and throw() must not violate the entry/exit criteria.
```

So yeah, `setjmp`/`longjmp` are also suspects.
Maybe not so much `goto` though https://godbolt.org/z/GZMugf 
https://godbolt.org/z/WUYcYD




Comment at: docs/clang-tidy/checks/openmp-exception-escape.rst:13
+structured block. If an exception is not caught in the same structured block
+it was thrown in, the behaviour is undefined / implementation defined,
+the program will likely terminate.

aaron.ballman wrote:
> Which is it -- undefined or implementation-defined?
I'm unable to anything specific in the spec, let's call this `Unspecified`.

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Brian Gesiak via Phabricator via cfe-commits]

modocache added a comment.

I'm pushing a revert. Sorry for the trouble!


Repository:
  rC Clang

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

https://reviews.llvm.org/D59076



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

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Brian Gesiak via Phabricator via cfe-commits]

modocache added a comment.

Sorry for the late response, I'm looking now. Should I revert this for now?


Repository:
  rC Clang

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

https://reviews.llvm.org/D59076



___
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

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:48
+return;
+  // Similarly, if C++ Exceptions are not enabled, nothing to do.
+  if (!getLangOpts().CPlusPlus || !getLangOpts().CXXExceptions)

lebedev.ri wrote:
> aaron.ballman wrote:
> > Do you have to worry about SEH exceptions in C for this functionality?
> I don't have a clue about SEH to be honest.
> (This check comes form the `bugprone-exception-escape` check)
> Likely the `ExceptionAnalyzer` would need to be upgraded first.
Hmm, that may be worth looking into, but can be done in a follow-up patch. SEH 
likely causes problems here for you as well.



Comment at: clang-tidy/openmp/ExceptionEscapeCheck.cpp:75-76
+  diag(Directive->getBeginLoc(),
+   "An exception thrown inside of the OpenMP '%0' region is not caught in "
+   "that same region.")
+  << getOpenMPDirectiveName(Directive->getDirectiveKind());

lebedev.ri wrote:
> aaron.ballman wrote:
> > Clang-tidy diagnostics are not complete sentences -- please make this 
> > horribly ungrammatical. ;-)
> Is `s/An/an` sufficient? :)
> I'm not sure what i can drop here without loosing context.
You need to remove the full stop from the end of the sentence as well. How 
about: `exception through inside OpenMP '%0' region is not caught within the 
region`



Comment at: docs/clang-tidy/checks/openmp-exception-escape.rst:10-11
+As per the OpenMP specification, structured block is an executable statement,
+possibly compound, with a single entry at the top and a single exit at the
+bottom. Which means, ``throw`` may not be used to to 'exit' out of the
+structured block. If an exception is not caught in the same structured block

lebedev.ri wrote:
> aaron.ballman wrote:
> > Does this mean setjmp/longjmp out of the block is also a dangerous 
> > activity? What about gotos?
> From D59214:
> 
> https://www.openmp.org/wp-content/uploads/OpenMP-API-Specification-5.0.pdf, 
> page 3:
> ```
> structured block
> 
> For C/C++, an executable statement, possibly compound, with a single entry at 
> the
> top and a single exit at the bottom, or an OpenMP construct.
> 
> COMMENT: See Section 2.1 on page 38 for restrictions on structured
> blocks.
> ```
> ```
> 2.1 Directive Format
> 
> Some executable directives include a structured block. A structured block:
> • may contain infinite loops where the point of exit is never reached;
> • may halt due to an IEEE exception;
> • may contain calls to exit(), _Exit(), quick_exit(), abort() or functions 
> with a
> _Noreturn specifier (in C) or a noreturn attribute (in C/C++);
> • may be an expression statement, iteration statement, selection statement, 
> or try block, provided
> that the corresponding compound statement obtained by enclosing it in { and } 
> would be a
> structured block; and
> 
> Restrictions
> Restrictions to structured blocks are as follows:
> • Entry to a structured block must not be the result of a branch.
> • The point of exit cannot be a branch out of the structured block.
> C / C++
> • The point of entry to a structured block must not be a call to setjmp().
> • longjmp() and throw() must not violate the entry/exit criteria.
> ```
> 
> So yeah, `setjmp`/`longjmp` are also suspects.
> Maybe not so much `goto` though https://godbolt.org/z/GZMugf 
> https://godbolt.org/z/WUYcYD
> 
This might be another future thing for the patch to handle. May be worth adding 
some FIXME comments to the code detailing the extensions we want to see.



Comment at: docs/clang-tidy/checks/openmp-exception-escape.rst:13
+structured block. If an exception is not caught in the same structured block
+it was thrown in, the behaviour is undefined / implementation defined,
+the program will likely terminate.

lebedev.ri wrote:
> aaron.ballman wrote:
> > Which is it -- undefined or implementation-defined?
> I'm unable to anything specific in the spec, let's call this `Unspecified`.
Unspecified also has special meaning -- you may want to double-check what the 
precise behavior is, but I suspect it is undefined behavior to violate these 
constraints.


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

r356774 - Revert "[coroutines][PR40978] Emit error for co_yield within catch block"

[Brian Gesiak via cfe-commits]

Author: modocache
Date: Fri Mar 22 09:08:29 2019
New Revision: 356774

URL: http://llvm.org/viewvc/llvm-project?rev=356774&view=rev
Log:
Revert "[coroutines][PR40978] Emit error for co_yield within catch block"

The commit https://reviews.llvm.org/rC356296 is causing a regression in nested
catch scopes, https://bugs.llvm.org/show_bug.cgi?id=41171. Revert this change
for now in order to un-break that problem report.


Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/include/clang/Sema/Scope.h
cfe/trunk/lib/Parse/ParseStmt.cpp
cfe/trunk/lib/Sema/Scope.cpp
cfe/trunk/lib/Sema/SemaCoroutine.cpp
cfe/trunk/test/SemaCXX/coroutines.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=356774&r1=356773&r2=356774&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Fri Mar 22 09:08:29 
2019
@@ -9271,8 +9271,6 @@ def err_coroutine_objc_method : Error<
   "Objective-C methods as coroutines are not yet supported">;
 def err_coroutine_unevaluated_context : Error<
   "'%0' cannot be used in an unevaluated context">;
-def err_coroutine_within_handler : Error<
-  "'%0' cannot be used in the handler of a try block">;
 def err_coroutine_outside_function : Error<
   "'%0' cannot be used outside a function">;
 def err_coroutine_invalid_func_context : Error<

Modified: cfe/trunk/include/clang/Sema/Scope.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Scope.h?rev=356774&r1=356773&r2=356774&view=diff
==
--- cfe/trunk/include/clang/Sema/Scope.h (original)
+++ cfe/trunk/include/clang/Sema/Scope.h Fri Mar 22 09:08:29 2019
@@ -131,9 +131,6 @@ public:
 
 /// We are between inheritance colon and the real class/struct definition 
scope.
 ClassInheritanceScope = 0x80,
-
-/// This is the scope of a C++ catch statement.
-CatchScope = 0x100,
   };
 
 private:

Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=356774&r1=356773&r2=356774&view=diff
==
--- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
+++ cfe/trunk/lib/Parse/ParseStmt.cpp Fri Mar 22 09:08:29 2019
@@ -2260,10 +2260,8 @@ StmtResult Parser::ParseCXXCatchBlock(bo
   // C++ 3.3.2p3:
   // The name in a catch exception-declaration is local to the handler and
   // shall not be redeclared in the outermost block of the handler.
-  unsigned ScopeFlags = Scope::DeclScope | Scope::ControlScope |
-Scope::CatchScope |
-(FnCatch ? Scope::FnTryCatchScope : 0);
-  ParseScope CatchScope(this, ScopeFlags);
+  ParseScope CatchScope(this, Scope::DeclScope | Scope::ControlScope |
+  (FnCatch ? Scope::FnTryCatchScope : 0));
 
   // exception-declaration is equivalent to '...' or a parameter-declaration
   // without default arguments.
@@ -2292,7 +2290,7 @@ StmtResult Parser::ParseCXXCatchBlock(bo
 return StmtError(Diag(Tok, diag::err_expected) << tok::l_brace);
 
   // FIXME: Possible draft standard bug: attribute-specifier should be allowed?
-  StmtResult Block(ParseCompoundStatement(/*isStmtExpr=*/false, ScopeFlags));
+  StmtResult Block(ParseCompoundStatement());
   if (Block.isInvalid())
 return Block;
 

Modified: cfe/trunk/lib/Sema/Scope.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Scope.cpp?rev=356774&r1=356773&r2=356774&view=diff
==
--- cfe/trunk/lib/Sema/Scope.cpp (original)
+++ cfe/trunk/lib/Sema/Scope.cpp Fri Mar 22 09:08:29 2019
@@ -166,9 +166,7 @@ void Scope::dumpImpl(raw_ostream &OS) co
   {SEHExceptScope, "SEHExceptScope"},
   {SEHFilterScope, "SEHFilterScope"},
   {CompoundStmtScope, "CompoundStmtScope"},
-  {ClassInheritanceScope, "ClassInheritanceScope"},
-  {CatchScope, "CatchScope"},
-  };
+  {ClassInheritanceScope, "ClassInheritanceScope"}};
 
   for (auto Info : FlagInfo) {
 if (Flags & Info.first) {

Modified: cfe/trunk/lib/Sema/SemaCoroutine.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCoroutine.cpp?rev=356774&r1=356773&r2=356774&view=diff
==
--- cfe/trunk/lib/Sema/SemaCoroutine.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCoroutine.cpp Fri Mar 22 09:08:29 2019
@@ -185,8 +185,21 @@ static QualType lookupCoroutineHandleTyp
 
 static bool isValidCoroutineContext(Sema &S, SourceLocation Loc,
 StringRef Keyword) {
-  // [expr.await]p2 dictates that 'co_await' and 'c

[PATCH] D59076: [coroutines][PR40978] Emit error for co_yield within catch block

[Brian Gesiak via Phabricator via cfe-commits]

modocache added a comment.

Reverted in rC356296 . I'll rework this 
change along with a test confirming https://bugs.llvm.org/show_bug.cgi?id=41171 
doesn't occur. Apologies!


Repository:
  rC Clang

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

https://reviews.llvm.org/D59076



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

[PATCH] D59485: [ASTImporter] Add an ImportInternal method to allow customizing Import behavior.

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

In D59485#1439570 , @martong wrote:

> In D59485#1439390 , @teemperor wrote:
>
> > > Well, I still don't understand how LLDB synthesis the AST. 
> > >  So you have a C++ module in a .pcm file. This means you could create an 
> > > AST from that with ASTReader from it's .clang_ast section, right? In that 
> > > case the AST should be complete with all type information. If this was 
> > > the case then I don't see why it is not possible to use 
> > > clang::ASTImporter to import templates and specializations, since we do 
> > > exactly that in CTU.
> > > 
> > > Or do you guys create the AST from the debug info, from the .debug_info 
> > > section of .pcm module file? And this is why AST is incomplete? I've got 
> > > this from 
> > > https://youtu.be/EgkZ8PTNSHQ?list=PL85Cf-ok7HpppFWlp2wX_-A1dkyFVlaEB&t=1565
> > >  If this is the case, then comes my naiive quiestion: Why don't you use 
> > > the ASTReader?
> >
> > There are no C++ modules involved in our use case beside the generic `std` 
> > module. No user-written code is read from a module and we don't have any 
> > PCM file beside the `std` module we build for the expression evaluator.
> >
> > We use the ASTReader to directly read the `std` module contents into the 
> > expression evaluation ASTContext, but this doesn't give us the decls for 
> > the instantiations the user has made (e.g. `std::vector`). We only 
> > have these user instantiations in the 'normal' debug info where we have a 
> > rather minimal AST (again, no modules are not involved in building this 
> > debug info AST). The `InternalImport` in the LLDB patch just stitches the 
> > module AST and the debug info AST together when we import something that we 
> > also have (in better quality from the C++ module) in the target ASTContext.
>
>
> Thank you for the explanation.
>  Now I understand you directly create specializations from the std module and 
> intercept the import to avoid importing broken specializations from the debug 
> info, this makes sense.


Really, just one last question to see the whole picture: If clang::ASTImporter 
were capable of handling a specialization/instantiation with missing info then 
we could use that. So the reason for this interception is that 
clang::ASTImporter::VisitClassTemplateSpecializationDecl cannot handle the 
specialization it receives because that or its dependent nodes has too many 
missing data, right?


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] D59700: Make clang-move use same file naming convention as other tools

[Nico Weber via Phabricator via cfe-commits]

thakis created this revision.
thakis added a reviewer: hokein.
Herald added subscribers: llvm-commits, ioeric, mgorny.
Herald added a project: LLVM.

In all the other clang-foo tools, the main library file is called
Foo.cpp and the file in the tool/ folder is called ClangFoo.cpp.
Do this for clang-move too.

No intended behavior change.


https://reviews.llvm.org/D59700

Files:
  clang-tools-extra/clang-move/CMakeLists.txt
  clang-tools-extra/clang-move/ClangMove.cpp
  clang-tools-extra/clang-move/ClangMove.h
  clang-tools-extra/clang-move/HelperDeclRefGraph.cpp
  clang-tools-extra/clang-move/Move.cpp
  clang-tools-extra/clang-move/Move.h
  clang-tools-extra/clang-move/tool/CMakeLists.txt
  clang-tools-extra/clang-move/tool/ClangMove.cpp
  clang-tools-extra/clang-move/tool/ClangMoveMain.cpp
  llvm/utils/gn/secondary/clang-tools-extra/clang-move/BUILD.gn
  llvm/utils/gn/secondary/clang-tools-extra/clang-move/tool/BUILD.gn


Index: llvm/utils/gn/secondary/clang-tools-extra/clang-move/tool/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-move/tool/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-move/tool/BUILD.gn
@@ -14,6 +14,6 @@
   ]
   include_dirs = [ ".." ]
   sources = [
-"ClangMoveMain.cpp",
+"ClangMove.cpp",
   ]
 }
Index: llvm/utils/gn/secondary/clang-tools-extra/clang-move/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-move/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-move/BUILD.gn
@@ -15,7 +15,7 @@
 "//llvm/lib/Support",
   ]
   sources = [
-"ClangMove.cpp",
+"Move.cpp",
 "HelperDeclRefGraph.cpp",
   ]
 }
Index: clang-tools-extra/clang-move/tool/ClangMove.cpp
===
--- clang-tools-extra/clang-move/tool/ClangMove.cpp
+++ clang-tools-extra/clang-move/tool/ClangMove.cpp
@@ -6,7 +6,7 @@
 //
 
//===--===//
 
-#include "ClangMove.h"
+#include "Move.h"
 #include "clang/Frontend/TextDiagnosticPrinter.h"
 #include "clang/Rewrite/Core/Rewriter.h"
 #include "clang/Tooling/ArgumentsAdjusters.h"
Index: clang-tools-extra/clang-move/tool/CMakeLists.txt
===
--- clang-tools-extra/clang-move/tool/CMakeLists.txt
+++ clang-tools-extra/clang-move/tool/CMakeLists.txt
@@ -1,7 +1,7 @@
 include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
 
 add_clang_executable(clang-move
-  ClangMoveMain.cpp
+  ClangMove.cpp
   )
 
 target_link_libraries(clang-move
Index: clang-tools-extra/clang-move/Move.cpp
===
--- clang-tools-extra/clang-move/Move.cpp
+++ clang-tools-extra/clang-move/Move.cpp
@@ -6,7 +6,7 @@
 //
 
//===--===//
 
-#include "ClangMove.h"
+#include "Move.h"
 #include "HelperDeclRefGraph.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Basic/SourceManager.h"
Index: clang-tools-extra/clang-move/HelperDeclRefGraph.cpp
===
--- clang-tools-extra/clang-move/HelperDeclRefGraph.cpp
+++ clang-tools-extra/clang-move/HelperDeclRefGraph.cpp
@@ -7,7 +7,7 @@
 
//===--===//
 
 #include "HelperDeclRefGraph.h"
-#include "ClangMove.h"
+#include "Move.h"
 #include "clang/AST/Decl.h"
 #include "llvm/Support/Debug.h"
 #include 
Index: clang-tools-extra/clang-move/CMakeLists.txt
===
--- clang-tools-extra/clang-move/CMakeLists.txt
+++ clang-tools-extra/clang-move/CMakeLists.txt
@@ -3,7 +3,7 @@
   )
 
 add_clang_library(clangMove
-  ClangMove.cpp
+  Move.cpp
   HelperDeclRefGraph.cpp
 
   LINK_LIBS


Index: llvm/utils/gn/secondary/clang-tools-extra/clang-move/tool/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-move/tool/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-move/tool/BUILD.gn
@@ -14,6 +14,6 @@
   ]
   include_dirs = [ ".." ]
   sources = [
-"ClangMoveMain.cpp",
+"ClangMove.cpp",
   ]
 }
Index: llvm/utils/gn/secondary/clang-tools-extra/clang-move/BUILD.gn
===
--- llvm/utils/gn/secondary/clang-tools-extra/clang-move/BUILD.gn
+++ llvm/utils/gn/secondary/clang-tools-extra/clang-move/BUILD.gn
@@ -15,7 +15,7 @@
 "//llvm/lib/Support",
   ]
   sources = [
-"ClangMove.cpp",
+"Move.cpp",
 "HelperDeclRefGraph.cpp",
   ]
 }
Index: clang-tools-extra/clang-move/tool/ClangMove.cpp
===
--- clang-tools-extra/clang-move/tool/ClangMove.cpp
+++ clang-tools-extra/clang-move/to

[PATCH] D59700: Make clang-move use same file naming convention as other tools

[Eric Liu via Phabricator via cfe-commits]

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

lgtm


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

https://reviews.llvm.org/D59700



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

r356776 - [ARM] Fix bug 39982 - pcs("aapcs-vfp") is not consistent

[Carey Williams via cfe-commits]

Author: carwil
Date: Fri Mar 22 09:20:45 2019
New Revision: 356776

URL: http://llvm.org/viewvc/llvm-project?rev=356776&view=rev
Log:
[ARM] Fix bug 39982 - pcs("aapcs-vfp") is not consistent

Correctly handle homogeneous aggregates when a
function's ABI is specified via the pcs attribute.

Bug: https://bugs.llvm.org/show_bug.cgi?id=39982
Differential Revision: https://reviews.llvm.org/D59094

Added:
cfe/trunk/test/CodeGenCXX/arm-pcs.cpp
Modified:
cfe/trunk/lib/CodeGen/TargetInfo.cpp

Modified: cfe/trunk/lib/CodeGen/TargetInfo.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/TargetInfo.cpp?rev=356776&r1=356775&r2=356776&view=diff
==
--- cfe/trunk/lib/CodeGen/TargetInfo.cpp (original)
+++ cfe/trunk/lib/CodeGen/TargetInfo.cpp Fri Mar 22 09:20:45 2019
@@ -5597,8 +5597,10 @@ public:
   ABIKind getABIKind() const { return Kind; }
 
 private:
-  ABIArgInfo classifyReturnType(QualType RetTy, bool isVariadic) const;
-  ABIArgInfo classifyArgumentType(QualType RetTy, bool isVariadic) const;
+  ABIArgInfo classifyReturnType(QualType RetTy, bool isVariadic,
+unsigned functionCallConv) const;
+  ABIArgInfo classifyArgumentType(QualType RetTy, bool isVariadic,
+  unsigned functionCallConv) const;
   ABIArgInfo classifyHomogeneousAggregate(QualType Ty, const Type *Base,
   uint64_t Members) const;
   ABIArgInfo coerceIllegalVector(QualType Ty) const;
@@ -5608,6 +5610,8 @@ private:
   bool isHomogeneousAggregateSmallEnough(const Type *Ty,
  uint64_t Members) const override;
 
+  bool isEffectivelyAAPCS_VFP(unsigned callConvention, bool acceptHalf) const;
+
   void computeInfo(CGFunctionInfo &FI) const override;
 
   Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
@@ -5728,11 +5732,13 @@ void WindowsARMTargetCodeGenInfo::setTar
 
 void ARMABIInfo::computeInfo(CGFunctionInfo &FI) const {
   if (!::classifyReturnType(getCXXABI(), FI, *this))
-FI.getReturnInfo() =
-classifyReturnType(FI.getReturnType(), FI.isVariadic());
+FI.getReturnInfo() = classifyReturnType(FI.getReturnType(), 
FI.isVariadic(),
+FI.getCallingConvention());
 
   for (auto &I : FI.arguments())
-I.info = classifyArgumentType(I.type, FI.isVariadic());
+I.info = classifyArgumentType(I.type, FI.isVariadic(),
+  FI.getCallingConvention());
+
 
   // Always honor user-specified calling convention.
   if (FI.getCallingConvention() != llvm::CallingConv::C)
@@ -5811,8 +5817,8 @@ ABIArgInfo ARMABIInfo::classifyHomogeneo
   return ABIArgInfo::getDirect(nullptr, 0, nullptr, false);
 }
 
-ABIArgInfo ARMABIInfo::classifyArgumentType(QualType Ty,
-bool isVariadic) const {
+ABIArgInfo ARMABIInfo::classifyArgumentType(QualType Ty, bool isVariadic,
+unsigned functionCallConv) const {
   // 6.1.2.1 The following argument types are VFP CPRCs:
   //   A single-precision floating-point type (including promoted
   //   half-precision types); A double-precision floating-point type;
@@ -5820,7 +5826,9 @@ ABIArgInfo ARMABIInfo::classifyArgumentT
   //   with a Base Type of a single- or double-precision floating-point type,
   //   64-bit containerized vectors or 128-bit containerized vectors with one
   //   to four Elements.
-  bool IsEffectivelyAAPCS_VFP = getABIKind() == AAPCS_VFP && !isVariadic;
+  // Variadic functions should always marshal to the base standard.
+  bool IsAAPCS_VFP =
+  !isVariadic && isEffectivelyAAPCS_VFP(functionCallConv, /* AAPCS16 */ 
false);
 
   Ty = useFirstFieldIfTransparentUnion(Ty);
 
@@ -5833,7 +5841,7 @@ ABIArgInfo ARMABIInfo::classifyArgumentT
   // half type natively, and does not need to interwork with AAPCS code.
   if ((Ty->isFloat16Type() || Ty->isHalfType()) &&
   !getContext().getLangOpts().NativeHalfArgsAndReturns) {
-llvm::Type *ResType = IsEffectivelyAAPCS_VFP ?
+llvm::Type *ResType = IsAAPCS_VFP ?
   llvm::Type::getFloatTy(getVMContext()) :
   llvm::Type::getInt32Ty(getVMContext());
 return ABIArgInfo::getDirect(ResType);
@@ -5857,7 +5865,7 @@ ABIArgInfo ARMABIInfo::classifyArgumentT
   if (isEmptyRecord(getContext(), Ty, true))
 return ABIArgInfo::getIgnore();
 
-  if (IsEffectivelyAAPCS_VFP) {
+  if (IsAAPCS_VFP) {
 // Homogeneous Aggregates need to be expanded when we can fit the aggregate
 // into VFP registers.
 const Type *Base = nullptr;
@@ -6014,10 +6022,12 @@ static bool isIntegerLikeType(QualType T
   return true;
 }
 
-ABIArgInfo ARMABIInfo::classifyReturnType(QualType RetTy,
-  bool isVariadic) const {
-  bool IsEffectivelyAAPCS_VFP =
-  (getABIKind() == AAPCS_VFP || getABIKind() == AAPCS16_VFP) && 

[PATCH] D59094: [ARM] Fix bug 39982 - pcs("aapcs-vfp") is not consistent

[Carey Williams via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC356776: [ARM] Fix bug 39982 - pcs("aapcs-vfp") is 
not consistent (authored by carwil, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D59094?vs=190798&id=191895#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D59094

Files:
  lib/CodeGen/TargetInfo.cpp
  test/CodeGenCXX/arm-pcs.cpp

Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -5597,8 +5597,10 @@
   ABIKind getABIKind() const { return Kind; }
 
 private:
-  ABIArgInfo classifyReturnType(QualType RetTy, bool isVariadic) const;
-  ABIArgInfo classifyArgumentType(QualType RetTy, bool isVariadic) const;
+  ABIArgInfo classifyReturnType(QualType RetTy, bool isVariadic,
+unsigned functionCallConv) const;
+  ABIArgInfo classifyArgumentType(QualType RetTy, bool isVariadic,
+  unsigned functionCallConv) const;
   ABIArgInfo classifyHomogeneousAggregate(QualType Ty, const Type *Base,
   uint64_t Members) const;
   ABIArgInfo coerceIllegalVector(QualType Ty) const;
@@ -5608,6 +5610,8 @@
   bool isHomogeneousAggregateSmallEnough(const Type *Ty,
  uint64_t Members) const override;
 
+  bool isEffectivelyAAPCS_VFP(unsigned callConvention, bool acceptHalf) const;
+
   void computeInfo(CGFunctionInfo &FI) const override;
 
   Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
@@ -5728,11 +5732,13 @@
 
 void ARMABIInfo::computeInfo(CGFunctionInfo &FI) const {
   if (!::classifyReturnType(getCXXABI(), FI, *this))
-FI.getReturnInfo() =
-classifyReturnType(FI.getReturnType(), FI.isVariadic());
+FI.getReturnInfo() = classifyReturnType(FI.getReturnType(), FI.isVariadic(),
+FI.getCallingConvention());
 
   for (auto &I : FI.arguments())
-I.info = classifyArgumentType(I.type, FI.isVariadic());
+I.info = classifyArgumentType(I.type, FI.isVariadic(),
+  FI.getCallingConvention());
+
 
   // Always honor user-specified calling convention.
   if (FI.getCallingConvention() != llvm::CallingConv::C)
@@ -5811,8 +5817,8 @@
   return ABIArgInfo::getDirect(nullptr, 0, nullptr, false);
 }
 
-ABIArgInfo ARMABIInfo::classifyArgumentType(QualType Ty,
-bool isVariadic) const {
+ABIArgInfo ARMABIInfo::classifyArgumentType(QualType Ty, bool isVariadic,
+unsigned functionCallConv) const {
   // 6.1.2.1 The following argument types are VFP CPRCs:
   //   A single-precision floating-point type (including promoted
   //   half-precision types); A double-precision floating-point type;
@@ -5820,7 +5826,9 @@
   //   with a Base Type of a single- or double-precision floating-point type,
   //   64-bit containerized vectors or 128-bit containerized vectors with one
   //   to four Elements.
-  bool IsEffectivelyAAPCS_VFP = getABIKind() == AAPCS_VFP && !isVariadic;
+  // Variadic functions should always marshal to the base standard.
+  bool IsAAPCS_VFP =
+  !isVariadic && isEffectivelyAAPCS_VFP(functionCallConv, /* AAPCS16 */ false);
 
   Ty = useFirstFieldIfTransparentUnion(Ty);
 
@@ -5833,7 +5841,7 @@
   // half type natively, and does not need to interwork with AAPCS code.
   if ((Ty->isFloat16Type() || Ty->isHalfType()) &&
   !getContext().getLangOpts().NativeHalfArgsAndReturns) {
-llvm::Type *ResType = IsEffectivelyAAPCS_VFP ?
+llvm::Type *ResType = IsAAPCS_VFP ?
   llvm::Type::getFloatTy(getVMContext()) :
   llvm::Type::getInt32Ty(getVMContext());
 return ABIArgInfo::getDirect(ResType);
@@ -5857,7 +5865,7 @@
   if (isEmptyRecord(getContext(), Ty, true))
 return ABIArgInfo::getIgnore();
 
-  if (IsEffectivelyAAPCS_VFP) {
+  if (IsAAPCS_VFP) {
 // Homogeneous Aggregates need to be expanded when we can fit the aggregate
 // into VFP registers.
 const Type *Base = nullptr;
@@ -6014,10 +6022,12 @@
   return true;
 }
 
-ABIArgInfo ARMABIInfo::classifyReturnType(QualType RetTy,
-  bool isVariadic) const {
-  bool IsEffectivelyAAPCS_VFP =
-  (getABIKind() == AAPCS_VFP || getABIKind() == AAPCS16_VFP) && !isVariadic;
+ABIArgInfo ARMABIInfo::classifyReturnType(QualType RetTy, bool isVariadic,
+  unsigned functionCallConv) const {
+
+  // Variadic functions should always marshal to the base standard.
+  bool IsAAPCS_VFP =
+  !isVariadic && isEffectivelyAAPCS_VFP(functionCallConv, /* AAPCS16 */ true);
 
   if (RetTy->isVoidType())
 return ABIArgInfo::getIgnore();
@@ -6038,7 +6048,7 @@
   // half type natively, and does not need to interwork wi

[PATCH] D59233: libclang/CIndexer.cpp: Use loadquery() on AIX for path to library

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast marked 6 inline comments as done.
hubert.reinterpretcast added a comment.
Herald added a subscriber: jsji.

Thanks @xingxue for the review. I will update for the error handling before 
committing.




Comment at: tools/libclang/CIndexer.cpp:61
+if (errno != ENOMEM)
+  llvm_unreachable("Encountered an unexpected loadquery() failure");
+

hubert.reinterpretcast wrote:
> Based on available documentation, this situation should not occur (and thus I 
> believe `llvm_unreachable` is appropriate).
I'll leave this one as `llvm_unreachable`, either the failure is resolved on a 
later call or the loop will terminate when the amount of memory to request 
(checked below) is too large.



Comment at: tools/libclang/CIndexer.cpp:65
+if ((BufSize & ~((-1u) >> 1u)) != 0u)
+  llvm_unreachable("BufSize needed for loadquery() too large");
+

hubert.reinterpretcast wrote:
> This situation is not impossible, but highly improbable. This is a 
> non-programmatic error, and the Programmer's Manual appears to recommend the 
> use of `report_fatal_error`. Some additional guidance would be appreciated.
I will change this to `report_fatal_error`.



Comment at: tools/libclang/CIndexer.cpp:84
+if (CurInfo->ldinfo_next == 0u)
+  llvm_unreachable("Cannot locate entry point in the loadquery() results");
+CurInfo = reinterpret_cast(reinterpret_cast(CurInfo) +

hubert.reinterpretcast wrote:
> This is also supposed to not happen.
I will change this to `report_fatal_error`. Infinite loops are unfriendly.


Repository:
  rC Clang

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

https://reviews.llvm.org/D59233



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

[PATCH] D59467: [clang] Adding the Likely Attribute from C++2a to AST

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D59467#1439585 , @Tyker wrote:

> about the semantic issue.
>  with the standard's latitude on where we can put the attribute, the 
> attribute can appear any where on the path of execution and must be matched 
> with the condition it corresponds to. we need to do it for diagnostics 
> purposes to detect conflicts and repetitions. and we need it again for the 
> codegen. but where in the ast can we store this information ?


I feel like this could be tracked in the StmtBits as well as via an attributed 
stmt node. We need to be able to handle cases like:

  if (foo) [[likely]] {
stmts...
  }
  
  if (bar) {
stmts...
[[likely]];
stmts...
  }
  
  if (baz)
[[likely]] stmt;

The attributed statement is required because we want ast pretty printing to 
also print out the `[[likely]]` attribute usage and AST matchers to 
appropriately find the attribute, etc, and we need to know which statement the 
attribute was attached to in order to get the positioning correct. However, for 
diagnostic purposes, we likely want to know information like "has this path 
already been attributed" and that can either be through some extra data on an 
AST node, or it could be an Analysis pass like we do for `[[fallthrough]]` 
support with a separate pass for CodeGen.

> is adding information to conditional statements Node acceptable ?

You can generally add information to AST nodes, but pay close attention to 
things like adding bits to a bit-field that suddenly cause the object size to 
increase and see if there are ways to avoid that if possible (and if not, it's 
still fine).

It might make sense to work your way backwards from the backend to see what 
possible ways we can support this feature. `__builtin_expect` may make sense 
for if statements, but for case and default labels in a switch, we may want to 
play with the branch weights. For catch handlers, we may need to thread 
information through yet another way. Once we know what kinds of paths we can 
actually do something interesting with, we may spot a pattern of how to expose 
the information through the AST.


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

https://reviews.llvm.org/D59467



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

[PATCH] D59702: Unbreak the build of compiler-rt on Linux/mips64el

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru created this revision.
sylvestre.ledru added a reviewer: atanasyan.
Herald added subscribers: llvm-commits, Sanitizers, jdoerfert, delcypher, 
arichardson, dberris, kubamracek, sdardis.
Herald added projects: LLVM, Sanitizers.
sylvestre.ledru added a comment.

Not sure what I am doing but it fixes the issue


Fails with:

/<>/llvm-toolchain-8-8~+rc5/projects/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h:343:72:
 error: size of array 'assertion_failed__73' is negative

  typedef char IMPL_PASTE(assertion_failed_##_, line)[2*(int)(pred)-1]
 ^

/<>/llvm-toolchain-8-8~+rc5/projects/compiler-rt/lib/sanitizer_common/sanitizer_internal_defs.h:337:30:
 note: in expansion of macro 'IMPL_COMPILER_ASSERT'

  #define COMPILER_CHECK(pred) IMPL_COMPILER_ASSERT(pred, __LINE__)
  ^~~~

/<>/llvm-toolchain-8-8~+rc5/projects/compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_linux.cc:73:1:
 note: in expansion of macro 'COMPILER_CHECK'

  COMPILER_CHECK(struct_kernel_stat_sz == sizeof(struct stat));


Repository:
  rCRT Compiler Runtime

https://reviews.llvm.org/D59702

Files:
  lib/sanitizer_common/sanitizer_platform_limits_linux.cc
  lib/sanitizer_common/sanitizer_platform_limits_posix.h


Index: lib/sanitizer_common/sanitizer_platform_limits_posix.h
===
--- lib/sanitizer_common/sanitizer_platform_limits_posix.h
+++ lib/sanitizer_common/sanitizer_platform_limits_posix.h
@@ -81,7 +81,7 @@
 #elif defined(__mips__)
   const unsigned struct_kernel_stat_sz =
  SANITIZER_ANDROID ? FIRST_32_SECOND_64(104, 128) :
- FIRST_32_SECOND_64(160, 216);
+ FIRST_32_SECOND_64(144, 104);
   const unsigned struct_kernel_stat64_sz = 104;
 #elif defined(__s390__) && !defined(__s390x__)
   const unsigned struct_kernel_stat_sz = 64;
Index: lib/sanitizer_common/sanitizer_platform_limits_linux.cc
===
--- lib/sanitizer_common/sanitizer_platform_limits_linux.cc
+++ lib/sanitizer_common/sanitizer_platform_limits_linux.cc
@@ -28,7 +28,7 @@
 // are not defined anywhere in userspace headers. Fake them. This seems to work
 // fine with newer headers, too.
 #include 
-#if defined(__x86_64__) ||  defined(__mips__)
+#if defined(__x86_64__)
 #include 
 #else
 #define ino_t __kernel_ino_t


Index: lib/sanitizer_common/sanitizer_platform_limits_posix.h
===
--- lib/sanitizer_common/sanitizer_platform_limits_posix.h
+++ lib/sanitizer_common/sanitizer_platform_limits_posix.h
@@ -81,7 +81,7 @@
 #elif defined(__mips__)
   const unsigned struct_kernel_stat_sz =
  SANITIZER_ANDROID ? FIRST_32_SECOND_64(104, 128) :
- FIRST_32_SECOND_64(160, 216);
+ FIRST_32_SECOND_64(144, 104);
   const unsigned struct_kernel_stat64_sz = 104;
 #elif defined(__s390__) && !defined(__s390x__)
   const unsigned struct_kernel_stat_sz = 64;
Index: lib/sanitizer_common/sanitizer_platform_limits_linux.cc
===
--- lib/sanitizer_common/sanitizer_platform_limits_linux.cc
+++ lib/sanitizer_common/sanitizer_platform_limits_linux.cc
@@ -28,7 +28,7 @@
 // are not defined anywhere in userspace headers. Fake them. This seems to work
 // fine with newer headers, too.
 #include 
-#if defined(__x86_64__) ||  defined(__mips__)
+#if defined(__x86_64__)
 #include 
 #else
 #define ino_t __kernel_ino_t
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D59702: Unbreak the build of compiler-rt on Linux/mips64el

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

Not sure what I am doing but it fixes the issue


Repository:
  rCRT Compiler Runtime

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

https://reviews.llvm.org/D59702



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