[PATCH] D51725: Allow un-setting the compilation database path

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

If this setting exposed directly the users in Theia or is it something that is 
exposed via a custom UI (e.g. choosing named build configs or something 
similar)?

In https://reviews.llvm.org/D51725#1232171, @simark wrote:

> I was assuming that restarting clangd might potentially be significantly more 
> costly than just changing a setting, but maybe I'm wrong.


It's only cheaper if changes in the compilation database paths don't affect 
compile commands. If they do, everything will get rebuilt on next access, which 
is costly.

> The point of switching compilation database path is usually to point to a 
> different build that has different flags, so all open files will get reparsed 
> anyway...  And just start clangd isn't particularly heavy.

+1, since all compile commands change, we probably never get a performance win 
in practice.
One thing that (I believe) we're currently doing is using the old preamble for 
completion. This allow to code complete before reparse of preamble is finished.
I'm not sure this works when changing compile commands, though. Have not seen 
the breakages yet, but it's also neither something we've been testing nor 
something that pops up often in practice.

> I'll investigate how difficult it is to make our frontend (Theia 
> ) restart clangd when the user switches 
> compilation database.

If that is feasible, might allow removing some code from clangd. Though don't 
expect that amount to be too high.
One potential complication to restoring the state of the language server. It 
should be just the list of open files, but I may be missing something.

If you'll decide to go with an option to reset the path, see the comment about 
making empty path special. `Optional>` does not play nicely with 
json serialization code and hard to read (even though it does look like the 
right thing to do from the type system perspective).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51725



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

[PATCH] D51971: [clangd] Use JSON format in benchmark requests reader

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:45
+  if (!JSONArray) {
+llvm::errs() << "Couldn't parse request.\n";
+  }

