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

2020-11-22 Thread 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 &URI, const CodeLensType &Type,
+  const Range &Range) {
+  return CodeLens{Range, None, CodeLensResolveData{Type, {URI, Range}}};
+}
+
+CodeLens Lens(const URIForFile &URI, const std::string &Title,
+  const CodeLensType &Type, const Range &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 &&CD : 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] D91930: [clangd] Implement textDocument/codeLens

2020-11-22 Thread 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 &URI, const CodeLensType &Type,
+  const Range &Range) {
+  return CodeLens{Range, None, CodeLensResolveData{Type, {URI, Range}}};
+}
+
+CodeLens Lens(const URIForFile &URI, const std::string &Title,
+  const CodeLensType &Type, const Range &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 &&CD : 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

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

2020-11-22 Thread 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 &URI, const CodeLensType &Type,
+  const Range &Range) {
+  return CodeLens{Range, None, CodeLensResolveData{Type, {URI, Range}}};
+}
+
+CodeLens lens(const URIForFile &URI, const std::string &Title,
+  const CodeLensType &Type, const Range &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 &&CD : *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",

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

2020-11-23 Thread WangWei via Phabricator via cfe-commits
lightmelodies added a comment.

In D91930#2411338 , @kadircet wrote:

> Thanks a lot for working on improving clangd!
>
> Can you also give a high-level overview of what kind of functionality you are 
> providing here? Looks like there's a lot going on here, and it would be nice 
> to know what you are attempting to do, rather than inferring that from the 
> code. It would also help future travellers, as they can just read the 
> description rather than going through the whole patch.

Thanks for your response. I have updated the summary and add some detailed 
explanation.


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

2020-11-26 Thread WangWei via Phabricator via cfe-commits
lightmelodies added a comment.

I have also considered these problems, and here is some points of my view. 
**Latency**
Consider llvm-project itself, **textDocument/codeLens** can be done in <500ms 
generally and <100ms if collect hierarchy is disabled in my notebook. Latency 
is significant in some rare case like //Type.h// which has too complex 
hierarchy relations. Meanwhile, vscode is smart to remember previous codelens's 
positions so the text rendering will not look too weird during long-time 
request. In fact, the biggest performance issue I find is that building AST 
when open a  new file costs too much time, which makes 
**textDocument/codeLens** quite slow, as well as other action such as  
**textDocument/documentSymbols**. That's why I modified //ClangdLSPServer.cpp// 
to keep AST for recent closed files.

| file| line count | cost| cost (without hierarchy) |
| ASTVector.h | 411| 4ms | 1ms  |
| APValue.h   | 693| 16 ms   | 8ms  |
| Decl.h  | 4599   | 213 ms  | 40 ms|
| Expr.h  | 6383   | 447 ms  | 61 ms|
| Type.h  | 7222   | 3765 ms | 47 ms|
|

**Batch**
That's definitely a problem with remote index since vscode can only do 
resolution per symbol. Maybe it will be better to resolve directly in  
**textDocument/codeLens** requests when deal with remote index. Just need add 
an option to allow users switch the strategy.

**Useless Lens**
Yes, we can see the base directly in class declaration. However c++ has forward 
declaration and type alias, in such case the base can not be seen directly. So 
someone may prefer to have these codelens. Typescript in vscode has options to 
allow users disable specific type of codelens. We can use similar strategy, 
e.g. **-codelens=references,base,derived** instead of **-codelens=true**.

I'm currently working on an implemention by traversing AST directly instead of 
using **textDocument/documentSymbols**, since symbols defined by macro are 
often reported a wrong range. It should save much time of **findNamedDeclAt** 
comparing to current implemention, and make it easier to add additional filters 
on symbols.


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] D96751: [clangd] Populate detail field in document symbols

2021-02-15 Thread WangWei via Phabricator via cfe-commits
lightmelodies created this revision.
lightmelodies added reviewers: sammccall, kadircet.
lightmelodies added a project: clang-tools-extra.
Herald added subscribers: usaxena95, arphaman.
lightmelodies requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.

This commit fix https://github.com/clangd/clangd/issues/520 and 
https://github.com/clangd/clangd/issues/601.
F15544293: image.png 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D96751

