[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.

Yes, you are write.

I would ensure in the script that `CHECK-MESSAGES XOR CHECK-NOTES` is
used, to avoid the mistake i made. I can do it in this diff as well.

Am 29.08.2018 um 20:29 schrieb Roman Lebedev via Phabricator:

> lebedev.ri added a comment.
> 
> In https://reviews.llvm.org/D51381#1217047, @JonasToth wrote:
> 
>> @lebedev.ri lets do it in the the other patch, to not split discussions.
> 
> Let's do it here instead, since that differential requires some changes to 
> this script.
> 
> In https://reviews.llvm.org/D51381#1217047, @JonasToth wrote:
> 
>> But what do you mean by `You would still have to duplicate the check-lines 
>> for error: though.`?
> 
> In it's current state, `CHECK-MESSAGES` implies 
> `-implicit-check-not={{warning|error}}`,
>  and `CHECK-NOTES` will imply `-implicit-check-not={{note|error}}`.
>  I.e. they both imply `-implicit-check-not=error`.
>  So **if** the check produces any `error:` output, **and** you want to use 
> **both** the `CHECK-NOTES` and `CHECK-MESSAGES`,
>  you need to write the check-line for every `error: ` with both the 
> `CHECK-NOTES` and `CHECK-MESSAGES` prefixes. 
>  This seems sub-optimal.
> 
> In https://reviews.llvm.org/D48714#1217044, @JonasToth wrote:
> 
>> 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.
> 
> I don't have a //strong// preference, but if that works i would **prefer** to 
> go that route, since that is what the initial intended semantics of the 
> `CHECK-NOTES`.
>  **If** that works for you?
> 
> Repository:
> 
>   rCTE Clang Tools Extra
> 
> https://reviews.llvm.org/D51381


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] D51381: [clang-tidy] fix check_clang_tidy to properly mix CHECK-NOTES and CHECK-MESSAGES

[Jonas Toth via Phabricator via cfe-commits]

JonasToth updated this revision to Diff 163273.
JonasToth added a comment.

- adjust check_clang_tidy to use either CHECK-NOTES or CHECK-MESSAGES but not 
both


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51381

Files:
  test/clang-tidy/check_clang_tidy.py


Index: test/clang-tidy/check_clang_tidy.py
===
--- test/clang-tidy/check_clang_tidy.py
+++ test/clang-tidy/check_clang_tidy.py
@@ -98,6 +98,9 @@
 sys.exit('%s, %s or %s not found in the input' % (check_fixes_prefix,
  check_messages_prefix, check_notes_prefix) )
 
+  if has_check_notes and has_check_messages:
+sys.exit('Please use either CHECK-NOTES or CHECK-MESSAGES but not both')
+
   # Remove the contents of the CHECK lines to avoid CHECKs matching on
   # themselves.  We need to keep the comments to preserve line numbers while
   # avoiding empty lines which could potentially trigger formatting-related


Index: test/clang-tidy/check_clang_tidy.py
===
--- test/clang-tidy/check_clang_tidy.py
+++ test/clang-tidy/check_clang_tidy.py
@@ -98,6 +98,9 @@
 sys.exit('%s, %s or %s not found in the input' % (check_fixes_prefix,
  check_messages_prefix, check_notes_prefix) )
 
+  if has_check_notes and has_check_messages:
+sys.exit('Please use either CHECK-NOTES or CHECK-MESSAGES but not both')
+
   # Remove the contents of the CHECK lines to avoid CHECKs matching on
   # themselves.  We need to keep the comments to preserve line numbers while
   # avoiding empty lines which could potentially trigger formatting-related
___
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 updated this revision to Diff 163274.
JonasToth added a comment.

- [Misc] migrate to CHECK-NOTES
- fix outcommented check messages as well


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D48714

Files:
  clang-tidy/hicpp/ExceptionBaseclassCheck.cpp
  test/clang-tidy/hicpp-exception-baseclass.cpp

Index: test/clang-tidy/hicpp-exception-baseclass.cpp
===
--- test/clang-tidy/hicpp-exception-baseclass.cpp
+++ test/clang-tidy/hicpp-exception-baseclass.cpp
@@ -2,6 +2,7 @@
 
 namespace std {
 class exception {};
+class invalid_argument : public exception {};
 } // namespace std
 
 class derived_exception : public std::exception {};
@@ -36,38 +37,38 @@
   try {
 throw non_derived_exception();
 // CHECK-NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
-// CHECK-NOTES: 9:1: note: type defined here
+// CHECK-NOTES: 10:1: note: type defined here
   } catch (non_derived_exception &e) {
   }
   throw non_derived_exception();
   // CHECK-NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'non_derived_exception' is not derived from 'std::exception'
-  // CHECK-NOTES: 9:1: note: type defined here
+  // CHECK-NOTES: 10:1: note: type defined here
 
 // FIXME: More complicated kinds of inheritance should be checked later, but there is
 // currently no way use ASTMatchers for this kind of task.
 #if 0
   // Handle private inheritance cases correctly.
   try {
 throw bad_inheritance();
-// CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
-// CHECK MESSAGES: 10:1: note: type defined here
+// CHECK NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
+// CHECK NOTES: 11:1: note: type defined here
 throw no_good_inheritance();
-// CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
-// CHECK MESSAGES: 11:1: note: type defined here
+// CHECK NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
+// CHECK NOTES: 12:1: note: type defined here
 throw really_creative();
-// CHECK MESSAGES: [[@LINE-1]]:11: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
-// CHECK MESSAGES: 12:1: note: type defined here
+// CHECK NOTES: [[@LINE-1]]:11: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
+// CHECK NOTES: 13:1: note: type defined here
   } catch (...) {
   }
   throw bad_inheritance();
-  // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
-  // CHECK MESSAGES: 10:1: note: type defined here
+  // CHECK NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'bad_inheritance' is not derived from 'std::exception'
+  // CHECK NOTES: 11:1: note: type defined here
   throw no_good_inheritance();
-  // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
-  // CHECK MESSAGES: 11:1: note: type defined here
+  // CHECK NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'no_good_inheritance' is not derived from 'std::exception'
+  // CHECK NOTES: 12:1: note: type defined here
   throw really_creative();
-  // CHECK MESSAGES: [[@LINE-1]]:9: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
-  // CHECK MESSAGES: 12:1: note: type defined here
+  // CHECK NOTES: [[@LINE-1]]:9: warning: throwing an exception whose type 'really_creative' is not derived from 'std::exception'
+  // CHECK NOTES: 13:1: note: type defined here
 #endif
 }
 
@@ -91,7 +92,7 @@
   throw deep_hierarchy(); // Ok
 
   try {
-throw terrible_idea(); // Ok, but multiple inheritance isn't clean
+throw terrible_idea();  // Ok, but multiple inheritance isn't clean
   } catch (std::exception &e) { // Can be caught as std::exception, even with multiple inheritance
   }
   throw terrible_idea(); // Ok, but multiple inheritance
@@ -101,14 +102,22 @@
 template 
 void ThrowException() { throw T(); }
 // CHECK-NOTES: [[@LINE-1]]:31: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception'
-// CHECK-NOTES: 120:1: note: type defined here
-// CHECK-NOTES: [[@LINE-3]]:31: warning: throwing an exception whose type 'bad_generic_exception' is not derived from 'std::exception'
-// CHECK-NOTES: 120:1: note: type defined here
-// CHECK-NOTES: [[@LINE-5]]:31: warning: throwing an exception whose type 'exotic_exception' is not derived from 'std::exception'
-// CHECK-NOTES: 123:1: note: t

[PATCH] D51381: [clang-tidy] fix check_clang_tidy to forbid mixing of CHECK-NOTES and CHECK-MESSAGES

[Roman Lebedev via Phabricator via cfe-commits]

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

LG, thank you!


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] D51475: [clangd] Load YAML static index asyncrhonously.

[Eric Liu via Phabricator via cfe-commits]

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

Loading static index can be slow (>10s for LLVM index). This allows
the clangd server to be initialized before index is loaded.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51475

Files:
  clangd/tool/CMakeLists.txt
  clangd/tool/ClangdMain.cpp

Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -14,6 +14,8 @@
 #include "index/SymbolYAML.h"
 #include "index/dex/DexIndex.h"
 #include "clang/Basic/Version.h"
+#include "clang/Basic/Version.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
@@ -39,22 +41,80 @@
 
 enum class PCHStorageFlag { Disk, Memory };
 
-// Build an in-memory static index for global symbols from a YAML-format file.
-// The size of global symbols should be relatively small, so that all symbols
-// can be managed in memory.
-std::unique_ptr buildStaticIndex(llvm::StringRef YamlSymbolFile) {
-  auto Buffer = llvm::MemoryBuffer::getFile(YamlSymbolFile);
-  if (!Buffer) {
-llvm::errs() << "Can't open " << YamlSymbolFile << "\n";
-return nullptr;
+// Loads the index asynchornously. This acts like an empty index before
+// finishing loading and proxies index requests to the loaded index after
+// loading.
+class AsyncLoadIndex : public SymbolIndex {
+public:
+  AsyncLoadIndex(
+  llvm::unique_function()> LoadIndex)
+  : AsyncLoad(runAsync(std::move(LoadIndex))) {}
+
+  bool
+  fuzzyFind(const FuzzyFindRequest &Req,
+llvm::function_ref Callback) const override {
+if (!index())
+  return false; // More
+return index()->fuzzyFind(Req, Callback);
   }
-  auto Slab = symbolsFromYAML(Buffer.get()->getBuffer());
-  SymbolSlab::Builder SymsBuilder;
-  for (auto Sym : Slab)
-SymsBuilder.insert(Sym);
 
-  return UseDex ? dex::DexIndex::build(std::move(SymsBuilder).build())
-: MemIndex::build(std::move(SymsBuilder).build());
+  void
+  lookup(const LookupRequest &Req,
+ llvm::function_ref Callback) const override {
+if (!index())
+  return;
+return index()->lookup(Req, Callback);
+  }
+
+  void findOccurrences(const OccurrencesRequest &Req,
+   llvm::function_ref
+   Callback) const override {
+if (!index())
+  return;
+return index()->findOccurrences(Req, Callback);
+  }
+
+  size_t estimateMemoryUsage() const override { return 0; }
+
+private:
+  const SymbolIndex *index() const {
+if (Index)
+  return Index.get();
+if (AsyncLoad.wait_for(std::chrono::seconds(0)) !=
+std::future_status::ready)
+  return nullptr;
+Index = AsyncLoad.get();
+return Index.get();
+  }
+  mutable std::unique_ptr Index;
+  mutable std::future> AsyncLoad;
+};
+// Asynchronously build an in-memory static index for global symbols from a
+// YAML-format file. The size of global symbols should be relatively small, so
+// that all symbols can be managed in memory.
+std::unique_ptr buildStaticIndex(llvm::StringRef YamlSymbolFile) {
+  return llvm::make_unique(
+  [YamlSymbolFile]() -> std::unique_ptr {
+trace::Span Tracer("Build static index");
+
+auto Buffer = llvm::MemoryBuffer::getFile(YamlSymbolFile);
+if (!Buffer) {
+  llvm::errs() << "Can't open " << YamlSymbolFile << "\n";
+  return nullptr;
+}
+
+SymbolSlab::Builder SymsBuilder;
+{
+  trace::Span Tracer("YAML to symbols");
+  auto Slab = symbolsFromYAML(Buffer.get()->getBuffer());
+  for (auto Sym : Slab)
+SymsBuilder.insert(Sym);
+}
+
+trace::Span Build("Build index");
+return UseDex ? dex::DexIndex::build(std::move(SymsBuilder).build())
+  : MemIndex::build(std::move(SymsBuilder).build());
+  });
 }
 
 } // namespace
Index: clangd/tool/CMakeLists.txt
===
--- clangd/tool/CMakeLists.txt
+++ clangd/tool/CMakeLists.txt
@@ -1,4 +1,4 @@
-include_directories(${CMAKE_CURRENT_SOURCE_DIR}/..)
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../)
 
 add_clang_tool(clangd
   ClangdMain.cpp
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51475: [clangd] Load YAML static index asynchronously.

[Eric Liu via Phabricator via cfe-commits]

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

- revert unintended change


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51475

Files:
  clangd/tool/ClangdMain.cpp

Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -14,6 +14,8 @@
 #include "index/SymbolYAML.h"
 #include "index/dex/DexIndex.h"
 #include "clang/Basic/Version.h"
+#include "clang/Basic/Version.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
@@ -39,22 +41,80 @@
 
 enum class PCHStorageFlag { Disk, Memory };
 
-// Build an in-memory static index for global symbols from a YAML-format file.
-// The size of global symbols should be relatively small, so that all symbols
-// can be managed in memory.
-std::unique_ptr buildStaticIndex(llvm::StringRef YamlSymbolFile) {
-  auto Buffer = llvm::MemoryBuffer::getFile(YamlSymbolFile);
-  if (!Buffer) {
-llvm::errs() << "Can't open " << YamlSymbolFile << "\n";
-return nullptr;
+// Loads the index asynchornously. This acts like an empty index before
+// finishing loading and proxies index requests to the loaded index after
+// loading.
+class AsyncLoadIndex : public SymbolIndex {
+public:
+  AsyncLoadIndex(
+  llvm::unique_function()> LoadIndex)
+  : AsyncLoad(runAsync(std::move(LoadIndex))) {}
+
+  bool
+  fuzzyFind(const FuzzyFindRequest &Req,
+llvm::function_ref Callback) const override {
+if (!index())
+  return false; // More
+return index()->fuzzyFind(Req, Callback);
   }
-  auto Slab = symbolsFromYAML(Buffer.get()->getBuffer());
-  SymbolSlab::Builder SymsBuilder;
-  for (auto Sym : Slab)
-SymsBuilder.insert(Sym);
 
-  return UseDex ? dex::DexIndex::build(std::move(SymsBuilder).build())
-: MemIndex::build(std::move(SymsBuilder).build());
+  void
+  lookup(const LookupRequest &Req,
+ llvm::function_ref Callback) const override {
+if (!index())
+  return;
+return index()->lookup(Req, Callback);
+  }
+
+  void findOccurrences(const OccurrencesRequest &Req,
+   llvm::function_ref
+   Callback) const override {
+if (!index())
+  return;
+return index()->findOccurrences(Req, Callback);
+  }
+
+  size_t estimateMemoryUsage() const override { return 0; }
+
+private:
+  const SymbolIndex *index() const {
+if (Index)
+  return Index.get();
+if (AsyncLoad.wait_for(std::chrono::seconds(0)) !=
+std::future_status::ready)
+  return nullptr;
+Index = AsyncLoad.get();
+return Index.get();
+  }
+  mutable std::unique_ptr Index;
+  mutable std::future> AsyncLoad;
+};
+// Asynchronously build an in-memory static index for global symbols from a
+// YAML-format file. The size of global symbols should be relatively small, so
+// that all symbols can be managed in memory.
+std::unique_ptr buildStaticIndex(llvm::StringRef YamlSymbolFile) {
+  return llvm::make_unique(
+  [YamlSymbolFile]() -> std::unique_ptr {
+trace::Span Tracer("Build static index");
+
+auto Buffer = llvm::MemoryBuffer::getFile(YamlSymbolFile);
+if (!Buffer) {
+  llvm::errs() << "Can't open " << YamlSymbolFile << "\n";
+  return nullptr;
+}
+
+SymbolSlab::Builder SymsBuilder;
+{
+  trace::Span Tracer("YAML to symbols");
+  auto Slab = symbolsFromYAML(Buffer.get()->getBuffer());
+  for (auto Sym : Slab)
+SymsBuilder.insert(Sym);
+}
+
+trace::Span Build("Build index");
+return UseDex ? dex::DexIndex::build(std::move(SymsBuilder).build())
+  : MemIndex::build(std::move(SymsBuilder).build());
+  });
 }
 
 } // namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



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)

ebevhan wrote:
> rjmccall wrote:
> > 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.
> Hm, alright. I figured it was better to both provide the exact section and 
> also include a summary here so you don't have to look it up.
> 
> Should I change it or is it good anyway?
It's fine to include a reference to the standard.  The way you've done it here 
makes it look like a quote at first glance, which it isn't.  You should just 
say "See C99 6.5.2.5p6" if you think that's an important citation to make.

In this case, though, I don't think it's a particularly valuable citation.  
Remember that this code is in the middle of a function dedicated to the rules 
of section 6.5.2.5, so unless you're actually quoting the text because there's 
some important subtlety, I think you can assume general familiarity (and/or 
that the reader has the standard open).  There *is* a better citation, though: 
the Embedded C spec explicitly says:

  If the compound literal occurs inside the body of a function, the type name 
shall not be qualified by an address-space qualifier.

I'd cite it with a title like "Embedded C modifications to C99 6.5.2.5".

By the way, LLVM code style leaves a space between `if` and the opening 
parenthesis of the condition.


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

[Petr Hosek via Phabricator via cfe-commits]

phosek added a comment.

What about other compiler-rt runtimes? Shouldn't we use the same approach for 
those?

In case of multiarch runtime layout, we already add 
 
the runtime directory to the new `LibraryPaths` list, which is added 

 to the list of library paths. We could consider doing the same for the default 
layout, in which case all `getCompilerRT` would need to return is the name of 
the library rather than a path, but we would need to use `-l` for all 
compiler-rt runtimes, not just builtins. That's going to require more changes, 
but I think it's going to result in a cleaner design.

@beanz would that work for Darwin as well?




Comment at: lib/Driver/ToolChains/CommonArgs.cpp:1164
+  case ToolChain::RLT_CompilerRT: {
+StringRef Path = TC.getCompilerRTArgString(Args, "builtins");
+StringRef Dir = llvm::sys::path::parent_path(Path);

Rather than parsing the return string below, wouldn't it be cleaner to modify 
`ToolChain::getCompilerRT` to return a tuple 
(`ToolChain::getCompilerRTArgString` could still return a string) and then use 
if here?



Comment at: lib/Driver/ToolChains/CommonArgs.cpp:1168
+if (Filename.consume_front("lib") && Filename.consume_back(".a")) {
+  CmdArgs.push_back(Args.MakeArgString("-L" + Dir));
+  CmdArgs.push_back(Args.MakeArgString("-l" + Filename));

When the multiarch runtime layout is being used, this will result in this path 
being added twice, once in the `ToolChain` constructor and once here.


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

r341033 - AMDGPU: Default to hidden visibility

[Matt Arsenault via cfe-commits]

Author: arsenm
Date: Thu Aug 30 01:18:06 2018
New Revision: 341033

URL: http://llvm.org/viewvc/llvm-project?rev=341033&view=rev
Log:
AMDGPU: Default to hidden visibility

Object linking isn't supported, so it's not useful
to emit default visibility. Default visibility requires
relocations we don't yet support for functions compiled
in another translation unit.

WebAssembly already does this, although they insert these
arguments in a different place for some reason.

Added:
cfe/trunk/test/Driver/amdgpu-visibility.cl
Modified:
cfe/trunk/lib/Driver/ToolChains/AMDGPU.cpp
cfe/trunk/lib/Driver/ToolChains/AMDGPU.h

Modified: cfe/trunk/lib/Driver/ToolChains/AMDGPU.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/AMDGPU.cpp?rev=341033&r1=341032&r2=341033&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/AMDGPU.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/AMDGPU.cpp Thu Aug 30 01:18:06 2018
@@ -98,3 +98,16 @@ AMDGPUToolChain::TranslateArgs(const Der
 
   return DAL;
 }
+
+void AMDGPUToolChain::addClangTargetOptions(
+const llvm::opt::ArgList &DriverArgs,
+llvm::opt::ArgStringList &CC1Args,
+Action::OffloadKind DeviceOffloadingKind) const {
+  // Default to "hidden" visibility, as object level linking will not be
+  // supported for the forseeable future.
+  if (!DriverArgs.hasArg(options::OPT_fvisibility_EQ,
+ options::OPT_fvisibility_ms_compat)) {
+CC1Args.push_back("-fvisibility");
+CC1Args.push_back("hidden");
+  }
+}

Modified: cfe/trunk/lib/Driver/ToolChains/AMDGPU.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/AMDGPU.h?rev=341033&r1=341032&r2=341033&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/AMDGPU.h (original)
+++ cfe/trunk/lib/Driver/ToolChains/AMDGPU.h Thu Aug 30 01:18:06 2018
@@ -61,6 +61,10 @@ public:
   llvm::opt::DerivedArgList *
   TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch,
 Action::OffloadKind DeviceOffloadKind) const override;
+
+  void addClangTargetOptions(const llvm::opt::ArgList &DriverArgs,
+ llvm::opt::ArgStringList &CC1Args,
+ Action::OffloadKind DeviceOffloadKind) const 
override;
 };
 
 } // end namespace toolchains

Added: cfe/trunk/test/Driver/amdgpu-visibility.cl
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/amdgpu-visibility.cl?rev=341033&view=auto
==
--- cfe/trunk/test/Driver/amdgpu-visibility.cl (added)
+++ cfe/trunk/test/Driver/amdgpu-visibility.cl Thu Aug 30 01:18:06 2018
@@ -0,0 +1,7 @@
+// RUN: %clang -### -target amdgcn-amd-amdhsa -x cl -c -emit-llvm %s 2>&1 | 
FileCheck -check-prefix=DEFAULT %s
+// RUN: %clang -### -target amdgcn-amd-amdhsa -x cl -c -emit-llvm 
-fvisibility=protected  %s 2>&1 | FileCheck -check-prefix=OVERRIDE-PROTECTED  %s
+// RUN: %clang -### -target amdgcn-amd-amdhsa -x cl -c -emit-llvm 
-fvisibility-ms-compat  %s 2>&1 | FileCheck -check-prefix=OVERRIDE-MS  %s
+
+// DEFAULT: "-fvisibility" "hidden"
+// OVERRIDE-PROTECTED: "-fvisibility" "protected"
+// OVERRIDE-MS:  "-fvisibility" "hidden" "-ftype-visibility" "default"


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

[PATCH] D51209: AMDGPU: Default to hidden visibility

[Matt Arsenault via Phabricator via cfe-commits]

arsenm closed this revision.
arsenm added a comment.

r341033


https://reviews.llvm.org/D51209



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

[libcxx] r341034 - Merging r340823:

[Hans Wennborg via cfe-commits]

Author: hans
Date: Thu Aug 30 01:19:15 2018
New Revision: 341034

URL: http://llvm.org/viewvc/llvm-project?rev=341034&view=rev
Log:
Merging r340823:

r340823 | marshall | 2018-08-28 15:29:30 +0200 (Tue, 28 Aug 2018) | 1 line

Use addressof instead of operator& in make_shared. Fixes PR38729. As a 
drive-by, make the same change in raw_storage_iterator (twice).


Modified:
libcxx/branches/release_70/   (props changed)
libcxx/branches/release_70/include/memory

libcxx/branches/release_70/test/std/utilities/memory/storage.iterator/raw_storage_iterator.base.pass.cpp

libcxx/branches/release_70/test/std/utilities/memory/storage.iterator/raw_storage_iterator.pass.cpp

libcxx/branches/release_70/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/allocate_shared.pass.cpp

libcxx/branches/release_70/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared.pass.cpp

Propchange: libcxx/branches/release_70/
--
--- svn:mergeinfo (original)
+++ svn:mergeinfo Thu Aug 30 01:19:15 2018
@@ -1,2 +1,2 @@
 /libcxx/branches/apple:136569-137939
-/libcxx/trunk:339431,339675,339697,339702,339741-339743,339794,339804,339816,339874,340406,340544
+/libcxx/trunk:339431,339675,339697,339702,339741-339743,339794,339804,339816,339874,340406,340544,340823

Modified: libcxx/branches/release_70/include/memory
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/branches/release_70/include/memory?rev=341034&r1=341033&r2=341034&view=diff
==
--- libcxx/branches/release_70/include/memory (original)
+++ libcxx/branches/release_70/include/memory Thu Aug 30 01:19:15 2018
@@ -1989,10 +1989,10 @@ public:
 _LIBCPP_INLINE_VISIBILITY explicit raw_storage_iterator(_OutputIterator 
__x) : __x_(__x) {}
 _LIBCPP_INLINE_VISIBILITY raw_storage_iterator& operator*() {return *this;}
 _LIBCPP_INLINE_VISIBILITY raw_storage_iterator& operator=(const _Tp& 
__element)
-{::new(&*__x_) _Tp(__element); return *this;}
+{::new(_VSTD::addressof(*__x_)) _Tp(__element); return *this;}
 #if _LIBCPP_STD_VER >= 14
 _LIBCPP_INLINE_VISIBILITY raw_storage_iterator& operator=(_Tp&& __element)
-{::new(&*__x_) _Tp(_VSTD::move(__element)); return *this;}
+{::new(_VSTD::addressof(*__x_)) _Tp(_VSTD::move(__element)); return 
*this;}
 #endif
 _LIBCPP_INLINE_VISIBILITY raw_storage_iterator& operator++() {++__x_; 
return *this;}
 _LIBCPP_INLINE_VISIBILITY raw_storage_iterator  operator++(int)
@@ -3682,7 +3682,7 @@ private:
 virtual void __on_zero_shared_weak() _NOEXCEPT;
 public:
 _LIBCPP_INLINE_VISIBILITY
-_Tp* get() _NOEXCEPT {return &__data_.second();}
+_Tp* get() _NOEXCEPT {return _VSTD::addressof(__data_.second());}
 };
 
 template 

Modified: 
libcxx/branches/release_70/test/std/utilities/memory/storage.iterator/raw_storage_iterator.base.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/branches/release_70/test/std/utilities/memory/storage.iterator/raw_storage_iterator.base.pass.cpp?rev=341034&r1=341033&r2=341034&view=diff
==
--- 
libcxx/branches/release_70/test/std/utilities/memory/storage.iterator/raw_storage_iterator.base.pass.cpp
 (original)
+++ 
libcxx/branches/release_70/test/std/utilities/memory/storage.iterator/raw_storage_iterator.base.pass.cpp
 Thu Aug 30 01:19:15 2018
@@ -15,6 +15,13 @@
 
 #include "test_macros.h"
 
+#if TEST_STD_VER >= 11
+#define DELETE_FUNCTION = delete
+#else
+#define DELETE_FUNCTION
+#endif
+
+
 int A_constructed = 0;
 
 struct A
@@ -27,6 +34,7 @@ public:
 ~A() {--A_constructed; data_ = 0;}
 
 bool operator==(int i) const {return data_ == i;}
+A* operator& () DELETE_FUNCTION;
 };
 
 int main()

Modified: 
libcxx/branches/release_70/test/std/utilities/memory/storage.iterator/raw_storage_iterator.pass.cpp
URL: 
http://llvm.org/viewvc/llvm-project/libcxx/branches/release_70/test/std/utilities/memory/storage.iterator/raw_storage_iterator.pass.cpp?rev=341034&r1=341033&r2=341034&view=diff
==
--- 
libcxx/branches/release_70/test/std/utilities/memory/storage.iterator/raw_storage_iterator.pass.cpp
 (original)
+++ 
libcxx/branches/release_70/test/std/utilities/memory/storage.iterator/raw_storage_iterator.pass.cpp
 Thu Aug 30 01:19:15 2018
@@ -16,6 +16,12 @@
 #include "test_macros.h"
 #include 
 
