[PATCH] D51402: [OpenCL] Adding cl_intel_planar_yuv extension

[Dmitry Sidorov via Phabricator via cfe-commits]

sidorovd created this revision.
sidorovd added reviewers: yaxunl, Anastasia.
Herald added a subscriber: cfe-commits.

Just adding a preprocessor #define for the extension.

Patch by Alexey Sotkin


Repository:
  rC Clang

https://reviews.llvm.org/D51402

Files:
  include/clang/Basic/OpenCLExtensions.def
  test/SemaOpenCL/extension-version.cl


Index: test/SemaOpenCL/extension-version.cl
===
--- test/SemaOpenCL/extension-version.cl
+++ test/SemaOpenCL/extension-version.cl
@@ -300,3 +300,11 @@
 #endif
 #pragma OPENCL EXTENSION cl_intel_subgroups_short : enable
 
+#if (__OPENCL_C_VERSION__ >= 120)
+#ifndef cl_intel_planar_yuv
+#error "Missing cl_intel_planar_yuv define"
+#endif
+#else
+// expected-warning@+2{{unsupported OpenCL extension 'cl_intel_planar_yuv' - 
ignoring}}
+#endif
+#pragma OPENCL EXTENSION cl_intel_planar_yuv : enable
Index: include/clang/Basic/OpenCLExtensions.def
===
--- include/clang/Basic/OpenCLExtensions.def
+++ include/clang/Basic/OpenCLExtensions.def
@@ -85,6 +85,7 @@
 // Intel OpenCL extensions
 OPENCLEXT_INTERNAL(cl_intel_subgroups, 120, ~0U)
 OPENCLEXT_INTERNAL(cl_intel_subgroups_short, 120, ~0U)
+OPENCLEXT_INTERNAL(cl_intel_planar_yuv, 120, ~0U)
 
 #undef OPENCLEXT_INTERNAL
 


Index: test/SemaOpenCL/extension-version.cl
===
--- test/SemaOpenCL/extension-version.cl
+++ test/SemaOpenCL/extension-version.cl
@@ -300,3 +300,11 @@
 #endif
 #pragma OPENCL EXTENSION cl_intel_subgroups_short : enable
 
+#if (__OPENCL_C_VERSION__ >= 120)
+#ifndef cl_intel_planar_yuv
+#error "Missing cl_intel_planar_yuv define"
+#endif
+#else
+// expected-warning@+2{{unsupported OpenCL extension 'cl_intel_planar_yuv' - ignoring}}
+#endif
+#pragma OPENCL EXTENSION cl_intel_planar_yuv : enable
Index: include/clang/Basic/OpenCLExtensions.def
===
--- include/clang/Basic/OpenCLExtensions.def
+++ include/clang/Basic/OpenCLExtensions.def
@@ -85,6 +85,7 @@
 // Intel OpenCL extensions
 OPENCLEXT_INTERNAL(cl_intel_subgroups, 120, ~0U)
 OPENCLEXT_INTERNAL(cl_intel_subgroups_short, 120, ~0U)
+OPENCLEXT_INTERNAL(cl_intel_planar_yuv, 120, ~0U)
 
 #undef OPENCLEXT_INTERNAL
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D48714: [clang-tidy] fix PR37913, templated exception factory diagnosed correctly

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D48714#1216989, @lebedev.ri wrote:

> In https://reviews.llvm.org/D48714#1216537, @JonasToth wrote:
>
> > I had to revert the `CHECK-NOTES` change that @lebedev.ri introduced with 
> > his revision. It fails the test, i think there is an inconsistency or so in 
> > the check-clang-tidy script. I will try to figure out whats the issue.
>
>
> So what was the issue? Do you get the same results if you undo the 
> https://reviews.llvm.org/D51381 and `s/CHECK-MESSAGES/CHECK-NOTES/`?


You are right that replacing all of it with `CHECK-NOTES` works as well.
`FileCheck` is run twice if you have `FIXMES` as well. Having another run for 
the notes is consistent with how it worked before.
If we go for the catch-all-with-one approach it might be a good idea to ensure 
that only one of `CHECK-MESSAGES` or `CHECK-NOTES` is present in the file and 
adjust the check_clang_tidy.py script a little.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48714



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

[PATCH] D51381: [clang-tidy] fix check_clang_tidy to properly mix CHECK-NOTES and CHECK-MESSAGES

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

@lebedev.ri lets do it in the the other patch, to not split discussions. But 
what do you mean by `You would still have to duplicate the check-lines for 
error: though.`?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51381



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

[PATCH] D51221: [clangd] Some nitpicking around the new split (preamble/main) dynamic index

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 163023.
sammccall added a comment.

(forgot some changes)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51221

Files:
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/ClangdUnit.cpp
  clangd/ClangdUnit.h
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/TUSchedulerTests.cpp

Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -45,7 +45,7 @@
 
 TEST_F(TUSchedulerTests, MissingFiles) {
   TUScheduler S(getDefaultAsyncThreadsCount(),
-/*StorePreamblesInMemory=*/true, noopParsingCallbacks(),
+/*StorePreamblesInMemory=*/true, nullptr,
 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
 ASTRetentionPolicy());
 
@@ -101,7 +101,7 @@
 Notification Ready;
 TUScheduler S(
 getDefaultAsyncThreadsCount(),
-/*StorePreamblesInMemory=*/true, noopParsingCallbacks(),
+/*StorePreamblesInMemory=*/true, nullptr,
 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
 ASTRetentionPolicy());
 auto Path = testPath("foo.cpp");
@@ -129,7 +129,7 @@
   std::atomic CallbackCount(0);
   {
 TUScheduler S(getDefaultAsyncThreadsCount(),
-  /*StorePreamblesInMemory=*/true, noopParsingCallbacks(),
+  /*StorePreamblesInMemory=*/true, nullptr,
   /*UpdateDebounce=*/std::chrono::seconds(1),
   ASTRetentionPolicy());
 // FIXME: we could probably use timeouts lower than 1 second here.
@@ -160,7 +160,7 @@
   // Run TUScheduler and collect some stats.
   {
 TUScheduler S(getDefaultAsyncThreadsCount(),
-  /*StorePreamblesInMemory=*/true, noopParsingCallbacks(),
+  /*StorePreamblesInMemory=*/true, nullptr,
   /*UpdateDebounce=*/std::chrono::milliseconds(50),
   ASTRetentionPolicy());
 
@@ -257,8 +257,7 @@
   ASTRetentionPolicy Policy;
   Policy.MaxRetainedASTs = 2;
   TUScheduler S(
-  /*AsyncThreadsCount=*/1, /*StorePreambleInMemory=*/true,
-  noopParsingCallbacks(),
+  /*AsyncThreadsCount=*/1, /*StorePreambleInMemory=*/true, nullptr,
   /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(), Policy);
 
   llvm::StringLiteral SourceContents = R"cpp(
@@ -307,8 +306,7 @@
 
 TEST_F(TUSchedulerTests, EmptyPreamble) {
   TUScheduler S(
-  /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
-  noopParsingCallbacks(),
+  /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true, nullptr,
   /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
   ASTRetentionPolicy());
 
@@ -354,8 +352,7 @@
   // Testing strategy: we update the file and schedule a few preamble reads at
   // the same time. All reads should get the same non-null preamble.
   TUScheduler S(
-  /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true,
-  noopParsingCallbacks(),
+  /*AsyncThreadsCount=*/4, /*StorePreambleInMemory=*/true, nullptr,
   /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
   ASTRetentionPolicy());
   auto Foo = testPath("foo.cpp");
@@ -388,7 +385,7 @@
 TEST_F(TUSchedulerTests, NoopOnEmptyChanges) {
   TUScheduler S(
   /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
-  /*StorePreambleInMemory=*/true, noopParsingCallbacks(),
+  /*StorePreambleInMemory=*/true, nullptr,
   /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
   ASTRetentionPolicy());
 
@@ -441,7 +438,7 @@
 TEST_F(TUSchedulerTests, NoChangeDiags) {
   TUScheduler S(
   /*AsyncThreadsCount=*/getDefaultAsyncThreadsCount(),
-  /*StorePreambleInMemory=*/true, noopParsingCallbacks(),
+  /*StorePreambleInMemory=*/true, nullptr,
   /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
   ASTRetentionPolicy());
 
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -251,11 +251,10 @@
   buildPreamble(
   FooCpp, *CI, /*OldPreamble=*/nullptr, tooling::CompileCommand(), PI,
   std::make_shared(), /*StoreInMemory=*/true,
-  [&Index, &IndexUpdated](PathRef FilePath, ASTContext &Ctx,
-  std::shared_ptr PP) {
+  [&](ASTContext &Ctx, std::shared_ptr PP) {
 EXPECT_FALSE(IndexUpdated) << "Expected only a single index update";
 IndexUpdated = true;
-Index.update(FilePath, &Ctx, std::move(PP));
+Index.update(FooCpp, &Ctx, std::move(PP));
   });
   ASSERT_TRUE(IndexUpdated);
 
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUSc

[PATCH] D51358: [driver] Do not pass "-flavor old-gnu" option to LLD linker

[Simon Atanasyan via Phabricator via cfe-commits]

atanasyan closed this revision.
atanasyan added a comment.

Fixed by https://reviews.llvm.org/rL340845.


Repository:
  rC Clang

https://reviews.llvm.org/D51358



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

[PATCH] D51310: [clangd] Implement iterator cost

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 163024.
kbobyrev marked 6 inline comments as done.
This revision is now accepted and ready to land.

https://reviews.llvm.org/D51310

Files:
  clang-tools-extra/clangd/index/dex/Iterator.cpp
  clang-tools-extra/clangd/index/dex/Iterator.h
  clang-tools-extra/unittests/clangd/DexIndexTests.cpp

Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -259,8 +259,8 @@
   createOr(create(L3), create(L4), create(L5)));
 
   EXPECT_EQ(llvm::to_string(*Nested),
-"(& (& [{1}, 3, 5, 8, 9, END] [{1}, 5, 7, 9, END]) (| [0, {5}, "
-"END] [0, {1}, 5, END] [{END}]))");
+"(& (| [{END}] [0, {5}, END] [0, {1}, 5, END]) (& [{1}, 5, 7, 9, "
+"END] [{1}, 3, 5, 8, 9, END]))");
 }
 
 TEST(DexIndexIterators, Limit) {
Index: clang-tools-extra/clangd/index/dex/Iterator.h
===
--- clang-tools-extra/clangd/index/dex/Iterator.h
+++ clang-tools-extra/clangd/index/dex/Iterator.h
@@ -95,6 +95,8 @@
   /// consume() must *not* be called on children that don't contain the current
   /// doc.
   virtual float consume() = 0;
+  /// Returns an estimate of advance() calls before the iterator is exhausted.
+  virtual size_t estimateSize() const = 0;
 
   virtual ~Iterator() {}
 
Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -48,6 +48,8 @@
 
   float consume() override { return DEFAULT_BOOST_SCORE; }
 
+  size_t estimateSize() const override { return Documents.size(); }
+
 private:
   llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
 OS << '[';
@@ -85,6 +87,18 @@
 assert(!Children.empty() && "AndIterator should have at least one child.");
 // Establish invariants.
 sync();
+// When children are sorted by the estimateSize(), sync() calls are more
+// effective. Each sync() starts with the first child and makes sure all
+// children point to the same element. If any child is "above" the previous
+// ones, the algorithm resets and and advances the children to the next
+// highest element starting from the front. When child iterators in the
+// beginning have smaller estimated size, the sync() will have less restarts
+// and become more effective.
+std::sort(begin(Children), end(Children),
+  [](const std::unique_ptr &LHS,
+ const std::unique_ptr &RHS) {
+return LHS->estimateSize() < RHS->estimateSize();
+  });
   }
 
   bool reachedEnd() const override { return ReachedEnd; }
@@ -114,6 +128,10 @@
 });
   }
 
+  size_t estimateSize() const override {
+return Children.front()->estimateSize();
+  }
+
 private:
   llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
 OS << "(& ";
@@ -146,9 +164,6 @@
   return;
 // If any child goes beyond given ID (i.e. ID is not the common item),
 // all children should be advanced to the next common item.
-// FIXME(kbobyrev): This is not a very optimized version; after costs
-// are introduced, cycle should break whenever ID exceeds current one
-// and cheapest children should be advanced over again.
 if (Child->peek() > SyncID) {
   SyncID = Child->peek();
   NeedsAdvance = true;
@@ -178,6 +193,7 @@
   OrIterator(std::vector> AllChildren)
   : Children(std::move(AllChildren)) {
 assert(Children.size() > 0 && "Or Iterator must have at least one child.");
+std::sort(begin(Children), end(Children));
   }
 
   /// Returns true if all children are exhausted.
@@ -235,6 +251,14 @@
 });
   }
 
+  size_t estimateSize() const override {
+return std::accumulate(
+begin(Children), end(Children), Children.front()->estimateSize(),
+[&](size_t Current, const std::unique_ptr &Child) {
+  return std::max(Current, Child->estimateSize());
+});
+  }
+
 private:
   llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
 OS << "(| ";
@@ -277,6 +301,8 @@
 
   float consume() override { return DEFAULT_BOOST_SCORE; }
 
+  size_t estimateSize() const override { return Size; }
+
 private:
   llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
 OS << "(TRUE {" << Index << "} out of " << Size << ")";
@@ -305,6 +331,8 @@
 
   float consume() override { return Child->consume() * Factor; }
 
+  size_t estimateSize() const override { return Child->estimateSize(); }
+
 private:
   llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
 OS << "(BOOST " << Factor << ' ' << *Child << ')';
@@ -342,6 +370,10 

[PATCH] D51310: [clangd] Implement iterator cost

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:93
+ const std::unique_ptr &RHS) {
+return LHS->cost() < RHS->cost();
+  });

sammccall wrote:
> (I'd actually suggest declaring operator< in iterator.h and making it compare 
> by cost, it seems natural/important enough, and saves duplicating this lambda)
Actually, I think sorting the children of OR iterator doesn't improve 
performance so I should probably just have a lambda in one place (`AND`) as it 
won't be used anywhere else.



Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:125
+  size_t cost() override {
+return std::accumulate(
+begin(Children), end(Children), Children.front()->cost(),

sammccall wrote:
> sammccall wrote:
> > The use of `std::accumulate` here seems less clear than the direct:
> > ```size_t Smallest = ...;
> > for (const auto& Child : Children)
> >   Smallest = std::min(Smallest, Child->cost());
> > return Smallest;
> > ```
> > For better or worse, functional styles don't have the most natural syntax 
> > in C++, and (IMO) shouldn't be used just because they fit.
> > 
> > (This is consistent with other places, but I also think that the same 
> > applies there)
> actually, we've sorted by cost, so don't we just want Children.front().cost() 
> here?
It's a sad day for the noble λ knights, but I agree.

I think I should push it like this in this patch (to comply with the common 
style) and then send out a separate patch with refactorings (I had few concerns 
about the inconsistency in the `Iterator.cpp`).


https://reviews.llvm.org/D51310



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

[PATCH] D51292: [docs] Update clang-rename documentation

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 163025.
kbobyrev marked 3 inline comments as done.
kbobyrev added a comment.

Moved the note about Clangd integration to the end, rephrased a bit.


https://reviews.llvm.org/D51292

Files:
  clang-tools-extra/docs/clang-rename.rst


Index: clang-tools-extra/docs/clang-rename.rst
===
--- clang-tools-extra/docs/clang-rename.rst
+++ clang-tools-extra/docs/clang-rename.rst
@@ -134,13 +134,23 @@
 -pn- Print the found symbol's name prior to 
renaming to stderr.
 -qualified-name=   - The fully qualified name of the symbol.
 
+Clangd Integration
+==
+
+:program:`clangd `_ uses
+:program:`clang-rename` infrastructure to handle renaming requests. Currently,
+it only supports renaming symbol within a single file, but in the future it 
will
+have much better support than the standalone tool. Consider switching to
+:program:`clangd` in the future since it is supported in most editors such as
+Vim, Emacs and Visual Studio Code.
+
 Vim Integration
 ===
 
 You can call :program:`clang-rename` directly from Vim! To set up
 :program:`clang-rename` integration for Vim see
 `clang-rename/tool/clang-rename.py
-`_.
+`_.
 
 Please note that **you have to save all buffers, in which the replacement will
 happen before running the tool**.
@@ -157,7 +167,7 @@
 You can also use :program:`clang-rename` while using Emacs! To set up
 :program:`clang-rename` integration for Emacs see
 `clang-rename/tool/clang-rename.el
-`_.
+`_.
 
 Once installed, you can point your cursor to symbols you want to rename, press
 `M-X`, type `clang-rename` and new desired name.


Index: clang-tools-extra/docs/clang-rename.rst
===
--- clang-tools-extra/docs/clang-rename.rst
+++ clang-tools-extra/docs/clang-rename.rst
@@ -134,13 +134,23 @@
 -pn- Print the found symbol's name prior to renaming to stderr.
 -qualified-name=   - The fully qualified name of the symbol.
 
+Clangd Integration
+==
+
+:program:`clangd `_ uses
+:program:`clang-rename` infrastructure to handle renaming requests. Currently,
+it only supports renaming symbol within a single file, but in the future it will
+have much better support than the standalone tool. Consider switching to
+:program:`clangd` in the future since it is supported in most editors such as
+Vim, Emacs and Visual Studio Code.
+
 Vim Integration
 ===
 
 You can call :program:`clang-rename` directly from Vim! To set up
 :program:`clang-rename` integration for Vim see
 `clang-rename/tool/clang-rename.py
-`_.
+`_.
 
 Please note that **you have to save all buffers, in which the replacement will
 happen before running the tool**.
@@ -157,7 +167,7 @@
 You can also use :program:`clang-rename` while using Emacs! To set up
 :program:`clang-rename` integration for Emacs see
 `clang-rename/tool/clang-rename.el
-`_.
+`_.
 
 Once installed, you can point your cursor to symbols you want to rename, press
 `M-X`, type `clang-rename` and new desired name.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51407: [Tooling] Do not restore working dir in ClangTool

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: ioeric, sammccall.

Resolve all relative paths before running the tool instead.

This fixes the usage of ClangTool in AllTUsExecutor. The executor will
try running multiple ClangTool instances in parallel with compile
commands that usually have the same working directory.

Changing working directory is a global operation, so we end up
changing working directory in the middle of running other actions,
which leads to spurious compile errors.


Repository:
  rC Clang

https://reviews.llvm.org/D51407

Files:
  lib/Tooling/Tooling.cpp


Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -411,15 +411,6 @@
   // This just needs to be some symbol in the binary.
   static int StaticSymbol;
 
-  std::string InitialDirectory;
-  if (llvm::ErrorOr CWD =
-  OverlayFileSystem->getCurrentWorkingDirectory()) {
-InitialDirectory = std::move(*CWD);
-  } else {
-llvm::report_fatal_error("Cannot detect current path: " +
- Twine(CWD.getError().message()));
-  }
-
   // First insert all absolute paths into the in-memory VFS. These are global
   // for all compile commands.
   if (SeenWorkingDirectories.insert("/").second)
@@ -431,9 +422,14 @@
 
   bool ProcessingFailed = false;
   bool FileSkipped = false;
-  for (const auto &SourcePath : SourcePaths) {
-std::string File(getAbsolutePath(SourcePath));
-
+  // Compute all absolute paths before we run any actions, as those will change
+  // the working directory.
+  std::vector AbsolutePaths;
+  AbsolutePaths.reserve(SourcePaths.size());
+  for (const auto &SourcePath : SourcePaths)
+AbsolutePaths.push_back(getAbsolutePath(SourcePath));
+
+  for (llvm::StringRef File : AbsolutePaths) {
 // Currently implementations of CompilationDatabase::getCompileCommands can
 // change the state of the file system (e.g.  prepare generated headers), 
so
 // this method needs to run right before we invoke the tool, as the next
@@ -498,11 +494,6 @@
 llvm::errs() << "Error while processing " << File << ".\n";
 ProcessingFailed = true;
   }
-  // Return to the initial directory to correctly resolve next file by
-  // relative path.
-  if 
(OverlayFileSystem->setCurrentWorkingDirectory(InitialDirectory.c_str()))
-llvm::report_fatal_error("Cannot chdir into \"" +
- Twine(InitialDirectory) + "\n!");
 }
   }
   return ProcessingFailed ? 1 : (FileSkipped ? 2 : 0);


Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -411,15 +411,6 @@
   // This just needs to be some symbol in the binary.
   static int StaticSymbol;
 
-  std::string InitialDirectory;
-  if (llvm::ErrorOr CWD =
-  OverlayFileSystem->getCurrentWorkingDirectory()) {
-InitialDirectory = std::move(*CWD);
-  } else {
-llvm::report_fatal_error("Cannot detect current path: " +
- Twine(CWD.getError().message()));
-  }
-
   // First insert all absolute paths into the in-memory VFS. These are global
   // for all compile commands.
   if (SeenWorkingDirectories.insert("/").second)
@@ -431,9 +422,14 @@
 
   bool ProcessingFailed = false;
   bool FileSkipped = false;
-  for (const auto &SourcePath : SourcePaths) {
-std::string File(getAbsolutePath(SourcePath));
-
+  // Compute all absolute paths before we run any actions, as those will change
+  // the working directory.
+  std::vector AbsolutePaths;
+  AbsolutePaths.reserve(SourcePaths.size());
+  for (const auto &SourcePath : SourcePaths)
+AbsolutePaths.push_back(getAbsolutePath(SourcePath));
+
+  for (llvm::StringRef File : AbsolutePaths) {
 // Currently implementations of CompilationDatabase::getCompileCommands can
 // change the state of the file system (e.g.  prepare generated headers), so
 // this method needs to run right before we invoke the tool, as the next
@@ -498,11 +494,6 @@
 llvm::errs() << "Error while processing " << File << ".\n";
 ProcessingFailed = true;
   }
-  // Return to the initial directory to correctly resolve next file by
-  // relative path.
-  if (OverlayFileSystem->setCurrentWorkingDirectory(InitialDirectory.c_str()))
-llvm::report_fatal_error("Cannot chdir into \"" +
- Twine(InitialDirectory) + "\n!");
 }
   }
   return ProcessingFailed ? 1 : (FileSkipped ? 2 : 0);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51407: [Tooling] Do not restore working dir in ClangTool

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: lib/Tooling/Tooling.cpp:419
-  } else {
-llvm::report_fatal_error("Cannot detect current path: " +
- Twine(CWD.getError().message()));

Should we still check if `CWD` is correctly set?


Repository:
  rC Clang

https://reviews.llvm.org/D51407



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

[PATCH] D51382: [MinGW] Don't mark external variables as DSO local

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: lib/CodeGen/CodeGenModule.cpp:737
+// potentially could come from another DLL as DSO local.
+if (GV->hasExternalLinkage() && GV->isDeclaration() &&
+isa(GV) && !GV->isThreadLocal())

rnk wrote:
> I think this linkage and declaration check should be 
> `GV->isDeclarationForLinker()`, since that will catch `available_externally` 
> globals as well. Those would come from a static data member of a class 
> template that has an explicit instantiation declaration:
> ```
> template 
> struct Foo {
>   static const int x = 42;
> };
> extern template struct Foo;
> const int *bar() {
>   return &Foo::x;
> }
> ```
> Yep, that does it on x86_64-windows-gnu. We probably want a stub for 
> `Foo::x`.
> 
> Can you add a test at clang/test/CodeGenCXX/dso-local.cpp for this?
Sure, will do.


Repository:
  rC Clang

https://reviews.llvm.org/D51382



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

[PATCH] D51382: [MinGW] Don't mark external variables as DSO local

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo updated this revision to Diff 163033.
mstorsjo added a comment.

Changed the condition to GV->isDeclarationForLinker(), updated tests 
accordingly (weak_bar in CodeGen/dso-local-executable.c lost the dso_local 
flag) and added more tests in CodeGenCXX/dso-local-executable.cpp as requested.


https://reviews.llvm.org/D51382

Files:
  lib/CodeGen/CodeGenModule.cpp
  test/CodeGen/dllimport.c
  test/CodeGen/dso-local-executable.c
  test/CodeGenCXX/dllexport.cpp
  test/CodeGenCXX/dllimport-members.cpp
  test/CodeGenCXX/dllimport.cpp
  test/CodeGenCXX/dso-local-executable.cpp

Index: test/CodeGenCXX/dso-local-executable.cpp
===
--- test/CodeGenCXX/dso-local-executable.cpp
+++ test/CodeGenCXX/dso-local-executable.cpp
@@ -1,5 +1,6 @@
 // RUN: %clang_cc1 -triple x86_64-pc-linux -mrelocation-model static -O1 -emit-llvm %s -o - | FileCheck --check-prefix=STATIC %s
 // RUN: %clang_cc1 -triple x86_64-pc-linux -mrelocation-model static -fno-plt -O1 -emit-llvm %s -o - | FileCheck --check-prefix=NOPLT %s
+// RUN: %clang_cc1 -triple x86_64-w64-mingw32 -O1 -emit-llvm %s -o - | FileCheck --check-prefix=MINGW %s
 
 // STATIC-DAG: @_ZTV1C = linkonce_odr dso_local unnamed_addr constant
 // STATIC-DAG: @_ZTS1C = linkonce_odr dso_local constant
@@ -19,6 +20,15 @@
 // NOPLT-DAG: define dso_local void @_ZN1CC1Ev(
 // NOPLT-DAG: define linkonce_odr dso_local void @_ZN1C3fooEv(
 
+// MINGW-DAG: @_ZTV1C = linkonce_odr dso_local unnamed_addr constant
+// MINGW-DAG: @_ZTS1C = linkonce_odr dso_local constant
+// MINGW-DAG: @_ZTI1C = linkonce_odr dso_local constant
+// MINGW-DAG: @_ZZ14useStaticLocalvE3obj = linkonce_odr dso_local global
+// MINGW-DAG: @_ZGVZN5guard1gEvE1a = linkonce_odr dso_local global
+// MINGW-DAG: define dso_local void @_ZN1CC2Ev(
+// MINGW-DAG: define dso_local void @_ZN1CC1Ev(
+// MINGW-DAG: define linkonce_odr dso_local void @_ZN1C3fooEv(
+
 struct C {
   C();
   virtual void foo() {}
@@ -48,15 +58,23 @@
 } // namespace guard
 
 
+// STATIC-DAG: @_ZN5test23barIiE1xE = available_externally dso_local constant i32
 // STATIC-DAG: define available_externally dso_local void @_ZN5test23barIcEC1Ev(
+// NOPLT-DAG: @_ZN5test23barIiE1xE = available_externally dso_local constant i32
 // NOPLT-DAG: define available_externally void @_ZN5test23barIcEC1Ev(
+// MINGW-DAG: @_ZN5test23barIiE1xE = available_externally constant i32
+// MINGW-DAG: define available_externally dso_local void @_ZN5test23barIcEC1Ev(
 namespace test2 {
 void foo();
 template 
 struct bar {
   virtual void zed();
+  static const int x = 42;
   bar() { foo(); }
 };
 extern template class bar;
 bar abc;
+const int *getX() {
+  return &bar::x;
+}
 } // namespace test2
Index: test/CodeGenCXX/dllimport.cpp
===
--- test/CodeGenCXX/dllimport.cpp
+++ test/CodeGenCXX/dllimport.cpp
@@ -78,7 +78,7 @@
 // NB: MSC issues a warning and makes GlobalRedecl3 dllexport. We follow GCC
 // and drop the dllimport with a warning.
 // MSC-DAG: @"?GlobalRedecl3@@3HA" = external dso_local global i32
-// GNU-DAG: @GlobalRedecl3= external dso_local global i32
+// GNU-DAG: @GlobalRedecl3= external global i32
 __declspec(dllimport) extern int GlobalRedecl3;
   extern int GlobalRedecl3; // dllimport ignored
 USEVAR(GlobalRedecl3)
@@ -137,7 +137,7 @@
 USEVAR(VarTmplRedecl2)
 
 // MSC-DAG: @"??$VarTmplRedecl3@UImplicitInst_Imported3HA" = external dso_local global i32
-// GNU-DAG: @_Z14VarTmplRedecl3I21ImplicitInst_ImportedE  = external dso_local global i32
+// GNU-DAG: @_Z14VarTmplRedecl3I21ImplicitInst_ImportedE  = external global i32
 template __declspec(dllimport) extern int VarTmplRedecl3;
 template   extern int VarTmplRedecl3; // dllimport ignored
 USEVAR(VarTmplRedecl3)
Index: test/CodeGenCXX/dllimport-members.cpp
===
--- test/CodeGenCXX/dllimport-members.cpp
+++ test/CodeGenCXX/dllimport-members.cpp
@@ -854,7 +854,7 @@
 // Not importing specialization of a member variable template without explicit
 // dllimport.
 // MSC-DAG: @"??$ImportedStaticVar@UExplicitSpec_NotImported@@@MemVarTmpl@@2HB" = external dso_local constant i32
-// GNU-DAG: @_ZN10MemVarTmpl17ImportedStaticVarI24ExplicitSpec_NotImportedEE   = external dso_local constant i32
+// GNU-DAG: @_ZN10MemVarTmpl17ImportedStaticVarI24ExplicitSpec_NotImportedEE   = external constant i32
 template<> const int MemVarTmpl::ImportedStaticVar;
 USEMV(MemVarTmpl, ImportedStaticVar)
 
Index: test/CodeGenCXX/dllexport.cpp
===
--- test/CodeGenCXX/dllexport.cpp
+++ test/CodeGenCXX/dllexport.cpp
@@ -43,7 +43,7 @@
 
 // M64-DAG: @__ImageBase = external dso_local constant i8
 
-// GNU-DAG: @_ZTVN10__cxxabiv117__class_type_infoE = external dso_local global
+// GNU-DAG: @_ZTVN10__cxxabiv117__cl

[PATCH] D51212: [OpenCL][Docs] Release notes for OpenCL in Clang

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340910: [Docs] Release notes for OpenCL (authored by 
stulova, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51212?vs=162362&id=163034#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51212

Files:
  cfe/branches/release_70/docs/ReleaseNotes.rst


Index: cfe/branches/release_70/docs/ReleaseNotes.rst
===
--- cfe/branches/release_70/docs/ReleaseNotes.rst
+++ cfe/branches/release_70/docs/ReleaseNotes.rst
@@ -222,10 +222,31 @@
 
 ...
 
-OpenCL C Language Changes in Clang
---
+OpenCL C/C++ Language Changes in Clang
+--
 
-...
+Miscellaneous changes in OpenCL C:
+
+- Added ``cles_khr_int64`` extension.
+
+- Added bug fixes and simplifications to Clang blocks in OpenCL mode.
+
+- Added compiler flag ``-cl-uniform-work-group-size`` to allow extra compile 
time optimisation.
+
+- Propagate ``denorms-are-zero`` attribute to IR if ``-cl-denorms-are-zero`` 
is passed to the compiler.
+
+- Separated ``read_only`` and ``write_only`` pipe IR types.
+
+- Fixed address space for the ``__func__`` predefined macro.
+
+- Improved diagnostics of kernel argument types.
+
+
+Started OpenCL C++ support:
+
+- Added ``-std/-cl-std=c++``.
+
+- Added support for keywords.
 
 OpenMP Support in Clang
 --


Index: cfe/branches/release_70/docs/ReleaseNotes.rst
===
--- cfe/branches/release_70/docs/ReleaseNotes.rst
+++ cfe/branches/release_70/docs/ReleaseNotes.rst
@@ -222,10 +222,31 @@
 
 ...
 
-OpenCL C Language Changes in Clang
---
+OpenCL C/C++ Language Changes in Clang
+--
 
-...
+Miscellaneous changes in OpenCL C:
+
+- Added ``cles_khr_int64`` extension.
+
+- Added bug fixes and simplifications to Clang blocks in OpenCL mode.
+
+- Added compiler flag ``-cl-uniform-work-group-size`` to allow extra compile time optimisation.
+
+- Propagate ``denorms-are-zero`` attribute to IR if ``-cl-denorms-are-zero`` is passed to the compiler.
+
+- Separated ``read_only`` and ``write_only`` pipe IR types.
+
+- Fixed address space for the ``__func__`` predefined macro.
+
+- Improved diagnostics of kernel argument types.
+
+
+Started OpenCL C++ support:
+
+- Added ``-std/-cl-std=c++``.
+
+- Added support for keywords.
 
 OpenMP Support in Clang
 --
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51407: [Tooling] Do not restore working dir in ClangTool

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: lib/Tooling/Tooling.cpp:419
-  } else {
-llvm::report_fatal_error("Cannot detect current path: " +
- Twine(CWD.getError().message()));

ioeric wrote:
> Should we still check if `CWD` is correctly set?
Not sure we know which CWD is correct at this point. Do you mean to check that 
`getCurrentWorkingDirectory ` does not return an error?


Repository:
  rC Clang

https://reviews.llvm.org/D51407



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

r340911 - [ARM] Set __ARM_FEATURE_SIMD32 for +dsp cores

[Sam Parker via cfe-commits]

Author: sam_parker
Date: Wed Aug 29 03:39:03 2018
New Revision: 340911

URL: http://llvm.org/viewvc/llvm-project?rev=340911&view=rev
Log:
[ARM] Set __ARM_FEATURE_SIMD32 for +dsp cores

ARM_FEATURE_DSP is already set for targets with the +dsp feature. In
the backend, this target feature is also used to represent the
availability of the of the instructions that the ACLE guard through
the __ARM_FEATURE_SIMD32 macro. We don't have any cores that
implement one and not the other, so set this macro for cores later
than V6 or for Cortex-M cores that the target parser, or user, reports
that the 'dsp' instructions are supported.

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

Modified:
cfe/trunk/lib/Basic/Targets/ARM.cpp
cfe/trunk/test/Preprocessor/arm-acle-6.4.c

Modified: cfe/trunk/lib/Basic/Targets/ARM.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/ARM.cpp?rev=340911&r1=340910&r2=340911&view=diff
==
--- cfe/trunk/lib/Basic/Targets/ARM.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/ARM.cpp Wed Aug 29 03:39:03 2018
@@ -661,7 +661,7 @@ void ARMTargetInfo::getTargetDefines(con
   }
 
   // ACLE 6.4.9 32-bit SIMD instructions
-  if (ArchVersion >= 6 && (CPUProfile != "M" || CPUAttr == "7EM"))
+  if (ArchVersion >= 6 || (CPUProfile == "M" && DSP))
 Builder.defineMacro("__ARM_FEATURE_SIMD32", "1");
 
   // ACLE 6.4.10 Hardware Integer Divide

Modified: cfe/trunk/test/Preprocessor/arm-acle-6.4.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/arm-acle-6.4.c?rev=340911&r1=340910&r2=340911&view=diff
==
--- cfe/trunk/test/Preprocessor/arm-acle-6.4.c (original)
+++ cfe/trunk/test/Preprocessor/arm-acle-6.4.c Wed Aug 29 03:39:03 2018
@@ -174,10 +174,14 @@
 // CHECK-V7M: __ARM_FEATURE_SAT 1
 // CHECK-V7M: __ARM_FEATURE_UNALIGNED 1
 
-// RUN: %clang -target arm-none-linux-eabi -march=armv7e-m -x c -E -dM %s -o - 
| FileCheck %s -check-prefix CHECK-V7EM
+// RUN: %clang -target arm-none-linux-eabi -march=armv7e-m -x c -E -dM %s -o - 
| FileCheck %s -check-prefix CHECK-M-DSP
+// RUN: %clang --target=arm-arm-none-eabi -mcpu=cortex-m4 -x c -E -dM %s -o - 
| FileCheck %s -check-prefix CHECK-M-DSP
+// RUN: %clang --target=arm-arm-none-eabi -mcpu=cortex-m7 -x c -E -dM %s -o - 
| FileCheck %s -check-prefix CHECK-M-DSP
+// RUN: %clang --target=arm-arm-none-eabi -mcpu=cortex-m33 -x c -E -dM %s -o - 
| FileCheck %s -check-prefix CHECK-M-DSP
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8m.main+dsp -x c -E -dM 
%s -o - | FileCheck %s -check-prefix CHECK-M-DSP
 
-// CHECK-V7EM: __ARM_FEATURE_DSP 1
-// CHECK-V7EM: __ARM_FEATURE_SIMD32 1
+// CHECK-M-DSP: __ARM_FEATURE_DSP 1
+// CHECK-M-DSP: __ARM_FEATURE_SIMD32 1
 
 // RUN: %clang -target arm-none-linux-eabi -march=armv8-a -x c -E -dM %s -o - 
| FileCheck %s -check-prefix CHECK-V8A
 


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

[PATCH] D51093: [ARM] Set __ARM_FEATURE_SIMD32 for +dsp cores

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340911: [ARM] Set __ARM_FEATURE_SIMD32 for +dsp cores 
(authored by sam_parker, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51093?vs=161925&id=163035#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51093

Files:
  cfe/trunk/lib/Basic/Targets/ARM.cpp
  cfe/trunk/test/Preprocessor/arm-acle-6.4.c


Index: cfe/trunk/lib/Basic/Targets/ARM.cpp
===
--- cfe/trunk/lib/Basic/Targets/ARM.cpp
+++ cfe/trunk/lib/Basic/Targets/ARM.cpp
@@ -661,7 +661,7 @@
   }
 
   // ACLE 6.4.9 32-bit SIMD instructions
-  if (ArchVersion >= 6 && (CPUProfile != "M" || CPUAttr == "7EM"))
+  if (ArchVersion >= 6 || (CPUProfile == "M" && DSP))
 Builder.defineMacro("__ARM_FEATURE_SIMD32", "1");
 
   // ACLE 6.4.10 Hardware Integer Divide
Index: cfe/trunk/test/Preprocessor/arm-acle-6.4.c
===
--- cfe/trunk/test/Preprocessor/arm-acle-6.4.c
+++ cfe/trunk/test/Preprocessor/arm-acle-6.4.c
@@ -174,10 +174,14 @@
 // CHECK-V7M: __ARM_FEATURE_SAT 1
 // CHECK-V7M: __ARM_FEATURE_UNALIGNED 1
 
-// RUN: %clang -target arm-none-linux-eabi -march=armv7e-m -x c -E -dM %s -o - 
| FileCheck %s -check-prefix CHECK-V7EM
+// RUN: %clang -target arm-none-linux-eabi -march=armv7e-m -x c -E -dM %s -o - 
| FileCheck %s -check-prefix CHECK-M-DSP
+// RUN: %clang --target=arm-arm-none-eabi -mcpu=cortex-m4 -x c -E -dM %s -o - 
| FileCheck %s -check-prefix CHECK-M-DSP
+// RUN: %clang --target=arm-arm-none-eabi -mcpu=cortex-m7 -x c -E -dM %s -o - 
| FileCheck %s -check-prefix CHECK-M-DSP
+// RUN: %clang --target=arm-arm-none-eabi -mcpu=cortex-m33 -x c -E -dM %s -o - 
| FileCheck %s -check-prefix CHECK-M-DSP
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8m.main+dsp -x c -E -dM 
%s -o - | FileCheck %s -check-prefix CHECK-M-DSP
 
-// CHECK-V7EM: __ARM_FEATURE_DSP 1
-// CHECK-V7EM: __ARM_FEATURE_SIMD32 1
+// CHECK-M-DSP: __ARM_FEATURE_DSP 1
+// CHECK-M-DSP: __ARM_FEATURE_SIMD32 1
 
 // RUN: %clang -target arm-none-linux-eabi -march=armv8-a -x c -E -dM %s -o - 
| FileCheck %s -check-prefix CHECK-V8A
 


Index: cfe/trunk/lib/Basic/Targets/ARM.cpp
===
--- cfe/trunk/lib/Basic/Targets/ARM.cpp
+++ cfe/trunk/lib/Basic/Targets/ARM.cpp
@@ -661,7 +661,7 @@
   }
 
   // ACLE 6.4.9 32-bit SIMD instructions
-  if (ArchVersion >= 6 && (CPUProfile != "M" || CPUAttr == "7EM"))
+  if (ArchVersion >= 6 || (CPUProfile == "M" && DSP))
 Builder.defineMacro("__ARM_FEATURE_SIMD32", "1");
 
   // ACLE 6.4.10 Hardware Integer Divide
Index: cfe/trunk/test/Preprocessor/arm-acle-6.4.c
===
--- cfe/trunk/test/Preprocessor/arm-acle-6.4.c
+++ cfe/trunk/test/Preprocessor/arm-acle-6.4.c
@@ -174,10 +174,14 @@
 // CHECK-V7M: __ARM_FEATURE_SAT 1
 // CHECK-V7M: __ARM_FEATURE_UNALIGNED 1
 
-// RUN: %clang -target arm-none-linux-eabi -march=armv7e-m -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-V7EM
+// RUN: %clang -target arm-none-linux-eabi -march=armv7e-m -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-M-DSP
+// RUN: %clang --target=arm-arm-none-eabi -mcpu=cortex-m4 -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-M-DSP
+// RUN: %clang --target=arm-arm-none-eabi -mcpu=cortex-m7 -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-M-DSP
+// RUN: %clang --target=arm-arm-none-eabi -mcpu=cortex-m33 -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-M-DSP
+// RUN: %clang --target=arm-arm-none-eabi -march=armv8m.main+dsp -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-M-DSP
 
-// CHECK-V7EM: __ARM_FEATURE_DSP 1
-// CHECK-V7EM: __ARM_FEATURE_SIMD32 1
+// CHECK-M-DSP: __ARM_FEATURE_DSP 1
+// CHECK-M-DSP: __ARM_FEATURE_SIMD32 1
 
 // RUN: %clang -target arm-none-linux-eabi -march=armv8-a -x c -E -dM %s -o - | FileCheck %s -check-prefix CHECK-V8A
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51407: [Tooling] Do not restore working dir in ClangTool

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: lib/Tooling/Tooling.cpp:419
-  } else {
-llvm::report_fatal_error("Cannot detect current path: " +
- Twine(CWD.getError().message()));

ilya-biryukov wrote:
> ioeric wrote:
> > Should we still check if `CWD` is correctly set?
> Not sure we know which CWD is correct at this point. Do you mean to check 
> that `getCurrentWorkingDirectory ` does not return an error?
Yes. 

Actually, after a closer look, I noticed `getAbsolutePath` below is just using 
the current working directory in real file system, which seems wrong. This is 
pre-existing but would be nice to fix since we are here :)


