[PATCH] D91721: [clangd] textDocument/implementation (LSP layer)

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

just nits.

Looks like the patch is based on the old revision (pre-merging tests are 
failing), I assume you have fixed the failure tests last week?




Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:615
 {"workspaceSymbolProvider", true},
+{"implementationProvider", true},
 {"referencesProvider", true},

nit: move it to line 605 (right after `definitionProvider`).



Comment at: clang-tools-extra/clangd/ClangdLSPServer.cpp:1443
   MsgHandler->bind("textDocument/references", ::onReference);
+  MsgHandler->bind("textDocument/implementation", 
::onImplementation);
   MsgHandler->bind("textDocument/switchSourceHeader", 
::onSwitchSourceHeader);

nit: `onGoToImplementation`, move a line up to be closer with 
Go-to-{declaration/definition}.



Comment at: clang-tools-extra/clangd/Protocol.h:1482
 
+struct ImplementationParams : public TextDocumentPositionParams {};
+bool fromJSON(const llvm::json::Value &, ImplementationParams &,

nit: since this is just a mirror of `TextDocumentPositionParams`, I'd use 
`TextDocumentPositionParams` directly (looks like this is a pattern of other 
features, go-to-definition etc.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91721

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

[PATCH] D91055: [clang-tidy] Introduce misc No Integer To Pointer Cast check

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

Ping.
If there's no pushback within next 24h i will commit this and wait for 
post-commit review (if any).
Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91055

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban added inline comments.



Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:134
 
 public:
   static cl::opt VerifyPreservedCFG;

kuhar wrote:
> yrouban wrote:
> > kuhar wrote:
> > > not necessary anymore
> > there can bee a need to disabled/enable (e.g. for some tests or for 
> > debugging).
> I meant the 'public:'. You made everything public at the very top of the 
> class.
sure


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

https://reviews.llvm.org/D91327

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Jakub Kuderski via Phabricator via cfe-commits]

kuhar added inline comments.



Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:134
 
 public:
   static cl::opt VerifyPreservedCFG;

yrouban wrote:
> kuhar wrote:
> > not necessary anymore
> there can bee a need to disabled/enable (e.g. for some tests or for 
> debugging).
I meant the 'public:'. You made everything public at the very top of the class.


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

https://reviews.llvm.org/D91327

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban added a comment.

In D91327#2408521 , @kuhar wrote:

> Looks fine to me, but I'm not confident enough to give an approval.

Thanks. I'm working closely with @skatkov and believe he have enough expertise 
to review fully.
The biggest issue with the current design is that we have to send FAM to the 
initialization of StandardInstrumentations/PreservedCFGCheckerInstrumentation. 
This looks asymmetric to the other analysis managers and it would be more 
reasonable to send FAM or all AMs to the instrumentation callbacks as 
parameters.


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

https://reviews.llvm.org/D91327

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

[PATCH] D91327: [NewPM] Redesign of PreserveCFG Checker

[Yevgeny Rouban via Phabricator via cfe-commits]

yrouban added inline comments.



Comment at: llvm/include/llvm/Passes/StandardInstrumentations.h:134
 
 public:
   static cl::opt VerifyPreservedCFG;

kuhar wrote:
> not necessary anymore
there can bee a need to disabled/enable (e.g. for some tests or for debugging).



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:759
+CFG::printDiff(dbgs(), GraphBefore, GraphAfter);
+report_fatal_error(Twine("CFG unexpectedly changed by ", Pass));
+  };

kuhar wrote:
> I think this will print with `errs()`. Would it make sense to flush `dbgs()` 
> ahead of printing with `errs()`?
I believe this comment is true for all callsites of  //report_fatal_error()//. 
If so it should be done inside.



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:771
+*const_cast(F)))
+  checkCFG(P, F->getName(), *Result, CFG(F, false /* TrackBBLifetime */));
 else