+#if TEST_STD_VER >= 11
+#define DELETE_FUNCTION = delete
+#else
+#define DELETE_FUNCTION
+#endif
+
 int A_constructed = 0;
 
 struct A
@@ -28,6 +34,7 @@ public:
 ~A() {--A_constructed; data_ = 0;}
 
 bool operator==(int i) const {

Re: [libcxx] r340823 - Use addressof instead of operator& in make_shared. Fixes PR38729. As a drive-by, make the same change in raw_storage_iterator (twice).

[Hans Wennborg via cfe-commits]

Merged to 7.0 in r341034.

On Tue, Aug 28, 2018 at 3:29 PM, Marshall Clow via cfe-commits
 wrote:
> Author: marshall
> Date: Tue Aug 28 06:29:30 2018
> New Revision: 340823
>
> URL: http://llvm.org/viewvc/llvm-project?rev=340823&view=rev
> Log:
> Use addressof instead of operator& in make_shared. Fixes PR38729. As a 
> drive-by, make the same change in raw_storage_iterator (twice).
>
> Modified:
> libcxx/trunk/include/memory
> 
> libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.base.pass.cpp
> 
> libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.pass.cpp
> 
> libcxx/trunk/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/allocate_shared.pass.cpp
> 
> libcxx/trunk/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/make_shared.pass.cpp
>
> Modified: libcxx/trunk/include/memory
> URL: 
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/memory?rev=340823&r1=340822&r2=340823&view=diff
> ==
> --- libcxx/trunk/include/memory (original)
> +++ libcxx/trunk/include/memory Tue Aug 28 06:29:30 2018
> @@ -1989,10 +1989,10 @@ public:
>  _LIBCPP_INLINE_VISIBILITY explicit raw_storage_iterator(_OutputIterator 
> __x) : __x_(__x) {}
>  _LIBCPP_INLINE_VISIBILITY raw_storage_iterator& operator*() {return 
> *this;}
>  _LIBCPP_INLINE_VISIBILITY raw_storage_iterator& operator=(const _Tp& 
> __element)
> -{::new(&*__x_) _Tp(__element); return *this;}
> +{::new(_VSTD::addressof(*__x_)) _Tp(__element); return *this;}
>  #if _LIBCPP_STD_VER >= 14
>  _LIBCPP_INLINE_VISIBILITY raw_storage_iterator& operator=(_Tp&& 
> __element)
> -{::new(&*__x_) _Tp(_VSTD::move(__element)); return *this;}
> +{::new(_VSTD::addressof(*__x_)) _Tp(_VSTD::move(__element)); return 
> *this;}
>  #endif
>  _LIBCPP_INLINE_VISIBILITY raw_storage_iterator& operator++() {++__x_; 
> return *this;}
>  _LIBCPP_INLINE_VISIBILITY raw_storage_iterator  operator++(int)
> @@ -3682,7 +3682,7 @@ private:
>  virtual void __on_zero_shared_weak() _NOEXCEPT;
>  public:
>  _LIBCPP_INLINE_VISIBILITY
> -_Tp* get() _NOEXCEPT {return &__data_.second();}
> +_Tp* get() _NOEXCEPT {return _VSTD::addressof(__data_.second());}
>  };
>
>  template 
>
> Modified: 
> libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.base.pass.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.base.pass.cpp?rev=340823&r1=340822&r2=340823&view=diff
> ==
> --- 
> libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.base.pass.cpp
>  (original)
> +++ 
> libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.base.pass.cpp
>  Tue Aug 28 06:29:30 2018
> @@ -15,6 +15,13 @@
>
>  #include "test_macros.h"
>
> +#if TEST_STD_VER >= 11
> +#define DELETE_FUNCTION = delete
> +#else
> +#define DELETE_FUNCTION
> +#endif
> +
> +
>  int A_constructed = 0;
>
>  struct A
> @@ -27,6 +34,7 @@ public:
>  ~A() {--A_constructed; data_ = 0;}
>
>  bool operator==(int i) const {return data_ == i;}
> +A* operator& () DELETE_FUNCTION;
>  };
>
>  int main()
>
> Modified: 
> libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.pass.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.pass.cpp?rev=340823&r1=340822&r2=340823&view=diff
> ==
> --- 
> libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.pass.cpp
>  (original)
> +++ 
> libcxx/trunk/test/std/utilities/memory/storage.iterator/raw_storage_iterator.pass.cpp
>  Tue Aug 28 06:29:30 2018
> @@ -16,6 +16,12 @@
>  #include "test_macros.h"
>  #include 
>
> +#if TEST_STD_VER >= 11
> +#define DELETE_FUNCTION = delete
> +#else
> +#define DELETE_FUNCTION
> +#endif
> +
>  int A_constructed = 0;
>
>  struct A
> @@ -28,6 +34,7 @@ public:
>  ~A() {--A_constructed; data_ = 0;}
>
>  bool operator==(int i) const {return data_ == i;}
> +A* operator& () DELETE_FUNCTION;
>  };
>
>  int main()
>
> Modified: 
> libcxx/trunk/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/allocate_shared.pass.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/allocate_shared.pass.cpp?rev=340823&r1=340822&r2=340823&view=diff
> ==
> --- 
> libcxx/trunk/test/std/utilities/memory/util.smartptr/util.smartptr.shared/util.smartptr.shared.create/allocate_shared.pass.cpp
>  (or

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

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clangd/Protocol.h:832
+
+  /// Position of the opening paren of the argument list.
+  /// This is a clangd-specific extension, it is only available via C++ API and

As noted offline, I misread this as being *inside* the paren - could maybe be 
more explicit?
```Position of the start of the argument list, including opening paren. e.g.
foo("first arg", 
   ^argListStart ^cursor
```



Comment at: unittests/clangd/CodeCompleteTests.cpp:914
 
+TEST(SignatureHelpTest, OpeningParen) {
+  Annotations Code(R"cpp(

Hmm, I think this test would be easier to follow if tests 1-5 were written 
separately - it's hard to spot all the locations and how the code interacts.

As a bonus, no need to mess around with explicit positions and the failure 
message can just echo the test:

```
for (const char* Test : {
  R"cpp(
int foo(int a, b, c);
int main() { foo(foo$p^(foo(10, 10, 10), ^); }
  )cpp",
  ...
}) {
  EXPECT_EQ(signatures(Test).argListStart, Annotations(Test).point("p")) << 
Test;
}
```


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51437



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

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

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan updated this revision to Diff 163277.
ebevhan edited the summary of this revision.
ebevhan added a comment.

Added Embedded-C clause quote.
Fixed formatting.


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,20 @@
   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) {
+// Embedded-C extensions to C99 6.5.2.5:
+//   "If the compound literal occurs inside the body of a function, the
+//   type name shall not be qualified by an address-space qualifier."
+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.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5733,12 +5733,20 @@
   LiteralExpr = Result.get();
 
   bool isFileScope = !CurContext->isFunctionOrMethod();
-  if (isFileScope &&
-  !LiteralExpr->isTypeDependent() &&
-  !LiteralExpr->isValueDependent() &&
-  !literalType->i

[PATCH] D51356: [docs][mips] Clang 7.0 Release notes

[Hans Wennborg via Phabricator via cfe-commits]

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

lgtm, thanks! Go ahead and commit directly to the branch, or let me know if 
you'd prefer me to do it.


Repository:
  rC Clang

https://reviews.llvm.org/D51356



___
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 forbid mixing of CHECK-NOTES and CHECK-MESSAGES

[Jonas Toth via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL341039: [clang-tidy] fix check_clang_tidy to forbid mixing 
of CHECK-NOTES and CHECK… (authored by JonasToth, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D51381

Files:
  clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py


Index: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
@@ -98,6 +98,9 @@
 sys.exit('%s, %s or %s not found in the input' % (check_fixes_prefix,
  check_messages_prefix, check_notes_prefix) )
 
+  if has_check_notes and has_check_messages:
+sys.exit('Please use either CHECK-NOTES or CHECK-MESSAGES but not both')
+
   # Remove the contents of the CHECK lines to avoid CHECKs matching on
   # themselves.  We need to keep the comments to preserve line numbers while
   # avoiding empty lines which could potentially trigger formatting-related


Index: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
===
--- clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
+++ clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
@@ -98,6 +98,9 @@
 sys.exit('%s, %s or %s not found in the input' % (check_fixes_prefix,
  check_messages_prefix, check_notes_prefix) )
 
+  if has_check_notes and has_check_messages:
+sys.exit('Please use either CHECK-NOTES or CHECK-MESSAGES but not both')
+
   # Remove the contents of the CHECK lines to avoid CHECKs matching on
   # themselves.  We need to keep the comments to preserve line numbers while
   # avoiding empty lines which could potentially trigger formatting-related
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r341039 - [clang-tidy] fix check_clang_tidy to forbid mixing of CHECK-NOTES and CHECK-MESSAGES

[Jonas Toth via cfe-commits]

Author: jonastoth
Date: Thu Aug 30 01:44:27 2018
New Revision: 341039

URL: http://llvm.org/viewvc/llvm-project?rev=341039&view=rev
Log:
[clang-tidy] fix check_clang_tidy to forbid mixing of CHECK-NOTES and 
CHECK-MESSAGES

Summary:
The check_clang_tidy.py script would allow mixing of `CHECK-NOTES` and 
`CHECK-MESSAGES` but running `FileCheck` for that would implicitly fail, 
because `CHECK-NOTES` bails out if there is a warning.

That means a clang-tidy test can not mix these constructs to check warnings 
with `CHECK-MESSAGES` and notes with `CHECK-NOTES`. The script gives now a 
clear error if that happens.

Reviewers: alexfh, aaron.ballman, lebedev.ri, hokein

Reviewed By: lebedev.ri

Subscribers: xazax.hun, cfe-commits

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

Modified:
clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py

Modified: clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py?rev=341039&r1=341038&r2=341039&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py (original)
+++ clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy.py Thu Aug 30 
01:44:27 2018
@@ -98,6 +98,9 @@ def main():
 sys.exit('%s, %s or %s not found in the input' % (check_fixes_prefix,
  check_messages_prefix, check_notes_prefix) )
 
+  if has_check_notes and has_check_messages:
+sys.exit('Please use either CHECK-NOTES or CHECK-MESSAGES but not both')
+
   # Remove the contents of the CHECK lines to avoid CHECKs matching on
   # themselves.  We need to keep the comments to preserve line numbers while
   # avoiding empty lines which could potentially trigger formatting-related


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

[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.

LGTM.


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 added a comment.

In https://reviews.llvm.org/D51440#1218817, @phosek wrote:

> What about other compiler-rt runtimes? Shouldn't we use the same approach for 
> those?


Yes, I guess so.

> In case of multiarch runtime layout, we already add 
> 
>  the runtime directory to the new `LibraryPaths` list, which is added 
> 
>  to the list of library paths. We could consider doing the same for the 
> default layout, in which case all `getCompilerRT` would need to return is the 
> name of the library rather than a path, but we would need to use `-l` for all 
> compiler-rt runtimes, not just builtins. That's going to require more 
> changes, but I think it's going to result in a cleaner design.

Yes, so it sounds.

I'll see if I can get to looking at that sometime soon. I had this patch lying 
around as an attempt to work around the libtool issue in 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27866 which doesn't seem to be 
going anywhere (and even if it did, it probably takes quite a bit of time 
before a new libtool release is made and it gets propagated to most places 
where I'd like to use this), and was curious if there was any specific reason 
for having this the way it was, or just the usual; historical reasons that it 
has started out like this and haven't had a need to change until now. If you 
otherwise would happen to be touching the same areas, feel free to pick it up 
;-) otherwise I'll try to look at addressing your points in a few days.


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] D50958: [clangd] WIP: xrefs for dynamic index.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 163284.
hokein marked 8 inline comments as done.
hokein added a comment.

- address review comments
- rescope the patch only focus on `findReferences` implementation.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50958

Files:
  clangd/XRefs.cpp
  clangd/XRefs.h
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -26,6 +26,7 @@
 using namespace llvm;
 
 namespace {
+using testing::_;
 using testing::ElementsAre;
 using testing::Field;
 using testing::IsEmpty;
@@ -1066,6 +1067,178 @@
   ElementsAre(Location{FooCppUri, FooWithoutHeader.range()}));
 }
 
+TEST(FindReferences, AllWithoutIndex) {
+  const char *Tests[] = {
+  R"cpp(// Local variable
+int main() {
+  int $foo[[foo]];
+  $foo[[^foo]] = 2;
+  int test1 = $foo[[foo]];
+}
+  )cpp",
+
+  R"cpp(// Struct
+namespace ns1 {
+struct $foo[[Foo]] {};
+} // namespace ns1
+int main() {
+  ns1::$foo[[Fo^o]]* Params;
+}
+  )cpp",
+
+  R"cpp(// Function
+int $foo[[foo]](int) {}
+int main() {
+  auto *X = &$foo[[^foo]];
+  $foo[[foo]](42)
+}
+  )cpp",
+
+  R"cpp(// Field
+struct Foo {
+  int $foo[[foo]];
+  Foo() : $foo[[foo]](0) {}
+};
+int main() {
+  Foo f;
+  f.$foo[[f^oo]] = 1;
+}
+  )cpp",
+
+  R"cpp(// Method call
+struct Foo { int [[foo]](); };
+int Foo::[[foo]]() {}
+int main() {
+  Foo f;
+  f.^foo();
+}
+  )cpp",
+
+  R"cpp(// Typedef
+typedef int $foo[[Foo]];
+int main() {
+  $foo[[^Foo]] bar;
+}
+  )cpp",
+
+  R"cpp(// Namespace
+namespace $foo[[ns]] {
+struct Foo {};
+} // namespace ns
+int main() { $foo[[^ns]]::Foo foo; }
+  )cpp",
+  };
+  for (const char *Test : Tests) {
+Annotations T(Test);
+auto AST = TestTU::withCode(T.code()).build();
+std::vector> ExpectedLocations;
+for (const auto &R : T.ranges("foo"))
+  ExpectedLocations.push_back(RangeIs(R));
+EXPECT_THAT(findReferences(AST, T.point()),
+ElementsAreArray(ExpectedLocations))
+<< Test;
+  }
+}
+
+class MockIndex : public SymbolIndex {
+public:
+  MOCK_CONST_METHOD2(fuzzyFind, bool(const FuzzyFindRequest &,
+ llvm::function_ref));
+  MOCK_CONST_METHOD2(lookup, void(const LookupRequest &,
+  llvm::function_ref));
+  MOCK_CONST_METHOD2(findOccurrences,
+ void(const OccurrencesRequest &,
+  llvm::function_ref));
+};
+
+TEST(FindReferences, QueryIndex) {
+  const char *Tests[] = {
+  // Refers to symbols from headers.
+  R"cpp(
+int main() {
+  F^oo foo;
+}
+  )cpp",
+  R"cpp(
+int main() {
+  f^unc();
+}
+  )cpp",
+  R"cpp(
+int main() {
+  return I^NT;
+}
+  )cpp",
+
+  // These are cases of file-local but not function-local symbols, we still
+  // query the index.
+  R"cpp(
+void MyF^unc() {}
+  )cpp",
+
+  R"cpp(
+int My^Int = 2;
+  )cpp",
+  };
+
+  TestTU TU;
+  TU.HeaderCode = R"(
+  class Foo {};
+  static const int INT = 3;
+  inline void func() {};
+  )";
+  MockIndex Index;
+  for (const char *Test : Tests) {
+Annotations T(Test);
+TU.Code = T.code();
+auto AST = TU.build();
+EXPECT_CALL(Index, findOccurrences(_, _));
+findReferences(AST, T.point(), &Index);
+  }
+}
+
+TEST(FindReferences, DontQueryIndex) {
+  // Don't query index for function-local symbols.
+  const char *Tests[] = {
+  R"cpp(// Local variable in function body
+int main() {
+  int $foo[[foo]];
+  $foo[[^foo]] = 2;
+}
+  )cpp",
+
+  R"cpp(// function parameter
+int f(int fo^o) {
+}
+  )cpp",
+
+  R"cpp(// function parameter in lambda
+int f(int foo) {
+  auto func = [](int a, int b) {
+return ^a = 2;
+  };
+}
+  )cpp",
+
+  R"cpp(// capture in lambda
+int f(int foo) {
+  int A;
+  auto func = [&A](int a, int b) {
+return a = ^A;
+  };
+}
+  )cpp",
+  };
+
+  MockIndex Index;
+  for (const char *Test : Tests) {
+Annotations T(Test);
+auto AST = TestTU::withCode(T.code()).build();
+EXPECT_CALL(Index, findOccurrences(_, _)).Times(0);
+findReferences(AST, T.point(), &Index);
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/XRefs.h
===
--- clangd

[PATCH] D50958: [clangd] Implement findReferences function

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clangd/XRefs.cpp:666
+std::vector references(ParsedAST &AST, Position Pos,
+ bool IncludeDeclaration,
+ const SymbolIndex *Index) {

sammccall wrote:
> I'm not sure unpacking the reference options into individual bools is going 
> to scale well. I'd suggest passing the whole struct instead.
Removed this parameter.



Comment at: clangd/XRefs.cpp:681
+   : "Non-local\n");
+  if (!clang::index::isFunctionLocalSymbol(D))
+NonLocalIDs.insert(*ID);

sammccall wrote:
> (This saves us hitting the index to look up references for one symbol, not 
> sure if it's worth it at all).
> 
> I wouldn't particularly trust the helpers in clang::index::. Indeed the 
> implementation looks wrong here (e.g. it would treat lambda captures as 
> global, I think?)
> 
> I'd prefer D->getParentFunctionOrMethod() here.
> 
> 
I think it is worth the effort here. At least for function-local symbols, 
traversing AST is sufficient to get complete xref results.



Comment at: clangd/XRefs.cpp:688
+  SymbolOccurrenceKind::Reference | SymbolOccurrenceKind::Definition;
+  if (IncludeDeclaration)
+Filter |= SymbolOccurrenceKind::Declaration;

sammccall wrote:
> I'm not sure this is the best interpretation of LSP `includeDeclaration`.
> 
> The LSP spec is very vague here, and we can usually assume it's written with 
> typescript in mind :-) where the declaration/definition distinction doesn't 
> really exist.
> It appears the distinction they're trying to draw is declaration vs "only" 
> reference, rather than definition vs "only" declaration. So I think if we're 
> going to respect this flag, we should *exclude* definitions when the flag is 
> false.
> 
> Alternatively we could punt on this and just ignore the flag for now, and add 
> it in a followup.
Ignore this flag, and always return all kinds.



Comment at: clangd/XRefs.cpp:694
+
+  SymbolCollector Collector({nullptr, &Opts}, {});
+  index::IndexingOptions IndexOpts;

sammccall wrote:
> Reusing symbolcollector here seems odd.
> 
> It forces us to go through SymbolID rather than just using the Decl*. This is 
> certainly reliable for global symbols, but I've got no idea how robust USRs 
> are for local symbols. It also ends up building the Symbol objects, which we 
> then throw away.
> 
> How much of SymbolCollector are we really reusing, vs a custom 
> IndexDataConsumer? How is this different from document-highlights? Maybe we 
> can reuse the consumer.
I refined the highlight IndexDataConsumer, and now the common code is shared.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50958



___
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 updated this revision to Diff 163286.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.

- Clarify the docs, add example.
- Parse each test separately.


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,54 @@
   EXPECT_EQ(1, Results.activeParameter);
 }
 
+TEST(SignatureHelpTest, OpeningParen) {
+  llvm::StringLiteral Tests[] = {// Recursive function call.
+ R"cpp(
+int foo(int a, int b, int c);
+int main() {
+  foo(foo $p^( foo(10, 10, 10), ^ )));
+})cpp",
+ // Functional type cast.
+ R"cpp(
+struct Foo {
+  Foo(int a, int b, int c);
+};
+int main() {
+  Foo $p^( 10, ^ );
+})cpp",
+ // New expression.
+ R"cpp(
+struct Foo {
+  Foo(int a, int b, int c);
+};
+int main() {
+  new Foo $p^( 10, ^ );
+})cpp",
+ // Macro expansion.
+ R"cpp(
+int foo(int a, int b, int c);
+#define FOO foo(
+
+int main() {
+  // Macro expansions.
+  $p^FOO 10, ^ );
+})cpp",
+ // Macro arguments.
+ R"cpp(
+int foo(int a, int b, int c);
+int main() {
+#define ID(X) X
+  ID(foo $p^( foo(10), ^ ))
+})cpp"};
+
+  for (auto Test : Tests) {
+Annotations Code(Test);
+EXPECT_EQ(signatures(Code.code(), Code.point()).argListStart,
+  Code.point("p"))
+<< "Test source:" << Test;
+  }
+}
+
 class IndexRequestCollector : public SymbolIndex {
 public:
   bool
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -828,6 +828,13 @@
 
   /// The active parameter of the active signature.
   int activeParameter = 0;
+
+  /// Position of the start of the argument list, including opening paren. e.g.
+  /// foo("first arg",   "second arg",
+  ///^-argListStart   ^-cursor
+  /// 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 added inline comments.



Comment at: unittests/clangd/CodeCompleteTests.cpp:914
 
+TEST(SignatureHelpTest, OpeningParen) {
+  Annotations Code(R"cpp(

sammccall wrote:
> Hmm, I think this test would be easier to follow if tests 1-5 were written 
> separately - it's hard to spot all the locations and how the code interacts.
> 
> As a bonus, no need to mess around with explicit positions and the failure 
> message can just echo the test:
> 
> ```
> for (const char* Test : {
>   R"cpp(
> int foo(int a, b, c);
> int main() { foo(foo$p^(foo(10, 10, 10), ^); }
>   )cpp",
>   ...
> }) {
>   EXPECT_EQ(signatures(Test).argListStart, Annotations(Test).point("p")) << 
> Test;
> }
> ```
Thanks! It's way better this way!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51437



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

[PATCH] D51296: [OpenCL] Traverse vector types for ocl extensions support

[Viktoria Maximova via Phabricator via cfe-commits]

vmaksimo updated this revision to Diff 163287.
vmaksimo retitled this revision from "[Sema] Traverse vector types for ocl 
extensions support" to "[OpenCL] Traverse vector types for ocl extensions 
support".
vmaksimo edited the summary of this revision.

Repository:
  rC Clang

https://reviews.llvm.org/D51296

Files:
  lib/Sema/Sema.cpp
  test/SemaOpenCL/extensions.cl


Index: test/SemaOpenCL/extensions.cl
===
--- test/SemaOpenCL/extensions.cl
+++ test/SemaOpenCL/extensions.cl
@@ -70,6 +70,13 @@
 // expected-error@-2{{use of type 'double' requires cl_khr_fp64 extension to 
be enabled}}
 #endif
 
+  typedef double double4 __attribute__((ext_vector_type(4)));
+  double4 d4 = {0.0f, 2.0f, 3.0f, 1.0f};
+#ifdef NOFP64
+// expected-error@-3 {{use of type 'double' requires cl_khr_fp64 extension to 
be enabled}}
+// expected-error@-3 {{use of type 'double4' (vector of 4 'double' values) 
requires cl_khr_fp64 extension to be enabled}}
+#endif
+
   (void) 1.0;
 
 #ifdef NOFP64
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -1852,6 +1853,14 @@
   if (auto TagT = dyn_cast(QT.getCanonicalType().getTypePtr()))
 Decl = TagT->getDecl();
   auto Loc = DS.getTypeSpecTypeLoc();
+
+  // Check extensions for vector types.
+  // e.g. double4 is not allowed when cl_khr_fp64 is absent.
+  if (QT->isExtVectorType()) {
+auto TypePtr = QT->castAs()->getElementType().getTypePtr();
+return checkOpenCLDisabledTypeOrDecl(TypePtr, Loc, QT, OpenCLTypeExtMap);
+  }
+
   if (checkOpenCLDisabledTypeOrDecl(Decl, Loc, QT, OpenCLDeclExtMap))
 return true;
 


Index: test/SemaOpenCL/extensions.cl
===
--- test/SemaOpenCL/extensions.cl
+++ test/SemaOpenCL/extensions.cl
@@ -70,6 +70,13 @@
 // expected-error@-2{{use of type 'double' requires cl_khr_fp64 extension to be enabled}}
 #endif
 
+  typedef double double4 __attribute__((ext_vector_type(4)));
+  double4 d4 = {0.0f, 2.0f, 3.0f, 1.0f};
+#ifdef NOFP64
+// expected-error@-3 {{use of type 'double' requires cl_khr_fp64 extension to be enabled}}
+// expected-error@-3 {{use of type 'double4' (vector of 4 'double' values) requires cl_khr_fp64 extension to be enabled}}
+#endif
+
   (void) 1.0;
 
 #ifdef NOFP64
Index: lib/Sema/Sema.cpp
===
--- lib/Sema/Sema.cpp
+++ lib/Sema/Sema.cpp
@@ -1852,6 +1853,14 @@
   if (auto TagT = dyn_cast(QT.getCanonicalType().getTypePtr()))
 Decl = TagT->getDecl();
   auto Loc = DS.getTypeSpecTypeLoc();
+
+  // Check extensions for vector types.
+  // e.g. double4 is not allowed when cl_khr_fp64 is absent.
+  if (QT->isExtVectorType()) {
+auto TypePtr = QT->castAs()->getElementType().getTypePtr();
+return checkOpenCLDisabledTypeOrDecl(TypePtr, Loc, QT, OpenCLTypeExtMap);
+  }
+
   if (checkOpenCLDisabledTypeOrDecl(Decl, Loc, QT, OpenCLDeclExtMap))
 return true;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51356: [docs][mips] Clang 7.0 Release notes

[Simon Dardis via Phabricator via cfe-commits]

sdardis added inline comments.



Comment at: ReleaseNotes.rst:121
+- On MIPS FreeBSD default CPUs have been changed to ``mips2``
+  for 32-bit targets and ``mips3`` for 32-bit targets.
+

``mips3`` for 64 bit targets.

D48499.


Repository:
  rC Clang

https://reviews.llvm.org/D51356



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

[PATCH] D51481: [clangd] Implement proximity path boosting for Dex

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added reviewers: ioeric, ilya-biryukov, sammccall.
kbobyrev added a project: clang-tools-extra.
Herald added subscribers: kadircet, arphaman, mgrang, jkorous, MaskRay, mgorny.

https://reviews.llvm.org/D51481

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/DexIndex.cpp
  clang-tools-extra/clangd/index/dex/DexIndex.h
  clang-tools-extra/clangd/index/dex/Token.cpp
  clang-tools-extra/clangd/index/dex/Token.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  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
@@ -7,6 +7,7 @@
 //
 //===--===//
 
+#include "FuzzyMatch.h"
 #include "TestIndex.h"
 #include "index/Index.h"
 #include "index/Merge.h"