Repository:
  rC Clang

https://reviews.llvm.org/D51407



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

[PATCH] D51331: [OPENMP] Create non-const ident_t structs.

[Andrey Churbanov via Phabricator via cfe-commits]

AndreyChurbanov added a comment.

In https://reviews.llvm.org/D51331#1216509, @ABataev wrote:

> When is ITT Notify used? Does it have some preconditions like debug info, 
> some optimizations level etc.?


The libittnotify library is used by Intel tools (VTune Amplifier, Inspector, 
Spatial Advisor, etc.), may be used by others but I am not aware of any.  Intel 
Tools development team still have no plans in near future to move to standard 
OMPT interface, mostly because the standard OpenMP Tools interface still lacks 
some important for customers features (like barrier imbalance reporting).

It does not have any preconditions, although the presence of debug info usually 
helps to get better info in the tool.  So, users are free to use tools without 
debug info and with any optimization level.


https://reviews.llvm.org/D51331



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

[PATCH] D51411: [OpenCL] Improve diagnostic of argument in address space conversion builtins

[Alistair Davies via Phabricator via cfe-commits]

AlistairD created this revision.
AlistairD added a reviewer: yaxunl.

This change adds a warning if a parameter is passed with named address space to 
parameter that is in generic address space.

For example:

  int i;
  to_private(&i); //generate warning as conversion from private to private is 
redundant.


https://reviews.llvm.org/D51411

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaChecking.cpp
  test/Misc/warning-flags.c
  test/SemaOpenCL/to_addr_builtin.cl


Index: test/SemaOpenCL/to_addr_builtin.cl
===
--- test/SemaOpenCL/to_addr_builtin.cl
+++ test/SemaOpenCL/to_addr_builtin.cl
@@ -43,13 +43,14 @@
   // expected-warning@-2{{incompatible integer to pointer conversion assigning 
to '__local int *' from 'int'}}
 #else
   // expected-error@-4{{assigning '__global int *' to '__local int *' changes 
address space of pointer}}
+  // expected-warning@-5{{passing non-generic address space pointer to 
to_global generates dynamic conversion where it is not needed}}
 #endif
 
   global char *glob_c = to_global(loc);
 #if __OPENCL_C_VERSION__ < CL_VERSION_2_0
   // expected-warning@-2{{incompatible integer to pointer conversion 
initializing '__global char *' with an expression of type 'int'}}
 #else
   // expected-warning@-4{{incompatible pointer types initializing '__global 
char *' with an expression of type '__global int *'}}
+  // expected-warning@-5{{passing non-generic address space pointer to 
to_global generates dynamic conversion where it is not needed}}
 #endif
-
 }
Index: test/Misc/warning-flags.c
===
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVER grow.  It should gradually shrink to 0.
 
-CHECK: Warnings without flags (76):
+CHECK: Warnings without flags (77):
 CHECK-NEXT:   ext_excess_initializers
 CHECK-NEXT:   ext_excess_initializers_in_char_array_initializer
 CHECK-NEXT:   ext_expected_semi_decl_list
@@ -77,6 +77,7 @@
 CHECK-NEXT:   warn_objc_property_copy_missing_on_block
 CHECK-NEXT:   warn_objc_protocol_qualifier_missing_id
 CHECK-NEXT:   warn_on_superclass_use
+CHECK-NEXT:   warn_opencl_generic_address_space_arg
 CHECK-NEXT:   warn_pp_convert_to_positive
 CHECK-NEXT:   warn_pp_expr_overflow
 CHECK-NEXT:   warn_pp_line_decimal
Index: lib/Sema/SemaChecking.cpp
===
--- lib/Sema/SemaChecking.cpp
+++ lib/Sema/SemaChecking.cpp
@@ -839,6 +839,13 @@
 return true;
   }
 
+  if (RT->getPointeeType().getAddressSpace() != LangAS::opencl_generic) {
+S.Diag(Call->getArg(0)->getLocStart(),
+   diag::warn_opencl_generic_address_space_arg)
+<< Call->getDirectCallee()->getNameInfo().getAsString()
+<< Call->getArg(0)->getSourceRange();
+  }
+
   RT = RT->getPointeeType();
   auto Qual = RT.getQualifiers();
   switch (BuiltinID) {
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -8579,6 +8579,9 @@
   "invalid prototype, variadic arguments are not allowed in OpenCL">;
 def err_opencl_requires_extension : Error<
   "use of %select{type|declaration}0 %1 requires %2 extension to be enabled">;
+def warn_opencl_generic_address_space_arg : Warning<
+  "passing non-generic address space pointer to %0"
+  " generates dynamic conversion where it is not needed">;
 
 // OpenCL v2.0 s6.13.6 -- Builtin Pipe Functions
 def err_opencl_builtin_pipe_first_arg : Error<


Index: test/SemaOpenCL/to_addr_builtin.cl
===
--- test/SemaOpenCL/to_addr_builtin.cl
+++ test/SemaOpenCL/to_addr_builtin.cl
@@ -43,13 +43,14 @@
   // expected-warning@-2{{incompatible integer to pointer conversion assigning to '__local int *' from 'int'}}
 #else
   // expected-error@-4{{assigning '__global int *' to '__local int *' changes address space of pointer}}
+  // expected-warning@-5{{passing non-generic address space pointer to to_global generates dynamic conversion where it is not needed}}
 #endif
 
   global char *glob_c = to_global(loc);
 #if __OPENCL_C_VERSION__ < CL_VERSION_2_0
   // expected-warning@-2{{incompatible integer to pointer conversion initializing '__global char *' with an expression of type 'int'}}
 #else
   // expected-warning@-4{{incompatible pointer types initializing '__global char *' with an expression of type '__global int *'}}
+  // expected-warning@-5{{passing non-generic address space pointer to to_global generates dynamic conversion where it is not needed}}
 #endif
-
 }
Index: test/Misc/warning-flags.c
===
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
 
 The list of warnings below should NEVE

[clang-tools-extra] r340915 - Introduce the abseil-str-cat-append check.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Aug 29 04:17:31 2018
New Revision: 340915

URL: http://llvm.org/viewvc/llvm-project?rev=340915&view=rev
Log:
Introduce the abseil-str-cat-append check.

This flags uses of absl::StrCat when absl::StrAppend should be used instead. 
Patch by Hugo Gonzalez and Benjamin Kramer.

Added:
clang-tools-extra/trunk/clang-tidy/abseil/StrCatAppendCheck.cpp
clang-tools-extra/trunk/clang-tidy/abseil/StrCatAppendCheck.h
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-str-cat-append.rst
clang-tools-extra/trunk/test/clang-tidy/abseil-str-cat-append.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp?rev=340915&r1=340914&r2=340915&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Wed Aug 29 
04:17:31 2018
@@ -14,6 +14,7 @@
 #include "FasterStrsplitDelimiterCheck.h"
 #include "NoNamespaceCheck.h"
 #include "StringFindStartswithCheck.h"
+#include "StrCatAppendCheck.h"
 
 namespace clang {
 namespace tidy {
@@ -29,6 +30,8 @@ public:
 CheckFactories.registerCheck("abseil-no-namespace");
 CheckFactories.registerCheck(
 "abseil-string-find-startswith");
+CheckFactories.registerCheck(
+"abseil-str-cat-append");
   }
 };
 