Files:
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/Hover.cpp
  clang-tools-extra/clangd/Hover.h
  clang-tools-extra/clangd/test/symbols.test

Index: clang-tools-extra/clangd/test/symbols.test
===
--- clang-tools-extra/clangd/test/symbols.test
+++ clang-tools-extra/clangd/test/symbols.test
@@ -1,5 +1,5 @@
 # RUN: clangd --index-file=%S/Inputs/symbols.test.yaml -lit-test < %s | FileCheck %s
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"workspace":{"symbol":{"symbolKind":{"valueSet": [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26],"trace":"off"}}
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"documentSymbol":{"hierarchicalDocumentSymbolSupport":true}},"workspace":{"symbol":{"symbolKind":{"valueSet": [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26],"trace":"off"}}
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void foo(); int main() { foo(); }\n"}}}
 ---
@@ -34,40 +34,54 @@
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:"result": [
 # CHECK-NEXT:  {
-# CHECK-NEXT:"containerName": "",
+# CHECK-NEXT:"detail": "void foo()",
 # CHECK-NEXT:"kind": 12,
-# CHECK-NEXT:"location": {
-# CHECK-NEXT:  "range": {
-# CHECK-NEXT:"end": {
-# CHECK-NEXT:  "character": {{.*}},
-# CHECK-NEXT:  "line": {{.*}}
-# CHECK-NEXT:},
-# CHECK-NEXT:"start": {
-# CHECK-NEXT:  "character": {{.*}},
-# CHECK-NEXT:  "line": {{.*}}
-# CHECK-NEXT:}
+# CHECK-NEXT:"name": "foo",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file://{{.*}}/main.cpp"
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
+# CHECK-NEXT:  }
 # CHECK-NEXT:},
-# CHECK-NEXT:"name": "foo"
-# CHECK-NEXT:  }
+# CHECK-NEXT:"selectionRange": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
 # CHECK-NEXT:  {
-# CHECK-NEXT:"containerName": "",
+# CHECK-NEXT:"detail": "int main()",
 # CHECK-NEXT:"kind": 12,
-# CHECK-NEXT:"location": {
-# CHECK-NEXT:  "range": {
-# CHECK-NEXT:"end": {
-# CHECK-NEXT:  "character": {{.*}},
-# CHECK-NEXT:  "line": {{.*}}
-# CHECK-NEXT:},
-# CHECK-NEXT:"start": {
-# CHECK-NEXT:  "character": {{.*}},
-# CHECK-NEXT:  "line": {{.*}}
-# CHECK-NEXT:}
+# CHECK-NEXT:"name": "main",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file://{{.*}}/main.cpp"
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
+# CHECK-NEXT:  }
 # CHECK-NEXT:},
-# CHECK-NEXT:"name": "main"
+# CHECK-NEXT:"selectionRange": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
 # CHECK-NEXT:  }
 # CHECK-NEXT:]
 # CHECK-NEXT:}
Index: clang-tools-extra/clangd/Hover.h
===
--- clang-tools-extra/clangd/Hover.h
+++ clang-tools-extra/clangd/Hover.h
@@ -17,6 +17,7 @@
 namespace clang {
 namespace clangd {

[PATCH] D96751: [clangd] Populate detail field in document symbols

2021-02-16 Thread WangWei via Phabricator via cfe-commits
lightmelodies updated this revision to Diff 324143.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96751

Files:
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/test/symbols.test

Index: clang-tools-extra/clangd/test/symbols.test
===
--- clang-tools-extra/clangd/test/symbols.test
+++ clang-tools-extra/clangd/test/symbols.test
@@ -1,5 +1,5 @@
 # RUN: clangd --index-file=%S/Inputs/symbols.test.yaml -lit-test < %s | FileCheck %s
-{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"workspace":{"symbol":{"symbolKind":{"valueSet": [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26],"trace":"off"}}
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{"processId":123,"rootPath":"clangd","capabilities":{"textDocument":{"documentSymbol":{"hierarchicalDocumentSymbolSupport":true}},"workspace":{"symbol":{"symbolKind":{"valueSet": [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26],"trace":"off"}}
 ---
 {"jsonrpc":"2.0","method":"textDocument/didOpen","params":{"textDocument":{"uri":"test:///main.cpp","languageId":"cpp","version":1,"text":"void foo(); int main() { foo(); }\n"}}}
 ---
@@ -34,40 +34,54 @@
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:"result": [
 # CHECK-NEXT:  {
-# CHECK-NEXT:"containerName": "",
+# CHECK-NEXT:"detail": "void ()",
 # CHECK-NEXT:"kind": 12,
-# CHECK-NEXT:"location": {
-# CHECK-NEXT:  "range": {
-# CHECK-NEXT:"end": {
-# CHECK-NEXT:  "character": {{.*}},
-# CHECK-NEXT:  "line": {{.*}}
-# CHECK-NEXT:},
-# CHECK-NEXT:"start": {
-# CHECK-NEXT:  "character": {{.*}},
-# CHECK-NEXT:  "line": {{.*}}
-# CHECK-NEXT:}
+# CHECK-NEXT:"name": "foo",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file://{{.*}}/main.cpp"
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
+# CHECK-NEXT:  }
 # CHECK-NEXT:},
-# CHECK-NEXT:"name": "foo"
-# CHECK-NEXT:  }
+# CHECK-NEXT:"selectionRange": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
+# CHECK-NEXT:  },
 # CHECK-NEXT:  {
-# CHECK-NEXT:"containerName": "",
+# CHECK-NEXT:"detail": "int ()",
 # CHECK-NEXT:"kind": 12,
-# CHECK-NEXT:"location": {
-# CHECK-NEXT:  "range": {
-# CHECK-NEXT:"end": {
-# CHECK-NEXT:  "character": {{.*}},
-# CHECK-NEXT:  "line": {{.*}}
-# CHECK-NEXT:},
-# CHECK-NEXT:"start": {
-# CHECK-NEXT:  "character": {{.*}},
-# CHECK-NEXT:  "line": {{.*}}
-# CHECK-NEXT:}
+# CHECK-NEXT:"name": "main",
+# CHECK-NEXT:"range": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
 # CHECK-NEXT:  },
-# CHECK-NEXT:  "uri": "file://{{.*}}/main.cpp"
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
+# CHECK-NEXT:  }
 # CHECK-NEXT:},
-# CHECK-NEXT:"name": "main"
+# CHECK-NEXT:"selectionRange": {
+# CHECK-NEXT:  "end": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
+# CHECK-NEXT:  },
+# CHECK-NEXT:  "start": {
+# CHECK-NEXT:"character": {{.*}},
+# CHECK-NEXT:"line": {{.*}}
+# CHECK-NEXT:  }
+# CHECK-NEXT:}
 # CHECK-NEXT:  }
 # CHECK-NEXT:]
 # CHECK-NEXT:}
Index: clang-tools-extra/clangd/FindSymbols.cpp
===
--- clang-tools-extra/clangd/FindSymbols.cpp
+++ clang-tools-extra/clangd/FindSymbols.cpp
@@ -172,6 +172,31 @@
 }
 
 namespace {
+
+std::string getSymbolDetail(ASTContext &Ctx, const NamedDecl &ND) {
+  PrintingPolicy P(Ctx.getPrintingPolicy());
+  P.SuppressScope = true;
+  P.SuppressUnwrittenScope = true;
+  P.AnonymousTagLocations = false;
+  P.PolishForDeclaration = true;
+  std::string Detail;
+  llvm::raw_string_ostream OS(Detail);
+  if (const auto *TP = ND.getDescribedTemplateParams()) {
+TP->print(OS, Ctx, P);
+  }
+  if (const auto *VD = dyn_cast(&ND)) {
+VD-

[PATCH] D96751: [clangd] Populate detail field in document symbols

2021-02-16 Thread WangWei via Phabricator via cfe-commits
lightmelodies added a comment.

Thanks for your response. Yes decl is too verbose in many cases.  In fact I 
have tried another concise version, but I can not decide which one is better. 
Unit test in FindSymbolsTests.cpp is also done, but I would like upload them 
after we reach an agreement on what behavior is prefered.
F15557452: image.png 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96751

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


[PATCH] D96751: [clangd] Populate detail field in document symbols

2021-02-18 Thread WangWei via Phabricator via cfe-commits
lightmelodies updated this revision to Diff 324582.
lightmelodies added a comment.

Change behavior of template and add unit tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96751

Files:
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/test/symbols.test
  clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp

Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -35,6 +35,7 @@
 }
 MATCHER_P(WithName, N, "") { return arg.name == N; }
 MATCHER_P(WithKind, Kind, "") { return arg.kind == Kind; }
+MATCHER_P(WithDetail, Detail, "") { return arg.detail == Detail; }
 MATCHER_P(SymRange, Range, "") { return arg.range == Range; }
 
 // GMock helpers for matching DocumentSymbol.
@@ -374,44 +375,55 @@
   EXPECT_THAT(
   getSymbols(TU.build()),
   ElementsAreArray(
-  {AllOf(WithName("Foo"), WithKind(SymbolKind::Class), Children()),
+  {AllOf(WithName("Foo"), WithKind(SymbolKind::Class),
+ WithDetail("class"), Children()),
AllOf(WithName("Foo"), WithKind(SymbolKind::Class),
- Children(AllOf(WithName("Foo"),
-WithKind(SymbolKind::Constructor), Children()),
-  AllOf(WithName("Foo"),
-WithKind(SymbolKind::Constructor), Children()),
-  AllOf(WithName("f"), WithKind(SymbolKind::Method),
-Children()),
-  AllOf(WithName("operator="),
-WithKind(SymbolKind::Method), Children()),
-  AllOf(WithName("~Foo"),
-WithKind(SymbolKind::Constructor), Children()),
-  AllOf(WithName("Nested"), WithKind(SymbolKind::Class),
-Children(AllOf(WithName("f"),
-   WithKind(SymbolKind::Method),
-   Children()),
-   AllOf(WithName("Friend"), WithKind(SymbolKind::Class), Children()),
-   AllOf(WithName("f1"), WithKind(SymbolKind::Function), Children()),
-   AllOf(WithName("f2"), WithKind(SymbolKind::Function), Children()),
-   AllOf(WithName("KInt"), WithKind(SymbolKind::Variable), Children()),
-   AllOf(WithName("kStr"), WithKind(SymbolKind::Variable), Children()),
-   AllOf(WithName("f1"), WithKind(SymbolKind::Function), Children()),
+ WithDetail("class"),
+ Children(
+ AllOf(WithName("Foo"), WithKind(SymbolKind::Constructor),
+   WithDetail("void ()"), Children()),
+ AllOf(WithName("Foo"), WithKind(SymbolKind::Constructor),
+   WithDetail("void (int)"), Children()),
+ AllOf(WithName("f"), WithKind(SymbolKind::Method),
+   WithDetail("void ()"), Children()),
+ AllOf(WithName("operator="), WithKind(SymbolKind::Method),
+   WithDetail("Foo &(const Foo &)"), Children()),
+ AllOf(WithName("~Foo"), WithKind(SymbolKind::Constructor),
+   WithDetail("void ()"), Children()),
+ AllOf(WithName("Nested"), WithKind(SymbolKind::Class),
+   WithDetail("class"),
+   Children(AllOf(
+   WithName("f"), WithKind(SymbolKind::Method),
+   WithDetail("void ()"), Children()),
+   AllOf(WithName("Friend"), WithKind(SymbolKind::Class),
+ WithDetail("class"), Children()),
+   AllOf(WithName("f1"), WithKind(SymbolKind::Function),
+ WithDetail("void ()"), Children()),
+   AllOf(WithName("f2"), WithKind(SymbolKind::Function),
+ WithDetail("void ()"), Children()),
+   AllOf(WithName("KInt"), WithKind(SymbolKind::Variable),
+ WithDetail("const int"), Children()),
+   AllOf(WithName("kStr"), WithKind(SymbolKind::Variable),
+ WithDetail("const char *"), Children()),
+   AllOf(WithName("f1"), WithKind(SymbolKind::Function),
+ WithDetail("void ()"), Children()),
AllOf(
-   WithName("foo"), WithKind(SymbolKind::Namespace),
-   Children(
-   AllOf(WithName("int32"), WithKind(SymbolKind::Class),
- Children()),
-   AllOf(WithName("int32_t"), WithKind(SymbolKind::Class),
- Children()),
-   AllOf(WithName("v1"), WithKind(Symb

[PATCH] D96751: [clangd] Populate detail field in document symbols

2021-02-18 Thread WangWei via Phabricator via cfe-commits
lightmelodies updated this revision to Diff 324599.

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

https://reviews.llvm.org/D96751

Files:
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/test/symbols.test
  clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp

Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -35,6 +35,7 @@
 }
 MATCHER_P(WithName, N, "") { return arg.name == N; }
 MATCHER_P(WithKind, Kind, "") { return arg.kind == Kind; }
+MATCHER_P(WithDetail, Detail, "") { return arg.detail == Detail; }
 MATCHER_P(SymRange, Range, "") { return arg.range == Range; }
 
 // GMock helpers for matching DocumentSymbol.
@@ -374,44 +375,55 @@
   EXPECT_THAT(
   getSymbols(TU.build()),
   ElementsAreArray(
-  {AllOf(WithName("Foo"), WithKind(SymbolKind::Class), Children()),
+  {AllOf(WithName("Foo"), WithKind(SymbolKind::Class),
+ WithDetail("class"), Children()),
AllOf(WithName("Foo"), WithKind(SymbolKind::Class),
- Children(AllOf(WithName("Foo"),
-WithKind(SymbolKind::Constructor), Children()),
-  AllOf(WithName("Foo"),
-WithKind(SymbolKind::Constructor), Children()),
-  AllOf(WithName("f"), WithKind(SymbolKind::Method),
-Children()),
-  AllOf(WithName("operator="),
-WithKind(SymbolKind::Method), Children()),
-  AllOf(WithName("~Foo"),
-WithKind(SymbolKind::Constructor), Children()),
-  AllOf(WithName("Nested"), WithKind(SymbolKind::Class),
-Children(AllOf(WithName("f"),
-   WithKind(SymbolKind::Method),
-   Children()),
-   AllOf(WithName("Friend"), WithKind(SymbolKind::Class), Children()),
-   AllOf(WithName("f1"), WithKind(SymbolKind::Function), Children()),
-   AllOf(WithName("f2"), WithKind(SymbolKind::Function), Children()),
-   AllOf(WithName("KInt"), WithKind(SymbolKind::Variable), Children()),
-   AllOf(WithName("kStr"), WithKind(SymbolKind::Variable), Children()),
-   AllOf(WithName("f1"), WithKind(SymbolKind::Function), Children()),
+ WithDetail("class"),
+ Children(
+ AllOf(WithName("Foo"), WithKind(SymbolKind::Constructor),
+   WithDetail("void ()"), Children()),
+ AllOf(WithName("Foo"), WithKind(SymbolKind::Constructor),
+   WithDetail("void (int)"), Children()),
+ AllOf(WithName("f"), WithKind(SymbolKind::Method),
+   WithDetail("void ()"), Children()),
+ AllOf(WithName("operator="), WithKind(SymbolKind::Method),
+   WithDetail("Foo &(const Foo &)"), Children()),
+ AllOf(WithName("~Foo"), WithKind(SymbolKind::Constructor),
+   WithDetail("void ()"), Children()),
+ AllOf(WithName("Nested"), WithKind(SymbolKind::Class),
+   WithDetail("class"),
+   Children(AllOf(
+   WithName("f"), WithKind(SymbolKind::Method),
+   WithDetail("void ()"), Children()),
+   AllOf(WithName("Friend"), WithKind(SymbolKind::Class),
+ WithDetail("class"), Children()),
+   AllOf(WithName("f1"), WithKind(SymbolKind::Function),
+ WithDetail("void ()"), Children()),
+   AllOf(WithName("f2"), WithKind(SymbolKind::Function),
+ WithDetail("void ()"), Children()),
+   AllOf(WithName("KInt"), WithKind(SymbolKind::Variable),
+ WithDetail("const int"), Children()),
+   AllOf(WithName("kStr"), WithKind(SymbolKind::Variable),
+ WithDetail("const char *"), Children()),
+   AllOf(WithName("f1"), WithKind(SymbolKind::Function),
+ WithDetail("void ()"), Children()),
AllOf(
-   WithName("foo"), WithKind(SymbolKind::Namespace),
-   Children(
-   AllOf(WithName("int32"), WithKind(SymbolKind::Class),
- Children()),
-   AllOf(WithName("int32_t"), WithKind(SymbolKind::Class),
- Children()),
-   AllOf(WithName("v1"), WithKind(SymbolKind::Variable),
- Children()),
-   AllOf(WithName("bar"), WithKind(SymbolKind

[PATCH] D96751: [clangd] Populate detail field in document symbols

2021-02-18 Thread WangWei via Phabricator via cfe-commits
lightmelodies updated this revision to Diff 324626.
lightmelodies added a comment.

Better printing for C++ constructor and destructor. Remove unnecessary test 
cases.


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

https://reviews.llvm.org/D96751

Files:
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/test/symbols.test
  clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp

Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
@@ -35,6 +35,7 @@
 }
 MATCHER_P(WithName, N, "") { return arg.name == N; }
 MATCHER_P(WithKind, Kind, "") { return arg.kind == Kind; }
+MATCHER_P(WithDetail, Detail, "") { return arg.detail == Detail; }
 MATCHER_P(SymRange, Range, "") { return arg.range == Range; }
 
 // GMock helpers for matching DocumentSymbol.
@@ -374,44 +375,55 @@
   EXPECT_THAT(
   getSymbols(TU.build()),
   ElementsAreArray(
-  {AllOf(WithName("Foo"), WithKind(SymbolKind::Class), Children()),
+  {AllOf(WithName("Foo"), WithKind(SymbolKind::Class),
+ WithDetail("class"), Children()),
AllOf(WithName("Foo"), WithKind(SymbolKind::Class),
- Children(AllOf(WithName("Foo"),
-WithKind(SymbolKind::Constructor), Children()),
-  AllOf(WithName("Foo"),
-WithKind(SymbolKind::Constructor), Children()),
-  AllOf(WithName("f"), WithKind(SymbolKind::Method),
-Children()),
-  AllOf(WithName("operator="),
-WithKind(SymbolKind::Method), Children()),
-  AllOf(WithName("~Foo"),
-WithKind(SymbolKind::Constructor), Children()),
-  AllOf(WithName("Nested"), WithKind(SymbolKind::Class),
-Children(AllOf(WithName("f"),
-   WithKind(SymbolKind::Method),
-   Children()),
-   AllOf(WithName("Friend"), WithKind(SymbolKind::Class), Children()),
-   AllOf(WithName("f1"), WithKind(SymbolKind::Function), Children()),
-   AllOf(WithName("f2"), WithKind(SymbolKind::Function), Children()),
-   AllOf(WithName("KInt"), WithKind(SymbolKind::Variable), Children()),
-   AllOf(WithName("kStr"), WithKind(SymbolKind::Variable), Children()),
-   AllOf(WithName("f1"), WithKind(SymbolKind::Function), Children()),
+ WithDetail("class"),
+ Children(
+ AllOf(WithName("Foo"), WithKind(SymbolKind::Constructor),
+   WithDetail("()"), Children()),
+ AllOf(WithName("Foo"), WithKind(SymbolKind::Constructor),
+   WithDetail("(int)"), Children()),
+ AllOf(WithName("f"), WithKind(SymbolKind::Method),
+   WithDetail("void ()"), Children()),
+ AllOf(WithName("operator="), WithKind(SymbolKind::Method),
+   WithDetail("Foo &(const Foo &)"), Children()),
+ AllOf(WithName("~Foo"), WithKind(SymbolKind::Constructor),
+   WithDetail(""), Children()),
+ AllOf(WithName("Nested"), WithKind(SymbolKind::Class),
+   WithDetail("class"),
+   Children(AllOf(
+   WithName("f"), WithKind(SymbolKind::Method),
+   WithDetail("void ()"), Children()),
+   AllOf(WithName("Friend"), WithKind(SymbolKind::Class),
+ WithDetail("class"), Children()),
+   AllOf(WithName("f1"), WithKind(SymbolKind::Function),
+ WithDetail("void ()"), Children()),
+   AllOf(WithName("f2"), WithKind(SymbolKind::Function),
+ WithDetail("void ()"), Children()),
+   AllOf(WithName("KInt"), WithKind(SymbolKind::Variable),
+ WithDetail("const int"), Children()),
+   AllOf(WithName("kStr"), WithKind(SymbolKind::Variable),
+ WithDetail("const char *"), Children()),
+   AllOf(WithName("f1"), WithKind(SymbolKind::Function),
+ WithDetail("void ()"), Children()),
AllOf(
-   WithName("foo"), WithKind(SymbolKind::Namespace),
-   Children(
-   AllOf(WithName("int32"), WithKind(SymbolKind::Class),
- Children()),
-   AllOf(WithName("int32_t"), WithKind(SymbolKind::Class),
- Children()),
-   AllOf(WithName("v1"), WithKind(SymbolKind::Variable),
-

[PATCH] D96751: [clangd] Populate detail field in document symbols

2021-02-18 Thread WangWei via Phabricator via cfe-commits
lightmelodies added a comment.

In D96751#2571346 , @sammccall wrote:

> Nice! This looks good to land as-is, I just have some suggestions where we 
> may want to mark behavior to revisit later, and some places where we could 
> trim the tests a bit.
>
> Do you have commit access, or want me to land this for you? (I'd need an 
> address for the commit)

I don't have commit access yet, it will be appreciated if you can land this for 
me (just use lightmelodies(lightmelod...@outlook.com))


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

https://reviews.llvm.org/D96751

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


[PATCH] D96751: [clangd] Populate detail field in document symbols

2021-02-18 Thread WangWei via Phabricator via cfe-commits
lightmelodies added a comment.

In D96751#2571563 , @sammccall wrote:

> Fantastic, thank you!

Maybe WithoutVoid.consume_front("void ");


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

https://reviews.llvm.org/D96751

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


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

2022-11-17 Thread WangWei via Phabricator via cfe-commits
lightmelodies added a comment.

In D131295#3933381 , @Trass3r wrote:

> One remaining issue is multiple lenses for template code like
>
>   template 
>   int i = 0;
>   
>   template int i<0>;
>   template int i<1>;
>   template int i<2>;
>   
>   template 
>   struct Foo {
>   int foo(); // I see 3 codelenses here
>   };
>   
>   template struct Foo<0>;
>   template struct Foo<1>;
>   
>   int main()
>   {
>   // return Foo<0>().foo() + Foo<1>().foo();
>   // return i<0> + i<1> + i<2>;
>   }

Well, multiple lenses occured due to 
https://github.com/lightmelodies/llvm-project/blob/codelens/clang-tools-extra/clangd/FindSymbols.cpp#L500
And can be fixed by 
https://github.com/lightmelodies/llvm-project/blob/codelens/clang-tools-extra/clangd/CodeLens.cpp#L69


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131295

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


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

2022-11-17 Thread WangWei via Phabricator via cfe-commits
lightmelodies added a comment.

In D131295#3935027 , @Trass3r wrote:

> Thanks, it seems to fix the base case, but I still see multiple lenses when I 
> add
>
>   template 
>   int Foo::foo()
>   {
>   return 0;
>   }

Interesting, I see the same issue with DocumentSymbols. Maybe because **foo** 
is explicit specialization (though outside **Foo<1>**) so both check are 
passed. However I can not find a way to detect such case right now.
F25364530: image.png 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131295

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


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

2022-11-21 Thread WangWei via Phabricator via cfe-commits
lightmelodies added a comment.

In D131295#3935759 , @Trass3r wrote:

> Yeah quite complex: https://godbolt.org/z/8T5Pqadro

Not very hard with a track of visited decls. It should also reslove other 
duplicate codelens.
https://github.com/lightmelodies/llvm-project/blob/b7d664bff5fcafb5a0758b73cd0b77e64cacb03c/clang-tools-extra/clangd/CodeLens.cpp#L69


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131295

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