Return from function after error?



Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:47
+  }
+  FuzzyFindRequest Request;
+  for (const auto &Item : *JSONArray->getAsArray()) {

Maybe declare this var inside the loop where it's used?



Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:48
+  FuzzyFindRequest Request;
+  for (const auto &Item : *JSONArray->getAsArray()) {
+fromJSON(Item, Request);

Check that the parsed value is an array and report an error if not?


https://reviews.llvm.org/D51971



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

[PATCH] D51996: [clangd] Simplify cancellation public API

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

> but the thing you're spending the CPU on is checking for cancellation...

Unless checking for cancellation is really cheap (which is doable, I think).
We should probably hit some of those in practice before doing something, though.




Comment at: clangd/Cancellation.h:81
+/// Always false if there is no active cancelable task.
+/// This isn't free (context lookup) - don't call it in a tight loop.
+bool isCancelled();

Allowing fast checking for cancellation seem important.
E.g. if we want to cancel in the middle of parsing, we'll probably be faced 
with sema callbacks, frequency of which we don't control, so we'd better design 
something that's fast to check.

I see two ways to deal with this:
- Keep the current design, but cache last access to context key.
- Add an API to get something that can quickly check for cancellation, i.e. 
something that would hold a reference to resolved context key.

Both could be added later, though, and I don't have any data, it's just my 
intuition.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51996



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

[PATCH] D52015: [XRay][clang] Always emit XRay attributes for LLVM IR

[Dean Michael Berris via Phabricator via cfe-commits]

dberris created this revision.
dberris added reviewers: mboerger, eizan.
Herald added subscribers: dexonsmith, mehdi_amini.

Before this change, we only emit the XRay attributes in LLVM IR when the
-fxray-instrument flag is provided. This may cause issues with thinlto
when the final binary is being built/linked with -fxray-instrument, and
the constitutent LLVM IR gets re-lowered with xray instrumentation.

With this change, we can honour the attributes provided in the source
code and preserve those in the IR. This way, even in thinlto builds, we
retain the attributes which say whether functions should never be XRay
instrumented, or whether they must always be XRay instrumented.

This change addresses llvm.org/PR38922.


https://reviews.llvm.org/D52015

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/xray-attributes-supported.cpp


Index: clang/test/CodeGen/xray-attributes-supported.cpp
===
--- clang/test/CodeGen/xray-attributes-supported.cpp
+++ clang/test/CodeGen/xray-attributes-supported.cpp
@@ -6,6 +6,18 @@
 // RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple mips64el-unknown-linux-gnu | FileCheck %s
 // RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple powerpc64le-unknown-linux-gnu | FileCheck %s
 
+// We also want to ensure that the attributes show up even if we explicitly 
turn
+// off XRay instrumentation. We let the back-end decide whether to honour the
+// attributes instead.
+//
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple mips-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple mipsel-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple mips64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple mips64el-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - 
-triple powerpc64le-unknown-linux-gnu | FileCheck %s
+
 // Make sure that the LLVM attribute for XRay-annotated functions do show up.
 [[clang::xray_always_instrument]] void foo() {
 // CHECK: define void @_Z3foov() #0
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1967,9 +1967,6 @@
 
 bool CodeGenModule::imbueXRayAttrs(llvm::Function *Fn, SourceLocation Loc,
StringRef Category) const {
-  if (!LangOpts.XRayInstrument)
-return false;
-
   const auto &XRayFilter = getContext().getXRayFilter();
   using ImbueAttr = XRayFunctionFilter::ImbueAttribute;
   auto Attr = ImbueAttr::NONE;
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -901,21 +901,20 @@
   }
 
   // Apply xray attributes to the function (as a string, for now)
-  bool InstrumentXray = ShouldXRayInstrumentFunction() &&
-CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
-XRayInstrKind::Function);
-  if (D && InstrumentXray) {
+  if (D) {
 if (const auto *XRayAttr = D->getAttr()) {
-  if (XRayAttr->alwaysXRayInstrument())
-Fn->addFnAttr("function-instrument", "xray-always");
-  if (XRayAttr->neverXRayInstrument())
-Fn->addFnAttr("function-instrument", "xray-never");
-  if (const auto *LogArgs = D->getAttr()) {
-Fn->addFnAttr("xray-log-args",
-  llvm::utostr(LogArgs->getArgumentCount()));
+  if (CGM.getCodeGenOpts().XRayInstrumentationBundle.has(
+  XRayInstrKind::Function)) {
+if (XRayAttr->alwaysXRayInstrument())
+  Fn->addFnAttr("function-instrument", "xray-always");
+if (XRayAttr->neverXRayInstrument())
+  Fn->addFnAttr("function-instrument", "xray-never");
+if (const auto *LogArgs = D->getAttr())
+  Fn->addFnAttr("xray-log-args",
+llvm::utostr(LogArgs->getArgumentCount()));
   }
 } else {
-  if (!CGM.imbueXRayAttrs(Fn, Loc))
+  if (ShouldXRayInstrumentFunction() && !CGM.imbueXRayAttrs(Fn, Loc))
 Fn->addFnAttr(
 "xray-instruction-threshold",
 llvm::itostr(CGM.getCodeGenOpts().XRayInstructionThreshold));


Index: clang/test/CodeGen/xray-attributes-supported.cpp
===

[PATCH] D51996: [clangd] Simplify cancellation public API

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Will let Kadir finish the review, consider lgtm'ed from me ;-)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51996



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

r342116 - [clang-format] Wrapped block after case label should not be merged into one line

[Owen Pan via cfe-commits]

Author: owenpan
Date: Thu Sep 13 00:27:15 2018
New Revision: 342116

URL: http://llvm.org/viewvc/llvm-project?rev=342116&view=rev
Log:
[clang-format] Wrapped block after case label should not be merged into one line

PR38854

Differential Revision: http://reviews.llvm.org/D51719

Modified:
cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp?rev=342116&r1=342115&r2=342116&view=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp Thu Sep 13 00:27:15 2018
@@ -323,6 +323,10 @@ private:
   kwId == clang::tok::objc_synchronized)
 return 0;
 }
+// Don't merge block with left brace wrapped after case labels
+if (TheLine->First->is(tok::l_brace) && I != AnnotatedLines.begin() &&
+I[-1]->First->isOneOf(tok::kw_case, tok::kw_default))
+  return 0;
 // Try to merge a block with left brace wrapped that wasn't yet covered
 if (TheLine->Last->is(tok::l_brace)) {
   return !Style.BraceWrapping.AfterFunction ||

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=342116&r1=342115&r2=342116&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Sep 13 00:27:15 2018
@@ -1064,6 +1064,32 @@ TEST_F(FormatTest, FormatsSwitchStatemen
"  return;\n"
"}",
getLLVMStyleWithColumns(34));
+
+  FormatStyle Style = getLLVMStyle();
+  Style.IndentCaseLabels = true;
+  Style.AllowShortBlocksOnASingleLine = false;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
+  EXPECT_EQ("switch (n)\n"
+"{\n"
+"  case 0:\n"
+"  {\n"
+"return false;\n"
+"  }\n"
+"  default:\n"
+"  {\n"
+"return true;\n"
+"  }\n"
+"}",
+format("switch (n) {\n"
+   "  case 0: {\n"
+   "return false;\n"
+   "  }\n"
+   "  default: {\n"
+   "return true;\n"
+   "  }\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, CaseRanges) {


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

[PATCH] D51719: Wrapped block after case label should not be merged into a single line (Bug 38854)

[Owen Pan via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342116: [clang-format] Wrapped block after case label should 
not be merged into one line (authored by owenpan, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51719?vs=164153&id=165208#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51719

Files:
  cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp


Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
@@ -323,6 +323,10 @@
   kwId == clang::tok::objc_synchronized)
 return 0;
 }
+// Don't merge block with left brace wrapped after case labels
+if (TheLine->First->is(tok::l_brace) && I != AnnotatedLines.begin() &&
+I[-1]->First->isOneOf(tok::kw_case, tok::kw_default))
+  return 0;
 // Try to merge a block with left brace wrapped that wasn't yet covered
 if (TheLine->Last->is(tok::l_brace)) {
   return !Style.BraceWrapping.AfterFunction ||
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -1064,6 +1064,32 @@
"  return;\n"
"}",
getLLVMStyleWithColumns(34));
+
+  FormatStyle Style = getLLVMStyle();
+  Style.IndentCaseLabels = true;
+  Style.AllowShortBlocksOnASingleLine = false;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
+  EXPECT_EQ("switch (n)\n"
+"{\n"
+"  case 0:\n"
+"  {\n"
+"return false;\n"
+"  }\n"
+"  default:\n"
+"  {\n"
+"return true;\n"
+"  }\n"
+"}",
+format("switch (n) {\n"
+   "  case 0: {\n"
+   "return false;\n"
+   "  }\n"
+   "  default: {\n"
+   "return true;\n"
+   "  }\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, CaseRanges) {


Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
@@ -323,6 +323,10 @@
   kwId == clang::tok::objc_synchronized)
 return 0;
 }
+// Don't merge block with left brace wrapped after case labels
+if (TheLine->First->is(tok::l_brace) && I != AnnotatedLines.begin() &&
+I[-1]->First->isOneOf(tok::kw_case, tok::kw_default))
+  return 0;
 // Try to merge a block with left brace wrapped that wasn't yet covered
 if (TheLine->Last->is(tok::l_brace)) {
   return !Style.BraceWrapping.AfterFunction ||
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -1064,6 +1064,32 @@
"  return;\n"
"}",
getLLVMStyleWithColumns(34));
+
+  FormatStyle Style = getLLVMStyle();
+  Style.IndentCaseLabels = true;
+  Style.AllowShortBlocksOnASingleLine = false;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
+  EXPECT_EQ("switch (n)\n"
+"{\n"
+"  case 0:\n"
+"  {\n"
+"return false;\n"
+"  }\n"
+"  default:\n"
+"  {\n"
+"return true;\n"
+"  }\n"
+"}",
+format("switch (n) {\n"
+   "  case 0: {\n"
+   "return false;\n"
+   "  }\n"
+   "  default: {\n"
+   "return true;\n"
+   "  }\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, CaseRanges) {
___
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 set up Clangd

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-tools-extra/docs/clangd.rst:111
 
+Editor Integration
+==

While we are here, would you mind including some other editors/plugins that are 
known to work with clangd (just so that people don't think vim is the only 
editor that works)? E.g. `Eglot` works on emacs, `vscode` apparently works as 
well. 

We should also mention that any other LSP clients should work with clangd in 
theory.


https://reviews.llvm.org/D51297



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

[PATCH] D51971: [clangd] Use JSON format in benchmark requests reader

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 165209.
kbobyrev marked 2 inline comments as done.

https://reviews.llvm.org/D51971

Files:
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/test/clangd/Inputs/requests.json
  clang-tools-extra/test/clangd/Inputs/requests.log
  clang-tools-extra/test/clangd/index-tools.test

Index: clang-tools-extra/test/clangd/index-tools.test
===
--- clang-tools-extra/test/clangd/index-tools.test
+++ clang-tools-extra/test/clangd/index-tools.test
@@ -1,3 +1,3 @@
 # RUN: global-symbol-builder %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs > %t.index
 # FIXME: By default, benchmarks are excluded from the list of default targets hence not built. Find a way to depend on benchmarks to run the next command.
-# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.log --benchmark_min_time=0.01 ; fi
+# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.json --benchmark_min_time=0.01 ; fi
Index: clang-tools-extra/test/clangd/Inputs/requests.log
===
--- clang-tools-extra/test/clangd/Inputs/requests.log
+++ /dev/null
@@ -1,5 +0,0 @@
-V[09:08:34.301] Code complete: fuzzyFind("s", scopes=[llvm::])
-V[09:08:34.368] Code complete: fuzzyFind("sy", scopes=[llvm::])
-V[09:08:34.442] Code complete: fuzzyFind("sys", scopes=[llvm::])
-V[09:09:12.075] Code complete: fuzzyFind("Dex", scopes=[clang::clangd::, clang::, clang::clangd::dex::])
-V[09:09:12.075] Code complete: fuzzyFind("Variable", scopes=[])
Index: clang-tools-extra/test/clangd/Inputs/requests.json
===
--- /dev/null
+++ clang-tools-extra/test/clangd/Inputs/requests.json
@@ -0,0 +1,7 @@
+[{"MaxCandidateCount":100,"ProximityPaths":["/usr/home/user/clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp"],"Query":"OMP","RestrictForCodeCompletion":true,"Scopes":["clang::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"s","RestrictForCodeCompletion":true,"Scopes":["llvm::", "::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sy","RestrictForCodeCompletion":true,"Scopes":["llvm::", "::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", "::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", "::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Dex","RestrictForCodeCompletion":true,"Scopes":["clang::clangd::", "clang::", "clang::clangd::dex::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Variable","RestrictForCodeCompletion":true,"Scopes":["::"]}]
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -19,56 +19,39 @@
 #include 
 
 const char *IndexFilename;
-const char *LogFilename;
+const char *RequestsFilename;
 
 namespace clang {
 namespace clangd {
 namespace {
 
-std::unique_ptr buildMem() {
-  return clang::clangd::loadIndex(IndexFilename, {}, false);
+std::unique_ptr buildMem() {
+  return loadIndex(IndexFilename, {}, false);
 }
 
-std::unique_ptr buildDex() {
-  return clang::clangd::loadIndex(IndexFilename, {}, true);
+std::unique_ptr buildDex() {
+  return loadIndex(IndexFilename, {}, true);
 }
 
-// This function processes user-provided Log file with fuzzy find requests in
-// the following format:
-//
-// fuzzyFind("UnqualifiedName", scopes=["clang::", "clang::clangd::"])
-//
-// It constructs vector of FuzzyFindRequests which is later used for the
-// benchmarks.
-std::vector extractQueriesFromLogs() {
-  llvm::Regex RequestMatcher("fuzzyFind\\(\"([a-zA-Z]*)\", scopes=\\[(.*)\\]");
-  llvm::SmallVector Matches;
-  std::ifstream InputStream(LogFilename);
+// Reads JSON array of serialized FuzzyFindRequest's from user-provided file.
+std::vector extractQueriesFromLogs() {
+  std::ifstream InputStream(RequestsFilename);
   std::string Log((std::istreambuf_iterator(InputStream)),
   std::istreambuf_iterator());
-  llvm::StringRef Temporary(Log);
-  llvm::SmallVector Strings;
-  Temporary.split(Strings, '\n');
 
-  clang::clangd::FuzzyFindRequest R;
-  R.MaxCandidateCount = 100;
+  std::vector Requests;
+  auto JSONArray = llvm::json::parse(Log);
 
-  llvm::SmallVector CommaSeparatedValues;
+  // Shutdown with an error if provided file couldn't be parsed.
+  if (!JSONArray || !JSONArray->getAsArray())
+llvm::errs() << "Couldn't parse request.\n";
 
-  std::vector RealRequests;
-  for (auto Line : Strings) {
-if (RequestMatcher.match(Line, &Matches)) {
-  R.Query = Matches[1];
-  CommaSeparatedV

[PATCH] D51971: [clangd] Use JSON format in benchmark requests reader

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:45
+  if (!JSONArray) {
+llvm::errs() << "Couldn't parse request.\n";
+  }

ilya-biryukov wrote:
> Return from function after error?
I thought that this should panic instead of returning empty `Requests`: 
otherwise it wouldn't be possible to detect problems in the benchmark driver in 
test, for example (it will just run benchmark over empty list of requests 
instead of doing something useful).


https://reviews.llvm.org/D51971



___
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 set up Clangd

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 165210.
kbobyrev marked an inline comment as done.

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,26 @@
 | Gen. Getters/Setters| No |   No |
 +-++--+
 
+Editor Integration
+==
+
+There are multiple editors and plugins that are known to work with Clangd, such
+as :program:`VSCode` and ``vscode-clangd`` extension, :program:`Emacs` and
+``Eglot`` plugin. Any full-featured LSP client should work with Clangd.
+
+Vim Integration
+---
+
+LanguageClient-neovim
+~
+
+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
+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,26 @@
 | Gen. Getters/Setters| No |   No |
 +-++--+
 
+Editor Integration
+==
+
+There are multiple editors and plugins that are known to work with Clangd, such
+as :program:`VSCode` and ``vscode-clangd`` extension, :program:`Emacs` and
+``Eglot`` plugin. Any full-featured LSP client should work with Clangd.
+
+Vim Integration
+---
+
+LanguageClient-neovim
+~
+
+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
+instructions.
+
 Getting Involved
 ==
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52016: [clangd] Don't create child AND and OR iterators with one posting list

[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, jkorous, MaskRay.

`AND( AND( Child ) ... )` -> `AND( Child ... )`
`AND( OR( Child) ... )` -> `AND( Child ... )`

This simple optimization results in 5-6% performance improvement in the 
benchmark with 2000 serialized `FuzzyFindRequest`s.


https://reviews.llvm.org/D52016

Files:
  clang-tools-extra/clangd/index/dex/Dex.cpp


Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -144,8 +144,10 @@
 if (It != InvertedIndex.end())
   TrigramIterators.push_back(create(It->second));
   }
-  if (!TrigramIterators.empty())
+  if (TrigramIterators.size() > 1)
 TopLevelChildren.push_back(createAnd(move(TrigramIterators)));
+  else if (TrigramIterators.size() == 1)
+TopLevelChildren.push_back(move(TrigramIterators.front()));
 
   // Generate scope tokens for search query.
   std::vector> ScopeIterators;
@@ -155,8 +157,10 @@
   ScopeIterators.push_back(create(It->second));
   }
   // Add OR iterator for scopes if there are any Scope Iterators.
-  if (!ScopeIterators.empty())
+  if (ScopeIterators.size() > 1)
 TopLevelChildren.push_back(createOr(move(ScopeIterators)));
+  else if (ScopeIterators.size() == 1)
+TopLevelChildren.push_back(move(ScopeIterators.front()));
 
   // Add proximity paths boosting.
   auto BoostingIterators = createFileProximityIterators(


Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -144,8 +144,10 @@
 if (It != InvertedIndex.end())
   TrigramIterators.push_back(create(It->second));
   }
-  if (!TrigramIterators.empty())
+  if (TrigramIterators.size() > 1)
 TopLevelChildren.push_back(createAnd(move(TrigramIterators)));
+  else if (TrigramIterators.size() == 1)
+TopLevelChildren.push_back(move(TrigramIterators.front()));
 
   // Generate scope tokens for search query.
   std::vector> ScopeIterators;
@@ -155,8 +157,10 @@
   ScopeIterators.push_back(create(It->second));
   }
   // Add OR iterator for scopes if there are any Scope Iterators.
-  if (!ScopeIterators.empty())
+  if (ScopeIterators.size() > 1)
 TopLevelChildren.push_back(createOr(move(ScopeIterators)));
+  else if (ScopeIterators.size() == 1)
+TopLevelChildren.push_back(move(ScopeIterators.front()));
 
   // Add proximity paths boosting.
   auto BoostingIterators = createFileProximityIterators(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51996: [clangd] Simplify cancellation public API

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

LGTM


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51996



___
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 set up Clangd

[Eric Liu via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/docs/clangd.rst:114
+
+There are multiple editors and plugins that are known to work with Clangd, such
+as :program:`VSCode` and ``vscode-clangd`` extension, :program:`Emacs` and

ioeric wrote:
> Thanks!
> 
> I would be nice if other editors can get their own sections like vim does :) 
> For example:
> 
> ```
> vscode
> (Something like vscode works well with clangd)
> 
> Vim
> ...
> 
> Emacs
> (Eglot)
> ```
*It* would be nice...


https://reviews.llvm.org/D51297



___
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 set up Clangd

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-tools-extra/docs/clangd.rst:114
+
+There are multiple editors and plugins that are known to work with Clangd, such
+as :program:`VSCode` and ``vscode-clangd`` extension, :program:`Emacs` and

Thanks!

I would be nice if other editors can get their own sections like vim does :) 
For example:

```
vscode
(Something like vscode works well with clangd)

Vim
...

Emacs
(Eglot)
```


https://reviews.llvm.org/D51297



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

[PATCH] D52016: [clangd] Don't create child AND and OR iterators with one posting list

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:148
+  if (TrigramIterators.size() > 1)
 TopLevelChildren.push_back(createAnd(move(TrigramIterators)));
+  else if (TrigramIterators.size() == 1)

Maybe special-case a single-iterator case in `createAnd` instead? Same with 
`createOr`.
Any cons to doing so?


https://reviews.llvm.org/D52016



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

[PATCH] D51971: [clangd] Use JSON format in benchmark requests reader

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:45
+  if (!JSONArray) {
+llvm::errs() << "Couldn't parse request.\n";
+  }

kbobyrev wrote:
> ilya-biryukov wrote:
> > Return from function after error?
> I thought that this should panic instead of returning empty `Requests`: 
> otherwise it wouldn't be possible to detect problems in the benchmark driver 
> in test, for example (it will just run benchmark over empty list of requests 
> instead of doing something useful).
Let's "panic" explicitly, e.g. with `exit(1)`.
We're currently triggering undefined behaviour, not something we want to do.


https://reviews.llvm.org/D51971



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

[PATCH] D52015: [XRay][clang] Always emit XRay attributes for LLVM IR

[Dean Michael Berris via Phabricator via cfe-commits]

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

This change is still incomplete -- we should really only convey that the 
'never' attribute gets preserved. Otherwise we're going to have to invent a way 
to communicate to the XRay pass in LLVM that we should ignore the XRay 
attributes.


https://reviews.llvm.org/D52015



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

[PATCH] D52015: [XRay][clang] Always emit XRay attributes for LLVM IR

[Marcus Boerger via Phabricator via cfe-commits]

mboerger accepted this revision.
mboerger added a comment.

LGTM


https://reviews.llvm.org/D52015



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

[PATCH] D52015: [XRay][clang] Always emit XRay attributes for LLVM IR

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 165219.
dberris added a subscriber: llvm-commits.
dberris added a comment.
This revision is now accepted and ready to land.

Adding an end-to-end test in compiler-rt to ensure that we are not suddenly 
instrumenting functions that must not be instrumented. Making this a 
version-locked commit between compiler-rt and clang.


https://reviews.llvm.org/D52015

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/xray-attributes-supported.cpp
  compiler-rt/test/xray/TestCases/Posix/clang-no-xray-instrument.cc

Index: compiler-rt/test/xray/TestCases/Posix/clang-no-xray-instrument.cc
===
--- /dev/null
+++ compiler-rt/test/xray/TestCases/Posix/clang-no-xray-instrument.cc
@@ -0,0 +1,11 @@
+// Test that we cannot actually find XRay instrumentation when we build with
+// -fno-xray-instrument but have code that's marked as 'xray_always_instrument'.
+//
+// RUN: %clangxx -fno-xray-instrument -c %s -o %t.o
+// RUN: not %llvm_xray extract -symbolize %t.o 2>&1 | FileCheck %s
+// REQUIRES: x86_64-target-arch
+// REQUIRES: built-in-llvm-tree
+
+// CHECK: llvm-xray: Cannot extract instrumentation map
+// CHECK-NOT: {{.*always_instrumented.*}}
+[[clang::xray_always_instrument]] int always_instrumented() { return 42; }
Index: clang/test/CodeGen/xray-attributes-supported.cpp
===
--- clang/test/CodeGen/xray-attributes-supported.cpp
+++ clang/test/CodeGen/xray-attributes-supported.cpp
@@ -1,19 +1,57 @@
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple x86_64-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mipsel-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64el-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple powerpc64le-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mipsel-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64el-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple powerpc64le-unknown-linux-gnu | FileCheck %s
+
+// We also want to ensure that the attributes show up even if we explicitly turn
+// off XRay instrumentation. We let the back-end decide whether to honour the
+// attributes instead.
+//
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple x86_64-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple arm-unknown-linux-gnu -target-abi apcs-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips-unknown-linux-gnu | \
+// RUN; FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mipsel-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64el-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple powerpc64le-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
 
 // Make sure that the LLVM attribute for 

[PATCH] D52015: [XRay][clang] Always emit XRay attributes for LLVM IR

[Dean Michael Berris via Phabricator via cfe-commits]

dberris updated this revision to Diff 165220.
dberris edited the summary of this revision.
dberris added a comment.

Revise description.


https://reviews.llvm.org/D52015

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/xray-attributes-supported.cpp
  compiler-rt/test/xray/TestCases/Posix/clang-no-xray-instrument.cc

Index: compiler-rt/test/xray/TestCases/Posix/clang-no-xray-instrument.cc
===
--- /dev/null
+++ compiler-rt/test/xray/TestCases/Posix/clang-no-xray-instrument.cc
@@ -0,0 +1,11 @@
+// Test that we cannot actually find XRay instrumentation when we build with
+// -fno-xray-instrument but have code that's marked as 'xray_always_instrument'.
+//
+// RUN: %clangxx -fno-xray-instrument -c %s -o %t.o
+// RUN: not %llvm_xray extract -symbolize %t.o 2>&1 | FileCheck %s
+// REQUIRES: x86_64-target-arch
+// REQUIRES: built-in-llvm-tree
+
+// CHECK: llvm-xray: Cannot extract instrumentation map
+// CHECK-NOT: {{.*always_instrumented.*}}
+[[clang::xray_always_instrument]] int always_instrumented() { return 42; }
Index: clang/test/CodeGen/xray-attributes-supported.cpp
===
--- clang/test/CodeGen/xray-attributes-supported.cpp
+++ clang/test/CodeGen/xray-attributes-supported.cpp
@@ -1,19 +1,57 @@
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple x86_64-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mipsel-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple mips64el-unknown-linux-gnu | FileCheck %s
-// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - -triple powerpc64le-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple x86_64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple arm-unknown-linux-gnu -target-abi apcs-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mipsel-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64el-unknown-linux-gnu | FileCheck %s
+// RUN: %clang_cc1 %s -fxray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple powerpc64le-unknown-linux-gnu | FileCheck %s
+
+// We also want to ensure that the attributes show up even if we explicitly turn
+// off XRay instrumentation. We let the back-end decide whether to honour the
+// attributes instead.
+//
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple x86_64-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple arm-unknown-linux-gnu -target-abi apcs-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips-unknown-linux-gnu | \
+// RUN; FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mipsel-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple mips64el-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
+// RUN: %clang_cc1 %s -fno-xray-instrument -std=c++11 -x c++ -emit-llvm -o - \
+// RUN: -triple powerpc64le-unknown-linux-gnu | \
+// RUN: FileCheck --check-prefix=NOXRAY %s
 
 // Make sure that the LLVM attribute for XRay-annotated functions do show up.
 [[clang::xray_always_instrument]] void foo() {
 // CHECK: define void @_Z3foov() #0
+// NOXRAY: define void @_Z3foov() #0
 };
 
 [[clang::xray_never_instrument]] void bar() {
 // CHECK: de

[PATCH] D51982: [clangd] Introduce PostingList interface

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Add documentation, don't expose `PostingList` interface implementations in the 
public API (similarly to `Iterator`).


https://reviews.llvm.org/D51982

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/Dex.cpp
  clang-tools-extra/clangd/index/dex/Dex.h
  clang-tools-extra/clangd/index/dex/Iterator.cpp
  clang-tools-extra/clangd/index/dex/Iterator.h
  clang-tools-extra/clangd/index/dex/PostingList.cpp
  clang-tools-extra/clangd/index/dex/PostingList.h
  clang-tools-extra/unittests/clangd/DexTests.cpp

Index: clang-tools-extra/unittests/clangd/DexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexTests.cpp
@@ -47,8 +47,8 @@
 }
 
 TEST(DexIterators, DocumentIterator) {
-  const PostingList L = {4, 7, 8, 20, 42, 100};
-  auto DocIterator = create(L);
+  const auto L = buildSparsePostingList({4, 7, 8, 20, 42, 100});
+  auto DocIterator = L->iterator();
 
   EXPECT_EQ(DocIterator->peek(), 4U);
   EXPECT_FALSE(DocIterator->reachedEnd());
@@ -70,28 +70,28 @@
 }
 
 TEST(DexIterators, AndWithEmpty) {
-  const PostingList L0;
-  const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000};
+  const auto L0 = buildSparsePostingList({});
+  const auto L1 = buildSparsePostingList({0, 5, 7, 10, 42, 320, 9000});
 
-  auto AndEmpty = createAnd(create(L0));
+  auto AndEmpty = createAnd(L0->iterator());
   EXPECT_TRUE(AndEmpty->reachedEnd());
 
-  auto AndWithEmpty = createAnd(create(L0), create(L1));
+  auto AndWithEmpty = createAnd(L0->iterator(), L1->iterator());
   EXPECT_TRUE(AndWithEmpty->reachedEnd());
 
   EXPECT_THAT(consumeIDs(*AndWithEmpty), ElementsAre());
 }
 
 TEST(DexIterators, AndTwoLists) {
-  const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000};
-  const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000};
+  const auto L0 = buildSparsePostingList({0, 5, 7, 10, 42, 320, 9000});
+  const auto L1 = buildSparsePostingList({0, 4, 7, 10, 30, 60, 320, 9000});
 
-  auto And = createAnd(create(L1), create(L0));
+  auto And = createAnd(L1->iterator(), L0->iterator());
 
   EXPECT_FALSE(And->reachedEnd());
   EXPECT_THAT(consumeIDs(*And), ElementsAre(0U, 7U, 10U, 320U, 9000U));
 
-  And = createAnd(create(L0), create(L1));
+  And = createAnd(L0->iterator(), L1->iterator());
 
   And->advanceTo(0);
   EXPECT_EQ(And->peek(), 0U);
@@ -107,11 +107,11 @@
 }
 
 TEST(DexIterators, AndThreeLists) {
-  const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000};
-  const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000};
-  const PostingList L2 = {1, 4, 7, 11, 30, 60, 320, 9000};
+  const auto L0 = buildSparsePostingList({0, 5, 7, 10, 42, 320, 9000});
+  const auto L1 = buildSparsePostingList({0, 4, 7, 10, 30, 60, 320, 9000});
+  const auto L2 = buildSparsePostingList({1, 4, 7, 11, 30, 60, 320, 9000});
 
-  auto And = createAnd(create(L0), create(L1), create(L2));
+  auto And = createAnd(L0->iterator(), L1->iterator(), L2->iterator());
   EXPECT_EQ(And->peek(), 7U);
   And->advanceTo(300);
   EXPECT_EQ(And->peek(), 320U);
@@ -121,24 +121,24 @@
 }
 
 TEST(DexIterators, OrWithEmpty) {
-  const PostingList L0;
-  const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000};
+  const auto L0 = buildSparsePostingList({});
+  const auto L1 = buildSparsePostingList({0, 5, 7, 10, 42, 320, 9000});
 
-  auto OrEmpty = createOr(create(L0));
+  auto OrEmpty = createOr(L0->iterator());
   EXPECT_TRUE(OrEmpty->reachedEnd());
 
-  auto OrWithEmpty = createOr(create(L0), create(L1));
+  auto OrWithEmpty = createOr(L0->iterator(), L1->iterator());
   EXPECT_FALSE(OrWithEmpty->reachedEnd());
 
   EXPECT_THAT(consumeIDs(*OrWithEmpty),
   ElementsAre(0U, 5U, 7U, 10U, 42U, 320U, 9000U));
 }
 
 TEST(DexIterators, OrTwoLists) {
-  const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000};
-  const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000};
+  const auto L0 = buildSparsePostingList({0, 5, 7, 10, 42, 320, 9000});
+  const auto L1 = buildSparsePostingList({0, 4, 7, 10, 30, 60, 320, 9000});
 
-  auto Or = createOr(create(L0), create(L1));
+  auto Or = createOr(L0->iterator(), L1->iterator());
 
   EXPECT_FALSE(Or->reachedEnd());
   EXPECT_EQ(Or->peek(), 0U);
@@ -161,18 +161,18 @@
   Or->advanceTo(9001);
   EXPECT_TRUE(Or->reachedEnd());
 
-  Or = createOr(create(L0), create(L1));
+  Or = createOr(L0->iterator(), L1->iterator());
 
   EXPECT_THAT(consumeIDs(*Or),
   ElementsAre(0U, 4U, 5U, 7U, 10U, 30U, 42U, 60U, 320U, 9000U));
 }
 
 TEST(DexIterators, OrThreeLists) {
-  const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000};
-  const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000};
-  const PostingList L2 = {1, 4, 7, 11, 30, 60, 320, 9000};
+  const auto L0 = buildSparsePostingList({0, 5, 7, 10, 42, 320, 9000});
+  const auto L1 = buildSparsePostingList({0, 4, 7, 10, 30, 60, 320, 9000});
+  const auto L2 = buildSparsePostingList

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

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Create different sections for few editors, link web page with the complete list 
of LSP clients.


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,40 @@
 | Gen. Getters/Setters| No |   No |
 +-++--+
 
+Editor Integration
+==
+
+Any full-featured Language Server Protocol Client implementation should work
+with :program:`Clangd`. This `list
+` contains information about
+extensions and plugins that are known to work for different editors.
+
+Vim Integration
+---
+
+LanguageClient-neovim
+~
+
+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
+instructions.
+
+VSCode Integration
+--
+
+:program:`VSCode` provides `vscode-clangd
+`
+which is developed as a part of Clang-Tools-Extra source tree.
+
+Emacs Integration
+-
+
+:program:`Emacs` provides `lsp-mode ` and
+`Eglot ` plugins for LSP integration.
+
 Getting Involved
 ==
 


Index: clang-tools-extra/docs/clangd.rst
===
--- clang-tools-extra/docs/clangd.rst
+++ clang-tools-extra/docs/clangd.rst
@@ -108,6 +108,40 @@
 | Gen. Getters/Setters| No |   No |
 +-++--+
 
+Editor Integration
+==
+
+Any full-featured Language Server Protocol Client implementation should work
+with :program:`Clangd`. This `list
+` contains information about
+extensions and plugins that are known to work for different editors.
+
+Vim Integration
+---
+
+LanguageClient-neovim
+~
+
+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
+instructions.
+
+VSCode Integration
+--
+
+:program:`VSCode` provides `vscode-clangd
+`
+which is developed as a part of Clang-Tools-Extra source tree.
+
+Emacs Integration
+-
+
+:program:`Emacs` provides `lsp-mode ` and
+`Eglot ` plugins for LSP integration.
+
 Getting Involved
 ==
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51990: [DebugInfo] Fix emitting of bit offset for ObjC

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:2369
+  ? CGM.getObjCRuntime().ComputeBitfieldBitOffset(CGM, ID, Field)
+  : 0;
 } else {

aprantl wrote:
> JDevlieghere wrote:
> > JDevlieghere wrote:
> > > aprantl wrote:
> > > > aprantl wrote:
> > > > > It might help to attempt some git blame archeology.
> > > > > Judging from the comment, it sounds like the debugger is supposed to 
> > > > > query the runtime for the *byte* offset and then add the bit offset 
> > > > > from DWARF? Could that make sense?
> > > > If that is the case, we'd need to relax llvm-dwarfdump --verify to 
> > > > accept this and make sure LLDB does the right thing instead.
> > > Ah I see, yeah that sounds reasonable and explains the comment which I 
> > > interpreted differently. Thanks! 
> > btw it was added in rL167503. 
> We should check whether emitting the offsets like this violates the DWARF 
> spec. If yes, it may be better to emit the static offsets like you are doing 
> here and then still have LLDB ignore everything but the bit-offsets from the 
> dynamic byte offset.
The standard says 

> The member entry corresponding to a data member that is defined in a 
> structure,
> union or class may have either a DW_AT_data_member_location attribute or a
> DW_AT_data_bit_offset attribute.

which to me sounds like they should be mutually exclusive. I ran the lldb test 
suite with my change and there were no new failures, which leads me to believe 
that the comment from r167503 still holds and lldb ignores this attribute, at 
least for Objective-C.


Repository:
  rC Clang

https://reviews.llvm.org/D51990



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

[PATCH] D52008: [analyzer] Handle forwarding reference better in ExprMutationAnalyzer.

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Thanks for working on this! I tried, and it appears to not fix the issue at 
hand.

-

  struct C1 {
C1(const C1* c, int num);
  };
  
  int x = 0;
  auto y = std::make_unique(nullptr, x); // <- still considered a mutation?

-

  struct C3 {}; // some class
  
  struct C2 {
C2(const int* whatever, int n, C3 zz);
  };
  
  int x = 0;
  std::vector v;
  v.emplace_back(nullptr, x, {}); // <- still considered a mutation?

And so on. These are hand-reduced, so hopefully you can reproduce?




Comment at: include/clang/Analysis/Analyses/ExprMutationAnalyzer.h:32
   const Stmt *findMutation(const Expr *Exp);
+  const Stmt *findDeclMutation(const Decl *Dec);
 

Thanks!
I know this has performance implications, but those will exist even if one has 
this in his own code.


Repository:
  rC Clang

https://reviews.llvm.org/D52008



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

[PATCH] D52016: [clangd] Don't create child AND and OR iterators with one posting list

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:148
+  if (TrigramIterators.size() > 1)
 TopLevelChildren.push_back(createAnd(move(TrigramIterators)));
+  else if (TrigramIterators.size() == 1)

ilya-biryukov wrote:
> Maybe special-case a single-iterator case in `createAnd` instead? Same with 
> `createOr`.
> Any cons to doing so?
I thought that this might result in some implicit query tree generation 
semantics. However, since there optimizations are likely to be added at some 
point, I guess we could start being implicit right now, especially given that 
there is no good way for the user code to inspect the query tree structure (and 
there's no good reason to do so, too).


https://reviews.llvm.org/D52016



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

[PATCH] D52016: [clangd] Don't create child AND and OR iterators with one posting list

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 165229.
kbobyrev marked an inline comment as done.

https://reviews.llvm.org/D52016

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


Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -90,7 +90,6 @@
 public:
   explicit AndIterator(std::vector> AllChildren)
   : Children(std::move(AllChildren)) {
-assert(!Children.empty() && "AND iterator should have at least one 
child.");
 // Establish invariants.
 sync();
 // When children are sorted by the estimateSize(), sync() calls are more
@@ -197,9 +196,7 @@
 class OrIterator : public Iterator {
 public:
   explicit OrIterator(std::vector> AllChildren)
-  : Children(std::move(AllChildren)) {
-assert(Children.size() > 0 && "OR iterator must have at least one child.");
-  }
+  : Children(std::move(AllChildren)) {}
 
   /// Returns true if all children are exhausted.
   bool reachedEnd() const override {
@@ -405,12 +402,18 @@
 
 std::unique_ptr
 createAnd(std::vector> Children) {
-  return llvm::make_unique(move(Children));
+  assert(!Children.empty() && "AND iterator should have at least one child.");
+  // If there is exactly one child, pull it one level up: AND(Child) -> Child.
+  return Children.size() == 1 ? std::move(Children.front())
+  : llvm::make_unique(move(Children));
 }
 
 std::unique_ptr
 createOr(std::vector> Children) {
-  return llvm::make_unique(move(Children));
+  assert(!Children.empty() && "OR iterator should have at least one child.");
+  // If there is exactly one child, pull it one level up: OR(Child) -> Child.
+  return Children.size() == 1 ? std::move(Children.front())
+  : llvm::make_unique(move(Children));
 }
 
 std::unique_ptr createTrue(DocID Size) {


Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -90,7 +90,6 @@
 public:
   explicit AndIterator(std::vector> AllChildren)
   : Children(std::move(AllChildren)) {
-assert(!Children.empty() && "AND iterator should have at least one child.");
 // Establish invariants.
 sync();
 // When children are sorted by the estimateSize(), sync() calls are more
@@ -197,9 +196,7 @@
 class OrIterator : public Iterator {
 public:
   explicit OrIterator(std::vector> AllChildren)
-  : Children(std::move(AllChildren)) {
-assert(Children.size() > 0 && "OR iterator must have at least one child.");
-  }
+  : Children(std::move(AllChildren)) {}
 
   /// Returns true if all children are exhausted.
   bool reachedEnd() const override {
@@ -405,12 +402,18 @@
 
 std::unique_ptr
 createAnd(std::vector> Children) {
-  return llvm::make_unique(move(Children));
+  assert(!Children.empty() && "AND iterator should have at least one child.");
+  // If there is exactly one child, pull it one level up: AND(Child) -> Child.
+  return Children.size() == 1 ? std::move(Children.front())
+  : llvm::make_unique(move(Children));
 }
 
 std::unique_ptr
 createOr(std::vector> Children) {
-  return llvm::make_unique(move(Children));
+  assert(!Children.empty() && "OR iterator should have at least one child.");
+  // If there is exactly one child, pull it one level up: OR(Child) -> Child.
+  return Children.size() == 1 ? std::move(Children.front())
+  : llvm::make_unique(move(Children));
 }
 
 std::unique_ptr createTrue(DocID Size) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51949: [WIP][clang-tidy] initial ideas to isolate variable declarations

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

In https://reviews.llvm.org/D51949#1232443, @kbobyrev wrote:

> I've been thinking about corner-cases (e.g. don't split `DeclStmt`s within 
> init-statement declaration) a while and it seems that there might be many of 
> them.


Yes, things I am currently thinnking of:

- `if (condition) int i, j, k = function(i,j);` is not allowed to be 
transformed, as it will change semantics.
- `for(int i, j; ..)` similar case
- the new C++17 if-init
- Everything related with Macros

I want the transformation and run it over with good test-suites and see what 
happens :)

> I didn't notice https://reviews.llvm.org/D27621 in the first place, it seems 
> to have a solid test suite (and it also does some basic formatting, which is 
> nice). It might be worthy to pick it up where it was left. Alternatively, you 
> might want to pull the test cases from that patch if you don't want to reuse 
> the code.

Totally, the test-suite looks very good. Its missing some VarDecl-FunctionDecl 
mixing and cases like this, but I plan to at least use the tests. You right 
with the basic formatting, at least indendation and new lines (as the other 
check does) are worth it.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51949



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

[PATCH] D51971: [clangd] Use JSON format in benchmark requests reader

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:45
+  if (!JSONArray) {
+llvm::errs() << "Couldn't parse request.\n";
+  }

ilya-biryukov wrote:
> kbobyrev wrote:
> > ilya-biryukov wrote:
> > > Return from function after error?
> > I thought that this should panic instead of returning empty `Requests`: 
> > otherwise it wouldn't be possible to detect problems in the benchmark 
> > driver in test, for example (it will just run benchmark over empty list of 
> > requests instead of doing something useful).
> Let's "panic" explicitly, e.g. with `exit(1)`.
> We're currently triggering undefined behaviour, not something we want to do.
Good point; fixed!


https://reviews.llvm.org/D51971



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

[clang-tools-extra] r342123 - [clangd] Rename global-symbol-builder to clangd-indexer.

[Ilya Biryukov via cfe-commits]

Author: ibiryukov
Date: Thu Sep 13 02:44:11 2018
New Revision: 342123

URL: http://llvm.org/viewvc/llvm-project?rev=342123&view=rev
Log:
[clangd] Rename global-symbol-builder to clangd-indexer.

Summary:
Given that the indexer binary is put directly into ./bin directory
when built, 'clangd-' prefix seems to provide better context to the
reader than 'global-'.

The new name is also shorter and easier to type.

Reviewers: ioeric, sammccall, kadircet

Reviewed By: ioeric, sammccall

Subscribers: kbobyrev, ilya-biryukov, mgorny, MaskRay, jkorous, arphaman, 
cfe-commits

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

Added:
clang-tools-extra/trunk/clangd/indexer/
clang-tools-extra/trunk/clangd/indexer/CMakeLists.txt
  - copied, changed from r342052, 
clang-tools-extra/trunk/clangd/global-symbol-builder/CMakeLists.txt
clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
  - copied, changed from r342052, 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
Removed:
clang-tools-extra/trunk/clangd/global-symbol-builder/CMakeLists.txt

clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
Modified:
clang-tools-extra/trunk/clangd/CMakeLists.txt
clang-tools-extra/trunk/test/CMakeLists.txt
clang-tools-extra/trunk/test/clangd/index-tools.test

Modified: clang-tools-extra/trunk/clangd/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/CMakeLists.txt?rev=342123&r1=342122&r2=342123&view=diff
==
--- clang-tools-extra/trunk/clangd/CMakeLists.txt (original)
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt Thu Sep 13 02:44:11 2018
@@ -73,7 +73,7 @@ if( LLVM_LIB_FUZZING_ENGINE OR LLVM_USE_
   add_subdirectory(fuzzer)
 endif()
 add_subdirectory(tool)
-add_subdirectory(global-symbol-builder)
+add_subdirectory(indexer)
 add_subdirectory(index/dex/dexp)
 
 if (LLVM_INCLUDE_BENCHMARKS)

Removed: clang-tools-extra/trunk/clangd/global-symbol-builder/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/global-symbol-builder/CMakeLists.txt?rev=342122&view=auto
==
--- clang-tools-extra/trunk/clangd/global-symbol-builder/CMakeLists.txt 
(original)
+++ clang-tools-extra/trunk/clangd/global-symbol-builder/CMakeLists.txt 
(removed)
@@ -1,20 +0,0 @@
-include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../)
-
-set(LLVM_LINK_COMPONENTS
-Support
-)
-
-add_clang_executable(global-symbol-builder
-  GlobalSymbolBuilderMain.cpp
-  )
-
-target_link_libraries(global-symbol-builder
-  PRIVATE
-  clangAST
-  clangIndex
-  clangDaemon
-  clangBasic
-  clangFrontend
-  clangLex
-  clangTooling
-)

Removed: 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?rev=342122&view=auto
==
--- 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
 (original)
+++ 
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
 (removed)
@@ -1,285 +0,0 @@
-//===--- GlobalSymbolBuilderMain.cpp -*- 
C++-*-===//
-//
-// The LLVM Compiler Infrastructure
-//
-// This file is distributed under the University of Illinois Open Source
-// License. See LICENSE.TXT for details.
-//
-//===--===//
-//
-// GlobalSymbolBuilder is a tool to extract symbols from a whole project.
-// This tool is **experimental** only. Don't use it in production code.
-//
-//===--===//
-
-#include "RIFF.h"
-#include "index/CanonicalIncludes.h"
-#include "index/Index.h"
-#include "index/Merge.h"
-#include "index/Serialization.h"
-#include "index/SymbolCollector.h"
-#include "index/SymbolYAML.h"
-#include "clang/Frontend/CompilerInstance.h"
-#include "clang/Frontend/FrontendActions.h"
-#include "clang/Index/IndexDataConsumer.h"
-#include "clang/Index/IndexingAction.h"
-#include "clang/Tooling/CommonOptionsParser.h"
-#include "clang/Tooling/Execution.h"
-#include "clang/Tooling/Tooling.h"
-#include "llvm/Support/CommandLine.h"
-#include "llvm/Support/MemoryBuffer.h"
-#include "llvm/Support/Path.h"
-#include "llvm/Support/Signals.h"
-#include "llvm/Support/ThreadPool.h"
-#include "llvm/Support/YAMLTraits.h"
-
-using namespace llvm;
-using namespace clang::tooling;
-using clang::clangd::SymbolSlab;
-
-namespace clang {
-namespace clangd {
-namespace {
-
-static llvm::cl::opt AssumedHeaderDir(
-"assume-header-dir",
-llvm::cl::desc("The index includes header that a symbol is defined in. "
-   "If the abs

[PATCH] D51987: [clangd] Rename global-symbol-builder to clangd-indexer.

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342123: [clangd] Rename global-symbol-builder to 
clangd-indexer. (authored by ibiryukov, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D51987?vs=165092&id=165233#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D51987

Files:
  clang-tools-extra/trunk/clangd/CMakeLists.txt
  clang-tools-extra/trunk/clangd/global-symbol-builder/CMakeLists.txt
  
clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
  clang-tools-extra/trunk/clangd/indexer/CMakeLists.txt
  clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
  clang-tools-extra/trunk/test/CMakeLists.txt
  clang-tools-extra/trunk/test/clangd/index-tools.test

Index: clang-tools-extra/trunk/clangd/CMakeLists.txt
===
--- clang-tools-extra/trunk/clangd/CMakeLists.txt
+++ clang-tools-extra/trunk/clangd/CMakeLists.txt
@@ -73,7 +73,7 @@
   add_subdirectory(fuzzer)
 endif()
 add_subdirectory(tool)
-add_subdirectory(global-symbol-builder)
+add_subdirectory(indexer)
 add_subdirectory(index/dex/dexp)
 
 if (LLVM_INCLUDE_BENCHMARKS)
Index: clang-tools-extra/trunk/clangd/indexer/CMakeLists.txt
===
--- clang-tools-extra/trunk/clangd/indexer/CMakeLists.txt
+++ clang-tools-extra/trunk/clangd/indexer/CMakeLists.txt
@@ -0,0 +1,20 @@
+include_directories(${CMAKE_CURRENT_SOURCE_DIR}/../)
+
+set(LLVM_LINK_COMPONENTS
+Support
+)
+
+add_clang_executable(clangd-indexer
+  IndexerMain.cpp
+  )
+
+target_link_libraries(clangd-indexer
+  PRIVATE
+  clangAST
+  clangIndex
+  clangDaemon
+  clangBasic
+  clangFrontend
+  clangLex
+  clangTooling
+)
Index: clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
===
--- clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
+++ clang-tools-extra/trunk/clangd/indexer/IndexerMain.cpp
@@ -0,0 +1,285 @@
+//===--- GlobalSymbolBuilderMain.cpp -*- C++-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+// GlobalSymbolBuilder is a tool to extract symbols from a whole project.
+// This tool is **experimental** only. Don't use it in production code.
+//
+//===--===//
+
+#include "RIFF.h"
+#include "index/CanonicalIncludes.h"
+#include "index/Index.h"
+#include "index/Merge.h"
+#include "index/Serialization.h"
+#include "index/SymbolCollector.h"
+#include "index/SymbolYAML.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendActions.h"
+#include "clang/Index/IndexDataConsumer.h"
+#include "clang/Index/IndexingAction.h"
+#include "clang/Tooling/CommonOptionsParser.h"
+#include "clang/Tooling/Execution.h"
+#include "clang/Tooling/Tooling.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/Signals.h"
+#include "llvm/Support/ThreadPool.h"
+#include "llvm/Support/YAMLTraits.h"
+
+using namespace llvm;
+using namespace clang::tooling;
+using clang::clangd::SymbolSlab;
+
+namespace clang {
+namespace clangd {
+namespace {
+
+static llvm::cl::opt AssumedHeaderDir(
+"assume-header-dir",
+llvm::cl::desc("The index includes header that a symbol is defined in. "
+   "If the absolute path cannot be determined (e.g. an "
+   "in-memory VFS) then the relative path is resolved against "
+   "this directory, which must be absolute. If this flag is "
+   "not given, such headers will have relative paths."),
+llvm::cl::init(""));
+
+static llvm::cl::opt MergeOnTheFly(
+"merge-on-the-fly",
+llvm::cl::desc(
+"Merges symbols for each processed translation unit as soon "
+"they become available. This results in a smaller memory "
+"usage and an almost instant reduce stage. Optimal for running as a "
+"standalone tool, but cannot be used with multi-process executors like "
+"MapReduce."),
+llvm::cl::init(true), llvm::cl::Hidden);
+
+enum IndexFormat { YAML, Binary };
+static llvm::cl::opt Format(
+"format", llvm::cl::desc("Format of the index to be written"),
+llvm::cl::values(clEnumValN(YAML, "yaml", "human-readable YAML format"),
+ clEnumValN(Binary, "binary", "binary RIFF format")),
+llvm::cl::init(YAML));
+
+/// Responsible for aggregating symbols from each processed file and producing
+/// the final results. All methods in this class must be thread-safe,
+/// 'consumeSymbols' may be called from mu

[PATCH] D51971: [clangd] Use JSON format in benchmark requests reader

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 165232.
kbobyrev marked 3 inline comments as done.

https://reviews.llvm.org/D51971

Files:
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/test/clangd/Inputs/requests.json
  clang-tools-extra/test/clangd/Inputs/requests.log
  clang-tools-extra/test/clangd/index-tools.test

Index: clang-tools-extra/test/clangd/index-tools.test
===
--- clang-tools-extra/test/clangd/index-tools.test
+++ clang-tools-extra/test/clangd/index-tools.test
@@ -1,3 +1,3 @@
 # RUN: global-symbol-builder %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs > %t.index
 # FIXME: By default, benchmarks are excluded from the list of default targets hence not built. Find a way to depend on benchmarks to run the next command.
-# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.log --benchmark_min_time=0.01 ; fi
+# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.json --benchmark_min_time=0.01 ; fi
Index: clang-tools-extra/test/clangd/Inputs/requests.log
===
--- clang-tools-extra/test/clangd/Inputs/requests.log
+++ /dev/null
@@ -1,5 +0,0 @@
-V[09:08:34.301] Code complete: fuzzyFind("s", scopes=[llvm::])
-V[09:08:34.368] Code complete: fuzzyFind("sy", scopes=[llvm::])
-V[09:08:34.442] Code complete: fuzzyFind("sys", scopes=[llvm::])
-V[09:09:12.075] Code complete: fuzzyFind("Dex", scopes=[clang::clangd::, clang::, clang::clangd::dex::])
-V[09:09:12.075] Code complete: fuzzyFind("Variable", scopes=[])
Index: clang-tools-extra/test/clangd/Inputs/requests.json
===
--- /dev/null
+++ clang-tools-extra/test/clangd/Inputs/requests.json
@@ -0,0 +1,7 @@
+[{"MaxCandidateCount":100,"ProximityPaths":["/usr/home/user/clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp"],"Query":"OMP","RestrictForCodeCompletion":true,"Scopes":["clang::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"s","RestrictForCodeCompletion":true,"Scopes":["llvm::", "::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sy","RestrictForCodeCompletion":true,"Scopes":["llvm::", "::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", "::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", "::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Dex","RestrictForCodeCompletion":true,"Scopes":["clang::clangd::", "clang::", "clang::clangd::dex::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Variable","RestrictForCodeCompletion":true,"Scopes":["::"]}]
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -19,56 +19,41 @@
 #include 
 
 const char *IndexFilename;
-const char *LogFilename;
+const char *RequestsFilename;
 
 namespace clang {
 namespace clangd {
 namespace {
 
-std::unique_ptr buildMem() {
-  return clang::clangd::loadIndex(IndexFilename, {}, false);
+std::unique_ptr buildMem() {
+  return loadIndex(IndexFilename, {}, false);
 }
 
-std::unique_ptr buildDex() {
-  return clang::clangd::loadIndex(IndexFilename, {}, true);
+std::unique_ptr buildDex() {
+  return loadIndex(IndexFilename, {}, true);
 }
 
-// This function processes user-provided Log file with fuzzy find requests in
-// the following format:
-//
-// fuzzyFind("UnqualifiedName", scopes=["clang::", "clang::clangd::"])
-//
-// It constructs vector of FuzzyFindRequests which is later used for the
-// benchmarks.
-std::vector extractQueriesFromLogs() {
-  llvm::Regex RequestMatcher("fuzzyFind\\(\"([a-zA-Z]*)\", scopes=\\[(.*)\\]");
-  llvm::SmallVector Matches;
-  std::ifstream InputStream(LogFilename);
+// Reads JSON array of serialized FuzzyFindRequest's from user-provided file.
+std::vector extractQueriesFromLogs() {
+  std::ifstream InputStream(RequestsFilename);
   std::string Log((std::istreambuf_iterator(InputStream)),
   std::istreambuf_iterator());
-  llvm::StringRef Temporary(Log);
-  llvm::SmallVector Strings;
-  Temporary.split(Strings, '\n');
 
-  clang::clangd::FuzzyFindRequest R;
-  R.MaxCandidateCount = 100;
+  std::vector Requests;
+  auto JSONArray = llvm::json::parse(Log);
 
-  llvm::SmallVector CommaSeparatedValues;
+  // Panic if the provided file couldn't be parsed.
+  if (!JSONArray || !JSONArray->getAsArray()) {
+llvm::errs() << "FATAL ERROR: Couldn't parse request.\n";
+exit(1);
+  }
 
-  std::vector RealRequests;
-  for (auto Line : Strings) {
-if (RequestMatcher.match(Line, &Matches)) {
-  R.Query = Matches[1];
-

[PATCH] D51971: [clangd] Use JSON format in benchmark requests reader

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Rebase on top of https://reviews.llvm.org/rL342123.


https://reviews.llvm.org/D51971

Files:
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/test/clangd/Inputs/requests.json
  clang-tools-extra/test/clangd/Inputs/requests.log
  clang-tools-extra/test/clangd/index-tools.test

Index: clang-tools-extra/test/clangd/index-tools.test
===
--- clang-tools-extra/test/clangd/index-tools.test
+++ clang-tools-extra/test/clangd/index-tools.test
@@ -1,3 +1,3 @@
-# RUN: global-symbol-builder %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs > %t.index
+# RUN: clangd-indexer %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs > %t.index
 # FIXME: By default, benchmarks are excluded from the list of default targets hence not built. Find a way to depend on benchmarks to run the next command.
-# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.log --benchmark_min_time=0.01 ; fi
+# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.json --benchmark_min_time=0.01 ; fi
Index: clang-tools-extra/test/clangd/Inputs/requests.log
===
--- clang-tools-extra/test/clangd/Inputs/requests.log
+++ /dev/null
@@ -1,5 +0,0 @@
-V[09:08:34.301] Code complete: fuzzyFind("s", scopes=[llvm::])
-V[09:08:34.368] Code complete: fuzzyFind("sy", scopes=[llvm::])
-V[09:08:34.442] Code complete: fuzzyFind("sys", scopes=[llvm::])
-V[09:09:12.075] Code complete: fuzzyFind("Dex", scopes=[clang::clangd::, clang::, clang::clangd::dex::])
-V[09:09:12.075] Code complete: fuzzyFind("Variable", scopes=[])
Index: clang-tools-extra/test/clangd/Inputs/requests.json
===
--- /dev/null
+++ clang-tools-extra/test/clangd/Inputs/requests.json
@@ -0,0 +1,7 @@
+[{"MaxCandidateCount":100,"ProximityPaths":["/usr/home/user/clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp"],"Query":"OMP","RestrictForCodeCompletion":true,"Scopes":["clang::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"s","RestrictForCodeCompletion":true,"Scopes":["llvm::", "::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sy","RestrictForCodeCompletion":true,"Scopes":["llvm::", "::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", "::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", "::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Dex","RestrictForCodeCompletion":true,"Scopes":["clang::clangd::", "clang::", "clang::clangd::dex::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Variable","RestrictForCodeCompletion":true,"Scopes":["::"]}]
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -19,56 +19,41 @@
 #include 
 
 const char *IndexFilename;
-const char *LogFilename;
+const char *RequestsFilename;
 
 namespace clang {
 namespace clangd {
 namespace {
 
-std::unique_ptr buildMem() {
-  return clang::clangd::loadIndex(IndexFilename, {}, false);
+std::unique_ptr buildMem() {
+  return loadIndex(IndexFilename, {}, false);
 }
 
-std::unique_ptr buildDex() {
-  return clang::clangd::loadIndex(IndexFilename, {}, true);
+std::unique_ptr buildDex() {
+  return loadIndex(IndexFilename, {}, true);
 }
 
-// This function processes user-provided Log file with fuzzy find requests in
-// the following format:
-//
-// fuzzyFind("UnqualifiedName", scopes=["clang::", "clang::clangd::"])
-//
-// It constructs vector of FuzzyFindRequests which is later used for the
-// benchmarks.
-std::vector extractQueriesFromLogs() {
-  llvm::Regex RequestMatcher("fuzzyFind\\(\"([a-zA-Z]*)\", scopes=\\[(.*)\\]");
-  llvm::SmallVector Matches;
-  std::ifstream InputStream(LogFilename);
+// Reads JSON array of serialized FuzzyFindRequest's from user-provided file.
+std::vector extractQueriesFromLogs() {
+  std::ifstream InputStream(RequestsFilename);
   std::string Log((std::istreambuf_iterator(InputStream)),
   std::istreambuf_iterator());
-  llvm::StringRef Temporary(Log);
-  llvm::SmallVector Strings;
-  Temporary.split(Strings, '\n');
 
-  clang::clangd::FuzzyFindRequest R;
-  R.MaxCandidateCount = 100;
+  std::vector Requests;
+  auto JSONArray = llvm::json::parse(Log);
 
-  llvm::SmallVector CommaSeparatedValues;
+  // Panic if the provided file couldn't be parsed.
+  if (!JSONArray || !JSONArray->getAsArray()) {
+llvm::errs() << "FATAL ERROR: Couldn't parse request.\n";
+exit(1);
+  }
 
-  std::vector RealRequ

[PATCH] D51971: [clangd] Use JSON format in benchmark requests reader

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Fix the diff after rebase.


https://reviews.llvm.org/D51971

Files:
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/test/clangd/Inputs/requests.json
  clang-tools-extra/test/clangd/Inputs/requests.log
  clang-tools-extra/test/clangd/index-tools.test

Index: clang-tools-extra/test/clangd/index-tools.test
===
--- clang-tools-extra/test/clangd/index-tools.test
+++ clang-tools-extra/test/clangd/index-tools.test
@@ -1,3 +1,3 @@
 # RUN: clangd-indexer %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs > %t.index
 # FIXME: By default, benchmarks are excluded from the list of default targets hence not built. Find a way to depend on benchmarks to run the next command.
-# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.log --benchmark_min_time=0.01 ; fi
+# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.json --benchmark_min_time=0.01 ; fi
Index: clang-tools-extra/test/clangd/Inputs/requests.log
===
--- clang-tools-extra/test/clangd/Inputs/requests.log
+++ /dev/null
@@ -1,5 +0,0 @@
-V[09:08:34.301] Code complete: fuzzyFind("s", scopes=[llvm::])
-V[09:08:34.368] Code complete: fuzzyFind("sy", scopes=[llvm::])
-V[09:08:34.442] Code complete: fuzzyFind("sys", scopes=[llvm::])
-V[09:09:12.075] Code complete: fuzzyFind("Dex", scopes=[clang::clangd::, clang::, clang::clangd::dex::])
-V[09:09:12.075] Code complete: fuzzyFind("Variable", scopes=[])
Index: clang-tools-extra/test/clangd/Inputs/requests.json
===
--- /dev/null
+++ clang-tools-extra/test/clangd/Inputs/requests.json
@@ -0,0 +1,7 @@
+[{"MaxCandidateCount":100,"ProximityPaths":["/usr/home/user/clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp"],"Query":"OMP","RestrictForCodeCompletion":true,"Scopes":["clang::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"s","RestrictForCodeCompletion":true,"Scopes":["llvm::", "::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sy","RestrictForCodeCompletion":true,"Scopes":["llvm::", "::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", "::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", "::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Dex","RestrictForCodeCompletion":true,"Scopes":["clang::clangd::", "clang::", "clang::clangd::dex::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Variable","RestrictForCodeCompletion":true,"Scopes":["::"]}]
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -19,56 +19,41 @@
 #include 
 
 const char *IndexFilename;
-const char *LogFilename;
+const char *RequestsFilename;
 
 namespace clang {
 namespace clangd {
 namespace {
 
-std::unique_ptr buildMem() {
-  return clang::clangd::loadIndex(IndexFilename, {}, false);
+std::unique_ptr buildMem() {
+  return loadIndex(IndexFilename, {}, false);
 }
 
-std::unique_ptr buildDex() {
-  return clang::clangd::loadIndex(IndexFilename, {}, true);
+std::unique_ptr buildDex() {
+  return loadIndex(IndexFilename, {}, true);
 }
 
-// This function processes user-provided Log file with fuzzy find requests in
-// the following format:
-//
-// fuzzyFind("UnqualifiedName", scopes=["clang::", "clang::clangd::"])
-//
-// It constructs vector of FuzzyFindRequests which is later used for the
-// benchmarks.
-std::vector extractQueriesFromLogs() {
-  llvm::Regex RequestMatcher("fuzzyFind\\(\"([a-zA-Z]*)\", scopes=\\[(.*)\\]");
-  llvm::SmallVector Matches;
-  std::ifstream InputStream(LogFilename);
+// Reads JSON array of serialized FuzzyFindRequest's from user-provided file.
+std::vector extractQueriesFromLogs() {
+  std::ifstream InputStream(RequestsFilename);
   std::string Log((std::istreambuf_iterator(InputStream)),
   std::istreambuf_iterator());
-  llvm::StringRef Temporary(Log);
-  llvm::SmallVector Strings;
-  Temporary.split(Strings, '\n');
 
-  clang::clangd::FuzzyFindRequest R;
-  R.MaxCandidateCount = 100;
+  std::vector Requests;
+  auto JSONArray = llvm::json::parse(Log);
 
-  llvm::SmallVector CommaSeparatedValues;
+  // Panic if the provided file couldn't be parsed.
+  if (!JSONArray || !JSONArray->getAsArray()) {
+llvm::errs() << "FATAL ERROR: Couldn't parse request.\n";
+exit(1);
+  }
 
-  std::vector RealRequests;
-  for (auto Line : Strings) {
-if (RequestMatcher.match(Line, &Matches)) {
-  R.Query = Matches[

[PATCH] D52016: [clangd] Don't create child AND and OR iterators with one posting list

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

Great improvement for such a simple change!


https://reviews.llvm.org/D52016



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

[PATCH] D52016: [clangd] Don't create child AND and OR iterators with one posting list

[Ilya Biryukov via Phabricator via cfe-commits]

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

LGTM with a nit.




Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:148
+  if (TrigramIterators.size() > 1)
 TopLevelChildren.push_back(createAnd(move(TrigramIterators)));
+  else if (TrigramIterators.size() == 1)

kbobyrev wrote:
> ilya-biryukov wrote:
> > Maybe special-case a single-iterator case in `createAnd` instead? Same with 
> > `createOr`.
> > Any cons to doing so?
> I thought that this might result in some implicit query tree generation 
> semantics. However, since there optimizations are likely to be added at some 
> point, I guess we could start being implicit right now, especially given that 
> there is no good way for the user code to inspect the query tree structure 
> (and there's no good reason to do so, too).
As long as observable behaviour does not change (and this change does not seem 
to change it), we should be good.
Creating optimized trees on-the-fly will make sure we always get optimal 
results, which is nice.



Comment at: clang-tools-extra/clangd/index/dex/Iterator.cpp:93
   : Children(std::move(AllChildren)) {
-assert(!Children.empty() && "AND iterator should have at least one 
child.");
 // Establish invariants.

Maybe keep the assertion here?
It's clear that `createAnd` is the only place that creates this class, so we 
can always trace the assertion back to its root-cause. Having the assertion in 
ctor guards against possible modifications of the code that would add more ways 
to construct `AndIterator`

Same for the assertion in `OrIterator`


https://reviews.llvm.org/D52016



___
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 set up Clangd

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/docs/clangd.rst:135
+
+:program:`VSCode` provides `vscode-clangd
+`

Maybe mention it is published in "Visual Studio Marketplace" and can be 
installed directly from VSCode?



https://reviews.llvm.org/D51297



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

[clang-tools-extra] r342124 - [clangd] Don't create child AND and OR iterators with one posting list

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Thu Sep 13 03:02:48 2018
New Revision: 342124

URL: http://llvm.org/viewvc/llvm-project?rev=342124&view=rev
Log:
[clangd] Don't create child AND and OR iterators with one posting list

`AND( AND( Child ) ... )` -> `AND( Child ... )`
`AND( OR( Child ) ... )` -> `AND( Child ... )`

This simple optimization results in 5-6% performance improvement in the
benchmark with 2000 serialized `FuzzyFindRequest`s.

Reviewed By: ilya-biryukov

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

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=342124&r1=342123&r2=342124&view=diff
==
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp Thu Sep 13 03:02:48 
2018
@@ -198,7 +198,7 @@ class OrIterator : public Iterator {
 public:
   explicit OrIterator(std::vector> AllChildren)
   : Children(std::move(AllChildren)) {
-assert(Children.size() > 0 && "OR iterator must have at least one child.");
+assert(!Children.empty() && "OR iterator should have at least one child.");
   }
 
   /// Returns true if all children are exhausted.
@@ -405,12 +405,16 @@ std::unique_ptr create(Posting
 
 std::unique_ptr
 createAnd(std::vector> Children) {
-  return llvm::make_unique(move(Children));
+  // If there is exactly one child, pull it one level up: AND(Child) -> Child.
+  return Children.size() == 1 ? std::move(Children.front())
+  : llvm::make_unique(move(Children));
 }
 
 std::unique_ptr
 createOr(std::vector> Children) {
-  return llvm::make_unique(move(Children));
+  // If there is exactly one child, pull it one level up: OR(Child) -> Child.
+  return Children.size() == 1 ? std::move(Children.front())
+  : llvm::make_unique(move(Children));
 }
 
 std::unique_ptr createTrue(DocID Size) {


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

[PATCH] D52016: [clangd] Don't create child AND and OR iterators with one posting list

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Move assertion back to ctors.


https://reviews.llvm.org/D52016

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


Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -198,7 +198,7 @@
 public:
   explicit OrIterator(std::vector> AllChildren)
   : Children(std::move(AllChildren)) {
-assert(Children.size() > 0 && "OR iterator must have at least one child.");
+assert(!Children.empty() && "OR iterator should have at least one child.");
   }
 
   /// Returns true if all children are exhausted.
@@ -405,12 +405,16 @@
 
 std::unique_ptr
 createAnd(std::vector> Children) {
-  return llvm::make_unique(move(Children));
+  // If there is exactly one child, pull it one level up: AND(Child) -> Child.
+  return Children.size() == 1 ? std::move(Children.front())
+  : llvm::make_unique(move(Children));
 }
 
 std::unique_ptr
 createOr(std::vector> Children) {
-  return llvm::make_unique(move(Children));
+  // If there is exactly one child, pull it one level up: OR(Child) -> Child.
+  return Children.size() == 1 ? std::move(Children.front())
+  : llvm::make_unique(move(Children));
 }
 
 std::unique_ptr createTrue(DocID Size) {


Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -198,7 +198,7 @@
 public:
   explicit OrIterator(std::vector> AllChildren)
   : Children(std::move(AllChildren)) {
-assert(Children.size() > 0 && "OR iterator must have at least one child.");
+assert(!Children.empty() && "OR iterator should have at least one child.");
   }
 
   /// Returns true if all children are exhausted.
@@ -405,12 +405,16 @@
 
 std::unique_ptr
 createAnd(std::vector> Children) {
-  return llvm::make_unique(move(Children));
+  // If there is exactly one child, pull it one level up: AND(Child) -> Child.
+  return Children.size() == 1 ? std::move(Children.front())
+  : llvm::make_unique(move(Children));
 }
 
 std::unique_ptr
 createOr(std::vector> Children) {
-  return llvm::make_unique(move(Children));
+  // If there is exactly one child, pull it one level up: OR(Child) -> Child.
+  return Children.size() == 1 ? std::move(Children.front())
+  : llvm::make_unique(move(Children));
 }
 
 std::unique_ptr createTrue(DocID Size) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52016: [clangd] Don't create child AND and OR iterators with one posting list

[Kirill Bobyrev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342124: [clangd] Don't create child AND and OR 
iterators with one posting list (authored by omtcyfz, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D52016?vs=165236&id=165237#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D52016

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


Index: clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
@@ -198,7 +198,7 @@
 public:
   explicit OrIterator(std::vector> AllChildren)
   : Children(std::move(AllChildren)) {
-assert(Children.size() > 0 && "OR iterator must have at least one child.");
+assert(!Children.empty() && "OR iterator should have at least one child.");
   }
 
   /// Returns true if all children are exhausted.
@@ -405,12 +405,16 @@
 
 std::unique_ptr
 createAnd(std::vector> Children) {
-  return llvm::make_unique(move(Children));
+  // If there is exactly one child, pull it one level up: AND(Child) -> Child.
+  return Children.size() == 1 ? std::move(Children.front())
+  : llvm::make_unique(move(Children));
 }
 
 std::unique_ptr
 createOr(std::vector> Children) {
-  return llvm::make_unique(move(Children));
+  // If there is exactly one child, pull it one level up: OR(Child) -> Child.
+  return Children.size() == 1 ? std::move(Children.front())
+  : llvm::make_unique(move(Children));
 }
 
 std::unique_ptr createTrue(DocID Size) {


Index: clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/trunk/clangd/index/dex/Iterator.cpp
@@ -198,7 +198,7 @@
 public:
   explicit OrIterator(std::vector> AllChildren)
   : Children(std::move(AllChildren)) {
-assert(Children.size() > 0 && "OR iterator must have at least one child.");
+assert(!Children.empty() && "OR iterator should have at least one child.");
   }
 
   /// Returns true if all children are exhausted.
@@ -405,12 +405,16 @@
 
 std::unique_ptr
 createAnd(std::vector> Children) {
-  return llvm::make_unique(move(Children));
+  // If there is exactly one child, pull it one level up: AND(Child) -> Child.
+  return Children.size() == 1 ? std::move(Children.front())
+  : llvm::make_unique(move(Children));
 }
 
 std::unique_ptr
 createOr(std::vector> Children) {
-  return llvm::make_unique(move(Children));
+  // If there is exactly one child, pull it one level up: OR(Child) -> Child.
+  return Children.size() == 1 ? std::move(Children.front())
+  : llvm::make_unique(move(Children));
 }
 
 std::unique_ptr createTrue(DocID Size) {
___
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 set up Clangd

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 165238.
kbobyrev marked an inline comment as done.

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,41 @@
 | Gen. Getters/Setters| No |   No |
 +-++--+
 
+Editor Integration
+==
+
+Any full-featured Language Server Protocol Client implementation should work
+with :program:`Clangd`. This `list
+` contains information about
+extensions and plugins that are known to work for different editors.
+
+Vim Integration
+---
+
+LanguageClient-neovim
+~
+
+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
+instructions.
+
+VSCode Integration
+--
+
+:program:`VSCode` provides `vscode-clangd
+`
+which is published in Visual Studio Marketplace and can be installed direcetly
+from :program:`VSCode`.
+
+Emacs Integration
+-
+
+:program:`Emacs` provides `lsp-mode ` and
+`Eglot ` plugins for LSP integration.
+
 Getting Involved
 ==
 


Index: clang-tools-extra/docs/clangd.rst
===
--- clang-tools-extra/docs/clangd.rst
+++ clang-tools-extra/docs/clangd.rst
@@ -108,6 +108,41 @@
 | Gen. Getters/Setters| No |   No |
 +-++--+
 
+Editor Integration
+==
+
+Any full-featured Language Server Protocol Client implementation should work
+with :program:`Clangd`. This `list
+` contains information about
+extensions and plugins that are known to work for different editors.
+
+Vim Integration
+---
+
+LanguageClient-neovim
+~
+
+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
+instructions.
+
+VSCode Integration
+--
+
+:program:`VSCode` provides `vscode-clangd
+`
+which is published in Visual Studio Marketplace and can be installed direcetly
+from :program:`VSCode`.
+
+Emacs Integration
+-
+
+:program:`Emacs` provides `lsp-mode ` and
+`Eglot ` plugins for LSP integration.
+
 Getting Involved
 ==
 
___
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 set up Clangd

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/docs/clangd.rst:135
+
+:program:`VSCode` provides `vscode-clangd
+`

ilya-biryukov wrote:
> Maybe mention it is published in "Visual Studio Marketplace" and can be 
> installed directly from VSCode?
> 
Right, I thought it might make sense to mention that it is developed in-tree 
here, but since it's the "Editor Integration" section it's probably not worth 
here and I also didn't notice this mention on top of the documentation page.


https://reviews.llvm.org/D51297



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

[PATCH] D51971: [clangd] Use JSON format in benchmark requests reader

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Fix empty scope in `requests.json`: `%s/"::"/""/g`.


https://reviews.llvm.org/D51971

Files:
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/test/clangd/Inputs/requests.json
  clang-tools-extra/test/clangd/Inputs/requests.log
  clang-tools-extra/test/clangd/index-tools.test

Index: clang-tools-extra/test/clangd/index-tools.test
===
--- clang-tools-extra/test/clangd/index-tools.test
+++ clang-tools-extra/test/clangd/index-tools.test
@@ -1,3 +1,3 @@
 # RUN: clangd-indexer %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs > %t.index
 # FIXME: By default, benchmarks are excluded from the list of default targets hence not built. Find a way to depend on benchmarks to run the next command.
-# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.log --benchmark_min_time=0.01 ; fi
+# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.json --benchmark_min_time=0.01 ; fi
Index: clang-tools-extra/test/clangd/Inputs/requests.log
===
--- clang-tools-extra/test/clangd/Inputs/requests.log
+++ /dev/null
@@ -1,5 +0,0 @@
-V[09:08:34.301] Code complete: fuzzyFind("s", scopes=[llvm::])
-V[09:08:34.368] Code complete: fuzzyFind("sy", scopes=[llvm::])
-V[09:08:34.442] Code complete: fuzzyFind("sys", scopes=[llvm::])
-V[09:09:12.075] Code complete: fuzzyFind("Dex", scopes=[clang::clangd::, clang::, clang::clangd::dex::])
-V[09:09:12.075] Code complete: fuzzyFind("Variable", scopes=[])
Index: clang-tools-extra/test/clangd/Inputs/requests.json
===
--- /dev/null
+++ clang-tools-extra/test/clangd/Inputs/requests.json
@@ -0,0 +1,7 @@
+[{"MaxCandidateCount":100,"ProximityPaths":["/usr/home/user/clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp"],"Query":"OMP","RestrictForCodeCompletion":true,"Scopes":["clang::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"s","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sy","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Dex","RestrictForCodeCompletion":true,"Scopes":["clang::clangd::", "clang::", "clang::clangd::dex::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Variable","RestrictForCodeCompletion":true,"Scopes":[""]}]
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -19,56 +19,41 @@
 #include 
 
 const char *IndexFilename;
-const char *LogFilename;
+const char *RequestsFilename;
 
 namespace clang {
 namespace clangd {
 namespace {
 
-std::unique_ptr buildMem() {
-  return clang::clangd::loadIndex(IndexFilename, {}, false);
+std::unique_ptr buildMem() {
+  return loadIndex(IndexFilename, {}, false);
 }
 
-std::unique_ptr buildDex() {
-  return clang::clangd::loadIndex(IndexFilename, {}, true);
+std::unique_ptr buildDex() {
+  return loadIndex(IndexFilename, {}, true);
 }
 
-// This function processes user-provided Log file with fuzzy find requests in
-// the following format:
-//
-// fuzzyFind("UnqualifiedName", scopes=["clang::", "clang::clangd::"])
-//
-// It constructs vector of FuzzyFindRequests which is later used for the
-// benchmarks.
-std::vector extractQueriesFromLogs() {
-  llvm::Regex RequestMatcher("fuzzyFind\\(\"([a-zA-Z]*)\", scopes=\\[(.*)\\]");
-  llvm::SmallVector Matches;
-  std::ifstream InputStream(LogFilename);
+// Reads JSON array of serialized FuzzyFindRequest's from user-provided file.
+std::vector extractQueriesFromLogs() {
+  std::ifstream InputStream(RequestsFilename);
   std::string Log((std::istreambuf_iterator(InputStream)),
   std::istreambuf_iterator());
-  llvm::StringRef Temporary(Log);
-  llvm::SmallVector Strings;
-  Temporary.split(Strings, '\n');
 
-  clang::clangd::FuzzyFindRequest R;
-  R.MaxCandidateCount = 100;
+  std::vector Requests;
+  auto JSONArray = llvm::json::parse(Log);
 
-  llvm::SmallVector CommaSeparatedValues;
+  // Panic if the provided file couldn't be parsed.
+  if (!JSONArray || !JSONArray->getAsArray()) {
+llvm::errs() << "FATAL ERROR: Couldn't parse request.\n";
+exit(1);
+  }
 
-  std::vector RealRequests;
-  for (auto Line : Strings) {
-if (RequestMatcher.match(Line, &Matches)) {
-  R.Q

[PATCH] D51971: [clangd] Use JSON format in benchmark requests reader

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:47
+  if (!JSONArray || !JSONArray->getAsArray()) {
+llvm::errs() << "FATAL ERROR: Couldn't parse request.\n";
+exit(1);

If there was a json parsing error, we should consume it and print to the user, 
i.e.
```
llvm::errs() << "Error when parsing json requests file: " << 
llvm::toString(std::move(JSONArray));
```

The user-facing error when an object is not an array should probably be a 
separate one.




Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:53
+FuzzyFindRequest Request;
+fromJSON(Item, Request);
+Requests.push_back(Request);

Check for return value of `fromJSON` and also report an error if it failed?


https://reviews.llvm.org/D51971



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

r342125 - Fix MSVC "illegal conversion; more than one user-defined conversion has been implicitly applied" warning. NFCI.

[Simon Pilgrim via cfe-commits]

Author: rksimon
Date: Thu Sep 13 03:10:18 2018
New Revision: 342125

URL: http://llvm.org/viewvc/llvm-project?rev=342125&view=rev
Log:
Fix MSVC "illegal conversion; more than one user-defined conversion has been 
implicitly applied" warning. NFCI.

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

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=342125&r1=342124&r2=342125&view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Thu Sep 13 03:10:18 2018
@@ -4163,7 +4163,7 @@ std::string Driver::GetFilePath(StringRe
   SmallString<128> P(Dir[0] == '=' ? SysRoot + Dir.substr(1) : Dir);
   llvm::sys::path::append(P, Name);
   if (llvm::sys::fs::exists(Twine(P)))
-return {P.str()};
+return P.str().str();
 }
 return None;
   };


___
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 set up Clangd

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov accepted this revision.
ilya-biryukov added a comment.

LGTM


https://reviews.llvm.org/D51297



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

[PATCH] D52020: [OpenCL] Improve extension-version.cl and to_addr_builtin.cl tests

[Alistair Davies via Phabricator via cfe-commits]

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

Adds cl_khr_depth_images to extension-version.cl, and extends 
to_addr_builtin.cl.


https://reviews.llvm.org/D52020

Files:
  test/SemaOpenCL/extension-version.cl
  test/SemaOpenCL/to_addr_builtin.cl

Index: test/SemaOpenCL/to_addr_builtin.cl
===
--- test/SemaOpenCL/to_addr_builtin.cl
+++ test/SemaOpenCL/to_addr_builtin.cl
@@ -5,6 +5,8 @@
   global int *glob;
   local int *loc;
   constant int *con;
+  private int *priv;
+  global float *glob_wrong_ty;
   typedef constant int const_int_ty;
   const_int_ty *con_typedef;
 
@@ -45,11 +47,70 @@
   // expected-error@-4{{assigning '__global int *' to '__local int *' changes address space of pointer}}
 #endif
 
+  loc = to_private(glob);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+  // expected-warning@-2{{implicit declaration of function 'to_private' is invalid in OpenCL}}
+  // expected-warning@-3{{incompatible integer to pointer conversion assigning to '__local int *' from 'int'}}
+#else
+  // expected-error@-5{{assigning 'int *' to '__local int *' changes address space of pointer}}
+#endif
+
+  loc = to_local(glob);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+  // expected-warning@-2{{implicit declaration of function 'to_local' is invalid in OpenCL}}
+  // expected-warning@-3{{incompatible integer to pointer conversion assigning to '__local int *' from 'int'}}
+#endif
+
+  priv = to_global(glob);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+  // expected-warning@-2{{incompatible integer to pointer conversion assigning to 'int *' from 'int'}}
+#else
+  // expected-error@-4{{assigning '__global int *' to 'int *' changes address space of pointer}}
+#endif
+
+  priv = to_private(glob);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+  // expected-warning@-2{{incompatible integer to pointer conversion assigning to 'int *' from 'int'}}
+#endif
+
+  priv = to_local(glob);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+  // expected-warning@-2{{incompatible integer to pointer conversion assigning to 'int *' from 'int'}}
+#else
+  // expected-error@-4{{assigning '__local int *' to 'int *' changes address space of pointer}}
+#endif
+
+  glob = to_global(glob);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+  // expected-warning@-2{{incompatible integer to pointer conversion assigning to '__global int *' from 'int'}}
+#endif
+
+  glob = to_private(glob);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+  // expected-warning@-2{{incompatible integer to pointer conversion assigning to '__global int *' from 'int'}}
+#else
+  // expected-error@-4{{assigning 'int *' to '__global int *' changes address space of pointer}}
+#endif
+
+  glob = to_local(glob);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+  // expected-warning@-2{{incompatible integer to pointer conversion assigning to '__global int *' from 'int'}}
+#else
+  // expected-error@-4{{assigning '__local int *' to '__global int *' changes address space of pointer}}
+#endif
+
   global char *glob_c = to_global(loc);
 #if __OPENCL_C_VERSION__ < CL_VERSION_2_0
   // expected-warning@-2{{incompatible integer to pointer conversion initializing '__global char *' with an expression of type 'int'}}
 #else
   // expected-warning@-4{{incompatible pointer types initializing '__global char *' with an expression of type '__global int *'}}
 #endif
 
+  glob_wrong_ty = to_global(glob);
+#if __OPENCL_C_VERSION__ < CL_VERSION_2_0
+  // expected-warning@-2{{incompatible integer to pointer conversion assigning to '__global float *' from 'int'}}
+#else
+  // expected-warning@-4{{incompatible pointer types assigning to '__global float *' from '__global int *'}}
+#endif
+
 }
Index: test/SemaOpenCL/extension-version.cl
===
--- test/SemaOpenCL/extension-version.cl
+++ test/SemaOpenCL/extension-version.cl
@@ -282,6 +282,18 @@
 #endif
 #pragma OPENCL EXTENSION cl_amd_media_ops2: enable
 
+#if (__OPENCL_C_VERSION__ >= 120)
+#ifndef cl_khr_depth_images
+#error "Missing cl_khr_depth_images define"
+#endif
+#else
+#ifdef cl_khr_depth_images
+#error "Incorrect cl_khr_depth_images define"
+#endif
+// expected-warning@+2{{unsupported OpenCL extension 'cl_khr_depth_images' - ignoring}}
+#endif
+#pragma OPENCL EXTENSION cl_khr_depth_images: enable
+
 #if (__OPENCL_C_VERSION__ >= 120)
 #ifndef cl_intel_subgroups
 #error "Missing cl_intel_subgroups define"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r342126 - [AArch64] Enable return address signing for static ctors

[Oliver Stannard via cfe-commits]

Author: olista01
Date: Thu Sep 13 03:25:36 2018
New Revision: 342126

URL: http://llvm.org/viewvc/llvm-project?rev=342126&view=rev
Log:
[AArch64] Enable return address signing for static ctors

Functions generated by clang and included in the .init_array section (such as
static constructors) do not follow the usual code path for adding
target-specific function attributes, so we have to add the return address
signing attribute here too, as is currently done for the sanitisers.

Differential revision: https://reviews.llvm.org/D51418


Added:
cfe/trunk/test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp
Modified:
cfe/trunk/lib/CodeGen/CGDeclCXX.cpp

Modified: cfe/trunk/lib/CodeGen/CGDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDeclCXX.cpp?rev=342126&r1=342125&r2=342126&view=diff
==
--- cfe/trunk/lib/CodeGen/CGDeclCXX.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGDeclCXX.cpp Thu Sep 13 03:25:36 2018
@@ -359,6 +359,12 @@ llvm::Function *CodeGenModule::CreateGlo
   !isInSanitizerBlacklist(SanitizerKind::ShadowCallStack, Fn, Loc))
 Fn->addFnAttr(llvm::Attribute::ShadowCallStack);
 
+  auto RASignKind = getCodeGenOpts().getSignReturnAddress();
+  if (RASignKind != CodeGenOptions::SignReturnAddressScope::None)
+Fn->addFnAttr("sign-return-address",
+  RASignKind == CodeGenOptions::SignReturnAddressScope::All
+  ? "all"
+  : "non-leaf");
   return Fn;
 }
 

Added: cfe/trunk/test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp?rev=342126&view=auto
==
--- cfe/trunk/test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp 
(added)
+++ cfe/trunk/test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp Thu 
Sep 13 03:25:36 2018
@@ -0,0 +1,21 @@
+// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - 
-msign-return-address=none  %s | \
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NONE
+// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - 
-msign-return-address=non-leaf %s | \
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-PARTIAL
+// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - 
-msign-return-address=all %s | \
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ALL
+
+struct Foo {
+  Foo() {}
+  ~Foo() {}
+};
+
+Foo f;
+
+// CHECK: @llvm.global_ctors {{.*}}i32 65535, void ()* @[[CTOR_FN:.*]], i8* 
null
+
+// CHECK: @[[CTOR_FN]]() #[[ATTR:[0-9]*]]
+
+// CHECK-NONE-NOT: attributes #[[ATTR]] = { {{.*}} 
"sign-return-address"={{.*}} }}
+// CHECK-PARTIAL: attributes #[[ATTR]] = { {{.*}} 
"sign-return-address"="non-leaf" {{.*}}}
+// CHECK-ALL: attributes #[[ATTR]] = { {{.*}} "sign-return-address"="all" 
{{.*}} }


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

[PATCH] D52008: [analyzer] Handle forwarding reference better in ExprMutationAnalyzer.

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

The new functionality looks very good. It can be used in a readability check 
that suggests `const` for parameters.




Comment at: include/clang/Analysis/Analyses/ExprMutationAnalyzer.h:32
   const Stmt *findMutation(const Expr *Exp);
+  const Stmt *findDeclMutation(const Decl *Dec);
 

lebedev.ri wrote:
> Thanks!
> I know this has performance implications, but those will exist even if one 
> has this in his own code.
The analysis itself should not be executed twice, as it is cached. Only the way 
to figuring out that its cached will be run. I think its acceptable.



Comment at: include/clang/Analysis/Analyses/ExprMutationAnalyzer.h:60
+public:
+  FunctionParmMutationAnalyzer(const FunctionDecl &Func, ASTContext &Context);
+

Why do we need a separate class for this?
I think you can just create another object of `ExprMutAnalyzer` and do the 
analysis with `findDeclMutation(FunctioParm)`.

The `Stmt` is the `functionDecl().getBody()`. Right now it could be that you 
pass in an functionDecl without body.

Could there something happen with extern templates that the body is not 
accessible?



Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:201
   equalsNode(Exp), parmVarDecl(hasType(nonConstReferenceType(;
+  const auto NotInstantiated = unless(hasDeclaration(isInstantiated()));
   const auto AsNonConstRefArg = anyOf(

I think this will not all cases correctly.

Say
```
template 
struct Foo {
  static void bar() { SomethingWith(T()); }
};
```

`bar` it self is not a template and `NotInstantiated` will be `false` (only 90% 
sure on that) as the declaration of `bar` does not match.
In another check I had to do this: `unless(has(expr(anyOf(isTypeDependent(), 
isValueDependent()` to ensure that there are no unresolved constructs in 
the stmt.



Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:318
+  parmVarDecl(hasType(nonConstReferenceType())).bind("parm"));
+  const auto IsInstantiated = hasDeclaration(isInstantiated());
+  const auto FuncDecl = hasDeclaration(functionDecl().bind("func"));

Same instantiation concerns



Comment at: lib/Analysis/ExprMutationAnalyzer.cpp:367
+// function body, check them eagerly here since they're typically trivial.
+for (const CXXCtorInitializer *Init : Ctor->inits()) {
+  ExprMutationAnalyzer InitAnalyzer(*Init->getInit(), Context);

Just to be sure, this will recurse ?

```
struct Foo {
  std::string s;
  Foo(std::string s) : s (std::move(s)) {}
};
```

`std::move` will be resolved through the new mechanism right?



Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:625
+  Results = match(withEnclosingCompound(declRefTo("x")), AST->getASTContext());
+  EXPECT_THAT(mutatedBy(Results, AST.get()), ElementsAre("g(y, x)"));
+}

Please add a `Results(declRefTo("y"), notMutated(y)`, same above



Comment at: unittests/Analysis/ExprMutationAnalyzerTest.cpp:659
+}
+
 TEST(ExprMutationAnalyzerTest, ArrayElementModified) {

There are tests missing for the constructors. Please ensure that `std::move` 
and `std::forward` are handled properly.


Repository:
  rC Clang

https://reviews.llvm.org/D52008



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

[PATCH] D51418: [AArch64] Enable return address signing for static ctors

[Oliver Stannard via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC342126: [AArch64] Enable return address signing for static 
ctors (authored by olista01, committed by ).
Herald added a subscriber: cfe-commits.

Repository:
  rC Clang

https://reviews.llvm.org/D51418

Files:
  lib/CodeGen/CGDeclCXX.cpp
  test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp


Index: test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp
===
--- test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp
+++ test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - 
-msign-return-address=none  %s | \
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NONE
+// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - 
-msign-return-address=non-leaf %s | \
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-PARTIAL
+// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - 
-msign-return-address=all %s | \
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ALL
+
+struct Foo {
+  Foo() {}
+  ~Foo() {}
+};
+
+Foo f;
+
+// CHECK: @llvm.global_ctors {{.*}}i32 65535, void ()* @[[CTOR_FN:.*]], i8* 
null
+
+// CHECK: @[[CTOR_FN]]() #[[ATTR:[0-9]*]]
+
+// CHECK-NONE-NOT: attributes #[[ATTR]] = { {{.*}} 
"sign-return-address"={{.*}} }}
+// CHECK-PARTIAL: attributes #[[ATTR]] = { {{.*}} 
"sign-return-address"="non-leaf" {{.*}}}
+// CHECK-ALL: attributes #[[ATTR]] = { {{.*}} "sign-return-address"="all" 
{{.*}} }
Index: lib/CodeGen/CGDeclCXX.cpp
===
--- lib/CodeGen/CGDeclCXX.cpp
+++ lib/CodeGen/CGDeclCXX.cpp
@@ -359,6 +359,12 @@
   !isInSanitizerBlacklist(SanitizerKind::ShadowCallStack, Fn, Loc))
 Fn->addFnAttr(llvm::Attribute::ShadowCallStack);
 
+  auto RASignKind = getCodeGenOpts().getSignReturnAddress();
+  if (RASignKind != CodeGenOptions::SignReturnAddressScope::None)
+Fn->addFnAttr("sign-return-address",
+  RASignKind == CodeGenOptions::SignReturnAddressScope::All
+  ? "all"
+  : "non-leaf");
   return Fn;
 }
 


Index: test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp
===
--- test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp
+++ test/CodeGenCXX/aarch64-sign-return-address-static-ctor.cpp
@@ -0,0 +1,21 @@
+// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=none  %s | \
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-NONE
+// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=non-leaf %s | \
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-PARTIAL
+// RUN: %clang -target aarch64-arm-none-eabi -S -emit-llvm -o - -msign-return-address=all %s | \
+// RUN: FileCheck %s --check-prefix=CHECK --check-prefix=CHECK-ALL
+
+struct Foo {
+  Foo() {}
+  ~Foo() {}
+};
+
+Foo f;
+
+// CHECK: @llvm.global_ctors {{.*}}i32 65535, void ()* @[[CTOR_FN:.*]], i8* null
+
+// CHECK: @[[CTOR_FN]]() #[[ATTR:[0-9]*]]
+
+// CHECK-NONE-NOT: attributes #[[ATTR]] = { {{.*}} "sign-return-address"={{.*}} }}
+// CHECK-PARTIAL: attributes #[[ATTR]] = { {{.*}} "sign-return-address"="non-leaf" {{.*}}}
+// CHECK-ALL: attributes #[[ATTR]] = { {{.*}} "sign-return-address"="all" {{.*}} }
Index: lib/CodeGen/CGDeclCXX.cpp
===
--- lib/CodeGen/CGDeclCXX.cpp
+++ lib/CodeGen/CGDeclCXX.cpp
@@ -359,6 +359,12 @@
   !isInSanitizerBlacklist(SanitizerKind::ShadowCallStack, Fn, Loc))
 Fn->addFnAttr(llvm::Attribute::ShadowCallStack);
 
+  auto RASignKind = getCodeGenOpts().getSignReturnAddress();
+  if (RASignKind != CodeGenOptions::SignReturnAddressScope::None)
+Fn->addFnAttr("sign-return-address",
+  RASignKind == CodeGenOptions::SignReturnAddressScope::All
+  ? "all"
+  : "non-leaf");
   return Fn;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52021: Fix Bug 38926: don't merge short case labels if followed by a block

[Owen Pan via Phabricator via cfe-commits]

owenpan created this revision.
owenpan added reviewers: djasper, klimek, sammccall, krasimir.
Herald added a subscriber: cfe-commits.

https://bugs.llvm.org/show_bug.cgi?id=38926


Repository:
  rC Clang

https://reviews.llvm.org/D52021

Files:
  lib/Format/UnwrappedLineFormatter.cpp
  unittests/Format/FormatTest.cpp


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1241,6 +1241,29 @@
"  return false;\n"
"}",
Style));
+  Style.AllowShortCaseLabelsOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
+  EXPECT_EQ("switch (n)\n"
+"{\n"
+"  case 0:\n"
+"  {\n"
+"return false;\n"
+"  }\n"
+"  default:\n"
+"  {\n"
+"return true;\n"
+"  }\n"
+"}",
+format("switch (n) {\n"
+   "  case 0: {\n"
+   "return false;\n"
+   "  }\n"
+   "  default: {\n"
+   "return true;\n"
+   "  }\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, FormatsLabels) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -428,6 +428,14 @@
 if (Limit == 0 || I + 1 == E ||
 I[1]->First->isOneOf(tok::kw_case, tok::kw_default))
   return 0;
+
+// Don't merge if a block follows the case label
+FormatToken *Tok = I[0]->Last;
+if (Tok->is(tok::comment))
+  Tok = Tok->getPreviousNonComment();
+if ((Tok && Tok->is(tok::l_brace)) || I[1]->First->is(tok::l_brace))
+  return 0;
+
 unsigned NumStmts = 0;
 unsigned Length = 0;
 bool EndsWithComment = false;


Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1241,6 +1241,29 @@
"  return false;\n"
"}",
Style));
+  Style.AllowShortCaseLabelsOnASingleLine = true;
+  Style.BreakBeforeBraces = FormatStyle::BS_Custom;
+  Style.BraceWrapping.AfterControlStatement = true;
+  EXPECT_EQ("switch (n)\n"
+"{\n"
+"  case 0:\n"
+"  {\n"
+"return false;\n"
+"  }\n"
+"  default:\n"
+"  {\n"
+"return true;\n"
+"  }\n"
+"}",
+format("switch (n) {\n"
+   "  case 0: {\n"
+   "return false;\n"
+   "  }\n"
+   "  default: {\n"
+   "return true;\n"
+   "  }\n"
+   "}",
+   Style));
 }
 
 TEST_F(FormatTest, FormatsLabels) {
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ lib/Format/UnwrappedLineFormatter.cpp
@@ -428,6 +428,14 @@
 if (Limit == 0 || I + 1 == E ||
 I[1]->First->isOneOf(tok::kw_case, tok::kw_default))
   return 0;
+
+// Don't merge if a block follows the case label
+FormatToken *Tok = I[0]->Last;
+if (Tok->is(tok::comment))
+  Tok = Tok->getPreviousNonComment();
+if ((Tok && Tok->is(tok::l_brace)) || I[1]->First->is(tok::l_brace))
+  return 0;
+
 unsigned NumStmts = 0;
 unsigned Length = 0;
 bool EndsWithComment = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51333: Diagnose likely typos in include statements

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

(No action needed, but curious on your thoughts)

One thing I'd like to do sometime is add code completion of filenames in 
#include directives.
This would mean listing the relevant directories from the include path. (VFS is 
slow for this today, but I have a patch out for it).

Maybe it would make sense to do that here and use edit-distance to pick the 
suggestion, like typo correction elsewhere?
Could be a way to extend this patch's behavior in the future.


https://reviews.llvm.org/D51333



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

Re: [PATCH] D51747: [clangd] Implement deprecation diagnostics with lower severity.

[Sam McCall via cfe-commits]

On Tue, Sep 11, 2018 at 3:43 PM Ilya Biryukov via Phabricator <
revi...@reviews.llvm.org> wrote:

> ilya-biryukov added a comment.
>
> In https://reviews.llvm.org/D51747#1229066, @sammccall wrote:
>
> > A few thoughts here:
> >
> > - does CDB describe user or project preferences? unclear.
>
>
> Agree, it's a mix, defaults are from the project but users can add extra
> flags.
>
> > - "show this warning for code I build" is a higher bar than "show this
> warning for code I edit". So CDB probably enables too few warnings.
> > - Some warnings play well with -Werror (like uninit warnings), some
> don't (like deprecated). -Werror projects often disable interesting
> warnings.
>
> Agreed, editors are different from build.
>
> > I'm not sure we should strictly follow the CDB, but the bar to override
> it should probably be high.
>
> WDYT in the long term about a more general mechanism (to allow users
> override compiler or warning flags at the clangd level?
> So that even if clangd is opinionated about the default warnings it
> enables, users have an option to override according to their preferences.
>
Yeah, I can see making "extra clang flags" a clangd flag (at some point we
really need .clangd config file or something...)

The scary thing about the extra flags is how they interact with driver mode
(clang-cl vs clang), but maybe that's the user's problem.


> In https://reviews.llvm.org/D51747#1230420, @kadircet wrote:
>
> > if user wants to see all diagnostics as a list suddenly they will get
> deprecations in that list as well :(.
>
>
> Yeah, some level of noise is probably inevitable.
>
>
> Repository:
>   rCTE Clang Tools Extra
>
> https://reviews.llvm.org/D51747
>
>
>
>
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51921: [VFS] vfs::directory_iterator yields path and file type instead of full Status

[Sam McCall via Phabricator via cfe-commits]

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

Path/getName() -> path(), Type -> type() for consistency with 
fs::directory_entry


Repository:
  rC Clang

https://reviews.llvm.org/D51921

Files:
  include/clang/Basic/VirtualFileSystem.h
  lib/Basic/VirtualFileSystem.cpp
  lib/Driver/ToolChains/BareMetal.cpp
  lib/Driver/ToolChains/Gnu.cpp
  lib/Frontend/CompilerInstance.cpp
  lib/Frontend/FrontendAction.cpp
  lib/Lex/HeaderSearch.cpp
  lib/Lex/ModuleMap.cpp
  lib/Lex/PPLexerChange.cpp
  unittests/Basic/VirtualFileSystemTest.cpp

Index: unittests/Basic/VirtualFileSystemTest.cpp
===
--- unittests/Basic/VirtualFileSystemTest.cpp
+++ unittests/Basic/VirtualFileSystemTest.cpp
@@ -104,21 +104,23 @@
   Path(_Path.str()) {
   for ( ; I != FilesAndDirs.end(); ++I) {
 if (isInPath(I->first)) {
-  CurrentEntry = I->second;
+  CurrentEntry =
+  vfs::directory_entry(I->second.getName(), I->second.getType());
   break;
 }
   }
 }
 std::error_code increment() override {
   ++I;
   for ( ; I != FilesAndDirs.end(); ++I) {
 if (isInPath(I->first)) {
-  CurrentEntry = I->second;
+  CurrentEntry =
+  vfs::directory_entry(I->second.getName(), I->second.getType());
   break;
 }
   }
   if (I == FilesAndDirs.end())
-CurrentEntry = vfs::Status();
+CurrentEntry = vfs::directory_entry();
   return std::error_code();
 }
   };
@@ -398,11 +400,11 @@
   ASSERT_FALSE(EC);
   ASSERT_NE(vfs::directory_iterator(), I);
   // Check either a or c, since we can't rely on the iteration order.
-  EXPECT_TRUE(I->getName().endswith("a") || I->getName().endswith("c"));
+  EXPECT_TRUE(I->path().endswith("a") || I->path().endswith("c"));
   I.increment(EC);
   ASSERT_FALSE(EC);
   ASSERT_NE(vfs::directory_iterator(), I);
-  EXPECT_TRUE(I->getName().endswith("a") || I->getName().endswith("c"));
+  EXPECT_TRUE(I->path().endswith("a") || I->path().endswith("c"));
   I.increment(EC);
   EXPECT_EQ(vfs::directory_iterator(), I);
 }
@@ -438,7 +440,7 @@
  << "EC message: " << EC2.message() << "\n";
 }
 ASSERT_FALSE(EC);
-EXPECT_TRUE(I->getName() == _b);
+EXPECT_TRUE(I->path() == _b);
   }
 }
 #endif
@@ -464,7 +466,7 @@
   std::vector Contents;
   for (auto E = vfs::recursive_directory_iterator(); !EC && I != E;
I.increment(EC)) {
-Contents.push_back(I->getName());
+Contents.push_back(I->path());
   }
 
   // Check contents, which may be in any order
@@ -507,7 +509,7 @@
I != E; I.increment(EC)) {
 auto EC2 = std::make_error_code(std::errc::no_such_file_or_directory);
 if (EC == EC2) {
-  VisitedBrokenSymlinks.push_back(I->getName());
+  VisitedBrokenSymlinks.push_back(I->path());
   continue;
 }
 // For bot debugging.
@@ -523,7 +525,7 @@
  << "EC message: " << EC2.message() << "\n";
 }
 ASSERT_FALSE(EC);
-VisitedNonBrokenSymlinks.push_back(I->getName());
+VisitedNonBrokenSymlinks.push_back(I->path());
   }
 
   // Check visited file names.
@@ -549,7 +551,7 @@
   // Do not rely on iteration order to check for contents, sort both
   // content vectors before comparison.
   for (DirIter E; !EC && I != E; I.increment(EC))
-InputToCheck.push_back(I->getName());
+InputToCheck.push_back(I->path());
 
   llvm::sort(InputToCheck.begin(), InputToCheck.end());
   llvm::sort(Expected.begin(), Expected.end());
@@ -656,14 +658,14 @@
   O->pushOverlay(Upper);
 
   std::error_code EC;
-  Lower->addRegularFile("/onlyInLow", sys::fs::owner_read);
-  Lower->addRegularFile("/hiddenByMid", sys::fs::owner_read);
-  Lower->addRegularFile("/hiddenByUp", sys::fs::owner_read);
-  Middle->addRegularFile("/onlyInMid", sys::fs::owner_write);
-  Middle->addRegularFile("/hiddenByMid", sys::fs::owner_write);
-  Middle->addRegularFile("/hiddenByUp", sys::fs::owner_write);
-  Upper->addRegularFile("/onlyInUp", sys::fs::owner_all);
-  Upper->addRegularFile("/hiddenByUp", sys::fs::owner_all);
+  Lower->addRegularFile("/onlyInLow");
+  Lower->addDirectory("/hiddenByMid");
+  Lower->addDirectory("/hiddenByUp");
+  Middle->addRegularFile("/onlyInMid");
+  Middle->addRegularFile("/hiddenByMid");
+  Middle->addDirectory("/hiddenByUp");
+  Upper->addRegularFile("/onlyInUp");
+  Upper->addRegularFile("/hiddenByUp");
   checkContents(
   O->dir_begin("/", EC),
   {"/hiddenByUp", "/onlyInUp", "/hiddenByMid", "/onlyInMid", "/onlyInLow"});
@@ -673,19 +675,19 @@
 std::error_code EC;
 vfs::directory_iterator I = O->dir_begin("/", EC), E;
 for ( ; !EC && I != E; I.increment(EC))
-  if (I->getName() == "/hiddenByUp")
+  if (I->path() == "/hiddenByUp")
 break;
 ASSERT_NE(E, I);
-EXPECT_EQ(sys::fs::owner_all, I->getPermissions());
+EXPECT_EQ(sys::fs::file_type::regular_file, I->type());
   }
 

[PATCH] D51971: [clangd] Use JSON format in benchmark requests reader

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 165248.
kbobyrev marked 3 inline comments as done.

https://reviews.llvm.org/D51971

Files:
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/clangd/index/dex/Iterator.cpp
  clang-tools-extra/test/clangd/Inputs/requests.json
  clang-tools-extra/test/clangd/Inputs/requests.log
  clang-tools-extra/test/clangd/index-tools.test

Index: clang-tools-extra/test/clangd/index-tools.test
===
--- clang-tools-extra/test/clangd/index-tools.test
+++ clang-tools-extra/test/clangd/index-tools.test
@@ -1,3 +1,3 @@
 # RUN: clangd-indexer %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs > %t.index
 # FIXME: By default, benchmarks are excluded from the list of default targets hence not built. Find a way to depend on benchmarks to run the next command.
-# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.log --benchmark_min_time=0.01 ; fi
+# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.json --benchmark_min_time=0.01 ; fi
Index: clang-tools-extra/test/clangd/Inputs/requests.log
===
--- clang-tools-extra/test/clangd/Inputs/requests.log
+++ /dev/null
@@ -1,5 +0,0 @@
-V[09:08:34.301] Code complete: fuzzyFind("s", scopes=[llvm::])
-V[09:08:34.368] Code complete: fuzzyFind("sy", scopes=[llvm::])
-V[09:08:34.442] Code complete: fuzzyFind("sys", scopes=[llvm::])
-V[09:09:12.075] Code complete: fuzzyFind("Dex", scopes=[clang::clangd::, clang::, clang::clangd::dex::])
-V[09:09:12.075] Code complete: fuzzyFind("Variable", scopes=[])
Index: clang-tools-extra/test/clangd/Inputs/requests.json
===
--- /dev/null
+++ clang-tools-extra/test/clangd/Inputs/requests.json
@@ -0,0 +1,7 @@
+[{"MaxCandidateCount":100,"ProximityPaths":["/usr/home/user/clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp"],"Query":"OMP","RestrictForCodeCompletion":true,"Scopes":["clang::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"s","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sy","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Dex","RestrictForCodeCompletion":true,"Scopes":["clang::clangd::", "clang::", "clang::clangd::dex::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Variable","RestrictForCodeCompletion":true,"Scopes":[""]}]
Index: clang-tools-extra/clangd/index/dex/Iterator.cpp
===
--- clang-tools-extra/clangd/index/dex/Iterator.cpp
+++ clang-tools-extra/clangd/index/dex/Iterator.cpp
@@ -198,7 +198,7 @@
 public:
   explicit OrIterator(std::vector> AllChildren)
   : Children(std::move(AllChildren)) {
-assert(Children.size() > 0 && "OR iterator must have at least one child.");
+assert(!Children.empty() && "OR iterator should have at least one child.");
   }
 
   /// Returns true if all children are exhausted.
@@ -405,12 +405,16 @@
 
 std::unique_ptr
 createAnd(std::vector> Children) {
-  return llvm::make_unique(move(Children));
+  // If there is exactly one child, pull it one level up: AND(Child) -> Child.
+  return Children.size() == 1 ? std::move(Children.front())
+  : llvm::make_unique(move(Children));
 }
 
 std::unique_ptr
 createOr(std::vector> Children) {
-  return llvm::make_unique(move(Children));
+  // If there is exactly one child, pull it one level up: OR(Child) -> Child.
+  return Children.size() == 1 ? std::move(Children.front())
+  : llvm::make_unique(move(Children));
 }
 
 std::unique_ptr createTrue(DocID Size) {
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -19,56 +19,46 @@
 #include 
 
 const char *IndexFilename;
-const char *LogFilename;
+const char *RequestsFilename;
 
 namespace clang {
 namespace clangd {
 namespace {
 
-std::unique_ptr buildMem() {
-  return clang::clangd::loadIndex(IndexFilename, {}, false);
+std::unique_ptr buildMem() {
+  return loadIndex(IndexFilename, {}, false);
 }
 
-std::unique_ptr buildDex() {
-  return clang::clangd::loadIndex(IndexFilename, {}, true);
+std::unique_ptr buildDex() {
+  return loadIndex(IndexFilename, {}, true);
 }
 
-// This function processes user-provided Log file with fuzzy f

[PATCH] D51921: [VFS] vfs::directory_iterator yields path and file type instead of full Status

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: include/clang/Basic/VirtualFileSystem.h:135
+  // For compatibility with old Status-based API. Prefer using Path directly.
+  StringRef getName() const { return Path; }
+};

sammccall wrote:
> bkramer wrote:
> > sammccall wrote:
> > > Backwards-compatibility notes:
> > > 
> > >  - Almost all users of `directory_iterator` require no source changes 
> > > (with this change)
> > >  - Implementations of VFS require changes if they support directory 
> > > iteration and do not merely wrap other VFSes. Anecdotally, most do not 
> > > require changes. 
> > > 
> > > So this weird API seems worth having to make out-of-tree breakages less 
> > > painful.
> > > Happy to update the internal uses though if that seems worthwhile.
> > Can we mirror llvm::sys::fs::directory_entry's interface? I want the APIs 
> > to be as close as possible. Upgrading users is not a big deal.
> How much of the interface are you talking about? :-)
> 
> Making these private and calling the accessors `file()` and `type()` is easy 
> of course, and consistency is nice.
> 
> Supporting status() with similar semantics+performance is both complicated 
> and... not a good idea, I think. See the other patch where I added a comment 
> like "this interface is wrong" and didn't fix it :-)
> 
> The other random functions on fs::directory_entry seem like random 
> implementation cruft to me.
I've done the `path()` and `type()` rename, so now we're consistent with 
fs::directory_entry.
And inconsistent with clang::vfs::Status, and with clang::DirectoryEntry (from 
FileManager).
Can't win em all!


Repository:
  rC Clang

https://reviews.llvm.org/D51921



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

[PATCH] D51982: [clangd] Introduce PostingList interface

[Eric Liu via Phabricator via cfe-commits]

ioeric added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:125
+  for (const auto &TokenToPostingList : TempInvertedIndex)
+InvertedIndex.insert(
+{TokenToPostingList.first,

nit: `InvertedIndex[TokenToPostingList.first] = buildSparsePostingList(...)` ?



Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:23
+/// compress underlying data.
+class SparsePostingList : public PostingList {
+public:

I'd probably call this something other than `SparsePostingList`. It doesn't 
really do anything fancier than just storing the vector. What if sparse posting 
lists have their own optimizations one day? So how about something more 
straightforward? E.g. `VectorPostingList` or `PlainPostingList`?



Comment at: clang-tools-extra/clangd/index/dex/PostingList.cpp:40
+  /// std::vector::const_iterator.
+  class SparseIterator : public Iterator {
+  public:

As this is just a helper class that is not exposed, I'd suggest making this a 
standalone class to make it easier to read.


https://reviews.llvm.org/D51982



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

[clang-tools-extra] r342129 - [docs] Provide pointers to known editor plugins and extensions

[Kirill Bobyrev via cfe-commits]

Author: omtcyfz
Date: Thu Sep 13 04:40:12 2018
New Revision: 342129

URL: http://llvm.org/viewvc/llvm-project?rev=342129&view=rev
Log:
[docs] Provide pointers to known editor plugins and extensions

Many editors provide extensions and plugins with LSP Client
functionality. Many of these are known to work with Clangd, this patch
points users to the relevant resources for better experience.

Reviewed By: ioeric, ilya-biryukov

Differential Revision

Modified:
clang-tools-extra/trunk/docs/clangd.rst

Modified: clang-tools-extra/trunk/docs/clangd.rst
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/docs/clangd.rst?rev=342129&r1=342128&r2=342129&view=diff
==
--- clang-tools-extra/trunk/docs/clangd.rst (original)
+++ clang-tools-extra/trunk/docs/clangd.rst Thu Sep 13 04:40:12 2018
@@ -108,6 +108,41 @@ extension to the protocol.
 | Gen. Getters/Setters| No |   No |
 +-++--+
 
+Editor Integration
+==
+
+Any full-featured Language Server Protocol Client implementation should work
+with :program:`Clangd`. This `list
+` contains information about
+extensions and plugins that are known to work for different editors.
+
+Vim Integration
+---
+
+LanguageClient-neovim
+~
+
+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
+instructions.
+
+VSCode Integration
+--
+
+:program:`VSCode` provides `vscode-clangd
+`
+which is published in Visual Studio Marketplace and can be installed direcetly
+from :program:`VSCode`.
+
+Emacs Integration
+-
+
+:program:`Emacs` provides `lsp-mode ` and
+`Eglot ` plugins for LSP integration.
+
 Getting Involved
 ==
 


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

[PATCH] D51921: [VFS] vfs::directory_iterator yields path and file type instead of full Status

[Kristina Brooks via Phabricator via cfe-commits]

kristina added a comment.

Added inline comments regarding code style and internal/external naming.




Comment at: include/clang/Basic/VirtualFileSystem.h:135
+
+public:
+  directory_entry() = default;

The consistency is mostly related to naming of class members or function args 
or locals, I think the external APIs should use whatever is most suitable, in 
this case `path()` and `type()` and `status()` would make sense style wise, I 
think. Just my two cents in, not insisting on any style comments this time :)


Repository:
  rC Clang

https://reviews.llvm.org/D51921



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

[PATCH] D51996: [clangd] Simplify cancellation public API

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In https://reviews.llvm.org/D51996#1232770, @ilya-biryukov wrote:

> > but the thing you're spending the CPU on is checking for cancellation...
>
> Unless checking for cancellation is really cheap (which is doable, I think).
>  We should probably hit some of those in practice before doing something, 
> though.


My point is that if a few pointer traversals/comparisons is significant, then 
the loop is so fast that checking for cancellations doesn't make sense anyway - 
we're aiming to save 10s of ms, not a few hundred nanos.

Like you say, maybe there are important cases where we can't tell that we're 
calling frequently, or can't easily avoid that. We don't have any yet though, 
so optimizing this seems premature.




Comment at: clangd/Cancellation.h:81
+/// Always false if there is no active cancelable task.
+/// This isn't free (context lookup) - don't call it in a tight loop.
+bool isCancelled();

ilya-biryukov wrote:
> Allowing fast checking for cancellation seem important.
> E.g. if we want to cancel in the middle of parsing, we'll probably be faced 
> with sema callbacks, frequency of which we don't control, so we'd better 
> design something that's fast to check.
> 
> I see two ways to deal with this:
> - Keep the current design, but cache last access to context key.
> - Add an API to get something that can quickly check for cancellation, i.e. 
> something that would hold a reference to resolved context key.
> 
> Both could be added later, though, and I don't have any data, it's just my 
> intuition.
Yeah, we have a few options if we get there here.
Maybe the simplest is

```
if (LLVM_UNLIKELY(++Counter % 1000 == 0))
 if (isCancelled())
  return;
```
(obviously this is limited, but it depends what problem we're facing)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51996



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

[clang-tools-extra] r342130 - [clangd] Simplify cancellation public API

[Sam McCall via cfe-commits]

Author: sammccall
Date: Thu Sep 13 04:47:48 2018
New Revision: 342130

URL: http://llvm.org/viewvc/llvm-project?rev=342130&view=rev
Log:
[clangd] Simplify cancellation public API

Summary:
Task is no longer exposed:
 - task cancellation is hidden as a std::function
 - task creation returns the new context directly
 - checking is via free function only, with no way to avoid the context lookup
The implementation is essentially the same, but a bit terser as it's hidden.

isCancelled() is now safe to use outside any task (it returns false).
This will leave us free to sprinkle cancellation in e.g. TUScheduler without
needing elaborate test setup, and lets callers that don't cancel "just work".

Updated the docs to describe the new expected use pattern.
One thing I noticed: there's nothing async-specific about the cancellation.
Async tasks can be cancelled from any thread (typically the one that created
them), sync tasks can be cancelled from any *other* thread in the same way.
So the docs now refer to "long-running" tasks instead of async ones.

Updated usage in code complete, without any structural changes.
I didn't update all the names of the helpers in ClangdLSPServer (these will
likely be moved to JSONRPCDispatcher anyway).

Reviewers: ilya-biryukov, kadircet

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

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

Modified:
clang-tools-extra/trunk/clangd/Cancellation.cpp
clang-tools-extra/trunk/clangd/Cancellation.h
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/unittests/clangd/CancellationTests.cpp

Modified: clang-tools-extra/trunk/clangd/Cancellation.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Cancellation.cpp?rev=342130&r1=342129&r2=342130&view=diff
==
--- clang-tools-extra/trunk/clangd/Cancellation.cpp (original)
+++ clang-tools-extra/trunk/clangd/Cancellation.cpp Thu Sep 13 04:47:48 2018
@@ -13,21 +13,21 @@
 namespace clang {
 namespace clangd {
 
-namespace {
-static Key TaskKey;
-} // namespace
-
 char CancelledError::ID = 0;
+static Key>> FlagKey;
 
-const Task &getCurrentTask() {
-  const auto TH = Context::current().getExisting(TaskKey);
-  assert(TH && "Fetched a nullptr for TaskHandle from context.");
-  return *TH;
+std::pair cancelableTask() {
+  auto Flag = std::make_shared>();
+  return {
+  Context::current().derive(FlagKey, Flag),
+  [Flag] { *Flag = true; },
+  };
 }
 
-Context setCurrentTask(ConstTaskHandle TH) {
-  assert(TH && "Trying to stash a nullptr as TaskHandle into context.");
-  return Context::current().derive(TaskKey, std::move(TH));
+bool isCancelled() {
+  if (auto *Flag = Context::current().get(FlagKey))
+return **Flag;
+  return false; // Not in scope of a task.
 }
 
 } // namespace clangd

Modified: clang-tools-extra/trunk/clangd/Cancellation.h
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/Cancellation.h?rev=342130&r1=342129&r2=342130&view=diff
==
--- clang-tools-extra/trunk/clangd/Cancellation.h (original)
+++ clang-tools-extra/trunk/clangd/Cancellation.h Thu Sep 13 04:47:48 2018
@@ -6,124 +6,82 @@
 // License. See LICENSE.TXT for details.
 //
 
//===--===//
-// Cancellation mechanism for async tasks. Roughly all the clients of this code
-// can be classified into three categories:
-// 1. The code that creates and schedules async tasks, e.g. TUScheduler.
-// 2. The callers of the async method that can cancel some of the running 
tasks,
-// e.g. `ClangdLSPServer`
-// 3. The code running inside the async task itself, i.e. code completion or
-// find definition implementation that run clang, etc.
-//
-// For (1), the guideline is to accept a callback for the result of async
-// operation and return a `TaskHandle` to allow cancelling the request.
-//
-//  TaskHandle someAsyncMethod(Runnable T,
-//  function)> Callback) {
-//   auto TH = Task::createHandle();
-//   WithContext ContextWithCancellationToken(TH);
-//   auto run = [](){
-// Callback(T());
+// Cancellation mechanism for long-running tasks.
+//
+// This manages interactions between:
+//
+// 1. Client code that starts some long-running work, and maybe cancels later.
+//
+//   std::pair Task = cancelableTask();
+//   {
+// WithContext Cancelable(std::move(Task.first));
+// Expected
+// deepThoughtAsync([](int answer){ errs() << answer; });
+//   }
+//   // ...some time later...
+//   if (User.fellAsleep())
+// Task.second();
+//
+//  (This example has an asynchronous computation, but synchronous examples
+//  work similarly - the Canceler should 

[PATCH] D51996: [clangd] Simplify cancellation public API

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE342130: [clangd] Simplify cancellation public API 
(authored by sammccall, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D51996?vs=165158&id=165251#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51996

Files:
  clangd/Cancellation.cpp
  clangd/Cancellation.h
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  unittests/clangd/CancellationTests.cpp

Index: unittests/clangd/CancellationTests.cpp
===
--- unittests/clangd/CancellationTests.cpp
+++ unittests/clangd/CancellationTests.cpp
@@ -13,57 +13,48 @@
 namespace {
 
 TEST(CancellationTest, CancellationTest) {
-  TaskHandle TH = Task::createHandle();
-  WithContext ContextWithCancellation(setCurrentTask(TH));
+  auto Task = cancelableTask();
+  WithContext ContextWithCancellation(std::move(Task.first));
   EXPECT_FALSE(isCancelled());
-  TH->cancel();
+  Task.second();
   EXPECT_TRUE(isCancelled());
 }
 
-TEST(CancellationTest, TaskTestHandleDiesContextLives) {
+TEST(CancellationTest, CancelerDiesContextLives) {
   llvm::Optional ContextWithCancellation;
   {
-TaskHandle TH = Task::createHandle();
-ContextWithCancellation.emplace(setCurrentTask(TH));
+auto Task = cancelableTask();
+ContextWithCancellation.emplace(std::move(Task.first));
 EXPECT_FALSE(isCancelled());
-TH->cancel();
+Task.second();
 EXPECT_TRUE(isCancelled());
   }
   EXPECT_TRUE(isCancelled());
 }
 
 TEST(CancellationTest, TaskContextDiesHandleLives) {
-  TaskHandle TH = Task::createHandle();
+  auto Task = cancelableTask();
   {
-WithContext ContextWithCancellation(setCurrentTask(TH));
+WithContext ContextWithCancellation(std::move(Task.first));
 EXPECT_FALSE(isCancelled());
-TH->cancel();
+Task.second();
 EXPECT_TRUE(isCancelled());
   }
   // Still should be able to cancel without any problems.
-  TH->cancel();
-}
-
-TEST(CancellationTest, CancellationToken) {
-  TaskHandle TH = Task::createHandle();
-  WithContext ContextWithCancellation(setCurrentTask(TH));
-  const auto &CT = getCurrentTask();
-  EXPECT_FALSE(CT.isCancelled());
-  TH->cancel();
-  EXPECT_TRUE(CT.isCancelled());
+  Task.second();
 }
 
 TEST(CancellationTest, AsynCancellationTest) {
   std::atomic HasCancelled(false);
   Notification Cancelled;
-  auto TaskToBeCancelled = [&](ConstTaskHandle CT) {
-WithContext ContextGuard(setCurrentTask(std::move(CT)));
+  auto TaskToBeCancelled = [&](Context Ctx) {
+WithContext ContextGuard(std::move(Ctx));
 Cancelled.wait();
 HasCancelled = isCancelled();
   };
-  TaskHandle TH = Task::createHandle();
-  std::thread AsyncTask(TaskToBeCancelled, TH);
-  TH->cancel();
+  auto Task = cancelableTask();
+  std::thread AsyncTask(TaskToBeCancelled, std::move(Task.first));
+  Task.second();
   Cancelled.notify();
   AsyncTask.join();
 
Index: clangd/Cancellation.h
===
--- clangd/Cancellation.h
+++ clangd/Cancellation.h
@@ -6,124 +6,82 @@
 // License. See LICENSE.TXT for details.
 //
 //===--===//
-// Cancellation mechanism for async tasks. Roughly all the clients of this code
-// can be classified into three categories:
-// 1. The code that creates and schedules async tasks, e.g. TUScheduler.
-// 2. The callers of the async method that can cancel some of the running tasks,
-// e.g. `ClangdLSPServer`
-// 3. The code running inside the async task itself, i.e. code completion or
-// find definition implementation that run clang, etc.
-//
-// For (1), the guideline is to accept a callback for the result of async
-// operation and return a `TaskHandle` to allow cancelling the request.
-//
-//  TaskHandle someAsyncMethod(Runnable T,
-//  function)> Callback) {
-//   auto TH = Task::createHandle();
-//   WithContext ContextWithCancellationToken(TH);
-//   auto run = [](){
-// Callback(T());
+// Cancellation mechanism for long-running tasks.
+//
+// This manages interactions between:
+//
+// 1. Client code that starts some long-running work, and maybe cancels later.
+//
+//   std::pair Task = cancelableTask();
+//   {
+// WithContext Cancelable(std::move(Task.first));
+// Expected
+// deepThoughtAsync([](int answer){ errs() << answer; });
 //   }
-//   // Start run() in a new async thread, and make sure to propagate Context.
-//   return TH;
-// }
-//
-// The callers of async methods (2) can issue cancellations and should be
-// prepared to handle `TaskCancelledError` result:
-//
-// void Caller() {
-//   // You should store this handle if you wanna cancel the task later on.
-//   TaskHandle TH = someAsyncMethod(Task, [](llvm::Expected R) {
-// if(/*check for task cancellation error*/)
-//   // Handle the error
-// //

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

[Oliver Stannard via Phabricator via cfe-commits]

olista01 added a comment.

This looks like it has the same problem as https://reviews.llvm.org/D51418 
(doesn't get applied to C++ static constructor functions).


Repository:
  rC Clang

https://reviews.llvm.org/D51429



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

[PATCH] D51747: [clangd] Implement deprecation diagnostics with lower severity.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

As discussed offline with Ilya, decided to keep the compile flag addition since 
it would be easier to pass the logic of a command line flag to that point. Also 
not changing the severity level, since they will show up on diagnostics lists 
in anyway it doesn't save much.




Comment at: clangd/ClangdServer.cpp:537
   C->CommandLine.push_back("-resource-dir=" + ResourceDir);
+  C->CommandLine.push_back("-Wdeprecated-declarations");
   return std::move(*C);

sammccall wrote:
> as noted above I think we should also have -Wno-error=deprecated-declarations
> 
> (do you want all of -Wdeprecated, actually?)
Yes for the second part and no for the first part. As we saw there are some 
configs out there that treat some types of deprecation warnings as errors, so 
we don't want to change that behavior.



Comment at: clangd/Diagnostics.cpp:299
+D.Severity =
+D.Category == "Deprecations" ? DiagnosticsEngine::Note : DiagLevel;
 return D;

sammccall wrote:
> not sure what the concrete benefits are from using Note rather than Warning. 
> It's semantically iffy, so if we do this it should have a comment justifying 
> it.
Agree on that one, decided on not implementing such a thing until it becomes 
necessary.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51747



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

[PATCH] D51747: [clangd] Implement deprecation diagnostics with lower severity.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 165252.
kadircet marked 5 inline comments as done.
kadircet added a comment.

- Resolve discussions.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51747

Files:
  clangd/ClangdServer.cpp
  unittests/clangd/ClangdUnitTests.cpp


Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -220,6 +220,44 @@
   }
 }
 
+TEST(DiagnosticsTest, DiagnosticDeprecated) {
+  Annotations Test(R"cpp(
+void foo() __attribute__(($foodeprecation[[deprecated]]));
+class A {
+public:
+  int x __attribute__(($xdeprecation[[deprecated]]));
+};
+int main() {
+  $foo[[foo]]();
+  return A().$x[[x]];
+}
+  )cpp");
+  EXPECT_THAT(
+  TestTU::withCode(Test.code()).build().getDiagnostics(),
+  ElementsAre(
+  AllOf(Diag(Test.range("foo"), "'foo' is deprecated"),
+WithNote(
+Diag(Test.range("foodeprecation"),
+ "'foo' has been explicitly marked deprecated here"))),
+  AllOf(Diag(Test.range("x"), "'x' is deprecated"),
+WithNote(
+Diag(Test.range("xdeprecation"),
+ "'x' has been explicitly marked deprecated here");
+}
+
+TEST(DiagnosticsTest, DiagnosticDeprecatedWithFix) {
+  Annotations Test(R"cpp(
+void bar();
+void foo() __attribute__((deprecated("", "bar")));
+int main() {
+  $deprecated[[foo]]();
+}
+  )cpp");
+  EXPECT_THAT(TestTU::withCode(Test.code()).build().getDiagnostics(),
+  ElementsAre(WithFix(Fix(Test.range("deprecated"), "bar",
+  "change 'foo' to 'bar'";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -531,9 +531,11 @@
   if (!C) // FIXME: Suppress diagnostics? Let the user know?
 C = CDB.getFallbackCommand(File);
 
-  // Inject the resource dir.
+  // Inject the resource dir. These flags are working for both gcc and clang-cl
+  // driver modes.
   // FIXME: Don't overwrite it if it's already there.
   C->CommandLine.push_back("-resource-dir=" + ResourceDir);
+  C->CommandLine.push_back("-Wdeprecated");
   return std::move(*C);
 }
 


Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -220,6 +220,44 @@
   }
 }
 
+TEST(DiagnosticsTest, DiagnosticDeprecated) {
+  Annotations Test(R"cpp(
+void foo() __attribute__(($foodeprecation[[deprecated]]));
+class A {
+public:
+  int x __attribute__(($xdeprecation[[deprecated]]));
+};
+int main() {
+  $foo[[foo]]();
+  return A().$x[[x]];
+}
+  )cpp");
+  EXPECT_THAT(
+  TestTU::withCode(Test.code()).build().getDiagnostics(),
+  ElementsAre(
+  AllOf(Diag(Test.range("foo"), "'foo' is deprecated"),
+WithNote(
+Diag(Test.range("foodeprecation"),
+ "'foo' has been explicitly marked deprecated here"))),
+  AllOf(Diag(Test.range("x"), "'x' is deprecated"),
+WithNote(
+Diag(Test.range("xdeprecation"),
+ "'x' has been explicitly marked deprecated here");
+}
+
+TEST(DiagnosticsTest, DiagnosticDeprecatedWithFix) {
+  Annotations Test(R"cpp(
+void bar();
+void foo() __attribute__((deprecated("", "bar")));
+int main() {
+  $deprecated[[foo]]();
+}
+  )cpp");
+  EXPECT_THAT(TestTU::withCode(Test.code()).build().getDiagnostics(),
+  ElementsAre(WithFix(Fix(Test.range("deprecated"), "bar",
+  "change 'foo' to 'bar'";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -531,9 +531,11 @@
   if (!C) // FIXME: Suppress diagnostics? Let the user know?
 C = CDB.getFallbackCommand(File);
 
-  // Inject the resource dir.
+  // Inject the resource dir. These flags are working for both gcc and clang-cl
+  // driver modes.
   // FIXME: Don't overwrite it if it's already there.
   C->CommandLine.push_back("-resource-dir=" + ResourceDir);
+  C->CommandLine.push_back("-Wdeprecated");
   return std::move(*C);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52004: [clangd] Allow all LSP methods to signal cancellation via $/cancelRequest

[Sam McCall via Phabricator via cfe-commits]

sammccall updated this revision to Diff 165253.
sammccall marked 2 inline comments as done.
sammccall added a comment.

rebase and address comments


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52004

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/JSONRPCDispatcher.cpp
  clangd/JSONRPCDispatcher.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h

Index: clangd/ProtocolHandlers.h
===
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -56,7 +56,6 @@
   virtual void onDocumentHighlight(TextDocumentPositionParams &Params) = 0;
   virtual void onHover(TextDocumentPositionParams &Params) = 0;
   virtual void onChangeConfiguration(DidChangeConfigurationParams &Params) = 0;
-  virtual void onCancelRequest(CancelParams &Params) = 0;
 };
 
 void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher,
Index: clangd/ProtocolHandlers.cpp
===
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -76,5 +76,4 @@
   Register("workspace/didChangeConfiguration",
&ProtocolCallbacks::onChangeConfiguration);
   Register("workspace/symbol", &ProtocolCallbacks::onWorkspaceSymbol);
-  Register("$/cancelRequest", &ProtocolCallbacks::onCancelRequest);
 }
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -886,20 +886,6 @@
 };
 bool fromJSON(const llvm::json::Value &, ReferenceParams &);
 
-struct CancelParams {
-  /// The request id to cancel.
-  /// This can be either a number or string, if it is a number simply print it
-  /// out and always use a string.
-  std::string ID;
-};
-llvm::json::Value toJSON(const CancelParams &);
-llvm::raw_ostream &operator<<(llvm::raw_ostream &, const CancelParams &);
-bool fromJSON(const llvm::json::Value &, CancelParams &);
-
-/// Param can be either of type string or number. Returns the result as a
-/// string.
-llvm::Optional parseNumberOrString(const llvm::json::Value *Param);
-
 } // namespace clangd
 } // namespace clang
 
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -623,38 +623,5 @@
   return fromJSON(Params, Base);
 }
 
-json::Value toJSON(const CancelParams &CP) {
-  return json::Object{{"id", CP.ID}};
-}
-
-llvm::raw_ostream &operator<<(llvm::raw_ostream &O, const CancelParams &CP) {
-  O << toJSON(CP);
-  return O;
-}
-
-llvm::Optional parseNumberOrString(const json::Value *Params) {
-  if (!Params)
-return llvm::None;
-  // ID is either a number or a string, check for both.
-  if(const auto AsString = Params->getAsString())
-return AsString->str();
-
-  if(const auto AsNumber = Params->getAsInteger())
-return itostr(AsNumber.getValue());
-
-  return llvm::None;
-}
-
-bool fromJSON(const json::Value &Params, CancelParams &CP) {
-  const auto ParamsAsObject = Params.getAsObject();
-  if (!ParamsAsObject)
-return false;
-  if (auto Parsed = parseNumberOrString(ParamsAsObject->get("id"))) {
-CP.ID = std::move(*Parsed);
-return true;
-  }
-  return false;
-}
-
 } // namespace clangd
 } // namespace clang
Index: clangd/JSONRPCDispatcher.h
===
--- clangd/JSONRPCDispatcher.h
+++ clangd/JSONRPCDispatcher.h
@@ -10,6 +10,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_JSONRPCDISPATCHER_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_JSONRPCDISPATCHER_H
 
+#include "Cancellation.h"
 #include "Logger.h"
 #include "Protocol.h"
 #include "Trace.h"
@@ -79,21 +80,32 @@
 /// registered Handler for the method received.
 class JSONRPCDispatcher {
 public:
-  // A handler responds to requests for a particular method name.
+  /// A handler responds to requests for a particular method name.
+  ///
+  /// JSONRPCDispatcher will mark the handler's context as cancelled if a
+  /// matching cancellation request is received. Handlers are encouraged to
+  /// check for cancellation and fail quickly in this case.
   using Handler = std::function;
 
   /// Create a new JSONRPCDispatcher. UnknownHandler is called when an unknown
   /// method is received.
-  JSONRPCDispatcher(Handler UnknownHandler)
-  : UnknownHandler(std::move(UnknownHandler)) {}
+  JSONRPCDispatcher(Handler UnknownHandler);
 
   /// Registers a Handler for the specified Method.
   void registerHandler(StringRef Method, Handler H);
 
   /// Parses a JSONRPC message and calls the Handler for it.
-  bool call(const llvm::json::Value &Message, JSONOutput &Out) const;
+  bool call(const llvm::json::Value &Message, JSONOutput &Out);
 
 private:
+  // Tracking cancellations needs a mutex: handlers may finish on a different
+  // thread, and that's when we clean up entries in the map.

[PATCH] D51297: [docs] Provide pointers to known editor plugins and extensions

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev closed this revision.
kbobyrev added a comment.

Unfortunately, forgot to add the revision link in the commit message of 
https://reviews.llvm.org/rCTE342129.


https://reviews.llvm.org/D51297



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

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

[Oliver Stannard via Phabricator via cfe-commits]

olista01 added inline comments.



Comment at: src/DwarfInstructions.hpp:210
+register unsigned long long x16 __asm("x16") = cfa;
+asm("autia1716": "+r"(x17): "r"(x16));
+returnAddress = x17;

I don't think this will work for cross-unwinding builds: for them, 
_LIBUNWIND_TARGET_AARCH64 is defined even when the compilation target is not 
AArch64, so this instruction won't exist.

Fully supporting cross-unwinding looks non-trivial: we'd need to either provide 
some way to ask the client to authenticate a pointer on the target, or strip 
the high bits of the pointer (which requires knowing the virtual address size 
of the target). For now, I think it's OK to not support cross-unwinding.



Comment at: src/Registers.hpp:1835
+  if (((regNum >= 0) && (regNum < 32)) || regNum == UNW_ARM64_RA_SIGN_STATE)
 return _registers.__x[regNum];
+

When regNum == UNW_ARM64_RA_SIGN_STATE, the index into __x is out of range. 
We'll need to add new storage to hold this value, I'd suggest replacing the 
current padding value in the GPRs struct, as that will avoid changing the 
layout expected by the context save/restore functions.



Comment at: src/Registers.hpp:1845
 _registers.__sp = value;
-  else if ((regNum >= 0) && (regNum < 32))
+  else if ((regNum >= 0) && (regNum < 32) || regNum == UNW_ARM64_RA_SIGN_STATE)
 _registers.__x[regNum] = value;

Ditto.


Repository:
  rUNW libunwind

https://reviews.llvm.org/D51432



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

[PATCH] D52004: [clangd] Allow all LSP methods to signal cancellation via $/cancelRequest

[Sam McCall via Phabricator via cfe-commits]

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

Update ClangdLSPServer comment, document cancelRequest further.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52004

Files:
  clangd/ClangdLSPServer.cpp
  clangd/ClangdLSPServer.h
  clangd/ClangdServer.cpp
  clangd/ClangdServer.h
  clangd/JSONRPCDispatcher.cpp
  clangd/JSONRPCDispatcher.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  clangd/ProtocolHandlers.cpp
  clangd/ProtocolHandlers.h

Index: clangd/ProtocolHandlers.h
===
--- clangd/ProtocolHandlers.h
+++ clangd/ProtocolHandlers.h
@@ -56,7 +56,6 @@
   virtual void onDocumentHighlight(TextDocumentPositionParams &Params) = 0;
   virtual void onHover(TextDocumentPositionParams &Params) = 0;
   virtual void onChangeConfiguration(DidChangeConfigurationParams &Params) = 0;
-  virtual void onCancelRequest(CancelParams &Params) = 0;
 };
 
 void registerCallbackHandlers(JSONRPCDispatcher &Dispatcher,
Index: clangd/ProtocolHandlers.cpp
===
--- clangd/ProtocolHandlers.cpp
+++ clangd/ProtocolHandlers.cpp
@@ -76,5 +76,4 @@
   Register("workspace/didChangeConfiguration",
&ProtocolCallbacks::onChangeConfiguration);
   Register("workspace/symbol", &ProtocolCallbacks::onWorkspaceSymbol);
-  Register("$/cancelRequest", &ProtocolCallbacks::onCancelRequest);
 }
Index: clangd/Protocol.h
===
--- clangd/Protocol.h
+++ clangd/Protocol.h
@@ -886,20 +886,6 @@
 };
 bool fromJSON(const llvm::json::Value &, ReferenceParams &);
 
-struct CancelParams {
-  /// The request id to cancel.
-  /// This can be either a number or string, if it is a number simply print it
-  /// out and always use a string.
-  std::string ID;
-};
-llvm::json::Value toJSON(const CancelParams &);
-llvm::raw_ostream &operator<<(llvm::raw_ostream &, const CancelParams &);
-bool fromJSON(const llvm::json::Value &, CancelParams &);
-
-/// Param can be either of type string or number. Returns the result as a
-/// string.
-llvm::Optional parseNumberOrString(const llvm::json::Value *Param);
-
 } // namespace clangd
 } // namespace clang
 
Index: clangd/Protocol.cpp
===
--- clangd/Protocol.cpp
+++ clangd/Protocol.cpp
@@ -623,38 +623,5 @@
   return fromJSON(Params, Base);
 }
 
-json::Value toJSON(const CancelParams &CP) {
-  return json::Object{{"id", CP.ID}};
-}
-
-llvm::raw_ostream &operator<<(llvm::raw_ostream &O, const CancelParams &CP) {
-  O << toJSON(CP);
-  return O;
-}
-
-llvm::Optional parseNumberOrString(const json::Value *Params) {
-  if (!Params)
-return llvm::None;
-  // ID is either a number or a string, check for both.
-  if(const auto AsString = Params->getAsString())
-return AsString->str();
-
-  if(const auto AsNumber = Params->getAsInteger())
-return itostr(AsNumber.getValue());
-
-  return llvm::None;
-}
-
-bool fromJSON(const json::Value &Params, CancelParams &CP) {
-  const auto ParamsAsObject = Params.getAsObject();
-  if (!ParamsAsObject)
-return false;
-  if (auto Parsed = parseNumberOrString(ParamsAsObject->get("id"))) {
-CP.ID = std::move(*Parsed);
-return true;
-  }
-  return false;
-}
-
 } // namespace clangd
 } // namespace clang
Index: clangd/JSONRPCDispatcher.h
===
--- clangd/JSONRPCDispatcher.h
+++ clangd/JSONRPCDispatcher.h
@@ -10,6 +10,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_JSONRPCDISPATCHER_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_JSONRPCDISPATCHER_H
 
+#include "Cancellation.h"
 #include "Logger.h"
 #include "Protocol.h"
 #include "Trace.h"
@@ -77,23 +78,37 @@
 
 /// Main JSONRPC entry point. This parses the JSONRPC "header" and calls the
 /// registered Handler for the method received.
+///
+/// The `$/cancelRequest` notification is handled by the dispatcher itself.
+/// It marks the matching request as cancelled, if it's still running.
 class JSONRPCDispatcher {
 public:
-  // A handler responds to requests for a particular method name.
+  /// A handler responds to requests for a particular method name.
+  ///
+  /// JSONRPCDispatcher will mark the handler's context as cancelled if a
+  /// matching cancellation request is received. Handlers are encouraged to
+  /// check for cancellation and fail quickly in this case.
   using Handler = std::function;
 
   /// Create a new JSONRPCDispatcher. UnknownHandler is called when an unknown
   /// method is received.
-  JSONRPCDispatcher(Handler UnknownHandler)
-  : UnknownHandler(std::move(UnknownHandler)) {}
+  JSONRPCDispatcher(Handler UnknownHandler);
 
   /// Registers a Handler for the specified Method.
   void registerHandler(StringRef Method, Handler H);
 
   /// Parses a JSONRPC message and calls the Handler for it.
-  bool call(const llvm::json::Value &Message, JSONOutput &Out

[PATCH] D52004: [clangd] Allow all LSP methods to signal cancellation via $/cancelRequest

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In https://reviews.llvm.org/D52004#1232512, @kadircet wrote:

> Wonder if we can still keep the onCancelRequest registry within 
> ProtocolHandler's scope, so that it is clear that we implement it. Other than 
> that seems fascinating, thanks!


Hmm, I'm not sure how to do that while keeping things simple. I've updated the 
documentation on ClangdLSPServer and JSONRPCDispatcher to try to clarify 
instead. PTAL




Comment at: clangd/JSONRPCDispatcher.cpp:246
+  auto StrID = llvm::to_string(ID); // JSON-serialize ID for map key.
+  auto Cookie = RequestCookie++;
+  std::lock_guard Lock(CancelMutex);

kadircet wrote:
> Maybe we can say something about this method is actually being invoked in a 
> sync manner and the reason why we have mutex below is due to context 
> destruction of the thread we might spawn later on. Because it bugged me at 
> first look not having this line under mutex as well, then I noticed actually 
> this was still a sync function.
Ah. Renamed this variable, commented the access, renamed the mutex to make it 
clear it's only for the map.
(The mutex itself has a comment about the fact that the *cleanups* are what 
happen off-thread)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52004



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

[PATCH] D51982: [clangd] Introduce PostingList interface

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 165256.
kbobyrev marked 3 inline comments as done.

https://reviews.llvm.org/D51982

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/Dex.cpp
  clang-tools-extra/clangd/index/dex/Dex.h
  clang-tools-extra/clangd/index/dex/Iterator.cpp
  clang-tools-extra/clangd/index/dex/Iterator.h
  clang-tools-extra/clangd/index/dex/PostingList.cpp
  clang-tools-extra/clangd/index/dex/PostingList.h
  clang-tools-extra/unittests/clangd/DexTests.cpp

Index: clang-tools-extra/unittests/clangd/DexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexTests.cpp
@@ -47,8 +47,8 @@
 }
 
 TEST(DexIterators, DocumentIterator) {
-  const PostingList L = {4, 7, 8, 20, 42, 100};
-  auto DocIterator = create(L);
+  const auto L = buildPlainPostingList({4, 7, 8, 20, 42, 100});
+  auto DocIterator = L->iterator();
 
   EXPECT_EQ(DocIterator->peek(), 4U);
   EXPECT_FALSE(DocIterator->reachedEnd());
@@ -70,28 +70,28 @@
 }
 
 TEST(DexIterators, AndWithEmpty) {
-  const PostingList L0;
-  const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000};
+  const auto L0 = buildPlainPostingList({});
+  const auto L1 = buildPlainPostingList({0, 5, 7, 10, 42, 320, 9000});
 
-  auto AndEmpty = createAnd(create(L0));
+  auto AndEmpty = createAnd(L0->iterator());
   EXPECT_TRUE(AndEmpty->reachedEnd());
 
-  auto AndWithEmpty = createAnd(create(L0), create(L1));
+  auto AndWithEmpty = createAnd(L0->iterator(), L1->iterator());
   EXPECT_TRUE(AndWithEmpty->reachedEnd());
 
   EXPECT_THAT(consumeIDs(*AndWithEmpty), ElementsAre());
 }
 
 TEST(DexIterators, AndTwoLists) {
-  const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000};
-  const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000};
+  const auto L0 = buildPlainPostingList({0, 5, 7, 10, 42, 320, 9000});
+  const auto L1 = buildPlainPostingList({0, 4, 7, 10, 30, 60, 320, 9000});
 
-  auto And = createAnd(create(L1), create(L0));
+  auto And = createAnd(L1->iterator(), L0->iterator());
 
   EXPECT_FALSE(And->reachedEnd());
   EXPECT_THAT(consumeIDs(*And), ElementsAre(0U, 7U, 10U, 320U, 9000U));
 
-  And = createAnd(create(L0), create(L1));
+  And = createAnd(L0->iterator(), L1->iterator());
 
   And->advanceTo(0);
   EXPECT_EQ(And->peek(), 0U);
@@ -107,11 +107,11 @@
 }
 
 TEST(DexIterators, AndThreeLists) {
-  const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000};
-  const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000};
-  const PostingList L2 = {1, 4, 7, 11, 30, 60, 320, 9000};
+  const auto L0 = buildPlainPostingList({0, 5, 7, 10, 42, 320, 9000});
+  const auto L1 = buildPlainPostingList({0, 4, 7, 10, 30, 60, 320, 9000});
+  const auto L2 = buildPlainPostingList({1, 4, 7, 11, 30, 60, 320, 9000});
 
-  auto And = createAnd(create(L0), create(L1), create(L2));
+  auto And = createAnd(L0->iterator(), L1->iterator(), L2->iterator());
   EXPECT_EQ(And->peek(), 7U);
   And->advanceTo(300);
   EXPECT_EQ(And->peek(), 320U);
@@ -121,24 +121,24 @@
 }
 
 TEST(DexIterators, OrWithEmpty) {
-  const PostingList L0;
-  const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000};
+  const auto L0 = buildPlainPostingList({});
+  const auto L1 = buildPlainPostingList({0, 5, 7, 10, 42, 320, 9000});
 
-  auto OrEmpty = createOr(create(L0));
+  auto OrEmpty = createOr(L0->iterator());
   EXPECT_TRUE(OrEmpty->reachedEnd());
 
-  auto OrWithEmpty = createOr(create(L0), create(L1));
+  auto OrWithEmpty = createOr(L0->iterator(), L1->iterator());
   EXPECT_FALSE(OrWithEmpty->reachedEnd());
 
   EXPECT_THAT(consumeIDs(*OrWithEmpty),
   ElementsAre(0U, 5U, 7U, 10U, 42U, 320U, 9000U));
 }
 
 TEST(DexIterators, OrTwoLists) {
-  const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000};
-  const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000};
+  const auto L0 = buildPlainPostingList({0, 5, 7, 10, 42, 320, 9000});
+  const auto L1 = buildPlainPostingList({0, 4, 7, 10, 30, 60, 320, 9000});
 
-  auto Or = createOr(create(L0), create(L1));
+  auto Or = createOr(L0->iterator(), L1->iterator());
 
   EXPECT_FALSE(Or->reachedEnd());
   EXPECT_EQ(Or->peek(), 0U);
@@ -161,18 +161,18 @@
   Or->advanceTo(9001);
   EXPECT_TRUE(Or->reachedEnd());
 
-  Or = createOr(create(L0), create(L1));
+  Or = createOr(L0->iterator(), L1->iterator());
 
   EXPECT_THAT(consumeIDs(*Or),
   ElementsAre(0U, 4U, 5U, 7U, 10U, 30U, 42U, 60U, 320U, 9000U));
 }
 
 TEST(DexIterators, OrThreeLists) {
-  const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000};
-  const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000};
-  const PostingList L2 = {1, 4, 7, 11, 30, 60, 320, 9000};
+  const auto L0 = buildPlainPostingList({0, 5, 7, 10, 42, 320, 9000});
+  const auto L1 = buildPlainPostingList({0, 4, 7, 10, 30, 60, 320, 9000});
+  const auto L2 = buildPlainPostingList({1, 4, 7, 11, 30, 60, 320, 9000});
 
-  auto Or = createOr(create(L0), create(L1), create(L2));
+  auto Or = createOr

[PATCH] D51747: [clangd] Implement deprecation diagnostics with lower severity.

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/ClangdServer.cpp:537
   C->CommandLine.push_back("-resource-dir=" + ResourceDir);
+  C->CommandLine.push_back("-Wdeprecated-declarations");
   return std::move(*C);

kadircet wrote:
> sammccall wrote:
> > as noted above I think we should also have 
> > -Wno-error=deprecated-declarations
> > 
> > (do you want all of -Wdeprecated, actually?)
> Yes for the second part and no for the first part. As we saw there are some 
> configs out there that treat some types of deprecation warnings as errors, so 
> we don't want to change that behavior.
Doesn't this cause problems when the build flags are `-Wno-deprecated -Werror` 
then?
We make this `-Wno-deprecated -Werror -Wdeprecated`, and now all uses of 
deprecated APIs are errors.

This seems like a common configuration, highly noticeable symptoms, and not 
what the user wants...


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51747



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

[PATCH] D52004: [clangd] Allow all LSP methods to signal cancellation via $/cancelRequest

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52004



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

[PATCH] D51921: [VFS] vfs::directory_iterator yields path and file type instead of full Status

[Benjamin Kramer via Phabricator via cfe-commits]

bkramer added inline comments.



Comment at: include/clang/Basic/VirtualFileSystem.h:135
+  // For compatibility with old Status-based API. Prefer using Path directly.
+  StringRef getName() const { return Path; }
+};

sammccall wrote:
> sammccall wrote:
> > bkramer wrote:
> > > sammccall wrote:
> > > > Backwards-compatibility notes:
> > > > 
> > > >  - Almost all users of `directory_iterator` require no source changes 
> > > > (with this change)
> > > >  - Implementations of VFS require changes if they support directory 
> > > > iteration and do not merely wrap other VFSes. Anecdotally, most do not 
> > > > require changes. 
> > > > 
> > > > So this weird API seems worth having to make out-of-tree breakages less 
> > > > painful.
> > > > Happy to update the internal uses though if that seems worthwhile.
> > > Can we mirror llvm::sys::fs::directory_entry's interface? I want the APIs 
> > > to be as close as possible. Upgrading users is not a big deal.
> > How much of the interface are you talking about? :-)
> > 
> > Making these private and calling the accessors `file()` and `type()` is 
> > easy of course, and consistency is nice.
> > 
> > Supporting status() with similar semantics+performance is both complicated 
> > and... not a good idea, I think. See the other patch where I added a 
> > comment like "this interface is wrong" and didn't fix it :-)
> > 
> > The other random functions on fs::directory_entry seem like random 
> > implementation cruft to me.
> I've done the `path()` and `type()` rename, so now we're consistent with 
> fs::directory_entry.
> And inconsistent with clang::vfs::Status, and with clang::DirectoryEntry 
> (from FileManager).
> Can't win em all!
Not exposing random fields and having the same names as the other 
directory_entry is what I wanted :)

status() is deep in YAGNI territory.


Repository:
  rC Clang

https://reviews.llvm.org/D51921



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

[PATCH] D51982: [clangd] Introduce PostingList interface

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

At a high level: making posting lists an abstract type adds another layer of 
indirection, which we can use to solve problems. It also has costs, mostly 
conceptual complexity.
What problems are we solving?

- if **we want to dynamically use a different representation for different 
data/scenarios**: this would be a good solution, do we have any evidence that 
this is the case? I thought the tentative conclusion was vbyte was good enough 
for all cases. What's the deal with dense/sparse?
- if **it's too hard to experiment with different posting list types**: how 
hard is it really? I'm not sure this justifies checking this change in.

I'd suggest as a first step making PostingList a concrete class - this would 
improve the API and enable experimentation. (Even experimentation with 
dynamically switching the implementation - it's easier behind a class boundary)




Comment at: clang-tools-extra/clangd/index/dex/PostingList.h:34
+
+/// PostingList is an interface for the storage of DocIDs which can be inserted
+/// to the Query Tree as a leaf by constructing Iterator over given 
PostingList.

this talks a lot about how a posting list can be implemented, and what it 
interacts with - I'd suggest removing/reducing that and talking about what it 
is, instead :-)



Comment at: clang-tools-extra/clangd/index/dex/PostingList.h:54
+
+/// Returns posting list with sparse in-memory representation. This is useful
+/// for small size posting lists with huge gaps between subsequent DocIDs.

why is the caller responsible for choosing the implementation?
It seems like buildPostingList could do this based on inspecting the docs



Comment at: clang-tools-extra/clangd/index/dex/PostingList.h:61
+std::unique_ptr
+buildPlainPostingList(const std::vector &&Documents);
+

PostingList::create()?


https://reviews.llvm.org/D51982



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

[PATCH] D52028: [clangd] Cleanup FuzzyFindRequest filtering limit semantics

[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, jkorous, MaskRay.

As discussed during https://reviews.llvm.org/D51860 review, it is better to use 
`llvm::Optional` here as it has clear semantics which reflect intended behavior.


https://reviews.llvm.org/D52028

Files:
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/MemIndex.cpp
  clang-tools-extra/clangd/index/dex/Dex.cpp
  clang-tools-extra/unittests/clangd/DexTests.cpp
  clang-tools-extra/unittests/clangd/IndexTests.cpp

Index: clang-tools-extra/unittests/clangd/IndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/IndexTests.cpp
+++ clang-tools-extra/unittests/clangd/IndexTests.cpp
@@ -78,10 +78,11 @@
   auto I = MemIndex::build(generateNumSymbols(0, 100), RefSlab());
   FuzzyFindRequest Req;
   Req.Query = "5";
-  Req.MaxCandidateCount = 3;
+  Req.Limit = 3;
   bool Incomplete;
   auto Matches = match(*I, Req, &Incomplete);
-  EXPECT_EQ(Matches.size(), Req.MaxCandidateCount);
+  EXPECT_TRUE(Req.Limit);
+  EXPECT_EQ(Matches.size(), *Req.Limit);
   EXPECT_TRUE(Incomplete);
 }
 
@@ -91,7 +92,7 @@
   RefSlab());
   FuzzyFindRequest Req;
   Req.Query = "lol";
-  Req.MaxCandidateCount = 2;
+  Req.Limit = 2;
   EXPECT_THAT(match(*I, Req),
   UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
 }
Index: clang-tools-extra/unittests/clangd/DexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexTests.cpp
@@ -482,7 +482,7 @@
   URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "lol";
-  Req.MaxCandidateCount = 2;
+  Req.Limit = 2;
   EXPECT_THAT(match(*I, Req),
   UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
 }
@@ -498,17 +498,19 @@
   FuzzyFindRequest Req;
   Req.Query = "2";
   Dex I(Symbols, URISchemes);
+  EXPECT_FALSE(Req.Limit);
   EXPECT_THAT(match(I, Req), ElementsAre("2", "2"));
 }
 
 TEST(DexTest, DexLimitedNumMatches) {
   auto I = Dex::build(generateNumSymbols(0, 100), URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "5";
-  Req.MaxCandidateCount = 3;
+  Req.Limit = 3;
   bool Incomplete;
   auto Matches = match(*I, Req, &Incomplete);
-  EXPECT_EQ(Matches.size(), Req.MaxCandidateCount);
+  EXPECT_TRUE(Req.Limit);
+  EXPECT_EQ(Matches.size(), *Req.Limit);
   EXPECT_TRUE(Incomplete);
 }
 
@@ -518,7 +520,7 @@
   URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "lol";
-  Req.MaxCandidateCount = 2;
+  Req.Limit = 2;
   EXPECT_THAT(match(*I, Req),
   UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
 }
@@ -596,7 +598,7 @@
   FuzzyFindRequest Req;
   Req.Query = "abc";
   // The best candidate can change depending on the proximity paths.
-  Req.MaxCandidateCount = 1;
+  Req.Limit = 1;
 
   // FuzzyFind request comes from the file which is far from the root: expect
   // CloseSymbol to come out.
Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -179,16 +179,17 @@
   // FIXME(kbobyrev): Pre-scoring retrieval threshold should be adjusted as
   // using 100x of the requested number might not be good in practice, e.g.
   // when the requested number of items is small.
-  const size_t ItemsToRetrieve = 100 * Req.MaxCandidateCount;
-  auto Root = createLimit(move(QueryIterator), ItemsToRetrieve);
+  auto Root = Req.Limit ? createLimit(move(QueryIterator), *Req.Limit * 100)
+: move(QueryIterator);
 
   using IDAndScore = std::pair;
   std::vector IDAndScores = consume(*Root);
 
   auto Compare = [](const IDAndScore &LHS, const IDAndScore &RHS) {
 return LHS.second > RHS.second;
   };
-  TopN Top(Req.MaxCandidateCount, Compare);
+  TopN Top(
+  Req.Limit ? *Req.Limit : std::numeric_limits::max(), Compare);
   for (const auto &IDAndScore : IDAndScores) {
 const DocID SymbolDocID = IDAndScore.first;
 const auto *Sym = Symbols[SymbolDocID];
@@ -205,7 +206,7 @@
   More = true;
   }
 
-  // Apply callback to the top Req.MaxCandidateCount items in the descending
+  // Apply callback to the top Req.Limit items in the descending
   // order of cumulative score.
   for (const auto &Item : std::move(Top).items())
 Callback(*Symbols[Item.first]);
Index: clang-tools-extra/clangd/index/MemIndex.cpp
===
--- clang-tools-extra/clangd/index/MemIndex.cpp
+++ clang-tools-extra/clangd/index/MemIndex.cpp
@@ -29,7 +29,8 @@
   assert(!StringRef(Req.Query).contains("::") &&
  "There must be no :: in query.");
 
-  TopN> Top(Req.MaxCandida

[PATCH] D51977: [clangd] Clarify and hide -index flag.

[Eric Liu via Phabricator via cfe-commits]

ioeric updated this revision to Diff 165263.
ioeric marked an inline comment as done.
ioeric added a comment.

- Remove `static`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51977

Files:
  clangd/tool/ClangdMain.cpp


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -130,10 +130,11 @@
 
 static llvm::cl::opt EnableIndex(
 "index",
-llvm::cl::desc("Enable index-based features such as global code completion 
"
-   "and searching for symbols. "
-   "Clang uses an index built from symbols in opened files"),
-llvm::cl::init(true));
+llvm::cl::desc(
+"Enable index-based features. By default, clangd maintains an index "
+"built from symbols in opened files. Global index support needs to "
+"enabled separatedly."),
+llvm::cl::init(true), llvm::cl::Hidden);
 
 static llvm::cl::opt
 ShowOrigins("debug-origin",


Index: clangd/tool/ClangdMain.cpp
===
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -130,10 +130,11 @@
 
 static llvm::cl::opt EnableIndex(
 "index",
-llvm::cl::desc("Enable index-based features such as global code completion "
-   "and searching for symbols. "
-   "Clang uses an index built from symbols in opened files"),
-llvm::cl::init(true));
+llvm::cl::desc(
+"Enable index-based features. By default, clangd maintains an index "
+"built from symbols in opened files. Global index support needs to "
+"enabled separatedly."),
+llvm::cl::init(true), llvm::cl::Hidden);
 
 static llvm::cl::opt
 ShowOrigins("debug-origin",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r342134 - [clangd] Clarify and hide -index flag.

[Eric Liu via cfe-commits]

Author: ioeric
Date: Thu Sep 13 05:53:23 2018
New Revision: 342134

URL: http://llvm.org/viewvc/llvm-project?rev=342134&view=rev
Log:
[clangd] Clarify and hide -index flag.

Summary:
The wording implies global index support, which is confusing.
As most users shouldn't care about this flag, also make it hidden to avoid
further confusion.

Reviewers: sammccall, ilya-biryukov

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

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

Modified:
clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp

Modified: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp?rev=342134&r1=342133&r2=342134&view=diff
==
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp (original)
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp Thu Sep 13 05:53:23 2018
@@ -130,10 +130,11 @@ static llvm::cl::opt InputMirrorFi
 
 static llvm::cl::opt EnableIndex(
 "index",
-llvm::cl::desc("Enable index-based features such as global code completion 
"
-   "and searching for symbols. "
-   "Clang uses an index built from symbols in opened files"),
-llvm::cl::init(true));
+llvm::cl::desc(
+"Enable index-based features. By default, clangd maintains an index "
+"built from symbols in opened files. Global index support needs to "
+"enabled separatedly."),
+llvm::cl::init(true), llvm::cl::Hidden);
 
 static llvm::cl::opt
 ShowOrigins("debug-origin",


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

[PATCH] D51977: [clangd] Clarify and hide -index flag.

[Eric Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342134: [clangd] Clarify and hide -index flag. (authored by 
ioeric, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D51977

Files:
  clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
@@ -130,10 +130,11 @@
 
 static llvm::cl::opt EnableIndex(
 "index",
-llvm::cl::desc("Enable index-based features such as global code completion 
"
-   "and searching for symbols. "
-   "Clang uses an index built from symbols in opened files"),
-llvm::cl::init(true));
+llvm::cl::desc(
+"Enable index-based features. By default, clangd maintains an index "
+"built from symbols in opened files. Global index support needs to "
+"enabled separatedly."),
+llvm::cl::init(true), llvm::cl::Hidden);
 
 static llvm::cl::opt
 ShowOrigins("debug-origin",


Index: clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/trunk/clangd/tool/ClangdMain.cpp
@@ -130,10 +130,11 @@
 
 static llvm::cl::opt EnableIndex(
 "index",
-llvm::cl::desc("Enable index-based features such as global code completion "
-   "and searching for symbols. "
-   "Clang uses an index built from symbols in opened files"),
-llvm::cl::init(true));
+llvm::cl::desc(
+"Enable index-based features. By default, clangd maintains an index "
+"built from symbols in opened files. Global index support needs to "
+"enabled separatedly."),
+llvm::cl::init(true), llvm::cl::Hidden);
 
 static llvm::cl::opt
 ShowOrigins("debug-origin",
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D52030: [clangd] Test benchmark if we can build it

[Sam McCall via Phabricator via cfe-commits]

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

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52030

Files:
  test/CMakeLists.txt
  test/clangd/index-tools.test


Index: test/clangd/index-tools.test
===
--- test/clangd/index-tools.test
+++ test/clangd/index-tools.test
@@ -1,3 +1,3 @@
 # RUN: clangd-indexer %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs > %t.index
-# FIXME: By default, benchmarks are excluded from the list of default targets 
hence not built. Find a way to depend on benchmarks to run the next command.
+# FIXME: This is fragile. Can we express the LLVM_BUILD_BENCHMARKS dep 
directly?
 # RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then 
%clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.log 
--benchmark_min_time=0.01 ; fi
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -72,6 +72,13 @@
 )
 endif()
 
+# clangd's benchmark has an optional test.
+if(LLVM_BUILD_BENCHMARKS)
+  list(APPEND CLANG_TOOLS_TEST_DEPS
+IndexBenchmark
+)
+endif()
+
 set(llvm_utils
   FileCheck count not
   )


Index: test/clangd/index-tools.test
===
--- test/clangd/index-tools.test
+++ test/clangd/index-tools.test
@@ -1,3 +1,3 @@
 # RUN: clangd-indexer %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs > %t.index
-# FIXME: By default, benchmarks are excluded from the list of default targets hence not built. Find a way to depend on benchmarks to run the next command.
+# FIXME: This is fragile. Can we express the LLVM_BUILD_BENCHMARKS dep directly?
 # RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.log --benchmark_min_time=0.01 ; fi
Index: test/CMakeLists.txt
===
--- test/CMakeLists.txt
+++ test/CMakeLists.txt
@@ -72,6 +72,13 @@
 )
 endif()
 
+# clangd's benchmark has an optional test.
+if(LLVM_BUILD_BENCHMARKS)
+  list(APPEND CLANG_TOOLS_TEST_DEPS
+IndexBenchmark
+)
+endif()
+
 set(llvm_utils
   FileCheck count not
   )
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D51747: [clangd] Implement deprecation diagnostics with lower severity.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Do not show up as errors even on codebases with -Werror.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51747

Files:
  clangd/ClangdServer.cpp
  unittests/clangd/ClangdUnitTests.cpp


Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -220,6 +220,44 @@
   }
 }
 
+TEST(DiagnosticsTest, DiagnosticDeprecated) {
+  Annotations Test(R"cpp(
+void foo() __attribute__(($foodeprecation[[deprecated]]));
+class A {
+public:
+  int x __attribute__(($xdeprecation[[deprecated]]));
+};
+int main() {
+  $foo[[foo]]();
+  return A().$x[[x]];
+}
+  )cpp");
+  EXPECT_THAT(
+  TestTU::withCode(Test.code()).build().getDiagnostics(),
+  ElementsAre(
+  AllOf(Diag(Test.range("foo"), "'foo' is deprecated"),
+WithNote(
+Diag(Test.range("foodeprecation"),
+ "'foo' has been explicitly marked deprecated here"))),
+  AllOf(Diag(Test.range("x"), "'x' is deprecated"),
+WithNote(
+Diag(Test.range("xdeprecation"),
+ "'x' has been explicitly marked deprecated here");
+}
+
+TEST(DiagnosticsTest, DiagnosticDeprecatedWithFix) {
+  Annotations Test(R"cpp(
+void bar();
+void foo() __attribute__((deprecated("", "bar")));
+int main() {
+  $deprecated[[foo]]();
+}
+  )cpp");
+  EXPECT_THAT(TestTU::withCode(Test.code()).build().getDiagnostics(),
+  ElementsAre(WithFix(Fix(Test.range("deprecated"), "bar",
+  "change 'foo' to 'bar'";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -531,9 +531,12 @@
   if (!C) // FIXME: Suppress diagnostics? Let the user know?
 C = CDB.getFallbackCommand(File);
 
-  // Inject the resource dir.
+  // Inject the resource dir. These flags are working for both gcc and clang-cl
+  // driver modes.
   // FIXME: Don't overwrite it if it's already there.
   C->CommandLine.push_back("-resource-dir=" + ResourceDir);
+  C->CommandLine.push_back("-Wdeprecated");
+  C->CommandLine.push_back("-Wno-error=deprecated");
   return std::move(*C);
 }
 


Index: unittests/clangd/ClangdUnitTests.cpp
===
--- unittests/clangd/ClangdUnitTests.cpp
+++ unittests/clangd/ClangdUnitTests.cpp
@@ -220,6 +220,44 @@
   }
 }
 
+TEST(DiagnosticsTest, DiagnosticDeprecated) {
+  Annotations Test(R"cpp(
+void foo() __attribute__(($foodeprecation[[deprecated]]));
+class A {
+public:
+  int x __attribute__(($xdeprecation[[deprecated]]));
+};
+int main() {
+  $foo[[foo]]();
+  return A().$x[[x]];
+}
+  )cpp");
+  EXPECT_THAT(
+  TestTU::withCode(Test.code()).build().getDiagnostics(),
+  ElementsAre(
+  AllOf(Diag(Test.range("foo"), "'foo' is deprecated"),
+WithNote(
+Diag(Test.range("foodeprecation"),
+ "'foo' has been explicitly marked deprecated here"))),
+  AllOf(Diag(Test.range("x"), "'x' is deprecated"),
+WithNote(
+Diag(Test.range("xdeprecation"),
+ "'x' has been explicitly marked deprecated here");
+}
+
+TEST(DiagnosticsTest, DiagnosticDeprecatedWithFix) {
+  Annotations Test(R"cpp(
+void bar();
+void foo() __attribute__((deprecated("", "bar")));
+int main() {
+  $deprecated[[foo]]();
+}
+  )cpp");
+  EXPECT_THAT(TestTU::withCode(Test.code()).build().getDiagnostics(),
+  ElementsAre(WithFix(Fix(Test.range("deprecated"), "bar",
+  "change 'foo' to 'bar'";
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -531,9 +531,12 @@
   if (!C) // FIXME: Suppress diagnostics? Let the user know?
 C = CDB.getFallbackCommand(File);
 
-  // Inject the resource dir.
+  // Inject the resource dir. These flags are working for both gcc and clang-cl
+  // driver modes.
   // FIXME: Don't overwrite it if it's already there.
   C->CommandLine.push_back("-resource-dir=" + ResourceDir);
+  C->CommandLine.push_back("-Wdeprecated");
+  C->CommandLine.push_back("-Wno-error=deprecated");
   return std::move(*C);
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] r342135 - [clangd] Allow all LSP methods to signal cancellation via $/cancelRequest

[Sam McCall via cfe-commits]

Author: sammccall
Date: Thu Sep 13 05:58:36 2018
New Revision: 342135

URL: http://llvm.org/viewvc/llvm-project?rev=342135&view=rev
Log:
[clangd] Allow all LSP methods to signal cancellation via $/cancelRequest

Summary:
The cancelable scopes are managed by JSONRPCDispatcher so that all Handlers
run in cancelable contexts.
(Previously ClangdServer did this, for code completion only).

Cancellation request processing is therefore also in JSONRPCDispatcher.
(Previously it was in ClangdLSPServer).

This doesn't actually make any new commands *respect* cancellation - they'd
need to check isCancelled() and bail out. But it opens the door to doing
this incrementally, and putting such logic in common machinery like TUScheduler.

I also rewrote the ClangdServer class/threading comments because I wanted to
add to it and I got carried away.

Reviewers: ilya-biryukov, kadircet

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

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

Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp
clang-tools-extra/trunk/clangd/JSONRPCDispatcher.h
clang-tools-extra/trunk/clangd/Protocol.cpp
clang-tools-extra/trunk/clangd/Protocol.h
clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
clang-tools-extra/trunk/clangd/ProtocolHandlers.h

Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=342135&r1=342134&r2=342135&view=diff
==
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Thu Sep 13 05:58:36 2018
@@ -8,7 +8,6 @@
 
//===--===//
 
 #include "ClangdLSPServer.h"
-#include "Cancellation.h"
 #include "Diagnostics.h"
 #include "JSONRPCDispatcher.h"
 #include "SourceCode.h"
@@ -71,11 +70,6 @@ SymbolKindBitset defaultSymbolKinds() {
   return Defaults;
 }
 
-std::string NormalizeRequestID(const json::Value &ID) {
-  auto NormalizedID = parseNumberOrString(&ID);
-  assert(NormalizedID && "Was not able to parse request id.");
-  return std::move(*NormalizedID);
-}
 } // namespace
 
 void ClangdLSPServer::onInitialize(InitializeParams &Params) {
@@ -347,21 +341,16 @@ void ClangdLSPServer::onCodeAction(CodeA
 }
 
 void ClangdLSPServer::onCompletion(TextDocumentPositionParams &Params) {
-  CreateSpaceForTaskHandle();
-  Canceler Cancel = Server.codeComplete(
-  Params.textDocument.uri.file(), Params.position, CCOpts,
-  [this](llvm::Expected List) {
-auto _ = llvm::make_scope_exit([this]() { CleanupTaskHandle(); });
-
-if (!List)
-  return replyError(List.takeError());
-CompletionList LSPList;
-LSPList.isIncomplete = List->HasMore;
-for (const auto &R : List->Completions)
-  LSPList.items.push_back(R.render(CCOpts));
-return reply(std::move(LSPList));
-  });
-  StoreTaskHandle(std::move(Cancel));
+  Server.codeComplete(Params.textDocument.uri.file(), Params.position, CCOpts,
+  [this](llvm::Expected List) {
+if (!List)
+  return replyError(List.takeError());
+CompletionList LSPList;
+LSPList.isIncomplete = List->HasMore;
+for (const auto &R : List->Completions)
+  LSPList.items.push_back(R.render(CCOpts));
+return reply(std::move(LSPList));
+  });
 }
 
 void ClangdLSPServer::onSignatureHelp(TextDocumentPositionParams &Params) {
@@ -629,48 +618,3 @@ GlobalCompilationDatabase &ClangdLSPServ
 return *CachingCDB;
   return *CDB;
 }
-
-void ClangdLSPServer::onCancelRequest(CancelParams &Params) {
-  std::lock_guard Lock(TaskHandlesMutex);
-  const auto &It = TaskHandles.find(Params.ID);
-  if (It == TaskHandles.end())
-return;
-  It->second();
-  TaskHandles.erase(It);
-}
-
-void ClangdLSPServer::CleanupTaskHandle() {
-  const json::Value *ID = getRequestId();
-  if (!ID)
-return;
-  std::string NormalizedID = NormalizeRequestID(*ID);
-  std::lock_guard Lock(TaskHandlesMutex);
-  TaskHandles.erase(NormalizedID);
-}
-
-void ClangdLSPServer::CreateSpaceForTaskHandle() {
-  const json::Value *ID = getRequestId();
-  if (!ID)
-return;
-  std::string NormalizedID = NormalizeRequestID(*ID);
-  std::lock_guard Lock(TaskHandlesMutex);
-  if (!TaskHandles.insert({NormalizedID, nullptr}).second)
-elog("Creation of space for task handle: {0} failed.", NormalizedID);
-}
-
-void ClangdLSPServer::StoreTaskHandle(Canceler TH) {
-  const json::Value *

[PATCH] D51747: [clangd] Implement deprecation diagnostics with lower severity.

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clangd/ClangdServer.cpp:537
   C->CommandLine.push_back("-resource-dir=" + ResourceDir);
+  C->CommandLine.push_back("-Wdeprecated-declarations");
   return std::move(*C);

sammccall wrote:
> kadircet wrote:
> > sammccall wrote:
> > > as noted above I think we should also have 
> > > -Wno-error=deprecated-declarations
> > > 
> > > (do you want all of -Wdeprecated, actually?)
> > Yes for the second part and no for the first part. As we saw there are some 
> > configs out there that treat some types of deprecation warnings as errors, 
> > so we don't want to change that behavior.
> Doesn't this cause problems when the build flags are `-Wno-deprecated 
> -Werror` then?
> We make this `-Wno-deprecated -Werror -Wdeprecated`, and now all uses of 
> deprecated APIs are errors.
> 
> This seems like a common configuration, highly noticeable symptoms, and not 
> what the user wants...
Well, adding -Wno-error=deprecated to get over this case. Now we might have 
problems if people wanted to see deprecation warnings as errors, but I believe 
this shouldn't be much of a problem since they will get those errors at built 
anytime and we won't be annoying other users(ones that show deprecation as 
warnings or not showing them at all, hopefully almost everyone?) too much.

WDYT?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51747



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

[PATCH] D52004: [clangd] Allow all LSP methods to signal cancellation via $/cancelRequest

[Sam McCall via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL342135: [clangd] Allow all LSP methods to signal 
cancellation via $/cancelRequest (authored by sammccall, committed by ).
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

https://reviews.llvm.org/D52004

Files:
  clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
  clang-tools-extra/trunk/clangd/ClangdLSPServer.h
  clang-tools-extra/trunk/clangd/ClangdServer.cpp
  clang-tools-extra/trunk/clangd/ClangdServer.h
  clang-tools-extra/trunk/clangd/JSONRPCDispatcher.cpp
  clang-tools-extra/trunk/clangd/JSONRPCDispatcher.h
  clang-tools-extra/trunk/clangd/Protocol.cpp
  clang-tools-extra/trunk/clangd/Protocol.h
  clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
  clang-tools-extra/trunk/clangd/ProtocolHandlers.h

Index: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h
@@ -26,8 +26,11 @@
 class JSONOutput;
 class SymbolIndex;
 
-/// This class provides implementation of an LSP server, glueing the JSON
-/// dispatch and ClangdServer together.
+/// This class exposes ClangdServer's capabilities via Language Server Protocol.
+///
+/// JSONRPCDispatcher binds the implemented ProtocolCallbacks methods
+/// (e.g. onInitialize) to corresponding JSON-RPC methods ("initialize").
+/// The server also supports $/cancelRequest (JSONRPCDispatcher provides this).
 class ClangdLSPServer : private DiagnosticsConsumer, private ProtocolCallbacks {
 public:
   /// If \p CompileCommandsDir has a value, compile_commands.json will be
@@ -76,7 +79,6 @@
   void onRename(RenameParams &Parames) override;
   void onHover(TextDocumentPositionParams &Params) override;
   void onChangeConfiguration(DidChangeConfigurationParams &Params) override;
-  void onCancelRequest(CancelParams &Params) override;
 
   std::vector getFixes(StringRef File, const clangd::Diagnostic &D);
 
@@ -169,21 +171,6 @@
   // the worker thread that may otherwise run an async callback on partially
   // destructed instance of ClangdLSPServer.
   ClangdServer Server;
-
-  // Holds cancelers for running requets. Key of the map is a serialized
-  // request id. FIXME: handle cancellation generically in JSONRPCDispatcher.
-  llvm::StringMap TaskHandles;
-  std::mutex TaskHandlesMutex;
-
-  // Following three functions are for managing TaskHandles map. They store or
-  // remove a task handle for the request-id stored in current Context.
-  // FIXME(kadircet): Wrap the following three functions in a RAII object to
-  // make sure these do not get misused. The object might be stored in the
-  // Context of the thread or moved around until a reply is generated for the
-  // request.
-  void CleanupTaskHandle();
-  void CreateSpaceForTaskHandle();
-  void StoreTaskHandle(Canceler TH);
 };
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
===
--- clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
+++ clang-tools-extra/trunk/clangd/ProtocolHandlers.cpp
@@ -76,5 +76,4 @@
   Register("workspace/didChangeConfiguration",
&ProtocolCallbacks::onChangeConfiguration);
   Register("workspace/symbol", &ProtocolCallbacks::onWorkspaceSymbol);
-  Register("$/cancelRequest", &ProtocolCallbacks::onCancelRequest);
 }
Index: clang-tools-extra/trunk/clangd/Protocol.h
===
--- clang-tools-extra/trunk/clangd/Protocol.h
+++ clang-tools-extra/trunk/clangd/Protocol.h
@@ -886,20 +886,6 @@
 };
 bool fromJSON(const llvm::json::Value &, ReferenceParams &);
 
-struct CancelParams {
-  /// The request id to cancel.
-  /// This can be either a number or string, if it is a number simply print it
-  /// out and always use a string.
-  std::string ID;
-};
-llvm::json::Value toJSON(const CancelParams &);
-llvm::raw_ostream &operator<<(llvm::raw_ostream &, const CancelParams &);
-bool fromJSON(const llvm::json::Value &, CancelParams &);
-
-/// Param can be either of type string or number. Returns the result as a
-/// string.
-llvm::Optional parseNumberOrString(const llvm::json::Value *Param);
-
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/trunk/clangd/JSONRPCDispatcher.h
===
--- clang-tools-extra/trunk/clangd/JSONRPCDispatcher.h
+++ clang-tools-extra/trunk/clangd/JSONRPCDispatcher.h
@@ -10,6 +10,7 @@
 #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_JSONRPCDISPATCHER_H
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_JSONRPCDISPATCHER_H
 
+#include "Cancellation.h"
 #include "Logger.h"
 #include "Protocol.h"
 #include "Trace.h"
@@ -77,23 +78,37 @@
 
 /// Main JSONRPC entry point. This parses the JSONRPC "header" and calls the
 /// registered Handler for the method received.
+

[PATCH] D51725: Allow un-setting the compilation database path

[Simon Marchi via Phabricator via cfe-commits]

simark added a comment.

In https://reviews.llvm.org/D51725#1232748, @ilya-biryukov wrote:

> If this setting exposed directly the users in Theia or is it something that 
> is exposed via a custom UI (e.g. choosing named build configs or something 
> similar)?


It is through a custom UI, that we name exactly that, build configurations.  
There is an example of the corresponding configuration here:

https://github.com/theia-ide/theia/blob/master/packages/cpp/README.md

>> I'll investigate how difficult it is to make our frontend (Theia 
>> ) restart clangd when the user switches 
>> compilation database.
> 
> If that is feasible, might allow removing some code from clangd. Though don't 
> expect that amount to be too high.
>  One potential complication to restoring the state of the language server. It 
> should be just the list of open files, but I may be missing something.

I am not sure I understand correctly your last point.  Of course, when 
restarting clangd, we need to send again a `didOpen` for each file the user has 
currently open in the editor.  But that should be  it?

> If you'll decide to go with an option to reset the path, see the comment 
> about making empty path special. `Optional>` does not play nicely 
> with json serialization code and hard to read (even though it does look like 
> the right thing to do from the type system perspective).

Yes, noted.  I preferred to avoid giving a special meaning to an empty string 
because we could avoid it.  But I wouldn't mind changing it if we go that route.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51725



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

[PATCH] D51747: [clangd] Show deprecation diagnostics

[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/ClangdServer.cpp:534
 
-  // Inject the resource dir.
+  // Inject the resource dir. These flags are working for both gcc and clang-cl
+  // driver modes.

nit: the "these flags" is not just for resource dir, can you move that second 
sentence onto a line above?



Comment at: clangd/ClangdServer.cpp:538
   C->CommandLine.push_back("-resource-dir=" + ResourceDir);
+  C->CommandLine.push_back("-Wdeprecated");
+  C->CommandLine.push_back("-Wno-error=deprecated");

each of these lines deserves a comment I think.
e.g.
`// Deprecations are often hidden for full-project build. They're useful in 
context.`
-Wdeprecated
`// Adding -Wdeprecated would trigger errors in projects what set -Werror.`
-Wno-error=deprecated



Comment at: unittests/clangd/ClangdUnitTests.cpp:223
 
+TEST(DiagnosticsTest, DiagnosticDeprecated) {
+  Annotations Test(R"cpp(

I think you should set TestTU.ExtraArgs to "-Wno-deprecated" (with a comment) 
to verify it's overridden.
Both for clarity, and because I think -Wdeprecated is actually on by default, 
so I think this test as written would pass without your change.



Comment at: unittests/clangd/ClangdUnitTests.cpp:248
+
+TEST(DiagnosticsTest, DiagnosticDeprecatedWithFix) {
+  Annotations Test(R"cpp(

I'm not sure exactly what this is testing - I think it's a combination of 
features that are tested elsewhere. Feel free to drop.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51747



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

[PATCH] D51982: [clangd] Introduce PostingList interface

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 165268.
kbobyrev marked 2 inline comments as done.

https://reviews.llvm.org/D51982

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/dex/Dex.cpp
  clang-tools-extra/clangd/index/dex/Dex.h
  clang-tools-extra/clangd/index/dex/Iterator.cpp
  clang-tools-extra/clangd/index/dex/Iterator.h
  clang-tools-extra/clangd/index/dex/PostingList.cpp
  clang-tools-extra/clangd/index/dex/PostingList.h
  clang-tools-extra/unittests/clangd/DexTests.cpp

Index: clang-tools-extra/unittests/clangd/DexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexTests.cpp
@@ -47,8 +47,8 @@
 }
 
 TEST(DexIterators, DocumentIterator) {
-  const PostingList L = {4, 7, 8, 20, 42, 100};
-  auto DocIterator = create(L);
+  const auto L = create({4, 7, 8, 20, 42, 100});
+  auto DocIterator = L->iterator();
 
   EXPECT_EQ(DocIterator->peek(), 4U);
   EXPECT_FALSE(DocIterator->reachedEnd());
@@ -70,28 +70,28 @@
 }
 
 TEST(DexIterators, AndWithEmpty) {
-  const PostingList L0;
-  const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000};
+  const auto L0 = create({});
+  const auto L1 = create({0, 5, 7, 10, 42, 320, 9000});
 
-  auto AndEmpty = createAnd(create(L0));
+  auto AndEmpty = createAnd(L0->iterator());
   EXPECT_TRUE(AndEmpty->reachedEnd());
 
-  auto AndWithEmpty = createAnd(create(L0), create(L1));
+  auto AndWithEmpty = createAnd(L0->iterator(), L1->iterator());
   EXPECT_TRUE(AndWithEmpty->reachedEnd());
 
   EXPECT_THAT(consumeIDs(*AndWithEmpty), ElementsAre());
 }
 
 TEST(DexIterators, AndTwoLists) {
-  const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000};
-  const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000};
+  const auto L0 = create({0, 5, 7, 10, 42, 320, 9000});
+  const auto L1 = create({0, 4, 7, 10, 30, 60, 320, 9000});
 
-  auto And = createAnd(create(L1), create(L0));
+  auto And = createAnd(L1->iterator(), L0->iterator());
 
   EXPECT_FALSE(And->reachedEnd());
   EXPECT_THAT(consumeIDs(*And), ElementsAre(0U, 7U, 10U, 320U, 9000U));
 
-  And = createAnd(create(L0), create(L1));
+  And = createAnd(L0->iterator(), L1->iterator());
 
   And->advanceTo(0);
   EXPECT_EQ(And->peek(), 0U);
@@ -107,11 +107,11 @@
 }
 
 TEST(DexIterators, AndThreeLists) {
-  const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000};
-  const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000};
-  const PostingList L2 = {1, 4, 7, 11, 30, 60, 320, 9000};
+  const auto L0 = create({0, 5, 7, 10, 42, 320, 9000});
+  const auto L1 = create({0, 4, 7, 10, 30, 60, 320, 9000});
+  const auto L2 = create({1, 4, 7, 11, 30, 60, 320, 9000});
 
-  auto And = createAnd(create(L0), create(L1), create(L2));
+  auto And = createAnd(L0->iterator(), L1->iterator(), L2->iterator());
   EXPECT_EQ(And->peek(), 7U);
   And->advanceTo(300);
   EXPECT_EQ(And->peek(), 320U);
@@ -121,24 +121,24 @@
 }
 
 TEST(DexIterators, OrWithEmpty) {
-  const PostingList L0;
-  const PostingList L1 = {0, 5, 7, 10, 42, 320, 9000};
+  const auto L0 = create({});
+  const auto L1 = create({0, 5, 7, 10, 42, 320, 9000});
 
-  auto OrEmpty = createOr(create(L0));
+  auto OrEmpty = createOr(L0->iterator());
   EXPECT_TRUE(OrEmpty->reachedEnd());
 
-  auto OrWithEmpty = createOr(create(L0), create(L1));
+  auto OrWithEmpty = createOr(L0->iterator(), L1->iterator());
   EXPECT_FALSE(OrWithEmpty->reachedEnd());
 
   EXPECT_THAT(consumeIDs(*OrWithEmpty),
   ElementsAre(0U, 5U, 7U, 10U, 42U, 320U, 9000U));
 }
 
 TEST(DexIterators, OrTwoLists) {
-  const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000};
-  const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000};
+  const auto L0 = create({0, 5, 7, 10, 42, 320, 9000});
+  const auto L1 = create({0, 4, 7, 10, 30, 60, 320, 9000});
 
-  auto Or = createOr(create(L0), create(L1));
+  auto Or = createOr(L0->iterator(), L1->iterator());
 
   EXPECT_FALSE(Or->reachedEnd());
   EXPECT_EQ(Or->peek(), 0U);
@@ -161,18 +161,18 @@
   Or->advanceTo(9001);
   EXPECT_TRUE(Or->reachedEnd());
 
-  Or = createOr(create(L0), create(L1));
+  Or = createOr(L0->iterator(), L1->iterator());
 
   EXPECT_THAT(consumeIDs(*Or),
   ElementsAre(0U, 4U, 5U, 7U, 10U, 30U, 42U, 60U, 320U, 9000U));
 }
 
 TEST(DexIterators, OrThreeLists) {
-  const PostingList L0 = {0, 5, 7, 10, 42, 320, 9000};
-  const PostingList L1 = {0, 4, 7, 10, 30, 60, 320, 9000};
-  const PostingList L2 = {1, 4, 7, 11, 30, 60, 320, 9000};
+  const auto L0 = create({0, 5, 7, 10, 42, 320, 9000});
+  const auto L1 = create({0, 4, 7, 10, 30, 60, 320, 9000});
+  const auto L2 = create({1, 4, 7, 11, 30, 60, 320, 9000});
 
-  auto Or = createOr(create(L0), create(L1), create(L2));
+  auto Or = createOr(L0->iterator(), L1->iterator(), L2->iterator());
 
   EXPECT_FALSE(Or->reachedEnd());
   EXPECT_EQ(Or->peek(), 0U);
@@ -221,19 +221,19 @@
   //  |1, 5, 7, 9|  |1, 5||0, 3, 5|
   //

[PATCH] D52028: [clangd] Cleanup FuzzyFindRequest filtering limit semantics

[Sam McCall via Phabricator via cfe-commits]

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

Thanks!




Comment at: clang-tools-extra/clangd/index/Index.h:477
   ///
-  /// Returns true if there may be more results (limited by MaxCandidateCount).
+  /// Returns true if there may be more results (limited by
+  /// FuzzyFindRequest.Limit).

nit: 'Req.Limit' would fit on the line



Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:182
   // when the requested number of items is small.
-  const size_t ItemsToRetrieve = 100 * Req.MaxCandidateCount;
-  auto Root = createLimit(move(QueryIterator), ItemsToRetrieve);

wow, this was overflowing!
I guess harmlessly to... numeric_limits::max() - 99 or something? weird.


https://reviews.llvm.org/D52028



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

[PATCH] D51725: Allow un-setting the compilation database path

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clangd/Protocol.h:368
+  /// both Optionals are instantiated.
+  llvm::Optional> compilationDatabasePath;
 

ilya-biryukov wrote:
> Not a big fan or something like this, but maybe give special meaning to empty 
> path instead of wrapping an optional into an optional?
> 
> Double optionals are a real pain to write and read.
Sorry just passing by since I've seen a similar usage that already exists in 
codebase, wanted to point it out.

Some api like this could be easier, to make the decision of using empty path or 
anything else for an invalid path and making it an implementation detail.

https://github.com/llvm-mirror/clang/blob/master/include/clang/Sema/DeclSpec.h#L53


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51725



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

[PATCH] D51971: [clangd] Use JSON format in benchmark requests reader

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:48
+llvm::errs() << "Error when parsing json requests file: "
+ << llvm::toString(JSONArray.takeError());
+exit(1);

We should only call `takeError()` when `JSONArray` is an error. Not an array 
value should be handled separately.

BTW, can we add tests with invalid inputs for both of these cases?



Comment at: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp:56
+if (!fromJSON(Item, Request)) {
+  llvm::errs() << "Error when parsing request.\n";
+  exit(1);

Maybe also output the request that failed to parse?
To help untangle the problem in case it happens.


https://reviews.llvm.org/D51971



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

[PATCH] D52028: [clangd] Cleanup FuzzyFindRequest filtering limit semantics

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Change `IndexBenchmark` accordingly, perform minor cleanup.


https://reviews.llvm.org/D52028

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/MemIndex.cpp
  clang-tools-extra/clangd/index/dex/Dex.cpp
  clang-tools-extra/unittests/clangd/DexTests.cpp
  clang-tools-extra/unittests/clangd/IndexTests.cpp

Index: clang-tools-extra/unittests/clangd/IndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/IndexTests.cpp
+++ clang-tools-extra/unittests/clangd/IndexTests.cpp
@@ -78,10 +78,11 @@
   auto I = MemIndex::build(generateNumSymbols(0, 100), RefSlab());
   FuzzyFindRequest Req;
   Req.Query = "5";
-  Req.MaxCandidateCount = 3;
+  Req.Limit = 3;
   bool Incomplete;
   auto Matches = match(*I, Req, &Incomplete);
-  EXPECT_EQ(Matches.size(), Req.MaxCandidateCount);
+  EXPECT_TRUE(Req.Limit);
+  EXPECT_EQ(Matches.size(), *Req.Limit);
   EXPECT_TRUE(Incomplete);
 }
 
@@ -91,7 +92,7 @@
   RefSlab());
   FuzzyFindRequest Req;
   Req.Query = "lol";
-  Req.MaxCandidateCount = 2;
+  Req.Limit = 2;
   EXPECT_THAT(match(*I, Req),
   UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
 }
Index: clang-tools-extra/unittests/clangd/DexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexTests.cpp
@@ -482,7 +482,7 @@
   URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "lol";
-  Req.MaxCandidateCount = 2;
+  Req.Limit = 2;
   EXPECT_THAT(match(*I, Req),
   UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
 }
@@ -498,17 +498,19 @@
   FuzzyFindRequest Req;
   Req.Query = "2";
   Dex I(Symbols, URISchemes);
+  EXPECT_FALSE(Req.Limit);
   EXPECT_THAT(match(I, Req), ElementsAre("2", "2"));
 }
 
 TEST(DexTest, DexLimitedNumMatches) {
   auto I = Dex::build(generateNumSymbols(0, 100), URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "5";
-  Req.MaxCandidateCount = 3;
+  Req.Limit = 3;
   bool Incomplete;
   auto Matches = match(*I, Req, &Incomplete);
-  EXPECT_EQ(Matches.size(), Req.MaxCandidateCount);
+  EXPECT_TRUE(Req.Limit);
+  EXPECT_EQ(Matches.size(), *Req.Limit);
   EXPECT_TRUE(Incomplete);
 }
 
@@ -518,7 +520,7 @@
   URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "lol";
-  Req.MaxCandidateCount = 2;
+  Req.Limit = 2;
   EXPECT_THAT(match(*I, Req),
   UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
 }
@@ -596,7 +598,7 @@
   FuzzyFindRequest Req;
   Req.Query = "abc";
   // The best candidate can change depending on the proximity paths.
-  Req.MaxCandidateCount = 1;
+  Req.Limit = 1;
 
   // FuzzyFind request comes from the file which is far from the root: expect
   // CloseSymbol to come out.
Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -179,16 +179,17 @@
   // FIXME(kbobyrev): Pre-scoring retrieval threshold should be adjusted as
   // using 100x of the requested number might not be good in practice, e.g.
   // when the requested number of items is small.
-  const size_t ItemsToRetrieve = 100 * Req.MaxCandidateCount;
-  auto Root = createLimit(move(QueryIterator), ItemsToRetrieve);
+  auto Root = Req.Limit ? createLimit(move(QueryIterator), *Req.Limit * 100)
+: move(QueryIterator);
 
   using IDAndScore = std::pair;
   std::vector IDAndScores = consume(*Root);
 
   auto Compare = [](const IDAndScore &LHS, const IDAndScore &RHS) {
 return LHS.second > RHS.second;
   };
-  TopN Top(Req.MaxCandidateCount, Compare);
+  TopN Top(
+  Req.Limit ? *Req.Limit : std::numeric_limits::max(), Compare);
   for (const auto &IDAndScore : IDAndScores) {
 const DocID SymbolDocID = IDAndScore.first;
 const auto *Sym = Symbols[SymbolDocID];
@@ -205,7 +206,7 @@
   More = true;
   }
 
-  // Apply callback to the top Req.MaxCandidateCount items in the descending
+  // Apply callback to the top Req.Limit items in the descending
   // order of cumulative score.
   for (const auto &Item : std::move(Top).items())
 Callback(*Symbols[Item.first]);
Index: clang-tools-extra/clangd/index/MemIndex.cpp
===
--- clang-tools-extra/clangd/index/MemIndex.cpp
+++ clang-tools-extra/clangd/index/MemIndex.cpp
@@ -29,7 +29,8 @@
   assert(!StringRef(Req.Query).contains("::") &&
  "There must be no :: in query.");
 
-  TopN> Top(Req.MaxCandidateCount);
+  TopN> Top(
+  Req.Limit ? 

[PATCH] D52028: [clangd] Cleanup FuzzyFindRequest filtering limit semantics

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/index/dex/Dex.cpp:182
   // when the requested number of items is small.
-  const size_t ItemsToRetrieve = 100 * Req.MaxCandidateCount;
-  auto Root = createLimit(move(QueryIterator), ItemsToRetrieve);

sammccall wrote:
> wow, this was overflowing!
> I guess harmlessly to... numeric_limits::max() - 99 or something? 
> weird.
Oh, yeah, good catch! I thought I had some workaround at some point, but seems 
that it was lost at some point.


https://reviews.llvm.org/D52028



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

[PATCH] D52028: [clangd] Cleanup FuzzyFindRequest filtering limit semantics

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Don't modify benchmarks since it'd clash with the parent patch in this case.


https://reviews.llvm.org/D52028

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/index/Index.cpp
  clang-tools-extra/clangd/index/Index.h
  clang-tools-extra/clangd/index/MemIndex.cpp
  clang-tools-extra/clangd/index/dex/Dex.cpp
  clang-tools-extra/unittests/clangd/DexTests.cpp
  clang-tools-extra/unittests/clangd/IndexTests.cpp

Index: clang-tools-extra/unittests/clangd/IndexTests.cpp
===
--- clang-tools-extra/unittests/clangd/IndexTests.cpp
+++ clang-tools-extra/unittests/clangd/IndexTests.cpp
@@ -78,10 +78,11 @@
   auto I = MemIndex::build(generateNumSymbols(0, 100), RefSlab());
   FuzzyFindRequest Req;
   Req.Query = "5";
-  Req.MaxCandidateCount = 3;
+  Req.Limit = 3;
   bool Incomplete;
   auto Matches = match(*I, Req, &Incomplete);
-  EXPECT_EQ(Matches.size(), Req.MaxCandidateCount);
+  EXPECT_TRUE(Req.Limit);
+  EXPECT_EQ(Matches.size(), *Req.Limit);
   EXPECT_TRUE(Incomplete);
 }
 
@@ -91,7 +92,7 @@
   RefSlab());
   FuzzyFindRequest Req;
   Req.Query = "lol";
-  Req.MaxCandidateCount = 2;
+  Req.Limit = 2;
   EXPECT_THAT(match(*I, Req),
   UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
 }
Index: clang-tools-extra/unittests/clangd/DexTests.cpp
===
--- clang-tools-extra/unittests/clangd/DexTests.cpp
+++ clang-tools-extra/unittests/clangd/DexTests.cpp
@@ -482,7 +482,7 @@
   URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "lol";
-  Req.MaxCandidateCount = 2;
+  Req.Limit = 2;
   EXPECT_THAT(match(*I, Req),
   UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
 }
@@ -498,17 +498,19 @@
   FuzzyFindRequest Req;
   Req.Query = "2";
   Dex I(Symbols, URISchemes);
+  EXPECT_FALSE(Req.Limit);
   EXPECT_THAT(match(I, Req), ElementsAre("2", "2"));
 }
 
 TEST(DexTest, DexLimitedNumMatches) {
   auto I = Dex::build(generateNumSymbols(0, 100), URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "5";
-  Req.MaxCandidateCount = 3;
+  Req.Limit = 3;
   bool Incomplete;
   auto Matches = match(*I, Req, &Incomplete);
-  EXPECT_EQ(Matches.size(), Req.MaxCandidateCount);
+  EXPECT_TRUE(Req.Limit);
+  EXPECT_EQ(Matches.size(), *Req.Limit);
   EXPECT_TRUE(Incomplete);
 }
 
@@ -518,7 +520,7 @@
   URISchemes);
   FuzzyFindRequest Req;
   Req.Query = "lol";
-  Req.MaxCandidateCount = 2;
+  Req.Limit = 2;
   EXPECT_THAT(match(*I, Req),
   UnorderedElementsAre("LaughingOutLoud", "LittleOldLady"));
 }
@@ -596,7 +598,7 @@
   FuzzyFindRequest Req;
   Req.Query = "abc";
   // The best candidate can change depending on the proximity paths.
-  Req.MaxCandidateCount = 1;
+  Req.Limit = 1;
 
   // FuzzyFind request comes from the file which is far from the root: expect
   // CloseSymbol to come out.
Index: clang-tools-extra/clangd/index/dex/Dex.cpp
===
--- clang-tools-extra/clangd/index/dex/Dex.cpp
+++ clang-tools-extra/clangd/index/dex/Dex.cpp
@@ -179,16 +179,17 @@
   // FIXME(kbobyrev): Pre-scoring retrieval threshold should be adjusted as
   // using 100x of the requested number might not be good in practice, e.g.
   // when the requested number of items is small.
-  const size_t ItemsToRetrieve = 100 * Req.MaxCandidateCount;
-  auto Root = createLimit(move(QueryIterator), ItemsToRetrieve);
+  auto Root = Req.Limit ? createLimit(move(QueryIterator), *Req.Limit * 100)
+: move(QueryIterator);
 
   using IDAndScore = std::pair;
   std::vector IDAndScores = consume(*Root);
 
   auto Compare = [](const IDAndScore &LHS, const IDAndScore &RHS) {
 return LHS.second > RHS.second;
   };
-  TopN Top(Req.MaxCandidateCount, Compare);
+  TopN Top(
+  Req.Limit ? *Req.Limit : std::numeric_limits::max(), Compare);
   for (const auto &IDAndScore : IDAndScores) {
 const DocID SymbolDocID = IDAndScore.first;
 const auto *Sym = Symbols[SymbolDocID];
@@ -205,7 +206,7 @@
   More = true;
   }
 
-  // Apply callback to the top Req.MaxCandidateCount items in the descending
+  // Apply callback to the top Req.Limit items in the descending
   // order of cumulative score.
   for (const auto &Item : std::move(Top).items())
 Callback(*Symbols[Item.first]);
Index: clang-tools-extra/clangd/index/MemIndex.cpp
===
--- clang-tools-extra/clangd/index/MemIndex.cpp
+++ clang-tools-extra/clangd/index/MemIndex.cpp
@@ -29,7 +29,8 @@
   assert(!StringRef(Req.Query).contains("::") &&
  "There must be no :: in query.");
 
-  TopN> Top(Req.MaxCandidateCount);
+  TopN> Top(
+  Req.Limit ? *Req.Limit : std::numeric_limits::max());
   FuzzyMatcher Filter(Req.Query);
   boo

[PATCH] D51747: [clangd] Show deprecation diagnostics

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Convert unit tests to lit tests and address comments.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51747

Files:
  clangd/ClangdServer.cpp
  test/clangd/override-no-deprecations.test


Index: test/clangd/override-no-deprecations.test
===
--- /dev/null
+++ test/clangd/override-no-deprecations.test
@@ -0,0 +1,55 @@
+# Test that we return deprecation diagnostics even when -Wno-deprecations is 
set.
+# And always return as warning even if -Werror is set.
+
+# RUN: rm -rf %t.dir/* && mkdir -p %t.dir
+# RUN: mkdir %t.dir/build
+# RUN: mkdir %t.dir/build-1
+# RUN: mkdir %t.dir/build-2
+# RUN: echo '[{"directory": "%/t.dir", "command": "c++ the-file.cpp", "file": 
"the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir/build-1", "command": "c++ -Wno-deprecated 
the-file.cpp", "file": "../the-file.cpp"}]' > 
%t.dir/build-1/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir/build-2", "command": "c++ -Wno-deprecated 
-Werror the-file.cpp", "file": "../the-file.cpp"}]' > 
%t.dir/build-2/compile_commands.json
+
+# RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
+
+# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
+# (with the extra slash in the front), so we add it here.
+# RUN: sed -e "s|file://\([A-Z]\):/|file:///\1:/|g" %t.test.1 > %t.test
+
+# RUN: clangd -lit-test < %t.test | FileCheck -strict-whitespace %t.test
+
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"file://INPUT_DIR/the-file.cpp","languageId":"cpp","version":1,"text":"void
 foo() __attribute__((deprecated));\nint main() { foo(); }"}}}
+# CHECK:   "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:   "params": {
+# CHECK-NEXT: "diagnostics": [
+# CHECK-NEXT:   {
+# CHECK-NEXT: "message": "'foo' is deprecated\n\nthe-file.cpp:1:27: 
note: 'foo' has been explicitly marked deprecated here",
+---
+{"jsonrpc":"2.0","id":0,"method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabasePath":"INPUT_DIR/build-1"}}}
+# CHECK:   "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:   "params": {
+# CHECK-NEXT: "diagnostics": [
+# CHECK-NEXT:   {
+# CHECK-NEXT: "message": "'foo' is deprecated\n\nthe-file.cpp:1:27: 
note: 'foo' has been explicitly marked deprecated here",
+---
+{"jsonrpc":"2.0","id":0,"method":"workspace/didChangeConfiguration","params":{"settings":{"compilationDatabasePath":"INPUT_DIR/build-2"}}}
+# CHECK:   "method": "textDocument/publishDiagnostics",
+# CHECK-NEXT:   "params": {
+# CHECK-NEXT: "diagnostics": [
+# CHECK-NEXT:   {
+# CHECK-NEXT: "message": "'foo' is deprecated\n\nthe-file.cpp:1:27: 
note: 'foo' has been explicitly marked deprecated here",
+# CHECK-NEXT: "range": {
+# CHECK-NEXT:   "end": {
+# CHECK-NEXT: "character": 16,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT:   },
+# CHECK-NEXT:   "start": {
+# CHECK-NEXT: "character": 13,
+# CHECK-NEXT: "line": 1
+# CHECK-NEXT:   }
+# CHECK-NEXT: }
+# CHECK-NEXT: "severity": 2
+---
+{"jsonrpc":"2.0","id":1,"method":"shutdown"}
+
Index: clangd/ClangdServer.cpp
===
--- clangd/ClangdServer.cpp
+++ clangd/ClangdServer.cpp
@@ -531,9 +531,15 @@
   if (!C) // FIXME: Suppress diagnostics? Let the user know?
 C = CDB.getFallbackCommand(File);
 
+  // These flags are working for both gcc and clang-cl driver modes.
   // Inject the resource dir.
   // FIXME: Don't overwrite it if it's already there.
   C->CommandLine.push_back("-resource-dir=" + ResourceDir);
+  // Deprecations are often hidden for full-project build. They're useful in
+  // context.
+  C->CommandLine.push_back("-Wdeprecated");
+  // Adding -Wdeprecated would trigger errors in projects what set -Werror.
+  C->CommandLine.push_back("-Wno-error=deprecated");
   return std::move(*C);
 }
 


Index: test/clangd/override-no-deprecations.test
===
--- /dev/null
+++ test/clangd/override-no-deprecations.test
@@ -0,0 +1,55 @@
+# Test that we return deprecation diagnostics even when -Wno-deprecations is set.
+# And always return as warning even if -Werror is set.
+
+# RUN: rm -rf %t.dir/* && mkdir -p %t.dir
+# RUN: mkdir %t.dir/build
+# RUN: mkdir %t.dir/build-1
+# RUN: mkdir %t.dir/build-2
+# RUN: echo '[{"directory": "%/t.dir", "command": "c++ the-file.cpp", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+# RUN: echo '[{"directory": "%/t.dir/build-1", "command": "c++ -Wno-deprecated the-file.cpp", "file": "../the-file.cpp"}]' > %t.dir/build-1/compile_comm

[PATCH] D51747: [clangd] Show deprecation diagnostics

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

When doing some manual testing saw that it causes a lot of slow down on vim, 
even when I have just about 200 diagnostics, will wait till a few others check 
that out. We might wanna limit the number of diagnostics and keep only the ones 
that are closest to current cursor position.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51747



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

[PATCH] D52030: [clangd] Test benchmark if we can build it

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Looks good, thanks!

While we're here: I'm wondering whether we should also introduce very basic 
test which would just run `clangd-indexer` since it doesn't depend on 
benchmarks and would be run on all buildbots.

Also, I was thinking whether we should ask some buildbot owners to enable 
`LLVM_BUILD_BENCHMARKS`, but it probably makes sense when we have more 
benchmarks (ideally, across different LLVM parts).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52030



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

[PATCH] D51725: Allow un-setting the compilation database path

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added a comment.

In https://reviews.llvm.org/D51725#1233254, @simark wrote:

> I am not sure I understand correctly your last point.  Of course, when 
> restarting clangd, we need to send again a `didOpen` for each file the user 
> has currently open in the editor.  But that should be  it?


Yes, this and any initial configuration settings.
My personal experience is from VSCode, which does not call `addDocument` IIUC. 
This results in clangd producing errors about running actions in non-opened 
files.

>> If you'll decide to go with an option to reset the path, see the comment 
>> about making empty path special. `Optional>` does not play nicely 
>> with json serialization code and hard to read (even though it does look like 
>> the right thing to do from the type system perspective).
> 
> Yes, noted.  I preferred to avoid giving a special meaning to an empty string 
> because we could avoid it.  But I wouldn't mind changing it if we go that 
> route.

Also not a big fan of it, but `Optional>` looks terrible. Maybe I'm 
missing an obvious pattern to make it simpler here, though.
WDYT about deserializing `null` to empty string? It would mean that specifying 
both `null` and `""` in json configuration yields the same results, but JSON 
that we send would look a bit nicer.




Comment at: clangd/Protocol.h:368
+  /// both Optionals are instantiated.
+  llvm::Optional> compilationDatabasePath;
 

kadircet wrote:
> ilya-biryukov wrote:
> > Not a big fan or something like this, but maybe give special meaning to 
> > empty path instead of wrapping an optional into an optional?
> > 
> > Double optionals are a real pain to write and read.
> Sorry just passing by since I've seen a similar usage that already exists in 
> codebase, wanted to point it out.
> 
> Some api like this could be easier, to make the decision of using empty path 
> or anything else for an invalid path and making it an implementation detail.
> 
> https://github.com/llvm-mirror/clang/blob/master/include/clang/Sema/DeclSpec.h#L53
It makes it less straight-forward, though.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D51725



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

[PATCH] D51971: [clangd] Use JSON format in benchmark requests reader

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 165276.
kbobyrev marked 2 inline comments as done.

https://reviews.llvm.org/D51971

Files:
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/test/clangd/Inputs/requests.json
  clang-tools-extra/test/clangd/Inputs/requests.log
  clang-tools-extra/test/clangd/index-tools.test

Index: clang-tools-extra/test/clangd/index-tools.test
===
--- clang-tools-extra/test/clangd/index-tools.test
+++ clang-tools-extra/test/clangd/index-tools.test
@@ -1,3 +1,5 @@
 # RUN: clangd-indexer %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs > %t.index
 # FIXME: By default, benchmarks are excluded from the list of default targets hence not built. Find a way to depend on benchmarks to run the next command.
-# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.log --benchmark_min_time=0.01 ; fi
+# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.json --benchmark_min_time=0.01 ; fi
+# Pass invalid JSON file and check that benchmarks fails to parse it.
+# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then not %clangd-benchmark-dir/IndexBenchmark %t.index %t --benchmark_min_time=0.01 ; fi
Index: clang-tools-extra/test/clangd/Inputs/requests.log
===
--- clang-tools-extra/test/clangd/Inputs/requests.log
+++ /dev/null
@@ -1,5 +0,0 @@
-V[09:08:34.301] Code complete: fuzzyFind("s", scopes=[llvm::])
-V[09:08:34.368] Code complete: fuzzyFind("sy", scopes=[llvm::])
-V[09:08:34.442] Code complete: fuzzyFind("sys", scopes=[llvm::])
-V[09:09:12.075] Code complete: fuzzyFind("Dex", scopes=[clang::clangd::, clang::, clang::clangd::dex::])
-V[09:09:12.075] Code complete: fuzzyFind("Variable", scopes=[])
Index: clang-tools-extra/test/clangd/Inputs/requests.json
===
--- /dev/null
+++ clang-tools-extra/test/clangd/Inputs/requests.json
@@ -0,0 +1,7 @@
+[{"MaxCandidateCount":100,"ProximityPaths":["/usr/home/user/clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp"],"Query":"OMP","RestrictForCodeCompletion":true,"Scopes":["clang::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"s","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sy","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Dex","RestrictForCodeCompletion":true,"Scopes":["clang::clangd::", "clang::", "clang::clangd::dex::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Variable","RestrictForCodeCompletion":true,"Scopes":[""]}]
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -19,56 +19,50 @@
 #include 
 
 const char *IndexFilename;
-const char *LogFilename;
+const char *RequestsFilename;
 
 namespace clang {
 namespace clangd {
 namespace {
 
-std::unique_ptr buildMem() {
-  return clang::clangd::loadIndex(IndexFilename, {}, false);
+std::unique_ptr buildMem() {
+  return loadIndex(IndexFilename, {}, false);
 }
 
-std::unique_ptr buildDex() {
-  return clang::clangd::loadIndex(IndexFilename, {}, true);
+std::unique_ptr buildDex() {
+  return loadIndex(IndexFilename, {}, true);
 }
 
-// This function processes user-provided Log file with fuzzy find requests in
-// the following format:
-//
-// fuzzyFind("UnqualifiedName", scopes=["clang::", "clang::clangd::"])
-//
-// It constructs vector of FuzzyFindRequests which is later used for the
-// benchmarks.
-std::vector extractQueriesFromLogs() {
-  llvm::Regex RequestMatcher("fuzzyFind\\(\"([a-zA-Z]*)\", scopes=\\[(.*)\\]");
-  llvm::SmallVector Matches;
-  std::ifstream InputStream(LogFilename);
+// Reads JSON array of serialized FuzzyFindRequest's from user-provided file.
+std::vector extractQueriesFromLogs() {
+  std::ifstream InputStream(RequestsFilename);
   std::string Log((std::istreambuf_iterator(InputStream)),
   std::istreambuf_iterator());
-  llvm::StringRef Temporary(Log);
-  llvm::SmallVector Strings;
-  Temporary.split(Strings, '\n');
 
-  clang::clangd::FuzzyFindRequest R;
-  R.MaxCandidateCount = 100;
+  std::vector Requests;
+  auto JSONArray = llvm::json::parse(Log);
 
-  llvm::SmallVector CommaSeparatedValues;
+  // Panic if the provided file couldn't be parsed.
+  if (!JSONArray) {
+llvm::errs() << "Error when parsing JSON reques

[PATCH] D51971: [clangd] Use JSON format in benchmark requests reader

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Fix a typo in test comments.


https://reviews.llvm.org/D51971

Files:
  clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
  clang-tools-extra/test/clangd/Inputs/requests.json
  clang-tools-extra/test/clangd/Inputs/requests.log
  clang-tools-extra/test/clangd/index-tools.test

Index: clang-tools-extra/test/clangd/index-tools.test
===
--- clang-tools-extra/test/clangd/index-tools.test
+++ clang-tools-extra/test/clangd/index-tools.test
@@ -1,3 +1,5 @@
 # RUN: clangd-indexer %p/Inputs/BenchmarkSource.cpp -- -I%p/Inputs > %t.index
 # FIXME: By default, benchmarks are excluded from the list of default targets hence not built. Find a way to depend on benchmarks to run the next command.
-# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.log --benchmark_min_time=0.01 ; fi
+# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then %clangd-benchmark-dir/IndexBenchmark %t.index %p/Inputs/requests.json --benchmark_min_time=0.01 ; fi
+# Pass invalid JSON file and check that IndexBenchmark fails to parse it.
+# RUN: if [ -f %clangd-benchmark-dir/IndexBenchmark ]; then not %clangd-benchmark-dir/IndexBenchmark %t.index %t --benchmark_min_time=0.01 ; fi
Index: clang-tools-extra/test/clangd/Inputs/requests.log
===
--- clang-tools-extra/test/clangd/Inputs/requests.log
+++ /dev/null
@@ -1,5 +0,0 @@
-V[09:08:34.301] Code complete: fuzzyFind("s", scopes=[llvm::])
-V[09:08:34.368] Code complete: fuzzyFind("sy", scopes=[llvm::])
-V[09:08:34.442] Code complete: fuzzyFind("sys", scopes=[llvm::])
-V[09:09:12.075] Code complete: fuzzyFind("Dex", scopes=[clang::clangd::, clang::, clang::clangd::dex::])
-V[09:09:12.075] Code complete: fuzzyFind("Variable", scopes=[])
Index: clang-tools-extra/test/clangd/Inputs/requests.json
===
--- /dev/null
+++ clang-tools-extra/test/clangd/Inputs/requests.json
@@ -0,0 +1,7 @@
+[{"MaxCandidateCount":100,"ProximityPaths":["/usr/home/user/clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp"],"Query":"OMP","RestrictForCodeCompletion":true,"Scopes":["clang::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"s","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sy","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"sys","RestrictForCodeCompletion":true,"Scopes":["llvm::", ""]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Dex","RestrictForCodeCompletion":true,"Scopes":["clang::clangd::", "clang::", "clang::clangd::dex::"]},
+{"MaxCandidateCount":100,"ProximityPaths":[],"Query":"Variable","RestrictForCodeCompletion":true,"Scopes":[""]}]
Index: clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
===
--- clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
+++ clang-tools-extra/clangd/benchmarks/IndexBenchmark.cpp
@@ -19,56 +19,50 @@
 #include 
 
 const char *IndexFilename;
-const char *LogFilename;
+const char *RequestsFilename;
 
 namespace clang {
 namespace clangd {
 namespace {
 
-std::unique_ptr buildMem() {
-  return clang::clangd::loadIndex(IndexFilename, {}, false);
+std::unique_ptr buildMem() {
+  return loadIndex(IndexFilename, {}, false);
 }
 
-std::unique_ptr buildDex() {
-  return clang::clangd::loadIndex(IndexFilename, {}, true);
+std::unique_ptr buildDex() {
+  return loadIndex(IndexFilename, {}, true);
 }
 
-// This function processes user-provided Log file with fuzzy find requests in
-// the following format:
-//
-// fuzzyFind("UnqualifiedName", scopes=["clang::", "clang::clangd::"])
-//
-// It constructs vector of FuzzyFindRequests which is later used for the
-// benchmarks.
-std::vector extractQueriesFromLogs() {
-  llvm::Regex RequestMatcher("fuzzyFind\\(\"([a-zA-Z]*)\", scopes=\\[(.*)\\]");
-  llvm::SmallVector Matches;
-  std::ifstream InputStream(LogFilename);
+// Reads JSON array of serialized FuzzyFindRequest's from user-provided file.
+std::vector extractQueriesFromLogs() {
+  std::ifstream InputStream(RequestsFilename);
   std::string Log((std::istreambuf_iterator(InputStream)),
   std::istreambuf_iterator());
-  llvm::StringRef Temporary(Log);
-  llvm::SmallVector Strings;
-  Temporary.split(Strings, '\n');
 
-  clang::clangd::FuzzyFindRequest R;
-  R.MaxCandidateCount = 100;
+  std::vector Requests;
+  auto JSONArray = llvm::json::parse(Log);
 
-  llvm::SmallVector CommaSeparatedValues;
+  // Panic if the provided file couldn't be parsed.
+  if (!JSONArray) {
+llvm::errs() << "Error when p

[PATCH] D52030: [clangd] Test benchmark if we can build it

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In https://reviews.llvm.org/D52030#1233323, @kbobyrev wrote:

> Looks good, thanks!
>
> While we're here: I'm wondering whether we should also introduce very basic 
> test which would just run `clangd-indexer` since it doesn't depend on 
> benchmarks and would be run on all buildbots.


Isn't that this test?

> Also, I was thinking whether we should ask some buildbot owners to enable 
> `LLVM_BUILD_BENCHMARKS`, but it probably makes sense when we have more 
> benchmarks (ideally, across different LLVM parts).

Hmm, I thought that was already the case. Isn't the benchmark stuff only 
not-built on the windows bots?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D52030



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