@@ -29,6 +30,12 @@
 namespace dex {
 namespace {
 
+std::vector URISchemes = {"file"};
+
+//===--===//
+// Query Iterators tests.
+//===--===//
+
 std::vector consumeIDs(Iterator &It) {
   auto IDAndScore = consume(It);
   std::vector IDs(IDAndScore.size());
@@ -324,16 +331,39 @@
   EXPECT_THAT(ElementBoost, 3);
 }
 
+//===--===//
+// Search Token tests.
+//===--===//
+
 testing::Matcher>
-trigramsAre(std::initializer_list Trigrams) {
+tokensAre(std::initializer_list Strings, Token::Kind Kind) {
   std::vector Tokens;
-  for (const auto &Symbols : Trigrams) {
-Tokens.push_back(Token(Token::Kind::Trigram, Symbols));
+  for (const auto &TokenData : Strings) {
+Tokens.push_back(Token(Kind, TokenData));
   }
   return testing::UnorderedElementsAreArray(Tokens);
 }
 
-TEST(DexIndexTrigrams, IdentifierTrigrams) {
+testing::Matcher>
+trigramsAre(std::initializer_list Trigrams) {
+  return tokensAre(Trigrams, Token::Kind::Trigram);
+}
+
+testing::Matcher>
+pathsAre(std::initializer_list Paths) {
+  return tokensAre(Paths, Token::Kind::Path);
+}
+
+testing::Matcher>> proximityPathsAre(
+std::initializer_list> ProximityPaths) {
+  std::vector> Result;
+  for (const auto &P : ProximityPaths) {
+Result.push_back({Token(Token::Kind::Path, P.first), P.second});
+  }
+  return testing::UnorderedElementsAreArray(Result);
+}
+
+TEST(DexSearchTokens, IdentifierTrigrams) {
   EXPECT_THAT(generateIdentifierTrigrams("X86"),
   trigramsAre({"x86", "x$$", "x8$"}));
 
@@ -374,7 +404,7 @@
"hkl", "ijk", "ikl", "jkl", "klm", "ab$", "ad$"}));
 }
 
-TEST(DexIndexTrigrams, QueryTrigrams) {
+TEST(DexSearchTokens, QueryTrigrams) {
   EXPECT_THAT(generateQueryTrigrams("c"), trigramsAre({"c$$"}));
   EXPECT_THAT(generateQueryTrigrams("cl"), trigramsAre({"cl$"}));
   EXPECT_THAT(generateQueryTrigrams("cla"), trigramsAre({"cla"}));
@@ -407,9 +437,31 @@
"hij", "ijk", "jkl", "klm"}));
 }
 
+TEST(DexSearchTokens, SymbolPath) {
+  EXPECT_THAT(generateProximityPaths(
+  "file:///clang-tools-extra/clangd/index/dex/Token.h"),
+  pathsAre({"file:///clang-tools-extra/clangd/index/dex/",
+"file:///clang-tools-extra/clangd/index/",
+"file:///clang-tools-extra/clangd/",
+"file:///clang-tools-extra/", "file:///"}));
+}
+
+TEST(DexSearchTokens, QueryProximityPaths) {
+  EXPECT_THAT(generateQueryProximityPaths("/a/b/c/d/e/f/g.h", URISchemes),
+  proximityPathsAre({{"file:///a/b/c/d/e/f/", 6.},
+ {"file:///a/b/c/d/e/", 5.},
+ {"file:///a/b/c/d/", 4.},
+ {"file:///a/b/c/", 3.},
+ {"file:///a/b/", 2.}}));
+}
+
+//===--===//
+// Index tests.
+//===--===//
+
 TEST(DexIndex, Lookup) {
   DexIndex I;
-  I.build(generateSymbols({"ns::abc", "ns::xyz"}));
+  I.build(generateSymbols({"ns::abc", "ns::xyz"}), URISchemes);
   EXPECT_THAT(lookup(I, SymbolID("ns::abc")), UnorderedElementsAre("ns::abc"));
   EXPECT_THAT(lookup(I, {SymbolID("ns::abc"), SymbolID("ns::xyz")}),
   UnorderedElementsAre("ns::abc", "ns::xyz"));
@@ -421,7 +473,8 @@
 TEST(DexIndex, FuzzyFind) {
   DexIndex Index;
   Index.build(generateSymbols({"ns::ABC", "ns::BCD", "::ABC", "ns::nested::ABC",
-   "other::ABC", "other::A"}));
+   "other::ABC", "other::A"}),
+  URISchemes);
   FuzzyFindReque

[PATCH] D51481: [clangd] Implement proximity path boosting for Dex

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 163294.
kbobyrev added a comment.

Stop query generation as soon as one valid URI scheme was found.


https://reviews.llvm.org/D51481

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/DexIndex.cpp
  clang-tools-extra/clangd/index/dex/DexIndex.h
  clang-tools-extra/clangd/index/dex/Token.cpp
  clang-tools-extra/clangd/index/dex/Token.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  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
@@ -7,6 +7,7 @@
 //
 //===--===//
 
+#include "FuzzyMatch.h"
 #include "TestIndex.h"
 #include "index/Index.h"
 #include "index/Merge.h"
@@ -29,6 +30,12 @@
 namespace dex {
 namespace {
 
+std::vector URISchemes = {"file"};
+
+//===--===//
+// Query Iterators tests.
+//===--===//
+
 std::vector consumeIDs(Iterator &It) {
   auto IDAndScore = consume(It);
   std::vector IDs(IDAndScore.size());
@@ -324,16 +331,39 @@
   EXPECT_THAT(ElementBoost, 3);
 }
 
+//===--===//
+// Search Token tests.
+//===--===//
+
 testing::Matcher>
-trigramsAre(std::initializer_list Trigrams) {
+tokensAre(std::initializer_list Strings, Token::Kind Kind) {
   std::vector Tokens;
-  for (const auto &Symbols : Trigrams) {
-Tokens.push_back(Token(Token::Kind::Trigram, Symbols));
+  for (const auto &TokenData : Strings) {
+Tokens.push_back(Token(Kind, TokenData));
   }
   return testing::UnorderedElementsAreArray(Tokens);
 }
 
-TEST(DexIndexTrigrams, IdentifierTrigrams) {
+testing::Matcher>
+trigramsAre(std::initializer_list Trigrams) {
+  return tokensAre(Trigrams, Token::Kind::Trigram);
+}
+
+testing::Matcher>
+pathsAre(std::initializer_list Paths) {
+  return tokensAre(Paths, Token::Kind::Path);
+}
+
+testing::Matcher>> proximityPathsAre(
+std::initializer_list> ProximityPaths) {
+  std::vector> Result;
+  for (const auto &P : ProximityPaths) {
+Result.push_back({Token(Token::Kind::Path, P.first), P.second});
+  }
+  return testing::UnorderedElementsAreArray(Result);
+}
+
+TEST(DexSearchTokens, IdentifierTrigrams) {
   EXPECT_THAT(generateIdentifierTrigrams("X86"),
   trigramsAre({"x86", "x$$", "x8$"}));
 
@@ -374,7 +404,7 @@
"hkl", "ijk", "ikl", "jkl", "klm", "ab$", "ad$"}));
 }
 
-TEST(DexIndexTrigrams, QueryTrigrams) {
+TEST(DexSearchTokens, QueryTrigrams) {
   EXPECT_THAT(generateQueryTrigrams("c"), trigramsAre({"c$$"}));
   EXPECT_THAT(generateQueryTrigrams("cl"), trigramsAre({"cl$"}));
   EXPECT_THAT(generateQueryTrigrams("cla"), trigramsAre({"cla"}));
@@ -407,9 +437,31 @@
"hij", "ijk", "jkl", "klm"}));
 }
 
+TEST(DexSearchTokens, SymbolPath) {
+  EXPECT_THAT(generateProximityPaths(
+  "file:///clang-tools-extra/clangd/index/dex/Token.h"),
+  pathsAre({"file:///clang-tools-extra/clangd/index/dex/",
+"file:///clang-tools-extra/clangd/index/",
+"file:///clang-tools-extra/clangd/",
+"file:///clang-tools-extra/", "file:///"}));
+}
+
+TEST(DexSearchTokens, QueryProximityPaths) {
+  EXPECT_THAT(generateQueryProximityPaths("/a/b/c/d/e/f/g.h", URISchemes),
+  proximityPathsAre({{"file:///a/b/c/d/e/f/", 6.},
+ {"file:///a/b/c/d/e/", 5.},
+ {"file:///a/b/c/d/", 4.},
+ {"file:///a/b/c/", 3.},
+ {"file:///a/b/", 2.}}));
+}
+
+//===--===//
+// Index tests.
+//===--===//
+
 TEST(DexIndex, Lookup) {
   DexIndex I;
-  I.build(generateSymbols({"ns::abc", "ns::xyz"}));
+  I.build(generateSymbols({"ns::abc", "ns::xyz"}), URISchemes);
   EXPECT_THAT(lookup(I, SymbolID("ns::abc")), UnorderedElementsAre("ns::abc"));
   EXPECT_THAT(lookup(I, {SymbolID("ns::abc"), SymbolID("ns::xyz")}),
   UnorderedElementsAre("ns::abc", "ns::xyz"));
@@ -421,7 +473,8 @@
 TEST(DexIndex, FuzzyFind) {
   DexIndex Index;
   Index.build(generateSymbols({"ns::ABC", "ns::BCD", "::ABC", "ns::nested::ABC",
-   "other::ABC", "other::A"}));
+   "other::ABC", "other::A"}),
+  URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "ABC";
   Req.Scopes = {"ns::"};
@@ -444,7 +497,8 @@
 

[PATCH] D51484: [OpenCL] Add support of cl_intel_device_side_avc_motion_estimation extension

[Alexey Sachkov via Phabricator via cfe-commits]

AlexeySachkov created this revision.
AlexeySachkov added reviewers: Anastasia, yaxunl.

Documentation can be found at 
https://www.khronos.org/registry/OpenCL/extensions/intel/cl_intel_device_side_avc_motion_estimation.txt

Patch by Kristina Bessonova


Repository:
  rC Clang

https://reviews.llvm.org/D51484

Files:
  include/clang-c/Index.h
  include/clang/AST/ASTContext.h
  include/clang/AST/Type.h
  include/clang/Basic/OpenCLExtensionTypes.def
  include/clang/Basic/OpenCLExtensions.def
  include/clang/Serialization/ASTBitCodes.h
  include/clang/module.modulemap
  lib/AST/ASTContext.cpp
  lib/AST/ASTImporter.cpp
  lib/AST/ExprConstant.cpp
  lib/AST/ItaniumMangle.cpp
  lib/AST/MicrosoftMangle.cpp
  lib/AST/NSAPI.cpp
  lib/AST/Type.cpp
  lib/AST/TypeLoc.cpp
  lib/Analysis/PrintfFormatString.cpp
  lib/CodeGen/CGDebugInfo.cpp
  lib/CodeGen/CGDebugInfo.h
  lib/CodeGen/CGOpenCLRuntime.cpp
  lib/CodeGen/CodeGenTypes.cpp
  lib/CodeGen/ItaniumCXXABI.cpp
  lib/Headers/opencl-c.h
  lib/Index/USRGeneration.cpp
  lib/Sema/Sema.cpp
  lib/Sema/SemaExpr.cpp
  lib/Serialization/ASTCommon.cpp
  lib/Serialization/ASTReader.cpp
  test/CodeGenOpenCL/intel-subgroups-avc-ext-types.cl
  test/SemaOpenCL/extension-version.cl
  tools/libclang/CIndex.cpp
  tools/libclang/CXType.cpp

Index: lib/Serialization/ASTReader.cpp
===
--- lib/Serialization/ASTReader.cpp
+++ lib/Serialization/ASTReader.cpp
@@ -6970,6 +6970,11 @@
   T = Context.SingletonId; \
   break;
 #include "clang/Basic/OpenCLImageTypes.def"
+#define EXT_OPAQUE_TYPE(ExtType, Id, Ext) \
+case PREDEF_TYPE_##Id##_ID: \
+  T = Context.Id##Ty; \
+  break;
+#include "clang/Basic/OpenCLExtensionTypes.def"
 case PREDEF_TYPE_SAMPLER_ID:
   T = Context.OCLSamplerTy;
   break;
Index: lib/Serialization/ASTCommon.cpp
===
--- lib/Serialization/ASTCommon.cpp
+++ lib/Serialization/ASTCommon.cpp
@@ -213,6 +213,11 @@
 ID = PREDEF_TYPE_##Id##_ID; \
 break;
 #include "clang/Basic/OpenCLImageTypes.def"
+#define EXT_OPAQUE_TYPE(ExtType, Id, Ext) \
+  case BuiltinType::Id: \
+ID = PREDEF_TYPE_##Id##_ID; \
+break;
+#include "clang/Basic/OpenCLExtensionTypes.def"
   case BuiltinType::OCLSampler:
 ID = PREDEF_TYPE_SAMPLER_ID;
 break;
Index: lib/Headers/opencl-c.h
===
--- lib/Headers/opencl-c.h
+++ lib/Headers/opencl-c.h
@@ -16193,6 +16193,628 @@
 void__ovld __conv intel_sub_group_block_write_us8( __global ushort* p, ushort8 data );
 #endif // cl_intel_subgroups_short
 
+#ifdef cl_intel_device_side_avc_motion_estimation
+#pragma OPENCL EXTENSION cl_intel_device_side_avc_motion_estimation : enable
+
+#define CLK_AVC_ME_MAJOR_16x16_INTEL 0x0
+#define CLK_AVC_ME_MAJOR_16x8_INTEL 0x1
+#define CLK_AVC_ME_MAJOR_8x16_INTEL 0x2
+#define CLK_AVC_ME_MAJOR_8x8_INTEL 0x3
+
+#define CLK_AVC_ME_MINOR_8x8_INTEL 0x0
+#define CLK_AVC_ME_MINOR_8x4_INTEL 0x1
+#define CLK_AVC_ME_MINOR_4x8_INTEL 0x2
+#define CLK_AVC_ME_MINOR_4x4_INTEL 0x3
+
+#define CLK_AVC_ME_MAJOR_FORWARD_INTEL 0x0
+#define CLK_AVC_ME_MAJOR_BACKWARD_INTEL 0x1
+#define CLK_AVC_ME_MAJOR_BIDIRECTIONAL_INTEL 0x2
+
+#define CLK_AVC_ME_PARTITION_MASK_ALL_INTEL 0x0
+#define CLK_AVC_ME_PARTITION_MASK_16x16_INTEL 0x7E
+#define CLK_AVC_ME_PARTITION_MASK_16x8_INTEL 0x7D
+#define CLK_AVC_ME_PARTITION_MASK_8x16_INTEL 0x7B
+#define CLK_AVC_ME_PARTITION_MASK_8x8_INTEL 0x77
+#define CLK_AVC_ME_PARTITION_MASK_8x4_INTEL 0x6F
+#define CLK_AVC_ME_PARTITION_MASK_4x8_INTEL 0x5F
+#define CLK_AVC_ME_PARTITION_MASK_4x4_INTEL 0x3F
+
+#define CLK_AVC_ME_SLICE_TYPE_PRED_INTEL 0x0
+#define CLK_AVC_ME_SLICE_TYPE_BPRED_INTEL 0x1
+#define CLK_AVC_ME_SLICE_TYPE_INTRA_INTEL 0x2
+
+#define CLK_AVC_ME_SEARCH_WINDOW_EXHAUSTIVE_INTEL 0x0
+#define CLK_AVC_ME_SEARCH_WINDOW_SMALL_INTEL 0x1
+#define CLK_AVC_ME_SEARCH_WINDOW_TINY_INTEL 0x2
+#define CLK_AVC_ME_SEARCH_WINDOW_EXTRA_TINY_INTEL 0x3
+#define CLK_AVC_ME_SEARCH_WINDOW_DIAMOND_INTEL 0x4
+#define CLK_AVC_ME_SEARCH_WINDOW_LARGE_DIAMOND_INTEL 0x5
+#define CLK_AVC_ME_SEARCH_WINDOW_RESERVED0_INTEL 0x6
+#define CLK_AVC_ME_SEARCH_WINDOW_RESERVED1_INTEL 0x7
+#define CLK_AVC_ME_SEARCH_WINDOW_CUSTOM_INTEL 0x8
+#define CLK_AVC_ME_SEARCH_WINDOW_16x12_RADIUS_INTEL 0x9
+#define CLK_AVC_ME_SEARCH_WINDOW_4x4_RADIUS_INTEL 0x2
+#define CLK_AVC_ME_SEARCH_WINDOW_2x2_RADIUS_INTEL 0xa
+
+#define CLK_AVC_ME_SAD_ADJUST_MODE_NONE_INTEL 0x0
+#define CLK_AVC_ME_SAD_ADJUST_MODE_HAAR_INTEL 0x2
+
+#define CLK_AVC_ME_SUBPIXEL_MODE_INTEGER_INTEL 0x0
+#define CLK_AVC_ME_SUBPIXEL_MODE_HPEL_INTEL 0x1
+#define CLK_AVC_ME_SUBPIXEL_MODE_QPEL_INTEL 0x3
+
+#define CLK_AVC_ME_COST_PRECISION_QPEL_INTEL 0x0
+#define CLK_AVC_ME_COST_PRECISION_HPEL_INTEL 0x1
+#define CLK_AVC_ME_COST_PRECISION_PEL_INTEL 0x2
+#define CLK_AVC_ME_COST_PRECISION_DPEL_INTEL 0x3
+
+#define CLK_AVC_ME_BIDIR_WEIGHT_QUARTER_INTEL 0x10
+#define CLK_AVC_ME

[PATCH] D51378: [OPENMP] Add support for nested 'declare target' directives

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

In https://reviews.llvm.org/D51378#1218184, @RaviNarayanaswamy wrote:

> We should just go with generating an error if the DeclareTargetNestingLevel 
> is not 0 at the end of compilation unit.  
>  Hard to detect if user accidentally forgot to have end declare in header 
> file and had it in the include file or it was intentional.


That will effectively forbid the legacy approach of doing

  #pragma omp declare target
  #include <...>
  #pragma omp end declare target

as in the test because `DeclareTargetNestingLevel` will be 1 throughout the 
header file. I think that's still relevant today, so the condition should be 
"has the same value as when entering this file".


Repository:
  rC Clang

https://reviews.llvm.org/D51378



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

[clang-tools-extra] r341057 - [clangd] Implement iterator cost

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Thu Aug 30 04:23:58 2018
New Revision: 341057

URL: http://llvm.org/viewvc/llvm-project?rev=341057&view=rev
Log:
[clangd] Implement iterator cost

This patch introduces iterator cost concept to improve the performance
of Dex query iterators (mainly, AND iterator). Benchmarks show that the
queries become ~10% faster.

Before

```
---
BenchmarkTime   CPU Iteration
---
DexAdHocQueries5883074 ns5883018 ns117
DexRealQ 959904457 ns  959898507 ns  1
```

After

```
---
BenchmarkTime   CPU Iteration
---
DexAdHocQueries5238403 ns5238361 ns130
DexRealQ 873275207 ns  873269453 ns  1
```

Reviewed by: sammccall

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

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

Modified: clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp?rev=341057&r1=341056&r2=341057&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp Thu Aug 30 04:23:58 
2018
@@ -48,6 +48,8 @@ public:
 
   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 @@ public:
 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 @@ public:
 });
   }
 
+  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 @@ private:
   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 @@ public:
   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 @@ public:
 });
   }
 
+  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 @@ public:
 
   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 @@ public:
 
   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 @@ public:
 return Child->consume();
   }
 
+  size_

[PATCH] D51310: [clangd] Implement iterator cost

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE341057: [clangd] Implement iterator cost (authored by 
omtcyfz, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51310?vs=163024&id=163297#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51310

Files:
  clangd/index/dex/Iterator.cpp
  clangd/index/dex/Iterator.h
  unittests/clangd/DexIndexTests.cpp

Index: clangd/index/dex/Iterator.cpp
===
--- clangd/index/dex/Iterator.cpp
+++ 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 @@
 return Child->consume();
   }
 
+  size_t estimateSize() const override {
+return std::min(Child->estimateSize(), Limit);
+  }
+
 private:
   llvm::raw_ostream &dump(llvm::raw_ostream &OS) const override {
 OS << "(LIMIT " << Limit << '(' << ItemsLeft << ") " << *Child << ')';
Index: clangd/index/dex/Iterator.h
===
--- clangd/index/dex/Iterator.h
+++ 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: unittests/clangd/DexIndexTests.cpp
===
--- unittests/clangd/DexIndexTests.cpp
+++ 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,

[PATCH] D51481: [clangd] Implement proximity path boosting for Dex

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

Some high-level comments :)




Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:45
+  void build(std::shared_ptr> Symbols,
+ llvm::ArrayRef URISchemes);
 

URI schemes are property of `Symbols`. We shouldn't need to pass specify the 
schemes again. Dex can collect all possible URI schemes when building the index.

I think we could generate proximity paths for index symbols without knowing 
about schemes (when building the index). For example, we could `canonicalize` 
the URI (as in `FileDistance.cpp`), for example, by converting `test:///x/y/z` 
to `/test:/x/y/z`.  For a query, we would first convert query proximity paths 
to URIs (of all possible schemes), perform the same canonicalization on URIs, 
and then use the canonicalized paths to compute proximity.



Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:64
 
-private:
+  /// For fuzzyFind() Dex retrieves getRetrievalItemsMultiplier() more items
+  /// than requested via Req.MaxCandidateCount in the first stage of filtering.

Why are values of multipliers interesting to users? Could these be 
implementation details in the cpp file?