skatkov wrote:
> why do you check before pass runs?
removed



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:778
   [this](StringRef P, const PreservedAnalyses ) {
-auto Before = GraphStackBefore.pop_back_val();
-assert(Before.first == P &&
+assert(PassStack.pop_back_val() == P &&
"Before and After callbacks must correspond");

skatkov wrote:
> you should not PassStack.pop_back_val() for product build?
PassStack is used only inside asserts



Comment at: llvm/lib/Passes/StandardInstrumentations.cpp:796
+auto *F = any_cast(IR);
+if (auto *GraphBefore = FAM.getCachedResult(
+*const_cast(F)))

to @skatkov
if //AfterPassCallback// is called before //AM.invalidate()// then we check CFG 
only if the condition (PassPA.allAnalysesInSetPreserved() || 
PassPA.allAnalysesInSetPreserved>()) holds.
if //AfterPassCallback// is called after //AM.invalidate()// then we check CFG 
only if the same condition holds and the PreservedCFGCheckerAnalysis is not 
invalidated. But it is invalidated according to the same condition.
All in all it does not matter if //AfterPassCallback// is called before or 
after //AM.invalidate()//.


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

https://reviews.llvm.org/D91327

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

[PATCH] D91844: [llvm][clang] Add checks for the smart pointers with the possibility to be null

[Ella Ma via Phabricator via cfe-commits]

OikawaKirie added a comment.

In D91844#2408897 , @dexonsmith wrote:

> Is it possible to split these up into separate patches for unrelated code?

Since these are reported by one static scan, and these reported places cannot 
be categorized with others, I choose to submit them in one patch for simplicity 
and avoiding spam. If it is necessary to separate them one by one, I will close 
this review and start a new one for each of them.

Or, maybe you are thinking of just separating the patch of clang with llvm? If 
so, I will start a new review just for the patch of clang and leave the patches 
of llvm here.




Comment at: clang/utils/TableGen/ClangAttrEmitter.cpp:1346-1353
   if (!Ptr) {
 // Search in reverse order so that the most-derived type is handled first.
 ArrayRef> Bases = Search->getSuperClasses();
 for (const auto  : llvm::reverse(Bases)) {
   if ((Ptr = createArgument(Arg, Attr, Base.first)))
 break;
 }

dexonsmith wrote:
> Can we just add a single assertion here? It looks to me like every caller 
> wants a valid return.
Ok, I will add an assertion here (below line 1353) in the new submits, and 
remove all other assertions I added in this file together with the checks on 
this pointer after the assertion (line 1355 and 1358).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91844

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

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

[Pengfei Wang via Phabricator via cfe-commits]

pengfei added a comment.

In D80344#2407305 , @tentzen wrote:

> In D80344#2407250 , @pengfei wrote:
>
>> Do we need to consider FP exceptions in _try block?
>
> Yes, FP exception is handled as long as FP exceptions are not disabled (Ex 
> via _controlfp() runtime) and FP exception code is filtered & handled via 
> ___except() statement (Ex, 
> ___except(GetExceptionCode()==EXCEPTION_FLT_INEXACT_RESULT)).

I see. If this is the case, you may need to assign FPE_Strict to _try block to 
preserve FP instructions' order.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80344

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

[PATCH] D91941: [clangd] Use WorkScheduler.run() in ClangdServer::resolveTypeHierarchy()

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

As discussed in https://reviews.llvm.org/D91123#inline-856770


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91941

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

[PATCH] D91941: [clangd] Use WorkScheduler.run() in ClangdServer::resolveTypeHierarchy()

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
nridge added a reviewer: kadircet.
Herald added subscribers: cfe-commits, usaxena95, arphaman.
Herald added a project: clang.
nridge requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91941

Files:
  clang-tools-extra/clangd/ClangdServer.cpp


Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -673,8 +673,11 @@
 void ClangdServer::resolveTypeHierarchy(
 TypeHierarchyItem Item, int Resolve, TypeHierarchyDirection Direction,
 Callback> CB) {
-  clangd::resolveTypeHierarchy(Item, Resolve, Direction, Index);
-  CB(Item);
+  WorkScheduler.run(
+  "Resolve Type Hierarchy", "", [=, CB = std::move(CB)]() mutable {
+clangd::resolveTypeHierarchy(Item, Resolve, Direction, Index);
+CB(Item);
+  });
 }
 
 void ClangdServer::prepareCallHierarchy(


Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -673,8 +673,11 @@
 void ClangdServer::resolveTypeHierarchy(
 TypeHierarchyItem Item, int Resolve, TypeHierarchyDirection Direction,
 Callback> CB) {
-  clangd::resolveTypeHierarchy(Item, Resolve, Direction, Index);
-  CB(Item);
+  WorkScheduler.run(
+  "Resolve Type Hierarchy", "", [=, CB = std::move(CB)]() mutable {
+clangd::resolveTypeHierarchy(Item, Resolve, Direction, Index);
+CB(Item);
+  });
 }
 
 void ClangdServer::prepareCallHierarchy(
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91124: [clangd] Call hierarchy (ClangdLSPServer layer)

[Nathan Ridge via Phabricator via cfe-commits]

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

Update as per API changes in xrefs patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91124

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/test/call-hierarchy.test
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp

Index: clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
===
--- clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
+++ clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp
@@ -37,6 +37,8 @@
   LSPTest() : LogSession(*this) {
 ClangdServer::Options  = Opts;
 Base = ClangdServer::optsForTest();
+// This is needed to we can test index-based operations like call hierarchy.
+Base.BuildDynamicSymbolIndex = true;
   }
 
   LSPClient () {
@@ -165,6 +167,33 @@
   stop();
   EXPECT_THAT(Tracer.takeMetric("lsp_latency", MethodName), testing::SizeIs(1));
 }
+
+TEST_F(LSPTest, IncomingCalls) {
+  Annotations Code(R"cpp(
+void calle^e(int);
+void caller1() {
+  [[callee]](42);
+}
+  )cpp");
+  auto  = start();
+  Client.didOpen("foo.cpp", Code.code());
+  auto Items = Client
+   .call("textDocument/prepareCallHierarchy",
+ llvm::json::Object{
+ {"textDocument", Client.documentID("foo.cpp")},
+ {"position", Code.point()}})
+   .takeValue();
+  auto FirstItem = (*Items.getAsArray())[0];
+  auto Calls = Client
+   .call("callHierarchy/incomingCalls",
+ llvm::json::Object{{"item", FirstItem}})
+   .takeValue();
+  auto FirstCall = *(*Calls.getAsArray())[0].getAsObject();
+  EXPECT_EQ(FirstCall["fromRanges"], llvm::json::Value{Code.range()});
+  auto From = *FirstCall["from"].getAsObject();
+  EXPECT_EQ(From["name"], "caller1");
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -5,6 +5,7 @@
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": {
 # CHECK-NEXT:"capabilities": {
+# CHECK-NEXT:  "callHierarchyProvider": true,
 # CHECK-NEXT:  "codeActionProvider": true,
 # CHECK-NEXT:  "completionProvider": {
 # CHECK-NEXT:"allCommitCharacters": [
Index: clang-tools-extra/clangd/test/call-hierarchy.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/call-hierarchy.test
@@ -0,0 +1,39 @@
+# RUN: clangd -lit-test < %s | FileCheck -strict-whitespace %s
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{},"trace":"off"}}
+---
+{"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"languageId":"cpp","text":"void callee(int);\nvoid caller1() {\n  callee(42);\n}\nvoid caller2() {\n  caller1();\n  caller1();\n}\nvoid caller3() {\n  caller1();\n  caller2();\n}\n","uri":"test:///main.cpp","version":1}}}
+---
+{"jsonrpc":"2.0","id":1,"method":"textDocument/prepareCallHierarchy","params":{"position":{"character":8,"line":0},"textDocument":{"uri":"test:///main.cpp"}}}
+#  CHECK:  "id": 1,
+# CHECK-NEXT:  "jsonrpc": "2.0",
+# CHECK-NEXT:  "result": [
+# CHECK-NEXT:{
+# CHECK-NEXT:  "data": "{{.*}}",
+# CHECK-NEXT:  "kind": 12,
+# CHECK-NEXT:  "name": "callee",
+# CHECK-NEXT:  "range": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 16,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 0,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "selectionRange": {
+# CHECK-NEXT:"end": {
+# CHECK-NEXT:  "character": 11,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:},
+# CHECK-NEXT:"start": {
+# CHECK-NEXT:  "character": 5,
+# CHECK-NEXT:  "line": 0
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "uri": "file://{{.*}}/clangd-test/main.cpp"
+# CHECK-NEXT:}
+---
+{"jsonrpc":"2.0","id":3,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
Index: clang-tools-extra/clangd/ClangdLSPServer.h
===
--- clang-tools-extra/clangd/ClangdLSPServer.h
+++ clang-tools-extra/clangd/ClangdLSPServer.h
@@ -132,6 +132,14 @@
Callback>);
   void onResolveTypeHierarchy(const ResolveTypeHierarchyItemParams &,

[PATCH] D88220: [C++20] P1825R0: More implicit moves

[Yang Fan via Phabricator via cfe-commits]

nullptr.cpp added a comment.

Ping


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88220

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

[PATCH] D91123: [clangd] Call hierarchy (ClangdServer layer)

[Nathan Ridge via Phabricator via cfe-commits]

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

Update as per API changes in xrefs patch


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91123

Files:
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h


Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -242,6 +242,14 @@
 TypeHierarchyDirection Direction,
 Callback> CB);
 
+  /// Get information about call hierarchy for a given position.
+  void prepareCallHierarchy(PathRef File, Position Pos,
+Callback> CB);
+
+  /// Resolve incoming calls for a given call hierarchy item.
+  void incomingCalls(const CallHierarchyItem ,
+ Callback>);
+
   /// Retrieve the top symbols from the workspace matching a query.
   void workspaceSymbols(StringRef Query, int Limit,
 Callback> CB);
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -677,6 +677,26 @@
   CB(Item);
 }
 
+void ClangdServer::prepareCallHierarchy(
+PathRef File, Position Pos, Callback> CB) {
+  auto Action = [File = File.str(), Pos,
+ CB = std::move(CB)](Expected InpAST) mutable {
+if (!InpAST)
+  return CB(InpAST.takeError());
+CB(clangd::prepareCallHierarchy(InpAST->AST, Pos, File));
+  };
+  WorkScheduler.runWithAST("Call Hierarchy", File, std::move(Action));
+}
+
+void ClangdServer::incomingCalls(
+const CallHierarchyItem ,
+Callback> CB) {
+  WorkScheduler.run("Incoming Calls", "",
+[CB = std::move(CB), Item, this]() mutable {
+  CB(clangd::incomingCalls(Item, Index));
+});
+}
+
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams ) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.


Index: clang-tools-extra/clangd/ClangdServer.h
===
--- clang-tools-extra/clangd/ClangdServer.h
+++ clang-tools-extra/clangd/ClangdServer.h
@@ -242,6 +242,14 @@
 TypeHierarchyDirection Direction,
 Callback> CB);
 
+  /// Get information about call hierarchy for a given position.
+  void prepareCallHierarchy(PathRef File, Position Pos,
+Callback> CB);
+
+  /// Resolve incoming calls for a given call hierarchy item.
+  void incomingCalls(const CallHierarchyItem ,
+ Callback>);
+
   /// Retrieve the top symbols from the workspace matching a query.
   void workspaceSymbols(StringRef Query, int Limit,
 Callback> CB);
Index: clang-tools-extra/clangd/ClangdServer.cpp
===
--- clang-tools-extra/clangd/ClangdServer.cpp
+++ clang-tools-extra/clangd/ClangdServer.cpp
@@ -677,6 +677,26 @@
   CB(Item);
 }
 
+void ClangdServer::prepareCallHierarchy(
+PathRef File, Position Pos, Callback> CB) {
+  auto Action = [File = File.str(), Pos,
+ CB = std::move(CB)](Expected InpAST) mutable {
+if (!InpAST)
+  return CB(InpAST.takeError());
+CB(clangd::prepareCallHierarchy(InpAST->AST, Pos, File));
+  };
+  WorkScheduler.runWithAST("Call Hierarchy", File, std::move(Action));
+}
+
+void ClangdServer::incomingCalls(
+const CallHierarchyItem ,
+Callback> CB) {
+  WorkScheduler.run("Incoming Calls", "",
+[CB = std::move(CB), Item, this]() mutable {
+  CB(clangd::incomingCalls(Item, Index));
+});
+}
+
 void ClangdServer::onFileEvent(const DidChangeWatchedFilesParams ) {
   // FIXME: Do nothing for now. This will be used for indexing and potentially
   // invalidating other caches.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D91122: [clangd] Call hierarchy (XRefs layer, incoming calls)

[Nathan Ridge via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/XRefs.cpp:1649
+  // into the same CallHierarchyIncomingCall.
+  llvm::DenseMap ResultMap;
+  // We can populate the keys of ResultMap, and the FromRanges fields of

kadircet wrote:
> nit: `s/ResultMap/CallsIn` ?
> 
> I would also suggest changing value type to `SmallVector` that way 
> rather than having a "partially filled" IncomingCall objects in the middle, 
> you can create valid ones directly within the lookup.
I went with `std::vector` as we eventually need that for the final 
result, and we can move it from one place to another.



Comment at: clang-tools-extra/clangd/XRefs.h:109
+/// Get call hierarchy information at \p Pos.
+llvm::Optional>
+prepareCallHierarchy(ParsedAST , Position Pos, const SymbolIndex *Index,

kadircet wrote:
> nridge wrote:
> > kadircet wrote:
> > > nridge wrote:
> > > > kadircet wrote:
> > > > > what's the distinction between empty and none return values ? (same 
> > > > > for others)
> > > > Generally speaking, a `None` result means "the input was not recognized 
> > > > in some way", while an empty vector result means "there are no results 
> > > > for this input".
> > > > 
> > > > For `prepareCallHierarchy`, the inputs encode a source location, and 
> > > > the operation asks "give me `CallHierarchyItem`s for suitable 
> > > > declarations (i.e. functions) at this location. So, `None` means "the 
> > > > source location could not be recognized" (`sourceLocationInMainFile` 
> > > > failed), while an empty result means "there are no declarations of 
> > > > functions at this location".
> > > > 
> > > > For `incomingCalls` and `outgoingCalls`, the inputs encode a 
> > > > declaration of a function, and the operation asks "give me its callers 
> > > > / callees". So, a `None` means "could not locate the function (i.e. 
> > > > symbol)", while an empty result means "this function has no callers / 
> > > > callees".
> > > > Generally speaking, a None result means "the input was not recognized 
> > > > in some way", while an empty vector result means "there are no results 
> > > > for this input".
> > > 
> > > Makes sense, but that sounds like an implementation detail. I don't see 
> > > any difference between the two from caller's point of view. Even the 
> > > logging happens inside the implementation. As for interpreting llvm::None 
> > > by the caller, i believe it is subject to change, so unless we return an 
> > > Expected, there's not much value in returning an Optional, and even in 
> > > such a case, caller probably can't do much but propagate the error (maybe 
> > > also try with Pos+/-1, but usually that's handled within the XRefs as 
> > > well).
> > > 
> > > Returning an empty container is also coherent with rest of the APIs we 
> > > have in XRefs.h.
> > > 
> > > So I believe we should drop the optionals here.
> > The protocol 
> > [specifies](https://microsoft.github.io/language-server-protocol/specifications/specification-3-16/#textDocument_prepareCallHierarchy)
> >  return types of `CallHierarchyItem[] | null` for `prepareCallHierarchy` 
> > and `CallHierarchyIncomingCall[] | null` for `incomingCalls`.
> > 
> > The `| null` in there suggests that the protocol intends for there to be a 
> > semantic difference  between an empty array and null, and that clients may 
> > want to do things differently in the two caes (e.g. show an "unable to 
> > compute call hierarchy" error dialog vs. show an emty tree).
> yes, that's indeed a possibility, and the same goes for other features like 
> textDocument/{definition,declaration,references}. AFAICT, we don't signal the 
> difference between "no results" and "something went wrong" for those, and 
> haven't received any complaints yet. So I'd rather keep them coherent, and 
> replace all of them if we see that there are clients taking different actions 
> for real.
Ok, removed the Optional for consistency with other requests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91122

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

[PATCH] D91122: [clangd] Call hierarchy (XRefs layer, incoming calls)

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 306951.
nridge marked 14 inline comments as done.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91122

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
  clang-tools-extra/clangd/unittests/TestTU.cpp

Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -156,7 +156,8 @@
 
 std::unique_ptr TestTU::index() const {
   auto AST = build();
-  auto Idx = std::make_unique(/*UseDex=*/true);
+  auto Idx = std::make_unique(/*UseDex=*/true,
+ /*CollectMainFileRefs=*/true);
   Idx->updatePreamble(testPath(Filename), /*Version=*/"null",
   AST.getASTContext(), AST.getPreprocessorPtr(),
   AST.getCanonicalIncludes());
Index: clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/CallHierarchyTests.cpp
@@ -0,0 +1,256 @@
+//===-- CallHierarchyTests.cpp  ---*- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "Annotations.h"
+#include "Compiler.h"
+#include "Matchers.h"
+#include "ParsedAST.h"
+#include "SyncAPI.h"
+#include "TestFS.h"
+#include "TestTU.h"
+#include "TestWorkspace.h"
+#include "XRefs.h"
+#include "index/FileIndex.h"
+#include "index/SymbolCollector.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
+#include "clang/Index/IndexingAction.h"
+#include "llvm/Support/Path.h"
+#include "llvm/Support/ScopedPrinter.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+namespace clang {
+namespace clangd {
+namespace {
+
+using ::testing::AllOf;
+using ::testing::ElementsAre;
+using ::testing::Field;
+using ::testing::Matcher;
+using ::testing::UnorderedElementsAre;
+
+// Helpers for matching call hierarchy data structures.
+MATCHER_P(WithName, N, "") { return arg.name == N; }
+MATCHER_P(WithSelectionRange, R, "") { return arg.selectionRange == R; }
+
+template 
+::testing::Matcher From(ItemMatcher M) {
+  return Field(::from, M);
+}
+template 
+::testing::Matcher FromRanges(RangeMatchers... M) {
+  return Field(::fromRanges,
+   UnorderedElementsAre(M...));
+}
+
+TEST(CallHierarchy, IncomingOneFile) {
+  Annotations Source(R"cpp(
+void call^ee(int);
+void caller1() {
+  $Callee[[callee]](42);
+}
+void caller2() {
+  $Caller1A[[caller1]]();
+  $Caller1B[[caller1]]();
+}
+void caller3() {
+  $Caller1C[[caller1]]();
+  $Caller2[[caller2]]();
+}
+  )cpp");
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+  auto Index = TU.index();
+
+  std::vector Items =
+  prepareCallHierarchy(AST, Source.point(), testPath(TU.Filename));
+  EXPECT_THAT(Items, ElementsAre(WithName("callee")));
+  auto IncomingLevel1 = incomingCalls(Items[0], Index.get());
+  EXPECT_THAT(IncomingLevel1,
+  ElementsAre(AllOf(From(WithName("caller1")),
+FromRanges(Source.range("Callee");
+
+  auto IncomingLevel2 = incomingCalls(IncomingLevel1[0].from, Index.get());
+  EXPECT_THAT(IncomingLevel2, UnorderedElementsAre(
+  AllOf(From(WithName("caller2")),
+FromRanges(Source.range("Caller1A"),
+   Source.range("Caller1B"))),
+  AllOf(From(WithName("caller3")),
+FromRanges(Source.range("Caller1C");
+
+  auto IncomingLevel3 = incomingCalls(IncomingLevel2[0].from, Index.get());
+  EXPECT_THAT(IncomingLevel3,
+  ElementsAre(AllOf(From(WithName("caller3")),
+FromRanges(Source.range("Caller2");
+
+  auto IncomingLevel4 = incomingCalls(IncomingLevel3[0].from, Index.get());
+  EXPECT_THAT(IncomingLevel4, ElementsAre());
+}
+
+TEST(CallHierarchy, MainFileOnlyRef) {
+  // In addition to testing that we store refs to main-file only symbols,
+  // this tests that anonymous namespaces do not interfere with the
+  // symbol re-identification process in callHierarchyItemToSymbo().
+  Annotations Source(R"cpp(
+void call^ee(int);
+namespace {
+  void caller1() {
+$Callee[[callee]](42);

[PATCH] D90749: [clangd] Introduce config compilation for External blocks

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

This breaks tests on Windows: http://45.33.8.238/win/28408/step_9.txt

Please take a look, and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90749

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

[PATCH] D91747: [Clang] Add __STDCPP_THREADS__ to standard predefine macros

[Zequan Wu via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG15a3ae1ab1a6: [Clang] Add __STDCPP_THREADS__ to standard 
predefine macros (authored by zequanwu).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91747

Files:
  clang/include/clang/Basic/CodeGenOptions.h
  clang/include/clang/Basic/LangOptions.def
  clang/include/clang/Basic/LangOptions.h
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/InitPreprocessor.cpp
  clang/test/CXX/cpp/cpp.predefined/p2.cpp
  clang/test/Preprocessor/init-aarch64.c

Index: clang/test/Preprocessor/init-aarch64.c
===
--- clang/test/Preprocessor/init-aarch64.c
+++ clang/test/Preprocessor/init-aarch64.c
@@ -233,6 +233,7 @@
 // AARCH64-NEXT: #define __SIZE_TYPE__ long unsigned int
 // AARCH64-NEXT: #define __SIZE_WIDTH__ 64
 // AARCH64_CXX: #define __STDCPP_DEFAULT_NEW_ALIGNMENT__ 16UL
+// AARCH64_CXX: #define __STDCPP_THREADS__ 1
 // AARCH64-NEXT: #define __STDC_HOSTED__ 1
 // AARCH64-NEXT: #define __STDC_UTF_16__ 1
 // AARCH64-NEXT: #define __STDC_UTF_32__ 1
Index: clang/test/CXX/cpp/cpp.predefined/p2.cpp
===
--- /dev/null
+++ clang/test/CXX/cpp/cpp.predefined/p2.cpp
@@ -0,0 +1,17 @@
+// RUN: split-file %s %t.dir
+// RUN: %clang_cc1 -verify %t.dir/defined.cpp
+// RUN: %clang_cc1 -verify -mthread-model posix %t.dir/defined.cpp
+// RUN: %clang_cc1 -verify -mthread-model single %t.dir/not-defined.cpp
+// RUN: %clang_cc1 -verify -x c %t.dir/not-defined.cpp
+
+//--- defined.cpp
+// expected-no-diagnostics
+#ifndef __STDCPP_THREADS__
+#error __STDCPP_THREADS__ is not defined in posix thread model.
+#endif
+
+//--- not-defined.cpp
+// expected-no-diagnostics
+#ifdef __STDCPP_THREADS__
+#error __STDCPP_THREADS__ is defined in single thread model.
+#endif
Index: clang/lib/Frontend/InitPreprocessor.cpp
===
--- clang/lib/Frontend/InitPreprocessor.cpp
+++ clang/lib/Frontend/InitPreprocessor.cpp
@@ -403,6 +403,12 @@
 Builder.defineMacro("__STDCPP_DEFAULT_NEW_ALIGNMENT__",
 Twine(TI.getNewAlign() / TI.getCharWidth()) +
 TI.getTypeConstantSuffix(TI.getSizeType()));
+
+//   -- __STDCPP_­THREADS__
+//  Defined, and has the value integer literal 1, if and only if a
+//  program can have more than one thread of execution.
+if (LangOpts.getThreadModel() == LangOptions::ThreadModelKind::POSIX)
+  Builder.defineMacro("__STDCPP_THREADS__", "1");
   }
 
   // In C11 these are environment macros. In C++11 they are only defined
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1044,12 +1044,6 @@
   Opts.StrictVTablePointers = Args.hasArg(OPT_fstrict_vtable_pointers);
   Opts.ForceEmitVTables = Args.hasArg(OPT_fforce_emit_vtables);
   Opts.UnwindTables = Args.hasArg(OPT_munwind_tables);
-  Opts.ThreadModel =
-  std::string(Args.getLastArgValue(OPT_mthread_model, "posix"));
-  if (Opts.ThreadModel != "posix" && Opts.ThreadModel != "single")
-Diags.Report(diag::err_drv_invalid_value)
-<< Args.getLastArg(OPT_mthread_model)->getAsString(Args)
-<< Opts.ThreadModel;
   Opts.TrapFuncName = std::string(Args.getLastArgValue(OPT_ftrap_function_EQ));
   Opts.UseInitArray = !Args.hasArg(OPT_fno_use_init_array);
 
@@ -3529,6 +3523,16 @@
   Args.hasFlag(OPT_fexperimental_relative_cxx_abi_vtables,
OPT_fno_experimental_relative_cxx_abi_vtables,
/*default=*/false);
+
+  std::string ThreadModel =
+  std::string(Args.getLastArgValue(OPT_mthread_model, "posix"));
+  if (ThreadModel != "posix" && ThreadModel != "single")
+Diags.Report(diag::err_drv_invalid_value)
+<< Args.getLastArg(OPT_mthread_model)->getAsString(Args) << ThreadModel;
+  Opts.setThreadModel(
+  llvm::StringSwitch(ThreadModel)
+  .Case("posix", LangOptions::ThreadModelKind::POSIX)
+  .Case("single", LangOptions::ThreadModelKind::Single));
 }
 
 static bool isStrictlyPreprocessorAction(frontend::ActionKind Action) {
Index: clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
===
--- clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
+++ clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
@@ -49,7 +49,7 @@
   const PreprocessorOptions 
   CodeGenOptions CodeGenOpts;
   const TargetOptions TargetOpts;
-  const LangOptions LangOpts;
+  LangOptions LangOpts;
   std::unique_ptr 

[clang] 15a3ae1 - [Clang] Add __STDCPP_THREADS__ to standard predefine macros

[Zequan Wu via cfe-commits]

Author: Zequan Wu
Date: 2020-11-22T16:05:53-08:00
New Revision: 15a3ae1ab1a64cc62041c32ba54914a9dd7b8361

URL: 
https://github.com/llvm/llvm-project/commit/15a3ae1ab1a64cc62041c32ba54914a9dd7b8361
DIFF: 
https://github.com/llvm/llvm-project/commit/15a3ae1ab1a64cc62041c32ba54914a9dd7b8361.diff

LOG: [Clang] Add __STDCPP_THREADS__ to standard predefine macros

According to https://eel.is/c++draft/cpp.predefined#2.6, `__STDCPP_THREADS__` 
is a predefined macro.

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

Added: 
clang/test/CXX/cpp/cpp.predefined/p2.cpp

Modified: 
clang/include/clang/Basic/CodeGenOptions.h
clang/include/clang/Basic/LangOptions.def
clang/include/clang/Basic/LangOptions.h
clang/lib/CodeGen/BackendUtil.cpp
clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
clang/lib/Frontend/CompilerInvocation.cpp
clang/lib/Frontend/InitPreprocessor.cpp
clang/test/Preprocessor/init-aarch64.c

Removed: 




diff  --git a/clang/include/clang/Basic/CodeGenOptions.h 
b/clang/include/clang/Basic/CodeGenOptions.h
index 6452d2bb25f6..e710c5792d76 100644
--- a/clang/include/clang/Basic/CodeGenOptions.h
+++ b/clang/include/clang/Basic/CodeGenOptions.h
@@ -211,9 +211,6 @@ class CodeGenOptions : public CodeGenOptionsBase {
   /// The name of the relocation model to use.
   llvm::Reloc::Model RelocationModel;
 
-  /// The thread model to use
-  std::string ThreadModel;
-
   /// If not an empty string, trap intrinsics are lowered to calls to this
   /// function instead of to trap instructions.
   std::string TrapFuncName;

diff  --git a/clang/include/clang/Basic/LangOptions.def 
b/clang/include/clang/Basic/LangOptions.def
index 3788ae87f6b9..9c573b43049c 100644
--- a/clang/include/clang/Basic/LangOptions.def
+++ b/clang/include/clang/Basic/LangOptions.def
@@ -330,6 +330,7 @@ VALUE_LANGOPT(TrivialAutoVarInitStopAfter, 32, 0,
  "stop trivial automatic variable initialization after the 
specified number of instances. Must be greater than 0.")
 ENUM_LANGOPT(SignedOverflowBehavior, SignedOverflowBehaviorTy, 2, 
SOB_Undefined,
  "signed integer overflow handling")
+ENUM_LANGOPT(ThreadModel  , ThreadModelKind, 2, ThreadModelKind::POSIX, 
"Thread Model")
 
 BENIGN_LANGOPT(ArrowDepth, 32, 256,
"maximum number of operator->s to follow")

diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index dea9d217cf0c..7806483ec5b5 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -232,6 +232,13 @@ class LangOptions : public LangOptionsBase {
 BKey
   };
 
+  enum class ThreadModelKind {
+/// POSIX Threads.
+POSIX,
+/// Single Threaded Environment.
+Single
+  };
+
 public:
   /// Set of enabled sanitizers.
   SanitizerSet Sanitize;

diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 816eaf3bf27e..243468598928 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -453,10 +453,14 @@ static bool initTargetOptions(DiagnosticsEngine ,
   const clang::TargetOptions ,
   const LangOptions ,
   const HeaderSearchOptions ) {
-  Options.ThreadModel =
-  llvm::StringSwitch(CodeGenOpts.ThreadModel)
-  .Case("posix", llvm::ThreadModel::POSIX)
-  .Case("single", llvm::ThreadModel::Single);
+  switch (LangOpts.getThreadModel()) {
+  case LangOptions::ThreadModelKind::POSIX:
+Options.ThreadModel = llvm::ThreadModel::POSIX;
+break;
+  case LangOptions::ThreadModelKind::Single:
+Options.ThreadModel = llvm::ThreadModel::Single;
+break;
+  }
 
   // Set float ABI type.
   assert((CodeGenOpts.FloatABI == "soft" || CodeGenOpts.FloatABI == "softfp" ||

diff  --git a/clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp 
b/clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
index 04bd6680e31c..de5c1a4c8f02 100644
--- a/clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
+++ b/clang/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
@@ -49,7 +49,7 @@ class PCHContainerGenerator : public ASTConsumer {
   const PreprocessorOptions 
   CodeGenOptions CodeGenOpts;
   const TargetOptions TargetOpts;
-  const LangOptions LangOpts;
+  LangOptions LangOpts;
   std::unique_ptr VMContext;
   std::unique_ptr M;
   std::unique_ptr Builder;
@@ -147,7 +147,7 @@ class PCHContainerGenerator : public ASTConsumer {
 // The debug info output isn't affected by CodeModel and
 // ThreadModel, but the backend expects them to be nonempty.
 CodeGenOpts.CodeModel = "default";
-CodeGenOpts.ThreadModel = "single";
+LangOpts.setThreadModel(LangOptions::ThreadModelKind::Single);
 CodeGenOpts.DebugTypeExtRefs = true;
 // When building a module MainFileName is the name of the modulemap 

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

[Shane via Phabricator via cfe-commits]

smhc added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:127-128
 getFileStyleFromOptions(const ClangTidyCheck::OptionsView ) {
   SmallVector, 0> Styles(
   SK_Count);
   SmallString<64> StyleString;

njames93 wrote:
> Making this change removes the need to NamingStyle to be copy 
> constructable/assignable.
Thanks - was looking for a solution to this.


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

https://reviews.llvm.org/D90282

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

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

[Shane via Phabricator via cfe-commits]

smhc updated this revision to Diff 306942.
smhc marked 4 inline comments as done.
smhc added a comment.

Updated after review comments.


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

https://reviews.llvm.org/D90282

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignored-regexp.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignored-regexp.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-ignored-regexp.cpp
@@ -0,0 +1,47 @@
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config='{CheckOptions: [ \
+// RUN: {key: readability-identifier-naming.ParameterCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ParameterIgnoredRegexp, value: "^[a-z]{1,2}$"}, \
+// RUN: {key: readability-identifier-naming.ClassCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.ClassIgnoredRegexp, value: "^fo$|^fooo$"}, \
+// RUN: {key: readability-identifier-naming.StructCase, value: CamelCase}, \
+// RUN: {key: readability-identifier-naming.StructIgnoredRegexp, value: "sooo|so|soo|$invalidregex["} \
+// RUN:  ]}'
+
+int testFunc(int a, char **b);
+int testFunc(int ab, char **ba);
+int testFunc(int abc, char **cba);
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'abc'
+// CHECK-MESSAGES: :[[@LINE-2]]:30: warning: invalid case style for parameter 'cba'
+// CHECK-FIXES: {{^}}int testFunc(int Abc, char **Cba);{{$}}
+int testFunc(int dE, char **eD);
+// CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for parameter 'dE'
+// CHECK-MESSAGES: :[[@LINE-2]]:29: warning: invalid case style for parameter 'eD'
+// CHECK-FIXES: {{^}}int testFunc(int DE, char **ED);{{$}}
+int testFunc(int Abc, char **Cba);
+
+class fo {
+};
+
+class fofo {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'fofo'
+  // CHECK-FIXES: {{^}}class Fofo {{{$}}
+};
+
+class foo {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'foo'
+  // CHECK-FIXES: {{^}}class Foo {{{$}}
+};
+
+class fooo {
+};
+
+class afooo {
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for class 'afooo'
+  // CHECK-FIXES: {{^}}class Afooo {{{$}}
+};
+
+struct soo {
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for struct 'soo'
+  // CHECK-FIXES: {{^}}struct Soo {{{$}}
+};
Index: clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
@@ -35,60 +35,60 @@
 
 The following options are describe below:
 
- - :option:`AbstractClassCase`, :option:`AbstractClassPrefix`, :option:`AbstractClassSuffix`
+ - :option:`AbstractClassCase`, :option:`AbstractClassPrefix`, :option:`AbstractClassSuffix`, :option:`AbstractClassIgnoredRegexp`
  - :option:`AggressiveDependentMemberLookup`
- - :option:`ClassCase`, :option:`ClassPrefix`, :option:`ClassSuffix`
- - :option:`ClassConstantCase`, :option:`ClassConstantPrefix`, :option:`ClassConstantSuffix`
- - :option:`ClassMemberCase`, :option:`ClassMemberPrefix`, :option:`ClassMemberSuffix`
- - :option:`ClassMethodCase`, :option:`ClassMethodPrefix`, :option:`ClassMethodSuffix`
- - :option:`ConstantCase`, :option:`ConstantPrefix`, :option:`ConstantSuffix`
- - :option:`ConstantMemberCase`, :option:`ConstantMemberPrefix`, :option:`ConstantMemberSuffix`
- - :option:`ConstantParameterCase`, :option:`ConstantParameterPrefix`, :option:`ConstantParameterSuffix`
- - :option:`ConstantPointerParameterCase`, :option:`ConstantPointerParameterPrefix`, :option:`ConstantPointerParameterSuffix`
- - :option:`ConstexprFunctionCase`, :option:`ConstexprFunctionPrefix`, :option:`ConstexprFunctionSuffix`
- - :option:`ConstexprMethodCase`, :option:`ConstexprMethodPrefix`, :option:`ConstexprMethodSuffix`
- - :option:`ConstexprVariableCase`, :option:`ConstexprVariablePrefix`, :option:`ConstexprVariableSuffix`
- - :option:`EnumCase`, :option:`EnumPrefix`, :option:`EnumSuffix`
- - :option:`EnumConstantCase`, :option:`EnumConstantPrefix`, :option:`EnumConstantSuffix`
- - :option:`FunctionCase`, :option:`FunctionPrefix`, :option:`FunctionSuffix`
+ - :option:`ClassCase`, :option:`ClassPrefix`, :option:`ClassSuffix`, :option:`ClassIgnoredRegexp`
+ - :option:`ClassConstantCase`, :option:`ClassConstantPrefix`, :option:`ClassConstantSuffix`, :option:`ClassConstantIgnoredRegexp`
+ - 

[PATCH] D91859: [clangd] Fix shared-lib builds

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

In D91859#2410252 , @nridge wrote:

> In D91859#2410002 , @kbobyrev wrote:
>
>> Ah, right. I think `clangd-remote-index` needs `protobuf` dependency. 
>> @nridge, I believe this should fix the problem you're seeing.
>
> Not sure if this is what you meant, but I fixed it by adding a `protobuf` 
> dependency to `clangd-index-server` in 
> clang-tools-extra/clangd/index/remote/server/CMakeLists.txt.
>
> After fixing that (and one more shared-libs dependency issue unrelated to 
> clangd), my build completed successfully.

Uh, yeah, sorry, I meant `clangd-index-server`, just used the wrong name for 
some reason. Thanks for checking!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91859

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

[PATCH] D91859: [clangd] Fix shared-lib builds

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D91859#2410002 , @kbobyrev wrote:

> Ah, right. I think `clangd-remote-index` needs `protobuf` dependency. 
> @nridge, I believe this should fix the problem you're seeing.

Not sure if this is what you meant, but I fixed it by adding a `protobuf` 
dependency to `clangd-index-server` in 
clang-tools-extra/clangd/index/remote/server/CMakeLists.txt.

After fixing that (and one more shared-libs dependency issue unrelated to 
clangd), my build completed successfully.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91859

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

[PATCH] D91859: [clangd] Fix shared-lib builds

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D91859#2410166 , @kadircet wrote:

> In D91859#2409743 , @nridge wrote:
>
>> I applied the patch locally and it fixes most of the linker errors but I'm 
>> still seeing one:
>>
>> [...]
>
> I am not able to reproduce the failure. Maybe we should just make the 
> dependencies introduced in `generate_protos` for grpc++ and protobuf PUBLIC, 
> to ensure they propagate into users (i believe that's the default for my 
> configuration for whatever reason). Can you share your cmake configuration ?

The cmake command I'm using for this build building is something like:

  cmake -G Ninja  -DLLVM_ENABLE_PROJECTS="clang;clang-tools-extra" 
-DLLVM_USE_LINKER=gold -DBUILD_SHARED_LIBS=ON \
  -DLLVM_OPTIMIZED_TABLEGEN=ON -DLLVM_TARGETS_TO_BUILD=X86 
-DLLVM_CCACHE_BUILD=ON -DCMAKE_BUILD_TYPE=Release \
  -DCLANGD_ENABLE_REMOTE=ON \
  -DLLVM_APPEND_VC_REV=OFF


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91859

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

[PATCH] D91908: [clang-tidy] Use compiled regex for AllowedRegexp in macro usage check

[Shane via Phabricator via cfe-commits]

smhc added a comment.

Thank you - I don't have commit access so am unable to merge it myself. The 
clang-tidy tests are passing fine.


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

https://reviews.llvm.org/D91908

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

[clang-tools-extra] 6553600 - [clangd] Fix use-after-free in ProjectAwareIndex tests

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-11-22T21:29:45+01:00
New Revision: 655360096f27f25a0e2f71729c1c879f1fd8d8a2

URL: 
https://github.com/llvm/llvm-project/commit/655360096f27f25a0e2f71729c1c879f1fd8d8a2
DIFF: 
https://github.com/llvm/llvm-project/commit/655360096f27f25a0e2f71729c1c879f1fd8d8a2.diff

LOG: [clangd] Fix use-after-free in ProjectAwareIndex tests

Added: 


Modified: 
clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp 
b/clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
index 8adac296ee60..0d14d2ed5d54 100644
--- a/clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
@@ -27,9 +27,9 @@ using testing::ElementsAre;
 using testing::IsEmpty;
 
 std::unique_ptr createIndex() {
-  std::vector Symbols = {symbol("1")};
-  return std::make_unique(std::move(Symbols), RefSlab(),
-RelationSlab());
+  SymbolSlab::Builder Builder;
+  Builder.insert(symbol("1"));
+  return MemIndex::build(std::move(Builder).build(), RefSlab(), 
RelationSlab());
 }
 
 TEST(ProjectAware, Test) {



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

[PATCH] D91860: [clangd] Move remote-index dependency from clangDaemon to ClangdMain

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcab313680703: [clangd] Use ProjectAwareIndex in ClangdMain 
(authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91860

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/index/ProjectAware.cpp
  clang-tools-extra/clangd/index/ProjectAware.h
  clang-tools-extra/clangd/tool/ClangdMain.cpp

Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -13,6 +13,9 @@
 #include "Protocol.h"
 #include "Transport.h"
 #include "index/Background.h"
+#include "index/Index.h"
+#include "index/Merge.h"
+#include "index/ProjectAware.h"
 #include "index/Serialization.h"
 #include "index/remote/Client.h"
 #include "refactor/Rename.h"
@@ -40,6 +43,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifndef _WIN32
 #include 
@@ -551,6 +555,27 @@
 const char TestScheme::TestDir[] = "/clangd-test";
 #endif
 
+std::unique_ptr
+loadExternalIndex(const Config::ExternalIndexSpec ,
+  AsyncTaskRunner ) {
+  switch (External.Kind) {
+  case Config::ExternalIndexSpec::Server:
+log("Associating {0} with remote index at {1}.", External.MountPoint,
+External.Location);
+return remote::getClient(External.Location, External.MountPoint);
+  case Config::ExternalIndexSpec::File:
+log("Associating {0} with monolithic index at {1}.", External.MountPoint,
+External.Location);
+auto NewIndex = std::make_unique(std::make_unique());
+Tasks.runAsync("Load-index:" + External.Location,
+   [File = External.Location, PlaceHolder = NewIndex.get()] {
+ if (auto Idx = loadIndex(File, /*UseDex=*/true))
+   PlaceHolder->reset(std::move(Idx));
+   });
+return std::move(NewIndex);
+  }
+  llvm_unreachable("Invalid ExternalIndexKind.");
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
@@ -726,6 +751,7 @@
 Opts.ResourceDir = ResourceDir;
   Opts.BuildDynamicSymbolIndex = EnableIndex;
   Opts.CollectMainFileRefs = CollectMainFileRefs;
+  std::vector> IdxStack;
   std::unique_ptr StaticIdx;
   std::future AsyncIndexLoad; // Block exit while loading the index.
   if (EnableIndex && !IndexFile.empty()) {
@@ -757,7 +783,15 @@
   }
 #endif
   Opts.BackgroundIndex = EnableBackgroundIndex;
-  Opts.StaticIndex = StaticIdx.get();
+  auto PAI = createProjectAwareIndex(loadExternalIndex);
+  if (StaticIdx) {
+IdxStack.emplace_back(std::move(StaticIdx));
+IdxStack.emplace_back(
+std::make_unique(PAI.get(), IdxStack.back().get()));
+Opts.StaticIndex = IdxStack.back().get();
+  } else {
+Opts.StaticIndex = PAI.get();
+  }
   Opts.AsyncThreadsCount = WorkerThreadsCount;
   Opts.BuildRecoveryAST = RecoveryAST;
   Opts.PreserveRecoveryASTType = RecoveryASTType;
Index: clang-tools-extra/clangd/index/ProjectAware.h
===
--- clang-tools-extra/clangd/index/ProjectAware.h
+++ clang-tools-extra/clangd/index/ProjectAware.h
@@ -23,11 +23,11 @@
 using IndexFactory = std::function(
 const Config::ExternalIndexSpec &, AsyncTaskRunner &)>;
 
-/// Returns an index that answers queries using external indices. IndexGenerator
-/// can be used to customize how to generate an index from an external source.
-/// The default implementation loads the index asynchronously on the
-/// AsyncTaskRunner. The index will appear empty until loaded.
-std::unique_ptr createProjectAwareIndex(IndexFactory = nullptr);
+/// Returns an index that answers queries using external indices. IndexFactory
+/// specifies how to generate an index from an external source.
+/// IndexFactory must be injected because this code cannot depend on the remote
+/// index client.
+std::unique_ptr createProjectAwareIndex(IndexFactory);
 } // namespace clangd
 } // namespace clang
 
Index: clang-tools-extra/clangd/index/ProjectAware.cpp
===
--- clang-tools-extra/clangd/index/ProjectAware.cpp
+++ clang-tools-extra/clangd/index/ProjectAware.cpp
@@ -15,7 +15,6 @@
 #include "index/Serialization.h"
 #include "index/Symbol.h"
 #include "index/SymbolID.h"
-#include "index/remote/Client.h"
 #include "support/Logger.h"
 #include "support/Threading.h"
 #include "support/Trace.h"
@@ -124,33 +123,11 @@
 Entry.first->getSecond() = Gen(External, Tasks);
   return Entry.first->second.get();
 }
-
-std::unique_ptr
-defaultFactory(const Config::ExternalIndexSpec ,
-   AsyncTaskRunner ) {
-  switch (External.Kind) {
-  case Config::ExternalIndexSpec::Server:
-log("Associating {0} with remote index at 

[PATCH] D90750: [clangd] Introduce ProjectAwareIndex

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG067ffbfe6018: [clangd] Introduce ProjectAwareIndex (authored 
by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90750

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/index/ProjectAware.cpp
  clang-tools-extra/clangd/index/ProjectAware.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp

Index: clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
@@ -0,0 +1,86 @@
+//===-- ProjectAwareIndexTests.cpp  ---*- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Config.h"
+#include "TestIndex.h"
+#include "index/Index.h"
+#include "index/MemIndex.h"
+#include "index/ProjectAware.h"
+#include "index/Ref.h"
+#include "index/Relation.h"
+#include "support/Context.h"
+#include "support/Threading.h"
+#include "llvm/ADT/StringRef.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+using testing::ElementsAre;
+using testing::IsEmpty;
+
+std::unique_ptr createIndex() {
+  std::vector Symbols = {symbol("1")};
+  return std::make_unique(std::move(Symbols), RefSlab(),
+RelationSlab());
+}
+
+TEST(ProjectAware, Test) {
+  IndexFactory Gen = [](const Config::ExternalIndexSpec &, AsyncTaskRunner &) {
+return createIndex();
+  };
+
+  auto Idx = createProjectAwareIndex(std::move(Gen));
+  FuzzyFindRequest Req;
+  Req.Query = "1";
+  Req.AnyScope = true;
+
+  EXPECT_THAT(match(*Idx, Req), IsEmpty());
+
+  Config C;
+  C.Index.External.emplace();
+  C.Index.External->Location = "test";
+  WithContextValue With(Config::Key, std::move(C));
+  EXPECT_THAT(match(*Idx, Req), ElementsAre("1"));
+  return;
+}
+
+TEST(ProjectAware, CreatedOnce) {
+  unsigned InvocationCount = 0;
+  IndexFactory Gen = [&](const Config::ExternalIndexSpec &, AsyncTaskRunner &) {
+++InvocationCount;
+return createIndex();
+  };
+
+  auto Idx = createProjectAwareIndex(std::move(Gen));
+  // No invocation at start.
+  EXPECT_EQ(InvocationCount, 0U);
+  FuzzyFindRequest Req;
+  Req.Query = "1";
+  Req.AnyScope = true;
+
+  // Cannot invoke without proper config.
+  match(*Idx, Req);
+  EXPECT_EQ(InvocationCount, 0U);
+
+  Config C;
+  C.Index.External.emplace();
+  C.Index.External->Location = "test";
+  WithContextValue With(Config::Key, std::move(C));
+  match(*Idx, Req);
+  // Now it should be created.
+  EXPECT_EQ(InvocationCount, 1U);
+  match(*Idx, Req);
+  // It is cached afterwards.
+  EXPECT_EQ(InvocationCount, 1U);
+  return;
+}
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -74,6 +74,7 @@
   PathMappingTests.cpp
   PreambleTests.cpp
   PrintASTTests.cpp
+  ProjectAwareIndexTests.cpp
   QualityTests.cpp
   RenameTests.cpp
   RIFFTests.cpp
Index: clang-tools-extra/clangd/index/ProjectAware.h
===
--- /dev/null
+++ clang-tools-extra/clangd/index/ProjectAware.h
@@ -0,0 +1,34 @@
+//===--- ProjectAware.h --*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_PROJECT_AWARE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_PROJECT_AWARE_H
+
+#include "Config.h"
+#include "index/Index.h"
+#include "support/Threading.h"
+#include 
+#include 
+namespace clang {
+namespace clangd {
+
+/// A functor to create an index for an external index specification. Functor
+/// should perform any high latency operation in a separate thread through
+/// AsyncTaskRunner.
+using IndexFactory = std::function(
+const Config::ExternalIndexSpec &, AsyncTaskRunner &)>;
+
+/// Returns an index that answers queries using external indices. IndexGenerator
+/// can be used to customize how to generate an index from an external 

[clang-tools-extra] cab3136 - [clangd] Use ProjectAwareIndex in ClangdMain

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-11-22T20:59:38+01:00
New Revision: cab313680703097f5f4642b348c43018f55a5a4f

URL: 
https://github.com/llvm/llvm-project/commit/cab313680703097f5f4642b348c43018f55a5a4f
DIFF: 
https://github.com/llvm/llvm-project/commit/cab313680703097f5f4642b348c43018f55a5a4f.diff

LOG: [clangd] Use ProjectAwareIndex in ClangdMain

Put project-aware-index between command-line specified static index and
ClangdServer indexes.

This also moves remote-index dependency from clangDaemon to ClangdMain
in an attempt to prevent cyclic dependency between clangDaemon and
remote-index-marshalling.

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

Added: 


Modified: 
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/index/ProjectAware.cpp
clang-tools-extra/clangd/index/ProjectAware.h
clang-tools-extra/clangd/tool/ClangdMain.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/CMakeLists.txt 
b/clang-tools-extra/clangd/CMakeLists.txt
index d02a5cf3f2ec..72b232b92c08 100644
--- a/clang-tools-extra/clangd/CMakeLists.txt
+++ b/clang-tools-extra/clangd/CMakeLists.txt
@@ -151,7 +151,6 @@ target_link_libraries(clangDaemon
   clangTidy
   ${ALL_CLANG_TIDY_CHECKS}
 
-  clangdRemoteIndex
   clangdSupport
   )
 

diff  --git a/clang-tools-extra/clangd/index/ProjectAware.cpp 
b/clang-tools-extra/clangd/index/ProjectAware.cpp
index 58685fd14bf5..63f8f823f3a7 100644
--- a/clang-tools-extra/clangd/index/ProjectAware.cpp
+++ b/clang-tools-extra/clangd/index/ProjectAware.cpp
@@ -15,7 +15,6 @@
 #include "index/Serialization.h"
 #include "index/Symbol.h"
 #include "index/SymbolID.h"
-#include "index/remote/Client.h"
 #include "support/Logger.h"
 #include "support/Threading.h"
 #include "support/Trace.h"
@@ -124,33 +123,11 @@ SymbolIndex *ProjectAwareIndex::getIndex() const {
 Entry.first->getSecond() = Gen(External, Tasks);
   return Entry.first->second.get();
 }
-
-std::unique_ptr
-defaultFactory(const Config::ExternalIndexSpec ,
-   AsyncTaskRunner ) {
-  switch (External.Kind) {
-  case Config::ExternalIndexSpec::Server:
-log("Associating {0} with remote index at {1}.", External.MountPoint,
-External.Location);
-return remote::getClient(External.Location, External.MountPoint);
-  case Config::ExternalIndexSpec::File:
-log("Associating {0} with monolithic index at {1}.", External.MountPoint,
-External.Location);
-auto NewIndex = std::make_unique(std::make_unique());
-Tasks.runAsync("Load-index:" + External.Location,
-   [File = External.Location, PlaceHolder = NewIndex.get()] {
- if (auto Idx = loadIndex(File, /*UseDex=*/true))
-   PlaceHolder->reset(std::move(Idx));
-   });
-return std::move(NewIndex);
-  }
-  llvm_unreachable("Invalid ExternalIndexKind.");
-}
 } // namespace
 
 std::unique_ptr createProjectAwareIndex(IndexFactory Gen) {
-  return std::make_unique(Gen ? std::move(Gen)
- : defaultFactory);
+  assert(Gen);
+  return std::make_unique(std::move(Gen));
 }
 } // namespace clangd
 } // namespace clang

diff  --git a/clang-tools-extra/clangd/index/ProjectAware.h 
b/clang-tools-extra/clangd/index/ProjectAware.h
index 2d7c6ba3ad88..af98ba612751 100644
--- a/clang-tools-extra/clangd/index/ProjectAware.h
+++ b/clang-tools-extra/clangd/index/ProjectAware.h
@@ -23,11 +23,11 @@ namespace clangd {
 using IndexFactory = std::function(
 const Config::ExternalIndexSpec &, AsyncTaskRunner &)>;
 
-/// Returns an index that answers queries using external indices. 
IndexGenerator
-/// can be used to customize how to generate an index from an external source.
-/// The default implementation loads the index asynchronously on the
-/// AsyncTaskRunner. The index will appear empty until loaded.
-std::unique_ptr createProjectAwareIndex(IndexFactory = nullptr);
+/// Returns an index that answers queries using external indices. IndexFactory
+/// specifies how to generate an index from an external source.
+/// IndexFactory must be injected because this code cannot depend on the remote
+/// index client.
+std::unique_ptr createProjectAwareIndex(IndexFactory);
 } // namespace clangd
 } // namespace clang
 

diff  --git a/clang-tools-extra/clangd/tool/ClangdMain.cpp 
b/clang-tools-extra/clangd/tool/ClangdMain.cpp
index 08d498f30873..1a70058cd8ab 100644
--- a/clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ b/clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -13,6 +13,9 @@
 #include "Protocol.h"
 #include "Transport.h"
 #include "index/Background.h"
+#include "index/Index.h"
+#include "index/Merge.h"
+#include "index/ProjectAware.h"
 #include "index/Serialization.h"
 #include "index/remote/Client.h"
 #include "refactor/Rename.h"
@@ -40,6 +43,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #ifndef _WIN32
 #include 
@@ -551,6 

[clang-tools-extra] 067ffbf - [clangd] Introduce ProjectAwareIndex

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-11-22T20:59:37+01:00
New Revision: 067ffbfe60180aa7b1fdd87b2b6e8ccc67a43a76

URL: 
https://github.com/llvm/llvm-project/commit/067ffbfe60180aa7b1fdd87b2b6e8ccc67a43a76
DIFF: 
https://github.com/llvm/llvm-project/commit/067ffbfe60180aa7b1fdd87b2b6e8ccc67a43a76.diff

LOG: [clangd] Introduce ProjectAwareIndex

An index implementation that can dispatch to a variety of indexes
depending on the file path. Enables clangd to work with multiple indexes in the
same instance, configured via config files.

Depends on D90749, D90746

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

Added: 
clang-tools-extra/clangd/index/ProjectAware.cpp
clang-tools-extra/clangd/index/ProjectAware.h
clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp

Modified: 
clang-tools-extra/clangd/CMakeLists.txt
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/unittests/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/clangd/CMakeLists.txt 
b/clang-tools-extra/clangd/CMakeLists.txt
index 3fd110e9e135..d02a5cf3f2ec 100644
--- a/clang-tools-extra/clangd/CMakeLists.txt
+++ b/clang-tools-extra/clangd/CMakeLists.txt
@@ -97,6 +97,7 @@ add_clang_library(clangDaemon
   index/IndexAction.cpp
   index/MemIndex.cpp
   index/Merge.cpp
+  index/ProjectAware.cpp
   index/Ref.cpp
   index/Relation.cpp
   index/Serialization.cpp
@@ -150,6 +151,7 @@ target_link_libraries(clangDaemon
   clangTidy
   ${ALL_CLANG_TIDY_CHECKS}
 
+  clangdRemoteIndex
   clangdSupport
   )
 

diff  --git a/clang-tools-extra/clangd/Config.h 
b/clang-tools-extra/clangd/Config.h
index 318220364e6e..79e94ef6fe37 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -97,4 +97,24 @@ struct Config {
 } // namespace clangd
 } // namespace clang
 
+namespace llvm {
+template <> struct DenseMapInfo {
+  using ExternalIndexSpec = clang::clangd::Config::ExternalIndexSpec;
+  static inline ExternalIndexSpec getEmptyKey() {
+return {ExternalIndexSpec::File, "", ""};
+  }
+  static inline ExternalIndexSpec getTombstoneKey() {
+return {ExternalIndexSpec::File, "TOMB", "STONE"};
+  }
+  static unsigned getHashValue(const ExternalIndexSpec ) {
+return llvm::hash_combine(Val.Kind, Val.Location, Val.MountPoint);
+  }
+  static bool isEqual(const ExternalIndexSpec ,
+  const ExternalIndexSpec ) {
+return std::tie(LHS.Kind, LHS.Location, LHS.MountPoint) ==
+   std::tie(RHS.Kind, RHS.Location, RHS.MountPoint);
+  }
+};
+} // namespace llvm
+
 #endif

diff  --git a/clang-tools-extra/clangd/index/ProjectAware.cpp 
b/clang-tools-extra/clangd/index/ProjectAware.cpp
new file mode 100644
index ..58685fd14bf5
--- /dev/null
+++ b/clang-tools-extra/clangd/index/ProjectAware.cpp
@@ -0,0 +1,156 @@
+//===--- ProjectAware.h --*- 
C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "ProjectAware.h"
+#include "Config.h"
+#include "index/Index.h"
+#include "index/MemIndex.h"
+#include "index/Merge.h"
+#include "index/Ref.h"
+#include "index/Serialization.h"
+#include "index/Symbol.h"
+#include "index/SymbolID.h"
+#include "index/remote/Client.h"
+#include "support/Logger.h"
+#include "support/Threading.h"
+#include "support/Trace.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ErrorHandling.h"
+#include 
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+namespace {
+class ProjectAwareIndex : public SymbolIndex {
+public:
+  size_t estimateMemoryUsage() const override;
+
+  /// Only queries the associated index with the current context.
+  void lookup(const LookupRequest ,
+  llvm::function_ref Callback) const 
override;
+
+  /// Query all indexes while prioritizing the associated one (if any).
+  bool refs(const RefsRequest ,
+llvm::function_ref Callback) const override;
+
+  /// Queries only the associates index when Req.RestrictForCodeCompletion is
+  /// set, otherwise queries all.
+  bool
+  fuzzyFind(const FuzzyFindRequest ,
+llvm::function_ref Callback) const override;
+
+  /// Query all indexes while prioritizing the associated one (if any).
+  void relations(const RelationsRequest ,
+ llvm::function_ref
+ Callback) const override;
+
+  ProjectAwareIndex(IndexFactory Gen) : Gen(std::move(Gen)) {}
+
+private:
+  // Returns the index associated with current context, if any.
+  SymbolIndex *getIndex() const;
+
+  // Storage for all the external 

[PATCH] D90749: [clangd] Introduce config compilation for External blocks

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGc9776c8d4ef7: [clangd] Introduce config compilation for 
External blocks (authored by kadircet).

Changed prior to commit:
  https://reviews.llvm.org/D90749?vs=304064=306933#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90749

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -9,9 +9,13 @@
 #include "Config.h"
 #include "ConfigFragment.h"
 #include "ConfigTesting.h"
+#include "Features.inc"
 #include "TestFS.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Path.h"
+#include "llvm/Support/SourceMgr.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 #include 
@@ -20,6 +24,8 @@
 namespace clangd {
 namespace config {
 namespace {
+using ::testing::AllOf;
+using ::testing::Contains;
 using ::testing::ElementsAre;
 using ::testing::IsEmpty;
 using ::testing::SizeIs;
@@ -196,6 +202,104 @@
 "0");
 }
 
+TEST_F(ConfigCompileTests, ExternalBlockWarnOnMultipleSource) {
+  Fragment::IndexBlock::ExternalBlock External;
+  External.File.emplace("");
+  External.Server.emplace("");
+  Frag.Index.External = std::move(External);
+  compileAndApply();
+  llvm::StringLiteral ExpectedDiag =
+#ifdef CLANGD_ENABLE_REMOTE
+  "Exactly one of File or Server must be set.";
+#else
+  "Clangd isn't compiled with remote index support, ignoring Server.";
+#endif
+  EXPECT_THAT(Diags.Diagnostics,
+  Contains(AllOf(DiagMessage(ExpectedDiag),
+ DiagKind(llvm::SourceMgr::DK_Error;
+}
+
+TEST_F(ConfigCompileTests, ExternalBlockErrOnNoSource) {
+  Frag.Index.External.emplace(Fragment::IndexBlock::ExternalBlock{});
+  compileAndApply();
+  EXPECT_THAT(
+  Diags.Diagnostics,
+  Contains(AllOf(DiagMessage("Exactly one of File or Server must be set."),
+ DiagKind(llvm::SourceMgr::DK_Error;
+}
+
+TEST_F(ConfigCompileTests, ExternalBlockDisablesBackgroundIndex) {
+  Parm.Path = "/foo/bar/baz.h";
+  Frag.Index.Background.emplace("Build");
+  Fragment::IndexBlock::ExternalBlock External;
+  External.File.emplace("/foo");
+  External.MountPoint.emplace("/foo/bar");
+  Frag.Index.External = std::move(External);
+  compileAndApply();
+  EXPECT_EQ(Conf.Index.Background, Config::BackgroundPolicy::Skip);
+}
+
+TEST_F(ConfigCompileTests, ExternalBlockMountPoint) {
+  auto GetFrag = [](llvm::StringRef Directory,
+llvm::Optional MountPoint) {
+Fragment Frag;
+Frag.Source.Directory = Directory.str();
+Fragment::IndexBlock::ExternalBlock External;
+External.File.emplace("/foo");
+if (MountPoint)
+  External.MountPoint.emplace(*MountPoint);
+Frag.Index.External = std::move(External);
+return Frag;
+  };
+
+  Parm.Path = "/foo/bar.h";
+  // Non-absolute MountPoint without a directory raises an error.
+  Frag = GetFrag("", "foo");
+  compileAndApply();
+  ASSERT_THAT(
+  Diags.Diagnostics,
+  ElementsAre(
+  AllOf(DiagMessage("MountPoint must be an absolute path, because this "
+"fragment is not associated with any directory."),
+DiagKind(llvm::SourceMgr::DK_Error;
+  ASSERT_FALSE(Conf.Index.External);
+
+  // Ok when relative.
+  Frag = GetFrag("/", "foo");
+  compileAndApply();
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+  ASSERT_TRUE(Conf.Index.External);
+  EXPECT_THAT(Conf.Index.External->MountPoint, "/foo");
+
+  // None defaults to ".".
+  Frag = GetFrag("/", llvm::None);
+  compileAndApply();
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+  ASSERT_TRUE(Conf.Index.External);
+  EXPECT_THAT(Conf.Index.External->MountPoint, "/");
+
+  // Without a file, external index is empty.
+  Parm.Path = "";
+  Frag = GetFrag("", "/foo");
+  compileAndApply();
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+  ASSERT_FALSE(Conf.Index.External);
+
+  // File outside MountPoint, no index.
+  Parm.Path = "/bar/baz.h";
+  Frag = GetFrag("", "/foo");
+  compileAndApply();
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+  ASSERT_FALSE(Conf.Index.External);
+
+  // File under MountPoint, index should be set.
+  Parm.Path = "/foo/baz.h";
+  Frag = GetFrag("", "/foo");
+  compileAndApply();
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+  ASSERT_TRUE(Conf.Index.External);
+  EXPECT_THAT(Conf.Index.External->MountPoint, "/foo");
+}
 } // namespace
 } // namespace config
 } // namespace clangd
Index: 

[PATCH] D90748: [clangd] Introduce config parsing for External blocks

[Kadir Cetinkaya via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG359e2f988dc5: [clangd] Introduce config parsing for External 
blocks (authored by kadircet).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90748

Files:
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp


Index: clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -10,6 +10,7 @@
 #include "ConfigFragment.h"
 #include "ConfigTesting.h"
 #include "Protocol.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/SourceMgr.h"
@@ -142,6 +143,25 @@
   ASSERT_THAT(Results, IsEmpty());
 }
 
+TEST(ParseYAML, ExternalBlock) {
+  CapturedDiags Diags;
+  Annotations YAML(R"yaml(
+Index:
+  External:
+File: "foo"
+Server: ^"bar"
+MountPoint: "baz"
+  )yaml");
+  auto Results =
+  Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback());
+  ASSERT_EQ(Results.size(), 1u);
+  ASSERT_TRUE(Results[0].Index.External);
+  EXPECT_THAT(*Results[0].Index.External.getValue()->File, Val("foo"));
+  EXPECT_THAT(*Results[0].Index.External.getValue()->MountPoint, Val("baz"));
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+  EXPECT_THAT(*Results[0].Index.External.getValue()->Server, Val("bar"));
+}
+
 } // namespace
 } // namespace config
 } // namespace clangd
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -5,7 +5,6 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 
//===--===//
-
 #include "ConfigFragment.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"
@@ -111,6 +110,22 @@
 DictParser Dict("Index", this);
 Dict.handle("Background",
 [&](Node ) { F.Background = scalarValue(N, "Background"); });
+Dict.handle("External", [&](Node ) {
+  Fragment::IndexBlock::ExternalBlock External;
+  parse(External, N);
+  F.External.emplace(std::move(External));
+  F.External->Range = N.getSourceRange();
+});
+Dict.parse(N);
+  }
+
+  void parse(Fragment::IndexBlock::ExternalBlock , Node ) {
+DictParser Dict("External", this);
+Dict.handle("File", [&](Node ) { F.File = scalarValue(N, "File"); });
+Dict.handle("Server",
+[&](Node ) { F.Server = scalarValue(N, "Server"); });
+Dict.handle("MountPoint",
+[&](Node ) { F.MountPoint = scalarValue(N, "MountPoint"); });
 Dict.parse(N);
   }
 
Index: clang-tools-extra/clangd/ConfigFragment.h
===
--- clang-tools-extra/clangd/ConfigFragment.h
+++ clang-tools-extra/clangd/ConfigFragment.h
@@ -162,6 +162,22 @@
 /// This is checked for translation units only, not headers they include.
 /// Legal values are "Build" or "Skip".
 llvm::Optional> Background;
+/// An external index uses data source outside of clangd itself. This is
+/// usually prepared using clangd-indexer.
+/// Exactly one source (File/Server) should be configured.
+struct ExternalBlock {
+  /// Path to an index file generated by clangd-indexer. Relative paths may
+  /// be used, if config fragment is associated with a directory.
+  llvm::Optional> File;
+  /// Address and port number for a clangd-index-server. e.g.
+  /// `123.1.1.1:13337`.
+  llvm::Optional> Server;
+  /// Source root governed by this index. Default is the directory
+  /// associated with the config fragment. Absolute in case of user config
+  /// and relative otherwise. Should always use forward-slashes.
+  llvm::Optional> MountPoint;
+};
+llvm::Optional> External;
   };
   IndexBlock Index;
 


Index: clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -10,6 +10,7 @@
 #include "ConfigFragment.h"
 #include "ConfigTesting.h"
 #include "Protocol.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/SourceMgr.h"
@@ -142,6 +143,25 @@
   ASSERT_THAT(Results, IsEmpty());
 }
 
+TEST(ParseYAML, ExternalBlock) {
+  CapturedDiags Diags;
+  Annotations YAML(R"yaml(
+Index:
+  External:
+File: "foo"
+Server: 

[clang-tools-extra] c9776c8 - [clangd] Introduce config compilation for External blocks

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-11-22T20:59:37+01:00
New Revision: c9776c8d4ef7c1d69b6d74b81627c4028396e7c1

URL: 
https://github.com/llvm/llvm-project/commit/c9776c8d4ef7c1d69b6d74b81627c4028396e7c1
DIFF: 
https://github.com/llvm/llvm-project/commit/c9776c8d4ef7c1d69b6d74b81627c4028396e7c1.diff

LOG: [clangd] Introduce config compilation for External blocks

Compilation logic for External blocks. A few of the high level points:
- Requires exactly one-of File/Server at a time:
  - Server is ignored in case of both, with a warning.
  - Having none is an error, would render ExternalBlock void.
- Ensures mountpoint is an absolute path:
  - Interprets it as relative to FragmentDirectory.
  - Defaults to FragmentDirectory when empty.
- Marks Background as Skip.

Depends on D90748.

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

Added: 


Modified: 
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Config.h 
b/clang-tools-extra/clangd/Config.h
index ff285d34633b..318220364e6e 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -26,6 +26,7 @@
 
 #include "support/Context.h"
 #include "llvm/ADT/FunctionExtras.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringMap.h"
 #include 
 #include 
@@ -58,10 +59,22 @@ struct Config {
   } CompileFlags;
 
   enum class BackgroundPolicy { Build, Skip };
+  /// Describes an external index configuration.
+  struct ExternalIndexSpec {
+enum { File, Server } Kind;
+/// This is one of:
+/// - Address of a clangd-index-server, in the form of "ip:port".
+/// - Absolute path to an index produced by clangd-indexer.
+std::string Location;
+/// Absolute path to source root this index is associated with, uses
+/// forward-slashes.
+std::string MountPoint;
+  };
   /// Controls background-index behavior.
   struct {
 /// Whether this TU should be indexed.
 BackgroundPolicy Background = BackgroundPolicy::Build;
+llvm::Optional External;
   } Index;
 
   /// Style of the codebase.

diff  --git a/clang-tools-extra/clangd/ConfigCompile.cpp 
b/clang-tools-extra/clangd/ConfigCompile.cpp
index 3f4dcd3c036d..846c6a170b38 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -27,11 +27,19 @@
 #include "Config.h"
 #include "ConfigFragment.h"
 #include "ConfigProvider.h"
+#include "Features.inc"
 #include "support/Logger.h"
 #include "support/Trace.h"
+#include "llvm/ADT/None.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/STLExtras.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/Format.h"
+#include "llvm/Support/FormatVariadic.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/Regex.h"
 #include "llvm/Support/SMLoc.h"
@@ -102,6 +110,27 @@ struct FragmentCompiler {
 return Result;
   }
 
+  llvm::Optional makeAbsolute(Located Path,
+   llvm::StringLiteral Description,
+   llvm::sys::path::Style Style) {
+if (llvm::sys::path::is_absolute(*Path))
+  return *Path;
+if (FragmentDirectory.empty()) {
+  diag(Error,
+   llvm::formatv(
+   "{0} must be an absolute path, because this fragment is not "
+   "associated with any directory.",
+   Description)
+   .str(),
+   Path.Range);
+  return llvm::None;
+}
+llvm::SmallString<256> AbsPath = llvm::StringRef(*Path);
+llvm::sys::fs::make_absolute(FragmentDirectory, AbsPath);
+llvm::sys::path::native(AbsPath, Style);
+return AbsPath.str().str();
+  }
+
   // Helper with similar API to StringSwitch, for parsing enum values.
   template  class EnumSwitch {
 FragmentCompiler 
@@ -243,6 +272,59 @@ struct FragmentCompiler {
 Out.Apply.push_back(
 [Val](const Params &, Config ) { C.Index.Background = *Val; });
 }
+if (F.External)
+  compile(std::move(**F.External), F.External->Range);
+  }
+
+  void compile(Fragment::IndexBlock::ExternalBlock &,
+   llvm::SMRange BlockRange) {
+#ifndef CLANGD_ENABLE_REMOTE
+if (External.Server) {
+  diag(Error, "Clangd isn't compiled with remote index support, ignoring "
+  "Server." External.Server->Range);
+  External.Server.reset();
+}
+#endif
+// Make sure exactly one of the Sources is set.
+unsigned SourceCount =
+External.File.hasValue() + External.Server.hasValue();
+if (SourceCount != 1) {
+  diag(Error, "Exactly one of File or Server must be set.", BlockRange);
+  return;
+}

[clang-tools-extra] 359e2f9 - [clangd] Introduce config parsing for External blocks

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-11-22T20:59:37+01:00
New Revision: 359e2f988dc560d519c91d3ee96a2ea99983f5d4

URL: 
https://github.com/llvm/llvm-project/commit/359e2f988dc560d519c91d3ee96a2ea99983f5d4
DIFF: 
https://github.com/llvm/llvm-project/commit/359e2f988dc560d519c91d3ee96a2ea99983f5d4.diff

LOG: [clangd] Introduce config parsing for External blocks

Enable configuration of remote and static indexes through config files
in addition to command line arguments.

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

Added: 


Modified: 
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ConfigFragment.h 
b/clang-tools-extra/clangd/ConfigFragment.h
index 766463e11e25..0e4ce638fc72 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -162,6 +162,22 @@ struct Fragment {
 /// This is checked for translation units only, not headers they include.
 /// Legal values are "Build" or "Skip".
 llvm::Optional> Background;
+/// An external index uses data source outside of clangd itself. This is
+/// usually prepared using clangd-indexer.
+/// Exactly one source (File/Server) should be configured.
+struct ExternalBlock {
+  /// Path to an index file generated by clangd-indexer. Relative paths may
+  /// be used, if config fragment is associated with a directory.
+  llvm::Optional> File;
+  /// Address and port number for a clangd-index-server. e.g.
+  /// `123.1.1.1:13337`.
+  llvm::Optional> Server;
+  /// Source root governed by this index. Default is the directory
+  /// associated with the config fragment. Absolute in case of user config
+  /// and relative otherwise. Should always use forward-slashes.
+  llvm::Optional> MountPoint;
+};
+llvm::Optional> External;
   };
   IndexBlock Index;
 

diff  --git a/clang-tools-extra/clangd/ConfigYAML.cpp 
b/clang-tools-extra/clangd/ConfigYAML.cpp
index 742abb42670f..a3bb1eb29adb 100644
--- a/clang-tools-extra/clangd/ConfigYAML.cpp
+++ b/clang-tools-extra/clangd/ConfigYAML.cpp
@@ -5,7 +5,6 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 
//===--===//
-
 #include "ConfigFragment.h"
 #include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallSet.h"
@@ -111,6 +110,22 @@ class Parser {
 DictParser Dict("Index", this);
 Dict.handle("Background",
 [&](Node ) { F.Background = scalarValue(N, "Background"); });
+Dict.handle("External", [&](Node ) {
+  Fragment::IndexBlock::ExternalBlock External;
+  parse(External, N);
+  F.External.emplace(std::move(External));
+  F.External->Range = N.getSourceRange();
+});
+Dict.parse(N);
+  }
+
+  void parse(Fragment::IndexBlock::ExternalBlock , Node ) {
+DictParser Dict("External", this);
+Dict.handle("File", [&](Node ) { F.File = scalarValue(N, "File"); });
+Dict.handle("Server",
+[&](Node ) { F.Server = scalarValue(N, "Server"); });
+Dict.handle("MountPoint",
+[&](Node ) { F.MountPoint = scalarValue(N, "MountPoint"); });
 Dict.parse(N);
   }
 

diff  --git a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp 
b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
index 9cdfdf9657d3..1da0f3a1cc71 100644
--- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -10,6 +10,7 @@
 #include "ConfigFragment.h"
 #include "ConfigTesting.h"
 #include "Protocol.h"
+#include "llvm/ADT/StringRef.h"
 #include "llvm/Support/SMLoc.h"
 #include "llvm/Support/ScopedPrinter.h"
 #include "llvm/Support/SourceMgr.h"
@@ -142,6 +143,25 @@ horrible
   ASSERT_THAT(Results, IsEmpty());
 }
 
+TEST(ParseYAML, ExternalBlock) {
+  CapturedDiags Diags;
+  Annotations YAML(R"yaml(
+Index:
+  External:
+File: "foo"
+Server: ^"bar"
+MountPoint: "baz"
+  )yaml");
+  auto Results =
+  Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback());
+  ASSERT_EQ(Results.size(), 1u);
+  ASSERT_TRUE(Results[0].Index.External);
+  EXPECT_THAT(*Results[0].Index.External.getValue()->File, Val("foo"));
+  EXPECT_THAT(*Results[0].Index.External.getValue()->MountPoint, Val("baz"));
+  ASSERT_THAT(Diags.Diagnostics, IsEmpty());
+  EXPECT_THAT(*Results[0].Index.External.getValue()->Server, Val("bar"));
+}
+
 } // namespace
 } // namespace config
 } // namespace clangd



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

[PATCH] D67112: [Sema] Introduce function reference conversion, NFC

[Aaron Puchert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG825f80e111f2: [Sema] Introduce function reference 
conversion, NFC (authored by aaronpuchert).

Changed prior to commit:
  https://reviews.llvm.org/D67112?vs=246862=306929#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67112

Files:
  clang/include/clang/AST/OperationKinds.def
  clang/include/clang/Sema/Initialization.h
  clang/lib/Sema/SemaInit.cpp
  clang/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p4-ast.cpp
  clang/test/CodeGenCXX/implicit-function-conversion.cpp

Index: clang/test/CodeGenCXX/implicit-function-conversion.cpp
===
--- clang/test/CodeGenCXX/implicit-function-conversion.cpp
+++ /dev/null
@@ -1,7 +0,0 @@
-// RUN: %clang_cc1 -emit-llvm %s -o - -triple=x86_64-unknown-linux -std=c++17 | FileCheck %s
-
-double a(double) noexcept;
-int b(double (&)(double));
-
-// CHECK: call i32 @_Z1bRFddE(double (double)* nonnull @_Z1ad)
-int c = b(a);
Index: clang/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p4-ast.cpp
===
--- /dev/null
+++ clang/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p4-ast.cpp
@@ -0,0 +1,18 @@
+// RUN: %clang_cc1 -std=c++17 -ast-dump %s | FileCheck %s
+
+void f() noexcept;
+
+// CHECK: VarDecl {{.*}} ref 'void (&)()' cinit
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void ()':'void ()' lvalue 
+// CHECK-NEXT: DeclRefExpr {{.*}} 'void () noexcept' lvalue Function {{.*}} 'f' 'void () noexcept'
+void ()() = f;
+
+struct X {
+  typedef void ()() noexcept;
+  operator ref();
+} x;
+
+// CHECK: VarDecl {{.*}} xp 'void (&)()' cinit
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void ()':'void ()' lvalue 
+// CHECK-NEXT: ImplicitCastExpr {{.*}} 'void () noexcept':'void () noexcept' lvalue 
+void ()() = x;
Index: clang/lib/Sema/SemaInit.cpp
===
--- clang/lib/Sema/SemaInit.cpp
+++ clang/lib/Sema/SemaInit.cpp
@@ -3442,6 +3442,7 @@
   case SK_QualificationConversionRValue:
   case SK_QualificationConversionXValue:
   case SK_QualificationConversionLValue:
+  case SK_FunctionReferenceConversion:
   case SK_AtomicConversion:
   case SK_ListInitialization:
   case SK_UnwrapInitList:
@@ -3620,6 +3621,13 @@
   Steps.push_back(S);
 }
 
+void InitializationSequence::AddFunctionReferenceConversionStep(QualType Ty) {
+  Step S;
+  S.Kind = SK_FunctionReferenceConversion;
+  S.Type = Ty;
+  Steps.push_back(S);
+}
+
 void InitializationSequence::AddAtomicConversionStep(QualType Ty) {
   Step S;
   S.Kind = SK_AtomicConversion;
@@ -4653,7 +4661,7 @@
   else if (RefConv & Sema::ReferenceConversions::ObjC)
 Sequence.AddObjCObjectConversionStep(cv1T1);
   else if (RefConv & Sema::ReferenceConversions::Function)
-Sequence.AddQualificationConversionStep(cv1T1, VK);
+Sequence.AddFunctionReferenceConversionStep(cv1T1);
   else if (RefConv & Sema::ReferenceConversions::Qualification) {
 if (!S.Context.hasSameType(cv1T4, cv1T1))
   Sequence.AddQualificationConversionStep(cv1T1, VK);
@@ -4755,12 +4763,12 @@
   Sequence.AddDerivedToBaseCastStep(cv1T1, VK_LValue);
 else
   Sequence.AddObjCObjectConversionStep(cv1T1);
-  } else if (RefConv & (Sema::ReferenceConversions::Qualification |
-Sema::ReferenceConversions::Function)) {
+  } else if (RefConv & Sema::ReferenceConversions::Qualification) {
 // Perform a (possibly multi-level) qualification conversion.
-// FIXME: Should we use a different step kind for function conversions?
 Sequence.AddQualificationConversionStep(cv1T1,
 Initializer->getValueKind());
+  } else if (RefConv & Sema::ReferenceConversions::Function) {
+Sequence.AddFunctionReferenceConversionStep(cv1T1);
   }
 
   // We only create a temporary here when binding a reference to a
@@ -8038,6 +8046,7 @@
   case SK_QualificationConversionLValue:
   case SK_QualificationConversionXValue:
   case SK_QualificationConversionRValue:
+  case SK_FunctionReferenceConversion:
   case SK_AtomicConversion:
   case SK_ConversionSequence:
   case SK_ConversionSequenceNoNarrowing:
@@ -8303,6 +8312,13 @@
   break;
 }
 
+case SK_FunctionReferenceConversion:
+  assert(CurInit.get()->isLValue() &&
+ "function reference should be lvalue");
+  CurInit =
+  S.ImpCastExprToType(CurInit.get(), Step->Type, CK_NoOp, VK_LValue);
+  break;
+
 case SK_AtomicConversion: {
   assert(CurInit.get()->isRValue() && "cannot convert glvalue to atomic");
   CurInit = S.ImpCastExprToType(CurInit.get(), Step->Type,
@@ -9563,6 +9579,10 @@
   OS << "qualification conversion (lvalue)";
   break;
 
+case SK_FunctionReferenceConversion:
+  OS << "function reference 

[clang] 825f80e - [Sema] Introduce function reference conversion, NFC

[Aaron Puchert via cfe-commits]

Author: Aaron Puchert
Date: 2020-11-22T20:51:57+01:00
New Revision: 825f80e111f2815a009084f65267be3b5bf0897a

URL: 
https://github.com/llvm/llvm-project/commit/825f80e111f2815a009084f65267be3b5bf0897a
DIFF: 
https://github.com/llvm/llvm-project/commit/825f80e111f2815a009084f65267be3b5bf0897a.diff

LOG: [Sema] Introduce function reference conversion, NFC

Technically 'noexcept' isn't a qualifier, so this should be a separate 
conversion.

Also make the test a pure frontend test.

Reviewed By: rsmith

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

Added: 
clang/test/CXX/dcl.decl/dcl.init/dcl.init.ref/p4-ast.cpp

Modified: 
clang/include/clang/AST/OperationKinds.def
clang/include/clang/Sema/Initialization.h
clang/lib/Sema/SemaInit.cpp

Removed: 
clang/test/CodeGenCXX/implicit-function-conversion.cpp



diff  --git a/clang/include/clang/AST/OperationKinds.def 
b/clang/include/clang/AST/OperationKinds.def
index 6daab1ffcb0a..7c82ab6e57ef 100644
--- a/clang/include/clang/AST/OperationKinds.def
+++ b/clang/include/clang/AST/OperationKinds.def
@@ -77,9 +77,10 @@ CAST_OPERATION(LValueToRValueBitCast)
 CAST_OPERATION(LValueToRValue)
 
 /// CK_NoOp - A conversion which does not affect the type other than
-/// (possibly) adding qualifiers.
+/// (possibly) adding qualifiers or removing noexcept.
 ///   int-> int
 ///   char** -> const char * const *
+///   void () noexcept -> void ()
 CAST_OPERATION(NoOp)
 
 /// CK_BaseToDerived - A conversion from a C++ class pointer/reference

diff  --git a/clang/include/clang/Sema/Initialization.h 
b/clang/include/clang/Sema/Initialization.h
index 6976e7c95c8b..2245c1505001 100644
--- a/clang/include/clang/Sema/Initialization.h
+++ b/clang/include/clang/Sema/Initialization.h
@@ -840,6 +840,9 @@ class InitializationSequence {
 /// Perform a qualification conversion, producing an lvalue.
 SK_QualificationConversionLValue,
 
+/// Perform a function reference conversion, see [dcl.init.ref]p4.
+SK_FunctionReferenceConversion,
+
 /// Perform a conversion adding _Atomic to a type.
 SK_AtomicConversion,
 
@@ -1288,6 +1291,10 @@ class InitializationSequence {
   void AddQualificationConversionStep(QualType Ty,
  ExprValueKind Category);
 
+  /// Add a new step that performs a function reference conversion to the
+  /// given type.
+  void AddFunctionReferenceConversionStep(QualType Ty);
+
   /// Add a new step that performs conversion from non-atomic to atomic
   /// type.
   void AddAtomicConversionStep(QualType Ty);

diff  --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index 5131ce446d04..6d2e6094e79c 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -3442,6 +3442,7 @@ void InitializationSequence::Step::Destroy() {
   case SK_QualificationConversionRValue:
   case SK_QualificationConversionXValue:
   case SK_QualificationConversionLValue:
+  case SK_FunctionReferenceConversion:
   case SK_AtomicConversion:
   case SK_ListInitialization:
   case SK_UnwrapInitList:
@@ -3620,6 +3621,13 @@ void 
InitializationSequence::AddQualificationConversionStep(QualType Ty,
   Steps.push_back(S);
 }
 
+void InitializationSequence::AddFunctionReferenceConversionStep(QualType Ty) {
+  Step S;
+  S.Kind = SK_FunctionReferenceConversion;
+  S.Type = Ty;
+  Steps.push_back(S);
+}
+
 void InitializationSequence::AddAtomicConversionStep(QualType Ty) {
   Step S;
   S.Kind = SK_AtomicConversion;
@@ -4653,7 +4661,7 @@ static OverloadingResult TryRefInitWithConversionFunction(
   else if (RefConv & Sema::ReferenceConversions::ObjC)
 Sequence.AddObjCObjectConversionStep(cv1T1);
   else if (RefConv & Sema::ReferenceConversions::Function)
-Sequence.AddQualificationConversionStep(cv1T1, VK);
+Sequence.AddFunctionReferenceConversionStep(cv1T1);
   else if (RefConv & Sema::ReferenceConversions::Qualification) {
 if (!S.Context.hasSameType(cv1T4, cv1T1))
   Sequence.AddQualificationConversionStep(cv1T1, VK);
@@ -4755,12 +4763,12 @@ static void TryReferenceInitializationCore(Sema ,
   Sequence.AddDerivedToBaseCastStep(cv1T1, VK_LValue);
 else
   Sequence.AddObjCObjectConversionStep(cv1T1);
-  } else if (RefConv & (Sema::ReferenceConversions::Qualification |
-Sema::ReferenceConversions::Function)) {
+  } else if (RefConv & Sema::ReferenceConversions::Qualification) {
 // Perform a (possibly multi-level) qualification conversion.
-// FIXME: Should we use a 
diff erent step kind for function conversions?
 Sequence.AddQualificationConversionStep(cv1T1,
 Initializer->getValueKind());
+  } else if (RefConv & Sema::ReferenceConversions::Function) {
+Sequence.AddFunctionReferenceConversionStep(cv1T1);
   }
 
   // We only create a temporary 

[PATCH] D91859: [clangd] Fix shared-lib builds

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

In D91859#2410166 , @kadircet wrote:

> In D91859#2409743 , @nridge wrote:
>
>> I applied the patch locally and it fixes most of the linker errors but I'm 
>> still seeing one:
>>
>>   [6/393] Linking CXX executable bin/clangd-index-server
>>   FAILED: bin/clangd-index-server
>>   : && /usr/bin/clang++-10 -fPIC -fvisibility-inlines-hidden 
>> -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra 
>> -Wno-unused-parameter -Wwrite-strings -Wcast-qual 
>> -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough 
>> -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
>> -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color 
>> -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual 
>> -Wno-nested-anon-types -O3 -DNDEBUG -fuse-ld=gold -Wl,-O3 
>> -Wl,--gc-sections 
>> tools/clang/tools/extra/clangd/index/remote/server/CMakeFiles/clangd-index-server.dir/Server.cpp.o
>>  -o bin/clangd-index-server  -Wl,-rpath,"\$ORIGIN/../lib"  -lpthread  
>> lib/libRemoteIndexServiceProto.so.12git  
>> lib/libclangdRemoteMarshalling.so.12git  -lgrpc++  
>> lib/libclangDaemon.so.12git  lib/libclangdSupport.so.12git  
>> lib/libRemoteIndexProto.so.12git  lib/libLLVMSupport.so.12git  
>> -Wl,-rpath-link,llvm/prod-build/lib && :
>>   
>> tools/clang/tools/extra/clangd/index/remote/server/CMakeFiles/clangd-index-server.dir/Server.cpp.o:Server.cpp:function
>>  
>> llvm::detail::stream_operator_format_adapter>  namespace)::RemoteIndexServer::TextProto>::format(llvm::raw_ostream&, 
>> llvm::StringRef): error: undefined reference to 
>> 'google::protobuf::Message::DebugString[abi:cxx11]() const'
>>   clang: error: linker command failed with exit code 1 (use -v to see 
>> invocation)
>
> I am not able to reproduce the failure. Maybe we should just make the 
> dependencies introduced in `generate_protos` for grpc++ and protobuf PUBLIC, 
> to ensure they propagate into users (i believe that's the default for my 
> configuration for whatever reason). Can you share your cmake configuration ?

I agree that would be a good thing but since `Server.cpp` explicitly uses 
`DebugString` I think explicitly specifying `protobuf` dependency there is 
still a way to go.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91859

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

[PATCH] D91915: [clang-tidy] Fix RenamerClangTidy checks trying to emit a fix that isnt a valid identifier

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:525
+return "; cannot be fixed because '" + Fixup +
+   "' is not a valid identifier";
 

njames93 wrote:
> gribozavr2 wrote:
> > njames93 wrote:
> > > gribozavr2 wrote:
> > > > I don't think we need to tell that to the user. When Clang can't 
> > > > provide a fix, it just silently omits it. It does not help the user to 
> > > > know. that Clang tried, but failed.
> > > > 
> > > > (This message could be also read as "this code is so bad it can't be 
> > > > fixed"...)
> > > So just return an empty string.
> > > The user will see the normal diagnostic message but there will be no 
> > > fix-its available.
> > Agreed.
> Maybe a slightly better idea is to use the same message as the empty fix up 
> case
> `; cannot be fixed automatically`. WDYT?
> 
Oh I see, this checker has a number of messages like that... Well, feel free to 
keep it, but what seems weird to me as a user is that for an input name of 
"_0Bad" the checker automatically chose somehow "0_Bad" (why?), and then 
concluded that it is not a good choice. This looks like irrelevant information 
because the new name is weird. I can totally see how reminding the user that 
"Break"->"break" is not easily possible because the latter is a keyword.

Feel free to ignore me since this checker already produces messages like this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91915

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

[PATCH] D91859: [clangd] Fix shared-lib builds

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

In D91859#2409743 , @nridge wrote:

> I applied the patch locally and it fixes most of the linker errors but I'm 
> still seeing one:
>
>   [6/393] Linking CXX executable bin/clangd-index-server
>   FAILED: bin/clangd-index-server
>   : && /usr/bin/clang++-10 -fPIC -fvisibility-inlines-hidden 
> -Werror=date-time -Werror=unguarded-availability-new -Wall -Wextra 
> -Wno-unused-parameter -Wwrite-strings -Wcast-qual 
> -Wmissing-field-initializers -pedantic -Wno-long-long -Wimplicit-fallthrough 
> -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
> -Wdelete-non-virtual-dtor -Wstring-conversion -fdiagnostics-color 
> -ffunction-sections -fdata-sections -fno-common -Woverloaded-virtual 
> -Wno-nested-anon-types -O3 -DNDEBUG -fuse-ld=gold -Wl,-O3 
> -Wl,--gc-sections 
> tools/clang/tools/extra/clangd/index/remote/server/CMakeFiles/clangd-index-server.dir/Server.cpp.o
>  -o bin/clangd-index-server  -Wl,-rpath,"\$ORIGIN/../lib"  -lpthread  
> lib/libRemoteIndexServiceProto.so.12git  
> lib/libclangdRemoteMarshalling.so.12git  -lgrpc++  
> lib/libclangDaemon.so.12git  lib/libclangdSupport.so.12git  
> lib/libRemoteIndexProto.so.12git  lib/libLLVMSupport.so.12git  
> -Wl,-rpath-link,llvm/prod-build/lib && :
>   
> tools/clang/tools/extra/clangd/index/remote/server/CMakeFiles/clangd-index-server.dir/Server.cpp.o:Server.cpp:function
>  
> llvm::detail::stream_operator_format_adapter  namespace)::RemoteIndexServer::TextProto>::format(llvm::raw_ostream&, 
> llvm::StringRef): error: undefined reference to 
> 'google::protobuf::Message::DebugString[abi:cxx11]() const'
>   clang: error: linker command failed with exit code 1 (use -v to see 
> invocation)

I am not able to reproduce the failure. Maybe we should just make the 
dependencies introduced in `generate_protos` for grpc++ and protobuf PUBLIC, to 
ensure they propagate into users (i believe that's the default for my 
configuration for whatever reason). Can you share your cmake configuration ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91859

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

[PATCH] D90750: [clangd] Introduce ProjectAwareIndex

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

- Address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90750

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/index/ProjectAware.cpp
  clang-tools-extra/clangd/index/ProjectAware.h
  clang-tools-extra/clangd/unittests/CMakeLists.txt
  clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp

Index: clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
===
--- /dev/null
+++ clang-tools-extra/clangd/unittests/ProjectAwareIndexTests.cpp
@@ -0,0 +1,86 @@
+//===-- ProjectAwareIndexTests.cpp  ---*- C++ -*---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "Config.h"
+#include "TestIndex.h"
+#include "index/Index.h"
+#include "index/MemIndex.h"
+#include "index/ProjectAware.h"
+#include "index/Ref.h"
+#include "index/Relation.h"
+#include "support/Context.h"
+#include "support/Threading.h"
+#include "llvm/ADT/StringRef.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+
+namespace clang {
+namespace clangd {
+using testing::ElementsAre;
+using testing::IsEmpty;
+
+std::unique_ptr createIndex() {
+  std::vector Symbols = {symbol("1")};
+  return std::make_unique(std::move(Symbols), RefSlab(),
+RelationSlab());
+}
+
+TEST(ProjectAware, Test) {
+  IndexFactory Gen = [](const Config::ExternalIndexSpec &, AsyncTaskRunner &) {
+return createIndex();
+  };
+
+  auto Idx = createProjectAwareIndex(std::move(Gen));
+  FuzzyFindRequest Req;
+  Req.Query = "1";
+  Req.AnyScope = true;
+
+  EXPECT_THAT(match(*Idx, Req), IsEmpty());
+
+  Config C;
+  C.Index.External.emplace();
+  C.Index.External->Location = "test";
+  WithContextValue With(Config::Key, std::move(C));
+  EXPECT_THAT(match(*Idx, Req), ElementsAre("1"));
+  return;
+}
+
+TEST(ProjectAware, CreatedOnce) {
+  unsigned InvocationCount = 0;
+  IndexFactory Gen = [&](const Config::ExternalIndexSpec &, AsyncTaskRunner &) {
+++InvocationCount;
+return createIndex();
+  };
+
+  auto Idx = createProjectAwareIndex(std::move(Gen));
+  // No invocation at start.
+  EXPECT_EQ(InvocationCount, 0U);
+  FuzzyFindRequest Req;
+  Req.Query = "1";
+  Req.AnyScope = true;
+
+  // Cannot invoke without proper config.
+  match(*Idx, Req);
+  EXPECT_EQ(InvocationCount, 0U);
+
+  Config C;
+  C.Index.External.emplace();
+  C.Index.External->Location = "test";
+  WithContextValue With(Config::Key, std::move(C));
+  match(*Idx, Req);
+  // Now it should be created.
+  EXPECT_EQ(InvocationCount, 1U);
+  match(*Idx, Req);
+  // It is cached afterwards.
+  EXPECT_EQ(InvocationCount, 1U);
+  return;
+}
+} // namespace clangd
+} // namespace clang
Index: clang-tools-extra/clangd/unittests/CMakeLists.txt
===
--- clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -73,6 +73,7 @@
   PathMappingTests.cpp
   PreambleTests.cpp
   PrintASTTests.cpp
+  ProjectAwareIndexTests.cpp
   QualityTests.cpp
   RenameTests.cpp
   RIFFTests.cpp
Index: clang-tools-extra/clangd/index/ProjectAware.h
===
--- /dev/null
+++ clang-tools-extra/clangd/index/ProjectAware.h
@@ -0,0 +1,34 @@
+//===--- ProjectAware.h --*- C++-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_PROJECT_AWARE_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_INDEX_PROJECT_AWARE_H
+
+#include "Config.h"
+#include "index/Index.h"
+#include "support/Threading.h"
+#include 
+#include 
+namespace clang {
+namespace clangd {
+
+/// A functor to create an index for an external index specification. Functor
+/// should perform any high latency operation in a separate thread through
+/// AsyncTaskRunner.
+using IndexFactory = std::function(
+const Config::ExternalIndexSpec &, AsyncTaskRunner &)>;
+
+/// Returns an index that answers queries using external indices. IndexGenerator
+/// can be used to customize how to generate an index from an external source.
+/// The default implementation loads the index asynchronously on the
+/// 

[PATCH] D91747: [Clang] Add __STDCPP_THREADS__ to standard predefine macros

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91747

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

[PATCH] D91915: [clang-tidy] Fix RenamerClangTidy checks trying to emit a fix that isnt a valid identifier

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:525
+return "; cannot be fixed because '" + Fixup +
+   "' is not a valid identifier";
 

gribozavr2 wrote:
> njames93 wrote:
> > gribozavr2 wrote:
> > > I don't think we need to tell that to the user. When Clang can't provide 
> > > a fix, it just silently omits it. It does not help the user to know. that 
> > > Clang tried, but failed.
> > > 
> > > (This message could be also read as "this code is so bad it can't be 
> > > fixed"...)
> > So just return an empty string.
> > The user will see the normal diagnostic message but there will be no 
> > fix-its available.
> Agreed.
Maybe a slightly better idea is to use the same message as the empty fix up case
`; cannot be fixed automatically`. WDYT?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91915

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

[PATCH] D91915: [clang-tidy] Fix RenamerClangTidy checks trying to emit a fix that isnt a valid identifier

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr2 added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:525
+return "; cannot be fixed because '" + Fixup +
+   "' is not a valid identifier";
 

njames93 wrote:
> gribozavr2 wrote:
> > I don't think we need to tell that to the user. When Clang can't provide a 
> > fix, it just silently omits it. It does not help the user to know. that 
> > Clang tried, but failed.
> > 
> > (This message could be also read as "this code is so bad it can't be 
> > fixed"...)
> So just return an empty string.
> The user will see the normal diagnostic message but there will be no fix-its 
> available.
Agreed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91915

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

[PATCH] D91915: [clang-tidy] Fix RenamerClangTidy checks trying to emit a fix that isnt a valid identifier

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:525
+return "; cannot be fixed because '" + Fixup +
+   "' is not a valid identifier";
 

gribozavr2 wrote:
> I don't think we need to tell that to the user. When Clang can't provide a 
> fix, it just silently omits it. It does not help the user to know. that Clang 
> tried, but failed.
> 
> (This message could be also read as "this code is so bad it can't be 
> fixed"...)
So just return an empty string.
The user will see the normal diagnostic message but there will be no fix-its 
available.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91915

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

[PATCH] D91915: [clang-tidy] Fix RenamerClangTidy checks trying to emit a fix that isnt a valid identifier

[Dmitri Gribenko via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:525
+return "; cannot be fixed because '" + Fixup +
+   "' is not a valid identifier";
 

I don't think we need to tell that to the user. When Clang can't provide a fix, 
it just silently omits it. It does not help the user to know. that Clang tried, 
but failed.

(This message could be also read as "this code is so bad it can't be fixed"...)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91915

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

[PATCH] D91930: [clangd] Implement textDocument/codeLens

[WangWei via Phabricator via cfe-commits]

lightmelodies updated this revision to Diff 306916.

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

https://reviews.llvm.org/D91930

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1966,6 +1966,83 @@
 URIForFile::canonicalize(testPath("bar.h"), "")})));
 }
 
+CodeLens lens(const URIForFile , const CodeLensType ,
+  const Range ) {
+  return CodeLens{Range, None, CodeLensResolveData{Type, {URI, Range}}};
+}
+
+CodeLens lens(const URIForFile , const std::string ,
+  const CodeLensType , const Range ) {
+  Command Cmd;
+  Cmd.title = Title;
+  Cmd.command = std::string(ExecuteCommandParams::CLANGD_CLIENT_CODELENS);
+  Cmd.codeLens = CodeLensResolveData{Type, {URI, Range}};
+  return CodeLens{Range, Cmd, None};
+}
+
+TEST(CodeLensTest, Reference) {
+  Annotations Main(R"cpp(
+class $X[[X]] {
+};
+int $main[[main]]() {
+  X x;
+}
+  )cpp");
+  TestTU TU;
+  TU.Code = std::string(Main.code());
+  auto AST = TU.build();
+  auto Path = URIForFile::canonicalize(testPath(TU.Filename), testRoot());
+  if (auto CodeLens1 =
+  getDocumentCodeLens(AST, nullptr, testPath(TU.Filename))) {
+EXPECT_THAT(
+*CodeLens1,
+ElementsAre(lens(Path, CodeLensType::Reference, Main.range("X")),
+lens(Path, CodeLensType::Reference, Main.range("main";
+std::vector CodeLens2;
+for (auto & : *CodeLens1) {
+  CodeLens2.emplace_back(
+  *resolveCodeLens(AST, CD, 0, nullptr, testPath(TU.Filename)));
+}
+EXPECT_THAT(
+CodeLens2,
+ElementsAre(
+lens(Path, "2 refs", CodeLensType::Reference, Main.range("X")),
+lens(Path, "1 refs", CodeLensType::Reference, Main.range("main";
+  }
+}
+
+TEST(CodeLensTest, Inheritance) {
+  Annotations Main(R"cpp(
+class $X[[X]] {
+public:
+  virtual void $m1[[method]]();
+};
+
+class $Y[[Y]] : public X {
+public:
+  void $m2[[method]]() override;
+};
+  )cpp");
+  TestTU TU;
+  TU.Code = std::string(Main.code());
+  auto AST = TU.build();
+  auto URI = URIForFile::canonicalize(testPath(TU.Filename), testRoot());
+  if (auto CodeLens =
+  getDocumentCodeLens(AST, TU.index().get(), testPath(TU.Filename))) {
+EXPECT_THAT(
+*CodeLens,
+ElementsAre(
+lens(URI, CodeLensType::Reference, Main.range("X")),
+lens(URI, "1 derived", CodeLensType::TypeChildren, Main.range("X")),
+lens(URI, CodeLensType::Reference, Main.range("m1")),
+lens(URI, "1 derived", CodeLensType::MemberChildren,
+ Main.range("m1")),
+lens(URI, CodeLensType::Reference, Main.range("Y")),
+lens(URI, "1 base", CodeLensType::TypeParent, Main.range("Y")),
+lens(URI, CodeLensType::Reference, Main.range("m2")),
+lens(URI, "1 base", CodeLensType::MemberParent, Main.range("m2";
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -330,6 +330,11 @@
 Hidden,
 };
 
+opt EnableCodeLens{
+"code-lens", cat(Features), desc("Enable preview of CodeLens feature"),
+init(false), Hidden,
+};
+
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -762,6 +767,7 @@
   Opts.BuildRecoveryAST = RecoveryAST;
   Opts.PreserveRecoveryASTType = RecoveryASTType;
   Opts.FoldingRanges = FoldingRanges;
+  Opts.CodeLens = EnableCodeLens;
 
   Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;
   Opts.CodeComplete.Limit = LimitResults;
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -63,7 +63,8 @@
 # CHECK-NEXT:  "executeCommandProvider": {
 # CHECK-NEXT:"commands": [
 # CHECK-NEXT:  "clangd.applyFix",
-# CHECK-NEXT:  "clangd.applyTweak"
+# CHECK-NEXT:  "clangd.applyTweak",
+# CHECK-NEXT:  

[PATCH] D91930: [clangd] Implement textDocument/codeLens

[WangWei via Phabricator via cfe-commits]

lightmelodies updated this revision to Diff 306912.
lightmelodies edited the summary of this revision.

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

https://reviews.llvm.org/D91930

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1966,6 +1966,80 @@
 URIForFile::canonicalize(testPath("bar.h"), "")})));
 }
 
+CodeLens Lens(const URIForFile , const CodeLensType ,
+  const Range ) {
+  return CodeLens{Range, None, CodeLensResolveData{Type, {URI, Range}}};
+}
+
+CodeLens Lens(const URIForFile , const std::string ,
+  const CodeLensType , const Range ) {
+  Command Cmd;
+  Cmd.title = Title;
+  Cmd.command = std::string(ExecuteCommandParams::CLANGD_CLIENT_CODELENS);
+  Cmd.codeLens = CodeLensResolveData{Type, {URI, Range}};
+  return CodeLens{Range, Cmd, None};
+}
+
+TEST(CodeLensTest, Reference) {
+  Annotations Main(R"cpp(
+class $X[[X]] {
+};
+int $main[[main]]() {
+  X x;
+}
+  )cpp");
+  TestTU TU;
+  TU.Code = std::string(Main.code());
+  auto AST = TU.build();
+  auto Path =
+  URIForFile::canonicalize(testPath(TU.Filename), testPath(TU.Filename));
+  auto CodeLens1 = *getDocumentCodeLens(AST, nullptr, testPath(TU.Filename));
+  EXPECT_THAT(
+  CodeLens1,
+  ElementsAre(Lens(Path, CodeLensType::Reference, Main.range("X")),
+  Lens(Path, CodeLensType::Reference, Main.range("main";
+  std::vector CodeLens2;
+  for (auto & : CodeLens1) {
+CodeLens2.emplace_back(
+*resolveCodeLens(AST, CD, 0, nullptr, testPath(TU.Filename)));
+  }
+  EXPECT_THAT(
+  CodeLens2,
+  ElementsAre(
+  Lens(Path, "2 refs", CodeLensType::Reference, Main.range("X")),
+  Lens(Path, "1 refs", CodeLensType::Reference, Main.range("main";
+}
+
+TEST(CodeLensTest, Inheritance) {
+  Annotations Main(R"cpp(
+class $X[[X]] {
+public:
+  virtual void $m1[[method]]();
+};
+
+class $Y[[Y]] : public X {
+public:
+  void $m2[[method]]() override;
+};
+  )cpp");
+  TestTU TU;
+  TU.Code = std::string(Main.code());
+  auto AST = TU.build();
+  auto URI =
+  URIForFile::canonicalize(testPath(TU.Filename), testPath(TU.Filename));
+  EXPECT_THAT(
+  *getDocumentCodeLens(AST, TU.index().get(), testPath(TU.Filename)),
+  ElementsAre(
+  Lens(URI, CodeLensType::Reference, Main.range("X")),
+  Lens(URI, "1 derived", CodeLensType::TypeChildren, Main.range("X")),
+  Lens(URI, CodeLensType::Reference, Main.range("m1")),
+  Lens(URI, "1 derived", CodeLensType::MemberChildren,
+   Main.range("m1")),
+  Lens(URI, CodeLensType::Reference, Main.range("Y")),
+  Lens(URI, "1 base", CodeLensType::TypeParent, Main.range("Y")),
+  Lens(URI, CodeLensType::Reference, Main.range("m2")),
+  Lens(URI, "1 base", CodeLensType::MemberParent, Main.range("m2";
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -330,6 +330,11 @@
 Hidden,
 };
 
+opt EnableCodeLens{
+"code-lens", cat(Features), desc("Enable preview of CodeLens feature"),
+init(false), Hidden,
+};
+
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -762,6 +767,7 @@
   Opts.BuildRecoveryAST = RecoveryAST;
   Opts.PreserveRecoveryASTType = RecoveryASTType;
   Opts.FoldingRanges = FoldingRanges;
+  Opts.CodeLens = EnableCodeLens;
 
   Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;
   Opts.CodeComplete.Limit = LimitResults;
Index: clang-tools-extra/clangd/test/initialize-params.test
===
--- clang-tools-extra/clangd/test/initialize-params.test
+++ clang-tools-extra/clangd/test/initialize-params.test
@@ -63,7 +63,8 @@
 # CHECK-NEXT:  "executeCommandProvider": {
 # CHECK-NEXT:"commands": [
 # CHECK-NEXT:  "clangd.applyFix",
-# CHECK-NEXT:  "clangd.applyTweak"
+# CHECK-NEXT:  "clangd.applyTweak",
+# CHECK-NEXT:  "clangd.server.codeLens"
 # CHECK-NEXT:]
 # 

[PATCH] D91930: [clangd] Implement textDocument/codeLens

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

Please upload the diff with full context and fix the failing unit tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91930

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

[PATCH] D91930: [clangd] Implement textDocument/codeLens

[WangWei via Phabricator via cfe-commits]

lightmelodies created this revision.
lightmelodies added reviewers: kadircet, sammccall.
lightmelodies added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, usaxena95, arphaman, javed.absar.
Herald added a project: clang.
lightmelodies requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

This commit adds support for 
https://microsoft.github.io/language-server-protocol/specification#textDocument_codeLens.
See also https://github.com/clangd/clangd/issues/442.
For client usage see 
https://github.com/lightmelodies/vscode-clangd/commit/c00f74d0d2403a4753dab8b8a0a8dbc8bb6ffdf4.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D91930

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/TUScheduler.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/test/initialize-params.test
  clang-tools-extra/clangd/tool/ClangdMain.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -1966,6 +1966,78 @@
 URIForFile::canonicalize(testPath("bar.h"), "")})));
 }
 
+CodeLens Lens(const URIForFile , const CodeLensType ,
+  const Range ) {
+  return CodeLens{Range, None, CodeLensResolveData{Type, {URI, Range}}};
+}
+
+CodeLens Lens(const URIForFile , const std::string ,
+  const CodeLensType , const Range ) {
+  Command Cmd;
+  Cmd.title = Title;
+  Cmd.command = std::string(ExecuteCommandParams::CLANGD_CLIENT_CODELENS);
+  Cmd.codeLens = CodeLensResolveData{Type, {URI, Range}};
+  return CodeLens{Range, Cmd, None};
+}
+
+TEST(CodeLensTest, Reference) {
+  Annotations Main(R"cpp(
+class $X[[X]] {
+};
+int $main[[main]]() {
+  X x;
+}
+  )cpp");
+  TestTU TU;
+  TU.Code = std::string(Main.code());
+  auto AST = TU.build();
+  auto Path = URIForFile::canonicalize(testPath(TU.Filename), "");
+  auto CodeLens1 = *getDocumentCodeLens(AST, nullptr, testPath(TU.Filename));
+  EXPECT_THAT(
+  CodeLens1,
+  ElementsAre(Lens(Path, CodeLensType::Reference, Main.range("X")),
+  Lens(Path, CodeLensType::Reference, Main.range("main";
+  std::vector CodeLens2;
+  for (auto & : CodeLens1) {
+CodeLens2.emplace_back(
+*resolveCodeLens(AST, CD, 0, nullptr, testPath(TU.Filename)));
+  }
+  EXPECT_THAT(
+  CodeLens2,
+  ElementsAre(
+  Lens(Path, "2 refs", CodeLensType::Reference, Main.range("X")),
+  Lens(Path, "1 refs", CodeLensType::Reference, Main.range("main";
+}
+
+TEST(CodeLensTest, Inheritance) {
+  Annotations Main(R"cpp(
+class $X[[X]] {
+public:
+  virtual void $m1[[method]]();
+};
+
+class $Y[[Y]] : public X {
+public:
+  void $m2[[method]]() override;
+};
+  )cpp");
+  TestTU TU;
+  TU.Code = std::string(Main.code());
+  auto AST = TU.build();
+  auto URI = URIForFile::canonicalize(testPath(TU.Filename), "");
+  EXPECT_THAT(
+  *getDocumentCodeLens(AST, TU.index().get(), testPath(TU.Filename)),
+  ElementsAre(
+  Lens(URI, CodeLensType::Reference, Main.range("X")),
+  Lens(URI, "1 derived", CodeLensType::TypeChildren, Main.range("X")),
+  Lens(URI, CodeLensType::Reference, Main.range("m1")),
+  Lens(URI, "1 derived", CodeLensType::MemberChildren,
+   Main.range("m1")),
+  Lens(URI, CodeLensType::Reference, Main.range("Y")),
+  Lens(URI, "1 base", CodeLensType::TypeParent, Main.range("Y")),
+  Lens(URI, CodeLensType::Reference, Main.range("m2")),
+  Lens(URI, "1 base", CodeLensType::MemberParent, Main.range("m2";
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -330,6 +330,11 @@
 Hidden,
 };
 
+opt EnableCodeLens{
+"code-lens", cat(Features), desc("Enable preview of CodeLens feature"),
+init(false), Hidden,
+};
+
 opt WorkerThreadsCount{
 "j",
 cat(Misc),
@@ -762,6 +767,7 @@
   Opts.BuildRecoveryAST = RecoveryAST;
   Opts.PreserveRecoveryASTType = RecoveryASTType;
   Opts.FoldingRanges = FoldingRanges;
+  Opts.CodeLens = EnableCodeLens;
 
   Opts.CodeComplete.IncludeIneligibleResults = IncludeIneligibleResults;
   Opts.CodeComplete.Limit = LimitResults;
Index: clang-tools-extra/clangd/test/initialize-params.test

[PATCH] D90282: [clang-tidy] Add IgnoreShortNames config to identifier naming checks

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:134-137
+if (!IgnoredRegexp.isValid()) {
+  llvm::errs() << "Invalid IgnoredRegexp regular expression: "
+   << IgnoredRegexpStr;
+}

Elide braces on single statement if.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:138-141
+  } else {
+// Store an 'invalid' Regexp if empty string
+IgnoredRegexp = llvm::Regex();
+  }

`IgnoredRegexp` will be default constructed anyway, so this doesn't add 
anything.


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

https://reviews.llvm.org/D90282

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

[PATCH] D91908: [clang-tidy] Use compiled regex for AllowedRegexp in macro usage check

[Nathan James via Phabricator via cfe-commits]

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

Because this is basically NFC and we already have tests for the AllowedRegexp 
option, there is no need to introduce new tests here.
So long as those current tests pass with this change, this should be perfectly 
fine.


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

https://reviews.llvm.org/D91908

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

[PATCH] D91859: [clangd] Fix shared-lib builds

[Kirill Bobyrev via Phabricator via cfe-commits]

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

Ah, right. I think `clangd-remote-index` needs `protobuf` dependency. @nridge, 
I believe this should fix the problem you're seeing. I'm somewhat confused as 
to why I'm not seeing the same problem though but it makes total sense to me.

In D91859#2409419 , @kbobyrev wrote:

> (also please add "Fixes https://github.com/clangd/clangd/issues/598; to the 
> patch description to close the related issue on GH)

Otherwise LGTM + patch description :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91859

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

[PATCH] D91410: [llvm][clang][mlir] Add checks for the return values from Target::createXXX to prevent protential null deref

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1756d67934bb: [llvm][clang][mlir] Add checks for the return 
values from Target::createXXX to… (authored by OikawaKirie, committed by 
MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D91410

Files:
  clang/tools/clang-fuzzer/handle-llvm/handle_llvm.cpp
  clang/tools/driver/cc1as_main.cpp
  llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
  llvm/lib/CodeGen/AsmPrinter/AsmPrinterInlineAsm.cpp
  llvm/lib/CodeGen/LLVMTargetMachine.cpp
  llvm/lib/CodeGen/ParallelCG.cpp
  llvm/lib/LTO/LTOBackend.cpp
  llvm/lib/LTO/LTOCodeGenerator.cpp
  llvm/lib/LTO/LTOModule.cpp
  llvm/lib/LTO/ThinLTOCodeGenerator.cpp
  llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp
  llvm/tools/llvm-exegesis/lib/LlvmState.cpp
  llvm/tools/llvm-exegesis/llvm-exegesis.cpp
  llvm/tools/llvm-mc/llvm-mc.cpp
  llvm/tools/llvm-mca/llvm-mca.cpp
  llvm/tools/llvm-ml/llvm-ml.cpp
  llvm/tools/llvm-objdump/MachODump.cpp
  llvm/tools/llvm-rtdyld/llvm-rtdyld.cpp
  mlir/lib/Conversion/GPUCommon/ConvertKernelFuncToBlob.cpp
  mlir/lib/ExecutionEngine/ExecutionEngine.cpp

Index: mlir/lib/ExecutionEngine/ExecutionEngine.cpp
===
--- mlir/lib/ExecutionEngine/ExecutionEngine.cpp
+++ mlir/lib/ExecutionEngine/ExecutionEngine.cpp
@@ -130,6 +130,10 @@
 
   std::unique_ptr machine(target->createTargetMachine(
   targetTriple, cpu, features.getString(), {}, {}));
+  if (!machine) {
+errs() << "Unable to create target machine\n";
+return true;
+  }
   llvmModule->setDataLayout(machine->createDataLayout());
   llvmModule->setTargetTriple(targetTriple);
   return false;
Index: mlir/lib/Conversion/GPUCommon/ConvertKernelFuncToBlob.cpp
===
--- mlir/lib/Conversion/GPUCommon/ConvertKernelFuncToBlob.cpp
+++ mlir/lib/Conversion/GPUCommon/ConvertKernelFuncToBlob.cpp
@@ -130,6 +130,10 @@
 }
 targetMachine.reset(target->createTargetMachine(triple.str(), targetChip,
 features, {}, {}));
+if (targetMachine == nullptr) {
+  emitError(loc, "connot initialize target machine");
+  return {};
+}
   }
 
   llvmModule.setDataLayout(targetMachine->createDataLayout());
Index: llvm/tools/llvm-rtdyld/llvm-rtdyld.cpp
===
--- llvm/tools/llvm-rtdyld/llvm-rtdyld.cpp
+++ llvm/tools/llvm-rtdyld/llvm-rtdyld.cpp
@@ -764,6 +764,8 @@
 ErrorAndExit("Unable to create disassembler!");
 
   std::unique_ptr MII(TheTarget->createMCInstrInfo());
+  if (!MII)
+ErrorAndExit("Unable to create target instruction info!");
 
   std::unique_ptr InstPrinter(
   TheTarget->createMCInstPrinter(Triple(TripleName), 0, *MAI, *MII, *MRI));
Index: llvm/tools/llvm-objdump/MachODump.cpp
===
--- llvm/tools/llvm-objdump/MachODump.cpp
+++ llvm/tools/llvm-objdump/MachODump.cpp
@@ -7200,10 +7200,32 @@
   else
 MachOMCPU = MCPU;
 
+#define CHECK_TARGET_INFO_CREATION(NAME)   \
+  do { \
+if (!NAME) {   \
+  WithColor::error(errs(), "llvm-objdump") \
+  << "couldn't initialize disassembler for target " << TripleName  \
+  << '\n'; \
+  return;  \
+}  \
+  } while (false)
+#define CHECK_THUMB_TARGET_INFO_CREATION(NAME) \
+  do { \
+if (!NAME) {   \
+  WithColor::error(errs(), "llvm-objdump") \
+  << "couldn't initialize disassembler for target " << ThumbTripleName \
+  << '\n'; \
+  return;  \
+}  \
+  } while (false)
+
   std::unique_ptr InstrInfo(TheTarget->createMCInstrInfo());
+  CHECK_TARGET_INFO_CREATION(InstrInfo);
   std::unique_ptr ThumbInstrInfo;
-  if (ThumbTarget)
+  if (ThumbTarget) {
 ThumbInstrInfo.reset(ThumbTarget->createMCInstrInfo());
+CHECK_THUMB_TARGET_INFO_CREATION(ThumbInstrInfo);
+  }
 
   // Package up features to be passed to target/subtarget
   std::string FeaturesStr;
@@ -7218,13 +7240,17 @@
   // Set up disassembler.
   

[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Douglas Chen via Phabricator via cfe-commits]

dougpuob marked 82 inline comments as done.
dougpuob added a comment.

Hi @aaron.ballman, thank you for your feedback. I will update my change later. 
Unrelated change were mixed with other commits when I against git master. I did 
it again then the problem was gone. I found the command, `arc diff master 
--preview`, knowing exactly changes before uploading diff by arc.




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:254
+  static constexpr std::pair CStrings[] = {
+  {"char*", "sz"}, {"char", "sz"}, {"wchar_t*", "wsz"}, {"wchar_t", 
"wsz"}};
+  for (const auto  : CStrings) {

aaron.ballman wrote:
> One thing that confused me was the plain `char` and `wchar_t` entries -- 
> those are for arrays of `char` and `wchar_t`, aren't they? Can we use 
> `char[]` to make that more clear? If not, can you add a comment to clarify?
I improved it. It will look like the following:
```
static constexpr std::pair CStrings[] = {
  {"char*", "sz"}, {"char[]", "sz"}, {"wchar_t*", "wsz"}, {"wchar_t[]", 
"wsz"}};
```



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:376-380
+  static constexpr std::pair HNCStrings[] = {
+  {"CharPrinter", "char*"},
+  {"CharArray", "char"},
+  {"WideCharPrinter", "wchar_t*"},
+  {"WideCharArray", "wchar_t"}};

aaron.ballman wrote:
> Similar question here as above, but less pressing because we at least have 
> the word "array" nearby.
Improved here too. It will change to:

```
  static constexpr std::pair HNCStrings[] = {
  {"CharPrinter", "char*"},
  {"CharArray", "char[]"},
  {"WideCharPrinter", "wchar_t*"},
  {"WideCharArray", "wchar_t[]"}};
```



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:431
 static IdentifierNamingCheck::FileStyle
-getFileStyleFromOptions(const ClangTidyCheck::OptionsView ) {
+getFileStyleFromOptions(const ClangTidyCheck::OptionsView ,
+IdentifierNamingCheck::HungarianNotationOption ) {

aaron.ballman wrote:
> Formatting
OK. I checked all the range including single-line if statements, and passing 
StringRef directly(not its reference).




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

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

[clang-tools-extra] 82c22f1 - [clangd] Fix compile error after 20b69af7

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-11-22T10:48:48Z
New Revision: 82c22f124816d1f260dc8f0626d56b459d1358b8

URL: 
https://github.com/llvm/llvm-project/commit/82c22f124816d1f260dc8f0626d56b459d1358b8
DIFF: 
https://github.com/llvm/llvm-project/commit/82c22f124816d1f260dc8f0626d56b459d1358b8.diff

LOG: [clangd] Fix compile error after 20b69af7

Some of the buildbots were failing due to what seems to be them using a non 
c++14 compilant std::string implementation.
Since c++14 std::basic_string::append(const basic_string, size_t, size_t) has a 
defaulted 3rd paramater, but some of the build bots were reporting that it 
wasn't defaulted in their implementation.

Added: 


Modified: 
clang-tools-extra/clangd/ConfigCompile.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ConfigCompile.cpp 
b/clang-tools-extra/clangd/ConfigCompile.cpp
index ff031238418d..3f4dcd3c036d 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -292,7 +292,8 @@ struct FragmentCompiler {
   Out.Apply.push_back(
   [Checks = std::move(Checks)](const Params &, Config ) {
 C.ClangTidy.Checks.append(
-Checks, C.ClangTidy.Checks.empty() ? /*skip comma*/ 1 : 0);
+Checks, C.ClangTidy.Checks.empty() ? /*skip comma*/ 1 : 0,
+std::string::npos);
   });
 if (!F.CheckOptions.empty()) {
   std::vector> CheckOptions;



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

[PATCH] D90531: [clangd] Add clang-tidy options to config

[Nathan James via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG20b69af7c9c8: [clangd] Add clang-tidy options to config 
(authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D90531

Files:
  clang-tools-extra/clangd/Config.h
  clang-tools-extra/clangd/ConfigCompile.cpp
  clang-tools-extra/clangd/ConfigFragment.h
  clang-tools-extra/clangd/ConfigYAML.cpp
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp

Index: clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp
@@ -35,6 +35,14 @@
   return false;
 }
 
+MATCHER_P2(PairVal, Value1, Value2, "") {
+  if (*arg.first == Value1 && *arg.second == Value2)
+return true;
+  *result_listener << "values are [" << *arg.first << ", " << *arg.second
+   << "]";
+  return false;
+}
+
 TEST(ParseYAML, SyntacticForms) {
   CapturedDiags Diags;
   const char *YAML = R"yaml(
@@ -50,10 +58,15 @@
 ---
 Index:
   Background: Skip
+---
+ClangTidy: 
+  CheckOptions: 
+IgnoreMacros: true
+example-check.ExampleOption: 0
   )yaml";
   auto Results = Fragment::parseYAML(YAML, "config.yaml", Diags.callback());
   EXPECT_THAT(Diags.Diagnostics, IsEmpty());
-  ASSERT_EQ(Results.size(), 3u);
+  ASSERT_EQ(Results.size(), 4u);
   EXPECT_FALSE(Results[0].If.HasUnrecognizedCondition);
   EXPECT_THAT(Results[0].If.PathMatch, ElementsAre(Val("abc")));
   EXPECT_THAT(Results[0].CompileFlags.Add, ElementsAre(Val("foo"), Val("bar")));
@@ -62,6 +75,9 @@
 
   ASSERT_TRUE(Results[2].Index.Background);
   EXPECT_EQ("Skip", *Results[2].Index.Background.getValue());
+  EXPECT_THAT(Results[3].ClangTidy.CheckOptions,
+  ElementsAre(PairVal("IgnoreMacros", "true"),
+  PairVal("example-check.ExampleOption", "0")));
 }
 
 TEST(ParseYAML, Locations) {
@@ -84,11 +100,11 @@
   CapturedDiags Diags;
   Annotations YAML(R"yaml(
 If:
-  [[UnknownCondition]]: "foo"
+  $unknown[[UnknownCondition]]: "foo"
 CompileFlags:
   Add: 'first'
 ---
-CompileFlags: {^
+CompileFlags: {$unexpected^
 )yaml");
   auto Results =
   Fragment::parseYAML(YAML.code(), "config.yaml", Diags.callback());
@@ -97,11 +113,13 @@
   Diags.Diagnostics,
   ElementsAre(AllOf(DiagMessage("Unknown If key UnknownCondition"),
 DiagKind(llvm::SourceMgr::DK_Warning),
-DiagPos(YAML.range().start), DiagRange(YAML.range())),
+DiagPos(YAML.range("unknown").start),
+DiagRange(YAML.range("unknown"))),
   AllOf(DiagMessage("Unexpected token. Expected Key, Flow "
 "Entry, or Flow Mapping End."),
 DiagKind(llvm::SourceMgr::DK_Error),
-DiagPos(YAML.point()), DiagRange(llvm::None;
+DiagPos(YAML.point("unexpected")),
+DiagRange(llvm::None;
 
   ASSERT_EQ(Results.size(), 1u); // invalid fragment discarded.
   EXPECT_THAT(Results.front().CompileFlags.Add, ElementsAre(Val("first")));
Index: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
===
--- clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
+++ clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
@@ -176,6 +176,26 @@
 ASSERT_THAT(Diags.Diagnostics, IsEmpty());
   }
 }
+
+TEST_F(ConfigCompileTests, Tidy) {
+  Frag.ClangTidy.Add.emplace_back("bugprone-use-after-move");
+  Frag.ClangTidy.Add.emplace_back("llvm-*");
+  Frag.ClangTidy.Remove.emplace_back("llvm-include-order");
+  Frag.ClangTidy.Remove.emplace_back("readability-*");
+  Frag.ClangTidy.CheckOptions.emplace_back(
+  std::make_pair(std::string("StrictMode"), std::string("true")));
+  Frag.ClangTidy.CheckOptions.emplace_back(std::make_pair(
+  std::string("example-check.ExampleOption"), std::string("0")));
+  EXPECT_TRUE(compileAndApply());
+  EXPECT_EQ(
+  Conf.ClangTidy.Checks,
+  "bugprone-use-after-move,llvm-*,-llvm-include-order,-readability-*");
+  EXPECT_EQ(Conf.ClangTidy.CheckOptions.size(), 2U);
+  EXPECT_EQ(Conf.ClangTidy.CheckOptions.lookup("StrictMode"), "true");
+  EXPECT_EQ(Conf.ClangTidy.CheckOptions.lookup("example-check.ExampleOption"),
+"0");
+}
+
 } // namespace
 } // namespace config
 } // namespace clangd
Index: clang-tools-extra/clangd/ConfigYAML.cpp
===
--- clang-tools-extra/clangd/ConfigYAML.cpp
+++ clang-tools-extra/clangd/ConfigYAML.cpp
@@ -40,6 +40,7 @@
 Dict.handle("CompileFlags", [&](Node ) 

[clang-tools-extra] 20b69af - [clangd] Add clang-tidy options to config

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-11-22T10:04:01Z
New Revision: 20b69af7c9c8bd9a621b05203f944bf94a3a4c26

URL: 
https://github.com/llvm/llvm-project/commit/20b69af7c9c8bd9a621b05203f944bf94a3a4c26
DIFF: 
https://github.com/llvm/llvm-project/commit/20b69af7c9c8bd9a621b05203f944bf94a3a4c26.diff

LOG: [clangd] Add clang-tidy options to config

First step of implementing clang-tidy configuration into clangd config.
This is just adding support for reading and verifying the clang tidy options 
from the config fragments.
No support is added for actually using the options within clang-tidy yet.

That will be added in a follow up as its a little more involved.

Reviewed By: sammccall

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

Added: 


Modified: 
clang-tools-extra/clangd/Config.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/ConfigFragment.h
clang-tools-extra/clangd/ConfigYAML.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Config.h 
b/clang-tools-extra/clangd/Config.h
index 087dc4fb805d..ff285d34633b 100644
--- a/clang-tools-extra/clangd/Config.h
+++ b/clang-tools-extra/clangd/Config.h
@@ -26,6 +26,7 @@
 
 #include "support/Context.h"
 #include "llvm/ADT/FunctionExtras.h"
+#include "llvm/ADT/StringMap.h"
 #include 
 #include 
 
@@ -70,6 +71,14 @@ struct Config {
 // ::). All nested namespaces are affected as well.
 std::vector FullyQualifiedNamespaces;
   } Style;
+
+  /// Configures what clang-tidy checks to run and options to use with them.
+  struct {
+// A comma-seperated list of globs to specify which clang-tidy checks to
+// run.
+std::string Checks;
+llvm::StringMap CheckOptions;
+  } ClangTidy;
 };
 
 } // namespace clangd

diff  --git a/clang-tools-extra/clangd/ConfigCompile.cpp 
b/clang-tools-extra/clangd/ConfigCompile.cpp
index e2823894dd11..ff031238418d 100644
--- a/clang-tools-extra/clangd/ConfigCompile.cpp
+++ b/clang-tools-extra/clangd/ConfigCompile.cpp
@@ -157,6 +157,7 @@ struct FragmentCompiler {
 compile(std::move(F.If));
 compile(std::move(F.CompileFlags));
 compile(std::move(F.Index));
+compile(std::move(F.ClangTidy));
   }
 
   void compile(Fragment::IfBlock &) {
@@ -264,6 +265,49 @@ struct FragmentCompiler {
 }
   }
 
+  void appendTidyCheckSpec(std::string ,
+   const Located , bool IsPositive) {
+StringRef Str = *Arg;
+// Don't support negating here, its handled if the item is in the Add or
+// Remove list.
+if (Str.startswith("-") || Str.contains(',')) {
+  diag(Error, "Invalid clang-tidy check name", Arg.Range);
+  return;
+}
+CurSpec += ',';
+if (!IsPositive)
+  CurSpec += '-';
+CurSpec += Str;
+  }
+
+  void compile(Fragment::ClangTidyBlock &) {
+std::string Checks;
+for (auto  : F.Add)
+  appendTidyCheckSpec(Checks, CheckGlob, true);
+
+for (auto  : F.Remove)
+  appendTidyCheckSpec(Checks, CheckGlob, false);
+
+if (!Checks.empty())
+  Out.Apply.push_back(
+  [Checks = std::move(Checks)](const Params &, Config ) {
+C.ClangTidy.Checks.append(
+Checks, C.ClangTidy.Checks.empty() ? /*skip comma*/ 1 : 0);
+  });
+if (!F.CheckOptions.empty()) {
+  std::vector> CheckOptions;
+  for (auto  : F.CheckOptions)
+CheckOptions.emplace_back(std::move(*Opt.first),
+  std::move(*Opt.second));
+  Out.Apply.push_back(
+  [CheckOptions = std::move(CheckOptions)](const Params &, Config ) {
+for (auto  : CheckOptions)
+  C.ClangTidy.CheckOptions.insert_or_assign(StringPair.first,
+StringPair.second);
+  });
+}
+  }
+
   constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error;
   constexpr static llvm::SourceMgr::DiagKind Warning =
   llvm::SourceMgr::DK_Warning;

diff  --git a/clang-tools-extra/clangd/ConfigFragment.h 
b/clang-tools-extra/clangd/ConfigFragment.h
index 65772715095f..766463e11e25 100644
--- a/clang-tools-extra/clangd/ConfigFragment.h
+++ b/clang-tools-extra/clangd/ConfigFragment.h
@@ -174,6 +174,29 @@ struct Fragment {
 std::vector> FullyQualifiedNamespaces;
   };
   StyleBlock Style;
+
+  /// Controls how clang-tidy will run over the code base.
+  ///
+  /// The settings are merged with any settings found in .clang-tidy
+  /// configiration files with these ones taking precedence.
+  struct ClangTidyBlock {
+std::vector> Add;
+/// List of checks to disable.
+/// Takes precedence over Add. To enable all llvm checks except include
+/// order:
+///   Add: llvm-*
+///   Remove: llvm-include-onder
+std::vector> Remove;
+
+/// A Key-Value