Modified: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt?rev=340915&r1=340914&r2=340915&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt Wed Aug 29 
04:17:31 2018
@@ -6,6 +6,7 @@ add_clang_library(clangTidyAbseilModule
   FasterStrsplitDelimiterCheck.cpp
   NoNamespaceCheck.cpp
   StringFindStartswithCheck.cpp
+  StrCatAppendCheck.cpp
 
   LINK_LIBS
   clangAST

Added: clang-tools-extra/trunk/clang-tidy/abseil/StrCatAppendCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/StrCatAppendCheck.cpp?rev=340915&view=auto
==
--- clang-tools-extra/trunk/clang-tidy/abseil/StrCatAppendCheck.cpp (added)
+++ clang-tools-extra/trunk/clang-tidy/abseil/StrCatAppendCheck.cpp Wed Aug 29 
04:17:31 2018
@@ -0,0 +1,102 @@
+//===--- StrCatAppendCheck.cpp - 
clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "StrCatAppendCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+namespace {
+// Skips any combination of temporary materialization, temporary binding and
+// implicit casting.
+AST_MATCHER_P(Stmt, IgnoringTemporaries, ast_matchers::internal::Matcher,
+  InnerMatcher) {
+  const Stmt *E = &Node;
+  while (true) {
+if (const auto *MTE = dyn_cast(E))
+  E = MTE->getTemporary();
+if (const auto *BTE = dyn_cast(E))
+  E = BTE->getSubExpr();
+if (const auto *ICE = dyn_cast(E))
+  E = ICE->getSubExpr();
+else
+  break;
+  }
+
+  return InnerMatcher.matches(*E, Finder, Builder);
+}
+
+}  // namespace
+
+// TODO: str += StrCat(...)
+//   str.append(StrCat(...))
+
+void StrCatAppendCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+   return;
+  const auto StrCat = functionDecl(hasName("::absl::StrCat"));
+  // The arguments of absl::StrCat are implicitly converted to AlphaNum. This 
+  // matches to the arguments because of that behavior. 
+  const auto AlphaNum = IgnoringTemporaries(cxxConstructExpr(
+  argumentCountIs(1), hasType(cxxRecordDecl(hasName("::absl::AlphaNum"))),
+  hasArgument(0, ignoringImpCasts(declRefExpr(to(equalsBoundNode("LHS")),
+  expr().bind("Arg0"));
+
+  const auto HasAnotherReferenceToLhs =
+  callExpr(hasAnyArgument(expr(hasDescendant(declRefExpr(
+  to(equalsBoundNode("LHS")), unless(equalsBoundNode("Arg0")));
+
+  // Now look for calls to operator= with an object on the LHS and a call to
+  // StrCat on the RHS. The first

[PATCH] D51061: [clang-tidy] abseil-str-cat-append

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In https://reviews.llvm.org/D51061#1215917, @hugoeg wrote:

> @aaron.ballman when you get the chance could you take another look at this 
> and commit if it is ready? My internship ends rather soon and this is a tad 
> time sensitive and I don't have commit access. Thank you for your time!


I've commit in r340915. Thank you for the patch!


https://reviews.llvm.org/D51061



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

[PATCH] D51300: [analyzer][UninitializedObjectChecker] No longer using nonloc::LazyCompoundVal

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

Szelethus added inline comments.



Comment at: 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:448-449
 
   Loc ThisLoc = Context.getSValBuilder().getCXXThis(CtorDecl->getParent(),
 Context.getStackFrame());
 

NoQ wrote:
> This totally needs `assert(CtorDecl == Context.getStackFrame()->getDecl())`. 
> Otherwise we're in big trouble because we'll be looking into a this-region 
> that doesn't exist on this stack frame.
> 
> On second thought, though, i guess we should put this assertion into the 
> constructor of `CXXThisRegion`. I'll do this.
> 
> Also there's an overload of `getCXXThis` that accepts the method itself, no 
> need to get parent.
U that wouldn't be very nice, because...



Comment at: 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:456-483
 static bool willObjectBeAnalyzedLater(const CXXConstructorDecl *Ctor,
   CheckerContext &Context) {
 
-  Optional CurrentObject = getObjectVal(Ctor, 
Context);
-  if (!CurrentObject)
+  const TypedValueRegion *CurrRegion = getConstructedRegion(Ctor, Context);
+  if (!CurrRegion)
 return false;
 

...`willBeAnalyzerLater()` relies on this, and it uses all sorts of constructor 
decls to check whether `Context.getLocationContext()->getDecl()` would be a 
subregion of another object. Are you sure that this is incorrect?


Repository:
  rC Clang

https://reviews.llvm.org/D51300



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

[clang-tools-extra] r340918 - Introduce the abseil-redundant-strcat-calls check.

[Aaron Ballman via cfe-commits]

Author: aaronballman
Date: Wed Aug 29 04:29:07 2018
New Revision: 340918

URL: http://llvm.org/viewvc/llvm-project?rev=340918&view=rev
Log:
Introduce the abseil-redundant-strcat-calls check.

This flags redundant calls to absl::StrCat where the result is being passed to 
another call to absl::StrCat or absl::StrAppend. Patch by Hugo Gonzalez and 
Samuel Benzaquen.

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

clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
clang-tools-extra/trunk/test/clang-tidy/abseil-redundant-strcat-calls.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst

Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp?rev=340918&r1=340917&r2=340918&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Wed Aug 29 
04:29:07 2018
@@ -13,6 +13,7 @@
 #include "DurationDivisionCheck.h"
 #include "FasterStrsplitDelimiterCheck.h"
 #include "NoNamespaceCheck.h"
+#include "RedundantStrcatCallsCheck.h"
 #include "StringFindStartswithCheck.h"
 #include "StrCatAppendCheck.h"
 
@@ -28,6 +29,8 @@ public:
 CheckFactories.registerCheck(
 "abseil-faster-strsplit-delimiter");
 CheckFactories.registerCheck("abseil-no-namespace");
+CheckFactories.registerCheck(
+"abseil-redundant-strcat-calls");
 CheckFactories.registerCheck(
 "abseil-string-find-startswith");
 CheckFactories.registerCheck(

Modified: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt?rev=340918&r1=340917&r2=340918&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt Wed Aug 29 
04:29:07 2018
@@ -5,6 +5,7 @@ add_clang_library(clangTidyAbseilModule
   DurationDivisionCheck.cpp
   FasterStrsplitDelimiterCheck.cpp
   NoNamespaceCheck.cpp
+  RedundantStrcatCallsCheck.cpp
   StringFindStartswithCheck.cpp
   StrCatAppendCheck.cpp
 

Added: clang-tools-extra/trunk/clang-tidy/abseil/RedundantStrcatCallsCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/RedundantStrcatCallsCheck.cpp?rev=340918&view=auto
==
--- clang-tools-extra/trunk/clang-tidy/abseil/RedundantStrcatCallsCheck.cpp 
(added)
+++ clang-tools-extra/trunk/clang-tidy/abseil/RedundantStrcatCallsCheck.cpp Wed 
Aug 29 04:29:07 2018
@@ -0,0 +1,140 @@
+//===--- RedundantStrcatCallsCheck.cpp - 
clang-tidy===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "RedundantStrcatCallsCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+// TODO: Features to add to the check:
+//  - Make it work if num_args > 26.
+//  - Remove empty literal string arguments.
+//  - Collapse consecutive literal string arguments into one (remove the ,).
+//  - Replace StrCat(a + b)  ->  StrCat(a, b)  if a or b are strings.
+//  - Make it work in macros if the outer and inner StrCats are both in the
+//argument.
+
+void RedundantStrcatCallsCheck::registerMatchers(MatchFinder* Finder) {
+  if (!getLangOpts().CPlusPlus) 
+   return;
+  const auto CallToStrcat =
+  callExpr(callee(functionDecl(hasName("::absl::StrCat";
+  const auto CallToStrappend =
+  callExpr(callee(functionDecl(hasName("::absl::StrAppend";
+  // Do not match StrCat() calls that are descendants of other StrCat calls.
+  // Those are handled on the ancestor call.
+  const auto CallToEither = callExpr(
+  callee(functionDecl(hasAnyName("::absl::StrCat", "::absl::StrAppend";
+  Finder->addMatcher(
+  callExpr(CallToStrcat, unless(hasAncestor(CallToEither))).bind("StrCat"),
+  this);
+  Finder->addMatcher(CallToStrappend.bind("StrAppend"), this);
+}
+
+namespace {
+
+struct StrCatCheckResult {
+  int NumCalls = 0;
+  std::vector Hints;
+};
+
+void RemoveCallLeaveArgs

[PATCH] D51132: [clang-tidy] abseil-redundant-strcat-calls-check

[Aaron Ballman via Phabricator via cfe-commits]

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

I've commit in r340918. Thank you for the patch!


https://reviews.llvm.org/D51132



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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Haojian Wu via Phabricator via cfe-commits]

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

LGTM with a nit, thanks for your work.




Comment at: clang-tidy/abseil/NoInternalDependenciesCheck.h:20
+/// Finds instances where the user depends on internal details and warns them
+/// against doing so.Should not run on internal Abseil files or Abseil source
+/// code.

This comment is out of date. The check can handle it, I think.


https://reviews.llvm.org/D50542



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

[PATCH] D51311: (WIP) Type hierarchy

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a subscriber: sammccall.
ilya-biryukov added inline comments.



Comment at: clangd/XRefs.cpp:669
+  const CXXMethodDecl *Candidate) {
+  // FIXME: How do I determine if Method overrides Candidate?
+

kadircet wrote:
> ilya-biryukov wrote:
> > simark wrote:
> > > kadircet wrote:
> > > > https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp
> > > > 
> > > > you can check this sample, which simply traverses all base classes and 
> > > > gets methods with the same name, then checks whether one is overload of 
> > > > the other. If it they are not overloads then one in the base classes 
> > > > gets overriden by the other.
> > > > 
> > > > 
> > > > Another approach could be to search for one method in others 
> > > > overriden_methods.
> > > > 
> > > > But they are both inefficient, I would suggest calling these functions 
> > > > only when one of them is defined in the base class of other then you 
> > > > can just check for name equality and not being an overload.
> > > > https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp
> > > 
> > > Did you mean to link to a particular line?
> > > 
> > > > you can check this sample, which simply traverses all base classes and 
> > > > gets methods with the same name, then checks whether one is overload of 
> > > > the other. If it they are not overloads then one in the base classes 
> > > > gets overriden by the other.
> > > 
> > > > Another approach could be to search for one method in others 
> > > > overriden_methods.
> > > 
> > > I have tried using `overriden_methods`, but it only contains methods 
> > > marked virtual.  For this feature, I would like to consider non-virtual 
> > > methods too.
> > > 
> > > > But they are both inefficient, I would suggest calling these functions 
> > > > only when one of them is defined in the base class of other then you 
> > > > can just check for name equality and not being an overload.
> > > 
> > > I am not sure I understand, but maybe it will be clearer when I will see 
> > > the sample.
> > See the code that actually computes a list for `override_methods()`: 
> > Sema::AddOverriddenMethods.
> > Did you mean to link to a particular line?
> 
> 
> Yeah sorry, I was trying to link the function ilya mentioned.
> https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp#L7615
> 
> Since you also mentioned checking non-virtual method as well you might wanna 
> look at 
> https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp#L7555 
> as well.
> 
> Also I got the chance to look the rest of the patch, as I mentioned above you 
> are already checking two methods iff one lies in the others base classes. So 
> you can simply check for name equality and not being an overload case.
Also wanted to bring up what @sammccall already mentioned on clangd-dev: we 
probably shouldn't show methods that hide base methods rather than override the 
virtual base methods (at least, by default).

Those might be useful, but unless we can have a clear UI indicator of whether a 
method is overriding a virtual base method or is hiding some other method, it 
would to add more confusion than benefit IMO.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51311



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

[PATCH] D51380: Fix incorrect usage of std::error_category

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM


Repository:
  rC Clang

https://reviews.llvm.org/D51380



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

[PATCH] D51380: Fix incorrect usage of std::error_category

[Alexandre Ganea via Phabricator via cfe-commits]

aganea added a comment.

Thank you!


Repository:
  rC Clang

https://reviews.llvm.org/D51380



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

[PATCH] D51416: [RTTI] Align rtti type string to prevent over-alignment

[Dave Green via Phabricator via cfe-commits]

dmgreen created this revision.
dmgreen added reviewers: efriedma, echristo, hfinkel, davide.

Previously the alignment on the newly created rtti string was not set,
meaning that DataLayout::getPreferredAlignment was free to overalign
it to 16 bytes. This causes unnecessary code bloat.


https://reviews.llvm.org/D51416

Files:
  lib/CodeGen/ItaniumCXXABI.cpp
  test/CodeGenCXX/vtable-align.cpp
  test/CodeGenCXX/vtable-linkage.cpp


Index: test/CodeGenCXX/vtable-linkage.cpp
===
--- test/CodeGenCXX/vtable-linkage.cpp
+++ test/CodeGenCXX/vtable-linkage.cpp
@@ -99,7 +99,7 @@
 // C has no key function, so its vtable should have weak_odr linkage
 // and hidden visibility (rdar://problem/7523229).
 // CHECK-DAG: @_ZTV1C = linkonce_odr unnamed_addr constant {{.*}}, comdat,
-// CHECK-DAG: @_ZTS1C = linkonce_odr constant {{.*}}, comdat{{$}}
+// CHECK-DAG: @_ZTS1C = linkonce_odr constant {{.*}}, comdat, align 1{{$}}
 // CHECK-DAG: @_ZTI1C = linkonce_odr constant {{.*}}, comdat{{$}}
 // CHECK-DAG: @_ZTT1C = linkonce_odr unnamed_addr constant {{.*}}, comdat{{$}}
 
@@ -120,26 +120,26 @@
 // defined in this translation unit, so its vtable should have
 // weak_odr linkage.
 // CHECK-DAG: @_ZTV1EIsE = weak_odr unnamed_addr constant {{.*}}, comdat,
-// CHECK-DAG: @_ZTS1EIsE = weak_odr constant {{.*}}, comdat{{$}}
+// CHECK-DAG: @_ZTS1EIsE = weak_odr constant {{.*}}, comdat, align 1{{$}}
 // CHECK-DAG: @_ZTI1EIsE = weak_odr constant {{.*}}, comdat{{$}}
 
 // F is an explicit template instantiation without a key
 // function, so its vtable should have weak_odr linkage
 // CHECK-DAG: @_ZTV1FIsE = weak_odr unnamed_addr constant {{.*}}, comdat,
-// CHECK-DAG: @_ZTS1FIsE = weak_odr constant {{.*}}, comdat{{$}}
+// CHECK-DAG: @_ZTS1FIsE = weak_odr constant {{.*}}, comdat, align 1{{$}}
 // CHECK-DAG: @_ZTI1FIsE = weak_odr constant {{.*}}, comdat{{$}}
 
 // E is an implicit template instantiation with a key function
 // defined in this translation unit, so its vtable should have
 // linkonce_odr linkage.
 // CHECK-DAG: @_ZTV1EIlE = linkonce_odr unnamed_addr constant {{.*}}, comdat,
-// CHECK-DAG: @_ZTS1EIlE = linkonce_odr constant {{.*}}, comdat{{$}}
+// CHECK-DAG: @_ZTS1EIlE = linkonce_odr constant {{.*}}, comdat, align 1{{$}}
 // CHECK-DAG: @_ZTI1EIlE = linkonce_odr constant {{.*}}, comdat{{$}}
 
 // F is an implicit template instantiation with no key function,
 // so its vtable should have linkonce_odr linkage.
 // CHECK-DAG: @_ZTV1FIlE = linkonce_odr unnamed_addr constant {{.*}}, comdat,
-// CHECK-DAG: @_ZTS1FIlE = linkonce_odr constant {{.*}}, comdat{{$}}
+// CHECK-DAG: @_ZTS1FIlE = linkonce_odr constant {{.*}}, comdat, align 1{{$}}
 // CHECK-DAG: @_ZTI1FIlE = linkonce_odr constant {{.*}}, comdat{{$}}
 
 // F is an explicit template instantiation declaration without a
@@ -171,7 +171,7 @@
 // F is an explicit specialization without a key function, so
 // its vtable should have linkonce_odr linkage.
 // CHECK-DAG: @_ZTV1FIcE = linkonce_odr unnamed_addr constant {{.*}}, comdat,
-// CHECK-DAG: @_ZTS1FIcE = linkonce_odr constant {{.*}}, comdat{{$}}
+// CHECK-DAG: @_ZTS1FIcE = linkonce_odr constant {{.*}}, comdat, align 1{{$}}
 // CHECK-DAG: @_ZTI1FIcE = linkonce_odr constant {{.*}}, comdat{{$}}
 
 // CHECK-DAG: @_ZTV1GIiE = linkonce_odr unnamed_addr constant {{.*}}, comdat,
Index: test/CodeGenCXX/vtable-align.cpp
===
--- test/CodeGenCXX/vtable-align.cpp
+++ test/CodeGenCXX/vtable-align.cpp
@@ -10,5 +10,6 @@
 void A::f() {}
 
 // CHECK-32: @_ZTV1A = unnamed_addr constant { [5 x i8*] } { [5 x i8*] [i8* 
null, i8* bitcast ({ i8*, i8* }* @_ZTI1A to i8*), i8* bitcast (void 
(%struct.A*)* @_ZN1A1fEv to i8*), i8* bitcast (void (%struct.A*)* @_ZN1A1gEv to 
i8*), i8* bitcast (void (%struct.A*)* @_ZN1A1hEv to i8*)] }, align 4
-
+// CHECK-32: @_ZTS1A = constant [3 x i8] c"1A\00", align 1
 // CHECK-64: @_ZTV1A = unnamed_addr constant { [5 x i8*] } { [5 x i8*] [i8* 
null, i8* bitcast ({ i8*, i8* }* @_ZTI1A to i8*), i8* bitcast (void 
(%struct.A*)* @_ZN1A1fEv to i8*), i8* bitcast (void (%struct.A*)* @_ZN1A1gEv to 
i8*), i8* bitcast (void (%struct.A*)* @_ZN1A1hEv to i8*)] }, align 8
+// CHECK-64: @_ZTS1A = constant [3 x i8] c"1A\00", align 1
Index: lib/CodeGen/ItaniumCXXABI.cpp
===
--- lib/CodeGen/ItaniumCXXABI.cpp
+++ lib/CodeGen/ItaniumCXXABI.cpp
@@ -2730,6 +2730,7 @@
 CGM.CreateOrReplaceCXXRuntimeVariable(Name, Init->getType(), Linkage);
 
   GV->setInitializer(Init);
+  GV->setAlignment(1); // prevent over-aligning the string
 
   return GV;
 }


Index: test/CodeGenCXX/vtable-linkage.cpp
===
--- test/CodeGenCXX/vtable-linkage.cpp
+++ test/CodeGenCXX/vtable-linkage.cpp
@@ -99,7 +99,7 @@
 // C has no key function, so its vtable should have weak_odr linkage
 // and hidden visibility (rdar://pr

[PATCH] D49916: [CodeGen] Add to emitted DebugLoc information about coverage when it's required

[calixte via Phabricator via cfe-commits]

calixte updated this revision to Diff 163062.
calixte added a comment.

- Use ImplicitCode instead of Covered
- Set Artificial location as ImplicitCode to avoid coverage on landing pads


Repository:
  rC Clang

https://reviews.llvm.org/D49916

Files:
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  lib/CodeGen/CodeGenFunction.cpp
  lib/CodeGen/CodeGenFunction.h

Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -786,7 +786,7 @@
   // If we should perform a cleanup, force them now.  Note that
   // this ends the cleanup scope before rescoping any labels.
   if (PerformCleanup) {
-ApplyDebugLocation DL(CGF, Range.getEnd());
+ApplyDebugLocation DL(CGF, Range.getEnd(), true /* ImplicitCode */);
 ForceCleanup();
   }
 }
Index: lib/CodeGen/CodeGenFunction.cpp
===
--- lib/CodeGen/CodeGenFunction.cpp
+++ lib/CodeGen/CodeGenFunction.cpp
@@ -1179,7 +1179,7 @@
   }
   // Emit a location at the end of the prologue.
   if (CGDebugInfo *DI = getDebugInfo())
-DI->EmitLocation(Builder, StartLoc);
+DI->EmitLocation(Builder, StartLoc, true /* ImplicitCode */);
 
   // TODO: Do we need to handle this in two places like we do with
   // target-features/target-cpu?
Index: lib/CodeGen/CGDebugInfo.h
===
--- lib/CodeGen/CGDebugInfo.h
+++ lib/CodeGen/CGDebugInfo.h
@@ -377,7 +377,9 @@
   /// Emit metadata to indicate a change in line/column information in
   /// the source file. If the location is invalid, the previous
   /// location will be reused.
-  void EmitLocation(CGBuilderTy &Builder, SourceLocation Loc);
+  /// \param ImplicitCode  True if the Loc must have coverage information
+  void EmitLocation(CGBuilderTy &Builder, SourceLocation Loc,
+bool ImplicitCode = false);
 
   /// Emit a call to llvm.dbg.function.start to indicate
   /// start of a new function.
@@ -657,16 +659,19 @@
 /// location or preferred location of the specified Expr.
 class ApplyDebugLocation {
 private:
-  void init(SourceLocation TemporaryLocation, bool DefaultToEmpty = false);
+  void init(SourceLocation TemporaryLocation, bool DefaultToEmpty = false,
+bool ImplicitCode = false);
   ApplyDebugLocation(CodeGenFunction &CGF, bool DefaultToEmpty,
- SourceLocation TemporaryLocation);
+ SourceLocation TemporaryLocation,
+ bool ImplicitCode = false);
 
   llvm::DebugLoc OriginalLocation;
   CodeGenFunction *CGF;
 
 public:
   /// Set the location to the (valid) TemporaryLocation.
-  ApplyDebugLocation(CodeGenFunction &CGF, SourceLocation TemporaryLocation);
+  ApplyDebugLocation(CodeGenFunction &CGF, SourceLocation TemporaryLocation,
+ bool ImplicitCode = false);
   ApplyDebugLocation(CodeGenFunction &CGF, const Expr *E);
   ApplyDebugLocation(CodeGenFunction &CGF, llvm::DebugLoc Loc);
   ApplyDebugLocation(ApplyDebugLocation &&Other) : CGF(Other.CGF) {
@@ -687,15 +692,17 @@
   /// SourceLocation to CGDebugInfo::setLocation() will result in the
   /// last valid location being reused.
   static ApplyDebugLocation CreateArtificial(CodeGenFunction &CGF) {
-return ApplyDebugLocation(CGF, false, SourceLocation());
+return ApplyDebugLocation(CGF, false, SourceLocation(),
+  true /* ImplicitCode */);
   }
   /// Apply TemporaryLocation if it is valid. Otherwise switch
   /// to an artificial debug location that has a valid scope, but no
   /// line information.
   static ApplyDebugLocation
   CreateDefaultArtificial(CodeGenFunction &CGF,
   SourceLocation TemporaryLocation) {
-return ApplyDebugLocation(CGF, false, TemporaryLocation);
+return ApplyDebugLocation(CGF, false, TemporaryLocation,
+  true /* ImplicitCode */);
   }
 
   /// Set the IRBuilder to not attach debug locations.  Note that
Index: lib/CodeGen/CGDebugInfo.cpp
===
--- lib/CodeGen/CGDebugInfo.cpp
+++ lib/CodeGen/CGDebugInfo.cpp
@@ -76,20 +76,22 @@
 }
 
 ApplyDebugLocation::ApplyDebugLocation(CodeGenFunction &CGF,
-   SourceLocation TemporaryLocation)
+   SourceLocation TemporaryLocation,
+   bool ImplicitCode)
 : CGF(&CGF) {
-  init(TemporaryLocation);
+  init(TemporaryLocation, false /* DefaultToEmpty */, ImplicitCode);
 }
 
 ApplyDebugLocation::ApplyDebugLocation(CodeGenFunction &CGF,
bool DefaultToEmpty,
-   SourceLocation TemporaryLocation)
+   SourceLocation TemporaryLocation,
+   

[PATCH] D51221: [clangd] Some nitpicking around the new split (preamble/main) dynamic index

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM




Comment at: clangd/ClangdServer.cpp:122
+  //  - symbols declared both in the main file and the preamble
+  // (Note that symbols *only* in the main file are not indexed).
   FileIndex MainFileIdx;

sammccall wrote:
> ilya-biryukov wrote:
> > I thought it was not true, but now I notice we actually don't index those 
> > symbols.
> > I think we should index them for workspaceSymbols, but not for completion. 
> > Any pointers to the code that filters them out?
> https://reviews.llvm.org/source/clang-tools-extra/browse/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp;340828$272
> 
> isExpansionInMainFile()
> 
> (having this buried so deep hurts readability and probably performance).
> 
> But I think this would be the behavior we want for code complete, to keep the 
> indexes tiny and incremental updates fast?
> WorkspaceSymbols is indeed a problem here :-( Tradeoffs...
I would assume the number of symbols in the main files is not very large, 
compared to the ones from the preamble.
We could try indexing those and set a flag they're not for code completion. 
Should give us a better workspaceSymbol without hurting anything else.



Comment at: clangd/ClangdServer.cpp:152
   WorkScheduler(Opts.AsyncThreadsCount, Opts.StorePreamblesInMemory,
-DynamicIdx ? *DynamicIdx : noopParsingCallbacks(),
+DynamicIdx ? DynamicIdx->makeUpdateCallbacks() : nullptr,
 Opts.UpdateDebounce, Opts.RetentionPolicy) {

sammccall wrote:
> ilya-biryukov wrote:
> > Any reason to prefer `nullptr` over the no-op callbacks?
> > Might be a personal preference, my reasoning is: having an extra check for 
> > null before on each of the call sites both adds unnecessary boilerplate and 
> > adds an extra branch before the virtual call (which is, technically, 
> > another form of a branch).
> > 
> > Not opposed to doing it either way, though.
> Basically I prefer the old behavior here (when it was std::function).
> Having to keep the callbacks object alive is a pain, and the shared instance 
> of the no-op implementation for this purpose seems a little silly.
> 
> We don't have the std::function copyability stuff which is a mixed bag but 
> nice for cases like this. But passing the reference from TUScheduler to its 
> ASTWorkers is internal enough that it doesn't bother me, WDYT?
> 
> > extra check for null before on each of the call sites 
> Note that the check is actually in the constructor, supporting `nullptr` is 
> just for brevity (particularly in tests).
> Having to keep the callbacks object alive is a pain, and the shared instance 
> of the no-op implementation for this purpose seems a little silly.
OTOH, this gives flexibility to the callers wrt to the lifetime, i.e. they 
don't **have** to create a separate object for the callbacks if they don't want 
to (one example of this pattern is ClangdLSPServer inheriting 
DiagnosticsConsumer and ProtocolCallbacks).

But happy to do it either way.




Comment at: clangd/TUScheduler.h:158
   const std::shared_ptr PCHOps;
-  ParsingCallbacks &Callbacks;
+  std::unique_ptr Callbacks;
   Semaphore Barrier;

NIT: maybe mention that this is never null in a comment?



Comment at: unittests/clangd/TUSchedulerTests.cpp:48
   TUScheduler S(getDefaultAsyncThreadsCount(),
-/*StorePreamblesInMemory=*/true, noopParsingCallbacks(),
+/*StorePreamblesInMemory=*/true, nullptr,
 /*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),

NIT: maybe add a name of the parameter here for better readability (nullptr can 
mean many things)?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51221



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

[PATCH] D51417: [analyzer][UninitializedObjectChecker] Updated comments

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

Szelethus created this revision.
Szelethus added reviewers: NoQ, george.karpenkov, xazax.hun, rnkovacs.
Herald added subscribers: cfe-commits, mikhail.ramalho, a.sidorin, szepet, 
whisperity.

Some of the comments are incorrect, imprecise, or simply nonexistent. Since I 
have a better grasp on how the analyzer works, it makes sense to update most of 
them in a single swoop.

I tried not to flood the code with comments too much, this amount feels just 
right to me.


Repository:
  rC Clang

https://reviews.llvm.org/D51417

Files:
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObject.h
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
  lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp

Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedPointee.cpp
@@ -1,4 +1,4 @@
-//===- UninitializedPointer.cpp --*- C++ -*-==//
+//===- UninitializedPointee.cpp --*- C++ -*-==//
 //
 // The LLVM Compiler Infrastructure
 //
@@ -90,9 +90,8 @@
 
 // Utility function declarations.
 
-/// Returns whether T can be (transitively) dereferenced to a void pointer type
-/// (void*, void**, ...). The type of the region behind a void pointer isn't
-/// known, and thus FD can not be analyzed.
+/// Returns whether \p T can be (transitively) dereferenced to a void pointer
+/// type (void*, void**, ...).
 static bool isVoidPointer(QualType T);
 
 using DereferenceInfo = std::pair;
@@ -107,9 +106,7 @@
 //   Methods for FindUninitializedFields.
 //===--===//
 
-// Note that pointers/references don't contain fields themselves, so in this
-// function we won't add anything to LocalChain.
-bool FindUninitializedFields::isPointerOrReferenceUninit(
+bool FindUninitializedFields::isDereferencableUninit(
 const FieldRegion *FR, FieldChainInfo LocalChain) {
 
   assert(isDereferencableType(FR->getDecl()->getType()) &&
@@ -224,12 +221,11 @@
   while (const MemRegion *Tmp = State->getSVal(R, DynT).getAsRegion()) {
 
 R = Tmp->getAs();
-
 if (!R)
   return None;
 
 // We found a cyclic pointer, like int *ptr = (int *)&ptr.
-// TODO: Report these fields too.
+// TODO: Should we report these fields too?
 if (!VisitedRegions.insert(R).second)
   return None;
 
Index: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
+++ lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp
@@ -94,7 +94,9 @@
 };
 
 /// Represents that the FieldNode that comes after this is declared in a base
-/// of the previous FieldNode.
+/// of the previous FieldNode. As such, this descendant doesn't wrap a
+/// FieldRegion, and is purely a tool to describe a relation between two other
+/// FieldRegion wrapping descendants.
 class BaseClass final : public FieldNode {
   const QualType BaseClassT;
 
@@ -291,7 +293,7 @@
 }
 
 if (isDereferencableType(T)) {
-  if (isPointerOrReferenceUninit(FR, LocalChain))
+  if (isDereferencableUninit(FR, LocalChain))
 ContainsUninitField = true;
   continue;
 }
@@ -309,7 +311,8 @@
 llvm_unreachable("All cases are handled!");
   }
 
-  // Checking bases.
+  // Checking bases. The checker will regard inherited data members as direct
+  // fields.
   const auto *CXXRD = dyn_cast(RD);
   if (!CXXRD)
 return ContainsUninitField;
@@ -356,6 +359,9 @@
 
 const FieldRegion *FieldChainInfo::getUninitRegion() const {
   assert(!Chain.isEmpty() && "Empty fieldchain!");
+
+  // ImmutableList::getHead() isn't a const method, hence the not too nice
+  // implementation.
   return (*Chain.begin()).getRegion();
 }
 
@@ -370,31 +376,11 @@
 /// Prints every element except the last to `Out`. Since ImmutableLists store
 /// elements in reverse order, and have no reverse iterators, we use a
 /// recursive function to print the fieldchain correctly. The last element in
-/// the chain is to be printed by `print`.
+/// the chain is to be printed by `FieldChainInfo::print`.
 static void printTail(llvm::raw_ostream &Out,
   const FieldChainInfo::FieldChainImpl *L);
 
-// TODO: This function constructs an incorrect string if a void pointer is a
-// part of the chain:
-//
-//   struct B { int x; }
-//
-//   struct A {
-// void *vptr;
-// A(void* vptr) : vptr(vptr) {}
-//   };
-//
-//   void f() {
-// B b;
-// A a(&b);
-//   }
-//
-// The note message will be "uninitialized field 'this->vptr->x'", even tho

[PATCH] D51279: [clangd] Implement findOccurrences interface in dynamic index.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 163064.
hokein marked 16 inline comments as done.
hokein added a comment.
Herald added a subscriber: mgrang.

Address review comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51279

Files:
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/MemIndex.cpp
  clangd/index/MemIndex.h
  clangd/index/Merge.cpp
  clangd/index/Merge.h
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/IndexTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp
  unittests/clangd/TestTU.cpp
  unittests/clangd/TestTU.h

Index: unittests/clangd/TestTU.h
===
--- unittests/clangd/TestTU.h
+++ unittests/clangd/TestTU.h
@@ -38,6 +38,16 @@
 return TU;
   }
 
+  static TestTU withAllCode(llvm::StringRef HeaderCode, llvm::StringRef Code,
+llvm::StringRef Filename = "") {
+TestTU TU;
+TU.HeaderCode = HeaderCode;
+TU.Code = Code;
+if (!Filename.empty())
+  TU.Filename = Filename;
+return TU;
+  }
+
   // The code to be compiled.
   std::string Code;
   std::string Filename = "TestTU.cpp";
Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -45,7 +45,7 @@
 
 SymbolSlab TestTU::headerSymbols() const {
   auto AST = build();
-  return indexAST(AST.getASTContext(), AST.getPreprocessorPtr());
+  return indexAST(AST.getASTContext(), AST.getPreprocessorPtr()).first;
 }
 
 std::unique_ptr TestTU::index() const {
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -74,6 +74,16 @@
 MATCHER_P(ForCodeCompletion, IsIndexedForCodeCompletion, "") {
   return arg.IsIndexedForCodeCompletion == IsIndexedForCodeCompletion;
 }
+MATCHER(OccurrenceRange, "") {
+  const clang::clangd::SymbolOccurrence& Pos = testing::get<0>(arg);
+  const clang::clangd::Range& Range = testing::get<1>(arg);
+  return std::tie(Pos.Location.Start.Line,
+  Pos.Location.Start.Column,
+  Pos.Location.End.Line,
+  Pos.Location.End.Column) ==
+ std::tie(Range.start.line, Range.start.character, Range.end.line,
+  Range.end.character);
+}
 
 namespace clang {
 namespace clangd {
@@ -237,6 +247,7 @@
 llvm::MemoryBuffer::getMemBuffer(MainCode));
 Invocation.run();
 Symbols = Factory->Collector->takeSymbols();
+SymbolOccurrences = Factory->Collector->takeOccurrences();
 return true;
   }
 
@@ -247,6 +258,7 @@
   std::string TestFileName;
   std::string TestFileURI;
   SymbolSlab Symbols;
+  SymbolOccurrenceSlab SymbolOccurrences;
   SymbolCollector::Options CollectorOpts;
   std::unique_ptr PragmaHandler;
 };
@@ -413,6 +425,67 @@
   ));
 }
 
+TEST_F(SymbolCollectorTest, Occurrences) {
+  Annotations Header(R"(
+  class $foo[[Foo]] {
+  public:
+$foo[[Foo]]() {}
+$foo[[Foo]](int);
+  };
+  class $bar[[Bar]];
+  void $func[[func]]();
+  )");
+  Annotations Main(R"(
+  class $bar[[Bar]] {};
+
+  void $func[[func]]();
+
+  void fff() {
+$foo[[Foo]] foo;
+$bar[[Bar]] bar;
+$func[[func]]();
+int abc = 0;
+$foo[[Foo]] foo2 = abc;
+  }
+  )");
+  Annotations SymbolsOnlyInMainCode(R"(
+  int a;
+  void b() {}
+  static const int c = 0;
+  class d {};
+  )");
+  CollectorOpts.OccurrenceFilter = SymbolOccurrenceKind::Declaration |
+   SymbolOccurrenceKind::Definition |
+   SymbolOccurrenceKind::Reference;
+  runSymbolCollector(Header.code(),
+ (Main.code() + SymbolsOnlyInMainCode.code()).str());
+  auto HeaderSymbols = TestTU::withHeaderCode(Header.code()).headerSymbols();
+
+  EXPECT_THAT(
+  SymbolOccurrences.find(findSymbol(Symbols, "Foo").ID),
+  testing::UnorderedPointwise(OccurrenceRange(), Main.ranges("foo")));
+  EXPECT_THAT(
+  SymbolOccurrences.find(findSymbol(Symbols, "Bar").ID),
+  testing::UnorderedPointwise(OccurrenceRange(), Main.ranges("bar")));
+  EXPECT_THAT(
+  SymbolOccurrences.find(findSymbol(Symbols, "func").ID),
+  testing::UnorderedPointwise(OccurrenceRange(), Main.ranges("func")));
+
+  // Retrieve IDs for symbols *only* in the main file, and verify these symbols
+  // are not collected.
+  auto MainSymbols =
+  TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
+  EXPECT_THAT(
+  SymbolOccurrences.find(findSymbol(MainSymbols, "a").ID),
+  testing::IsEmpty());
+  EXPECT_THAT(
+  SymbolOccurrences.find(findSymbol(MainSymbols, "b").ID),
+  testing::IsEmpty());
+  EXPECT_THAT(
+  SymbolOccurren

[PATCH] D51279: [clangd] Implement findOccurrences interface in dynamic index.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 163066.
hokein added a comment.

Minor cleanup.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51279

Files:
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/MemIndex.cpp
  clangd/index/MemIndex.h
  clangd/index/Merge.cpp
  clangd/index/Merge.h
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolCollector.h
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/IndexTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp
  unittests/clangd/TestTU.cpp
  unittests/clangd/TestTU.h

Index: unittests/clangd/TestTU.h
===
--- unittests/clangd/TestTU.h
+++ unittests/clangd/TestTU.h
@@ -38,6 +38,16 @@
 return TU;
   }
 
+  static TestTU withAllCode(llvm::StringRef HeaderCode, llvm::StringRef Code,
+llvm::StringRef Filename = "") {
+TestTU TU;
+TU.HeaderCode = HeaderCode;
+TU.Code = Code;
+if (!Filename.empty())
+  TU.Filename = Filename;
+return TU;
+  }
+
   // The code to be compiled.
   std::string Code;
   std::string Filename = "TestTU.cpp";
Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -45,7 +45,7 @@
 
 SymbolSlab TestTU::headerSymbols() const {
   auto AST = build();
-  return indexAST(AST.getASTContext(), AST.getPreprocessorPtr());
+  return indexAST(AST.getASTContext(), AST.getPreprocessorPtr()).first;
 }
 
 std::unique_ptr TestTU::index() const {
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -74,6 +74,16 @@
 MATCHER_P(ForCodeCompletion, IsIndexedForCodeCompletion, "") {
   return arg.IsIndexedForCodeCompletion == IsIndexedForCodeCompletion;
 }
+MATCHER(OccurrenceRange, "") {
+  const clang::clangd::SymbolOccurrence& Pos = testing::get<0>(arg);
+  const clang::clangd::Range& Range = testing::get<1>(arg);
+  return std::tie(Pos.Location.Start.Line,
+  Pos.Location.Start.Column,
+  Pos.Location.End.Line,
+  Pos.Location.End.Column) ==
+ std::tie(Range.start.line, Range.start.character, Range.end.line,
+  Range.end.character);
+}
 
 namespace clang {
 namespace clangd {
@@ -237,6 +247,7 @@
 llvm::MemoryBuffer::getMemBuffer(MainCode));
 Invocation.run();
 Symbols = Factory->Collector->takeSymbols();
+SymbolOccurrences = Factory->Collector->takeOccurrences();
 return true;
   }
 
@@ -247,6 +258,7 @@
   std::string TestFileName;
   std::string TestFileURI;
   SymbolSlab Symbols;
+  SymbolOccurrenceSlab SymbolOccurrences;
   SymbolCollector::Options CollectorOpts;
   std::unique_ptr PragmaHandler;
 };
@@ -413,6 +425,67 @@
   ));
 }
 
+TEST_F(SymbolCollectorTest, Occurrences) {
+  Annotations Header(R"(
+  class $foo[[Foo]] {
+  public:
+$foo[[Foo]]() {}
+$foo[[Foo]](int);
+  };
+  class $bar[[Bar]];
+  void $func[[func]]();
+  )");
+  Annotations Main(R"(
+  class $bar[[Bar]] {};
+
+  void $func[[func]]();
+
+  void fff() {
+$foo[[Foo]] foo;
+$bar[[Bar]] bar;
+$func[[func]]();
+int abc = 0;
+$foo[[Foo]] foo2 = abc;
+  }
+  )");
+  Annotations SymbolsOnlyInMainCode(R"(
+  int a;
+  void b() {}
+  static const int c = 0;
+  class d {};
+  )");
+  CollectorOpts.OccurrenceFilter = SymbolOccurrenceKind::Declaration |
+   SymbolOccurrenceKind::Definition |
+   SymbolOccurrenceKind::Reference;
+  runSymbolCollector(Header.code(),
+ (Main.code() + SymbolsOnlyInMainCode.code()).str());
+  auto HeaderSymbols = TestTU::withHeaderCode(Header.code()).headerSymbols();
+
+  EXPECT_THAT(
+  SymbolOccurrences.find(findSymbol(Symbols, "Foo").ID),
+  testing::UnorderedPointwise(OccurrenceRange(), Main.ranges("foo")));
+  EXPECT_THAT(
+  SymbolOccurrences.find(findSymbol(Symbols, "Bar").ID),
+  testing::UnorderedPointwise(OccurrenceRange(), Main.ranges("bar")));
+  EXPECT_THAT(
+  SymbolOccurrences.find(findSymbol(Symbols, "func").ID),
+  testing::UnorderedPointwise(OccurrenceRange(), Main.ranges("func")));
+
+  // Retrieve IDs for symbols *only* in the main file, and verify these symbols
+  // are not collected.
+  auto MainSymbols =
+  TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols();
+  EXPECT_THAT(
+  SymbolOccurrences.find(findSymbol(MainSymbols, "a").ID),
+  testing::IsEmpty());
+  EXPECT_THAT(
+  SymbolOccurrences.find(findSymbol(MainSymbols, "b").ID),
+  testing::IsEmpty());
+  EXPECT_THAT(
+  SymbolOccurrences.find(findSymbol(MainSymbols, "c").ID),
+  testing::IsEmpty());
+}
+
 TEST_F(Sym

[PATCH] D51279: [clangd] Implement findOccurrences interface in dynamic index.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks for the comments.




Comment at: clangd/index/FileIndex.cpp:48
 
+  if (TopLevelDecls) { // index main AST, set occurrence flag.
+CollectorOpts.OccurrenceFilter = SymbolOccurrenceKind::Declaration |

ilya-biryukov wrote:
> There is an implicit assumption (`TopLevelDecls != null` iff indexing the 
> main file AST in dynamic index) that does not seem quite obvious.
> Maybe add two overloads (for indexing the main AST and for indexing the 
> preamble AST) or add an extra parameter to be explicit more about those 
> choices?
This is a hacky way to detect whether we are indexing main AST or preamble AST 
-- because the current `FileIndex::update(PathRef Path, ...)` interface doesn't 
contain this information, and we use it both for `PreambleAST` and `MainAST` 
indexing in the client side (`ParsingCallback`).

I think we might want two dedicated methods (`updateMainAST`, and 
`updatePreambleAST`) in `FileSymbol`, and I think we should address it in a 
follow-up patch. WDYT?



Comment at: clangd/index/FileIndex.h:50
 
+  std::vector> allOccurrenceSlabs() 
const;
+

sammccall wrote:
> sammccall wrote:
> > This return type seems inconsistent with the design of this class.
> > The purpose of the `shared_ptr>` in `allSymbols` is to 
> > avoid exposing the concrete storage.
> > The analogue would I guess be `shared_ptr > vector>>`.
> > 
> > The cost of creating that does seem a little gross though.
> allOccurrences
> (name should reflect the semantics, not the type)
> The cost of creating that does seem a little gross though.

Previously I tried to avoid the cost of iterating/copying all occurrences. 
After some investigations, it seems that the number of symbol occurrences is 
small (~500 for `SemaDecl.cpp`, < 100 for `CodeComplete.cpp`). The cost should 
not be too expensive.



Comment at: clangd/index/FileIndex.h:100
 /// level decls obtained from \p AST are indexed.
-SymbolSlab
+std::pair
 indexAST(ASTContext &AST, std::shared_ptr PP,

ilya-biryukov wrote:
> Maybe use a struct to allow named fields? Would mean a tiny amount of extra 
> code in `indexAST`, but should make the client code slightly nicer.
I think the type of `pair` explicitly express its meaning well enough? And this 
function is exposed for unittests, I'm happy to do the change if you insist.



Comment at: clangd/index/MemIndex.cpp:36
   auto Idx = llvm::make_unique();
-  Idx->build(getSymbolsFromSlab(std::move(Slab)));
+  Idx->build(getSymbolsFromSlab(std::move(Slab)), {});
   return std::move(Idx);

sammccall wrote:
> we should find a way to avoid having functions implicitly zero out parts of 
> the index like this!
Made it explicit.



Comment at: clangd/index/Merge.cpp:95
+Static->findOccurrences(Req, [&](const SymbolOccurrence& O) {
+  if (llvm::is_contained(SeenFiles, O.Location.FileURI))
+return;

ilya-biryukov wrote:
> `llvm::is_contained` seems to be linear time, maybe replace with hash-table 
> lookup.
> Or am I missing something?
Good catch, I thought it had a trait for `Map`, `Set`, but it turns out that 
`is_contained` is only a `std::find` wrapper :(


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51279



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

[PATCH] D51417: [analyzer][UninitializedObjectChecker] Updated comments

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

Szelethus added inline comments.



Comment at: 
lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:377-397
-// TODO: This function constructs an incorrect string if a void pointer is a
-// part of the chain:
-//
-//   struct B { int x; }
-//
-//   struct A {
-// void *vptr;

Fixed in rC339653, but apparently I forgot to remove this.


Repository:
  rC Clang

https://reviews.llvm.org/D51417



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

[PATCH] D51360: [clang-tidy] Use simple string matching instead of Regex

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks very much for improving it!




Comment at: clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h:45
-  llvm::Regex RE(
-  "absl/(algorithm|base|container|debugging|memory|meta|numeric|strings|"
-  "synchronization|time|types|utility)");

The logic is a bit subtle here.  Agree that using `regex` is overkill.

We still want substring match here, since the Filename does not always start 
with `absl/`, so your improvement will change the behavior. I think we should 
do `absl/[absl-libraries]` substring match.


https://reviews.llvm.org/D51360



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

[PATCH] D51331: [OPENMP] Create non-const ident_t structs.

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


https://reviews.llvm.org/D51331



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

[PATCH] D51407: [Tooling] Do not restore working dir in ClangTool

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 163071.
ilya-biryukov added a comment.

- Use vfs for file accesses
- Report errors when getAbsolutePath fails, skip those files


Repository:
  rC Clang

https://reviews.llvm.org/D51407

Files:
  lib/Tooling/Tooling.cpp


Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -199,21 +199,25 @@
FileName, ToolName);
 }
 
-std::string getAbsolutePath(StringRef File) {
+static llvm::Expected getAbsolutePath(vfs::FileSystem &FS,
+   StringRef File) {
   StringRef RelativePath(File);
   // FIXME: Should '.\\' be accepted on Win32?
   if (RelativePath.startswith("./")) {
 RelativePath = RelativePath.substr(strlen("./"));
   }
 
   SmallString<1024> AbsolutePath = RelativePath;
-  std::error_code EC = llvm::sys::fs::make_absolute(AbsolutePath);
-  assert(!EC);
-  (void)EC;
+  if (auto EC = FS.makeAbsolute(AbsolutePath))
+return llvm::errorCodeToError(EC);
   llvm::sys::path::native(AbsolutePath);
   return AbsolutePath.str();
 }
 
+std::string getAbsolutePath(StringRef File) {
+  return llvm::cantFail(getAbsolutePath(*vfs::getRealFileSystem(), File));
+}
+
 void addTargetAndModeForProgramName(std::vector &CommandLine,
 StringRef InvokedAs) {
   if (!CommandLine.empty() && !InvokedAs.empty()) {
@@ -411,15 +415,6 @@
   // This just needs to be some symbol in the binary.
   static int StaticSymbol;
 
-  std::string InitialDirectory;
-  if (llvm::ErrorOr CWD =
-  OverlayFileSystem->getCurrentWorkingDirectory()) {
-InitialDirectory = std::move(*CWD);
-  } else {
-llvm::report_fatal_error("Cannot detect current path: " +
- Twine(CWD.getError().message()));
-  }
-
   // First insert all absolute paths into the in-memory VFS. These are global
   // for all compile commands.
   if (SeenWorkingDirectories.insert("/").second)
@@ -431,9 +426,22 @@
 
   bool ProcessingFailed = false;
   bool FileSkipped = false;
+  // Compute all absolute paths before we run any actions, as those will change
+  // the working directory.
+  std::vector AbsolutePaths;
+  AbsolutePaths.reserve(SourcePaths.size());
   for (const auto &SourcePath : SourcePaths) {
-std::string File(getAbsolutePath(SourcePath));
+auto AbsPath = getAbsolutePath(*OverlayFileSystem, SourcePath);
+if (!AbsPath) {
+  llvm::errs() << "Skipping " << SourcePath
+   << ". Error while getting an absolute path: "
+   << llvm::toString(AbsPath.takeError()) << "\n";
+  continue;
+}
+AbsolutePaths.push_back(std::move(*AbsPath));
+  }
 
+  for (llvm::StringRef File : AbsolutePaths) {
 // Currently implementations of CompilationDatabase::getCompileCommands can
 // change the state of the file system (e.g.  prepare generated headers), 
so
 // this method needs to run right before we invoke the tool, as the next
@@ -498,11 +506,6 @@
 llvm::errs() << "Error while processing " << File << ".\n";
 ProcessingFailed = true;
   }
-  // Return to the initial directory to correctly resolve next file by
-  // relative path.
-  if 
(OverlayFileSystem->setCurrentWorkingDirectory(InitialDirectory.c_str()))
-llvm::report_fatal_error("Cannot chdir into \"" +
- Twine(InitialDirectory) + "\n!");
 }
   }
   return ProcessingFailed ? 1 : (FileSkipped ? 2 : 0);


Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -199,21 +199,25 @@
FileName, ToolName);
 }
 
-std::string getAbsolutePath(StringRef File) {
+static llvm::Expected getAbsolutePath(vfs::FileSystem &FS,
+   StringRef File) {
   StringRef RelativePath(File);
   // FIXME: Should '.\\' be accepted on Win32?
   if (RelativePath.startswith("./")) {
 RelativePath = RelativePath.substr(strlen("./"));
   }
 
   SmallString<1024> AbsolutePath = RelativePath;
-  std::error_code EC = llvm::sys::fs::make_absolute(AbsolutePath);
-  assert(!EC);
-  (void)EC;
+  if (auto EC = FS.makeAbsolute(AbsolutePath))
+return llvm::errorCodeToError(EC);
   llvm::sys::path::native(AbsolutePath);
   return AbsolutePath.str();
 }
 
+std::string getAbsolutePath(StringRef File) {
+  return llvm::cantFail(getAbsolutePath(*vfs::getRealFileSystem(), File));
+}
+
 void addTargetAndModeForProgramName(std::vector &CommandLine,
 StringRef InvokedAs) {
   if (!CommandLine.empty() && !InvokedAs.empty()) {
@@ -411,15 +415,6 @@
   // This just needs to be some symbol in the binary.
   static int StaticSymbol;
 
-  std::string InitialDirectory;
-  if (llvm::ErrorOr CWD =
-  OverlayFileSystem->getCu

[PATCH] D51407: [Tooling] Do not restore working dir in ClangTool

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: lib/Tooling/Tooling.cpp:419
-  } else {
-llvm::report_fatal_error("Cannot detect current path: " +
- Twine(CWD.getError().message()));

ioeric wrote:
> ilya-biryukov wrote:
> > ioeric wrote:
> > > Should we still check if `CWD` is correctly set?
> > Not sure we know which CWD is correct at this point. Do you mean to check 
> > that `getCurrentWorkingDirectory ` does not return an error?
> Yes. 
> 
> Actually, after a closer look, I noticed `getAbsolutePath` below is just 
> using the current working directory in real file system, which seems wrong. 
> This is pre-existing but would be nice to fix since we are here :)
What do we do if CWD is wrong? Checking for errors in getAbsolutePath and 
setCurrentWorkingDirectory should be enough and that's what we already do.

> Actually, after a closer look, I noticed getAbsolutePath below is just using 
> the current working directory in real file system, which seems wrong. This is 
> pre-existing but would be nice to fix since we are here :)
Unfortunately, `getAbsolutePath` is a public API (with not too many usages, but 
still). I've added a helper function that goes through VFS and forwarding to it 
in the `getAbsoltuePath` for now, but we might want to look at changing the API 
there.


Repository:
  rC Clang

https://reviews.llvm.org/D51407



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

[PATCH] D51380: Fix incorrect usage of std::error_category

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Thanks for fixing this.


Repository:
  rC Clang

https://reviews.llvm.org/D51380



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

[PATCH] D51279: [clangd] Implement findOccurrences interface in dynamic index.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/index/FileIndex.cpp:48
 
+  if (TopLevelDecls) { // index main AST, set occurrence flag.
+CollectorOpts.OccurrenceFilter = SymbolOccurrenceKind::Declaration |

hokein wrote:
> ilya-biryukov wrote:
> > There is an implicit assumption (`TopLevelDecls != null` iff indexing the 
> > main file AST in dynamic index) that does not seem quite obvious.
> > Maybe add two overloads (for indexing the main AST and for indexing the 
> > preamble AST) or add an extra parameter to be explicit more about those 
> > choices?
> This is a hacky way to detect whether we are indexing main AST or preamble 
> AST -- because the current `FileIndex::update(PathRef Path, ...)` interface 
> doesn't contain this information, and we use it both for `PreambleAST` and 
> `MainAST` indexing in the client side (`ParsingCallback`).
> 
> I think we might want two dedicated methods (`updateMainAST`, and 
> `updatePreambleAST`) in `FileSymbol`, and I think we should address it in a 
> follow-up patch. WDYT?
Either a dedicated methods or a separate parameter LG.
And it's probably ok to do it with a follow-up change if you want to keep the 
scope of this patch smaller. That should be a pretty small change overall, so 
it should be fine either way.



Comment at: clangd/index/FileIndex.h:100
 /// level decls obtained from \p AST are indexed.
-SymbolSlab
+std::pair
 indexAST(ASTContext &AST, std::shared_ptr PP,

hokein wrote:
> ilya-biryukov wrote:
> > Maybe use a struct to allow named fields? Would mean a tiny amount of extra 
> > code in `indexAST`, but should make the client code slightly nicer.
> I think the type of `pair` explicitly express its meaning well enough? And 
> this function is exposed for unittests, I'm happy to do the change if you 
> insist.
Not really, that was just a suggestion. Feel free to ignore if the current 
version looks better to you.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51279



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

[PATCH] D51422: [clangd] Factor out the data-swapping functionality from MemIndex/DexIndex.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kbobyrev.
Herald added subscribers: cfe-commits, kadircet, arphaman, mgrang, jkorous, 
MaskRay, ioeric, ilya-biryukov.

This is now handled by a wrapper class SwapIndex, so MemIndex/DexIndex can be
immutable and focus on their job.

Old and busted:
 I have a MemIndex, which holds a shared_ptr>, which keeps the
 symbol slab alive. I update by calling build(shared_ptr>).

New hotness: I have a SwapIndex, which holds a shared_ptr, which
 holds a MemIndex and also keeps any data backing it alive.
 I update by building a new MemIndex and calling SwapIndex::reset().

This resulted in a bunch of interface churn (some places previously using
unique_ptr should now be shared_ptr, and some using MemIndex() + MemIndex::build
should now call the static MemIndex::build factory instead).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51422

Files:
  clangd/index/FileIndex.cpp
  clangd/index/FileIndex.h
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/MemIndex.cpp
  clangd/index/MemIndex.h
  clangd/index/dex/DexIndex.cpp
  clangd/index/dex/DexIndex.h
  clangd/tool/ClangdMain.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/DexIndexTests.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/IndexTests.cpp
  unittests/clangd/TestIndex.cpp
  unittests/clangd/TestIndex.h
  unittests/clangd/TestTU.cpp
  unittests/clangd/TestTU.h

Index: unittests/clangd/TestTU.h
===
--- unittests/clangd/TestTU.h
+++ unittests/clangd/TestTU.h
@@ -51,7 +51,7 @@
 
   ParsedAST build() const;
   SymbolSlab headerSymbols() const;
-  std::unique_ptr index() const;
+  std::shared_ptr index() const;
 };
 
 // Look up an index symbol by qualified name, which must be unique.
Index: unittests/clangd/TestTU.cpp
===
--- unittests/clangd/TestTU.cpp
+++ unittests/clangd/TestTU.cpp
@@ -48,7 +48,7 @@
   return indexAST(AST.getASTContext(), AST.getPreprocessorPtr());
 }
 
-std::unique_ptr TestTU::index() const {
+std::shared_ptr TestTU::index() const {
   return MemIndex::build(headerSymbols());
 }
 
Index: unittests/clangd/TestIndex.h
===
--- unittests/clangd/TestIndex.h
+++ unittests/clangd/TestIndex.h
@@ -23,26 +23,11 @@
 // Creates Symbol instance and sets SymbolID to given QualifiedName.
 Symbol symbol(llvm::StringRef QName);
 
-// Bundles symbol pointers with the actual symbol slab the pointers refer to in
-// order to ensure that the slab isn't destroyed while it's used by and index.
-struct SlabAndPointers {
-  SymbolSlab Slab;
-  std::vector Pointers;
-};
+// Create a slab of symbols with the given qualified names as IDs and names.
+SymbolSlab generateSymbols(std::vector QualifiedNames);
 
-// Create a slab of symbols with the given qualified names as both IDs and
-// names. The life time of the slab is managed by the returned shared pointer.
-// If \p WeakSymbols is provided, it will be pointed to the managed object in
-// the returned shared pointer.
-std::shared_ptr>
-generateSymbols(std::vector QualifiedNames,
-std::weak_ptr *WeakSymbols = nullptr);
-
-// Create a slab of symbols with IDs and names [Begin, End], otherwise identical
-// to the `generateSymbols` above.
-std::shared_ptr>
-generateNumSymbols(int Begin, int End,
-   std::weak_ptr *WeakSymbols = nullptr);
+// Create a slab of symbols with IDs and names [Begin, End].
+SymbolSlab generateNumSymbols(int Begin, int End);
 
 // Returns fully-qualified name out of given symbol.
 std::string getQualifiedName(const Symbol &Sym);
Index: unittests/clangd/TestIndex.cpp
===
--- unittests/clangd/TestIndex.cpp
+++ unittests/clangd/TestIndex.cpp
@@ -26,30 +26,18 @@
   return Sym;
 }
 
-std::shared_ptr>
-generateSymbols(std::vector QualifiedNames,
-std::weak_ptr *WeakSymbols) {
+SymbolSlab generateSymbols(std::vector QualifiedNames) {
   SymbolSlab::Builder Slab;
   for (llvm::StringRef QName : QualifiedNames)
 Slab.insert(symbol(QName));
-
-  auto Storage = std::make_shared();
-  Storage->Slab = std::move(Slab).build();
-  for (const auto &Sym : Storage->Slab)
-Storage->Pointers.push_back(&Sym);
-  if (WeakSymbols)
-*WeakSymbols = Storage;
-  auto *Pointers = &Storage->Pointers;
-  return {std::move(Storage), Pointers};
+  return std::move(Slab).build();
 }
 
-std::shared_ptr>
-generateNumSymbols(int Begin, int End,
-   std::weak_ptr *WeakSymbols) {
+SymbolSlab generateNumSymbols(int Begin, int End) {
   std::vector Names;
   for (int i = Begin; i <= End; i++)
 Names.push_back(std::to_string(i));
-  return generateSymbols(Names, WeakSymbols);
+  return generateSymbols(Names);
 }
 
 std::string getQualifiedName(const Symbol &Sym) {
Index: unittests/clangd/Inde

[PATCH] D51291: [clangd] Support multiple #include headers in one symbol.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 163074.
ioeric added a comment.

- minor cleanup


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51291

Files:
  clangd/CodeComplete.cpp
  clangd/index/Index.cpp
  clangd/index/Index.h
  clangd/index/Merge.cpp
  clangd/index/SymbolCollector.cpp
  clangd/index/SymbolYAML.cpp
  unittests/clangd/CodeCompleteTests.cpp
  unittests/clangd/FileIndexTests.cpp
  unittests/clangd/IndexTests.cpp
  unittests/clangd/SymbolCollectorTests.cpp

Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -53,7 +53,11 @@
 MATCHER_P(DeclURI, P, "") { return arg.CanonicalDeclaration.FileURI == P; }
 MATCHER_P(DefURI, P, "") { return arg.Definition.FileURI == P; }
 MATCHER_P(IncludeHeader, P, "") {
-  return arg.Detail && arg.Detail->IncludeHeader == P;
+  return (arg.IncludeHeaders.size() == 1) &&
+ (arg.IncludeHeaders.begin()->IncludeHeader == P);
+}
+MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References,  "") {
+  return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
 }
 MATCHER_P(DeclRange, Pos, "") {
   return std::tie(arg.CanonicalDeclaration.Start.Line,
@@ -696,6 +700,11 @@
 Line: 1
 Column: 1
 IsIndexedForCodeCompletion:true
+IncludeHeaders:
+  - Header:'include1'
+References:7
+  - Header:'include2'
+References:3
 Detail:
   Documentation:'Foo doc'
   ReturnType:'int'
@@ -730,6 +739,11 @@
  Doc("Foo doc"), ReturnType("int"),
  DeclURI("file:///path/foo.h"),
  ForCodeCompletion(true;
+  auto &Sym1 = *Symbols1.begin();
+  assert(Sym1.Detail);
+  EXPECT_THAT(Sym1.IncludeHeaders,
+  UnorderedElementsAre(IncludeHeaderWithRef("include1", 7u),
+   IncludeHeaderWithRef("include2", 3u)));
   auto Symbols2 = symbolsFromYAML(YAML2);
   EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(
 QName("clang::Foo2"), Labeled("Foo2-sig"),
@@ -751,9 +765,10 @@
 TEST_F(SymbolCollectorTest, IncludeHeaderSameAsFileURI) {
   CollectorOpts.CollectIncludePath = true;
   runSymbolCollector("class Foo {};", /*Main=*/"");
-  EXPECT_THAT(Symbols,
-  UnorderedElementsAre(AllOf(QName("Foo"), DeclURI(TestHeaderURI),
- IncludeHeader(TestHeaderURI;
+  EXPECT_THAT(Symbols, UnorderedElementsAre(
+   AllOf(QName("Foo"), DeclURI(TestHeaderURI;
+  EXPECT_THAT(Symbols.begin()->IncludeHeaders,
+  UnorderedElementsAre(IncludeHeaderWithRef(TestHeaderURI, 1u)));
 }
 
 #ifndef _WIN32
Index: unittests/clangd/IndexTests.cpp
===
--- unittests/clangd/IndexTests.cpp
+++ unittests/clangd/IndexTests.cpp
@@ -243,6 +243,48 @@
   EXPECT_EQ(M.Name, "right");
 }
 
+MATCHER_P2(IncludeHeaderWithRef, IncludeHeader, References,  "") {
+  return (arg.IncludeHeader == IncludeHeader) && (arg.References == References);
+}
+
+TEST(MergeTest, MergeIncludesOnDifferentDefinitions) {
+  Symbol L, R;
+  L.Name = "left";
+  R.Name = "right";
+  L.ID = R.ID = SymbolID("hello");
+  L.IncludeHeaders.emplace_back("common", 1);
+  R.IncludeHeaders.emplace_back("common", 1);
+  R.IncludeHeaders.emplace_back("new", 1);
+
+  Symbol::Details Scratch;
+
+  // Both have no definition.
+  Symbol M = mergeSymbol(L, R, &Scratch);
+  EXPECT_THAT(M.IncludeHeaders,
+  UnorderedElementsAre(IncludeHeaderWithRef("common", 2u),
+   IncludeHeaderWithRef("new", 1u)));
+
+  // Only merge references of the same includes but do not merge new #includes.
+  L.Definition.FileURI = "file:/left.h";
+  M = mergeSymbol(L, R, &Scratch);
+  EXPECT_THAT(M.IncludeHeaders,
+  UnorderedElementsAre(IncludeHeaderWithRef("common", 2u)));
+
+  // Definitions are the same.
+  R.Definition.FileURI = "file:/right.h";
+  M = mergeSymbol(L, R, &Scratch);
+  EXPECT_THAT(M.IncludeHeaders,
+  UnorderedElementsAre(IncludeHeaderWithRef("common", 2u),
+   IncludeHeaderWithRef("new", 1u)));
+
+  // Definitions are different.
+  R.Definition.FileURI = "file:/right.h";
+  M = mergeSymbol(L, R, &Scratch);
+  EXPECT_THAT(M.IncludeHeaders,
+  UnorderedElementsAre(IncludeHeaderWithRef("common", 2u),
+   IncludeHeaderWithRef("new", 1u)));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: unittests/clangd/FileIndexTests.cpp
===
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -177,7 +177,7 @@
   Req.Query = "";
   bool SeenSymbol = false;
   M.fuzzyFind(Req, [&]

[PATCH] D51341: [HEADER] Overloadable function candidates for half/double types

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Sema/SemaOverload.cpp:6063
+  // OpenCL: A candidate function that uses extentions that are not enabled or
+  // supported is not viable.
+  if (getLangOpts().OpenCL) {

I would imagine if extension isn't supported the candidate function with data 
type defined by extension shouldn't be available at all during compilation?

Also is there any good way to generalize this for all types and extensions 
including vendor ones that are added with the pragmas?
https://clang.llvm.org/docs/UsersManual.html#opencl-extensions



Comment at: test/SemaOpenCL/half-double-overload.cl:7
+
+int __attribute__((overloadable)) goo(float in1, float in2);
+half __attribute__((overloadable)) goo(double in1, double in2);

I think it will be clearer if candidates with non-half parameters moved out of 
extension enabled block.



Comment at: test/SemaOpenCL/half-double-overload.cl:18
+float __attribute__((overloadable)) foo_err(half in1, half in2);
+// expected-note@-1 {{candidate disabled due to OpenCL extension}}
+float __attribute__((overloadable)) foo_err(half in1, int in2);

Wondering if better message could be:
  candidate unavailable as it requires OpenCL extension to be disabled

Could it also print the extension name?


https://reviews.llvm.org/D51341



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

[PATCH] D51341: [HEADER] Overloadable function candidates for half/double types

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

I think this is related to: https://reviews.llvm.org/D46501


https://reviews.llvm.org/D51341



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

[PATCH] D51411: [OpenCL] Improve diagnostic of argument in address space conversion builtins

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Is this a feature requested by users?

I can understand that this may be useful to pinpoint some conversions which are 
potentially harmful to performance. However such conversions can often be 
eliminated by optimization, which makes this warning less useful.

On the other hand, too many warnings is a nuance to users, therefore I am 
wondering whether this warning should be off by default.

Do you have any chance to have any feedback from users about how useful or 
intrusive this warning is.


https://reviews.llvm.org/D51411



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

[PATCH] D51402: [OpenCL] Adding cl_intel_planar_yuv extension

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

Will this extension require any Clang changes that are not possible to add 
using this approach for vendor extensions:
https://clang.llvm.org/docs/UsersManual.html#opencl-extensions

If not I would suggest to add this to the header file instead.


Repository:
  rC Clang

https://reviews.llvm.org/D51402



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

[PATCH] D51411: [OpenCL] Improve diagnostic of argument in address space conversion builtins

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: include/clang/Basic/DiagnosticSemaKinds.td:8584
+  "passing non-generic address space pointer to %0"
+  " generates dynamic conversion where it is not needed">;
 

How about change this to:

may cause dynamic conversion affecting performance


https://reviews.llvm.org/D51411



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

[PATCH] D51292: [docs] Update clang-rename documentation

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Leaving some comments, but also suggest getting the final LGTM from the owner 
of the doc (@ioeric?)




Comment at: clang-tools-extra/docs/clang-rename.rst:141
+:program:`clangd `_ uses
+:program:`clang-rename` infrastructure to handle renaming requests. Currently,
+it only supports renaming symbol within a single file, but in the future it 
will

`handle renaming requests` seems to assume some familiarity with LSP. Maybe 
rephrase?



Comment at: clang-tools-extra/docs/clang-rename.rst:170
 `clang-rename/tool/clang-rename.el
-`_.
+`_.
 

Accidentally linked to clang-rename.py instead clang-rename.el?


https://reviews.llvm.org/D51292



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

[PATCH] D43783: [OpenCL] Remove block invoke function from emitted block literal struct

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In https://reviews.llvm.org/D43783#1215573, @Anastasia wrote:

> In https://reviews.llvm.org/D43783#1212485, @yaxunl wrote:
>
> > In https://reviews.llvm.org/D43783#1204353, @svenvh wrote:
> >
> > > Sorry for digging up an old commit...
> > >
> > > Apparently this broke block arguments, e.g. the following test case:
> > >
> > >   int foo(int (^ bl)(void)) {
> > > return bl();
> > >   }
> > >  
> > >   int get21() {
> > > return foo(^{return 21;});
> > >   }
> > >  
> > >   int get42() {
> > > return foo(^{return 42;});
> > >   }
> > >
> > >
> > > In particular, the VarDecl that `getBlockExpr()` sees doesn't have an 
> > > initializer when the called block comes from an argument (causing clang 
> > > to crash).
> >
> >
> > Sorry for the delay. I think block should not be allowed as function 
> > argument since this generally leads indirect function calls therefore 
> > requires support of function pointer. It will rely on optimizations to get 
> > rid of indirect function calls.
>
>
> The idea was to allow blocks as function parameters because they are 
> statically known at each function call. This can be resolved later at IR 
> level instead of frontend. I am also not sure there can be other corner cases 
> where it wouldn't work in Clang since we can't leverage analysis passes here. 
> I feel this might be a risky thing to do in Clang. Currently it doesn't work 
> for the examples provided and therefore breaking the compliance with the spec.


I agree this patch should be reverted for conformance to spec. I will do that. 
Thanks.


Repository:
  rC Clang

https://reviews.llvm.org/D43783



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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg updated this revision to Diff 163083.
hugoeg marked an inline comment as done.
hugoeg added a comment.

nit comment fixed


https://reviews.llvm.org/D50542

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/NoInternalDependenciesCheck.cpp
  clang-tidy/abseil/NoInternalDependenciesCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-no-internal-dependencies.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/Inputs/absl/external-file.h
  test/clang-tidy/Inputs/absl/strings/internal-file.h
  test/clang-tidy/abseil-no-internal-dependencies.cpp

Index: test/clang-tidy/abseil-no-internal-dependencies.cpp
===
--- test/clang-tidy/abseil-no-internal-dependencies.cpp
+++ test/clang-tidy/abseil-no-internal-dependencies.cpp
@@ -0,0 +1,39 @@
+// RUN: %check_clang_tidy %s abseil-no-internal-dependencies %t,  -- -- -I %S/Inputs
+// RUN: clang-tidy -checks='-*, abseil-no-internal-dependencies' -header-filter='.*' %s -- -I %S/Inputs 2>&1 | FileCheck %s
+
+#include "absl/strings/internal-file.h"
+// CHECK-NOT: warning:
+
+#include "absl/external-file.h"
+// CHECK: absl/external-file.h:6:24: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil [abseil-no-internal-dependencies]
+
+void DirectAcess() {
+  absl::strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+
+  absl::strings_internal::InternalTemplateFunction("a");
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+
+class FriendUsage {
+  friend struct absl::container_internal::InternalStruct;
+  // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+};
+
+namespace absl {
+void OpeningNamespace() {
+  strings_internal::InternalFunction();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: do not reference any 'internal' namespaces; those implementation details are reserved to Abseil
+}
+} // namespace absl
+
+// should not trigger warnings
+void CorrectUsage() {
+  std::string Str = absl::StringsFunction("a");
+  absl::SomeContainer b;
+}
+
+namespace absl {
+SomeContainer b;
+std::string Str = absl::StringsFunction("a");
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/strings/internal-file.h
===
--- test/clang-tidy/Inputs/absl/strings/internal-file.h
+++ test/clang-tidy/Inputs/absl/strings/internal-file.h
@@ -1 +1,33 @@
-namespace absl {}
+namespace std {
+struct string {
+  string(const char *);
+  ~string();
+};
+} // namespace std
+
+namespace absl {
+std::string StringsFunction(std::string s1) { return s1; }
+class SomeContainer {};
+namespace strings_internal {
+void InternalFunction() {}
+template  P InternalTemplateFunction(P a) {}
+} // namespace strings_internal
+
+namespace container_internal {
+struct InternalStruct {};
+} // namespace container_internal
+} // namespace absl
+
+// should not trigger warnings because inside Abseil files
+void DirectAcessInternal() {
+  absl::strings_internal::InternalFunction();
+  absl::strings_internal::InternalTemplateFunction("a");
+}
+
+class FriendUsageInternal {
+  friend struct absl::container_internal::InternalStruct;
+};
+
+namespace absl {
+void OpeningNamespaceInternally() { strings_internal::InternalFunction(); }
+} // namespace absl
Index: test/clang-tidy/Inputs/absl/external-file.h
===
--- test/clang-tidy/Inputs/absl/external-file.h
+++ test/clang-tidy/Inputs/absl/external-file.h
@@ -1 +1,6 @@
-namespace absl {}
+namespace absl {
+namespace base_internal {
+void InternalFunction() {}
+} // namespace base_internal 
+} //namespace absl
+void DirectAccess2() { absl::base_internal::InternalFunction(); }
Index: docs/clang-tidy/checks/list.rst
===
--- docs/clang-tidy/checks/list.rst
+++ docs/clang-tidy/checks/list.rst
@@ -6,6 +6,7 @@
 .. toctree::
abseil-duration-division
abseil-faster-strsplit-delimiter
+   abseil-no-internal-dependencies
abseil-no-namespace
abseil-string-find-startswith
android-cloexec-accept
Index: docs/clang-tidy/checks/abseil-no-internal-dependencies.rst
===
--- docs/clang-tidy/checks/abseil-no-internal-dependencies.rst
+++ docs/clang-tidy/checks/abseil-no-internal-dependencies.rst
@@ -0,0 +1,24 @@
+subl.. title:: clang-tidy - abseil-no-internal-dependencies
+
+abseil-no-internal-dependencies
+===
+
+Warns if code using Abseil depends on internal details. If something is in a
+namespace that includ

[PATCH] D49244: Always search sysroot for GCC installs

[David Greene via Phabricator via cfe-commits]

greened added a comment.

I agree that option 2 seems best.  It's consistent with options overriding more 
general configuration/environment variables and more specific options 
overriding more general options.  If it turns out option 1 is needed in the 
future, it should be a simple change given an option 2 implementation.


Repository:
  rC Clang

https://reviews.llvm.org/D49244



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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D50542#1217511, @hugoeg wrote:

> nit comment fixed


DO you have commit rights or shall i commit for you?


https://reviews.llvm.org/D50542



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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

In https://reviews.llvm.org/D50542#1217515, @JonasToth wrote:

> In https://reviews.llvm.org/D50542#1217511, @hugoeg wrote:
>
> > nit comment fixed
>
>
> DO you have commit rights or shall i commit for you?


I do not have commit rights so I would appreciate it if you would commit me. 
Thanks!


https://reviews.llvm.org/D50542



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

[clang-tools-extra] r340928 - [clang-tidy] Add abseil-no-internal-dependencies check

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Wed Aug 29 07:23:15 2018
New Revision: 340928

URL: http://llvm.org/viewvc/llvm-project?rev=340928&view=rev
Log:
[clang-tidy] Add abseil-no-internal-dependencies check

Finds instances where the user depends on internal details and warns them 
against doing so.
Should not be run on internal Abseil files or Abseil source code.

Patch by hugoeg!

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

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

clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-no-internal-dependencies.rst
clang-tools-extra/trunk/test/clang-tidy/abseil-no-internal-dependencies.cpp
Modified:
clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
clang-tools-extra/trunk/docs/ReleaseNotes.rst
clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/external-file.h
clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/strings/internal-file.h

Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp?rev=340928&r1=340927&r2=340928&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp Wed Aug 29 
07:23:15 2018
@@ -12,6 +12,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "DurationDivisionCheck.h"
 #include "FasterStrsplitDelimiterCheck.h"
+#include "NoInternalDependenciesCheck.h"
 #include "NoNamespaceCheck.h"
 #include "RedundantStrcatCallsCheck.h"
 #include "StringFindStartswithCheck.h"
@@ -28,6 +29,8 @@ public:
 "abseil-duration-division");
 CheckFactories.registerCheck(
 "abseil-faster-strsplit-delimiter");
+CheckFactories.registerCheck(
+"abseil-no-internal-dependencies");
 CheckFactories.registerCheck("abseil-no-namespace");
 CheckFactories.registerCheck(
 "abseil-redundant-strcat-calls");

Modified: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt?rev=340928&r1=340927&r2=340928&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt Wed Aug 29 
07:23:15 2018
@@ -4,6 +4,7 @@ add_clang_library(clangTidyAbseilModule
   AbseilTidyModule.cpp
   DurationDivisionCheck.cpp
   FasterStrsplitDelimiterCheck.cpp
+  NoInternalDependenciesCheck.cpp
   NoNamespaceCheck.cpp
   RedundantStrcatCallsCheck.cpp
   StringFindStartswithCheck.cpp

Added: clang-tools-extra/trunk/clang-tidy/abseil/NoInternalDependenciesCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/NoInternalDependenciesCheck.cpp?rev=340928&view=auto
==
--- clang-tools-extra/trunk/clang-tidy/abseil/NoInternalDependenciesCheck.cpp 
(added)
+++ clang-tools-extra/trunk/clang-tidy/abseil/NoInternalDependenciesCheck.cpp 
Wed Aug 29 07:23:15 2018
@@ -0,0 +1,48 @@
+//===--- NoInternalDependenciesCheck.cpp - 
clang-tidy--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "NoInternalDependenciesCheck.h"
+#include "AbseilMatcher.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+void NoInternalDependenciesCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().CPlusPlus)
+return;
+
+  // TODO: refactor matcher to be configurable or just match on any internal
+  // access from outside the enclosing namespace.
+
+  Finder->addMatcher(
+  nestedNameSpecifierLoc(loc(specifiesNamespace(namespaceDecl(
+ matchesName("internal"),
+ hasParent(namespaceDecl(hasName("absl")),
+ unless(isInAbseilFile()))
+  .bind("InternalDep"),
+  this);
+}
+
+void NoInternalDependenciesCheck::check(const MatchFinder::MatchResult 
&Result) {
+  const auto *InternalDependency =
+  Result.Nodes.getNodeAs("InternalDep");
+
+  diag(InternalDependency->getBeginLoc(),
+   "do not reference any 'internal' namespaces; those implementation "
+   "detail

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Committed r340928. Thanks very much!

Will you (and your team) upstream even more abseil checks? I think it might be 
reasonable to ask for commit rights.


https://reviews.llvm.org/D50542



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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Jonas Toth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340928: [clang-tidy] Add abseil-no-internal-dependencies 
check (authored by JonasToth, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D50542?vs=163083&id=163085#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D50542

Files:
  clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/abseil/NoInternalDependenciesCheck.cpp
  clang-tools-extra/trunk/clang-tidy/abseil/NoInternalDependenciesCheck.h
  clang-tools-extra/trunk/docs/ReleaseNotes.rst
  
clang-tools-extra/trunk/docs/clang-tidy/checks/abseil-no-internal-dependencies.rst
  clang-tools-extra/trunk/docs/clang-tidy/checks/list.rst
  clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/external-file.h
  clang-tools-extra/trunk/test/clang-tidy/Inputs/absl/strings/internal-file.h
  clang-tools-extra/trunk/test/clang-tidy/abseil-no-internal-dependencies.cpp

Index: clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
===
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilTidyModule.cpp
@@ -12,6 +12,7 @@
 #include "../ClangTidyModuleRegistry.h"
 #include "DurationDivisionCheck.h"
 #include "FasterStrsplitDelimiterCheck.h"
+#include "NoInternalDependenciesCheck.h"
 #include "NoNamespaceCheck.h"
 #include "RedundantStrcatCallsCheck.h"
 #include "StringFindStartswithCheck.h"
@@ -28,6 +29,8 @@
 "abseil-duration-division");
 CheckFactories.registerCheck(
 "abseil-faster-strsplit-delimiter");
+CheckFactories.registerCheck(
+"abseil-no-internal-dependencies");
 CheckFactories.registerCheck("abseil-no-namespace");
 CheckFactories.registerCheck(
 "abseil-redundant-strcat-calls");
Index: clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
===
--- clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
+++ clang-tools-extra/trunk/clang-tidy/abseil/CMakeLists.txt
@@ -4,6 +4,7 @@
   AbseilTidyModule.cpp
   DurationDivisionCheck.cpp
   FasterStrsplitDelimiterCheck.cpp
+  NoInternalDependenciesCheck.cpp
   NoNamespaceCheck.cpp
   RedundantStrcatCallsCheck.cpp
   StringFindStartswithCheck.cpp
Index: clang-tools-extra/trunk/clang-tidy/abseil/NoInternalDependenciesCheck.h
===
--- clang-tools-extra/trunk/clang-tidy/abseil/NoInternalDependenciesCheck.h
+++ clang-tools-extra/trunk/clang-tidy/abseil/NoInternalDependenciesCheck.h
@@ -0,0 +1,36 @@
+//===--- NoInternalDependenciesCheck.h - clang-tidy--*- C++ -*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NOINTERNALDEPSCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NOINTERNALDEPSCHECK_H
+
+#include "../ClangTidy.h"
+
+namespace clang {
+namespace tidy {
+namespace abseil {
+
+/// Finds instances where the user depends on internal details and warns them
+/// against doing so.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/abseil-no-internal-dependencies.html
+class NoInternalDependenciesCheck : public ClangTidyCheck {
+public:
+  NoInternalDependenciesCheck(StringRef Name, ClangTidyContext *Context)
+  : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+};
+
+} // namespace abseil
+} // namespace tidy
+} // namespace clang
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_ABSEIL_NOINTERNALDEPSCHECK_H
Index: clang-tools-extra/trunk/clang-tidy/abseil/NoInternalDependenciesCheck.cpp
===
--- clang-tools-extra/trunk/clang-tidy/abseil/NoInternalDependenciesCheck.cpp
+++ clang-tools-extra/trunk/clang-tidy/abseil/NoInternalDependenciesCheck.cpp
@@ -0,0 +1,48 @@
+//===--- NoInternalDependenciesCheck.cpp - clang-tidy--===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+
+#include "NoInternalDependenciesCheck.h"
+#include "AbseilMatcher.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang {
+nam

[PATCH] D51426: [Sema] Prohibit function-scope compound literals with address spaces.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan created this revision.
ebevhan added reviewers: efriedma, rjmccall.
Herald added a subscriber: cfe-commits.

In C, compound literals in function scope are lvalues with
automatic storage duration. This means that generally, they
cannot be address space-qualified since you cannot have
address space-qualified objects with automatic storage duration.

We might want to check the 'alloca address space' here, but
neither ASTContext nor TargetInfo seem to have this.


Repository:
  rC Clang

https://reviews.llvm.org/D51426

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaExpr.cpp
  test/Sema/address_spaces.c


Index: test/Sema/address_spaces.c
===
--- test/Sema/address_spaces.c
+++ test/Sema/address_spaces.c
@@ -73,3 +73,17 @@
 char* cmp(_AS1 char *x,  _AS2 char *y) {
   return x < y ? x : y; // expected-error{{conditional operator with the 
second and third operands of type  ('__attribute__((address_space(1))) char *' 
and '__attribute__((address_space(2))) char *') which are pointers to 
non-overlapping address spaces}}
 }
+
+struct SomeStruct {
+  int a;
+  long b;
+  int c;
+};
+
+// Compound literals in function scope are lvalues with automatic storage 
duration,
+// so they cannot realistically be qualified with an address space.
+void as_compound_literal() {
+  (_AS1 struct SomeStruct){1, 2, 3}; // expected-error {{compound literal in 
function scope may not be qualified with an address space}}
+  (_AS1 char[]){"test"}; // expected-error {{compound literal in function 
scope may not be qualified with an address space}}
+  (_AS1 char[]){'a', 'b', 'c'}; // expected-error {{compound literal in 
function scope may not be qualified with an address space}}
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5733,12 +5733,19 @@
   LiteralExpr = Result.get();
 
   bool isFileScope = !CurContext->isFunctionOrMethod();
-  if (isFileScope &&
-  !LiteralExpr->isTypeDependent() &&
-  !LiteralExpr->isValueDependent() &&
-  !literalType->isDependentType()) { // 6.5.2.5p3
-if (CheckForConstantInitializer(LiteralExpr, literalType))
-  return ExprError();
+  if (isFileScope) {
+if(!LiteralExpr->isTypeDependent() &&
+   !LiteralExpr->isValueDependent() &&
+   !literalType->isDependentType()) // C99 6.5.2.5p3
+  if (CheckForConstantInitializer(LiteralExpr, literalType))
+return ExprError();
+  } else if(literalType.getAddressSpace() != LangAS::opencl_private &&
+literalType.getAddressSpace() != LangAS::Default) {
+// C99 6.5.2.5p6: Function scope compound literals must have automatic
+// storage which generally excludes address space-qualified ones.
+Diag(LParenLoc, diag::err_compound_literal_with_address_space)
+  << SourceRange(LParenLoc, LiteralExpr->getSourceRange().getEnd());
+return ExprError();
   }
 
   // In C, compound literals are l-values for some reason.
Index: include/clang/Basic/DiagnosticSemaKinds.td
===
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -2603,6 +2603,8 @@
   "parameter may not be qualified with an address space">;
 def err_field_with_address_space : Error<
   "field may not be qualified with an address space">;
+def err_compound_literal_with_address_space : Error<
+  "compound literal in function scope may not be qualified with an address 
space">;
 def err_attr_objc_ownership_redundant : Error<
   "the type %0 is already explicitly ownership-qualified">;
 def err_invalid_nsnumber_type : Error<


Index: test/Sema/address_spaces.c
===
--- test/Sema/address_spaces.c
+++ test/Sema/address_spaces.c
@@ -73,3 +73,17 @@
 char* cmp(_AS1 char *x,  _AS2 char *y) {
   return x < y ? x : y; // expected-error{{conditional operator with the second and third operands of type  ('__attribute__((address_space(1))) char *' and '__attribute__((address_space(2))) char *') which are pointers to non-overlapping address spaces}}
 }
+
+struct SomeStruct {
+  int a;
+  long b;
+  int c;
+};
+
+// Compound literals in function scope are lvalues with automatic storage duration,
+// so they cannot realistically be qualified with an address space.
+void as_compound_literal() {
+  (_AS1 struct SomeStruct){1, 2, 3}; // expected-error {{compound literal in function scope may not be qualified with an address space}}
+  (_AS1 char[]){"test"}; // expected-error {{compound literal in function scope may not be qualified with an address space}}
+  (_AS1 char[]){'a', 'b', 'c'}; // expected-error {{compound literal in function scope may not be qualified with an address space}}
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.

[PATCH] D51302: [OpenCL] Relax diagnostics on OpenCL access qualifiers

[Alexey Sachkov via Phabricator via cfe-commits]

AlexeySachkov updated this revision to Diff 163086.
AlexeySachkov added a comment.

Applied comment from Anastasia


https://reviews.llvm.org/D51302

Files:
  lib/Sema/SemaDeclAttr.cpp
  lib/Sema/SemaType.cpp
  test/SemaOpenCL/access-qualifier.cl

Index: lib/Sema/SemaType.cpp
===
--- lib/Sema/SemaType.cpp
+++ lib/Sema/SemaType.cpp
@@ -7083,23 +7083,43 @@
   }
 
   if (const TypedefType* TypedefTy = CurType->getAs()) {
-QualType PointeeTy = TypedefTy->desugar();
-S.Diag(Attr.getLoc(), diag::err_opencl_multiple_access_qualifiers);
+QualType BaseTy = TypedefTy->desugar();
 
 std::string PrevAccessQual;
-switch (cast(PointeeTy.getTypePtr())->getKind()) {
-  #define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
-case BuiltinType::Id:  \
-  PrevAccessQual = #Access;\
-  break;
-  #include "clang/Basic/OpenCLImageTypes.def"
-default:
-  assert(0 && "Unable to find corresponding image type.");
+if (BaseTy->isPipeType()) {
+  if (TypedefTy->getDecl()->hasAttr()) {
+OpenCLAccessAttr *Attr =
+TypedefTy->getDecl()->getAttr();
+PrevAccessQual = Attr->getSpelling();
+  } else {
+PrevAccessQual = "read_only";
+  }
+} else if (const BuiltinType* ImgType = BaseTy->getAs()) {
+
+  switch (ImgType->getKind()) {
+#define IMAGE_TYPE(ImgType, Id, SingletonId, Access, Suffix) \
+  case BuiltinType::Id:  \
+PrevAccessQual = #Access;\
+break;
+#include "clang/Basic/OpenCLImageTypes.def"
+  default:
+llvm_unreachable("Unable to find corresponding image type.");
+  }
+} else {
+  llvm_unreachable("unexpected type");
+}
+StringRef AttrName = Attr.getName()->getName();
+if (PrevAccessQual == AttrName.ltrim("_")) {
+  // Duplicated qualifiers
+  S.Diag(Attr.getLoc(), diag::warn_duplicate_declspec)
+ << AttrName << Attr.getRange();
+} else {
+  // Contradicting qualifiers
+  S.Diag(Attr.getLoc(), diag::err_opencl_multiple_access_qualifiers);
 }
 
 S.Diag(TypedefTy->getDecl()->getBeginLoc(),
-   diag::note_opencl_typedef_access_qualifier)
-<< PrevAccessQual;
+   diag::note_opencl_typedef_access_qualifier) << PrevAccessQual;
   } else if (CurType->isPipeType()) {
 if (Attr.getSemanticSpelling() == OpenCLAccessAttr::Keyword_write_only) {
   QualType ElemType = CurType->getAs()->getElementType();
Index: lib/Sema/SemaDeclAttr.cpp
===
--- lib/Sema/SemaDeclAttr.cpp
+++ lib/Sema/SemaDeclAttr.cpp
@@ -5890,10 +5890,16 @@
 
   // Check if there is only one access qualifier.
   if (D->hasAttr()) {
-S.Diag(AL.getLoc(), diag::err_opencl_multiple_access_qualifiers)
-<< D->getSourceRange();
-D->setInvalidDecl(true);
-return;
+if (D->getAttr()->getSemanticSpelling() ==
+AL.getSemanticSpelling()) {
+  S.Diag(AL.getLoc(), diag::warn_duplicate_declspec)
+  << AL.getName()->getName() << AL.getRange();
+} else {
+  S.Diag(AL.getLoc(), diag::err_opencl_multiple_access_qualifiers)
+  << D->getSourceRange();
+  D->setInvalidDecl(true);
+  return;
+}
   }
 
   // OpenCL v2.0 s6.6 - read_write can be used for image types to specify that an
Index: test/SemaOpenCL/access-qualifier.cl
===
--- test/SemaOpenCL/access-qualifier.cl
+++ test/SemaOpenCL/access-qualifier.cl
@@ -60,7 +60,7 @@
 
 kernel void k11(read_only write_only image1d_t i){} // expected-error{{multiple access qualifiers}}
 
-kernel void k12(read_only read_only image1d_t i){} // expected-error{{multiple access qualifiers}}
+kernel void k12(read_only read_only image1d_t i){} // expected-warning {{duplicate 'read_only' declaration specifier}}
 
 #if __OPENCL_C_VERSION__ >= 200
 kernel void k13(read_write pipe int i){} // expected-error{{access qualifier 'read_write' can not be used for 'read_only pipe int'}}
@@ -78,3 +78,33 @@
 #if __OPENCL_C_VERSION__ < 200
 kernel void test_image3d_wo(write_only image3d_t img) {} // expected-error {{use of type '__write_only image3d_t' requires cl_khr_3d_image_writes extension to be enabled}}
 #endif
+
+#if __OPENCL_C_VERSION__ >= 200
+kernel void read_write_twice_typedef(read_write img1d_rw i){} // expected-warning {{duplicate 'read_write' declaration specifier}}
+// expected-note@-74 {{previously declared 'read_write' here}}
+
+kernel void pipe_ro_twice(read_only read_only pipe int i){} // expected-warning{{duplicate 'read_only' declaration specifier}}
+// Conflicting access qualifiers
+kernel void pipe_ro_twice_tw(read_write read_only read_only pipe int i){} // expected-error{{access qualifier 'read_write' 

[PATCH] D51380: Fix incorrect usage of std::error_category

[Alexandre Ganea via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC340929: [Preamble] Fix incorrect usage of 
std::error_category (authored by aganea, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D51380

Files:
  lib/Frontend/PrecompiledPreamble.cpp


Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -742,8 +742,10 @@
   return nullptr;
 }
 
+static llvm::ManagedStatic 
BuildPreambleErrCategory;
+
 std::error_code clang::make_error_code(BuildPreambleError Error) {
-  return std::error_code(static_cast(Error), 
BuildPreambleErrorCategory());
+  return std::error_code(static_cast(Error), *BuildPreambleErrCategory);
 }
 
 const char *BuildPreambleErrorCategory::name() const noexcept {


Index: lib/Frontend/PrecompiledPreamble.cpp
===
--- lib/Frontend/PrecompiledPreamble.cpp
+++ lib/Frontend/PrecompiledPreamble.cpp
@@ -742,8 +742,10 @@
   return nullptr;
 }
 
+static llvm::ManagedStatic BuildPreambleErrCategory;
+
 std::error_code clang::make_error_code(BuildPreambleError Error) {
-  return std::error_code(static_cast(Error), BuildPreambleErrorCategory());
+  return std::error_code(static_cast(Error), *BuildPreambleErrCategory);
 }
 
 const char *BuildPreambleErrorCategory::name() const noexcept {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Hugo Gonzalez via Phabricator via cfe-commits]

hugoeg added a comment.

In https://reviews.llvm.org/D50542#1217517, @JonasToth wrote:

> Committed r340928. Thanks very much!
>
> Will you (and your team) upstream even more abseil checks? I think it might 
> be reasonable to ask for commit rights.


We will likely not be up streaming more in the near future. Possibly one or 
two. I am also an intern really close to being done, so it will probably not be 
worth applying for commit rights. Thank you though for all the help.


Repository:
  rL LLVM

https://reviews.llvm.org/D50542



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

[PATCH] D50542: [clang-tidy] Add abseil-no-internal-deps check

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

Ok. Then we can commit for you guys, no problem :)

Am 29.08.2018 um 16:34 schrieb Hugo Gonzalez via Phabricator:

> hugoeg added a comment.
> 
> In https://reviews.llvm.org/D50542#1217517, @JonasToth wrote:
> 
>> Committed r340928. Thanks very much!
>> 
>> Will you (and your team) upstream even more abseil checks? I think it might 
>> be reasonable to ask for commit rights.
> 
> We will likely not be up streaming more in the near future. Possibly one or 
> two. I am also an intern really close to being done, so it will probably not 
> be worth applying for commit rights. Thank you though for all the help.
> 
> Repository:
> 
>   rL LLVM
> 
> https://reviews.llvm.org/D50542


Repository:
  rL LLVM

https://reviews.llvm.org/D50542



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

r340929 - [Preamble] Fix incorrect usage of std::error_category

[Alexandre Ganea via cfe-commits]

Author: aganea
Date: Wed Aug 29 07:28:04 2018
New Revision: 340929

URL: http://llvm.org/viewvc/llvm-project?rev=340929&view=rev
Log:
[Preamble] Fix incorrect usage of std::error_category

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

Modified:
cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp

Modified: cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp?rev=340929&r1=340928&r2=340929&view=diff
==
--- cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp (original)
+++ cfe/trunk/lib/Frontend/PrecompiledPreamble.cpp Wed Aug 29 07:28:04 2018
@@ -742,8 +742,10 @@ std::unique_ptr PreambleCal
   return nullptr;
 }
 
+static llvm::ManagedStatic 
BuildPreambleErrCategory;
+
 std::error_code clang::make_error_code(BuildPreambleError Error) {
-  return std::error_code(static_cast(Error), 
BuildPreambleErrorCategory());
+  return std::error_code(static_cast(Error), *BuildPreambleErrCategory);
 }
 
 const char *BuildPreambleErrorCategory::name() const noexcept {


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

[PATCH] D51292: [docs] Update clang-rename documentation

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-tools-extra/docs/clang-rename.rst:140
+
+:program:`clangd `_ uses
+:program:`clang-rename` infrastructure to handle renaming requests. Currently,

nit: `clangd *shares* the renaming infrastructure of clang-rename...`



Comment at: clang-tools-extra/docs/clang-rename.rst:141
+:program:`clangd `_ uses
+:program:`clang-rename` infrastructure to handle renaming requests. Currently,
+it only supports renaming symbol within a single file, but in the future it 
will

ilya-biryukov wrote:
> `handle renaming requests` seems to assume some familiarity with LSP. Maybe 
> rephrase?
> Currently it only supports renaming symbol within a single file, but in the 
> future it will have much better support than the standalone tool.
This seems clangd specific and can easily get outdated when the support is 
actually added in clangd. Consider moving it to clangd's documentation if we do 
want to advertise `rename` at this point?


https://reviews.llvm.org/D51292



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

[PATCH] D51311: (WIP) Type hierarchy

[Simon Marchi via Phabricator via cfe-commits]

simark added inline comments.



Comment at: clangd/Protocol.h:891
+
+  std::vector Parents;
+

ilya-biryukov wrote:
> What is the proposal to add children (i.e. overriden methods) to the 
> hierarchy?
> This seems like a place where we might want to have multiple requests from 
> the clients when expanding the nodes.
In my prototype, I fetch the whole parent hierarchy in a single request.  In 
the proposal at

  https://github.com/Microsoft/vscode-languageserver-node/pull/346

it has been suggested to only fetch one level at the time, and have the client 
issue as many request as it wants (until possibly getting the whole hierarchy). 
 I don't know what the protocol will end up like. 



Comment at: clangd/ProtocolHandlers.cpp:70
   Register("textDocument/hover", &ProtocolCallbacks::onHover);
+  Register("textDocument/typeHierarchy", &ProtocolCallbacks::onTypeHierarchy);
   Register("textDocument/documentSymbol", 
&ProtocolCallbacks::onDocumentSymbol);

kadircet wrote:
> LSP doesn't have such an entry, maybe we should use [[ 
> https://microsoft.github.io/language-server-protocol/specification#workspace_executeCommand
>  | workspace/executeCommand ]]. WDYT @ilya-biryukov 
I don't intend to merge this patch before the protocol actually defines the way 
to get type hierarchy.  So this will be updated to reflect what the protocol 
will actually define.



Comment at: clangd/XRefs.cpp:669
+  const CXXMethodDecl *Candidate) {
+  // FIXME: How do I determine if Method overrides Candidate?
+

ilya-biryukov wrote:
> kadircet wrote:
> > ilya-biryukov wrote:
> > > simark wrote:
> > > > kadircet wrote:
> > > > > https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp
> > > > > 
> > > > > you can check this sample, which simply traverses all base classes 
> > > > > and gets methods with the same name, then checks whether one is 
> > > > > overload of the other. If it they are not overloads then one in the 
> > > > > base classes gets overriden by the other.
> > > > > 
> > > > > 
> > > > > Another approach could be to search for one method in others 
> > > > > overriden_methods.
> > > > > 
> > > > > But they are both inefficient, I would suggest calling these 
> > > > > functions only when one of them is defined in the base class of other 
> > > > > then you can just check for name equality and not being an overload.
> > > > > https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp
> > > > 
> > > > Did you mean to link to a particular line?
> > > > 
> > > > > you can check this sample, which simply traverses all base classes 
> > > > > and gets methods with the same name, then checks whether one is 
> > > > > overload of the other. If it they are not overloads then one in the 
> > > > > base classes gets overriden by the other.
> > > > 
> > > > > Another approach could be to search for one method in others 
> > > > > overriden_methods.
> > > > 
> > > > I have tried using `overriden_methods`, but it only contains methods 
> > > > marked virtual.  For this feature, I would like to consider non-virtual 
> > > > methods too.
> > > > 
> > > > > But they are both inefficient, I would suggest calling these 
> > > > > functions only when one of them is defined in the base class of other 
> > > > > then you can just check for name equality and not being an overload.
> > > > 
> > > > I am not sure I understand, but maybe it will be clearer when I will 
> > > > see the sample.
> > > See the code that actually computes a list for `override_methods()`: 
> > > Sema::AddOverriddenMethods.
> > > Did you mean to link to a particular line?
> > 
> > 
> > Yeah sorry, I was trying to link the function ilya mentioned.
> > https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp#L7615
> > 
> > Since you also mentioned checking non-virtual method as well you might 
> > wanna look at 
> > https://github.com/llvm-mirror/clang/blob/master/lib/Sema/SemaDecl.cpp#L7555
> >  as well.
> > 
> > Also I got the chance to look the rest of the patch, as I mentioned above 
> > you are already checking two methods iff one lies in the others base 
> > classes. So you can simply check for name equality and not being an 
> > overload case.
> Also wanted to bring up what @sammccall already mentioned on clangd-dev: we 
> probably shouldn't show methods that hide base methods rather than override 
> the virtual base methods (at least, by default).
> 
> Those might be useful, but unless we can have a clear UI indicator of whether 
> a method is overriding a virtual base method or is hiding some other method, 
> it would to add more confusion than benefit IMO.
> Also I got the chance to look the rest of the patch, as I mentioned above you 
> are already checking two methods iff one lies in the others base classes. So 
> you can simply check for name equality and not being an overload case.

To check for

[PATCH] D51407: [Tooling] Do not restore working dir in ClangTool

[Eric Liu via Phabricator via cfe-commits]

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

lg




Comment at: lib/Tooling/Tooling.cpp:202
 
-std::string getAbsolutePath(StringRef File) {
+static llvm::Expected getAbsolutePath(vfs::FileSystem &FS,
+   StringRef File) {

I think this is a better alternative to the original `getAbsolutePath` as a 
public interface. Maybe also expose it? 


Repository:
  rC Clang

https://reviews.llvm.org/D51407



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

[PATCH] D51429: [AArch64] Return Address Signing B Key Support

[Luke Cheeseman via Phabricator via cfe-commits]

LukeCheeseman created this revision.
LukeCheeseman added reviewers: kcc, pcc, eugenis, vlad.tsyrklevich.
Herald added a reviewer: javed.absar.
Herald added subscribers: cfe-commits, kristof.beyls.

- Add command line options for selecting the B key when enabling return address 
signing (also tidy up the msign_return_address tablegen definiton by suffixing 
the option with EQ)
- This expands the -msign-return-address= to now be 
-msign-return-address=[+] where  is one of a_key or b_key
- Generate the "sign-return-address-key" function attribute with the relevant 
key


Repository:
  rC Clang

https://reviews.llvm.org/D51429

Files:
  include/clang/Driver/Options.td
  include/clang/Frontend/CodeGenOptions.def
  include/clang/Frontend/CodeGenOptions.h
  lib/CodeGen/TargetInfo.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/aarch64-sign-return-address.c

Index: test/CodeGen/aarch64-sign-return-address.c
===
--- test/CodeGen/aarch64-sign-return-address.c
+++ test/CodeGen/aarch64-sign-return-address.c
@@ -1,6 +1,8 @@
 // RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=none  %s | FileCheck %s --check-prefix=CHECK-NONE
 // RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=non-leaf %s | FileCheck %s --check-prefix=CHECK-PARTIAL
 // RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=all %s | FileCheck %s --check-prefix=CHECK-ALL
+// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=all+a_key %s | FileCheck %s --check-prefix=CHECK-A-KEY
+// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=all+b_key %s | FileCheck %s --check-prefix=CHECK-B-KEY
 
 // CHECK-NONE: @foo() #[[ATTR:[0-9]*]]
 // CHECK-NONE-NOT: attributes #[[ATTR]] = { {{.*}} "sign-return-address"={{.*}} }}
@@ -11,4 +13,10 @@
 // CHECK-ALL: @foo() #[[ATTR:[0-9]*]]
 // CHECK-ALL: attributes #[[ATTR]] = { {{.*}} "sign-return-address"="all" {{.*}} }
 
+// CHECK-A-KEY: @foo() #[[ATTR:[0-9]*]]
+// CHECK-A-KEY: attributes #[[ATTR]] = { {{.*}} "sign-return-address"="all" "sign-return-address-key"="a_key" {{.*}} }
+
+// CHECK-B-KEY: @foo() #[[ATTR:[0-9]*]]
+// CHECK-B-KEY: attributes #[[ATTR]] = { {{.*}} "sign-return-address"="all" "sign-return-address-key"="b_key" {{.*}} }
+
 void foo() {}
Index: lib/Frontend/CompilerInvocation.cpp
===
--- lib/Frontend/CompilerInvocation.cpp
+++ lib/Frontend/CompilerInvocation.cpp
@@ -1131,8 +1131,11 @@
 
   Opts.Addrsig = Args.hasArg(OPT_faddrsig);
 
-  if (Arg *A = Args.getLastArg(OPT_msign_return_address)) {
-StringRef SignScope = A->getValue();
+  if (Arg *A = Args.getLastArg(OPT_msign_return_address_EQ)) {
+const auto SignScopeKey = StringRef(A->getValue()).split('+');
+StringRef SignScope = SignScopeKey.first;
+StringRef SignKey = SignScopeKey.second;
+
 if (SignScope.equals_lower("none"))
   Opts.setSignReturnAddress(CodeGenOptions::SignReturnAddressScope::None);
 else if (SignScope.equals_lower("all"))
@@ -1142,7 +1145,17 @@
   CodeGenOptions::SignReturnAddressScope::NonLeaf);
 else
   Diags.Report(diag::err_drv_invalid_value)
-  << A->getAsString(Args) << A->getValue();
+  << A->getAsString(Args) << SignScope;
+
+if (!SignScope.empty() && !SignKey.empty()) {
+  if (SignKey.equals_lower("a_key"))
+Opts.setSignReturnAddressKey(CodeGenOptions::SignReturnAddressKeyValue::AKey);
+  else if (SignKey.equals_lower("b_key"))
+Opts.setSignReturnAddressKey(CodeGenOptions::SignReturnAddressKeyValue::BKey);
+  else
+Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args)
+  << SignKey;
+}
   }
 
   Opts.KeepStaticConsts = Args.hasArg(OPT_fkeep_static_consts);
Index: lib/Driver/ToolChains/Clang.cpp
===
--- lib/Driver/ToolChains/Clang.cpp
+++ lib/Driver/ToolChains/Clang.cpp
@@ -1456,7 +1456,7 @@
   CmdArgs.push_back("-aarch64-enable-global-merge=true");
   }
 
-  if (Arg *A = Args.getLastArg(options::OPT_msign_return_address)) {
+  if (Arg *A = Args.getLastArg(options::OPT_msign_return_address_EQ)) {
 CmdArgs.push_back(
 Args.MakeArgString(Twine("-msign-return-address=") + A->getValue()));
   }
Index: lib/CodeGen/TargetInfo.cpp
===
--- lib/CodeGen/TargetInfo.cpp
+++ lib/CodeGen/TargetInfo.cpp
@@ -4985,6 +4985,12 @@
   Kind == CodeGenOptions::SignReturnAddressScope::All
   ? "all"
   : "non-leaf");
+
+auto Key = CGM.getCodeGenOpts().getSignReturnAddressKey();
+Fn->addFnAttr("sign-return-address-key",
+  Key == CodeGenOptions::Si

[PATCH] D51321: [Tooling] Improve handling of CL-style options

[Hamza Sood via Phabricator via cfe-commits]

hamzasood updated this revision to Diff 163098.
hamzasood added a comment.

I've addressed most of your comments and resolved the merge conflicts 
introduced in r340838.


https://reviews.llvm.org/D51321

Files:
  lib/Tooling/InterpolatingCompilationDatabase.cpp
  unittests/Tooling/CompilationDatabaseTest.cpp

Index: unittests/Tooling/CompilationDatabaseTest.cpp
===
--- unittests/Tooling/CompilationDatabaseTest.cpp
+++ unittests/Tooling/CompilationDatabaseTest.cpp
@@ -14,6 +14,8 @@
 #include "clang/Tooling/JSONCompilationDatabase.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/Support/Path.h"
+
+#define GTEST_HAS_COMBINE 1
 #include "gtest/gtest.h"
 
 namespace clang {
@@ -644,15 +646,53 @@
   }
 };
 
-class InterpolateTest : public ::testing::Test {
+enum class DriverExe { Clang, ClangCL };
+enum class DriverMode { Implicit, GCC, CL };
+
+class InterpolateTest
+: public ::testing::TestWithParam> {
+
+  StringRef getClangExe() const {
+switch (std::get<0>(GetParam())) {
+case DriverExe::Clang:   return "clang";
+case DriverExe::ClangCL: return "clang-cl";
+}
+llvm_unreachable("Invalid DriverExe");
+  }
+
+  StringRef getDriverModeFlag() const {
+switch (std::get<1>(GetParam())) {
+case DriverMode::Implicit: return StringRef();
+case DriverMode::GCC: return "--driver-mode=gcc";
+case DriverMode::CL:  return "--driver-mode=cl";
+}
+llvm_unreachable("Invalid DriverMode");
+  }
+
 protected:
+  bool isTestingCL() const {
+switch (std::get<1>(GetParam())) {
+case DriverMode::Implicit:
+  return std::get<0>(GetParam()) == DriverExe::ClangCL;
+case DriverMode::GCC: return false;
+case DriverMode::CL:  return true;
+}
+llvm_unreachable("Invalid DriverMode");
+  }
+
   // Adds an entry to the underlying compilation database.
   // A flag is injected: -D , so the command used can be identified.
   void add(llvm::StringRef File, llvm::StringRef Flags = "") {
-llvm::SmallVector Argv = {"clang", File, "-D", File};
+llvm::SmallVector Argv;
+Argv.push_back(getClangExe());
+if (!getDriverModeFlag().empty())
+  Argv.push_back(getDriverModeFlag());
+Argv.append({"-D", File});
 llvm::SplitString(Flags, Argv);
+
 llvm::SmallString<32> Dir;
 llvm::sys::path::system_temp_directory(false, Dir);
+
 Entries[path(File)].push_back(
 {Dir, path(File), {Argv.begin(), Argv.end()}, "foo.o"});
   }
@@ -675,69 +715,124 @@
 ->getCompileCommands(path(F));
 if (Results.empty())
   return "none";
-// drop the input file argument, so tests don't have to deal with path().
-EXPECT_EQ(Results[0].CommandLine.back(), path(F))
-<< "Last arg should be the file";
-Results[0].CommandLine.pop_back();
-return llvm::join(Results[0].CommandLine, " ");
+
+// To simply the test checks, we're building a new command line that doesn't
+// contain the uninteresting stuff (exe name etc.).
+ArrayRef CmdLine = Results[0].CommandLine;
+
+// Drop the clang executable path.
+EXPECT_EQ(CmdLine.front(), getClangExe()) << "First arg should be clang";
+CmdLine = CmdLine.drop_front();
+
+// Drop the --driver-mode flag.
+if (!getDriverModeFlag().empty()) {
+  EXPECT_EQ(CmdLine.front(), getDriverModeFlag())
+<< "Incorrect driver mode flag";
+  CmdLine = CmdLine.drop_front();
+}
+
+// Drop the input file.
+EXPECT_EQ(CmdLine.back(), path(F)) << "Last arg should be the file";
+CmdLine = CmdLine.drop_back();
+
+return llvm::join(CmdLine, " ");
   }
 
   MemCDB::EntryMap Entries;
 };
 
-TEST_F(InterpolateTest, Nearby) {
+TEST_P(InterpolateTest, Nearby) {
   add("dir/foo.cpp");
   add("dir/bar.cpp");
   add("an/other/foo.cpp");
 
   // great: dir and name both match (prefix or full, case insensitive)
-  EXPECT_EQ(getCommand("dir/f.cpp"), "clang -D dir/foo.cpp");
-  EXPECT_EQ(getCommand("dir/FOO.cpp"), "clang -D dir/foo.cpp");
+  EXPECT_EQ(getCommand("dir/f.cpp"), "-D dir/foo.cpp");
+  EXPECT_EQ(getCommand("dir/FOO.cpp"), "-D dir/foo.cpp");
   // no name match. prefer matching dir, break ties by alpha
-  EXPECT_EQ(getCommand("dir/a.cpp"), "clang -D dir/bar.cpp");
+  EXPECT_EQ(getCommand("dir/a.cpp"), "-D dir/bar.cpp");
   // an exact name match beats one segment of directory match
   EXPECT_EQ(getCommand("some/other/bar.h"),
-"clang -D dir/bar.cpp -x c++-header");
+isTestingCL()
+? "-D dir/bar.cpp /TP"
+: "-D dir/bar.cpp -x c++-header");
   // two segments of directory match beat a prefix name match
-  EXPECT_EQ(getCommand("an/other/b.cpp"), "clang -D an/other/foo.cpp");
+  EXPECT_EQ(getCommand("an/other/b.cpp"), "-D an/other/foo.cpp");
   // if nothing matches at all, we still get the closest alpha match
   EXPECT_EQ(getCommand("below/some/obscure/path.cpp"),
-"clang -D an/other/foo.cpp");
+"-D an/oth

[PATCH] D51321: [Tooling] Improve handling of CL-style options

[Hamza Sood via Phabricator via cfe-commits]

hamzasood marked 10 inline comments as done.
hamzasood added inline comments.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:124
+// Determine whether the given file path is the clang-cl executable.
+static bool checkIsCLMode(const std::vector &CmdLine,
+  const llvm::opt::OptTable &OptTable) {

hamzasood wrote:
> sammccall wrote:
> > nit: a two-value enum would be clearer and allow for terser variable names 
> > (`Mode`)
> The advantage of a Boolean is that it makes the checks simpler. E.g. `if 
> (isCL)` instead of `if (mode == DriverMode::CL)` or something.
> 
> Happy to change it though if you still disagree.
Also, there're more than just 2 driver modes. But we only care about cl and not 
cl.



Comment at: lib/Tooling/InterpolatingCompilationDatabase.cpp:156
+
+// Transform the command line to an llvm ArgList.
+// Make sure that OldArgs lives for at least as long as this variable as 
the

hamzasood wrote:
> sammccall wrote:
> > would you mind reverting this change and just wrapping the old Argv in an 
> > InputArgList?
> > I'm not sure the lifetime comments and std::transform aid readability here.
> The change was more about safety than readability. The old approach left an 
> InputArgList of dangling pointers after moving the new args into the cmd 
> object. This way there’s no way to accidentally access deallocated memory by 
> using the InputArgList.
> 
> As above, happy to change if you still disagree.
I've restructured it so hopefully this is less of a concern.



Comment at: unittests/Tooling/CompilationDatabaseTest.cpp:652
+
+class InterpolateTest
+  : public ::testing::TestWithParam> {

hamzasood wrote:
> sammccall wrote:
> > I'm not sure the parameterized test is the right model here.
> > The `INSTANTIATE_TEST_P` and friends makes the test fixture pretty 
> > complicated, and failure messages hard to relate to input conditions.
> > Then the actual tests have a bunch of assertions conditional on which mode 
> > we're in.
> > Finally, we don't actually test any mixed-mode database.
> > 
> > Could we write this a little more directly?:
> >  - pass the driver mode flag to `add()` in the normal way, in the Flags 
> > param
> >  - require callers to "add" to pass "clang" or "clang-cl". (It's OK that 
> > this makes existing cases a bit more verbose)
> >  - explicitly test the clang-cl and --driver-mode cases we care about, 
> > rather than running every assertion in every configuration
> > 
> > e.g.
> > ```
> > TEST_F(InterpolateTest, ClangCL) {
> >   add("foo.cc", "clang");
> >   add("bar.cc", "clang-cl");
> >   add("baz.cc", "clang --driver-mode=clang-cl");
> > 
> >   EXPECT_EQ(getCommand("a/bar.h"), "clang-cl -D a/baz.cc /TP");
> > }
> > ```
> The intent was to make sure that the existing cases (that shouldn’t depend on 
> the driver mode) don’t break, which admittedly happened while I was working 
> on the patch.
> 
> Most of the tests aren’t conditional on the mode, and I think it’s important 
> that they’re tested in all configurations. The parameterisation also lets us 
> test as `clang-cl`, `clang-cl —driver-mode=cl`, and `clang —driver-mode=cl`. 
> Which are all valid ways of using CL mode.
To clarify: the parameterisation runs the test 6 times. The `isTestingCL()` 
check narrows that down to 3.


https://reviews.llvm.org/D51321



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

[PATCH] D51382: [MinGW] Don't mark external variables as DSO local

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm




Comment at: test/CodeGen/dso-local-executable.c:14
+// MINGW-DAG: @bar = external global i32
+// MINGW-DAG: @weak_bar = extern_weak global i32
+// MINGW-DAG: declare dso_local void @foo()

I take it that was a side effect of the isDeclarationForLinker change? I think 
that's correct.


https://reviews.llvm.org/D51382



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

[PATCH] D51393: Provide a method for generating deterministic IDs for pointers allocated in BumpPtrAllocator

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added inline comments.



Comment at: llvm/include/llvm/Support/Allocator.h:292
+const char *P = reinterpret_cast(Ptr);
+for (size_t Idx=0; Idx < Slabs.size(); Idx++) {
+  const char *S = reinterpret_cast(Slabs[Idx]);

clang-format?



Comment at: llvm/include/llvm/Support/Allocator.h:295
+  if (P >= S && P < S + computeSlabSize(Idx))
+return std::make_pair(Idx, P - S);
+}

`return {Idx, P - S};`


https://reviews.llvm.org/D51393



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

[PATCH] D51393: Provide a method for generating deterministic IDs for pointers allocated in BumpPtrAllocator

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

Just for consideration: The raw pointers in dumps are sometimes useful while in 
a debugger session, because you can cast a pointer and dump the object in the 
debugger.


https://reviews.llvm.org/D51393



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

[PATCH] D51432: [AArch64] Unwinding support for return address signing

[Luke Cheeseman via Phabricator via cfe-commits]

LukeCheeseman created this revision.
LukeCheeseman added reviewers: pcc, kcc, eugenis, vlad.tsyrklevich.
Herald added a reviewer: javed.absar.
Herald added subscribers: cfe-commits, chrib, JDevlieghere, kristof.beyls.

- When return address signing is enabled, the LR may be signed on function entry
- When an exception is thrown the return address is inspected used to unwind 
the call stack
- Before this happens, the return address must be correctly authenticated to 
avoid causing an abort by dereferencing the signed pointer


Repository:
  rUNW libunwind

https://reviews.llvm.org/D51432

Files:
  include/libunwind.h
  src/DwarfInstructions.hpp
  src/DwarfParser.hpp
  src/Registers.hpp
  src/dwarf2.h

Index: src/dwarf2.h
===
--- src/dwarf2.h
+++ src/dwarf2.h
@@ -49,7 +49,10 @@
   // GNU extensions
   DW_CFA_GNU_window_save  = 0x2D,
   DW_CFA_GNU_args_size= 0x2E,
-  DW_CFA_GNU_negative_offset_extended = 0x2F
+  DW_CFA_GNU_negative_offset_extended = 0x2F,
+
+  // AARCH64 extensions
+  DW_CFA_AARCH64_negate_ra_state  = 0x2D
 };
 
 
Index: src/Registers.hpp
===
--- src/Registers.hpp
+++ src/Registers.hpp
@@ -1819,6 +1819,8 @@
 return false;
   if (regNum > 95)
 return false;
+  if (regNum == UNW_ARM64_RA_SIGN_STATE)
+return true;
   if ((regNum > 31) && (regNum < 64))
 return false;
   return true;
@@ -1829,17 +1831,18 @@
 return _registers.__pc;
   if (regNum == UNW_REG_SP)
 return _registers.__sp;
-  if ((regNum >= 0) && (regNum < 32))
+  if (((regNum >= 0) && (regNum < 32)) || regNum == UNW_ARM64_RA_SIGN_STATE)
 return _registers.__x[regNum];
+
   _LIBUNWIND_ABORT("unsupported arm64 register");
 }
 
 inline void Registers_arm64::setRegister(int regNum, uint64_t value) {
   if (regNum == UNW_REG_IP)
 _registers.__pc = value;
   else if (regNum == UNW_REG_SP)
 _registers.__sp = value;
-  else if ((regNum >= 0) && (regNum < 32))
+  else if ((regNum >= 0) && (regNum < 32) || regNum == UNW_ARM64_RA_SIGN_STATE)
 _registers.__x[regNum] = value;
   else
 _LIBUNWIND_ABORT("unsupported arm64 register");
Index: src/DwarfParser.hpp
===
--- src/DwarfParser.hpp
+++ src/DwarfParser.hpp
@@ -666,6 +666,14 @@
   _LIBUNWIND_TRACE_DWARF(
   "DW_CFA_GNU_negative_offset_extended(%" PRId64 ")\n", offset);
   break;
+
+#if defined(_LIBUNWIND_TARGET_AARCH64)
+case DW_CFA_AARCH64_negate_ra_state:
+  results->savedRegisters[UNW_ARM64_RA_SIGN_STATE].value ^= 0x1;
+  _LIBUNWIND_TRACE_DWARF("DW_CFA_AARCH64_negate_ra_state\n");
+  break;
+#endif
+
 default:
   operand = opcode & 0x3F;
   switch (opcode & 0xC0) {
Index: src/DwarfInstructions.hpp
===
--- src/DwarfInstructions.hpp
+++ src/DwarfInstructions.hpp
@@ -198,6 +198,20 @@
   // restoring SP means setting it to CFA.
   newRegisters.setSP(cfa);
 
+#if defined(_LIBUNWIND_TARGET_AARCH64)
+  // If the target is aarch64 then the return address may have been signed
+  // using the v8.3 pointer authentication extensions. The original
+  // return address needs to be authenticated before the return address is
+  // restored. autia1716 is used instead of autia as autia1716 assembles
+  // to a NOP on pre-v8.3a architectures.
+  if (prolog.savedRegisters[UNW_ARM64_RA_SIGN_STATE].value) {
+register unsigned long long x17 __asm("x17") = returnAddress;
+register unsigned long long x16 __asm("x16") = cfa;
+asm("autia1716": "+r"(x17): "r"(x16));
+returnAddress = x17;
+  }
+#endif
+
   // Return address is address after call site instruction, so setting IP to
   // that does simualates a return.
   newRegisters.setIP(returnAddress);
Index: include/libunwind.h
===
--- include/libunwind.h
+++ include/libunwind.h
@@ -547,6 +547,8 @@
   UNW_ARM64_X31 = 31,
   UNW_ARM64_SP  = 31,
   // reserved block
+  UNW_ARM64_RA_SIGN_STATE = 34,
+  // reserved block
   UNW_ARM64_D0  = 64,
   UNW_ARM64_D1  = 65,
   UNW_ARM64_D2  = 66,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51331: [OPENMP] Create non-const ident_t structs.

[Mike Rice via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC340934: [OPENMP] Create non-const ident_t objects. (authored 
by mikerice, committed by ).

Repository:
  rC Clang

https://reviews.llvm.org/D51331

Files:
  lib/CodeGen/CGOpenMPRuntime.cpp
  test/OpenMP/distribute_codegen.cpp
  test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
  test/OpenMP/distribute_parallel_for_proc_bind_codegen.cpp
  test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
  test/OpenMP/distribute_parallel_for_simd_proc_bind_codegen.cpp
  test/OpenMP/distribute_simd_codegen.cpp
  test/OpenMP/for_codegen.cpp
  test/OpenMP/for_firstprivate_codegen.cpp
  test/OpenMP/for_lastprivate_codegen.cpp
  test/OpenMP/for_linear_codegen.cpp
  test/OpenMP/for_reduction_codegen.cpp
  test/OpenMP/for_reduction_codegen_UDR.cpp
  test/OpenMP/ordered_codegen.cpp
  test/OpenMP/parallel_codegen.cpp
  test/OpenMP/parallel_copyin_codegen.cpp
  test/OpenMP/parallel_for_codegen.cpp
  test/OpenMP/parallel_num_threads_codegen.cpp
  test/OpenMP/parallel_proc_bind_codegen.cpp
  test/OpenMP/parallel_reduction_codegen.cpp
  test/OpenMP/sections_codegen.cpp
  test/OpenMP/sections_firstprivate_codegen.cpp
  test/OpenMP/sections_lastprivate_codegen.cpp
  test/OpenMP/sections_reduction_codegen.cpp
  test/OpenMP/single_codegen.cpp
  test/OpenMP/single_firstprivate_codegen.cpp
  test/OpenMP/target_parallel_codegen.cpp
  test/OpenMP/target_parallel_for_codegen.cpp
  test/OpenMP/target_parallel_for_simd_codegen.cpp
  test/OpenMP/target_parallel_if_codegen.cpp
  test/OpenMP/target_parallel_num_threads_codegen.cpp
  test/OpenMP/target_teams_codegen.cpp
  test/OpenMP/target_teams_distribute_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_proc_bind_codegen.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_proc_bind_codegen.cpp
  test/OpenMP/target_teams_distribute_simd_codegen.cpp
  test/OpenMP/target_teams_num_teams_codegen.cpp
  test/OpenMP/target_teams_thread_limit_codegen.cpp
  test/OpenMP/teams_codegen.cpp
  test/OpenMP/teams_distribute_parallel_for_num_threads_codegen.cpp
  test/OpenMP/teams_distribute_parallel_for_proc_bind_codegen.cpp
  test/OpenMP/teams_distribute_parallel_for_simd_num_threads_codegen.cpp
  test/OpenMP/teams_distribute_parallel_for_simd_proc_bind_codegen.cpp
  test/OpenMP/threadprivate_codegen.cpp

Index: test/OpenMP/target_teams_distribute_codegen.cpp
===
--- test/OpenMP/target_teams_distribute_codegen.cpp
+++ test/OpenMP/target_teams_distribute_codegen.cpp
@@ -40,7 +40,7 @@
 
 // CHECK-DAG: %struct.ident_t = type { i32, i32, i32, i32, i8* }
 // CHECK-DAG: [[STR:@.+]] = private unnamed_addr constant [23 x i8] c";unknown;unknown;0;0;;\00"
-// CHECK-DAG: [[DEF_LOC:@.+]] = private unnamed_addr constant %struct.ident_t { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* [[STR]], i32 0, i32 0) }
+// CHECK-DAG: [[DEF_LOC:@.+]] = private unnamed_addr global %struct.ident_t { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* [[STR]], i32 0, i32 0) }
 
 // CHECK-DAG: [[TT:%.+]] = type { i64, i8 }
 // CHECK-DAG: [[S1:%.+]] = type { double }
Index: test/OpenMP/parallel_num_threads_codegen.cpp
===
--- test/OpenMP/parallel_num_threads_codegen.cpp
+++ test/OpenMP/parallel_num_threads_codegen.cpp
@@ -15,7 +15,7 @@
 // CHECK-DAG: [[IDENT_T_TY:%.+]] = type { i32, i32, i32, i32, i8* }
 // CHECK-DAG: [[S_TY:%.+]] = type { [[INTPTR_T_TY:i[0-9]+]], [[INTPTR_T_TY]], [[INTPTR_T_TY]] }
 // CHECK-DAG: [[STR:@.+]] = private unnamed_addr constant [23 x i8] c";unknown;unknown;0;0;;\00"
-// CHECK-DAG: [[DEF_LOC_2:@.+]] = private unnamed_addr constant [[IDENT_T_TY]] { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* [[STR]], i32 0, i32 0) }
+// CHECK-DAG: [[DEF_LOC_2:@.+]] = private unnamed_addr global [[IDENT_T_TY]] { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* [[STR]], i32 0, i32 0) }
 
 void foo();
 
Index: test/OpenMP/target_teams_thread_limit_codegen.cpp
===
--- test/OpenMP/target_teams_thread_limit_codegen.cpp
+++ test/OpenMP/target_teams_thread_limit_codegen.cpp
@@ -40,7 +40,7 @@
 
 // CHECK-DAG: %struct.ident_t = type { i32, i32, i32, i32, i8* }
 // CHECK-DAG: [[STR:@.+]] = private unnamed_addr constant [23 x i8] c";unknown;unknown;0;0;;\00"
-// CHECK-DAG: [[DEF_LOC:@.+]] = private unnamed_addr constant %struct.ident_t { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* [[STR]], i32 0, i32 0) }
+// CHECK-DAG: [[DEF_LOC:@.+]] = private unnamed_addr global %struct.ident_t { i32 0, i32 2, i32 0, i32 0, i8* getelementptr inbounds ([23 x i8], [23 x i8]* [[STR]], i32 0, i32 0) }
 
 // CHECK-DAG: [[S1:%.+]] = type { double }
 // CHECK-DAG: [[ENTTY:%.+]] = type { i

r340934 - [OPENMP] Create non-const ident_t objects.

[Mike Rice via cfe-commits]

Author: mikerice
Date: Wed Aug 29 08:45:11 2018
New Revision: 340934

URL: http://llvm.org/viewvc/llvm-project?rev=340934&view=rev
Log:
[OPENMP] Create non-const ident_t objects.

Currently ident_t objects are created const when debug info is not
enabled, but the libittnotify libray in the OpenMP runtime writes to
the reserved_2 field (See __kmp_itt_region_forking in
openmp/runtime/src/kmp_itt.inl).  Now create ident_t objects non-const.

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

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/test/OpenMP/distribute_codegen.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_proc_bind_codegen.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
cfe/trunk/test/OpenMP/distribute_parallel_for_simd_proc_bind_codegen.cpp
cfe/trunk/test/OpenMP/distribute_simd_codegen.cpp
cfe/trunk/test/OpenMP/for_codegen.cpp
cfe/trunk/test/OpenMP/for_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/for_lastprivate_codegen.cpp
cfe/trunk/test/OpenMP/for_linear_codegen.cpp
cfe/trunk/test/OpenMP/for_reduction_codegen.cpp
cfe/trunk/test/OpenMP/for_reduction_codegen_UDR.cpp
cfe/trunk/test/OpenMP/ordered_codegen.cpp
cfe/trunk/test/OpenMP/parallel_codegen.cpp
cfe/trunk/test/OpenMP/parallel_copyin_codegen.cpp
cfe/trunk/test/OpenMP/parallel_for_codegen.cpp
cfe/trunk/test/OpenMP/parallel_num_threads_codegen.cpp
cfe/trunk/test/OpenMP/parallel_proc_bind_codegen.cpp
cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp
cfe/trunk/test/OpenMP/sections_codegen.cpp
cfe/trunk/test/OpenMP/sections_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/sections_lastprivate_codegen.cpp
cfe/trunk/test/OpenMP/sections_reduction_codegen.cpp
cfe/trunk/test/OpenMP/single_codegen.cpp
cfe/trunk/test/OpenMP/single_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_for_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_for_simd_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_if_codegen.cpp
cfe/trunk/test/OpenMP/target_parallel_num_threads_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_codegen.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_proc_bind_codegen.cpp

cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_proc_bind_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_distribute_simd_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_num_teams_codegen.cpp
cfe/trunk/test/OpenMP/target_teams_thread_limit_codegen.cpp
cfe/trunk/test/OpenMP/teams_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_num_threads_codegen.cpp
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_proc_bind_codegen.cpp

cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_num_threads_codegen.cpp

cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_proc_bind_codegen.cpp
cfe/trunk/test/OpenMP/threadprivate_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=340934&r1=340933&r2=340934&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Wed Aug 29 08:45:11 2018
@@ -1437,17 +1437,17 @@ static void buildStructValue(ConstantStr
 
 template 
 static llvm::GlobalVariable *
-createConstantGlobalStruct(CodeGenModule &CGM, QualType Ty,
-   ArrayRef Data, const Twine &Name,
-   As &&... Args) {
+createGlobalStruct(CodeGenModule &CGM, QualType Ty, bool IsConstant,
+   ArrayRef Data, const Twine &Name,
+   As &&... Args) {
   const auto *RD = cast(Ty->getAsTagDecl());
   const CGRecordLayout &RL = CGM.getTypes().getCGRecordLayout(RD);
   ConstantInitBuilder CIBuilder(CGM);
   ConstantStructBuilder Fields = CIBuilder.beginStruct(RL.getLLVMType());
   buildStructValue(Fields, CGM, RD, RL, Data);
   return Fields.finishAndCreateGlobal(
-  Name, CGM.getContext().getAlignOfGlobalVarInChars(Ty),
-  /*isConstant=*/true, std::forward(Args)...);
+  Name, CGM.getContext().getAlignOfGlobalVarInChars(Ty), IsConstant,
+  std::forward(Args)...);
 }
 
 template 
@@ -1482,8 +1482,9 @@ Address CGOpenMPRuntime::getOrCreateDefa
   llvm::ConstantInt::getNullValue(CGM.Int32Ty),
   llvm::ConstantInt::getNullValue(CGM.Int32Ty),
   DefaultOpenMPPSource};
-llvm::GlobalValue *DefaultOpenMPLocation = createConstantGlobalStruct(
-CGM, IdentQTy, Data, "", llvm::GlobalValue::PrivateLinkage);
+llvm::GlobalValue *DefaultOpenMPLo

[PATCH] D51331: [OPENMP] Create non-const ident_t structs.

[Mike Rice via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340934: [OPENMP] Create non-const ident_t objects. (authored 
by mikerice, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51331?vs=162776&id=163105#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51331

Files:
  cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
  cfe/trunk/test/OpenMP/distribute_codegen.cpp
  cfe/trunk/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
  cfe/trunk/test/OpenMP/distribute_parallel_for_proc_bind_codegen.cpp
  cfe/trunk/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
  cfe/trunk/test/OpenMP/distribute_parallel_for_simd_proc_bind_codegen.cpp
  cfe/trunk/test/OpenMP/distribute_simd_codegen.cpp
  cfe/trunk/test/OpenMP/for_codegen.cpp
  cfe/trunk/test/OpenMP/for_firstprivate_codegen.cpp
  cfe/trunk/test/OpenMP/for_lastprivate_codegen.cpp
  cfe/trunk/test/OpenMP/for_linear_codegen.cpp
  cfe/trunk/test/OpenMP/for_reduction_codegen.cpp
  cfe/trunk/test/OpenMP/for_reduction_codegen_UDR.cpp
  cfe/trunk/test/OpenMP/ordered_codegen.cpp
  cfe/trunk/test/OpenMP/parallel_codegen.cpp
  cfe/trunk/test/OpenMP/parallel_copyin_codegen.cpp
  cfe/trunk/test/OpenMP/parallel_for_codegen.cpp
  cfe/trunk/test/OpenMP/parallel_num_threads_codegen.cpp
  cfe/trunk/test/OpenMP/parallel_proc_bind_codegen.cpp
  cfe/trunk/test/OpenMP/parallel_reduction_codegen.cpp
  cfe/trunk/test/OpenMP/sections_codegen.cpp
  cfe/trunk/test/OpenMP/sections_firstprivate_codegen.cpp
  cfe/trunk/test/OpenMP/sections_lastprivate_codegen.cpp
  cfe/trunk/test/OpenMP/sections_reduction_codegen.cpp
  cfe/trunk/test/OpenMP/single_codegen.cpp
  cfe/trunk/test/OpenMP/single_firstprivate_codegen.cpp
  cfe/trunk/test/OpenMP/target_parallel_codegen.cpp
  cfe/trunk/test/OpenMP/target_parallel_for_codegen.cpp
  cfe/trunk/test/OpenMP/target_parallel_for_simd_codegen.cpp
  cfe/trunk/test/OpenMP/target_parallel_if_codegen.cpp
  cfe/trunk/test/OpenMP/target_parallel_num_threads_codegen.cpp
  cfe/trunk/test/OpenMP/target_teams_codegen.cpp
  cfe/trunk/test/OpenMP/target_teams_distribute_codegen.cpp
  
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_proc_bind_codegen.cpp
  
cfe/trunk/test/OpenMP/target_teams_distribute_parallel_for_simd_proc_bind_codegen.cpp
  cfe/trunk/test/OpenMP/target_teams_distribute_simd_codegen.cpp
  cfe/trunk/test/OpenMP/target_teams_num_teams_codegen.cpp
  cfe/trunk/test/OpenMP/target_teams_thread_limit_codegen.cpp
  cfe/trunk/test/OpenMP/teams_codegen.cpp
  cfe/trunk/test/OpenMP/teams_distribute_parallel_for_num_threads_codegen.cpp
  cfe/trunk/test/OpenMP/teams_distribute_parallel_for_proc_bind_codegen.cpp
  
cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_num_threads_codegen.cpp
  cfe/trunk/test/OpenMP/teams_distribute_parallel_for_simd_proc_bind_codegen.cpp
  cfe/trunk/test/OpenMP/threadprivate_codegen.cpp

Index: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -1437,17 +1437,17 @@
 
 template 
 static llvm::GlobalVariable *
-createConstantGlobalStruct(CodeGenModule &CGM, QualType Ty,
-   ArrayRef Data, const Twine &Name,
-   As &&... Args) {
+createGlobalStruct(CodeGenModule &CGM, QualType Ty, bool IsConstant,
+   ArrayRef Data, const Twine &Name,
+   As &&... Args) {
   const auto *RD = cast(Ty->getAsTagDecl());
   const CGRecordLayout &RL = CGM.getTypes().getCGRecordLayout(RD);
   ConstantInitBuilder CIBuilder(CGM);
   ConstantStructBuilder Fields = CIBuilder.beginStruct(RL.getLLVMType());
   buildStructValue(Fields, CGM, RD, RL, Data);
   return Fields.finishAndCreateGlobal(
-  Name, CGM.getContext().getAlignOfGlobalVarInChars(Ty),
-  /*isConstant=*/true, std::forward(Args)...);
+  Name, CGM.getContext().getAlignOfGlobalVarInChars(Ty), IsConstant,
+  std::forward(Args)...);
 }
 
 template 
@@ -1482,8 +1482,9 @@
   llvm::ConstantInt::getNullValue(CGM.Int32Ty),
   llvm::ConstantInt::getNullValue(CGM.Int32Ty),
   DefaultOpenMPPSource};
-llvm::GlobalValue *DefaultOpenMPLocation = createConstantGlobalStruct(
-CGM, IdentQTy, Data, "", llvm::GlobalValue::PrivateLinkage);
+llvm::GlobalValue *DefaultOpenMPLocation =
+createGlobalStruct(CGM, IdentQTy, /*IsConstant=*/false, Data, "",
+   llvm::GlobalValue::PrivateLinkage);
 DefaultOpenMPLocation->setUnnamedAddr(
 llvm::GlobalValue::UnnamedAddr::Global);
 
@@ -3765,8 +3766,8 @@
DeviceImages, Index),
   HostEntriesBegin, HostEntriesEnd};
   std::string Descriptor = getName({"omp_offloading", "descriptor"});
-  llvm::GlobalVariable *Desc = creat

[PATCH] D51434: [HIP] Add -amdgpu-internalize-symbols option to opt

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: tra.
Herald added subscribers: t-tye, tpr, dstuttard, wdng, kzhuravl.

AMDGPU backend needs -amdgpu-internalize-symbols option for opt to
work around a limitation of no PLT support, otherwise there is compilation
error at -O0.


https://reviews.llvm.org/D51434

Files:
  lib/Driver/ToolChains/HIP.cpp
  test/Driver/hip-toolchain.hip


Index: test/Driver/hip-toolchain.hip
===
--- test/Driver/hip-toolchain.hip
+++ test/Driver/hip-toolchain.hip
@@ -29,7 +29,7 @@
 // CHECK-SAME: "-o" [[LINKED_BC_DEV1:".*-gfx803-linked-.*bc"]]
 
 // CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV1]] "-mtriple=amdgcn-amd-amdhsa"
-// CHECK-SAME: "-mcpu=gfx803"
+// CHECK-SAME: "-mcpu=gfx803" "-amdgpu-internalize-symbols"
 // CHECK-SAME: "-o" [[OPT_BC_DEV1:".*-gfx803-optimized.*bc"]]
 
 // CHECK: [[LLC: ".*llc"]] [[OPT_BC_DEV1]] "-mtriple=amdgcn-amd-amdhsa"
@@ -55,7 +55,7 @@
 // CHECK-SAME: "-o" [[LINKED_BC_DEV2:".*-gfx900-linked-.*bc"]]
 
 // CHECK: [[OPT]] [[LINKED_BC_DEV2]] "-mtriple=amdgcn-amd-amdhsa"
-// CHECK-SAME: "-mcpu=gfx900"
+// CHECK-SAME: "-mcpu=gfx900" "-amdgpu-internalize-symbols"
 // CHECK-SAME: "-o" [[OPT_BC_DEV2:".*-gfx900-optimized.*bc"]]
 
 // CHECK: [[LLC]] [[OPT_BC_DEV2]] "-mtriple=amdgcn-amd-amdhsa"
Index: lib/Driver/ToolChains/HIP.cpp
===
--- lib/Driver/ToolChains/HIP.cpp
+++ lib/Driver/ToolChains/HIP.cpp
@@ -135,6 +135,9 @@
   }
   OptArgs.push_back("-mtriple=amdgcn-amd-amdhsa");
   OptArgs.push_back(Args.MakeArgString("-mcpu=" + SubArchName));
+  // AMDGPU backend needs this option for whole program compilation to
+  // work around the limitation of no PLT support.
+  OptArgs.push_back("-amdgpu-internalize-symbols");
   OptArgs.push_back("-o");
   std::string TmpFileName = C.getDriver().GetTemporaryPath(
   OutputFilePrefix.str() + "-optimized", "bc");


Index: test/Driver/hip-toolchain.hip
===
--- test/Driver/hip-toolchain.hip
+++ test/Driver/hip-toolchain.hip
@@ -29,7 +29,7 @@
 // CHECK-SAME: "-o" [[LINKED_BC_DEV1:".*-gfx803-linked-.*bc"]]
 
 // CHECK: [[OPT:".*opt"]] [[LINKED_BC_DEV1]] "-mtriple=amdgcn-amd-amdhsa"
-// CHECK-SAME: "-mcpu=gfx803"
+// CHECK-SAME: "-mcpu=gfx803" "-amdgpu-internalize-symbols"
 // CHECK-SAME: "-o" [[OPT_BC_DEV1:".*-gfx803-optimized.*bc"]]
 
 // CHECK: [[LLC: ".*llc"]] [[OPT_BC_DEV1]] "-mtriple=amdgcn-amd-amdhsa"
@@ -55,7 +55,7 @@
 // CHECK-SAME: "-o" [[LINKED_BC_DEV2:".*-gfx900-linked-.*bc"]]
 
 // CHECK: [[OPT]] [[LINKED_BC_DEV2]] "-mtriple=amdgcn-amd-amdhsa"
-// CHECK-SAME: "-mcpu=gfx900"
+// CHECK-SAME: "-mcpu=gfx900" "-amdgpu-internalize-symbols"
 // CHECK-SAME: "-o" [[OPT_BC_DEV2:".*-gfx900-optimized.*bc"]]
 
 // CHECK: [[LLC]] [[OPT_BC_DEV2]] "-mtriple=amdgcn-amd-amdhsa"
Index: lib/Driver/ToolChains/HIP.cpp
===
--- lib/Driver/ToolChains/HIP.cpp
+++ lib/Driver/ToolChains/HIP.cpp
@@ -135,6 +135,9 @@
   }
   OptArgs.push_back("-mtriple=amdgcn-amd-amdhsa");
   OptArgs.push_back(Args.MakeArgString("-mcpu=" + SubArchName));
+  // AMDGPU backend needs this option for whole program compilation to
+  // work around the limitation of no PLT support.
+  OptArgs.push_back("-amdgpu-internalize-symbols");
   OptArgs.push_back("-o");
   std::string TmpFileName = C.getDriver().GetTemporaryPath(
   OutputFilePrefix.str() + "-optimized", "bc");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51382: [MinGW] Don't mark external variables as DSO local

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added inline comments.



Comment at: test/CodeGen/dso-local-executable.c:14
+// MINGW-DAG: @bar = external global i32
+// MINGW-DAG: @weak_bar = extern_weak global i32
+// MINGW-DAG: declare dso_local void @foo()

rnk wrote:
> I take it that was a side effect of the isDeclarationForLinker change? I 
> think that's correct.
Yes, it changed due to that. It probably won't hurt at least.


https://reviews.llvm.org/D51382



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

[PATCH] D51436: [CodeComplete] Report location of opening parens for signature help

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: kadircet, ioeric.

Used in clangd.


Repository:
  rC Clang

https://reviews.llvm.org/D51436

Files:
  include/clang/Sema/CodeCompleteConsumer.h
  include/clang/Sema/Sema.h
  lib/Frontend/ASTUnit.cpp
  lib/Parse/ParseDecl.cpp
  lib/Parse/ParseExpr.cpp
  lib/Parse/ParseExprCXX.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Sema/CodeCompleteConsumer.cpp
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/paren_locs.cpp
  tools/libclang/CIndexCodeCompletion.cpp

Index: tools/libclang/CIndexCodeCompletion.cpp
===
--- tools/libclang/CIndexCodeCompletion.cpp
+++ tools/libclang/CIndexCodeCompletion.cpp
@@ -653,7 +653,8 @@
 
 void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
OverloadCandidate *Candidates,
-   unsigned NumCandidates) override {
+   unsigned NumCandidates,
+   SourceLocation OpenParLoc) override {
   StoredResults.reserve(StoredResults.size() + NumCandidates);
   for (unsigned I = 0; I != NumCandidates; ++I) {
 CodeCompletionString *StoredCompletion
Index: test/CodeCompletion/paren_locs.cpp
===
--- /dev/null
+++ test/CodeCompletion/paren_locs.cpp
@@ -0,0 +1,33 @@
+void foo(int a, int b);
+void foo(int a, int b, int c);
+
+void test() {
+  foo(10, );
+  // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:5:10 %s -o - \
+  // RUN: | FileCheck -check-prefix=CHECK-CC1 %s
+  // CHECK-CC1: OPENING_PAREN_LOC: {{.*}}paren_locs.cpp:5:6
+
+#define FOO foo(
+  FOO 10, );
+#undef FOO
+  // RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:11:10 %s -o - \
+  // RUN: | FileCheck -check-prefix=CHECK-CC2 %s
+  // CHECK-CC2: OPENING_PAREN_LOC: {{.*}}paren_locs.cpp:11:3
+
+  struct Foo {
+Foo(int a, int b);
+Foo(int a, int b, int c);
+  };
+  Foo a(10, );
+  // RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:21:12 %s -o - \
+  // RUN: | FileCheck -check-prefix=CHECK-CC3 %s
+  // CHECK-CC3: OPENING_PAREN_LOC: {{.*}}paren_locs.cpp:21:8
+  Foo(10, );
+  // RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:25:10 %s -o - \
+  // RUN: | FileCheck -check-prefix=CHECK-CC4 %s
+  // CHECK-CC4: OPENING_PAREN_LOC: {{.*}}paren_locs.cpp:25:6
+  new Foo(10, );
+  // RUN: not %clang_cc1 -fsyntax-only -code-completion-at=%s:29:15 %s -o - \
+  // RUN: | FileCheck -check-prefix=CHECK-CC5 %s
+  // CHECK-CC5: OPENING_PAREN_LOC: {{.*}}paren_locs.cpp:29:10
+}
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -4435,10 +4435,11 @@
   return ParamType;
 }
 
-static void CodeCompleteOverloadResults(Sema &SemaRef, Scope *S,
-MutableArrayRef Candidates,
-unsigned CurrentArg,
- bool CompleteExpressionWithCurrentArg = true) {
+static void
+CodeCompleteOverloadResults(Sema &SemaRef, Scope *S,
+MutableArrayRef Candidates,
+unsigned CurrentArg, SourceLocation OpenParLoc,
+bool CompleteExpressionWithCurrentArg = true) {
   QualType ParamType;
   if (CompleteExpressionWithCurrentArg)
 ParamType = getParamType(SemaRef, Candidates, CurrentArg);
@@ -4449,12 +4450,12 @@
 SemaRef.CodeCompleteExpression(S, ParamType);
 
   if (!Candidates.empty())
-SemaRef.CodeCompleter->ProcessOverloadCandidates(SemaRef, CurrentArg,
- Candidates.data(),
- Candidates.size());
+SemaRef.CodeCompleter->ProcessOverloadCandidates(
+SemaRef, CurrentArg, Candidates.data(), Candidates.size(), OpenParLoc);
 }
 
-void Sema::CodeCompleteCall(Scope *S, Expr *Fn, ArrayRef Args) {
+void Sema::CodeCompleteCall(Scope *S, Expr *Fn, ArrayRef Args,
+SourceLocation OpenParLoc) {
   if (!CodeCompleter)
 return;
 
@@ -4552,12 +4553,13 @@
   }
 
   mergeCandidatesWithResults(*this, Results, CandidateSet, Loc);
-  CodeCompleteOverloadResults(*this, S, Results, Args.size(),
+  CodeCompleteOverloadResults(*this, S, Results, Args.size(), OpenParLoc,
   !CandidateSet.empty());
 }
 
 void Sema::CodeCompleteConstructor(Scope *S, QualType Type, SourceLocation Loc,
-   ArrayRef Args) {
+   ArrayRef Args,
+   SourceLocation OpenParLoc) {
   if (!CodeCompleter)
 return;
 
@@ -4592,7 +4594,7 @@
 
   SmallVector Results;
   mergeCandidatesWithResults(*this, Results, CandidateSet, Loc);
-  CodeComple

[PATCH] D51437: [clangd] Report position of opening paren in singature help

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 163116.
ilya-biryukov added a comment.

- Format the code


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51437

Files:
  clangd/CodeComplete.cpp
  clangd/Protocol.h
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -825,8 +825,7 @@
 
   EXPECT_TRUE(Results.Completions.empty());
 }
-
-SignatureHelp signatures(StringRef Text,
+SignatureHelp signatures(StringRef Text, Position Point,
  std::vector IndexSymbols = {}) {
   std::unique_ptr Index;
   if (!IndexSymbols.empty())
@@ -840,9 +839,14 @@
 
   ClangdServer Server(CDB, FS, DiagConsumer, Opts);
   auto File = testPath("foo.cpp");
+  runAddDocument(Server, File, Text);
+  return cantFail(runSignatureHelp(Server, File, Point));
+}
+
+SignatureHelp signatures(StringRef Text,
+ std::vector IndexSymbols = {}) {
   Annotations Test(Text);
-  runAddDocument(Server, File, Test.code());
-  return cantFail(runSignatureHelp(Server, File, Test.point()));
+  return signatures(Test.code(), Test.point(), std::move(IndexSymbols));
 }
 
 MATCHER_P(ParamsAre, P, "") {
@@ -907,6 +911,38 @@
   EXPECT_EQ(1, Results.activeParameter);
 }
 
+TEST(SignatureHelpTest, OpeningParen) {
+  Annotations Code(R"cpp(
+int foo(int a, int b, int c);
+struct Foo {
+  Foo(int a, int b, int c);
+};
+int main() {
+  // Check recursive functions.
+  foo(foo $par1^( foo(10, 10, 10), $pos1^ )));
+  // Types.
+  Foo $par2^( 10, $pos2^ );
+  // new experssions.
+  new Foo $par3^( 10, $pos3^ );
+
+  // Macro expansions.
+#define FOO foo(
+  $par4^FOO 10, $pos4^ );
+
+  // Macro arguments.
+#define ID(X) X
+  ID(foo $par5^( foo(10), $pos5^ ))
+}
+  )cpp");
+
+  for (int Test = 1; Test <= 5; ++Test) {
+EXPECT_EQ(
+signatures(Code.code(), Code.point("pos" + itostr(Test))).argListStart,
+Code.point("par" + itostr(Test)))
+<< "for test number " << Test;
+  }
+}
+
 class IndexRequestCollector : public SymbolIndex {
 public:
   bool
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -828,6 +828,11 @@
 
   /// The active parameter of the active signature.
   int activeParameter = 0;
+
+  /// Position of the opening paren of the argument list.
+  /// This is a clangd-specific extension, it is only available via C++ API and
+  /// not currently serialized for the LSP.
+  Position argListStart;
 };
 llvm::json::Value toJSON(const SignatureHelp &);
 
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -794,7 +794,17 @@
 
   void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
  OverloadCandidate *Candidates,
- unsigned NumCandidates) override {
+ unsigned NumCandidates,
+ SourceLocation OpenParLoc) override {
+assert(!OpenParLoc.isInvalid());
+SourceManager &SrcMgr = S.getSourceManager();
+OpenParLoc = SrcMgr.getFileLoc(OpenParLoc);
+if (SrcMgr.isInMainFile(OpenParLoc))
+  SigHelp.argListStart = sourceLocToPosition(SrcMgr, OpenParLoc);
+else
+  elog("Location oustide main file in signature help: {0}",
+   OpenParLoc.printToString(SrcMgr));
+
 std::vector ScoredSignatures;
 SigHelp.signatures.reserve(NumCandidates);
 ScoredSignatures.reserve(NumCandidates);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51437: [clangd] Report position of opening paren in singature help

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added a reviewer: sammccall.
Herald added subscribers: kadircet, arphaman, jkorous, MaskRay, ioeric.
ilya-biryukov added a dependency: D51436: [CodeComplete] Report location of 
opening parens for signature help.
ilya-biryukov updated this revision to Diff 163116.
ilya-biryukov added a comment.

- Format the code


Only accessible via the C++ API at the moment.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51437

Files:
  clangd/CodeComplete.cpp
  clangd/Protocol.h
  unittests/clangd/CodeCompleteTests.cpp

Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -825,8 +825,7 @@
 
   EXPECT_TRUE(Results.Completions.empty());
 }
-
-SignatureHelp signatures(StringRef Text,
+SignatureHelp signatures(StringRef Text, Position Point,
  std::vector IndexSymbols = {}) {
   std::unique_ptr Index;
   if (!IndexSymbols.empty())
@@ -840,9 +839,14 @@
 
   ClangdServer Server(CDB, FS, DiagConsumer, Opts);
   auto File = testPath("foo.cpp");
+  runAddDocument(Server, File, Text);
+  return cantFail(runSignatureHelp(Server, File, Point));
+}
+
+SignatureHelp signatures(StringRef Text,
+ std::vector IndexSymbols = {}) {
   Annotations Test(Text);
-  runAddDocument(Server, File, Test.code());
-  return cantFail(runSignatureHelp(Server, File, Test.point()));
+  return signatures(Test.code(), Test.point(), std::move(IndexSymbols));
 }
 
 MATCHER_P(ParamsAre, P, "") {
@@ -907,6 +911,38 @@
   EXPECT_EQ(1, Results.activeParameter);
 }
 
+TEST(SignatureHelpTest, OpeningParen) {
+  Annotations Code(R"cpp(
+int foo(int a, int b, int c);
+struct Foo {
+  Foo(int a, int b, int c);
+};
+int main() {
+  // Check recursive functions.
+  foo(foo $par1^( foo(10, 10, 10), $pos1^ )));
+  // Types.
+  Foo $par2^( 10, $pos2^ );
+  // new experssions.
+  new Foo $par3^( 10, $pos3^ );
+
+  // Macro expansions.
+#define FOO foo(
+  $par4^FOO 10, $pos4^ );
+
+  // Macro arguments.
+#define ID(X) X
+  ID(foo $par5^( foo(10), $pos5^ ))
+}
+  )cpp");
+
+  for (int Test = 1; Test <= 5; ++Test) {
+EXPECT_EQ(
+signatures(Code.code(), Code.point("pos" + itostr(Test))).argListStart,
+Code.point("par" + itostr(Test)))
+<< "for test number " << Test;
+  }
+}
+
 class IndexRequestCollector : public SymbolIndex {
 public:
   bool
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -828,6 +828,11 @@
 
   /// The active parameter of the active signature.
   int activeParameter = 0;
+
+  /// Position of the opening paren of the argument list.
+  /// This is a clangd-specific extension, it is only available via C++ API and
+  /// not currently serialized for the LSP.
+  Position argListStart;
 };
 llvm::json::Value toJSON(const SignatureHelp &);
 
Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -794,7 +794,17 @@
 
   void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
  OverloadCandidate *Candidates,
- unsigned NumCandidates) override {
+ unsigned NumCandidates,
+ SourceLocation OpenParLoc) override {
+assert(!OpenParLoc.isInvalid());
+SourceManager &SrcMgr = S.getSourceManager();
+OpenParLoc = SrcMgr.getFileLoc(OpenParLoc);
+if (SrcMgr.isInMainFile(OpenParLoc))
+  SigHelp.argListStart = sourceLocToPosition(SrcMgr, OpenParLoc);
+else
+  elog("Location oustide main file in signature help: {0}",
+   OpenParLoc.printToString(SrcMgr));
+
 std::vector ScoredSignatures;
 SigHelp.signatures.reserve(NumCandidates);
 ScoredSignatures.reserve(NumCandidates);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51438: [clangd] Run SignatureHelp using an up-to-date preamble, waiting if needed.

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: ilya-biryukov.
Herald added subscribers: cfe-commits, kadircet, arphaman, jkorous, MaskRay, 
ioeric, javed.absar.

After code completion inserts a header, running signature help using the old
preamble will usually fail. So we add support for consistent preamble reads.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51438

Files:
  clangd/ClangdServer.cpp
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  unittests/clangd/TUSchedulerTests.cpp

Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -63,7 +63,7 @@
 ASSERT_FALSE(bool(AST));
 ignoreError(AST.takeError());
   });
-  S.runWithPreamble("", Missing,
+  S.runWithPreamble("", Missing, TUScheduler::Stale,
 [&](llvm::Expected Preamble) {
   ASSERT_FALSE(bool(Preamble));
   ignoreError(Preamble.takeError());
@@ -75,20 +75,22 @@
   S.runWithAST("", Added, [&](llvm::Expected AST) {
 EXPECT_TRUE(bool(AST));
   });
-  S.runWithPreamble("", Added, [&](llvm::Expected Preamble) {
-EXPECT_TRUE(bool(Preamble));
-  });
+  S.runWithPreamble("", Added, TUScheduler::Stale,
+[&](llvm::Expected Preamble) {
+  EXPECT_TRUE(bool(Preamble));
+});
   S.remove(Added);
 
   // Assert that all operations fail after removing the file.
   S.runWithAST("", Added, [&](llvm::Expected AST) {
 ASSERT_FALSE(bool(AST));
 ignoreError(AST.takeError());
   });
-  S.runWithPreamble("", Added, [&](llvm::Expected Preamble) {
-ASSERT_FALSE(bool(Preamble));
-ignoreError(Preamble.takeError());
-  });
+  S.runWithPreamble("", Added, TUScheduler::Stale,
+[&](llvm::Expected Preamble) {
+  ASSERT_FALSE(bool(Preamble));
+  ignoreError(Preamble.takeError());
+});
   // remove() shouldn't crash on missing files.
   S.remove(Added);
 }
@@ -229,7 +231,7 @@
 {
   WithContextValue WithNonce(NonceKey, ++Nonce);
   S.runWithPreamble(
-  "CheckPreamble", File,
+  "CheckPreamble", File, TUScheduler::Stale,
   [File, Inputs, Nonce, &Mut, &TotalPreambleReads](
   llvm::Expected Preamble) {
 EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
@@ -324,7 +326,7 @@
   auto WithEmptyPreamble = R"cpp(int main() {})cpp";
   S.update(Foo, getInputs(Foo, WithPreamble), WantDiagnostics::Auto,
[](std::vector) {});
-  S.runWithPreamble("getNonEmptyPreamble", Foo,
+  S.runWithPreamble("getNonEmptyPreamble", Foo, TUScheduler::Stale,
 [&](llvm::Expected Preamble) {
   // We expect to get a non-empty preamble.
   EXPECT_GT(cantFail(std::move(Preamble))
@@ -340,7 +342,7 @@
[](std::vector) {});
   // Wait for the preamble is being built.
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
-  S.runWithPreamble("getEmptyPreamble", Foo,
+  S.runWithPreamble("getEmptyPreamble", Foo, TUScheduler::Stale,
 [&](llvm::Expected Preamble) {
   // We expect to get an empty preamble.
   EXPECT_EQ(cantFail(std::move(Preamble))
@@ -372,7 +374,7 @@
[](std::vector) {});
   for (int I = 0; I < ReadsToSchedule; ++I) {
 S.runWithPreamble(
-"test", Foo,
+"test", Foo, TUScheduler::Stale,
 [I, &PreamblesMut, &Preambles](llvm::Expected IP) {
   std::lock_guard Lock(PreamblesMut);
   Preambles[I] = cantFail(std::move(IP)).Preamble;
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -119,19 +119,28 @@
   void runWithAST(llvm::StringRef Name, PathRef File,
   Callback Action);
 
-  /// Schedule an async read of the Preamble.
-  /// The preamble may be stale, generated from an older version of the file.
-  /// Reading from locations in the preamble may cause the files to be re-read.
-  /// This gives callers two options:
-  /// - validate that the preamble is still valid, and only use it in this case
-  /// - accept that preamble contents may be outdated, and try to avoid reading
-  ///   source code from headers.
+  /// Controls whether preamble reads 
+  enum PreambleConsistency {
+/// The preamble is generated from the current version of the file.
+/// If the content was recently updated, we will wait until we have a
+/// preamble that reflects that update.
+/// This is the slowest option, and may be delayed by other tasks.
+Consistent,
+/// The preamble may be generated from an older version of the file.
+/// Reading from locations in the preamble may cause files to 

[PATCH] D51438: [clangd] Run SignatureHelp using an up-to-date preamble, waiting if needed.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(This needs new TUScheduler tests for `Consistent` mode, but would like some 
feedback on the implementation first)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51438



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

[PATCH] D51438: [clangd] Run SignatureHelp using an up-to-date preamble, waiting if needed.

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 163121.
sammccall added a comment.

Finish a sentence


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51438

Files:
  clangd/ClangdServer.cpp
  clangd/TUScheduler.cpp
  clangd/TUScheduler.h
  unittests/clangd/TUSchedulerTests.cpp

Index: unittests/clangd/TUSchedulerTests.cpp
===
--- unittests/clangd/TUSchedulerTests.cpp
+++ unittests/clangd/TUSchedulerTests.cpp
@@ -63,7 +63,7 @@
 ASSERT_FALSE(bool(AST));
 ignoreError(AST.takeError());
   });
-  S.runWithPreamble("", Missing,
+  S.runWithPreamble("", Missing, TUScheduler::Stale,
 [&](llvm::Expected Preamble) {
   ASSERT_FALSE(bool(Preamble));
   ignoreError(Preamble.takeError());
@@ -75,20 +75,22 @@
   S.runWithAST("", Added, [&](llvm::Expected AST) {
 EXPECT_TRUE(bool(AST));
   });
-  S.runWithPreamble("", Added, [&](llvm::Expected Preamble) {
-EXPECT_TRUE(bool(Preamble));
-  });
+  S.runWithPreamble("", Added, TUScheduler::Stale,
+[&](llvm::Expected Preamble) {
+  EXPECT_TRUE(bool(Preamble));
+});
   S.remove(Added);
 
   // Assert that all operations fail after removing the file.
   S.runWithAST("", Added, [&](llvm::Expected AST) {
 ASSERT_FALSE(bool(AST));
 ignoreError(AST.takeError());
   });
-  S.runWithPreamble("", Added, [&](llvm::Expected Preamble) {
-ASSERT_FALSE(bool(Preamble));
-ignoreError(Preamble.takeError());
-  });
+  S.runWithPreamble("", Added, TUScheduler::Stale,
+[&](llvm::Expected Preamble) {
+  ASSERT_FALSE(bool(Preamble));
+  ignoreError(Preamble.takeError());
+});
   // remove() shouldn't crash on missing files.
   S.remove(Added);
 }
@@ -229,7 +231,7 @@
 {
   WithContextValue WithNonce(NonceKey, ++Nonce);
   S.runWithPreamble(
-  "CheckPreamble", File,
+  "CheckPreamble", File, TUScheduler::Stale,
   [File, Inputs, Nonce, &Mut, &TotalPreambleReads](
   llvm::Expected Preamble) {
 EXPECT_THAT(Context::current().get(NonceKey), Pointee(Nonce));
@@ -324,7 +326,7 @@
   auto WithEmptyPreamble = R"cpp(int main() {})cpp";
   S.update(Foo, getInputs(Foo, WithPreamble), WantDiagnostics::Auto,
[](std::vector) {});
-  S.runWithPreamble("getNonEmptyPreamble", Foo,
+  S.runWithPreamble("getNonEmptyPreamble", Foo, TUScheduler::Stale,
 [&](llvm::Expected Preamble) {
   // We expect to get a non-empty preamble.
   EXPECT_GT(cantFail(std::move(Preamble))
@@ -340,7 +342,7 @@
[](std::vector) {});
   // Wait for the preamble is being built.
   ASSERT_TRUE(S.blockUntilIdle(timeoutSeconds(10)));
-  S.runWithPreamble("getEmptyPreamble", Foo,
+  S.runWithPreamble("getEmptyPreamble", Foo, TUScheduler::Stale,
 [&](llvm::Expected Preamble) {
   // We expect to get an empty preamble.
   EXPECT_EQ(cantFail(std::move(Preamble))
@@ -372,7 +374,7 @@
[](std::vector) {});
   for (int I = 0; I < ReadsToSchedule; ++I) {
 S.runWithPreamble(
-"test", Foo,
+"test", Foo, TUScheduler::Stale,
 [I, &PreamblesMut, &Preambles](llvm::Expected IP) {
   std::lock_guard Lock(PreamblesMut);
   Preambles[I] = cantFail(std::move(IP)).Preamble;
Index: clangd/TUScheduler.h
===
--- clangd/TUScheduler.h
+++ clangd/TUScheduler.h
@@ -119,19 +119,28 @@
   void runWithAST(llvm::StringRef Name, PathRef File,
   Callback Action);
 
-  /// Schedule an async read of the Preamble.
-  /// The preamble may be stale, generated from an older version of the file.
-  /// Reading from locations in the preamble may cause the files to be re-read.
-  /// This gives callers two options:
-  /// - validate that the preamble is still valid, and only use it in this case
-  /// - accept that preamble contents may be outdated, and try to avoid reading
-  ///   source code from headers.
+  /// Controls whether preamble reads wait for the preamble to be up-to-date.
+  enum PreambleConsistency {
+/// The preamble is generated from the current version of the file.
+/// If the content was recently updated, we will wait until we have a
+/// preamble that reflects that update.
+/// This is the slowest option, and may be delayed by other tasks.
+Consistent,
+/// The preamble may be generated from an older version of the file.
+/// Reading from locations in the preamble may cause files to be re-read.
+/// This gives callers two options:
+/// - validate that the preamble is still valid, and only use it if so
+/// - accept that the preamble contents may be outdated, and tr

[PATCH] D51407: [Tooling] Do not restore working dir in ClangTool

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 163122.
ilya-biryukov added a comment.

- Expose getAbsolutePath in the public API


Repository:
  rC Clang

https://reviews.llvm.org/D51407

Files:
  include/clang/Tooling/Tooling.h
  lib/Tooling/Tooling.cpp

Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -199,21 +199,25 @@
FileName, ToolName);
 }
 
-std::string getAbsolutePath(StringRef File) {
+llvm::Expected getAbsolutePath(vfs::FileSystem &FS,
+StringRef File) {
   StringRef RelativePath(File);
   // FIXME: Should '.\\' be accepted on Win32?
   if (RelativePath.startswith("./")) {
 RelativePath = RelativePath.substr(strlen("./"));
   }
 
   SmallString<1024> AbsolutePath = RelativePath;
-  std::error_code EC = llvm::sys::fs::make_absolute(AbsolutePath);
-  assert(!EC);
-  (void)EC;
+  if (auto EC = FS.makeAbsolute(AbsolutePath))
+return llvm::errorCodeToError(EC);
   llvm::sys::path::native(AbsolutePath);
   return AbsolutePath.str();
 }
 
+std::string getAbsolutePath(StringRef File) {
+  return llvm::cantFail(getAbsolutePath(*vfs::getRealFileSystem(), File));
+}
+
 void addTargetAndModeForProgramName(std::vector &CommandLine,
 StringRef InvokedAs) {
   if (!CommandLine.empty() && !InvokedAs.empty()) {
@@ -411,15 +415,6 @@
   // This just needs to be some symbol in the binary.
   static int StaticSymbol;
 
-  std::string InitialDirectory;
-  if (llvm::ErrorOr CWD =
-  OverlayFileSystem->getCurrentWorkingDirectory()) {
-InitialDirectory = std::move(*CWD);
-  } else {
-llvm::report_fatal_error("Cannot detect current path: " +
- Twine(CWD.getError().message()));
-  }
-
   // First insert all absolute paths into the in-memory VFS. These are global
   // for all compile commands.
   if (SeenWorkingDirectories.insert("/").second)
@@ -431,9 +426,22 @@
 
   bool ProcessingFailed = false;
   bool FileSkipped = false;
+  // Compute all absolute paths before we run any actions, as those will change
+  // the working directory.
+  std::vector AbsolutePaths;
+  AbsolutePaths.reserve(SourcePaths.size());
   for (const auto &SourcePath : SourcePaths) {
-std::string File(getAbsolutePath(SourcePath));
+auto AbsPath = getAbsolutePath(*OverlayFileSystem, SourcePath);
+if (!AbsPath) {
+  llvm::errs() << "Skipping " << SourcePath
+   << ". Error while getting an absolute path: "
+   << llvm::toString(AbsPath.takeError()) << "\n";
+  continue;
+}
+AbsolutePaths.push_back(std::move(*AbsPath));
+  }
 
+  for (llvm::StringRef File : AbsolutePaths) {
 // Currently implementations of CompilationDatabase::getCompileCommands can
 // change the state of the file system (e.g.  prepare generated headers), so
 // this method needs to run right before we invoke the tool, as the next
@@ -498,11 +506,6 @@
 llvm::errs() << "Error while processing " << File << ".\n";
 ProcessingFailed = true;
   }
-  // Return to the initial directory to correctly resolve next file by
-  // relative path.
-  if (OverlayFileSystem->setCurrentWorkingDirectory(InitialDirectory.c_str()))
-llvm::report_fatal_error("Cannot chdir into \"" +
- Twine(InitialDirectory) + "\n!");
 }
   }
   return ProcessingFailed ? 1 : (FileSkipped ? 2 : 0);
Index: include/clang/Tooling/Tooling.h
===
--- include/clang/Tooling/Tooling.h
+++ include/clang/Tooling/Tooling.h
@@ -459,6 +459,10 @@
 /// \param File Either an absolute or relative path.
 std::string getAbsolutePath(StringRef File);
 
+/// An overload of getAbsolutePath that works over the provided \p FS.
+llvm::Expected getAbsolutePath(vfs::FileSystem &FS,
+StringRef File);
+
 /// Changes CommandLine to contain implicit flags that would have been
 /// defined had the compiler driver been invoked through the path InvokedAs.
 ///
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r340937 - [Tooling] Do not restore working dir in ClangTool

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Wed Aug 29 09:35:31 2018
New Revision: 340937

URL: http://llvm.org/viewvc/llvm-project?rev=340937&view=rev
Log:
[Tooling] Do not restore working dir in ClangTool

Summary:
Resolve all relative paths before running the tool instead.

This fixes the usage of ClangTool in AllTUsExecutor. The executor will
try running multiple ClangTool instances in parallel with compile
commands that usually have the same working directory.

Changing working directory is a global operation, so we end up
changing working directory in the middle of running other actions,
which leads to spurious compile errors.

Reviewers: ioeric, sammccall

Reviewed By: ioeric

Subscribers: cfe-commits

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

Modified:
cfe/trunk/include/clang/Tooling/Tooling.h
cfe/trunk/lib/Tooling/Tooling.cpp

Modified: cfe/trunk/include/clang/Tooling/Tooling.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Tooling/Tooling.h?rev=340937&r1=340936&r2=340937&view=diff
==
--- cfe/trunk/include/clang/Tooling/Tooling.h (original)
+++ cfe/trunk/include/clang/Tooling/Tooling.h Wed Aug 29 09:35:31 2018
@@ -459,6 +459,10 @@ inline std::unique_ptr getAbsolutePath(vfs::FileSystem &FS,
+StringRef File);
+
 /// Changes CommandLine to contain implicit flags that would have been
 /// defined had the compiler driver been invoked through the path InvokedAs.
 ///

Modified: cfe/trunk/lib/Tooling/Tooling.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Tooling/Tooling.cpp?rev=340937&r1=340936&r2=340937&view=diff
==
--- cfe/trunk/lib/Tooling/Tooling.cpp (original)
+++ cfe/trunk/lib/Tooling/Tooling.cpp Wed Aug 29 09:35:31 2018
@@ -199,7 +199,8 @@ bool runToolOnCodeWithArgs(
FileName, ToolName);
 }
 
-std::string getAbsolutePath(StringRef File) {
+llvm::Expected getAbsolutePath(vfs::FileSystem &FS,
+StringRef File) {
   StringRef RelativePath(File);
   // FIXME: Should '.\\' be accepted on Win32?
   if (RelativePath.startswith("./")) {
@@ -207,13 +208,16 @@ std::string getAbsolutePath(StringRef Fi
   }
 
   SmallString<1024> AbsolutePath = RelativePath;
-  std::error_code EC = llvm::sys::fs::make_absolute(AbsolutePath);
-  assert(!EC);
-  (void)EC;
+  if (auto EC = FS.makeAbsolute(AbsolutePath))
+return llvm::errorCodeToError(EC);
   llvm::sys::path::native(AbsolutePath);
   return AbsolutePath.str();
 }
 
+std::string getAbsolutePath(StringRef File) {
+  return llvm::cantFail(getAbsolutePath(*vfs::getRealFileSystem(), File));
+}
+
 void addTargetAndModeForProgramName(std::vector &CommandLine,
 StringRef InvokedAs) {
   if (!CommandLine.empty() && !InvokedAs.empty()) {
@@ -411,15 +415,6 @@ int ClangTool::run(ToolAction *Action) {
   // This just needs to be some symbol in the binary.
   static int StaticSymbol;
 
-  std::string InitialDirectory;
-  if (llvm::ErrorOr CWD =
-  OverlayFileSystem->getCurrentWorkingDirectory()) {
-InitialDirectory = std::move(*CWD);
-  } else {
-llvm::report_fatal_error("Cannot detect current path: " +
- Twine(CWD.getError().message()));
-  }
-
   // First insert all absolute paths into the in-memory VFS. These are global
   // for all compile commands.
   if (SeenWorkingDirectories.insert("/").second)
@@ -431,9 +426,22 @@ int ClangTool::run(ToolAction *Action) {
 
   bool ProcessingFailed = false;
   bool FileSkipped = false;
+  // Compute all absolute paths before we run any actions, as those will change
+  // the working directory.
+  std::vector AbsolutePaths;
+  AbsolutePaths.reserve(SourcePaths.size());
   for (const auto &SourcePath : SourcePaths) {
-std::string File(getAbsolutePath(SourcePath));
+auto AbsPath = getAbsolutePath(*OverlayFileSystem, SourcePath);
+if (!AbsPath) {
+  llvm::errs() << "Skipping " << SourcePath
+   << ". Error while getting an absolute path: "
+   << llvm::toString(AbsPath.takeError()) << "\n";
+  continue;
+}
+AbsolutePaths.push_back(std::move(*AbsPath));
+  }
 
+  for (llvm::StringRef File : AbsolutePaths) {
 // Currently implementations of CompilationDatabase::getCompileCommands can
 // change the state of the file system (e.g.  prepare generated headers), 
so
 // this method needs to run right before we invoke the tool, as the next
@@ -498,11 +506,6 @@ int ClangTool::run(ToolAction *Action) {
 llvm::errs() << "Error while processing " << File << ".\n";
 ProcessingFailed = true;
   }
-  // Return to the initial directory to correctly resolve next file by
-  // relative path.
-  if 
(OverlayFileSystem->setCurrentWorkingDirectory(InitialDirectory.c_str()))
-

[PATCH] D51407: [Tooling] Do not restore working dir in ClangTool

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL340937: [Tooling] Do not restore working dir in ClangTool 
(authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D51407

Files:
  cfe/trunk/include/clang/Tooling/Tooling.h
  cfe/trunk/lib/Tooling/Tooling.cpp

Index: cfe/trunk/include/clang/Tooling/Tooling.h
===
--- cfe/trunk/include/clang/Tooling/Tooling.h
+++ cfe/trunk/include/clang/Tooling/Tooling.h
@@ -459,6 +459,10 @@
 /// \param File Either an absolute or relative path.
 std::string getAbsolutePath(StringRef File);
 
+/// An overload of getAbsolutePath that works over the provided \p FS.
+llvm::Expected getAbsolutePath(vfs::FileSystem &FS,
+StringRef File);
+
 /// Changes CommandLine to contain implicit flags that would have been
 /// defined had the compiler driver been invoked through the path InvokedAs.
 ///
Index: cfe/trunk/lib/Tooling/Tooling.cpp
===
--- cfe/trunk/lib/Tooling/Tooling.cpp
+++ cfe/trunk/lib/Tooling/Tooling.cpp
@@ -199,21 +199,25 @@
FileName, ToolName);
 }
 
-std::string getAbsolutePath(StringRef File) {
+llvm::Expected getAbsolutePath(vfs::FileSystem &FS,
+StringRef File) {
   StringRef RelativePath(File);
   // FIXME: Should '.\\' be accepted on Win32?
   if (RelativePath.startswith("./")) {
 RelativePath = RelativePath.substr(strlen("./"));
   }
 
   SmallString<1024> AbsolutePath = RelativePath;
-  std::error_code EC = llvm::sys::fs::make_absolute(AbsolutePath);
-  assert(!EC);
-  (void)EC;
+  if (auto EC = FS.makeAbsolute(AbsolutePath))
+return llvm::errorCodeToError(EC);
   llvm::sys::path::native(AbsolutePath);
   return AbsolutePath.str();
 }
 
+std::string getAbsolutePath(StringRef File) {
+  return llvm::cantFail(getAbsolutePath(*vfs::getRealFileSystem(), File));
+}
+
 void addTargetAndModeForProgramName(std::vector &CommandLine,
 StringRef InvokedAs) {
   if (!CommandLine.empty() && !InvokedAs.empty()) {
@@ -411,15 +415,6 @@
   // This just needs to be some symbol in the binary.
   static int StaticSymbol;
 
-  std::string InitialDirectory;
-  if (llvm::ErrorOr CWD =
-  OverlayFileSystem->getCurrentWorkingDirectory()) {
-InitialDirectory = std::move(*CWD);
-  } else {
-llvm::report_fatal_error("Cannot detect current path: " +
- Twine(CWD.getError().message()));
-  }
-
   // First insert all absolute paths into the in-memory VFS. These are global
   // for all compile commands.
   if (SeenWorkingDirectories.insert("/").second)
@@ -431,9 +426,22 @@
 
   bool ProcessingFailed = false;
   bool FileSkipped = false;
+  // Compute all absolute paths before we run any actions, as those will change
+  // the working directory.
+  std::vector AbsolutePaths;
+  AbsolutePaths.reserve(SourcePaths.size());
   for (const auto &SourcePath : SourcePaths) {
-std::string File(getAbsolutePath(SourcePath));
+auto AbsPath = getAbsolutePath(*OverlayFileSystem, SourcePath);
+if (!AbsPath) {
+  llvm::errs() << "Skipping " << SourcePath
+   << ". Error while getting an absolute path: "
+   << llvm::toString(AbsPath.takeError()) << "\n";
+  continue;
+}
+AbsolutePaths.push_back(std::move(*AbsPath));
+  }
 
+  for (llvm::StringRef File : AbsolutePaths) {
 // Currently implementations of CompilationDatabase::getCompileCommands can
 // change the state of the file system (e.g.  prepare generated headers), so
 // this method needs to run right before we invoke the tool, as the next
@@ -498,11 +506,6 @@
 llvm::errs() << "Error while processing " << File << ".\n";
 ProcessingFailed = true;
   }
-  // Return to the initial directory to correctly resolve next file by
-  // relative path.
-  if (OverlayFileSystem->setCurrentWorkingDirectory(InitialDirectory.c_str()))
-llvm::report_fatal_error("Cannot chdir into \"" +
- Twine(InitialDirectory) + "\n!");
 }
   }
   return ProcessingFailed ? 1 : (FileSkipped ? 2 : 0);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51407: [Tooling] Do not restore working dir in ClangTool

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC340937: [Tooling] Do not restore working dir in ClangTool 
(authored by ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51407?vs=163122&id=163127#toc

Repository:
  rC Clang

https://reviews.llvm.org/D51407

Files:
  include/clang/Tooling/Tooling.h
  lib/Tooling/Tooling.cpp

Index: include/clang/Tooling/Tooling.h
===
--- include/clang/Tooling/Tooling.h
+++ include/clang/Tooling/Tooling.h
@@ -459,6 +459,10 @@
 /// \param File Either an absolute or relative path.
 std::string getAbsolutePath(StringRef File);
 
+/// An overload of getAbsolutePath that works over the provided \p FS.
+llvm::Expected getAbsolutePath(vfs::FileSystem &FS,
+StringRef File);
+
 /// Changes CommandLine to contain implicit flags that would have been
 /// defined had the compiler driver been invoked through the path InvokedAs.
 ///
Index: lib/Tooling/Tooling.cpp
===
--- lib/Tooling/Tooling.cpp
+++ lib/Tooling/Tooling.cpp
@@ -199,21 +199,25 @@
FileName, ToolName);
 }
 
-std::string getAbsolutePath(StringRef File) {
+llvm::Expected getAbsolutePath(vfs::FileSystem &FS,
+StringRef File) {
   StringRef RelativePath(File);
   // FIXME: Should '.\\' be accepted on Win32?
   if (RelativePath.startswith("./")) {
 RelativePath = RelativePath.substr(strlen("./"));
   }
 
   SmallString<1024> AbsolutePath = RelativePath;
-  std::error_code EC = llvm::sys::fs::make_absolute(AbsolutePath);
-  assert(!EC);
-  (void)EC;
+  if (auto EC = FS.makeAbsolute(AbsolutePath))
+return llvm::errorCodeToError(EC);
   llvm::sys::path::native(AbsolutePath);
   return AbsolutePath.str();
 }
 
+std::string getAbsolutePath(StringRef File) {
+  return llvm::cantFail(getAbsolutePath(*vfs::getRealFileSystem(), File));
+}
+
 void addTargetAndModeForProgramName(std::vector &CommandLine,
 StringRef InvokedAs) {
   if (!CommandLine.empty() && !InvokedAs.empty()) {
@@ -411,15 +415,6 @@
   // This just needs to be some symbol in the binary.
   static int StaticSymbol;
 
-  std::string InitialDirectory;
-  if (llvm::ErrorOr CWD =
-  OverlayFileSystem->getCurrentWorkingDirectory()) {
-InitialDirectory = std::move(*CWD);
-  } else {
-llvm::report_fatal_error("Cannot detect current path: " +
- Twine(CWD.getError().message()));
-  }
-
   // First insert all absolute paths into the in-memory VFS. These are global
   // for all compile commands.
   if (SeenWorkingDirectories.insert("/").second)
@@ -431,9 +426,22 @@
 
   bool ProcessingFailed = false;
   bool FileSkipped = false;
+  // Compute all absolute paths before we run any actions, as those will change
+  // the working directory.
+  std::vector AbsolutePaths;
+  AbsolutePaths.reserve(SourcePaths.size());
   for (const auto &SourcePath : SourcePaths) {
-std::string File(getAbsolutePath(SourcePath));
+auto AbsPath = getAbsolutePath(*OverlayFileSystem, SourcePath);
+if (!AbsPath) {
+  llvm::errs() << "Skipping " << SourcePath
+   << ". Error while getting an absolute path: "
+   << llvm::toString(AbsPath.takeError()) << "\n";
+  continue;
+}
+AbsolutePaths.push_back(std::move(*AbsPath));
+  }
 
+  for (llvm::StringRef File : AbsolutePaths) {
 // Currently implementations of CompilationDatabase::getCompileCommands can
 // change the state of the file system (e.g.  prepare generated headers), so
 // this method needs to run right before we invoke the tool, as the next
@@ -498,11 +506,6 @@
 llvm::errs() << "Error while processing " << File << ".\n";
 ProcessingFailed = true;
   }
-  // Return to the initial directory to correctly resolve next file by
-  // relative path.
-  if (OverlayFileSystem->setCurrentWorkingDirectory(InitialDirectory.c_str()))
-llvm::report_fatal_error("Cannot chdir into \"" +
- Twine(InitialDirectory) + "\n!");
 }
   }
   return ProcessingFailed ? 1 : (FileSkipped ? 2 : 0);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51084: Implement -Watomic-implicit-seq-cst

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In https://reviews.llvm.org/D51084#1216913, @rjmccall wrote:

> It says the type of the assignment expression, not the type of the LHS.
>
> C11 [6.5.16]p2: "The type of an assignment expression is the type the left 
> operand would have after lvalue conversion."
>
> C11 [6.3.2.1]p2: "...this is called lvalue conversion. If the lvalue has 
> qualified type, the value has the unqualified version of the type of the 
> lvalue; additionally, if the lvalue has atomic type, the value has the 
> non-atomic version of the type of the lvalue; otherwise, the value has the 
> type of the lvalue."
>
> The RHS is not converted to have atomic type.


Right that would be nonsensical because you'd need to load it atomically. The C 
`_Atomic` semantics match what C++ `std::atomic` does:

  _Atomic(int) atom;
  void ass(int i) {
  atom = i;
  }

is the same as:

  #include 
  std::atomic atom;
  void ass(int i) {
  atom = i;
  }

They're meant to be interchangeable.

This warning is meant to warn on C's implicit `seq_cst`. When I get to writing 
the C++ version of the warning I'll warn about C++'s implicit `seq_cst`.


Repository:
  rC Clang

https://reviews.llvm.org/D51084



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

[PATCH] D51434: [HIP] Add -amdgpu-internalize-symbols option to opt

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

Could you elaborate on what exactly is the problem this patch fixes?
I don't see how internalizing the symbols connects to PLTs. My understanding is 
that PLTs are used to provide stubs for symbols to be resolved by dynamic 
linker at runtime. AFAICT AMD does not use shared libs on device side. What do 
I miss?


https://reviews.llvm.org/D51434



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

[PATCH] D51359: Adding HardLink Support to VirtualFileSystem.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 updated this revision to Diff 163128.
usaxena95 added a comment.

  Created InMemoryHardLink Class which can handle the resolved InMemoryFile


Repository:
  rC Clang

https://reviews.llvm.org/D51359

Files:
  include/clang/Basic/VirtualFileSystem.h
  lib/Basic/VirtualFileSystem.cpp
  unittests/Basic/VirtualFileSystemTest.cpp

Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -17,6 +17,7 @@
 #include "llvm/Support/SourceMgr.h"
 #include "gtest/gtest.h"
 #include 
+#include 
 
 using namespace clang;
 using namespace llvm;
@@ -695,6 +696,22 @@
   InMemoryFileSystemTest()
   : FS(/*UseNormalizedPaths=*/false),
 NormalizedFS(/*UseNormalizedPaths=*/true) {}
+
+public:
+  void ExpectHardLink(Twine From, Twine To, const string& ExpectedBuffer) {
+auto OpenedFrom = FS.openFileForRead(From);
+ASSERT_FALSE(OpenedFrom.getError());
+auto OpenedTo = FS.openFileForRead(To);
+ASSERT_FALSE(OpenedTo.getError());
+ASSERT_EQ((*OpenedFrom)->status()->getSize(),
+  (*OpenedTo)->status()->getSize());
+ASSERT_EQ((*OpenedFrom)->status()->getUniqueID(),
+  (*OpenedTo)->status()->getUniqueID());
+ASSERT_EQ((*OpenedFrom)->getBuffer(From)->get()->getBuffer().data(),
+  (*OpenedTo)->getBuffer(To)->get()->getBuffer().data());
+ASSERT_EQ((*OpenedFrom)->getBuffer(From)->get()->getBuffer().data(),
+  ExpectedBuffer);
+  }
 };
 
 TEST_F(InMemoryFileSystemTest, IsEmpty) {
@@ -958,6 +975,84 @@
   ASSERT_EQ("../b/c", getPosixPath(It->getName()));
 }
 
+TEST_F(InMemoryFileSystemTest, AddHardLinkToFile) {
+  std::pair From = {"/path/to/FROM/file",
+"content of FROM file"};
+  std::pair To = {"/path/to/TO/file",
+  "content of TO file"};
+  FS.addFile(To.first, 0, MemoryBuffer::getMemBuffer(To.second));
+  EXPECT_TRUE(FS.addHardLink(From.first, To.first));
+  ExpectHardLink(From.first, To.first, To.second.data());
+}
+
+TEST_F(InMemoryFileSystemTest, AddHardLinkInChainPattern) {
+  StringRef content = "content of target file";
+  Twine link0 = "/path/to/0/link";
+  Twine link1 = "/path/to/1/link";
+  Twine link2 = "/path/to/2/link";
+  Twine target = "/path/to/target";
+  FS.addFile(target, 0, MemoryBuffer::getMemBuffer(content));
+  EXPECT_TRUE(FS.addHardLink(link2, target));
+  EXPECT_TRUE(FS.addHardLink(link1, link2));
+  EXPECT_TRUE(FS.addHardLink(link0, link1));
+  ExpectHardLink(link0, target, content.data());
+  ExpectHardLink(link1, target, content.data());
+  ExpectHardLink(link2, target, content.data());
+}
+
+TEST_F(InMemoryFileSystemTest, AddHardLinkToAFileThatWasNotAddedBefore) {
+  Twine link = "/path/to/link";
+  Twine target = "/path/to/target";
+  EXPECT_FALSE(FS.addHardLink(link, target));
+}
+
+TEST_F(InMemoryFileSystemTest, AddHardLinkFromAFileThatWasAddedBefore) {
+  Twine link = "/path/to/link";
+  StringRef content_link = "content of link";
+  Twine target = "/path/to/target";
+  StringRef content = "content of target";
+  FS.addFile(target, 0, MemoryBuffer::getMemBuffer(content));
+  FS.addFile(link, 0, MemoryBuffer::getMemBuffer(content_link));
+  EXPECT_FALSE(FS.addHardLink(link, target));
+}
+
+TEST_F(InMemoryFileSystemTest, AddSameHardLinkMoreThanOnce) {
+  Twine link = "/path/to/link";
+  Twine target = "/path/to/target";
+  StringRef content = "content of target";
+  FS.addFile(target, 0, MemoryBuffer::getMemBuffer(content));
+  EXPECT_TRUE(FS.addHardLink(link, target));
+  EXPECT_FALSE(FS.addHardLink(link, target));
+}
+
+TEST_F(InMemoryFileSystemTest, AddFileInPlaceOfAHardLinkWithSameContent) {
+  Twine link = "/path/to/link";
+  Twine target = "/path/to/target";
+  StringRef content = "content of target";
+  EXPECT_TRUE(FS.addFile(target, 0, MemoryBuffer::getMemBuffer(content)));
+  EXPECT_TRUE(FS.addHardLink(link, target));
+  EXPECT_TRUE(FS.addFile(link, 0, MemoryBuffer::getMemBuffer(content)));
+}
+
+TEST_F(InMemoryFileSystemTest, AddFileInPlaceOfAHardLinkWithDifferentContent) {
+  Twine link = "/path/to/link";
+  Twine target = "/path/to/target";
+  StringRef content = "content of target";
+  StringRef link_content = "different content of link";
+  EXPECT_TRUE(FS.addFile(target, 0, MemoryBuffer::getMemBuffer(content)));
+  EXPECT_TRUE(FS.addHardLink(link, target));
+  EXPECT_FALSE(FS.addFile(link, 0, MemoryBuffer::getMemBuffer(link_content)));
+}
+
+TEST_F(InMemoryFileSystemTest, AddHardLinkToADirectory) {
+  Twine dir = "path/to/dummy/dir";
+  Twine link = "/path/to/link";
+  Twine dummy_file = dir + "/target";
+  StringRef content = "content of target";
+  EXPECT_TRUE(FS.addFile(dummy_file, 0, MemoryBuffer::getMemBuffer(content)));
+  EXPECT_FALSE(FS.addHardLink(link, dir));
+}
+
 // NOTE: in the tests below, we use '//root/' as our root directory, since it is
 // 

[PATCH] D51426: [Sema] Prohibit function-scope compound literals with address spaces.

[John McCall via Phabricator via cfe-commits]

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

LGTM, with one minor suggestion.




Comment at: lib/Sema/SemaExpr.cpp:5745
+// C99 6.5.2.5p6: Function scope compound literals must have automatic
+// storage which generally excludes address space-qualified ones.
+Diag(LParenLoc, diag::err_compound_literal_with_address_space)

Usually when we mention a standard section like this, it's a prelude to a 
quote.  If you're just paraphrasing, I think we can trust people to find the 
right standard section.


Repository:
  rC Clang

https://reviews.llvm.org/D51426



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

[PATCH] D51440: [ToolChains] Link to compiler-rt with -L + -l when possible

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: manojgupta, chandlerc, beanz.
Herald added subscribers: aheejin, sbc100, dberris, srhines.
Herald added a reviewer: javed.absar.

This avoids a libtool issue 
(https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27866), where libtool fails to 
pick up the default linked libraries when they are referred to via a direct 
path to the static library, instead of as a -L + -l pair, as for libgcc.

Or is there some reason not to add the clang_rt dir to the linker path with -L?


Repository:
  rC Clang

https://reviews.llvm.org/D51440

Files:
  lib/Driver/ToolChains/CommonArgs.cpp
  test/Driver/arm-compiler-rt.c
  test/Driver/fuchsia.c
  test/Driver/fuchsia.cpp
  test/Driver/linux-ld.c
  test/Driver/wasm-toolchain.c
  test/Driver/wasm-toolchain.cpp

Index: test/Driver/wasm-toolchain.cpp
===
--- test/Driver/wasm-toolchain.cpp
+++ test/Driver/wasm-toolchain.cpp
@@ -14,10 +14,10 @@
 
 // RUN: %clangxx -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo --stdlib=c++ -fuse-ld=wasm-ld %s 2>&1 | FileCheck -check-prefix=LINK %s
 // LINK: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+// LINK: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "-lclang_rt.builtins-wasm32" "-o" "a.out"
 
 // A basic C++ link command-line with optimization.
 
 // RUN: %clangxx -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo %s --stdlib=c++ -fuse-ld=wasm-ld 2>&1 | FileCheck -check-prefix=LINK_OPT %s
 // LINK_OPT: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_OPT: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+// LINK_OPT: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc++" "-lc++abi" "-lc" "-lclang_rt.builtins-wasm32" "-o" "a.out"
Index: test/Driver/wasm-toolchain.c
===
--- test/Driver/wasm-toolchain.c
+++ test/Driver/wasm-toolchain.c
@@ -14,10 +14,10 @@
 
 // RUN: %clang -### -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo -fuse-ld=wasm-ld %s 2>&1 | FileCheck -check-prefix=LINK %s
 // LINK: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+// LINK: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc" "-lclang_rt.builtins-wasm32" "-o" "a.out"
 
 // A basic C link command-line with optimization.
 
 // RUN: %clang -### -O2 -no-canonical-prefixes -target wasm32-unknown-unknown --sysroot=/foo -fuse-ld=wasm-ld %s 2>&1 | FileCheck -check-prefix=LINK_OPT %s
 // LINK_OPT: clang{{.*}}" "-cc1" {{.*}} "-o" "[[temp:[^"]*]]"
-// LINK_OPT: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out"
+// LINK_OPT: wasm-ld{{.*}}" "-L/foo/lib" "crt1.o" "[[temp]]" "-lc" "-lclang_rt.builtins-wasm32" "-o" "a.out"
Index: test/Driver/linux-ld.c
===
--- test/Driver/linux-ld.c
+++ test/Driver/linux-ld.c
@@ -66,9 +66,9 @@
 // CHECK-LD-RT: "-L[[SYSROOT]]/usr/lib/gcc/x86_64-unknown-linux/4.6.0/../../.."
 // CHECK-LD-RT: "-L[[SYSROOT]]/lib"
 // CHECK-LD-RT: "-L[[SYSROOT]]/usr/lib"
-// CHECK-LD-RT: libclang_rt.builtins-x86_64.a"
+// CHECK-LD-RT: "-lclang_rt.builtins-x86_64"
 // CHECK-LD-RT: "-lc"
-// CHECK-LD-RT: libclang_rt.builtins-x86_64.a"
+// CHECK-LD-RT: "-lclang_rt.builtins-x86_64"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=i686-unknown-linux \
@@ -87,9 +87,9 @@
 // CHECK-LD-RT-I686: "-L[[SYSROOT]]/usr/lib/gcc/i686-unknown-linux/4.6.0/../../.."
 // CHECK-LD-RT-I686: "-L[[SYSROOT]]/lib"
 // CHECK-LD-RT-I686: "-L[[SYSROOT]]/usr/lib"
-// CHECK-LD-RT-I686: libclang_rt.builtins-i386.a"
+// CHECK-LD-RT-I686: "-lclang_rt.builtins-i386"
 // CHECK-LD-RT-I686: "-lc"
-// CHECK-LD-RT-I686: libclang_rt.builtins-i386.a"
+// CHECK-LD-RT-I686: "-lclang_rt.builtins-i386"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=arm-linux-androideabi \
@@ -102,9 +102,9 @@
 // CHECK-LD-RT-ANDROID: "--eh-frame-hdr"
 // CHECK-LD-RT-ANDROID: "-m" "armelf_linux_eabi"
 // CHECK-LD-RT-ANDROID: "-dynamic-linker"
-// CHECK-LD-RT-ANDROID: libclang_rt.builtins-arm-android.a"
+// CHECK-LD-RT-ANDROID: "-lclang_rt.builtins-arm-android"
 // CHECK-LD-RT-ANDROID: "-lc"
-// CHECK-LD-RT-ANDROID: libclang_rt.builtins-arm-android.a"
+// CHECK-LD-RT-ANDROID: "-lclang_rt.builtins-arm-android"
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
Index: test/Driver/fuchsia.cpp

[PATCH] D51359: Adding HardLink Support to VirtualFileSystem.

[UTKARSH SAXENA via Phabricator via cfe-commits]

usaxena95 marked 8 inline comments as done.
usaxena95 added a comment.

Applied the suggested changes. Added InMemoryHardLink as a subclass of 
InMemoryNode.  Modified the tests accordingly.


Repository:
  rC Clang

https://reviews.llvm.org/D51359



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

[PATCH] D51441: Add predefined macro __gnu_linux__ for proper aux-triple

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl created this revision.
yaxunl added a reviewer: tra.
Herald added a subscriber: krytarowski.

Clang predefine macro `__linx__` for aux-triple with Linux OS
but does not predefine macro `__gnu_linux__`. This causes
some compilation error for certain applications, e.g. Eigen.

This patch fixes that.


https://reviews.llvm.org/D51441

Files:
  lib/Frontend/InitPreprocessor.cpp
  test/Preprocessor/predefined-macros.c


Index: test/Preprocessor/predefined-macros.c
===
--- test/Preprocessor/predefined-macros.c
+++ test/Preprocessor/predefined-macros.c
@@ -183,9 +183,11 @@
 // CHECK-HIP: #define __HIP__ 1
 
 // RUN: %clang_cc1 %s -E -dM -o - -x hip -triple amdgcn-amd-amdhsa \
-// RUN:   -fcuda-is-device \
+// RUN:   -aux-triple x86_64-unknown-linux-gnu -fcuda-is-device \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-HIP-DEV
 // CHECK-HIP-DEV-NOT: #define __CUDA_ARCH__
 // CHECK-HIP-DEV: #define __HIPCC__ 1
 // CHECK-HIP-DEV: #define __HIP_DEVICE_COMPILE__ 1
 // CHECK-HIP-DEV: #define __HIP__ 1
+// CHECK_HIP-DEV: #define __linux__ 1
+// CHECK_HIP-DEV: #define __gnu_linux__ 1
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -1123,6 +1123,8 @@
   if (AuxTriple.getOS() == llvm::Triple::Linux) {
 Builder.defineMacro("__ELF__");
 Builder.defineMacro("__linux__");
+if (AuxTriple.getEnvironment() == llvm::Triple::GNU)
+  Builder.defineMacro("__gnu_linux__");
 // Used in features.h. If this is omitted, math.h doesn't declare float
 // versions of the functions in bits/mathcalls.h.
 if (LangOpts.CPlusPlus)


Index: test/Preprocessor/predefined-macros.c
===
--- test/Preprocessor/predefined-macros.c
+++ test/Preprocessor/predefined-macros.c
@@ -183,9 +183,11 @@
 // CHECK-HIP: #define __HIP__ 1
 
 // RUN: %clang_cc1 %s -E -dM -o - -x hip -triple amdgcn-amd-amdhsa \
-// RUN:   -fcuda-is-device \
+// RUN:   -aux-triple x86_64-unknown-linux-gnu -fcuda-is-device \
 // RUN:   | FileCheck -match-full-lines %s --check-prefix=CHECK-HIP-DEV
 // CHECK-HIP-DEV-NOT: #define __CUDA_ARCH__
 // CHECK-HIP-DEV: #define __HIPCC__ 1
 // CHECK-HIP-DEV: #define __HIP_DEVICE_COMPILE__ 1
 // CHECK-HIP-DEV: #define __HIP__ 1
+// CHECK_HIP-DEV: #define __linux__ 1
+// CHECK_HIP-DEV: #define __gnu_linux__ 1
Index: lib/Frontend/InitPreprocessor.cpp
===
--- lib/Frontend/InitPreprocessor.cpp
+++ lib/Frontend/InitPreprocessor.cpp
@@ -1123,6 +1123,8 @@
   if (AuxTriple.getOS() == llvm::Triple::Linux) {
 Builder.defineMacro("__ELF__");
 Builder.defineMacro("__linux__");
+if (AuxTriple.getEnvironment() == llvm::Triple::GNU)
+  Builder.defineMacro("__gnu_linux__");
 // Used in features.h. If this is omitted, math.h doesn't declare float
 // versions of the functions in bits/mathcalls.h.
 if (LangOpts.CPlusPlus)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51434: [HIP] Add -amdgpu-internalize-symbols option to opt

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In https://reviews.llvm.org/D51434#1217772, @tra wrote:

> Could you elaborate on what exactly is the problem this patch fixes?
>  I don't see how internalizing the symbols connects to PLTs. My understanding 
> is that PLTs are used to provide stubs for symbols to be resolved by dynamic 
> linker at runtime. AFAICT AMD does not use shared libs on device side. What 
> do I miss?


When AMDGPU backend generates ELF containing device ISA's, any non-kernel 
functions require
PLT support to be emitted to ELF. However AMDGPU backend currently does not 
support that,
which causes error.

Internalization will eliminate any non-kernel functions, therefore works around 
this limitation.


https://reviews.llvm.org/D51434



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

[PATCH] D51440: [ToolChains] Link to compiler-rt with -L + -l when possible

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a reviewer: phosek.
beanz added a subscriber: phosek.
beanz added a comment.

Looping in @phosek.


Repository:
  rC Clang

https://reviews.llvm.org/D51440



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

[PATCH] D51333: Diagnose likely typos in include statements

[Christy Lee via Phabricator via cfe-commits]

christylee updated this revision to Diff 163132.
christylee edited the summary of this revision.
christylee added a comment.

Merged warning with existing file_not_found_error.


https://reviews.llvm.org/D51333

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Lex/PPDirectives.cpp
  lib/Lex/Pragma.cpp
  test/Preprocessor/include-likely-typo.c


Index: test/Preprocessor/include-likely-typo.c
===
--- /dev/null
+++ test/Preprocessor/include-likely-typo.c
@@ -0,0 +1,2 @@
+// RUN: not %clang_cc1 -verify -frewrite-includes
+#include "" @expected-error {{'' file not found, possibly 
due to leading or trailing non-alphanumeric characters in the file name}}
Index: lib/Lex/Pragma.cpp
===
--- lib/Lex/Pragma.cpp
+++ lib/Lex/Pragma.cpp
@@ -514,7 +514,7 @@
  nullptr, CurDir, nullptr, nullptr, nullptr, nullptr);
   if (!File) {
 if (!SuppressIncludeNotFoundError)
-  Diag(FilenameTok, diag::err_pp_file_not_found) << Filename;
+  Diag(FilenameTok, diag::err_pp_file_not_found) << Filename << 0;
 return;
   }
 
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1859,6 +1859,7 @@
   // If the file could not be located and it was included via angle
   // brackets, we can attempt a lookup as though it were a quoted path to
   // provide the user with a possible fixit.
+  bool isFileNotFoundLikelyTypo = false;
   if (isAngled) {
 File = LookupFile(
 FilenameLoc,
@@ -1868,16 +1869,27 @@
 Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped);
 if (File) {
   SourceRange Range(FilenameTok.getLocation(), CharEnd);
-  Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal) <<
-Filename <<
-FixItHint::CreateReplacement(Range, "\"" + Filename.str() + "\"");
+  Diag(FilenameTok, diag::err_pp_file_not_found_not_fatal)
+  << Filename
+  << FixItHint::CreateReplacement(Range,
+  "\"" + Filename.str() + "\"")
+  << isFileNotFoundLikelyTypo;
 }
   }
 
   // If the file is still not found, just go with the vanilla diagnostic
-  if (!File)
-Diag(FilenameTok, diag::err_pp_file_not_found) << Filename
-   << FilenameRange;
+  if (!File) {
+// Assuming filename logically starts and end with alphnumeric
+// character
+if (!isAlphanumeric(Filename.front()) ||
+!isAlphanumeric(Filename.back())) {
+  isFileNotFoundLikelyTypo = true;
+  Diag(FilenameTok, diag::err_pp_file_not_found)
+  << Filename << isFileNotFoundLikelyTypo;
+}
+Diag(FilenameTok, diag::err_pp_file_not_found)
+<< Filename << FilenameRange << isFileNotFoundLikelyTypo;
+  }
 }
   }
 
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -403,7 +403,9 @@
 def err_pp_invalid_directive : Error<"invalid preprocessing directive">;
 def err_pp_directive_required : Error<
   "%0 must be used within a preprocessing directive">;
-def err_pp_file_not_found : Error<"'%0' file not found">, DefaultFatal;
+def err_pp_file_not_found : Error<
+  "'%0' file not found%select{|, possibly due to leading or trailing "
+  "non-alphanumeric characters in the file name}1">, DefaultFatal;
 def err_pp_through_header_not_found : Error<
   "'%0' required for precompiled header not found">, DefaultFatal;
 def err_pp_through_header_not_seen : Error<


Index: test/Preprocessor/include-likely-typo.c
===
--- /dev/null
+++ test/Preprocessor/include-likely-typo.c
@@ -0,0 +1,2 @@
+// RUN: not %clang_cc1 -verify -frewrite-includes
+#include "" @expected-error {{'' file not found, possibly due to leading or trailing non-alphanumeric characters in the file name}}
Index: lib/Lex/Pragma.cpp
===
--- lib/Lex/Pragma.cpp
+++ lib/Lex/Pragma.cpp
@@ -514,7 +514,7 @@
  nullptr, CurDir, nullptr, nullptr, nullptr, nullptr);
   if (!File) {
 if (!SuppressIncludeNotFoundError)
-  Diag(FilenameTok, diag::err_pp_file_not_found) << Filename;
+  Diag(FilenameTok, diag::err_pp_file_not_found) << Filename << 0;
 return;
   }
 
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1859,6 +1859,7 @@
   // If the file could not be located and it was included via angle
   

[PATCH] D51434: [HIP] Add -amdgpu-internalize-symbols option to opt

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

I could not find anything about PLTs in AMDGPU-ABI 
,
 nor could I find anything relevant on google.
I still have no idea why PLTs are required in this case. Without that info, the 
problem may as well be due to unintended requirement for PLT that this patch 
would hide.

I'm going to defer to someone more familiar with amdgpu to tell whether that's 
the right fix for the problem.


https://reviews.llvm.org/D51434



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