Comment at: clang-tools-extra/clangd/index/dex/Token.h:99
+/// Boost starts with Count and decreases by 1 for each parent directory token.
+std::vector>
+generateQueryProximityPaths(llvm::StringRef AbsolutePath,

Note that `boost != up traversal distance` in most cases. We could return the 
distance here and let users decide what the exact boost score is.



Comment at: clang-tools-extra/unittests/clangd/DexIndexTests.cpp:33
 
+std::vector URISchemes = {"file"};
+

Use `unittest` scheme defined in TestFS.cpp


https://reviews.llvm.org/D51481



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

[PATCH] D51446: [OpenMP][bugfix] Add missing macros for Power

[Jonas Hahnfeld via Phabricator via cfe-commits]

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

Please also update the test.




Comment at: lib/Frontend/InitPreprocessor.cpp:1115-1130
   case llvm::Triple::ppc64:
+if (AuxTI.getLongDoubleWidth() == 128) {
+  Builder.defineMacro("__LONG_DOUBLE_128__");
+  Builder.defineMacro("__LONGDOUBLE128");
+}
 Builder.defineMacro("__powerpc64__");
 Builder.defineMacro("_CALL_ELF", "1");

I'd suggest to merge these two:
```lang=c++
  case llvm::Triple::ppc64:
  case llvm::Triple::ppc64le:
Builder.defineMacro("__powerpc64__");

StringRef ABI = AuxTI.getABI();
// Set _CALL_ELF macro needed for gnu/stubs.h
if (ABI == "elfv1" || ABI == "elfv1-qpx")
  Builder.defineMacro("_CALL_ELF", "1");
if (ABI == "elfv2")
  Builder.defineMacro("_CALL_ELF", "2");

// TODO: Add comment where this is needed and for what reason.
if (AuxTI.getLongDoubleWidth() == 128) {
  Builder.defineMacro("__LONG_DOUBLE_128__");
  Builder.defineMacro("__LONGDOUBLE128");
}
break;


Repository:
  rC Clang

https://reviews.llvm.org/D51446



___
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

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 163302.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

Ah, correct. I totally forgot about the `^string$` for the exact match.

This should not change behavior now. I believe what you propose would make the 
code easier (there'd only be `AbseilLibraries = {"absl/algorithm", ...}` and 
the final `return std::any_of(..., [](...) { return Path.find(Library) != 
StringRef::npos; });` but the performance would be worse (because of the 
presence of the `absl/` prefix in each entry). Would you consider this less 
efficient but easier-to-follow approach better?


https://reviews.llvm.org/D51360

Files:
  clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h


Index: clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
===
--- clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
+++ clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
@@ -6,9 +6,10 @@
 // License. See LICENSE.TXT for details.
 //
 
//===--===//
-//
+
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include 
 
 namespace clang {
 namespace ast_matchers {
@@ -28,23 +29,30 @@
 ///
 /// Usable as: Matcher, Matcher, Matcher,
 /// Matcher
-
-AST_POLYMORPHIC_MATCHER(isInAbseilFile,
-AST_POLYMORPHIC_SUPPORTED_TYPES(
-Decl, Stmt, TypeLoc, NestedNameSpecifierLoc)) {
+AST_POLYMORPHIC_MATCHER(
+isInAbseilFile, AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc,
+NestedNameSpecifierLoc)) {
   auto &SourceManager = Finder->getASTContext().getSourceManager();
   SourceLocation Loc = Node.getBeginLoc();
   if (Loc.isInvalid())
 return false;
   const FileEntry *FileEntry =
   SourceManager.getFileEntryForID(SourceManager.getFileID(Loc));
   if (!FileEntry)
 return false;
-  StringRef Filename = FileEntry->getName();
-  llvm::Regex RE(
-  "absl/(algorithm|base|container|debugging|memory|meta|numeric|strings|"
-  "synchronization|time|types|utility)");
-  return RE.match(Filename);
+  StringRef Path = FileEntry->getName();
+  const static llvm::SmallString<5> AbslPrefix("absl/");
+  size_t PrefixPosition = Path.find(AbslPrefix);
+  if (PrefixPosition == StringRef::npos)
+return false;
+  Path = Path.drop_front(PrefixPosition + AbslPrefix.size());
+  static const char *AbseilLibraries[] = {
+  "algorithm",   "base", "container", "debugging",
+  "memory",  "meta", "numeric",   "strings",
+  "synchronization", "time", "types", "utility"};
+  return std::any_of(
+  std::begin(AbseilLibraries), std::end(AbseilLibraries),
+  [&](const char *Library) { return Path.startswith(Library); });
 }
 
 } // namespace ast_matchers


Index: clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
===
--- clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
+++ clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
@@ -6,9 +6,10 @@
 // License. See LICENSE.TXT for details.
 //
 //===--===//
-//
+
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include 
 
 namespace clang {
 namespace ast_matchers {
@@ -28,23 +29,30 @@
 ///
 /// Usable as: Matcher, Matcher, Matcher,
 /// Matcher
-
-AST_POLYMORPHIC_MATCHER(isInAbseilFile,
-AST_POLYMORPHIC_SUPPORTED_TYPES(
-Decl, Stmt, TypeLoc, NestedNameSpecifierLoc)) {
+AST_POLYMORPHIC_MATCHER(
+isInAbseilFile, AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc,
+NestedNameSpecifierLoc)) {
   auto &SourceManager = Finder->getASTContext().getSourceManager();
   SourceLocation Loc = Node.getBeginLoc();
   if (Loc.isInvalid())
 return false;
   const FileEntry *FileEntry =
   SourceManager.getFileEntryForID(SourceManager.getFileID(Loc));
   if (!FileEntry)
 return false;
-  StringRef Filename = FileEntry->getName();
-  llvm::Regex RE(
-  "absl/(algorithm|base|container|debugging|memory|meta|numeric|strings|"
-  "synchronization|time|types|utility)");
-  return RE.match(Filename);
+  StringRef Path = FileEntry->getName();
+  const static llvm::SmallString<5> AbslPrefix("absl/");
+  size_t PrefixPosition = Path.find(AbslPrefix);
+  if (PrefixPosition == StringRef::npos)
+return false;
+  Path = Path.drop_front(PrefixPosition + AbslPrefix.size());
+  static const char *AbseilLibraries[] = {
+  "algorithm",   "base", "container", "debugging",
+  "memory",  "meta", "numeric",   "strings",
+  "synchronization", "time", "types", "utility"};
+  return std::any_of(
+  std::begin(AbseilLibraries), std::end(AbseilLibraries),
+  [&](const char *Library) { ret

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

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 163305.
kbobyrev marked 4 inline comments as done.
kbobyrev added a comment.
Herald added a subscriber: arphaman.

- Fix the `.py` (should be `.el` in the second case) typo
- Move piece about rename request to Clangd docs and advertise it htere
- Use better wording for Clang-Rename docs advertising Clangd


https://reviews.llvm.org/D51292

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


Index: clang-tools-extra/docs/clangd.rst
===
--- clang-tools-extra/docs/clangd.rst
+++ clang-tools-extra/docs/clangd.rst
@@ -108,6 +108,17 @@
 | Gen. Getters/Setters| No |   No |
 +-++--+
 
+
+Rename Limitations
+==
+
+:program:`Clangd` shares the renaming infrastructure with `Clang-Rename
+`_ - a standalone refactoring
+tool. Currently, :program:`Clangd` only supports renaming a symbol within a
+single source file (as opposed to :program:`clang-rename` which handles the
+whole translation unit), but in the future it will have much better support
+than the standalone tool.
+
 Getting Involved
 ==
 
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,20 @@
 -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 `_ shares the
+renaming infrastructure of clang-rename. Consider switching to 
:program:`clangd`
+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 +164,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/clangd.rst
===
--- clang-tools-extra/docs/clangd.rst
+++ clang-tools-extra/docs/clangd.rst
@@ -108,6 +108,17 @@
 | Gen. Getters/Setters| No |   No |
 +-++--+
 
+
+Rename Limitations
+==
+
+:program:`Clangd` shares the renaming infrastructure with `Clang-Rename
+`_ - a standalone refactoring
+tool. Currently, :program:`Clangd` only supports renaming a symbol within a
+single source file (as opposed to :program:`clang-rename` which handles the
+whole translation unit), but in the future it will have much better support
+than the standalone tool.
+
 Getting Involved
 ==
 
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,20 @@
 -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 `_ shares the
+renaming infrastructure of clang-rename. Consider switching to :program:`clangd`
+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

[PATCH] D51485: [Sema][NFC] Trivial cleanup in ActOnCallExpr

[Jan Korous via Phabricator via cfe-commits]

jkorous created this revision.
jkorous added reviewers: rsmith, doug.gregor, JDevlieghere, thegameg.
jkorous added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith.

Use logical or operator instead of a bool variable and if/else.


Repository:
  rC Clang

https://reviews.llvm.org/D51485

Files:
  lib/Sema/SemaExpr.cpp


Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5330,13 +5330,7 @@
 
 // Determine whether this is a dependent call inside a C++ template,
 // in which case we won't do any semantic analysis now.
-bool Dependent = false;
-if (Fn->isTypeDependent())
-  Dependent = true;
-else if (Expr::hasAnyTypeDependentArguments(ArgExprs))
-  Dependent = true;
-
-if (Dependent) {
+if (Fn->isTypeDependent() || Expr::hasAnyTypeDependentArguments(ArgExprs)) 
{
   if (ExecConfig) {
 return new (Context) CUDAKernelCallExpr(
 Context, Fn, cast(ExecConfig), ArgExprs,


Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5330,13 +5330,7 @@
 
 // Determine whether this is a dependent call inside a C++ template,
 // in which case we won't do any semantic analysis now.
-bool Dependent = false;
-if (Fn->isTypeDependent())
-  Dependent = true;
-else if (Expr::hasAnyTypeDependentArguments(ArgExprs))
-  Dependent = true;
-
-if (Dependent) {
+if (Fn->isTypeDependent() || Expr::hasAnyTypeDependentArguments(ArgExprs)) {
   if (ExecConfig) {
 return new (Context) CUDAKernelCallExpr(
 Context, Fn, cast(ExecConfig), ArgExprs,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51297: [docs] Create a guide for Vim users on how to setup Clangd

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 163306.
kbobyrev added a comment.

Now that there's a Wiki page on how to use Clangd with LanguageClient-neovim, 
we should just redirect (Neo)Vim users to that page.


https://reviews.llvm.org/D51297

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


Index: clang-tools-extra/docs/clangd.rst
===
--- clang-tools-extra/docs/clangd.rst
+++ clang-tools-extra/docs/clangd.rst
@@ -108,6 +108,17 @@
 | Gen. Getters/Setters| No |   No |
 +-++--+
 
+
+Vim Integration
+===
+
+One of the options of using :program:`Clangd` in :program:`vim` (or
+:program:`nvim`) is to utilize `LanguageClient-neovim
+`_ plugin. Please see the
+`Clangd Wiki page
+`_ for the
+instructions.
+
 Getting Involved
 ==
 


Index: clang-tools-extra/docs/clangd.rst
===
--- clang-tools-extra/docs/clangd.rst
+++ clang-tools-extra/docs/clangd.rst
@@ -108,6 +108,17 @@
 | Gen. Getters/Setters| No |   No |
 +-++--+
 
+
+Vim Integration
+===
+
+One of the options of using :program:`Clangd` in :program:`vim` (or
+:program:`nvim`) is to utilize `LanguageClient-neovim
+`_ plugin. Please see the
+`Clangd Wiki page
+`_ for the
+instructions.
+
 Getting Involved
 ==
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

- It would be useful for the C++ API (CodeCompletion struct) to provide the 
includes/edits in ranked order, if possible. Frontends could experiment with 
showing all the options.
- Still not sure that adding more complicated behavior to Code Complete (vs 
just improving ranking) is the right thing for now (bigger comment below)




Comment at: clangd/CodeComplete.cpp:303
+
+// Pick the most popular include, only if it has way more references than
+// the rest of includes; otherwise, we just give up and don't insert

I'm not sure about this:
 - we're adding new user-visible behavior which is potentially confusing (not 
inserting even though insertion is turned on and we're offering a completion 
that requires insertion)
 - it's not clear that first > 10 * second is the right heuristic or tuning
 - this probably isn't the right place for this heuristic - e.g. it causes us 
not to show the location of the symbol in the detail field. (IIRC, the fact 
that this is only shown when header insertion is active is a limitation we 
wanted to fix, not furither entrench).

Can we start with the simplest behavior, closest to what we do today: just 
choose the most popular include?

Probably the most forward-looking way to do this is to pick the winner when we 
construct the CompletionCandidate and stash it in a field there. (The scoring 
function is going to become more complex as we add proximity, and this function 
gets called twice. Also this function won't have access to the info needed for 
proximity).



Comment at: clangd/CodeComplete.cpp:316
+  });
+if (Includes.begin()->References <
+10 * std::next(Includes.begin())->References)

nit: Includes[0] and Includes[1]?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51291



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

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

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added a comment.

Thanks!

I don't have commit access, so it would be appreciated if someone could commit 
it.


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] D51438: [clangd] Run SignatureHelp using an up-to-date preamble, waiting if needed.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

The implementation is really simple. LG!
To get proper operation order in tests, we can wait in the diagnostics callback 
that runs on the worker thread (IIRC, we do that in some of the other tests 
too).




Comment at: clangd/TUScheduler.cpp:188
+  /// immediately, or later on the worker thread if it's not yet ready.
+  /// Threadsafe, but should not be called from the worker thread.
+  void getCurrentPreamble(

As discussed offline, it looks like it shouldn't be a problem to call it from 
the working thread, so we could probably remove this restriction from the 
comment.
Not that it's the right thing to do or that we have a use-case for that yet.



Comment at: clangd/TUScheduler.cpp:474
+llvm::unique_function)> Callback) 
{
+  if (RunSync)
+return Callback(getPossiblyStalePreamble());

It seems we could remove the special-casing of `RunSync` and still get correct 
results (the Requests queue is always empty).
But feel free to keep it for clarity.



Comment at: clangd/TUScheduler.h:123
+  /// 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.

Maybe use a strongly-typed enum outside the class?
`TUScheduler::Stale` will turn into `PreambleConsistency::Stale` at the 
call-site. The main upside is that it does not pollute completions inside the 
class with enumerators.

Just a suggestion, feel free to ignore.


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] D51485: [Sema][NFC] Trivial cleanup in ActOnCallExpr

[Jonas Devlieghere via Phabricator via cfe-commits]

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

Nice cleanup, LGTM.


Repository:
  rC Clang

https://reviews.llvm.org/D51485



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

[PATCH] D51481: [clangd] Implement proximity path boosting for Dex

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:45
+  void build(std::shared_ptr> Symbols,
+ llvm::ArrayRef URISchemes);
 

ioeric wrote:
> URI schemes are property of `Symbols`. We shouldn't need to pass specify the 
> schemes again. Dex can collect all possible URI schemes when building the 
> index.
> 
> I think we could generate proximity paths for index symbols without knowing 
> about schemes (when building the index). For example, we could `canonicalize` 
> the URI (as in `FileDistance.cpp`), for example, by converting 
> `test:///x/y/z` to `/test:/x/y/z`.  For a query, we would first convert query 
> proximity paths to URIs (of all possible schemes), perform the same 
> canonicalization on URIs, and then use the canonicalized paths to compute 
> proximity.
We had an offline discussion about URIs which might be interesting.

Fetching posting lists for all URI schemes at query time seems wasteful, 
@ilya-biryukov pointed out that in practice each file is going to exist in some 
preferred URI space, and we should only compare that one.
The only obstacle to doing that is that the preference order for URI schemes is 
not global, each place we need it it's accepted as configuration.

Maybe we should change that: as part of registering the URI schemes, we define 
a preferred order. And instead of the current operation `makeURI(File, scheme)` 
we should just offer APIs like `makePreferredURI(File)` and `makeFileURI(File)`.


https://reviews.llvm.org/D51481



___
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 accepted this revision.
hokein added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D51360#1219024, @kbobyrev wrote:

> Ah, correct. I totally forgot about the `^string$` for the exact match.
>
> This should not change behavior now. I believe what you propose would make 
> the code easier (there'd only be `AbseilLibraries = {"absl/algorithm", ...}` 
> and the final `return std::any_of(..., [](...) { return Path.find(Library) != 
> StringRef::npos; });` but the performance would be worse (because of the 
> presence of the `absl/` prefix in each entry). Would you consider this less 
> efficient but easier-to-follow approach better?


It was my suggestion. I'm fine with the current way as long as we have a clear 
comment explaining it.




Comment at: clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h:43
 return false;
-  StringRef Filename = FileEntry->getName();
-  llvm::Regex RE(
-  "absl/(algorithm|base|container|debugging|memory|meta|numeric|strings|"
-  "synchronization|time|types|utility)");
-  return RE.match(Filename);
+  StringRef Path = FileEntry->getName();
+  const static llvm::SmallString<5> AbslPrefix("absl/");

I think we should have a comment explaining we are matching 
`absl/[absl-libraries]` -- the current code is not as straightforward as 
before. 


https://reviews.llvm.org/D51360



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

[clang-tools-extra] r341060 - [clangd] Fix tests after rL341057

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Thu Aug 30 05:29:36 2018
New Revision: 341060

URL: http://llvm.org/viewvc/llvm-project?rev=341060&view=rev
Log:
[clangd] Fix tests after rL341057

Since OR iterator children are not longer sorted by the estimated size,
string representation should be different.

Modified:
clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp

Modified: clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp?rev=341060&r1=341059&r2=341060&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/DexIndexTests.cpp Thu Aug 30 
05:29:36 2018
@@ -259,7 +259,7 @@ TEST(DexIndexIterators, StringRepresenta
   createOr(create(L3), create(L4), create(L5)));
 
   EXPECT_EQ(llvm::to_string(*Nested),
-"(& (| [{END}] [0, {5}, END] [0, {1}, 5, END]) (& [{1}, 5, 7, 9, "
+"(& (| [0, {5}, END] [0, {1}, 5, END] [{END}]) (& [{1}, 5, 7, 9, "
 "END] [{1}, 3, 5, 8, 9, END]))");
 }
 


___
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

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 163309.
kbobyrev marked an inline comment as done.
kbobyrev added a comment.

Mention that the code actually performs `absl/(any|absl|library)` substring 
match.


https://reviews.llvm.org/D51360

Files:
  clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h


Index: clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
===
--- clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
+++ clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
@@ -6,9 +6,10 @@
 // License. See LICENSE.TXT for details.
 //
 
//===--===//
-//
+
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include 
 
 namespace clang {
 namespace ast_matchers {
@@ -28,23 +29,32 @@
 ///
 /// Usable as: Matcher, Matcher, Matcher,
 /// Matcher
-
-AST_POLYMORPHIC_MATCHER(isInAbseilFile,
-AST_POLYMORPHIC_SUPPORTED_TYPES(
-Decl, Stmt, TypeLoc, NestedNameSpecifierLoc)) {
+AST_POLYMORPHIC_MATCHER(
+isInAbseilFile, AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc,
+NestedNameSpecifierLoc)) {
   auto &SourceManager = Finder->getASTContext().getSourceManager();
   SourceLocation Loc = Node.getBeginLoc();
   if (Loc.isInvalid())
 return false;
   const FileEntry *FileEntry =
   SourceManager.getFileEntryForID(SourceManager.getFileID(Loc));
   if (!FileEntry)
 return false;
-  StringRef Filename = FileEntry->getName();
-  llvm::Regex RE(
-  "absl/(algorithm|base|container|debugging|memory|meta|numeric|strings|"
-  "synchronization|time|types|utility)");
-  return RE.match(Filename);
+  // Determine whether filepath contains "absl/[absl-library]" substring, where
+  // [absl-library] is AbseilLibraries list entry.
+  StringRef Path = FileEntry->getName();
+  const static llvm::SmallString<5> AbslPrefix("absl/");
+  size_t PrefixPosition = Path.find(AbslPrefix);
+  if (PrefixPosition == StringRef::npos)
+return false;
+  Path = Path.drop_front(PrefixPosition + AbslPrefix.size());
+  static const char *AbseilLibraries[] = {
+  "algorithm",   "base", "container", "debugging",
+  "memory",  "meta", "numeric",   "strings",
+  "synchronization", "time", "types", "utility"};
+  return std::any_of(
+  std::begin(AbseilLibraries), std::end(AbseilLibraries),
+  [&](const char *Library) { return Path.startswith(Library); });
 }
 
 } // namespace ast_matchers


Index: clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
===
--- clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
+++ clang-tools-extra/clang-tidy/abseil/AbseilMatcher.h
@@ -6,9 +6,10 @@
 // License. See LICENSE.TXT for details.
 //
 //===--===//
-//
+
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include 
 
 namespace clang {
 namespace ast_matchers {
@@ -28,23 +29,32 @@
 ///
 /// Usable as: Matcher, Matcher, Matcher,
 /// Matcher
-
-AST_POLYMORPHIC_MATCHER(isInAbseilFile,
-AST_POLYMORPHIC_SUPPORTED_TYPES(
-Decl, Stmt, TypeLoc, NestedNameSpecifierLoc)) {
+AST_POLYMORPHIC_MATCHER(
+isInAbseilFile, AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc,
+NestedNameSpecifierLoc)) {
   auto &SourceManager = Finder->getASTContext().getSourceManager();
   SourceLocation Loc = Node.getBeginLoc();
   if (Loc.isInvalid())
 return false;
   const FileEntry *FileEntry =
   SourceManager.getFileEntryForID(SourceManager.getFileID(Loc));
   if (!FileEntry)
 return false;
-  StringRef Filename = FileEntry->getName();
-  llvm::Regex RE(
-  "absl/(algorithm|base|container|debugging|memory|meta|numeric|strings|"
-  "synchronization|time|types|utility)");
-  return RE.match(Filename);
+  // Determine whether filepath contains "absl/[absl-library]" substring, where
+  // [absl-library] is AbseilLibraries list entry.
+  StringRef Path = FileEntry->getName();
+  const static llvm::SmallString<5> AbslPrefix("absl/");
+  size_t PrefixPosition = Path.find(AbslPrefix);
+  if (PrefixPosition == StringRef::npos)
+return false;
+  Path = Path.drop_front(PrefixPosition + AbslPrefix.size());
+  static const char *AbseilLibraries[] = {
+  "algorithm",   "base", "container", "debugging",
+  "memory",  "meta", "numeric",   "strings",
+  "synchronization", "time", "types", "utility"};
+  return std::any_of(
+  std::begin(AbseilLibraries), std::end(AbseilLibraries),
+  [&](const char *Library) { return Path.startswith(Library); });
 }
 
 } // namespace ast_matchers
___
cfe-commits mailing list
cfe-commits@li

Re: r341009 - Adjust Attr representation so that changes to documentation don't affect

[Aaron Ballman via cfe-commits]

On Wed, Aug 29, 2018 at 9:01 PM, Richard Smith via cfe-commits
 wrote:
> Author: rsmith
> Date: Wed Aug 29 18:01:07 2018
> New Revision: 341009
>
> URL: http://llvm.org/viewvc/llvm-project?rev=341009&view=rev
> Log:
> Adjust Attr representation so that changes to documentation don't affect
> how we parse source code.
>
> Instead of implicitly opting all undocumented attributes out of '#pragma
> clang attribute' support, explicitly opt them all out and remove the
> documentation check from TableGen.
>
> (No new attributes should be added without documentation, so this has
> little chance of backsliding. We already support the pragma on one
> undocumented attribute, so we don't even want to enforce our old
> "rule".)
>
> No functionality change intended.

Thank you for this, Richard. I think this is a good change.

~Aaron

>
> Modified:
> cfe/trunk/include/clang/Basic/Attr.td
> cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp
>
> Modified: cfe/trunk/include/clang/Basic/Attr.td
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=341009&r1=341008&r2=341009&view=diff
> ==
> --- cfe/trunk/include/clang/Basic/Attr.td (original)
> +++ cfe/trunk/include/clang/Basic/Attr.td Wed Aug 29 18:01:07 2018
> @@ -473,13 +473,12 @@ class Attr {
>// in a class template definition.
>bit MeaningfulToClassTemplateDefinition = 0;
>// Set to true if this attribute can be used with '#pragma clang 
> attribute'.
> -  // By default, when this value is false, an attribute is supported by the
> -  // '#pragma clang attribute' only when:
> -  // - It has documentation.
> +  // By default, an attribute is supported by the '#pragma clang attribute'
> +  // only when:
>// - It has a subject list whose subjects can be represented using subject
>//   match rules.
>// - It has GNU/CXX11 spelling and doesn't require delayed parsing.
> -  bit ForcePragmaAttributeSupport = 0;
> +  bit PragmaAttributeSupport;
>// Lists language options, one of which is required to be true for the
>// attribute to be applicable. If empty, no language options are required.
>list LangOpts = [];
> @@ -546,6 +545,7 @@ class IgnoredAttr : Attr {
>let ASTNode = 0;
>let SemaHandler = 0;
>let Documentation = [Undocumented];
> +  let PragmaAttributeSupport = 0;
>  }
>
>  //
> @@ -564,6 +564,7 @@ def AddressSpace : TypeAttr {
>let Spellings = [Clang<"address_space">];
>let Args = [IntArgument<"AddressSpace">];
>let Documentation = [Undocumented];
> +  let PragmaAttributeSupport = 0;
>  }
>
>  def Alias : Attr {
> @@ -571,6 +572,7 @@ def Alias : Attr {
>let Args = [StringArgument<"Aliasee">];
>let Subjects = SubjectList<[Function, GlobalVar], ErrorDiag>;
>let Documentation = [Undocumented];
> +  let PragmaAttributeSupport = 0;
>  }
>
>  def Aligned : InheritableAttr {
> @@ -583,6 +585,7 @@ def Aligned : InheritableAttr {
>Keyword<"_Alignas">]>,
> Accessor<"isDeclspec",[Declspec<"align">]>];
>let Documentation = [Undocumented];
> +  let PragmaAttributeSupport = 0;
>  }
>
>  def AlignValue : Attr {
> @@ -610,12 +613,14 @@ def AlignMac68k : InheritableAttr {
>let Spellings = [];
>let SemaHandler = 0;
>let Documentation = [Undocumented];
> +  let PragmaAttributeSupport = 0;
>  }
>
>  def AlwaysInline : InheritableAttr {
>let Spellings = [GCC<"always_inline">, Keyword<"__forceinline">];
>let Subjects = SubjectList<[Function]>;
>let Documentation = [Undocumented];
> +  let PragmaAttributeSupport = 0;
>  }
>
>  def Artificial : InheritableAttr {
> @@ -661,6 +666,7 @@ def AnalyzerNoReturn : InheritableAttr {
>// analyzer?
>let Spellings = [GNU<"analyzer_noreturn">];
>let Documentation = [Undocumented];
> +  let PragmaAttributeSupport = 0;
>  }
>
>  def Annotate : InheritableParamAttr {
> @@ -668,7 +674,7 @@ def Annotate : InheritableParamAttr {
>let Args = [StringArgument<"Annotation">];
>// Ensure that the annotate attribute can be used with
>// '#pragma clang attribute' even though it has no subject list.
> -  let ForcePragmaAttributeSupport = 1;
> +  let PragmaAttributeSupport = 1;
>let Documentation = [Undocumented];
>  }
>
> @@ -703,6 +709,7 @@ def AsmLabel : InheritableAttr {
>let Args = [StringArgument<"Label">];
>let SemaHandler = 0;
>let Documentation = [Undocumented];
> +  let PragmaAttributeSupport = 0;
>  }
>
>  def Availability : InheritableAttr {
> @@ -769,6 +776,7 @@ def Blocks : InheritableAttr {
>let Spellings = [Clang<"blocks">];
>let Args = [EnumArgument<"Type", "BlockType", ["byref"], ["ByRef"]>];
>let Documentation = [Undocumented];
> +  let PragmaAttributeSupport = 0;
>  }
>
>  def Bounded : IgnoredAttr {
> @@ -787,6 +795,7 @@ def CDecl : DeclOrTypeAttr {
>let Spellings = [GCC<"cdecl">, Keyword<"__cdecl">, Keyword<"_cdecl">];
>  //

[clang-tools-extra] r341061 - [clang-tidy] Use simple string matching instead of Regex

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Thu Aug 30 05:42:19 2018
New Revision: 341061

URL: http://llvm.org/viewvc/llvm-project?rev=341061&view=rev
Log:
[clang-tidy] Use simple string matching instead of Regex

Instead of parsing and compiling the `llvm::Regex` each time, it's
faster to use basic string matching for filename prefix check.

Reviewed by: hokein

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

Modified:
clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h

Modified: clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h?rev=341061&r1=341060&r2=341061&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h (original)
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h Thu Aug 30 
05:42:19 2018
@@ -6,9 +6,10 @@
 // License. See LICENSE.TXT for details.
 //
 
//===--===//
-//
+
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include 
 
 namespace clang {
 namespace ast_matchers {
@@ -28,10 +29,9 @@ namespace ast_matchers {
 ///
 /// Usable as: Matcher, Matcher, Matcher,
 /// Matcher
-
-AST_POLYMORPHIC_MATCHER(isInAbseilFile,
-AST_POLYMORPHIC_SUPPORTED_TYPES(
-Decl, Stmt, TypeLoc, NestedNameSpecifierLoc)) {
+AST_POLYMORPHIC_MATCHER(
+isInAbseilFile, AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc,
+NestedNameSpecifierLoc)) {
   auto &SourceManager = Finder->getASTContext().getSourceManager();
   SourceLocation Loc = Node.getBeginLoc();
   if (Loc.isInvalid())
@@ -40,11 +40,21 @@ AST_POLYMORPHIC_MATCHER(isInAbseilFile,
   SourceManager.getFileEntryForID(SourceManager.getFileID(Loc));
   if (!FileEntry)
 return false;
-  StringRef Filename = FileEntry->getName();
-  llvm::Regex RE(
-  "absl/(algorithm|base|container|debugging|memory|meta|numeric|strings|"
-  "synchronization|time|types|utility)");
-  return RE.match(Filename);
+  // Determine whether filepath contains "absl/[absl-library]" substring, where
+  // [absl-library] is AbseilLibraries list entry.
+  StringRef Path = FileEntry->getName();
+  const static llvm::SmallString<5> AbslPrefix("absl/");
+  size_t PrefixPosition = Path.find(AbslPrefix);
+  if (PrefixPosition == StringRef::npos)
+return false;
+  Path = Path.drop_front(PrefixPosition + AbslPrefix.size());
+  static const char *AbseilLibraries[] = {
+  "algorithm",   "base", "container", "debugging",
+  "memory",  "meta", "numeric",   "strings",
+  "synchronization", "time", "types", "utility"};
+  return std::any_of(
+  std::begin(AbseilLibraries), std::end(AbseilLibraries),
+  [&](const char *Library) { return Path.startswith(Library); });
 }
 
 } // namespace ast_matchers


___
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

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL341061: [clang-tidy] Use simple string matching instead of 
Regex (authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51360?vs=163309&id=163310#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51360

Files:
  clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h


Index: clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h
===
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h
@@ -6,9 +6,10 @@
 // License. See LICENSE.TXT for details.
 //
 
//===--===//
-//
+
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include 
 
 namespace clang {
 namespace ast_matchers {
@@ -28,23 +29,32 @@
 ///
 /// Usable as: Matcher, Matcher, Matcher,
 /// Matcher
-
-AST_POLYMORPHIC_MATCHER(isInAbseilFile,
-AST_POLYMORPHIC_SUPPORTED_TYPES(
-Decl, Stmt, TypeLoc, NestedNameSpecifierLoc)) {
+AST_POLYMORPHIC_MATCHER(
+isInAbseilFile, AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc,
+NestedNameSpecifierLoc)) {
   auto &SourceManager = Finder->getASTContext().getSourceManager();
   SourceLocation Loc = Node.getBeginLoc();
   if (Loc.isInvalid())
 return false;
   const FileEntry *FileEntry =
   SourceManager.getFileEntryForID(SourceManager.getFileID(Loc));
   if (!FileEntry)
 return false;
-  StringRef Filename = FileEntry->getName();
-  llvm::Regex RE(
-  "absl/(algorithm|base|container|debugging|memory|meta|numeric|strings|"
-  "synchronization|time|types|utility)");
-  return RE.match(Filename);
+  // Determine whether filepath contains "absl/[absl-library]" substring, where
+  // [absl-library] is AbseilLibraries list entry.
+  StringRef Path = FileEntry->getName();
+  const static llvm::SmallString<5> AbslPrefix("absl/");
+  size_t PrefixPosition = Path.find(AbslPrefix);
+  if (PrefixPosition == StringRef::npos)
+return false;
+  Path = Path.drop_front(PrefixPosition + AbslPrefix.size());
+  static const char *AbseilLibraries[] = {
+  "algorithm",   "base", "container", "debugging",
+  "memory",  "meta", "numeric",   "strings",
+  "synchronization", "time", "types", "utility"};
+  return std::any_of(
+  std::begin(AbseilLibraries), std::end(AbseilLibraries),
+  [&](const char *Library) { return Path.startswith(Library); });
 }
 
 } // namespace ast_matchers


Index: clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h
===
--- clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h
+++ clang-tools-extra/trunk/clang-tidy/abseil/AbseilMatcher.h
@@ -6,9 +6,10 @@
 // License. See LICENSE.TXT for details.
 //
 //===--===//
-//
+
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
+#include 
 
 namespace clang {
 namespace ast_matchers {
@@ -28,23 +29,32 @@
 ///
 /// Usable as: Matcher, Matcher, Matcher,
 /// Matcher
-
-AST_POLYMORPHIC_MATCHER(isInAbseilFile,
-AST_POLYMORPHIC_SUPPORTED_TYPES(
-Decl, Stmt, TypeLoc, NestedNameSpecifierLoc)) {
+AST_POLYMORPHIC_MATCHER(
+isInAbseilFile, AST_POLYMORPHIC_SUPPORTED_TYPES(Decl, Stmt, TypeLoc,
+NestedNameSpecifierLoc)) {
   auto &SourceManager = Finder->getASTContext().getSourceManager();
   SourceLocation Loc = Node.getBeginLoc();
   if (Loc.isInvalid())
 return false;
   const FileEntry *FileEntry =
   SourceManager.getFileEntryForID(SourceManager.getFileID(Loc));
   if (!FileEntry)
 return false;
-  StringRef Filename = FileEntry->getName();
-  llvm::Regex RE(
-  "absl/(algorithm|base|container|debugging|memory|meta|numeric|strings|"
-  "synchronization|time|types|utility)");
-  return RE.match(Filename);
+  // Determine whether filepath contains "absl/[absl-library]" substring, where
+  // [absl-library] is AbseilLibraries list entry.
+  StringRef Path = FileEntry->getName();
+  const static llvm::SmallString<5> AbslPrefix("absl/");
+  size_t PrefixPosition = Path.find(AbslPrefix);
+  if (PrefixPosition == StringRef::npos)
+return false;
+  Path = Path.drop_front(PrefixPosition + AbslPrefix.size());
+  static const char *AbseilLibraries[] = {
+  "algorithm",   "base", "container", "debugging",
+  "memory",  "meta", "numeric",   "strings",
+  "synchronization", "time", "types", "utility"};
+  return std::any_of(
+  std::begin(AbseilLibraries), std::end(AbseilLibraries),
+  

[PATCH] D50438: [clangd] Sort GoToDefinition results.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Sorry, I just realized I didn't reply the comments in the first review (they 
were in my draft).




Comment at: clangd/XRefs.cpp:71
 
+struct DeclInfo {
+  const Decl *D;

ilya-biryukov wrote:
> NIT: maybe call `Occurence` instead? As this is actually a `Decl` with some 
> extra data, computed based on the expression it originated from. Occurence 
> seems like a nice that for that kind of thing.
I'm not sure what's a good name here, but I think we'd better avoid using 
`Occurrence`, because this term is used by `xrefs` (and we will have xrefs 
implementation in this file).



Comment at: clangd/XRefs.cpp:105
+// Sort results. Declarations being referenced explicitly come first.
+std::sort(Result.begin(), Result.end(),
+  [](const DeclInfo &L, const DeclInfo &R) {

ilya-biryukov wrote:
> ilya-biryukov wrote:
> > Maybe sort further at the callers instead?
> > It would be a more intrusive change, but would also give a well-defined 
> > result order for findDefinitions and other cases. E.g. findDefinitions 
> > currently gives results in an arbitrary order (specifically, the one 
> > supplied by DenseMap+sort) when multiple decls are returned.
> > WDYT?
> Just to clarify the original suggestion.
> 
> Maybe we can sort the `(location, is_explicit)` pairs instead of the `(decl, 
> is_explicit)` pairs?
> Sorting based on pointer equality (see `L.D < R.D`) provides 
> non-deterministic results and we can have fully deterministic order on 
> locations.
> 
> DeclarationAndMacrosFinder can return the results in arbitrary orders and the 
> client code would be responsible for getting locations and finally sorting 
> them.
> WDYT?
I think we'd better sort them in `DeclarationAndMacrosFinder` -- because we 
have a few clients in `XRefs.cpp` using this class, and it seems that they 
don't have their specific needs for sorting, having them sorting results seems 
unnecessary.



Comment at: clangd/XRefs.cpp:128
+  void insertDecl(const Decl *D, bool IsExplicitReferenced) {
+auto It = Decls.find(D);
+if (It != Decls.end())

ilya-biryukov wrote:
> Maybe simplify to `Decls[D] |= IsExplicitReferenced`?
Good point!



Comment at: clangd/XRefs.cpp:297
+  for (const auto &DI : Symbols.Decls) {
+const auto *D = DI.D;
 // Fake key for symbols don't have USR (no SymbolID).

ilya-biryukov wrote:
> Maybe use explicit type here too or rename the field from 'D' to something 
> more descriptive (e.g. `Decl `)?
use explicit type here (I avoided using `Decl` as a variable name, because 
`Decl` is a type name in clang already).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50438



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

[PATCH] D51481: [clangd] Implement proximity path boosting for Dex

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:45
+  void build(std::shared_ptr> Symbols,
+ llvm::ArrayRef URISchemes);
 

sammccall wrote:
> ioeric wrote:
> > URI schemes are property of `Symbols`. We shouldn't need to pass specify 
> > the schemes again. Dex can collect all possible URI schemes when building 
> > the index.
> > 
> > I think we could generate proximity paths for index symbols without knowing 
> > about schemes (when building the index). For example, we could 
> > `canonicalize` the URI (as in `FileDistance.cpp`), for example, by 
> > converting `test:///x/y/z` to `/test:/x/y/z`.  For a query, we would first 
> > convert query proximity paths to URIs (of all possible schemes), perform 
> > the same canonicalization on URIs, and then use the canonicalized paths to 
> > compute proximity.
> We had an offline discussion about URIs which might be interesting.
> 
> Fetching posting lists for all URI schemes at query time seems wasteful, 
> @ilya-biryukov pointed out that in practice each file is going to exist in 
> some preferred URI space, and we should only compare that one.
> The only obstacle to doing that is that the preference order for URI schemes 
> is not global, each place we need it it's accepted as configuration.
> 
> Maybe we should change that: as part of registering the URI schemes, we 
> define a preferred order. And instead of the current operation `makeURI(File, 
> scheme)` we should just offer APIs like `makePreferredURI(File)` and 
> `makeFileURI(File)`.
That sounds like a good idea. 

We just need to be careful that indexes do not use non-preferred schemes (e.g. 
standalone indexer doesn't have the preferred scheme registered). This 
shouldn't be a problem in practice though.


https://reviews.llvm.org/D51481



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

[PATCH] D50438: [clangd] Sort GoToDefinition results.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 163311.
hokein marked 3 inline comments as done.
hokein added a comment.

Address review comments, return deterministic results.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50438

Files:
  clangd/XRefs.cpp
  unittests/clangd/XRefsTests.cpp

Index: unittests/clangd/XRefsTests.cpp
===
--- unittests/clangd/XRefsTests.cpp
+++ unittests/clangd/XRefsTests.cpp
@@ -311,6 +311,50 @@
   }
 }
 
+TEST(GoToDefinition, Rank) {
+  auto T = Annotations(R"cpp(
+struct $foo1[[Foo]] {
+  $foo2[[Foo]]();
+  $foo3[[Foo]](Foo&&);
+  $foo4[[Foo]](const char*);
+};
+
+Foo $f[[f]]();
+
+void $g[[g]](Foo foo);
+
+void call() {
+  const char* $str[[str]] = "123";
+  Foo a = $1^str;
+  Foo b = Foo($2^str);
+  Foo c = $3^f();
+  $4^g($5^f());
+  g($6^str);
+}
+  )cpp");
+  auto AST = TestTU::withCode(T.code()).build();
+  EXPECT_THAT(findDefinitions(AST, T.point("1")),
+  ElementsAre(RangeIs(T.range("str")), RangeIs(T.range("foo4";
+  EXPECT_THAT(findDefinitions(AST, T.point("2")),
+  ElementsAre(RangeIs(T.range("str";
+  EXPECT_THAT(findDefinitions(AST, T.point("3")),
+  ElementsAre(RangeIs(T.range("f")), RangeIs(T.range("foo3";
+  EXPECT_THAT(findDefinitions(AST, T.point("4")),
+  ElementsAre(RangeIs(T.range("g";
+  EXPECT_THAT(findDefinitions(AST, T.point("5")),
+  ElementsAre(RangeIs(T.range("f")),
+  RangeIs(T.range("foo3";
+
+  auto DefinitionAtPoint6 = findDefinitions(AST, T.point("6"));
+  EXPECT_EQ(3ul, DefinitionAtPoint6.size());
+  EXPECT_THAT(
+  DefinitionAtPoint6,
+  HasSubsequence(RangeIs(T.range("str")), RangeIs(T.range("foo4";
+  EXPECT_THAT(
+  DefinitionAtPoint6,
+  HasSubsequence(RangeIs(T.range("str")), RangeIs(T.range("foo3";
+}
+
 TEST(GoToDefinition, RelPathsInCompileCommand) {
   // The source is in "/clangd-test/src".
   // We build in "/clangd-test/build".
Index: clangd/XRefs.cpp
===
--- clangd/XRefs.cpp
+++ clangd/XRefs.cpp
@@ -68,10 +68,20 @@
   const MacroInfo *Info;
 };
 
+struct DeclInfo {
+  const Decl *D;
+  // Indicates the declaration is referenced by an explicit AST node.
+  bool IsReferencedExplicitly = false;
+};
+
 /// Finds declarations locations that a given source location refers to.
 class DeclarationAndMacrosFinder : public index::IndexDataConsumer {
-  std::vector Decls;
   std::vector MacroInfos;
+  // The value of the map indicates whether the declaration has been referenced
+  // explicitly in the code.
+  // True means the declaration is explicitly referenced at least once; false
+  // otherwise.
+  llvm::DenseMap Decls;
   const SourceLocation &SearchedLocation;
   const ASTContext *
   Preprocessor &PP;
@@ -82,13 +92,25 @@
  ASTContext &AST, Preprocessor &PP)
   : SearchedLocation(SearchedLocation), AST(AST), PP(PP) {}
 
-  std::vector takeDecls() {
-// Don't keep the same declaration multiple times.
-// This can happen when nodes in the AST are visited twice.
-std::sort(Decls.begin(), Decls.end());
-auto Last = std::unique(Decls.begin(), Decls.end());
-Decls.erase(Last, Decls.end());
-return std::move(Decls);
+  // Get all DeclInfo of the found declarations.
+  // The results are sorted by "IsReferencedExplicitly" and declaration
+  // location.
+  std::vector getFoundDecls() const {
+std::vector Result;
+for (auto It : Decls) {
+  Result.emplace_back();
+  Result.back().D = It.first;
+  Result.back().IsReferencedExplicitly = It.second;
+}
+
+// Sort results. Declarations being referenced explicitly come first.
+std::sort(Result.begin(), Result.end(),
+  [](const DeclInfo &L, const DeclInfo &R) {
+if (L.IsReferencedExplicitly != R.IsReferencedExplicitly)
+  return L.IsReferencedExplicitly > R.IsReferencedExplicitly;
+return L.D->getBeginLoc() < R.D->getBeginLoc();
+  });
+return Result;
   }
 
   std::vector takeMacroInfos() {
@@ -112,15 +134,30 @@
   SourceLocation Loc,
   index::IndexDataConsumer::ASTNodeInfo ASTNode) override {
 if (Loc == SearchedLocation) {
+  // Check whether the E has an implicit AST node (e.g. ImplicitCastExpr).
+  auto hasImplicitExpr = [](const Expr *E) {
+if (!E || E->child_begin() == E->child_end())
+  return false;
+// Use the first child is good enough for most cases -- normally the
+// expression returned by handleDeclOccurence contains exactly one
+// child expression.
+const auto *FirstChild = *E->child_begin();
+return llvm::isa(FirstChild) ||
+   llvm::isa(FirstChild) ||
+   llvm::isa(

[PATCH] D51475: [clangd] Load YAML static index asynchronously.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/tool/ClangdMain.cpp:86
+  return nullptr;
+Index = AsyncLoad.get();
+return Index.get();

I believe is a data race (multiple threads may run this line concurrently).
You would want some synchronization around this, `std::shared_future` could be 
a good fit


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51475



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

[PATCH] D51475: [clangd] Load YAML static index asynchronously.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Any reason to not just wait for the index to load? Is this a UX concern or a 
problem when experimenting?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51475



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

r341063 - [CodeComplete] Report location of opening parens for signature help

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu Aug 30 06:08:03 2018
New Revision: 341063

URL: http://llvm.org/viewvc/llvm-project?rev=341063&view=rev
Log:
[CodeComplete] Report location of opening parens for signature help

Summary: Used in clangd.

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: ioeric, kadircet, cfe-commits

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

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

Modified: cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h?rev=341063&r1=341062&r2=341063&view=diff
==
--- cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h (original)
+++ cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h Thu Aug 30 06:08:03 2018
@@ -1114,9 +1114,13 @@ public:
   /// \param Candidates an array of overload candidates.
   ///
   /// \param NumCandidates the number of overload candidates
+  ///
+  /// \param OpenParLoc location of the opening parenthesis of the argument
+  ///list.
   virtual void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
  OverloadCandidate *Candidates,
- unsigned NumCandidates) {}
+ unsigned NumCandidates,
+ SourceLocation OpenParLoc) {}
   //@}
 
   /// Retrieve the allocator that will be used to allocate
@@ -1166,7 +1170,8 @@ public:
 
   void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
  OverloadCandidate *Candidates,
- unsigned NumCandidates) override;
+ unsigned NumCandidates,
+ SourceLocation OpenParLoc) override;
 
   bool isResultFilteredOut(StringRef Filter, CodeCompletionResult Results) 
override;
 

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=341063&r1=341062&r2=341063&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Aug 30 06:08:03 2018
@@ -10230,9 +10230,11 @@ public:
   const VirtSpecifiers *VS = nullptr);
   void CodeCompleteBracketDeclarator(Scope *S);
   void CodeCompleteCase(Scope *S);
-  void CodeCompleteCall(Scope *S, Expr *Fn, ArrayRef Args);
+  void CodeCompleteCall(Scope *S, Expr *Fn, ArrayRef Args,
+SourceLocation OpenParLoc);
   void CodeCompleteConstructor(Scope *S, QualType Type, SourceLocation Loc,
-   ArrayRef Args);
+   ArrayRef Args,
+   SourceLocation OpenParLoc);
   void CodeCompleteInitializer(Scope *S, Decl *D);
   void CodeCompleteReturn(Scope *S);
   void CodeCompleteAfterIf(Scope *S);

Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=341063&r1=341062&r2=341063&view=diff
==
--- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
+++ cfe/trunk/lib/Frontend/ASTUnit.cpp Thu Aug 30 06:08:03 2018
@@ -1911,8 +1911,10 @@ namespace {
 
 void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
OverloadCandidate *Candidates,
-   unsigned NumCandidates) override {
-  Next.ProcessOverloadCandidates(S, CurrentArg, Candidates, NumCandidates);
+   unsigned NumCandidates,
+   SourceLocation OpenParLoc) override {
+  Next.ProcessOverloadCandidates(S, CurrentArg, Candidates, NumCandidates,
+ OpenParLoc);
 }
 
 CodeCompletionAllocator &getAllocator() override {

Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=341063&r1=341062&r2=341063&view=diff
==
--- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
+++ cfe/trunk/lib/Parse/ParseDecl.cpp Thu Aug 30 06:08:03 2018
@@ -2304,7 +2304,7 @@ Decl *Parser::ParseDeclarationAfterDecla
 auto ConstructorCompleter = [&, ThisVar

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

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL341063: [CodeComplete] Report location of opening parens for 
signature help (authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D51436

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

Index: cfe/trunk/tools/libclang/CIndexCodeCompletion.cpp
===
--- cfe/trunk/tools/libclang/CIndexCodeCompletion.cpp
+++ cfe/trunk/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: cfe/trunk/include/clang/Sema/Sema.h
===
--- cfe/trunk/include/clang/Sema/Sema.h
+++ cfe/trunk/include/clang/Sema/Sema.h
@@ -10230,9 +10230,11 @@
   const VirtSpecifiers *VS = nullptr);
   void CodeCompleteBracketDeclarator(Scope *S);
   void CodeCompleteCase(Scope *S);
-  void CodeCompleteCall(Scope *S, Expr *Fn, ArrayRef Args);
+  void CodeCompleteCall(Scope *S, Expr *Fn, ArrayRef Args,
+SourceLocation OpenParLoc);
   void CodeCompleteConstructor(Scope *S, QualType Type, SourceLocation Loc,
-   ArrayRef Args);
+   ArrayRef Args,
+   SourceLocation OpenParLoc);
   void CodeCompleteInitializer(Scope *S, Decl *D);
   void CodeCompleteReturn(Scope *S);
   void CodeCompleteAfterIf(Scope *S);
Index: cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
===
--- cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
+++ cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h
@@ -1114,9 +1114,13 @@
   /// \param Candidates an array of overload candidates.
   ///
   /// \param NumCandidates the number of overload candidates
+  ///
+  /// \param OpenParLoc location of the opening parenthesis of the argument
+  ///list.
   virtual void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
  OverloadCandidate *Candidates,
- unsigned NumCandidates) {}
+ unsigned NumCandidates,
+ SourceLocation OpenParLoc) {}
   //@}
 
   /// Retrieve the allocator that will be used to allocate
@@ -1166,7 +1170,8 @@
 
   void ProcessOverloadCandidates(Sema &S, unsigned CurrentArg,
  OverloadCandidate *Candidates,
- unsigned NumCandidates) override;
+ unsigned NumCandidates,
+ SourceLocation OpenParLoc) override;
 
   bool isResultFilteredOut(StringRef Filter, CodeCompletionResult Results) override;
 
Index: cfe/trunk/test/CodeCompletion/paren_locs.cpp
===
--- cfe/trunk/test/CodeCompletion/paren_locs.cpp
+++ cfe/trunk/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-C

[clang-tools-extra] r341065 - [clangd] Report position of opening paren in singature help

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu Aug 30 06:14:31 2018
New Revision: 341065

URL: http://llvm.org/viewvc/llvm-project?rev=341065&view=rev
Log:
[clangd] Report position of opening paren in singature help

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

Reviewers: sammccall

Reviewed By: sammccall

Subscribers: ioeric, MaskRay, jkorous, arphaman, kadircet, cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/CodeComplete.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp

Modified: clang-tools-extra/trunk/clangd/CodeComplete.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CodeComplete.cpp?rev=341065&r1=341064&r2=341065&view=diff
==
--- clang-tools-extra/trunk/clangd/CodeComplete.cpp (original)
+++ clang-tools-extra/trunk/clangd/CodeComplete.cpp Thu Aug 30 06:14:31 2018
@@ -794,7 +794,17 @@ public:
 
   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);

Modified: clang-tools-extra/trunk/clangd/Protocol.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Protocol.h?rev=341065&r1=341064&r2=341065&view=diff
==
--- clang-tools-extra/trunk/clangd/Protocol.h (original)
+++ clang-tools-extra/trunk/clangd/Protocol.h Thu Aug 30 06:14:31 2018
@@ -828,6 +828,13 @@ struct SignatureHelp {
 
   /// The active parameter of the active signature.
   int activeParameter = 0;
+
+  /// Position of the start of the argument list, including opening paren. e.g.
+  /// foo("first arg",   "second arg",
+  ///^-argListStart   ^-cursor
+  /// 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 &);
 

Modified: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp?rev=341065&r1=341064&r2=341065&view=diff
==
--- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp Thu Aug 30 
06:14:31 2018
@@ -825,8 +825,7 @@ TEST(CompletionTest, IgnoreCompleteInExc
 
   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 @@ SignatureHelp signatures(StringRef Text,
 
   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,54 @@ TEST(SignatureHelpTest, ActiveArg) {
   EXPECT_EQ(1, Results.activeParameter);
 }
 
+TEST(SignatureHelpTest, OpeningParen) {
+  llvm::StringLiteral Tests[] = {// Recursive function call.
+ R"cpp(
+int foo(int a, int b, int c);
+int main() {
+  foo(foo $p^( foo(10, 10, 10), ^ )));
+})cpp",
+ // Functional type cast.
+ R"cpp(
+struct Foo {
+  Foo(int a, int b, int c);
+};
+int main() {
+  Foo $p^( 10, ^ );
+})cpp",
+ // New expression.
+ R"cpp(
+struct Foo {
+  Foo(int a, int b, int c);
+};
+int main() {
+  new Foo $p^( 10, ^ );
+})cpp",
+ // Macro expansion.

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

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE341065: [clangd] Report position of opening paren in 
singature help (authored by ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51437?vs=163286&id=163317#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51437

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

Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -828,6 +828,13 @@
 
   /// The active parameter of the active signature.
   int activeParameter = 0;
+
+  /// Position of the start of the argument list, including opening paren. e.g.
+  /// foo("first arg",   "second arg",
+  ///^-argListStart   ^-cursor
+  /// 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);
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,54 @@
   EXPECT_EQ(1, Results.activeParameter);
 }
 
+TEST(SignatureHelpTest, OpeningParen) {
+  llvm::StringLiteral Tests[] = {// Recursive function call.
+ R"cpp(
+int foo(int a, int b, int c);
+int main() {
+  foo(foo $p^( foo(10, 10, 10), ^ )));
+})cpp",
+ // Functional type cast.
+ R"cpp(
+struct Foo {
+  Foo(int a, int b, int c);
+};
+int main() {
+  Foo $p^( 10, ^ );
+})cpp",
+ // New expression.
+ R"cpp(
+struct Foo {
+  Foo(int a, int b, int c);
+};
+int main() {
+  new Foo $p^( 10, ^ );
+})cpp",
+ // Macro expansion.
+ R"cpp(
+int foo(int a, int b, int c);
+#define FOO foo(
+
+int main() {
+  // Macro expansions.
+  $p^FOO 10, ^ );
+})cpp",
+ // Macro arguments.
+ R"cpp(
+int foo(int a, int b, int c);
+int main() {
+#define ID(X) X
+  ID(foo $p^( foo(10), ^ ))
+})cpp"};
+
+  for (auto Test : Tests) {
+Annotations Code(Test);
+EXPECT_EQ(signatures(Code.code(), Code.point()).argListStart,
+  Code.point("p"))
+<< "Test source:" << Test;
+  }
+}
+
 class IndexRequestCollector : public SymbolIndex {
 public:
   bool
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D50246: [RISCV] Add support for computing sysroot for riscv32-unknown-elf

[Alex Bradbury via Phabricator via cfe-commits]

asb accepted this revision.
asb added a comment.

Looks good to me, thanks!


Repository:
  rC Clang

https://reviews.llvm.org/D50246



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

[clang-tools-extra] r341066 - [clangd] Remove UB introduced in rL341057

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Thu Aug 30 06:30:34 2018
New Revision: 341066

URL: http://llvm.org/viewvc/llvm-project?rev=341066&view=rev
Log:
[clangd] Remove UB introduced in rL341057

Modified:
clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp

Modified: clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp?rev=341066&r1=341065&r2=341066&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp Thu Aug 30 06:30:34 
2018
@@ -193,7 +193,6 @@ public:
   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.


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

[PATCH] D51446: [OpenMP][bugfix] Add missing macros for Power

[Kelvin Li via Phabricator via cfe-commits]

kkwli0 added a comment.

Need to update the test too?


Repository:
  rC Clang

https://reviews.llvm.org/D51446



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

[PATCH] D51475: [clangd] Load YAML static index asynchronously.

[Eric Liu via Phabricator via cfe-commits]

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

- Fix data race.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51475

Files:
  clangd/tool/ClangdMain.cpp

Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -14,6 +14,8 @@
 #include "index/SymbolYAML.h"
 #include "index/dex/DexIndex.h"
 #include "clang/Basic/Version.h"
+#include "clang/Basic/Version.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
@@ -23,6 +25,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -39,22 +42,86 @@
 
 enum class PCHStorageFlag { Disk, Memory };
 
-// Build an in-memory static index for global symbols from a YAML-format file.
-// The size of global symbols should be relatively small, so that all symbols
-// can be managed in memory.
-std::unique_ptr buildStaticIndex(llvm::StringRef YamlSymbolFile) {
-  auto Buffer = llvm::MemoryBuffer::getFile(YamlSymbolFile);
-  if (!Buffer) {
-llvm::errs() << "Can't open " << YamlSymbolFile << "\n";
-return nullptr;
+// Loads the index asynchornously. This acts like an empty index before
+// finishing loading and proxies index requests to the loaded index after
+// loading.
+class AsyncLoadIndex : public SymbolIndex {
+public:
+  AsyncLoadIndex(
+  llvm::unique_function()> LoadIndex)
+  : AsyncLoad(runAsync(std::move(LoadIndex))) {}
+
+  bool
+  fuzzyFind(const FuzzyFindRequest &Req,
+llvm::function_ref Callback) const override {
+if (!index())
+  return false; // More
+return index()->fuzzyFind(Req, Callback);
+  }
+
+  void
+  lookup(const LookupRequest &Req,
+ llvm::function_ref Callback) const override {
+if (!index())
+  return;
+return index()->lookup(Req, Callback);
+  }
+
+  void findOccurrences(const OccurrencesRequest &Req,
+   llvm::function_ref
+   Callback) const override {
+if (!index())
+  return;
+return index()->findOccurrences(Req, Callback);
   }
-  auto Slab = symbolsFromYAML(Buffer.get()->getBuffer());
-  SymbolSlab::Builder SymsBuilder;
-  for (auto Sym : Slab)
-SymsBuilder.insert(Sym);
 
-  return UseDex ? dex::DexIndex::build(std::move(SymsBuilder).build())
-: MemIndex::build(std::move(SymsBuilder).build());
+  size_t estimateMemoryUsage() const override { return 0; }
+
+private:
+  const SymbolIndex *index() const {
+if (Index) // Index doesn't change once initialized, so no need for mutex.
+  return Index.get();
+
+std::lock_guard Lock(Mutex);
+if (Index) // Make sure Index has not been set by the last mutex owner.
+  return Index.get();
+if (AsyncLoad.wait_for(std::chrono::seconds(0)) !=
+std::future_status::ready)
+  return nullptr;
+
+Index = AsyncLoad.get();
+return Index.get();
+  }
+  mutable std::unique_ptr Index;
+  mutable std::future> AsyncLoad;
+  mutable std::mutex Mutex;
+};
+// Asynchronously build an in-memory static index for global symbols from a
+// YAML-format file. The size of global symbols should be relatively small, so
+// that all symbols can be managed in memory.
+std::unique_ptr buildStaticIndex(llvm::StringRef YamlSymbolFile) {
+  return llvm::make_unique(
+  [YamlSymbolFile]() -> std::unique_ptr {
+trace::Span Tracer("Build static index");
+
+auto Buffer = llvm::MemoryBuffer::getFile(YamlSymbolFile);
+if (!Buffer) {
+  llvm::errs() << "Can't open " << YamlSymbolFile << "\n";
+  return nullptr;
+}
+
+SymbolSlab::Builder SymsBuilder;
+{
+  trace::Span Tracer("YAML to symbols");
+  auto Slab = symbolsFromYAML(Buffer.get()->getBuffer());
+  for (auto Sym : Slab)
+SymsBuilder.insert(Sym);
+}
+
+trace::Span Build("Build index");
+return UseDex ? dex::DexIndex::build(std::move(SymsBuilder).build())
+  : MemIndex::build(std::move(SymsBuilder).build());
+  });
 }
 
 } // namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51475: [clangd] Load YAML static index asynchronously.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

In https://reviews.llvm.org/D51475#1219133, @ilya-biryukov wrote:

> Any reason to not just wait for the index to load? Is this a UX concern or a 
> problem when experimenting?


The index loading can be slow. When using LLVM YAML index, I need to wait for 
>10s before clangd starts giving me anything useful. We could potentially speed 
up loading (e.g. replacing yaml), but the index can be arbitrary large. I think 
it's an improvement in general to be able to get clangd running before index is 
loaded.




Comment at: clangd/tool/ClangdMain.cpp:86
+  return nullptr;
+Index = AsyncLoad.get();
+return Index.get();

ilya-biryukov wrote:
> I believe is a data race (multiple threads may run this line concurrently).
> You would want some synchronization around this, `std::shared_future` could 
> be a good fit
Nice catch. 

I am a bit hesitated about using `shared_future` though. It could potentially 
get rid of the mutex but would require much more careful use. For example, 
`Index` can be set to the same value repeatedly, which makes me a bit nervous.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51475



___
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 163322.
sammccall marked 2 inline comments as done.
sammccall added a comment.
Herald added a subscriber: jfb.

Add test, address comments.


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
@@ -22,6 +22,7 @@
 using ::testing::_;
 using ::testing::AnyOf;
 using ::testing::Each;
+using ::testing::ElementsAre;
 using ::testing::Pair;
 using ::testing::Pointee;
 using ::testing::UnorderedElementsAre;
@@ -63,7 +64,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 +76,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);
 }
@@ -148,6 +151,64 @@
   EXPECT_EQ(2, CallbackCount);
 }
 
+static std::vector includes(const PreambleData *Preamble) {
+  std::vector Result;
+  if (Preamble)
+for (const auto &Inclusion : Preamble->Includes.MainFileIncludes)
+  Result.push_back(Inclusion.Written);
+  return Result;
+}
+
+TEST_F(TUSchedulerTests, PreambleConsistency) {
+  std::atomic CallbackCount(0);
+  {
+Notification InconsistentReadDone; // Must live longest.
+TUScheduler S(
+getDefaultAsyncThreadsCount(), /*StorePreamblesInMemory=*/true,
+noopParsingCallbacks(),
+/*UpdateDebounce=*/std::chrono::steady_clock::duration::zero(),
+ASTRetentionPolicy());
+auto Path = testPath("foo.cpp");
+// Schedule two updates (A, B) and two preamble reads (stale, consistent).
+// The stale read should see A, and the consistent read should see B.
+// (We recognize the preambles by their included files).
+S.update(Path, getInputs(Path, "#include "), WantDiagnostics::Yes,
+ [&](std::vector Diags) {
+   // This callback runs in between the two preamble updates.
+
+   // This blocks update B, preventing it from winning the race
+   // against the stale read.
+   // If the first read was instead consistent, this would deadlock.
+   InconsistentReadDone.wait();
+   // This delays update B, preventing it from winning a race
+   // against the consistent read. The consistent read sees B
+   // only because it waits for it.
+   // If the second read was stale, it would usually see A.
+   std::this_thread::sleep_for(std::chrono::milliseconds(100));
+ });
+S.update(Path, getInputs(Path, "#include "), WantDiagnostics::Yes,
+ [&](std::vector Diags) {});
+
+S.runWithPreamble("StaleRead", Path, TUScheduler::Stale,
+  [&](llvm::Expected Pre) {
+ASSERT_TRUE(bool(Pre));
+assert(bool(Pre));
+EXPECT_THAT(includes(Pre->Preamble),
+ElementsAre(""));
+InconsistentReadDone.notify();
+++CallbackCount;
+  });
+S.runWithPreamble("ConsistentRead", Path, TUScheduler::Consistent,
+  [&](llvm::Expected Pre) {
+ASSERT_TRUE(bool(Pre));
+EXPECT_THAT(includes(Pre->Preamble),
+ElementsAre(""));
+++CallbackCount;
+  });
+  }
+  EXPECT_EQ(2, CallbackCount);
+}
+
 TEST_F(TUSchedulerTests, ManyUpdates) {
   const int FilesCount = 3;
   const int UpdatesPerFile = 10;
@@ -229,7 +290,7 

[PATCH] D51475: [clangd] Load YAML static index asynchronously.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D51475#1219184, @ioeric wrote:

> The index loading can be slow. When using LLVM YAML index, I need to wait for 
> >10s before clangd starts giving me anything useful. We could potentially 
> speed up loading (e.g. replacing yaml), but the index can be arbitrary large. 
> I think it's an improvement in general to be able to get clangd running 
> before index is loaded.


I would trade-off those 10 seconds for giving consistent experience (i.e. 
avoiding confusing the users with different modes of completion (with and 
without index, etc.)).
But not terribly opposed to that.




Comment at: clangd/tool/ClangdMain.cpp:86
+  return nullptr;
+Index = AsyncLoad.get();
+return Index.get();

ioeric wrote:
> ilya-biryukov wrote:
> > I believe is a data race (multiple threads may run this line concurrently).
> > You would want some synchronization around this, `std::shared_future` could 
> > be a good fit
> Nice catch. 
> 
> I am a bit hesitated about using `shared_future` though. It could potentially 
> get rid of the mutex but would require much more careful use. For example, 
> `Index` can be set to the same value repeatedly, which makes me a bit nervous.
The following pattern should give proper results:
```
class AsyncIndex : public Index {
public:
  AsyncIndex(std::shared_future IndexFut);
  //
private;
  const SymbolIndex *index() const {
 if (IndexFut.wait(0) != ready)
   return nullptr;
 return &IndexFut.get();
  }

  std::shared_futureIndexFut;
};


AsyncIndex AI(std::async([]() { /* load the index here */ return Index; });
```


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51475



___
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.

[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:
  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] D51475: [clangd] Load YAML static index asynchronously.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

In https://reviews.llvm.org/D51475#1219197, @ilya-biryukov wrote:

> In https://reviews.llvm.org/D51475#1219184, @ioeric wrote:
>
> > The index loading can be slow. When using LLVM YAML index, I need to wait 
> > for >10s before clangd starts giving me anything useful. We could 
> > potentially speed up loading (e.g. replacing yaml), but the index can be 
> > arbitrary large. I think it's an improvement in general to be able to get 
> > clangd running before index is loaded.
>
>
> I would trade-off those 10 seconds for giving consistent experience (i.e. 
> avoiding confusing the users with different modes of completion (with and 
> without index, etc.)).
>  But not terribly opposed to that.


I think the inconsistency is benign and should be worth 10 seconds (which for 
me personally is loong...). Besides, this is just LLVM index; the loading could 
be arbitrary long depending on the corpus size.




Comment at: clangd/tool/ClangdMain.cpp:86
+  return nullptr;
+Index = AsyncLoad.get();
+return Index.get();

ilya-biryukov wrote:
> ioeric wrote:
> > ilya-biryukov wrote:
> > > I believe is a data race (multiple threads may run this line 
> > > concurrently).
> > > You would want some synchronization around this, `std::shared_future` 
> > > could be a good fit
> > Nice catch. 
> > 
> > I am a bit hesitated about using `shared_future` though. It could 
> > potentially get rid of the mutex but would require much more careful use. 
> > For example, `Index` can be set to the same value repeatedly, which makes 
> > me a bit nervous.
> The following pattern should give proper results:
> ```
> class AsyncIndex : public Index {
> public:
>   AsyncIndex(std::shared_future IndexFut);
>   //
> private;
>   const SymbolIndex *index() const {
>  if (IndexFut.wait(0) != ready)
>return nullptr;
>  return &IndexFut.get();
>   }
> 
>   std::shared_futureIndexFut;
> };
> 
> 
> AsyncIndex AI(std::async([]() { /* load the index here */ return Index; });
> ```
My concern with this approach is that we are still calling `wait` even though 
it's not necessary once Index has been loaded.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51475



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

[PATCH] D51434: [HIP] Add -fvisibility hidden option to clang

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: lib/Driver/ToolChains/HIP.cpp:256
+CC1Args.append({"-fvisibility", "hidden"});
 }
 

arsenm wrote:
> We should probably start subclassing the HIP toolchain from AMDGPU and share 
> more of this
I'd like to defer the refactoring of HIP and AMDGPU toolchain to another patch. 
There are stuff in HIP toolchain which should go to AMDGPU toolchain. We need 
to sort out the relation between HIP and AMDGPU toolchain in that patch.



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] D51434: [HIP] Add -fvisibility hidden option to clang

[Matt Arsenault via Phabricator via cfe-commits]

arsenm accepted this revision.
arsenm added inline comments.
This revision is now accepted and ready to land.



Comment at: lib/Driver/ToolChains/HIP.cpp:256
+CC1Args.append({"-fvisibility", "hidden"});
 }
 

yaxunl wrote:
> arsenm wrote:
> > We should probably start subclassing the HIP toolchain from AMDGPU and 
> > share more of this
> I'd like to defer the refactoring of HIP and AMDGPU toolchain to another 
> patch. There are stuff in HIP toolchain which should go to AMDGPU toolchain. 
> We need to sort out the relation between HIP and AMDGPU toolchain in that 
> patch.
> 
Yes


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] D51434: [HIP] Add -fvisibility hidden option to clang

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added inline comments.



Comment at: lib/Driver/ToolChains/HIP.cpp:256
+CC1Args.append({"-fvisibility", "hidden"});
 }
 

arsenm wrote:
> yaxunl wrote:
> > arsenm wrote:
> > > We should probably start subclassing the HIP toolchain from AMDGPU and 
> > > share more of this
> > I'd like to defer the refactoring of HIP and AMDGPU toolchain to another 
> > patch. There are stuff in HIP toolchain which should go to AMDGPU 
> > toolchain. We need to sort out the relation between HIP and AMDGPU 
> > toolchain in that patch.
> > 
> Yes
I also have a patch refactoring the AMDGPU toolchain that would conflict


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] D51434: [HIP] Add -fvisibility hidden option to clang

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added inline comments.



Comment at: lib/Driver/ToolChains/HIP.cpp:256
+CC1Args.append({"-fvisibility", "hidden"});
 }
 

arsenm wrote:
> arsenm wrote:
> > yaxunl wrote:
> > > arsenm wrote:
> > > > We should probably start subclassing the HIP toolchain from AMDGPU and 
> > > > share more of this
> > > I'd like to defer the refactoring of HIP and AMDGPU toolchain to another 
> > > patch. There are stuff in HIP toolchain which should go to AMDGPU 
> > > toolchain. We need to sort out the relation between HIP and AMDGPU 
> > > toolchain in that patch.
> > > 
> > Yes
> I also have a patch refactoring the AMDGPU toolchain that would conflict
I will commit this patch then we will start reviewing your patch for 
refactoring AMDGPU toolchain.


https://reviews.llvm.org/D51434



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

r341073 - [OPENMP][NVPTX] Add options -f[no-]openmp-cuda-force-full-runtime.

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Thu Aug 30 07:45:24 2018
New Revision: 341073

URL: http://llvm.org/viewvc/llvm-project?rev=341073&view=rev
Log:
[OPENMP][NVPTX] Add options -f[no-]openmp-cuda-force-full-runtime.

Added options -f[no-]openmp-cuda-force-full-runtime to [not] force use
of the full runtime for OpenMP offloading to CUDA devices.

Added:
cfe/trunk/test/OpenMP/nvptx_force_full_runtime_SPMD_codegen.cpp
Modified:
cfe/trunk/include/clang/Basic/LangOptions.def
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/test/Driver/openmp-offload-gpu.c

Modified: cfe/trunk/include/clang/Basic/LangOptions.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=341073&r1=341072&r2=341073&view=diff
==
--- cfe/trunk/include/clang/Basic/LangOptions.def (original)
+++ cfe/trunk/include/clang/Basic/LangOptions.def Thu Aug 30 07:45:24 2018
@@ -203,6 +203,7 @@ LANGOPT(OpenMPSimd, 1, 0, "Use S
 LANGOPT(OpenMPUseTLS  , 1, 0, "Use TLS for threadprivates or runtime 
calls")
 LANGOPT(OpenMPIsDevice, 1, 0, "Generate code only for OpenMP target 
device")
 LANGOPT(OpenMPCUDAMode, 1, 0, "Generate code for OpenMP pragmas in 
SIMT/SPMD mode")
+LANGOPT(OpenMPCUDAForceFullRuntime , 1, 0, "Force to use full runtime in all 
constructs when offloading to CUDA devices")
 LANGOPT(OpenMPHostCXXExceptions, 1, 0, "C++ exceptions handling in the 
host code.")
 LANGOPT(RenderScript  , 1, 0, "RenderScript")
 

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=341073&r1=341072&r2=341073&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Thu Aug 30 07:45:24 2018
@@ -1531,6 +1531,10 @@ def fopenmp_cuda_mode : Flag<["-"], "fop
   Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
 def fno_openmp_cuda_mode : Flag<["-"], "fno-openmp-cuda-mode">, Group,
   Flags<[NoArgumentUnused, HelpHidden]>;
+def fopenmp_cuda_force_full_runtime : Flag<["-"], 
"fopenmp-cuda-force-full-runtime">, Group,
+  Flags<[CC1Option, NoArgumentUnused, HelpHidden]>;
+def fno_openmp_cuda_force_full_runtime : Flag<["-"], 
"fno-openmp-cuda-force-full-runtime">, Group,
+  Flags<[NoArgumentUnused, HelpHidden]>;
 def fno_optimize_sibling_calls : Flag<["-"], "fno-optimize-sibling-calls">, 
Group;
 def foptimize_sibling_calls : Flag<["-"], "foptimize-sibling-calls">, 
Group;
 def fno_escaping_block_tail_calls : Flag<["-"], 
"fno-escaping-block-tail-calls">, Group, Flags<[CC1Option]>;

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp?rev=341073&r1=341072&r2=341073&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp Thu Aug 30 07:45:24 2018
@@ -1218,7 +1218,8 @@ void CGOpenMPRuntimeNVPTX::emitSPMDEntry
   EST.ExitBB = CGF.createBasicBlock(".exit");
 
   // Initialize the OMP state in the runtime; called by all active threads.
-  bool RequiresFullRuntime = !supportsLightweightRuntime(CGF.getContext(), D);
+  bool RequiresFullRuntime = CGM.getLangOpts().OpenMPCUDAForceFullRuntime ||
+ !supportsLightweightRuntime(CGF.getContext(), D);
   llvm::Value *Args[] = {
   getThreadLimit(CGF, /*IsInSPMDExecutionMode=*/true),
   /*RequiresOMPRuntime=*/

Modified: cfe/trunk/lib/Driver/ToolChains/Clang.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Clang.cpp?rev=341073&r1=341072&r2=341073&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Clang.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Clang.cpp Thu Aug 30 07:45:24 2018
@@ -4039,8 +4039,16 @@ void Clang::ConstructJob(Compilation &C,
 
   // When in OpenMP offloading mode with NVPTX target, forward
   // cuda-mode flag
-  Args.AddLastArg(CmdArgs, options::OPT_fopenmp_cuda_mode,
-  options::OPT_fno_openmp_cuda_mode);
+  if (Args.hasFlag(options::OPT_fopenmp_cuda_mode,
+   options::OPT_fno_openmp_cuda_mode, /*Default=*/false))
+CmdArgs.push_back("-fopenmp-cuda-mode");
+
+  // When in OpenMP offloading mode with NVPTX target, check if full 
runtime
+  // is required.
+  if (Args.hasFlag(options::OPT_fopenmp_cuda_force_full_runtime,
+   options::OPT_fno_openmp_cuda_force_full_runtime,
+   /*Default=*/false))
+CmdArgs.push_back

r341074 - [Sema][NFC] Trivial cleanup in ActOnCallExpr

[Jan Korous via cfe-commits]

Author: jkorous
Date: Thu Aug 30 07:46:48 2018
New Revision: 341074

URL: http://llvm.org/viewvc/llvm-project?rev=341074&view=rev
Log:
[Sema][NFC] Trivial cleanup in ActOnCallExpr

Use logical or operator instead of a bool variable and if/else.

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

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

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=341074&r1=341073&r2=341074&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Aug 30 07:46:48 2018
@@ -5330,13 +5330,7 @@ ExprResult Sema::ActOnCallExpr(Scope *Sc
 
 // Determine whether this is a dependent call inside a C++ template,
 // in which case we won't do any semantic analysis now.
-bool Dependent = false;
-if (Fn->isTypeDependent())
-  Dependent = true;
-else if (Expr::hasAnyTypeDependentArguments(ArgExprs))
-  Dependent = true;
-
-if (Dependent) {
+if (Fn->isTypeDependent() || Expr::hasAnyTypeDependentArguments(ArgExprs)) 
{
   if (ExecConfig) {
 return new (Context) CUDAKernelCallExpr(
 Context, Fn, cast(ExecConfig), ArgExprs,


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

[PATCH] D51485: [Sema][NFC] Trivial cleanup in ActOnCallExpr

[Jan Korous via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL341074: [Sema][NFC] Trivial cleanup in ActOnCallExpr 
(authored by jkorous, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51485?vs=163304&id=163327#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51485

Files:
  cfe/trunk/lib/Sema/SemaExpr.cpp


Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -5330,13 +5330,7 @@
 
 // Determine whether this is a dependent call inside a C++ template,
 // in which case we won't do any semantic analysis now.
-bool Dependent = false;
-if (Fn->isTypeDependent())
-  Dependent = true;
-else if (Expr::hasAnyTypeDependentArguments(ArgExprs))
-  Dependent = true;
-
-if (Dependent) {
+if (Fn->isTypeDependent() || Expr::hasAnyTypeDependentArguments(ArgExprs)) 
{
   if (ExecConfig) {
 return new (Context) CUDAKernelCallExpr(
 Context, Fn, cast(ExecConfig), ArgExprs,


Index: cfe/trunk/lib/Sema/SemaExpr.cpp
===
--- cfe/trunk/lib/Sema/SemaExpr.cpp
+++ cfe/trunk/lib/Sema/SemaExpr.cpp
@@ -5330,13 +5330,7 @@
 
 // Determine whether this is a dependent call inside a C++ template,
 // in which case we won't do any semantic analysis now.
-bool Dependent = false;
-if (Fn->isTypeDependent())
-  Dependent = true;
-else if (Expr::hasAnyTypeDependentArguments(ArgExprs))
-  Dependent = true;
-
-if (Dependent) {
+if (Fn->isTypeDependent() || Expr::hasAnyTypeDependentArguments(ArgExprs)) {
   if (ExecConfig) {
 return new (Context) CUDAKernelCallExpr(
 Context, Fn, cast(ExecConfig), ArgExprs,
___
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 marked 7 inline comments as done.
usaxena95 added a comment.

> Could we try removing Status from the base class and move it into 
> `InMemoryFile` and `InMemoryDir`?

Moved the Stat and **getStatus** into the subclasses. User of **getStatus** now 
calls **getStatus** of individual subclasses.




Comment at: lib/Basic/VirtualFileSystem.cpp:526
+  InMemoryNode *ResolvedNode;
+  StringRef Name;
+

ilya-biryukov wrote:
> Can we get away without storing `Name`? `toString()` can just say `"hard link 
> to "  + ResolveNode->toString()`
Yeah we can do that. It is just for debug purpose only.



Comment at: lib/Basic/VirtualFileSystem.cpp:529
+public:
+  InMemoryHardLink(Status Stat, InMemoryNode *ResolvedNode)
+  : InMemoryNode(std::move(Stat), IME_HARD_LINK),

ilya-biryukov wrote:
> ilya-biryukov wrote:
> > Why do we need the Stat?
> > We can get it from `ResolveNode` if we want to store a copy. Any reason why 
> > it would be different?
> ResolveNode can't be `null`, right? Maybe use a reference instead?
> Also, maybe make it const?
Changed it to reference. Can't make it const as the there it is involved in 
methods that are non const (like getResolvedFile returns a pointer, getBuffer 
is not const)



Comment at: lib/Basic/VirtualFileSystem.cpp:535
+  InMemoryNode *getResolvedNode() { return ResolvedNode; }
+  llvm::MemoryBuffer *getBuffer() {
+return static_cast(ResolvedNode)->getBuffer();

ilya-biryukov wrote:
> Could we inline `getBuffer` and `getStatus`? Having the client code that's 
> explicit about resolving the references seems like a good idea.
Removed these methods. Moved the logic to client code.


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] D51475: [clangd] Load YAML static index asynchronously.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/tool/ClangdMain.cpp:86
+  return nullptr;
+Index = AsyncLoad.get();
+return Index.get();

ioeric wrote:
> ilya-biryukov wrote:
> > ioeric wrote:
> > > ilya-biryukov wrote:
> > > > I believe is a data race (multiple threads may run this line 
> > > > concurrently).
> > > > You would want some synchronization around this, `std::shared_future` 
> > > > could be a good fit
> > > Nice catch. 
> > > 
> > > I am a bit hesitated about using `shared_future` though. It could 
> > > potentially get rid of the mutex but would require much more careful use. 
> > > For example, `Index` can be set to the same value repeatedly, which makes 
> > > me a bit nervous.
> > The following pattern should give proper results:
> > ```
> > class AsyncIndex : public Index {
> > public:
> >   AsyncIndex(std::shared_future IndexFut);
> >   //
> > private;
> >   const SymbolIndex *index() const {
> >  if (IndexFut.wait(0) != ready)
> >return nullptr;
> >  return &IndexFut.get();
> >   }
> > 
> >   std::shared_futureIndexFut;
> > };
> > 
> > 
> > AsyncIndex AI(std::async([]() { /* load the index here */ return Index; });
> > ```
> My concern with this approach is that we are still calling `wait` even though 
> it's not necessary once Index has been loaded.
Maybe not bother before we know it causes actual performance issues? My bet is 
that it would never become a bottleneck, given the amount of work we're doing 
in addition to every call here.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51475



___
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 163330.
usaxena95 marked 3 inline comments as done.
usaxena95 added a comment.

- Moved Stat to the subclasses of InMemoryNode


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,126 @@
   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));
+}
+
+TEST_F(InMemoryFileSystemTest, AddHardLinkFromADirectory) {
+  Twine 

[PATCH] D51481: [clangd] Implement proximity path boosting for Dex

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 163332.
kbobyrev marked 2 inline comments as done.
kbobyrev added a comment.

Address couple of comments.


https://reviews.llvm.org/D51481

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/DexIndex.cpp
  clang-tools-extra/clangd/index/dex/DexIndex.h
  clang-tools-extra/clangd/index/dex/Token.cpp
  clang-tools-extra/clangd/index/dex/Token.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/unittests/clangd/DexIndexTests.cpp
  clang-tools-extra/unittests/clangd/TestFS.cpp
  clang-tools-extra/unittests/clangd/TestFS.h

Index: clang-tools-extra/unittests/clangd/TestFS.h
===
--- clang-tools-extra/unittests/clangd/TestFS.h
+++ clang-tools-extra/unittests/clangd/TestFS.h
@@ -59,7 +59,7 @@
 };
 
 // Returns an absolute (fake) test directory for this OS.
-const char *testRoot();
+const std::string testRoot();
 
 // Returns a suitable absolute path for this OS.
 std::string testPath(PathRef File);
Index: clang-tools-extra/unittests/clangd/TestFS.cpp
===
--- clang-tools-extra/unittests/clangd/TestFS.cpp
+++ clang-tools-extra/unittests/clangd/TestFS.cpp
@@ -64,7 +64,7 @@
   FileName, std::move(CommandLine), "")};
 }
 
-const char *testRoot() {
+const std::string testRoot() {
 #ifdef _WIN32
   return "C:\\clangd-test";
 #else
Index: clang-tools-extra/unittests/clangd/DexIndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexIndexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexIndexTests.cpp
@@ -7,8 +7,10 @@
 //
 //===--===//
 
+#include "FuzzyMatch.h"
 #include "TestIndex.h"
 #include "index/Index.h"
+#include "TestFS.h"
 #include "index/Merge.h"
 #include "index/dex/DexIndex.h"
 #include "index/dex/Iterator.h"
@@ -29,6 +31,8 @@
 namespace dex {
 namespace {
 
+std::vector URISchemes = {"unittest"};
+
 std::vector consumeIDs(Iterator &It) {
   auto IDAndScore = consume(It);
   std::vector IDs(IDAndScore.size());
@@ -325,14 +329,33 @@
 }
 
 testing::Matcher>
-trigramsAre(std::initializer_list Trigrams) {
+tokensAre(std::initializer_list Strings, Token::Kind Kind) {
   std::vector Tokens;
-  for (const auto &Symbols : Trigrams) {
-Tokens.push_back(Token(Token::Kind::Trigram, Symbols));
+  for (const auto &TokenData : Strings) {
+Tokens.push_back(Token(Kind, TokenData));
   }
   return testing::UnorderedElementsAreArray(Tokens);
 }
 
+testing::Matcher>
+trigramsAre(std::initializer_list Trigrams) {
+  return tokensAre(Trigrams, Token::Kind::Trigram);
+}
+
+testing::Matcher>
+pathsAre(std::initializer_list Paths) {
+  return tokensAre(Paths, Token::Kind::Path);
+}
+
+testing::Matcher>> proximityPathsAre(
+std::initializer_list> ProximityPaths) {
+  std::vector> Result;
+  for (const auto &P : ProximityPaths) {
+Result.push_back({Token(Token::Kind::Path, P.first), P.second});
+  }
+  return testing::UnorderedElementsAreArray(Result);
+}
+
 TEST(DexIndexTrigrams, IdentifierTrigrams) {
   EXPECT_THAT(generateIdentifierTrigrams("X86"),
   trigramsAre({"x86", "x$$", "x8$"}));
@@ -407,9 +430,27 @@
"hij", "ijk", "jkl", "klm"}));
 }
 
+TEST(DexSearchTokens, SymbolPath) {
+  EXPECT_THAT(generateProximityPaths(
+  "unittest:///clang-tools-extra/clangd/index/dex/Token.h"),
+  pathsAre({"unittest:///clang-tools-extra/clangd/index/dex/",
+"unittest:///clang-tools-extra/clangd/index/",
+"unittest:///clang-tools-extra/clangd/",
+"unittest:///clang-tools-extra/", "unittest:///"}));
+}
+
+TEST(DexSearchTokens, QueryProximityDistances) {
+  EXPECT_THAT(generateQueryProximityPaths(testRoot() + "/a/b/c/d/e/f/g.h", URISchemes),
+  proximityPathsAre({{"unittest:///a/b/c/d/e/f/", 0},
+ {"unittest:///a/b/c/d/e/", 1},
+ {"unittest:///a/b/c/d/", 2},
+ {"unittest:///a/b/c/", 3},
+ {"unittest:///a/b/", 4}}));
+}
+
 TEST(DexIndex, Lookup) {
   DexIndex I;
-  I.build(generateSymbols({"ns::abc", "ns::xyz"}));
+  I.build(generateSymbols({"ns::abc", "ns::xyz"}), URISchemes);
   EXPECT_THAT(lookup(I, SymbolID("ns::abc")), UnorderedElementsAre("ns::abc"));
   EXPECT_THAT(lookup(I, {SymbolID("ns::abc"), SymbolID("ns::xyz")}),
   UnorderedElementsAre("ns::abc", "ns::xyz"));
@@ -421,7 +462,8 @@
 TEST(DexIndex, FuzzyFind) {
   DexIndex Index;
   Index.build(generateSymbols({"ns::ABC", "ns::BCD", "::ABC", "ns::nested::ABC",
-   "other::ABC", "other::A"}));
+   "other::ABC", "other::A"}),
+  URISchemes);
   FuzzyFindRequest Req;
   R

[PATCH] D51481: [clangd] Implement proximity path boosting for Dex

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:45
+  void build(std::shared_ptr> Symbols,
+ llvm::ArrayRef URISchemes);
 

sammccall wrote:
> ioeric wrote:
> > URI schemes are property of `Symbols`. We shouldn't need to pass specify 
> > the schemes again. Dex can collect all possible URI schemes when building 
> > the index.
> > 
> > I think we could generate proximity paths for index symbols without knowing 
> > about schemes (when building the index). For example, we could 
> > `canonicalize` the URI (as in `FileDistance.cpp`), for example, by 
> > converting `test:///x/y/z` to `/test:/x/y/z`.  For a query, we would first 
> > convert query proximity paths to URIs (of all possible schemes), perform 
> > the same canonicalization on URIs, and then use the canonicalized paths to 
> > compute proximity.
> We had an offline discussion about URIs which might be interesting.
> 
> Fetching posting lists for all URI schemes at query time seems wasteful, 
> @ilya-biryukov pointed out that in practice each file is going to exist in 
> some preferred URI space, and we should only compare that one.
> The only obstacle to doing that is that the preference order for URI schemes 
> is not global, each place we need it it's accepted as configuration.
> 
> Maybe we should change that: as part of registering the URI schemes, we 
> define a preferred order. And instead of the current operation `makeURI(File, 
> scheme)` we should just offer APIs like `makePreferredURI(File)` and 
> `makeFileURI(File)`.
I also thought about extracting schemes during the build stage earlier today, 
couldn't figure out whether it's a good thing: it's probably tiny overhead for 
the build stage (if we just pass schemes the server is aware of we avoid 
looking for new `scheme:` in the prefixes), but it might be worth considering 
the architecture (and the fact that even though the server might be aware of 
many schemes, some of them might not occur in the index and that would induce 
small overhead on the query side).

Could you please elaborate your suggestion about the `canonicalize`? I'm not 
sure I caught that: IIUC the current approach (which doesn't `canonicalize` 
URIs) should be sufficient for our needs. I'm not sure why we would want 
`/test:/x/y/z/` over `test:///x/y/z/` if the extracted tokens are `test:///`, 
`test:///x/`, `test:///x/y/`, `test:///x/y/z/` (which is the case right now).



Comment at: clang-tools-extra/clangd/index/dex/DexIndex.h:64
 
-private:
+  /// For fuzzyFind() Dex retrieves getRetrievalItemsMultiplier() more items
+  /// than requested via Req.MaxCandidateCount in the first stage of filtering.

ioeric wrote:
> Why are values of multipliers interesting to users? Could these be 
> implementation details in the cpp file?
Actually, my understanding is that users might want to have full access to the 
multipliers at some point to control the performance/quality ratio.

And it's also useful for the tests: otherwise the last one would have to 
hard-code number of generated symbols to ensure only boosted ones are in the 
returned list. It would have to be updated each time these internal multipliers 
are and we might update them often/make logic less clear (by allowing users to 
control these parameters).

What do you think?


https://reviews.llvm.org/D51481



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

[PATCH] D51488: [Sema][NFC] Small cleanup - remove dead code from ActOnCallExpr() ?

[Jan Korous via Phabricator via cfe-commits]

jkorous created this revision.
jkorous added reviewers: rsmith, vsapsai, JDevlieghere.
jkorous added a project: clang.
Herald added subscribers: cfe-commits, dexonsmith.

Since there's no change to ArgExprs between here and the previous early exit 
starting at L 5333 it's effectively a dead code.

On the other hand a possible counter-argument is that code gets a bit more 
brittle.

WDYT?


Repository:
  rC Clang

https://reviews.llvm.org/D51488

Files:
  Sema/SemaExpr.cpp


Index: Sema/SemaExpr.cpp
===
--- Sema/SemaExpr.cpp
+++ Sema/SemaExpr.cpp
@@ -5369,9 +5369,6 @@
 
 // We aren't supposed to apply this logic if there's an '&' involved.
 if (!find.HasFormOfMemberPointer) {
-  if (Expr::hasAnyTypeDependentArguments(ArgExprs))
-return new (Context) CallExpr(
-Context, Fn, ArgExprs, Context.DependentTy, VK_RValue, RParenLoc);
   OverloadExpr *ovl = find.Expression;
   if (UnresolvedLookupExpr *ULE = dyn_cast(ovl))
 return BuildOverloadedCallExpr(


Index: Sema/SemaExpr.cpp
===
--- Sema/SemaExpr.cpp
+++ Sema/SemaExpr.cpp
@@ -5369,9 +5369,6 @@
 
 // We aren't supposed to apply this logic if there's an '&' involved.
 if (!find.HasFormOfMemberPointer) {
-  if (Expr::hasAnyTypeDependentArguments(ArgExprs))
-return new (Context) CallExpr(
-Context, Fn, ArgExprs, Context.DependentTy, VK_RValue, RParenLoc);
   OverloadExpr *ovl = find.Expression;
   if (UnresolvedLookupExpr *ULE = dyn_cast(ovl))
 return BuildOverloadedCallExpr(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51475: [clangd] Load YAML static index asynchronously.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

I don't have a strong opinion on async vs sync - startup time is important and 
we shouldn't block simple AST-based functionality on the index, but this 
introduces some slightly confusing UX for that speed.

However I think this should be based on https://reviews.llvm.org/D51422 which 
extracts most of what you need out of MemIndex into the new `SwapIndex`.
So static index would just be initialized as an empty SwapIndex, then spawn a 
thread that loads the YAML and calls SwapIndex::reset.
This will get avoid adding nontrivial threading stuff to the main file.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51475



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

[PATCH] D51475: [clangd] Load YAML static index asynchronously.

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(Sorry for catching this earlier, and that the patch isn't landed yet - feel 
free to pick up the review, else @kbobyrev will take a first pass tomorrow I 
think)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51475



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

[clang-tools-extra] r341076 - [clangd] Run SignatureHelp using an up-to-date preamble, waiting if needed.

[Sam McCall via cfe-commits]

Author: sammccall
Date: Thu Aug 30 08:07:34 2018
New Revision: 341076

URL: http://llvm.org/viewvc/llvm-project?rev=341076&view=rev
Log:
[clangd] Run SignatureHelp using an up-to-date preamble, waiting if needed.

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

Reviewers: ilya-biryukov

Subscribers: javed.absar, ioeric, MaskRay, jkorous, arphaman, kadircet, 
cfe-commits

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

Modified:
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/TUScheduler.cpp
clang-tools-extra/trunk/clangd/TUScheduler.h
clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp

Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=341076&r1=341075&r2=341076&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Thu Aug 30 08:07:34 2018
@@ -230,7 +230,8 @@ TaskHandle ClangdServer::codeComplete(Pa
 // is called as soon as results are available.
   };
 
-  WorkScheduler.runWithPreamble("CodeComplete", File,
+  // We use a potentially-stale preamble because latency is critical here.
+  WorkScheduler.runWithPreamble("CodeComplete", File, TUScheduler::Stale,
 Bind(Task, File.str(), std::move(CB)));
   return TH;
 }
@@ -252,7 +253,11 @@ void ClangdServer::signatureHelp(PathRef
  IP->Contents, Pos, FS, PCHs, Index));
   };
 
-  WorkScheduler.runWithPreamble("SignatureHelp", File,
+  // Unlike code completion, we wait for an up-to-date preamble here.
+  // Signature help is often triggered after code completion. If the code
+  // completion inserted a header to make the symbol available, then using
+  // the old preamble would yield useless results.
+  WorkScheduler.runWithPreamble("SignatureHelp", File, TUScheduler::Consistent,
 Bind(Action, File.str(), std::move(CB)));
 }
 

Modified: clang-tools-extra/trunk/clangd/TUScheduler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/TUScheduler.cpp?rev=341076&r1=341075&r2=341076&view=diff
==
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp (original)
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp Thu Aug 30 08:07:34 2018
@@ -183,6 +183,10 @@ public:
   bool blockUntilIdle(Deadline Timeout) const;
 
   std::shared_ptr getPossiblyStalePreamble() const;
+  /// Obtain a preamble reflecting all updates so far. Threadsafe.
+  /// It may be delivered immediately, or later on the worker thread.
+  void getCurrentPreamble(
+  llvm::unique_function)>);
   /// Wait for the first build of preamble to finish. Preamble itself can be
   /// accessed via getPossibleStalePreamble(). Note that this function will
   /// return after an unsuccessful build of the preamble too, i.e. result of
@@ -464,6 +468,34 @@ ASTWorker::getPossiblyStalePreamble() co
   return LastBuiltPreamble;
 }
 
+void ASTWorker::getCurrentPreamble(
+llvm::unique_function)> Callback) 
{
+  // We could just call startTask() to throw the read on the queue, knowing
+  // it will run after any updates. But we know this task is cheap, so to
+  // improve latency we cheat: insert it on the queue after the last update.
+  std::unique_lock Lock(Mutex);
+  auto LastUpdate =
+  std::find_if(Requests.rbegin(), Requests.rend(),
+   [](const Request &R) { return R.UpdateType.hasValue(); });
+  // If there were no writes in the queue, the preamble is ready now.
+  if (LastUpdate == Requests.rend()) {
+Lock.unlock();
+return Callback(getPossiblyStalePreamble());
+  }
+  assert(!RunSync && "Running synchronously, but queue is non-empty!");
+  Requests.insert(LastUpdate.base(),
+  Request{Bind(
+  [this](decltype(Callback) Callback) {
+Callback(getPossiblyStalePreamble());
+  },
+  std::move(Callback)),
+  "GetPreamble", steady_clock::now(),
+  Context::current().clone(),
+  /*UpdateType=*/llvm::None});
+  Lock.unlock();
+  RequestsCV.notify_all();
+}
+
 void ASTWorker::waitForFirstPreamble() const {
   PreambleWasBuilt.wait();
 }
@@ -711,7 +743,7 @@ void TUScheduler::runWithAST(
 }
 
 void TUScheduler::runWithPreamble(
-llvm::StringRef Name, PathRef File,
+llvm::StringRef Name, PathRef File, PreambleConsistency Consistency,
 llvm::unique_function)> Action) {
   auto It = Files.find(File);
   if (It == Files.end()) {
@@ -731,22 +763,40 @@ void TUScheduler::r

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

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL341076: [clangd] Run SignatureHelp using an up-to-date 
preamble, waiting if needed. (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D51438

Files:
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/TUScheduler.cpp
  clang-tools-extra/trunk/clangd/TUScheduler.h
  clang-tools-extra/trunk/unittests/clangd/TUSchedulerTests.cpp

Index: clang-tools-extra/trunk/clangd/TUScheduler.h
===
--- clang-tools-extra/trunk/clangd/TUScheduler.h
+++ clang-tools-extra/trunk/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 try to avoid
+///   reading source code from headers.
+/// This is the fastest option, usually a preamble is available immediately.
+Stale,
+  };
+  /// Schedule an async read of the preamble.
   /// If there's no preamble yet (because the file was just opened), we'll wait
-  /// for it to build. The preamble may still be null if it fails to build or is
-  /// empty.
-  /// If an error occurs during processing, it is forwarded to the \p Action
-  /// callback.
+  /// for it to build. The result may be null if it fails to build or is empty.
+  /// If an error occurs, it is forwarded to the \p Action callback.
   void runWithPreamble(llvm::StringRef Name, PathRef File,
+   PreambleConsistency Consistency,
Callback Action);
 
   /// Wait until there are no scheduled or running tasks.
Index: clang-tools-extra/trunk/clangd/TUScheduler.cpp
===
--- clang-tools-extra/trunk/clangd/TUScheduler.cpp
+++ clang-tools-extra/trunk/clangd/TUScheduler.cpp
@@ -183,6 +183,10 @@
   bool blockUntilIdle(Deadline Timeout) const;
 
   std::shared_ptr getPossiblyStalePreamble() const;
+  /// Obtain a preamble reflecting all updates so far. Threadsafe.
+  /// It may be delivered immediately, or later on the worker thread.
+  void getCurrentPreamble(
+  llvm::unique_function)>);
   /// Wait for the first build of preamble to finish. Preamble itself can be
   /// accessed via getPossibleStalePreamble(). Note that this function will
   /// return after an unsuccessful build of the preamble too, i.e. result of
@@ -464,6 +468,34 @@
   return LastBuiltPreamble;
 }
 
+void ASTWorker::getCurrentPreamble(
+llvm::unique_function)> Callback) {
+  // We could just call startTask() to throw the read on the queue, knowing
+  // it will run after any updates. But we know this task is cheap, so to
+  // improve latency we cheat: insert it on the queue after the last update.
+  std::unique_lock Lock(Mutex);
+  auto LastUpdate =
+  std::find_if(Requests.rbegin(), Requests.rend(),
+   [](const Request &R) { return R.UpdateType.hasValue(); });
+  // If there were no writes in the queue, the preamble is ready now.
+  if (LastUpdate == Requests.rend()) {
+Lock.unlock();
+return Callback(getPossiblyStalePreamble());
+  }
+  assert(!RunSync && "Running synchronously, but queue is non-empty!");
+  Requests.insert(LastUpdate.base(),
+  Request{Bind(
+  [this](decltype(Callback) Callback) {
+Callback(getPossiblyStalePreamble());
+  },
+  std::move(Callback)),
+  "GetPreamble", steady_clock::now(),
+  Context::current().clone(),
+  /*UpdateType=*/llvm::None});
+  Lock.unlock();
+  RequestsCV.notify_all();
+}

r341077 - [HIP] Add -fvisibility hidden option to clang

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Thu Aug 30 08:10:20 2018
New Revision: 341077

URL: http://llvm.org/viewvc/llvm-project?rev=341077&view=rev
Log:
[HIP] Add -fvisibility hidden option to clang

AMDGPU target need -fvisibility hidden option for clang to
work around a limitation of no PLT support, otherwise there is compilation
error at -O0.

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

Modified:
cfe/trunk/lib/Driver/ToolChains/HIP.cpp
cfe/trunk/test/Driver/hip-toolchain.hip

Modified: cfe/trunk/lib/Driver/ToolChains/HIP.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/HIP.cpp?rev=341077&r1=341076&r2=341077&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/HIP.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/HIP.cpp Thu Aug 30 08:10:20 2018
@@ -247,6 +247,12 @@ void HIPToolChain::addClangTargetOptions
   if (DriverArgs.hasFlag(options::OPT_fcuda_rdc, options::OPT_fno_cuda_rdc,
  false))
 CC1Args.push_back("-fcuda-rdc");
+
+  // Default to "hidden" visibility, as object level linking will not be
+  // supported for the foreseeable future.
+  if (!DriverArgs.hasArg(options::OPT_fvisibility_EQ,
+ options::OPT_fvisibility_ms_compat))
+CC1Args.append({"-fvisibility", "hidden"});
 }
 
 llvm::opt::DerivedArgList *

Modified: cfe/trunk/test/Driver/hip-toolchain.hip
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hip-toolchain.hip?rev=341077&r1=341076&r2=341077&view=diff
==
--- cfe/trunk/test/Driver/hip-toolchain.hip (original)
+++ cfe/trunk/test/Driver/hip-toolchain.hip Thu Aug 30 08:10:20 2018
@@ -15,13 +15,15 @@
 // CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa" 
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu" "-emit-llvm-bc"
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx803"
-// CHECK-SAME: "-fcuda-is-device" {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
+// CHECK-SAME: {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
 
 // CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu" "-emit-llvm-bc"
 // CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-target-cpu" "gfx803"
-// CHECK-SAME: "-fcuda-is-device" {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
+// CHECK-SAME: {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[B_SRC:".*b.hip"]]
 
 // CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC]] [[B_BC]]


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

[PATCH] D51434: [HIP] Add -fvisibility hidden option to clang

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL341077: [HIP] Add -fvisibility hidden option to clang 
(authored by yaxunl, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51434?vs=163196&id=163340#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51434

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


Index: cfe/trunk/test/Driver/hip-toolchain.hip
===
--- cfe/trunk/test/Driver/hip-toolchain.hip
+++ cfe/trunk/test/Driver/hip-toolchain.hip
@@ -15,13 +15,15 @@
 // CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa" 
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu" "-emit-llvm-bc"
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx803"
-// CHECK-SAME: "-fcuda-is-device" {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
+// CHECK-SAME: {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
 
 // CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu" "-emit-llvm-bc"
 // CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-target-cpu" "gfx803"
-// CHECK-SAME: "-fcuda-is-device" {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
+// CHECK-SAME: {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[B_SRC:".*b.hip"]]
 
 // CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC]] [[B_BC]]
Index: cfe/trunk/lib/Driver/ToolChains/HIP.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/HIP.cpp
+++ cfe/trunk/lib/Driver/ToolChains/HIP.cpp
@@ -247,6 +247,12 @@
   if (DriverArgs.hasFlag(options::OPT_fcuda_rdc, options::OPT_fno_cuda_rdc,
  false))
 CC1Args.push_back("-fcuda-rdc");
+
+  // Default to "hidden" visibility, as object level linking will not be
+  // supported for the foreseeable future.
+  if (!DriverArgs.hasArg(options::OPT_fvisibility_EQ,
+ options::OPT_fvisibility_ms_compat))
+CC1Args.append({"-fvisibility", "hidden"});
 }
 
 llvm::opt::DerivedArgList *


Index: cfe/trunk/test/Driver/hip-toolchain.hip
===
--- cfe/trunk/test/Driver/hip-toolchain.hip
+++ cfe/trunk/test/Driver/hip-toolchain.hip
@@ -15,13 +15,15 @@
 // CHECK: [[CLANG:".*clang.*"]] "-cc1" "-triple" "amdgcn-amd-amdhsa" 
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu" "-emit-llvm-bc"
 // CHECK-SAME: {{.*}} "-main-file-name" "a.cu" {{.*}} "-target-cpu" "gfx803"
-// CHECK-SAME: "-fcuda-is-device" {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
+// CHECK-SAME: {{.*}} "-o" [[A_BC:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[A_SRC:".*a.cu"]]
 
 // CHECK: [[CLANG]] "-cc1" "-triple" "amdgcn-amd-amdhsa"
 // CHECK-SAME: "-aux-triple" "x86_64-unknown-linux-gnu" "-emit-llvm-bc"
 // CHECK-SAME: {{.*}} "-main-file-name" "b.hip" {{.*}} "-target-cpu" "gfx803"
-// CHECK-SAME: "-fcuda-is-device" {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
+// CHECK-SAME: "-fcuda-is-device" "-fvisibility" "hidden"
+// CHECK-SAME: {{.*}} "-o" [[B_BC:".*bc"]] "-x" "hip"
 // CHECK-SAME: {{.*}} [[B_SRC:".*b.hip"]]
 
 // CHECK: [[LLVM_LINK:"*.llvm-link"]] [[A_BC]] [[B_BC]]
Index: cfe/trunk/lib/Driver/ToolChains/HIP.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/HIP.cpp
+++ cfe/trunk/lib/Driver/ToolChains/HIP.cpp
@@ -247,6 +247,12 @@
   if (DriverArgs.hasFlag(options::OPT_fcuda_rdc, options::OPT_fno_cuda_rdc,
  false))
 CC1Args.push_back("-fcuda-rdc");
+
+  // Default to "hidden" visibility, as object level linking will not be
+  // supported for the foreseeable future.
+  if (!DriverArgs.hasArg(options::OPT_fvisibility_EQ,
+ options::OPT_fvisibility_ms_compat))
+CC1Args.append({"-fvisibility", "hidden"});
 }
 
 llvm::opt::DerivedArgList *
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51475: [clangd] Load YAML static index asynchronously.

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/tool/ClangdMain.cpp:86
+  return nullptr;
+Index = AsyncLoad.get();
+return Index.get();

ilya-biryukov wrote:
> ioeric wrote:
> > ilya-biryukov wrote:
> > > ioeric wrote:
> > > > ilya-biryukov wrote:
> > > > > I believe is a data race (multiple threads may run this line 
> > > > > concurrently).
> > > > > You would want some synchronization around this, `std::shared_future` 
> > > > > could be a good fit
> > > > Nice catch. 
> > > > 
> > > > I am a bit hesitated about using `shared_future` though. It could 
> > > > potentially get rid of the mutex but would require much more careful 
> > > > use. For example, `Index` can be set to the same value repeatedly, 
> > > > which makes me a bit nervous.
> > > The following pattern should give proper results:
> > > ```
> > > class AsyncIndex : public Index {
> > > public:
> > >   AsyncIndex(std::shared_future IndexFut);
> > >   //
> > > private;
> > >   const SymbolIndex *index() const {
> > >  if (IndexFut.wait(0) != ready)
> > >return nullptr;
> > >  return &IndexFut.get();
> > >   }
> > > 
> > >   std::shared_futureIndexFut;
> > > };
> > > 
> > > 
> > > AsyncIndex AI(std::async([]() { /* load the index here */ return Index; 
> > > });
> > > ```
> > My concern with this approach is that we are still calling `wait` even 
> > though it's not necessary once Index has been loaded.
> Maybe not bother before we know it causes actual performance issues? My bet 
> is that it would never become a bottleneck, given the amount of work we're 
> doing in addition to every call here.
Sure, the performance overhead is small in both cases. But I would still try to 
avoid `wait` e.g. OS can decide to `yield` even if the timeout is 0. 


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51475



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

[PATCH] D51475: [clangd] Load YAML static index asynchronously.

[Eric Liu via Phabricator via cfe-commits]

ioeric added a comment.

In https://reviews.llvm.org/D51475#1219291, @sammccall wrote:

> I don't have a strong opinion on async vs sync - startup time is important 
> and we shouldn't block simple AST-based functionality on the index, but this 
> introduces some slightly confusing UX for that speed.
>
> However I think this should be based on https://reviews.llvm.org/D51422 which 
> extracts most of what you need out of MemIndex into the new `SwapIndex`.
>  So static index would just be initialized as an empty SwapIndex, then spawn 
> a thread that loads the YAML and calls SwapIndex::reset.
>  This will get avoid adding nontrivial threading stuff to the main file.


Oops, didn't realize that. That sounds great. Thanks!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51475



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



Comment at: clangd/TUScheduler.cpp:474
+llvm::unique_function)> Callback) 
{
+  if (RunSync)
+return Callback(getPossiblyStalePreamble());

ilya-biryukov wrote:
> It seems we could remove the special-casing of `RunSync` and still get 
> correct results (the Requests queue is always empty).
> But feel free to keep it for clarity.
Right, of course :-)
Replaced this with an assert before we write to the queue.



Comment at: clangd/TUScheduler.h:123
+  /// 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.

ilya-biryukov wrote:
> Maybe use a strongly-typed enum outside the class?
> `TUScheduler::Stale` will turn into `PreambleConsistency::Stale` at the 
> call-site. The main upside is that it does not pollute completions inside the 
> class with enumerators.
> 
> Just a suggestion, feel free to ignore.
Yeah, the downside to that is that it *does* pollute the clangd:: namespace. So 
both are a bit sad.

This header is fairly widely visible (since it's included by clangdserver) and 
this API is fairly narrowly interesting, so as far as completion goes I think I 
prefer it being hidden in TUScheduler.


Repository:
  rL LLVM

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] D50438: [clangd] Sort GoToDefinition results.

[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.

LG, thanks.
And a question of what are the things we can accidentally misdetect as explicit




Comment at: clangd/XRefs.cpp:105
+// Sort results. Declarations being referenced explicitly come first.
+std::sort(Result.begin(), Result.end(),
+  [](const DeclInfo &L, const DeclInfo &R) {

hokein wrote:
> ilya-biryukov wrote:
> > ilya-biryukov wrote:
> > > Maybe sort further at the callers instead?
> > > It would be a more intrusive change, but would also give a well-defined 
> > > result order for findDefinitions and other cases. E.g. findDefinitions 
> > > currently gives results in an arbitrary order (specifically, the one 
> > > supplied by DenseMap+sort) when multiple decls are returned.
> > > WDYT?
> > Just to clarify the original suggestion.
> > 
> > Maybe we can sort the `(location, is_explicit)` pairs instead of the 
> > `(decl, is_explicit)` pairs?
> > Sorting based on pointer equality (see `L.D < R.D`) provides 
> > non-deterministic results and we can have fully deterministic order on 
> > locations.
> > 
> > DeclarationAndMacrosFinder can return the results in arbitrary orders and 
> > the client code would be responsible for getting locations and finally 
> > sorting them.
> > WDYT?
> I think we'd better sort them in `DeclarationAndMacrosFinder` -- because we 
> have a few clients in `XRefs.cpp` using this class, and it seems that they 
> don't have their specific needs for sorting, having them sorting results 
> seems unnecessary.
That LG, as long as we have a deterministic order, we should be fine



Comment at: clangd/XRefs.cpp:141
+  return false;
+// Use the first child is good enough for most cases -- normally the
+// expression returned by handleDeclOccurence contains exactly one

Do we know which cases break? Just wondering is there are more reliable ways to 
handle those cases?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50438



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

[PATCH] D50246: [RISCV] Add support for computing sysroot for riscv32-unknown-elf

[Lewis Revill via Phabricator via cfe-commits]

lewis-revill updated this revision to Diff 163339.
lewis-revill added a comment.

Rebased and ensured that -fuse-init-array is always used (as intended in 
https://reviews.llvm.org/D50043), since addClangTargetOptions is overidden for 
this patch.


Repository:
  rC Clang

https://reviews.llvm.org/D50246

Files:
  lib/Driver/ToolChains/RISCV.cpp
  lib/Driver/ToolChains/RISCV.h
  test/Driver/riscv32-toolchain.c

Index: test/Driver/riscv32-toolchain.c
===
--- test/Driver/riscv32-toolchain.c
+++ test/Driver/riscv32-toolchain.c
@@ -19,14 +19,28 @@
 // C-RV32-BAREMETAL-ILP32: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
 // C-RV32-BAREMETAL-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
 
+// RUN: %clang %s -### -no-canonical-prefixes \
+// RUN:   -target riscv32-unknown-elf \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree 2>&1 \
+// RUN:   | FileCheck -check-prefix=C-RV32-BAREMETAL-NOSYSROOT-ILP32 %s
+
+// C-RV32-BAREMETAL-NOSYSROOT-ILP32: "-fuse-init-array"
+// C-RV32-BAREMETAL-NOSYSROOT-ILP32: "{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../bin{{/|}}riscv32-unknown-elf-ld"
+// C-RV32-BAREMETAL-NOSYSROOT-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf/lib{{/|}}crt0.o"
+// C-RV32-BAREMETAL-NOSYSROOT-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtbegin.o"
+// C-RV32-BAREMETAL-NOSYSROOT-ILP32: "-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf{{/|}}lib"
+// C-RV32-BAREMETAL-NOSYSROOT-ILP32: "-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1"
+// C-RV32-BAREMETAL-NOSYSROOT-ILP32: "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
+// C-RV32-BAREMETAL-NOSYSROOT-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
+
 // RUN: %clangxx %s -### -no-canonical-prefixes \
 // RUN:   -target riscv32-unknown-elf -stdlib=libstdc++ \
 // RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree \
 // RUN:   --sysroot=%S/Inputs/basic_riscv32_tree/riscv32-unknown-elf 2>&1 \
 // RUN:   | FileCheck -check-prefix=CXX-RV32-BAREMETAL-ILP32 %s
 
 // CXX-RV32-BAREMETAL-ILP32: "-fuse-init-array"
-// CXX-RV32-BAREMETAL-ILP32: "-internal-isystem" "{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf/include/c++{{/|}}8.0.1"
+// CXX-RV32-BAREMETAL-ILP32: "-internal-isystem" "{{.*}}Inputs/basic_riscv32_tree/riscv32-unknown-elf/include/c++{{/|}}8.0.1"
 // CXX-RV32-BAREMETAL-ILP32: "{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../bin{{/|}}riscv32-unknown-elf-ld"
 // CXX-RV32-BAREMETAL-ILP32: "--sysroot={{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf"
 // CXX-RV32-BAREMETAL-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/riscv32-unknown-elf/lib{{/|}}crt0.o"
@@ -36,6 +50,21 @@
 // CXX-RV32-BAREMETAL-ILP32: "-lstdc++" "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
 // CXX-RV32-BAREMETAL-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
 
+// RUN: %clangxx %s -### -no-canonical-prefixes \
+// RUN:   -target riscv32-unknown-elf -stdlib=libstdc++ \
+// RUN:   --gcc-toolchain=%S/Inputs/basic_riscv32_tree 2>&1 \
+// RUN:   | FileCheck -check-prefix=CXX-RV32-BAREMETAL-NOSYSROOT-ILP32 %s
+
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-fuse-init-array"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-internal-isystem" "{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf/include/c++{{/|}}8.0.1"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "{{.*}}Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../bin{{/|}}riscv32-unknown-elf-ld"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf/lib{{/|}}crt0.o"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtbegin.o"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1/../../../../riscv32-unknown-elf/lib"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-L{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "-lstdc++" "--start-group" "-lc" "-lgloss" "--end-group" "-lgcc"
+// CXX-RV32-BAREMETAL-NOSYSROOT-ILP32: "{{.*}}/Inputs/basic_riscv32_tree/lib/gcc/riscv32-unknown-elf/8.0.1{{/|}}crtend.o"
+
 // RUN: %clang %s -### -no-canonical-prefixes -fuse-ld=ld \
 // RUN:   -target riscv32-linux-unknown-elf \
 // RUN:   --gcc-toolchain=%S/Inputs/multilib_riscv_linux_sdk \
Index: lib/Driver/ToolChains/RISCV.h
===
--- lib/Driver/ToolC

[PATCH] D51378: [OPENMP] Add support for nested 'declare target' directives

[Patrick Lyster via Phabricator via cfe-commits]

patricklyster added a comment.

In https://reviews.llvm.org/D51378#1218184, @RaviNarayanaswamy wrote:

> We should just go with generating an error if the DeclareTargetNestingLevel 
> is not 0 at the end of compilation unit.  
>  Hard to detect if user accidentally forgot to have end declare in header 
> file and had it in the include file or it was intentional.


This is the current behaviour


Repository:
  rC Clang

https://reviews.llvm.org/D51378



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

[PATCH] D51488: [Sema][NFC] Small cleanup - remove dead code from ActOnCallExpr() ?

[Jonas Devlieghere via Phabricator via cfe-commits]

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

Assuming `ArgExprs` doesn't change between those two calls this seems fine.




Comment at: Sema/SemaExpr.cpp:5338
 Context.DependentTy, VK_RValue, RParenLoc);
   } else {
 

While you're at it you might as well remove the else branch here.


Repository:
  rC Clang

https://reviews.llvm.org/D51488



___
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.

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: include/clang/Basic/VirtualFileSystem.h:359
 
+  /// Add a HardLink from one file to another.
+  /// Makes the UniqueID of To file same as that of From file. If CopyBuffer is

ilya-biryukov wrote:
> Maybe use the common spelling: "hard link"? Otherwise it feels like there is 
> a HardLink class somewhere.
There are still instances of HardLink. Maybe update them too?



Comment at: include/clang/Basic/VirtualFileSystem.h:340
+   Optional Perms,
+   detail::InMemoryNode *HardLink);
 

- Maybe name it differently, e.g. ResolvedHardLink or HardLinkTarget?
- Maybe document that how this function behaves for non-null vs null values 
(i.e. creates dirs/files vs links to files)?



Comment at: include/clang/Basic/VirtualFileSystem.h:359
+  public:
+  /// Add a HardLink to a File.
+  /// The To path must be an existing file or a hardlink. The From file must 
not

Maybe document that it does nothing for directories?
Also, any reason to not support directories at this point? The limitation seem 
artificial at this point.



Comment at: lib/Basic/VirtualFileSystem.cpp:485
   StringRef getFileName() const {
-return llvm::sys::path::filename(Stat.getName());
+return llvm::sys::path::filename(FileName.data());
   }

Maybe store the result of `llvm::sys::path::filename()` instead of the full 
name in the first place?



Comment at: lib/Basic/VirtualFileSystem.cpp:709
+  if (auto Link = dyn_cast(Node)) {
+return Link->getResolvedFile()->getBuffer()->getBuffer() ==
+   Buffer->getBuffer();

Why do we need to pass a buffer when creating a hard link?
Maybe just assert it's null?



Comment at: lib/Basic/VirtualFileSystem.cpp:725
+  return addFile(P, ModificationTime,
+ llvm::MemoryBuffer::getMemBuffer(
+ Buffer->getBuffer(), Buffer->getBufferIdentifier()),

Why not pass the buffer unchanged? 



Comment at: lib/Basic/VirtualFileSystem.cpp:811
+if (auto Link = dyn_cast(*Node))
+  return Link->getResolvedFile()->getStatus(Path.str());
+  }

`lookupInMemoryNode` never returns link nodes, right? Maybe assert the link 
case is not possible?



Comment at: lib/Basic/VirtualFileSystem.cpp:846
+CurrentEntry = Dir->getStatus(Path);
+  if (auto File = dyn_cast(I->second.get()))
+CurrentEntry = File->getStatus(Path);

NIT: add else?



Comment at: lib/Basic/VirtualFileSystem.cpp:848
+CurrentEntry = File->getStatus(Path);
+  if (auto Link = dyn_cast(I->second.get()))
+CurrentEntry = Link->getResolvedFile()->getStatus(Path);

NIT: add else?



Comment at: lib/Basic/VirtualFileSystem.cpp:529
+public:
+  InMemoryHardLink(Status Stat, InMemoryNode *ResolvedNode)
+  : InMemoryNode(std::move(Stat), IME_HARD_LINK),

usaxena95 wrote:
> ilya-biryukov wrote:
> > ilya-biryukov wrote:
> > > Why do we need the Stat?
> > > We can get it from `ResolveNode` if we want to store a copy. Any reason 
> > > why it would be different?
> > ResolveNode can't be `null`, right? Maybe use a reference instead?
> > Also, maybe make it const?
> Changed it to reference. Can't make it const as the there it is involved in 
> methods that are non const (like getResolvedFile returns a pointer, getBuffer 
> is not const)
Both methods could be made const, though, right?
It looks like the buffers are always copied when returned from the file system 
(because the VFS interface returns a `unique_ptr`).
`getResolvedFile` can also return a const node.



Comment at: unittests/Basic/VirtualFileSystemTest.cpp:701
+public:
+  void ExpectHardLink(Twine From, Twine To, const string &ExpectedBuffer) {
+auto OpenedFrom = FS.openFileForRead(From);

Since gtest only reports one level of locations for the failing assertion, 
debugging test failures would be hard here (we won't see the actual test 
locations, the errors will always point into `ExpectHardLink` function.

We probably want to write a matcher and use `EXPECT_THAT`, so we can write 
something like
```
EXPECT_THAT(File, IsHardLinkTo(LinkTarget))
```

To write a matcher, we could use the gmock macros:
```
MATCHER_P(IsHardLinkTo, To, "") {
  llvm::StringRef From = arg;
  llvm::StringRef To = arg;
  // return true when matches, false otherwise
}
```


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

r341082 - [libFuzzer] Port to Windows

[Matt Morehouse via cfe-commits]

Author: morehouse
Date: Thu Aug 30 08:54:44 2018
New Revision: 341082

URL: http://llvm.org/viewvc/llvm-project?rev=341082&view=rev
Log:
[libFuzzer] Port to Windows

Summary:
Port libFuzzer to windows-msvc.
This patch allows libFuzzer targets to be built and run on Windows, using 
-fsanitize=fuzzer and/or fsanitize=fuzzer-no-link. It allows these forms of 
coverage instrumentation to work on Windows as well.
It does not fix all issues, such as those with -fsanitize-coverage=stack-depth, 
which is not usable on Windows as of this patch.
It also does not fix any libFuzzer integration tests. Nearly all of them fail 
to compile, fixing them will come in a later patch, so libFuzzer tests are 
disabled on Windows until them.

Patch By: metzman

Reviewers: morehouse, rnk

Reviewed By: morehouse, rnk

Subscribers: #sanitizers, delcypher, morehouse, kcc, eraman

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

Modified:
cfe/trunk/lib/Driver/ToolChains/MSVC.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/MSVC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/MSVC.cpp?rev=341082&r1=341081&r2=341082&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/MSVC.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/MSVC.cpp Thu Aug 30 08:54:44 2018
@@ -365,6 +365,17 @@ void visualstudio::Linker::ConstructJob(
 CmdArgs.push_back(Args.MakeArgString(std::string("-implib:") + 
ImplibName));
   }
 
+  if (TC.getSanitizerArgs().needsFuzzer()) {
+if (!Args.hasArg(options::OPT_shared))
+  CmdArgs.push_back(
+  Args.MakeArgString(std::string("-wholearchive:") +
+ TC.getCompilerRTArgString(Args, "fuzzer", 
false)));
+CmdArgs.push_back(Args.MakeArgString("-debug"));
+// Prevent the linker from padding sections we use for instrumentation
+// arrays.
+CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
+  }
+
   if (TC.getSanitizerArgs().needsAsanRt()) {
 CmdArgs.push_back(Args.MakeArgString("-debug"));
 CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
@@ -1298,6 +1309,8 @@ MSVCToolChain::ComputeEffectiveClangTrip
 SanitizerMask MSVCToolChain::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::Address;
+  Res |= SanitizerKind::Fuzzer;
+  Res |= SanitizerKind::FuzzerNoLink;
   Res &= ~SanitizerKind::CFIMFCall;
   return Res;
 }


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

[PATCH] D51022: [libFuzzer] Port to Windows

[Matt Morehouse via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC341082: [libFuzzer] Port to Windows (authored by morehouse, 
committed by ).
Herald added a subscriber: cfe-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51022?vs=163338&id=163348#toc

Repository:
  rC Clang

https://reviews.llvm.org/D51022

Files:
  lib/Driver/ToolChains/MSVC.cpp


Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -365,6 +365,17 @@
 CmdArgs.push_back(Args.MakeArgString(std::string("-implib:") + 
ImplibName));
   }
 
+  if (TC.getSanitizerArgs().needsFuzzer()) {
+if (!Args.hasArg(options::OPT_shared))
+  CmdArgs.push_back(
+  Args.MakeArgString(std::string("-wholearchive:") +
+ TC.getCompilerRTArgString(Args, "fuzzer", 
false)));
+CmdArgs.push_back(Args.MakeArgString("-debug"));
+// Prevent the linker from padding sections we use for instrumentation
+// arrays.
+CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
+  }
+
   if (TC.getSanitizerArgs().needsAsanRt()) {
 CmdArgs.push_back(Args.MakeArgString("-debug"));
 CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
@@ -1298,6 +1309,8 @@
 SanitizerMask MSVCToolChain::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::Address;
+  Res |= SanitizerKind::Fuzzer;
+  Res |= SanitizerKind::FuzzerNoLink;
   Res &= ~SanitizerKind::CFIMFCall;
   return Res;
 }


Index: lib/Driver/ToolChains/MSVC.cpp
===
--- lib/Driver/ToolChains/MSVC.cpp
+++ lib/Driver/ToolChains/MSVC.cpp
@@ -365,6 +365,17 @@
 CmdArgs.push_back(Args.MakeArgString(std::string("-implib:") + ImplibName));
   }
 
+  if (TC.getSanitizerArgs().needsFuzzer()) {
+if (!Args.hasArg(options::OPT_shared))
+  CmdArgs.push_back(
+  Args.MakeArgString(std::string("-wholearchive:") +
+ TC.getCompilerRTArgString(Args, "fuzzer", false)));
+CmdArgs.push_back(Args.MakeArgString("-debug"));
+// Prevent the linker from padding sections we use for instrumentation
+// arrays.
+CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
+  }
+
   if (TC.getSanitizerArgs().needsAsanRt()) {
 CmdArgs.push_back(Args.MakeArgString("-debug"));
 CmdArgs.push_back(Args.MakeArgString("-incremental:no"));
@@ -1298,6 +1309,8 @@
 SanitizerMask MSVCToolChain::getSupportedSanitizers() const {
   SanitizerMask Res = ToolChain::getSupportedSanitizers();
   Res |= SanitizerKind::Address;
+  Res |= SanitizerKind::Fuzzer;
+  Res |= SanitizerKind::FuzzerNoLink;
   Res &= ~SanitizerKind::CFIMFCall;
   return Res;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Eric Liu via Phabricator via cfe-commits]

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

- Return all possible #includes in CodeComplete.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51291

Files:
  clangd/CodeComplete.cpp
  clangd/CodeComplete.h
  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 

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

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clangd/index/FileIndex.h:47
+  // The shared_ptr keeps the symbols alive.
+  std::shared_ptr buildMemIndex();
 

Maybe avoid hardcoding the index name, so that we could potentially switch to 
use a different index implementation?

We might also want to allow user to specify different index implementations for 
file index e.g. main file dynamic index might prefer MemIndex while Dex might 
be a better choice for the preamble index. 



Comment at: clangd/index/MemIndex.h:30
+  /// Builds an index from a slab. The shared_ptr manages the slab's lifetime.
+  static std::shared_ptr build(SymbolSlab Slab);
 

(It's a bit unfortunate that this has to return `shared_ptr` now)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51422



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

[PATCH] D51177: [DEBUGINFO] Add support for emission of the debug directives only.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev updated this revision to Diff 163353.
ABataev added a comment.

Address comment from David.


Repository:
  rC Clang

https://reviews.llvm.org/D51177

Files:
  include/clang/Basic/DebugInfoOptions.h
  include/clang/Driver/Options.td
  lib/CodeGen/CGDebugInfo.cpp
  lib/Driver/ToolChains/Clang.cpp
  lib/Frontend/CompilerInvocation.cpp
  test/CodeGen/debug-info-gline-tables-only.c
  test/CodeGen/debug-info-gline-tables-only2.c
  test/CodeGen/debug-info-line.c
  test/CodeGen/debug-info-macro.c
  test/CodeGen/debug-info-scope.c
  test/CodeGen/lifetime-debuginfo-1.c
  test/CodeGen/lifetime-debuginfo-2.c
  test/CodeGenCXX/crash.cpp
  test/CodeGenCXX/debug-info-blocks.cpp
  test/CodeGenCXX/debug-info-gline-tables-only.cpp
  test/CodeGenCXX/debug-info-line.cpp
  test/CodeGenCXX/debug-info-ms-dtor-thunks.cpp
  test/CodeGenCXX/debug-info-namespace.cpp
  test/CodeGenCXX/debug-info-template-explicit-specialization.cpp
  test/CodeGenCXX/debug-info-windows-dtor.cpp
  test/CodeGenCXX/linetable-virtual-variadic.cpp
  test/CodeGenObjCXX/debug-info-line.mm
  test/CodeGenObjCXX/pr14474-gline-tables-only.mm
  test/Driver/debug-options.c

Index: test/Driver/debug-options.c
===
--- test/Driver/debug-options.c
+++ test/Driver/debug-options.c
@@ -119,6 +119,25 @@
 // RUN: %clang -### -c -gline-tables-only -g0 %s 2>&1 \
 // RUN: | FileCheck -check-prefix=GLTO_NO %s
 //
+// RUN: %clang -### -c -gline-directives-only %s -target x86_64-apple-darwin 2>&1 \
+// RUN: | FileCheck -check-prefix=GLIO_ONLY %s
+// RUN: %clang -### -c -gline-directives-only %s -target i686-pc-openbsd 2>&1 \
+// RUN: | FileCheck -check-prefix=GLIO_ONLY_DWARF2 %s
+// RUN: %clang -### -c -gline-directives-only %s -target x86_64-pc-freebsd10.0 2>&1 \
+// RUN: | FileCheck -check-prefix=GLIO_ONLY_DWARF2 %s
+// RUN: %clang -### -c -gline-directives-only -g %s -target x86_64-linux-gnu 2>&1 \
+// RUN: | FileCheck -check-prefix=G_ONLY %s
+// RUN: %clang -### -c -gline-directives-only -g %s -target x86_64-apple-darwin16 2>&1 \
+// RUN: | FileCheck -check-prefix=G_STANDALONE -check-prefix=G_DWARF4 %s
+// RUN: %clang -### -c -gline-directives-only -g %s -target i686-pc-openbsd 2>&1 \
+// RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
+// RUN: %clang -### -c -gline-directives-only -g %s -target x86_64-pc-freebsd10.0 2>&1 \
+// RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
+// RUN: %clang -### -c -gline-directives-only -g %s -target i386-pc-solaris 2>&1 \
+// RUN: | FileCheck -check-prefix=G_ONLY_DWARF2 %s
+// RUN: %clang -### -c -gline-directives-only -g0 %s 2>&1 \
+// RUN: | FileCheck -check-prefix=GLIO_NO %s
+//
 // RUN: %clang -### -c -grecord-gcc-switches %s 2>&1 \
 // | FileCheck -check-prefix=GRECORD %s
 // RUN: %clang -### -c -gno-record-gcc-switches %s 2>&1 \
@@ -178,6 +197,9 @@
 // RUN: %clang -### -gmodules -gline-tables-only %s 2>&1 \
 // RUN:| FileCheck -check-prefix=GLTO_ONLY %s
 //
+// RUN: %clang -### -gmodules -gline-directives-only %s 2>&1 \
+// RUN:| FileCheck -check-prefix=GLIO_ONLY %s
+//
 // G: "-cc1"
 // G: "-debug-info-kind=limited"
 //
@@ -204,6 +226,15 @@
 // GLTO_ONLY_DWARF2: "-debug-info-kind=line-tables-only"
 // GLTO_ONLY_DWARF2: "-dwarf-version=2"
 //
+// GLIO_ONLY: "-cc1"
+// GLIO_ONLY-NOT: "-dwarf-ext-refs"
+// GLIO_ONLY: "-debug-info-kind=line-directives-only"
+// GLIO_ONLY-NOT: "-dwarf-ext-refs"
+//
+// GLIO_ONLY_DWARF2: "-cc1"
+// GLIO_ONLY_DWARF2: "-debug-info-kind=line-directives-only"
+// GLIO_ONLY_DWARF2: "-dwarf-version=2"
+//
 // G_ONLY: "-cc1"
 // G_ONLY: "-debug-info-kind=limited"
 //
@@ -225,6 +256,10 @@
 // GLTO_NO: "-cc1"
 // GLTO_NO-NOT: -debug-info-kind=
 //
+// This tests asserts that "-gline-directives-only" "-g0" disables debug info.
+// GLIO_NO: "-cc1"
+// GLIO_NO-NOT: -debug-info-kind=
+//
 // GRECORD: "-dwarf-debug-flags"
 // GRECORD: -### -c -grecord-gcc-switches
 //
Index: test/CodeGenObjCXX/pr14474-gline-tables-only.mm
===
--- test/CodeGenObjCXX/pr14474-gline-tables-only.mm
+++ test/CodeGenObjCXX/pr14474-gline-tables-only.mm
@@ -1,6 +1,8 @@
 // PR 14474
 // RUN: %clang_cc1 -triple i386-apple-macosx10.6.0 -emit-llvm \
 // RUN:   -debug-info-kind=line-tables-only -x objective-c++ -o /dev/null %s
+// RUN: %clang_cc1 -triple i386-apple-macosx10.6.0 -emit-llvm \
+// RUN:   -debug-info-kind=line-directives-only -x objective-c++ -o /dev/null %s
 
 typedef signed char BOOL;
 @class NSInvocation, NSMethodSignature, NSCoder, NSString, NSEnumerator;
Index: test/CodeGenObjCXX/debug-info-line.mm
===
--- test/CodeGenObjCXX/debug-info-line.mm
+++ test/CodeGenObjCXX/debug-info-line.mm
@@ -1,4 +1,5 @@
 // RUN: %clang_cc1 -triple x86_64-unknown-windows-gnu -fcxx-exceptions -fexceptions -debu

[PATCH] D51329: [Attribute/Diagnostics] Print macro instead of whole attribute for address_space

[Leonard Chan via Phabricator via cfe-commits]

leonardchan marked 5 inline comments as done.
leonardchan added a comment.

@rsmith When you clarify what you mean by `outermost macro expansion`? Say I 
have the following:

  #define ATTR __attribute__
  #define AS(i) address_space(i)
  #define AS1 ATTR((address_space(1)))
  #define AS2 ATTR((AS(2)))
  
  int cmp(AS1 char *x, AS2 char *y) {
return x < y ? x : y;
  }

`AS1 char *` and `AS2 char *` would be diagnosed for an error relating to 
mismatched types, but I think `outermost macro expansion` implies either `ATTR` 
or `AS` would be the identifier stored as opposed to `AS1` and `AS2`.

Would it be simpler to instead just check:

- the starting `__attribute__` token was the start of a macro expansion
- the ending right parenthesis token was the end of a macro expansion
- both start and end tokens had the same location to indicate this attribute 
was used in a macro

After fulfulling these, we can call `Lexer::getSourceText()` to get the 
spelling of the macro from this SourceLocation.


Repository:
  rC Clang

https://reviews.llvm.org/D51329



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

r341083 - Test the cross-product of how libgcc-related arguments are passed to the linker.

[Sterling Augustine via cfe-commits]

Author: saugustine
Date: Thu Aug 30 09:37:06 2018
New Revision: 341083

URL: http://llvm.org/viewvc/llvm-project?rev=341083&view=rev
Log:
Test the cross-product of how libgcc-related arguments are passed to the linker.


Modified:
cfe/trunk/test/Driver/linux-ld.c

Modified: cfe/trunk/test/Driver/linux-ld.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/linux-ld.c?rev=341083&r1=341082&r2=341083&view=diff
==
--- cfe/trunk/test/Driver/linux-ld.c (original)
+++ cfe/trunk/test/Driver/linux-ld.c Thu Aug 30 09:37:06 2018
@@ -150,6 +150,88 @@
 //
 // RUN: %clang -no-canonical-prefixes %s -### -o %t.o 2>&1 \
 // RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-NO-LIBGCC %s
+// CHECK-CLANG-NO-LIBGCC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-NO-LIBGCC: "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed"
+// CHECK-CLANG-NO-LIBGCC: "-lc"
+// CHECK-CLANG-NO-LIBGCC: "-lgcc" "--as-needed" "-lgcc_s" "--no-as-needed"
+//
+// RUN: %clang++ -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANGXX-NO-LIBGCC %s
+// CHECK-CLANGXX-NO-LIBGCC: "{{.*}}ld{{(.exe)?}}" "--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANGXX-NO-LIBGCC: "-lgcc_s" "-lgcc"
+// CHECK-CLANGXX-NO-LIBGCC: "-lc"
+// CHECK-CLANGXX-NO-LIBGCC: "-lgcc_s" "-lgcc"
+//
+// RUN: %clang -static -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-NO-LIBGCC-STATIC %s
+// CHECK-CLANG-NO-LIBGCC-STATIC: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-NO-LIBGCC-STATIC: "--start-group" "-lgcc" "-lgcc_eh" "-lc" 
"--end-group"
+//
+// RUN: %clang -dynamic -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-NO-LIBGCC-DYNAMIC %s
+// CHECK-CLANG-NO-LIBGCC-DYNAMIC: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-NO-LIBGCC-DYNAMIC: "-lgcc" "--as-needed" "-lgcc_s" 
"--no-as-needed"
+// CHECK-CLANG-NO-LIBGCC-DYNAMIC: "-lc"
+// CHECK-CLANG-NO-LIBGCC-DYNAMIC: "-lgcc" "--as-needed" "-lgcc_s" 
"--no-as-needed"
+//
+// RUN: %clang -static-libgcc -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-STATIC-LIBGCC %s
+// CHECK-CLANG-STATIC-LIBGCC: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-STATIC-LIBGCC: "-lgcc" "-lgcc_eh"
+// CHECK-CLANG-STATIC-LIBGCC: "-lc"
+// CHECK-CLANG-STATIC-LIBGCC: "-lgcc" "-lgcc_eh"
+//
+// RUN: %clang -static-libgcc -dynamic -no-canonical-prefixes %s -### -o %t.o 
2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-STATIC-LIBGCC-DYNAMIC %s
+// CHECK-CLANG-STATIC-LIBGCC-DYNAMIC: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// CHECK-CLANG-STATIC-LIBGCC-DYNAMIC: "-lgcc" "-lgcc_eh"
+// CHECK-CLANG-STATIC-LIBGCC-DYNAMIC: "-lc"
+// CHECK-CLANG-STATIC-LIBGCC-DYNAMIC: "-lgcc" "-lgcc_eh"
+//
+// RUN: %clang -shared-libgcc -no-canonical-prefixes %s -### -o %t.o 2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-SHARED-LIBGCC %s
+// CHECK-CLANG-SHARED-LIBGCC: warning: argument unused during compilation: 
'-shared-libgcc'
+// This will be the correct check once the driver supports -shared-libgcc
+// SKIP-CHECK-CLANG-SHARED-LIBGCC: "{{.*}}ld{{(.exe)?}}" 
"--sysroot=[[SYSROOT:[^"]+]]"
+// SKIP-CHECK-CLANG-SHARED-LIBGCC: "-lgcc_s" "-lgcc"
+// SKIP-CHECK-CLANG-SHARED-LIBGCC: "-lc"
+// SKIP-CHECK-CLANG-SHARED-LIBGCC: "-lgcc_s" "-lgcc"
+//
+// RUN: %clang -shared-libgcc -dynamic -no-canonical-prefixes %s -### -o %t.o 
2>&1 \
+// RUN: --target=x86_64-unknown-linux -rtlib=platform \
+// RUN: --gcc-toolchain="" \
+// RUN: --sysroot=%S/Inputs/basic_linux_tree \
+// RUN:   | FileCheck --check-prefix=CHECK-CLANG-SHARED-LIBGCC-DYNAMIC %s
+// CHECK-CLANG-SHARED-LIBGCC-DYNAMIC: warning: argument unused during 
compilation: '-shared-libgcc'
+// CHECK-CLANG-SHARED-LIBGCC-DYNAMIC: "{{.*}}ld{{(.exe)?}}" 
"--sysroo