[PATCH] D41345: [clangd] Add more symbol information for code completion.
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE322097: [clangd] Add more symbol information for code
completion. (authored by ioeric, committed by ).
Changed prior to commit:
https://reviews.llvm.org/D41345?vs=129106=129110#toc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/CodeComplete.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
clangd/index/SymbolYAML.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -9,7 +9,6 @@
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
#include "clang/Basic/VirtualFileSystem.h"
@@ -26,12 +25,24 @@
#include
#include
+using testing::AllOf;
using testing::Eq;
using testing::Field;
+using testing::Not;
using testing::UnorderedElementsAre;
using testing::UnorderedElementsAreArray;
// GMock helpers for matching Symbol.
+MATCHER_P(Labeled, Label, "") { return arg.CompletionLabel == Label; }
+MATCHER(HasDetail, "") { return arg.Detail; }
+MATCHER_P(Detail, D, "") {
+ return arg.Detail && arg.Detail->CompletionDetail == D;
+}
+MATCHER_P(Doc, D, "") { return arg.Detail && arg.Detail->Documentation == D; }
+MATCHER_P(Plain, Text, "") { return arg.CompletionPlainInsertText == Text; }
+MATCHER_P(Snippet, S, "") {
+ return arg.CompletionSnippetInsertText == S;
+}
MATCHER_P(QName, Name, "") {
return (arg.Scope + (arg.Scope.empty() ? "" : "::") + arg.Name).str() == Name;
}
@@ -147,6 +158,38 @@
QName("foo::baz")}));
}
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+ const std::string Main = R"(
+namespace nx {
+/// Foo comment.
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("nx"),
+ AllOf(QName("nx::ff"),
+ Labeled("ff(int x, double y)"),
+ Detail("int"), Doc("Foo comment.";
+}
+
+TEST_F(SymbolCollectorTest, PlainAndSnippet) {
+ const std::string Main = R"(
+namespace nx {
+void f() {}
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ QName("nx"),
+ AllOf(QName("nx::f"), Labeled("f()"), Plain("f"), Snippet("f()")),
+ AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Plain("ff"),
+Snippet("ff(${1:int x}, ${2:double y})";
+}
+
TEST_F(SymbolCollectorTest, YAMLConversions) {
const std::string YAML1 = R"(
---
@@ -160,6 +203,12 @@
StartOffset: 0
EndOffset: 1
FilePath:/path/foo.h
+CompletionLabel:'Foo1-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+Detail:
+ Documentation:'Foo doc'
+ CompletionDetail:'int'
...
)";
const std::string YAML2 = R"(
@@ -174,15 +223,21 @@
StartOffset: 10
EndOffset: 12
FilePath:/path/foo.h
+CompletionLabel:'Foo2-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+CompletionSnippetInsertText:'snippet'
...
)";
auto Symbols1 = SymbolFromYAML(YAML1);
- EXPECT_THAT(Symbols1,
- UnorderedElementsAre(QName("clang::Foo1")));
+ EXPECT_THAT(Symbols1, UnorderedElementsAre(
+AllOf(QName("clang::Foo1"), Labeled("Foo1-label"),
+ Doc("Foo doc"), Detail("int";
auto Symbols2 = SymbolFromYAML(YAML2);
- EXPECT_THAT(Symbols2,
- UnorderedElementsAre(QName("clang::Foo2")));
+ EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(QName("clang::Foo2"),
+ Labeled("Foo2-label"),
+ Not(HasDetail();
std::string ConcatenatedYAML =
SymbolsToYAML(Symbols1) + SymbolsToYAML(Symbols2);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -68,6 +68,8 @@
MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
MATCHER_P(Kind, K, "") { return arg.kind == K; }
MATCHER_P(Filter, F, "") { return arg.filterText == F; }
+MATCHER_P(Doc, D, "") { return arg.documentation == D; }
+MATCHER_P(Detail, D, "") {
[PATCH] D41345: [clangd] Add more symbol information for code completion.
ioeric updated this revision to Diff 129106.
ioeric marked an inline comment as done.
ioeric added a comment.
- Addrress review comment.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/CodeComplete.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
clangd/index/SymbolYAML.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -9,7 +9,6 @@
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
#include "clang/Basic/VirtualFileSystem.h"
@@ -26,12 +25,24 @@
#include
#include
+using testing::AllOf;
using testing::Eq;
using testing::Field;
+using testing::Not;
using testing::UnorderedElementsAre;
using testing::UnorderedElementsAreArray;
// GMock helpers for matching Symbol.
+MATCHER_P(Labeled, Label, "") { return arg.CompletionLabel == Label; }
+MATCHER(HasDetail, "") { return arg.Detail; }
+MATCHER_P(Detail, D, "") {
+ return arg.Detail && arg.Detail->CompletionDetail == D;
+}
+MATCHER_P(Doc, D, "") { return arg.Detail && arg.Detail->Documentation == D; }
+MATCHER_P(Plain, Text, "") { return arg.CompletionPlainInsertText == Text; }
+MATCHER_P(Snippet, S, "") {
+ return arg.CompletionSnippetInsertText == S;
+}
MATCHER_P(QName, Name, "") {
return (arg.Scope + (arg.Scope.empty() ? "" : "::") + arg.Name).str() == Name;
}
@@ -147,6 +158,38 @@
QName("foo::baz")}));
}
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+ const std::string Main = R"(
+namespace nx {
+/// Foo comment.
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("nx"),
+ AllOf(QName("nx::ff"),
+ Labeled("ff(int x, double y)"),
+ Detail("int"), Doc("Foo comment.";
+}
+
+TEST_F(SymbolCollectorTest, PlainAndSnippet) {
+ const std::string Main = R"(
+namespace nx {
+void f() {}
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ QName("nx"),
+ AllOf(QName("nx::f"), Labeled("f()"), Plain("f"), Snippet("f()")),
+ AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Plain("ff"),
+Snippet("ff(${1:int x}, ${2:double y})";
+}
+
TEST_F(SymbolCollectorTest, YAMLConversions) {
const std::string YAML1 = R"(
---
@@ -160,6 +203,12 @@
StartOffset: 0
EndOffset: 1
FilePath:/path/foo.h
+CompletionLabel:'Foo1-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+Detail:
+ Documentation:'Foo doc'
+ CompletionDetail:'int'
...
)";
const std::string YAML2 = R"(
@@ -174,15 +223,21 @@
StartOffset: 10
EndOffset: 12
FilePath:/path/foo.h
+CompletionLabel:'Foo2-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+CompletionSnippetInsertText:'snippet'
...
)";
auto Symbols1 = SymbolFromYAML(YAML1);
- EXPECT_THAT(Symbols1,
- UnorderedElementsAre(QName("clang::Foo1")));
+ EXPECT_THAT(Symbols1, UnorderedElementsAre(
+AllOf(QName("clang::Foo1"), Labeled("Foo1-label"),
+ Doc("Foo doc"), Detail("int";
auto Symbols2 = SymbolFromYAML(YAML2);
- EXPECT_THAT(Symbols2,
- UnorderedElementsAre(QName("clang::Foo2")));
+ EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(QName("clang::Foo2"),
+ Labeled("Foo2-label"),
+ Not(HasDetail();
std::string ConcatenatedYAML =
SymbolToYAML(Symbols1) + SymbolToYAML(Symbols2);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -68,6 +68,8 @@
MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
MATCHER_P(Kind, K, "") { return arg.kind == K; }
MATCHER_P(Filter, F, "") { return arg.filterText == F; }
+MATCHER_P(Doc, D, "") { return arg.documentation == D; }
+MATCHER_P(Detail, D, "") { return arg.detail == D; }
MATCHER_P(PlainText, Text, "") {
return arg.insertTextFormat == clangd::InsertTextFormat::PlainText &&
[PATCH] D41345: [clangd] Add more symbol information for code completion.
sammccall accepted this revision.
sammccall added inline comments.
Comment at: clangd/index/SymbolYAML.cpp:78
+assert(io.getContext());
+Detail = static_cast(io.getContext())
+ ->Allocate();
this removes the Detail object if it's empty - this seems maybe unneccesary and
certainly the wrong layer. It seems enough to do
if (!outputting)
Detail = (allocate)
else if (!Detail)
return;
io.mapOptional("Documentation", Detail->Documentation);
// etc
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41345: [clangd] Add more symbol information for code completion.
ioeric added inline comments. Comment at: clangd/index/Index.h:122 + + llvm::Optional Detail; + sammccall wrote: > ioeric wrote: > > sammccall wrote: > > > ioeric wrote: > > > > sammccall wrote: > > > > > I think you probably want a raw pointer rather than optional: > > > > > - reduce the size of the struct when it's absent > > > > > - make it inheritance-friendly so we can hang index-specific info > > > > > off it > > > > > (raw pointer rather than unique_ptr because it's owned by a slab not > > > > > by malloc, but unique_ptr is ok for now) > > > > > > > > > This is not easy for now with `unique_ptr` because of this line :( > > > > https://github.com/llvm-mirror/clang-tools-extra/blob/3565d1a1a692fc9f5c21e634b470535da2bb4d25/clangd/index/SymbolYAML.cpp#L141). > > > > > > > > > > > > This shouldn't be an issue when we have the optimized symbol slab, > > > > where we store raw pointers. And we would probably want to serialize > > > > the whole slab instead of the individual symbols anyway. > > > > > > > > > reduce the size of the struct when it's absent > > > > `llvm::Optional` doesn't take much more space, so the size should be > > > > fine. > > > > > > > > > make it inheritance-friendly so we can hang index-specific info off it > > > > Could you elaborate on `index-specific info`? It's unclear to me how > > > > this would be used. > > > > This is not easy for now with unique_ptr because of this line > > > Oh no, somehow i missed this during review. > > > We shouldn't be relying on symbols being copyable. I'll try to work out > > > how to fix this and delete the copy constructor. > > > > > > > This shouldn't be an issue when we have the optimized symbol slab, > > > > where we store raw pointers. > > > Sure. That's not a big monolithic/mysterous thing though, storing the > > > details in the slab can be done in this patch... If you think it'll be > > > easier once strings are arena-based, then maybe we should delay this > > > patch until that's done, rather than make that work bigger. > > > > > > > And we would probably want to serialize the whole slab instead of the > > > > individual symbols anyway. > > > This i'm less sure about, but I don't think it matters. > > > > > > > llvm::Optional doesn't take much more space, so the size should be fine. > > > Optional takes the same size as the details itself (plus one bool). This > > > is fairly small for now, but I think a major point of Details is to > > > expand it in the future? > > > > > > > Could you elaborate on index-specific info? It's unclear to me how this > > > > would be used. > > > Yeah, this is something we talked about in the meeting with Marc-Andre > > > but it's not really obvious - what's the point of allowing Details to be > > > extended if clangd has to consume it? > > > > > > It sounded like he might have use cases for using index infrastructure > > > outside clangd. We might also have google-internal index features we want > > > (matching generated code to proto fields?). I'm not really sure how > > > compelling this argument is. > > Thanks for the allocator change! > > > > `Details` now contains just stringrefs. I wonder how much we want it to be > > inherit-friendly at this point, as the size is relatively small now. If you > > think this is a better way to go, I'll make the structure contain strings > > and store the whole structure in arena. This would require some tweaks for > > yamls tho but shouldn't be hard. > So this needs to be at least optional I think - at the moment, how does an > API user know whether to fetch the rest of the details? > > FWIW, the size difference is already significant: symbol is 136 bytes if this > is a unique_ptr, and 164 bytes if it's optional (+20%) - StringRefs are still > pretty big. > But I don't feel really strongly about this. Done. Made it a raw pointer. PTAL Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41345 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41345: [clangd] Add more symbol information for code completion.
ioeric updated this revision to Diff 129075.
ioeric added a comment.
- Merge branch 'master' of http://llvm.org/git/clang-tools-extra into symbol
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/CodeComplete.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
clangd/index/SymbolYAML.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -9,7 +9,6 @@
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
#include "clang/Basic/VirtualFileSystem.h"
@@ -26,12 +25,24 @@
#include
#include
+using testing::AllOf;
using testing::Eq;
using testing::Field;
+using testing::Not;
using testing::UnorderedElementsAre;
using testing::UnorderedElementsAreArray;
// GMock helpers for matching Symbol.
+MATCHER_P(Labeled, Label, "") { return arg.CompletionLabel == Label; }
+MATCHER(HasDetail, "") { return arg.Detail; }
+MATCHER_P(Detail, D, "") {
+ return arg.Detail && arg.Detail->CompletionDetail == D;
+}
+MATCHER_P(Doc, D, "") { return arg.Detail && arg.Detail->Documentation == D; }
+MATCHER_P(Plain, Text, "") { return arg.CompletionPlainInsertText == Text; }
+MATCHER_P(Snippet, S, "") {
+ return arg.CompletionSnippetInsertText == S;
+}
MATCHER_P(QName, Name, "") {
return (arg.Scope + (arg.Scope.empty() ? "" : "::") + arg.Name).str() == Name;
}
@@ -147,6 +158,38 @@
QName("foo::baz")}));
}
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+ const std::string Main = R"(
+namespace nx {
+/// Foo comment.
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("nx"),
+ AllOf(QName("nx::ff"),
+ Labeled("ff(int x, double y)"),
+ Detail("int"), Doc("Foo comment.";
+}
+
+TEST_F(SymbolCollectorTest, PlainAndSnippet) {
+ const std::string Main = R"(
+namespace nx {
+void f() {}
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ QName("nx"),
+ AllOf(QName("nx::f"), Labeled("f()"), Plain("f"), Snippet("f()")),
+ AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Plain("ff"),
+Snippet("ff(${1:int x}, ${2:double y})";
+}
+
TEST_F(SymbolCollectorTest, YAMLConversions) {
const std::string YAML1 = R"(
---
@@ -160,6 +203,12 @@
StartOffset: 0
EndOffset: 1
FilePath:/path/foo.h
+CompletionLabel:'Foo1-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+Detail:
+ Documentation:'Foo doc'
+ CompletionDetail:'int'
...
)";
const std::string YAML2 = R"(
@@ -174,15 +223,21 @@
StartOffset: 10
EndOffset: 12
FilePath:/path/foo.h
+CompletionLabel:'Foo2-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+CompletionSnippetInsertText:'snippet'
...
)";
auto Symbols1 = SymbolFromYAML(YAML1);
- EXPECT_THAT(Symbols1,
- UnorderedElementsAre(QName("clang::Foo1")));
+ EXPECT_THAT(Symbols1, UnorderedElementsAre(
+AllOf(QName("clang::Foo1"), Labeled("Foo1-label"),
+ Doc("Foo doc"), Detail("int";
auto Symbols2 = SymbolFromYAML(YAML2);
- EXPECT_THAT(Symbols2,
- UnorderedElementsAre(QName("clang::Foo2")));
+ EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(QName("clang::Foo2"),
+ Labeled("Foo2-label"),
+ Not(HasDetail();
std::string ConcatenatedYAML =
SymbolToYAML(Symbols1) + SymbolToYAML(Symbols2);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -68,6 +68,8 @@
MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
MATCHER_P(Kind, K, "") { return arg.kind == K; }
MATCHER_P(Filter, F, "") { return arg.filterText == F; }
+MATCHER_P(Doc, D, "") { return arg.documentation == D; }
+MATCHER_P(Detail, D, "") { return arg.detail == D; }
MATCHER_P(PlainText, Text, "") {
return arg.insertTextFormat == clangd::InsertTextFormat::PlainText &&
[PATCH] D41345: [clangd] Add more symbol information for code completion.
ioeric updated this revision to Diff 129059.
ioeric marked an inline comment as done.
ioeric added a comment.
- Minor cleanup.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/CodeComplete.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
clangd/index/SymbolYAML.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -9,7 +9,6 @@
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
#include "clang/Basic/VirtualFileSystem.h"
@@ -26,11 +25,23 @@
#include
#include
+using testing::AllOf;
using testing::Eq;
using testing::Field;
+using testing::Not;
using testing::UnorderedElementsAre;
// GMock helpers for matching Symbol.
+MATCHER_P(Labeled, Label, "") { return arg.CompletionLabel == Label; }
+MATCHER(HasDetail, "") { return arg.Detail; }
+MATCHER_P(Detail, D, "") {
+ return arg.Detail && arg.Detail->CompletionDetail == D;
+}
+MATCHER_P(Doc, D, "") { return arg.Detail && arg.Detail->Documentation == D; }
+MATCHER_P(Plain, Text, "") { return arg.CompletionPlainInsertText == Text; }
+MATCHER_P(Snippet, S, "") {
+ return arg.CompletionSnippetInsertText == S;
+}
MATCHER_P(QName, Name, "") {
return (arg.Scope + (arg.Scope.empty() ? "" : "::") + arg.Name).str() == Name;
}
@@ -109,6 +120,38 @@
QName("f1"), QName("f2")));
}
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+ const std::string Main = R"(
+namespace nx {
+/// Foo comment.
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("nx"),
+ AllOf(QName("nx::ff"),
+ Labeled("ff(int x, double y)"),
+ Detail("int"), Doc("Foo comment.";
+}
+
+TEST_F(SymbolCollectorTest, PlainAndSnippet) {
+ const std::string Main = R"(
+namespace nx {
+void f() {}
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ QName("nx"),
+ AllOf(QName("nx::f"), Labeled("f()"), Plain("f"), Snippet("f()")),
+ AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Plain("ff"),
+Snippet("ff(${1:int x}, ${2:double y})";
+}
+
TEST_F(SymbolCollectorTest, YAMLConversions) {
const std::string YAML1 = R"(
---
@@ -122,6 +165,12 @@
StartOffset: 0
EndOffset: 1
FilePath:/path/foo.h
+CompletionLabel:'Foo1-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+Detail:
+ Documentation:'Foo doc'
+ CompletionDetail:'int'
...
)";
const std::string YAML2 = R"(
@@ -136,15 +185,21 @@
StartOffset: 10
EndOffset: 12
FilePath:/path/foo.h
+CompletionLabel:'Foo2-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+CompletionSnippetInsertText:'snippet'
...
)";
auto Symbols1 = SymbolFromYAML(YAML1);
- EXPECT_THAT(Symbols1,
- UnorderedElementsAre(QName("clang::Foo1")));
+ EXPECT_THAT(Symbols1, UnorderedElementsAre(
+AllOf(QName("clang::Foo1"), Labeled("Foo1-label"),
+ Doc("Foo doc"), Detail("int";
auto Symbols2 = SymbolFromYAML(YAML2);
- EXPECT_THAT(Symbols2,
- UnorderedElementsAre(QName("clang::Foo2")));
+ EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(QName("clang::Foo2"),
+ Labeled("Foo2-label"),
+ Not(HasDetail();
std::string ConcatenatedYAML =
SymbolToYAML(Symbols1) + SymbolToYAML(Symbols2);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -68,6 +68,8 @@
MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
MATCHER_P(Kind, K, "") { return arg.kind == K; }
MATCHER_P(Filter, F, "") { return arg.filterText == F; }
+MATCHER_P(Doc, D, "") { return arg.documentation == D; }
+MATCHER_P(Detail, D, "") { return arg.detail == D; }
MATCHER_P(PlainText, Text, "") {
return arg.insertTextFormat == clangd::InsertTextFormat::PlainText &&
arg.insertText == Text;
@@
[PATCH] D41345: [clangd] Add more symbol information for code completion.
ioeric updated this revision to Diff 129058.
ioeric marked 2 inline comments as done.
ioeric added a comment.
- [clangd] Address review comments; made Detail a pointer.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/CodeComplete.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
clangd/index/SymbolYAML.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -9,7 +9,6 @@
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
#include "clang/Basic/VirtualFileSystem.h"
@@ -26,11 +25,23 @@
#include
#include
+using testing::AllOf;
using testing::Eq;
using testing::Field;
+using testing::Not;
using testing::UnorderedElementsAre;
// GMock helpers for matching Symbol.
+MATCHER_P(Labeled, Label, "") { return arg.CompletionLabel == Label; }
+MATCHER(HasDetail, "") { return arg.Detail; }
+MATCHER_P(Detail, D, "") {
+ return arg.Detail && arg.Detail->CompletionDetail == D;
+}
+MATCHER_P(Doc, D, "") { return arg.Detail && arg.Detail->Documentation == D; }
+MATCHER_P(Plain, Text, "") { return arg.CompletionPlainInsertText == Text; }
+MATCHER_P(Snippet, S, "") {
+ return arg.CompletionSnippetInsertText == S;
+}
MATCHER_P(QName, Name, "") {
return (arg.Scope + (arg.Scope.empty() ? "" : "::") + arg.Name).str() == Name;
}
@@ -109,6 +120,38 @@
QName("f1"), QName("f2")));
}
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+ const std::string Main = R"(
+namespace nx {
+/// Foo comment.
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("nx"),
+ AllOf(QName("nx::ff"),
+ Labeled("ff(int x, double y)"),
+ Detail("int"), Doc("Foo comment.";
+}
+
+TEST_F(SymbolCollectorTest, PlainAndSnippet) {
+ const std::string Main = R"(
+namespace nx {
+void f() {}
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ QName("nx"),
+ AllOf(QName("nx::f"), Labeled("f()"), Plain("f"), Snippet("f()")),
+ AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Plain("ff"),
+Snippet("ff(${1:int x}, ${2:double y})";
+}
+
TEST_F(SymbolCollectorTest, YAMLConversions) {
const std::string YAML1 = R"(
---
@@ -122,6 +165,12 @@
StartOffset: 0
EndOffset: 1
FilePath:/path/foo.h
+CompletionLabel:'Foo1-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+Detail:
+ Documentation:'Foo doc'
+ CompletionDetail:'int'
...
)";
const std::string YAML2 = R"(
@@ -136,15 +185,21 @@
StartOffset: 10
EndOffset: 12
FilePath:/path/foo.h
+CompletionLabel:'Foo2-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+CompletionSnippetInsertText:'snippet'
...
)";
auto Symbols1 = SymbolFromYAML(YAML1);
- EXPECT_THAT(Symbols1,
- UnorderedElementsAre(QName("clang::Foo1")));
+ EXPECT_THAT(Symbols1, UnorderedElementsAre(
+AllOf(QName("clang::Foo1"), Labeled("Foo1-label"),
+ Doc("Foo doc"), Detail("int";
auto Symbols2 = SymbolFromYAML(YAML2);
- EXPECT_THAT(Symbols2,
- UnorderedElementsAre(QName("clang::Foo2")));
+ EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(QName("clang::Foo2"),
+ Labeled("Foo2-label"),
+ Not(HasDetail();
std::string ConcatenatedYAML =
SymbolToYAML(Symbols1) + SymbolToYAML(Symbols2);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -68,6 +68,8 @@
MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
MATCHER_P(Kind, K, "") { return arg.kind == K; }
MATCHER_P(Filter, F, "") { return arg.filterText == F; }
+MATCHER_P(Doc, D, "") { return arg.documentation == D; }
+MATCHER_P(Detail, D, "") { return arg.detail == D; }
MATCHER_P(PlainText, Text, "") {
return arg.insertTextFormat ==
[PATCH] D41345: [clangd] Add more symbol information for code completion.
sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Just some nits
Comment at: clangd/index/Index.h:122
+
+ llvm::Optional Detail;
+
ioeric wrote:
> sammccall wrote:
> > ioeric wrote:
> > > sammccall wrote:
> > > > I think you probably want a raw pointer rather than optional:
> > > > - reduce the size of the struct when it's absent
> > > > - make it inheritance-friendly so we can hang index-specific info off
> > > > it
> > > > (raw pointer rather than unique_ptr because it's owned by a slab not by
> > > > malloc, but unique_ptr is ok for now)
> > > >
> > > This is not easy for now with `unique_ptr` because of this line :(
> > > https://github.com/llvm-mirror/clang-tools-extra/blob/3565d1a1a692fc9f5c21e634b470535da2bb4d25/clangd/index/SymbolYAML.cpp#L141).
> > >
> > >
> > > This shouldn't be an issue when we have the optimized symbol slab, where
> > > we store raw pointers. And we would probably want to serialize the whole
> > > slab instead of the individual symbols anyway.
> > >
> > > > reduce the size of the struct when it's absent
> > > `llvm::Optional` doesn't take much more space, so the size should be fine.
> > >
> > > > make it inheritance-friendly so we can hang index-specific info off it
> > > Could you elaborate on `index-specific info`? It's unclear to me how this
> > > would be used.
> > > This is not easy for now with unique_ptr because of this line
> > Oh no, somehow i missed this during review.
> > We shouldn't be relying on symbols being copyable. I'll try to work out how
> > to fix this and delete the copy constructor.
> >
> > > This shouldn't be an issue when we have the optimized symbol slab, where
> > > we store raw pointers.
> > Sure. That's not a big monolithic/mysterous thing though, storing the
> > details in the slab can be done in this patch... If you think it'll be
> > easier once strings are arena-based, then maybe we should delay this patch
> > until that's done, rather than make that work bigger.
> >
> > > And we would probably want to serialize the whole slab instead of the
> > > individual symbols anyway.
> > This i'm less sure about, but I don't think it matters.
> >
> > > llvm::Optional doesn't take much more space, so the size should be fine.
> > Optional takes the same size as the details itself (plus one bool). This is
> > fairly small for now, but I think a major point of Details is to expand it
> > in the future?
> >
> > > Could you elaborate on index-specific info? It's unclear to me how this
> > > would be used.
> > Yeah, this is something we talked about in the meeting with Marc-Andre but
> > it's not really obvious - what's the point of allowing Details to be
> > extended if clangd has to consume it?
> >
> > It sounded like he might have use cases for using index infrastructure
> > outside clangd. We might also have google-internal index features we want
> > (matching generated code to proto fields?). I'm not really sure how
> > compelling this argument is.
> Thanks for the allocator change!
>
> `Details` now contains just stringrefs. I wonder how much we want it to be
> inherit-friendly at this point, as the size is relatively small now. If you
> think this is a better way to go, I'll make the structure contain strings and
> store the whole structure in arena. This would require some tweaks for yamls
> tho but shouldn't be hard.
So this needs to be at least optional I think - at the moment, how does an API
user know whether to fetch the rest of the details?
FWIW, the size difference is already significant: symbol is 136 bytes if this
is a unique_ptr, and 164 bytes if it's optional (+20%) - StringRefs are still
pretty big.
But I don't feel really strongly about this.
Comment at: clangd/index/SymbolCollector.cpp:71
+SymbolCollector::SymbolCollector()
+: CompletionAllocator(std::make_shared()),
+ CompletionTUInfo(CompletionAllocator) {}
you shouldn't initialize these both here and in `initialize()`, right?
Comment at: clangd/index/SymbolCollector.cpp:140
+S.CompletionPlainInsertText = PlainInsertText;
+if (PlainInsertText != SnippetInsertText)
+ S.CompletionSnippetInsertText = SnippetInsertText;
why conditional? note we're interning the strings anyway, and an empty
stringref is the same size as a full one
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41345: [clangd] Add more symbol information for code completion.
ioeric added inline comments.
Comment at: clangd/index/Index.h:122
+
+ llvm::Optional Detail;
+
sammccall wrote:
> ioeric wrote:
> > sammccall wrote:
> > > I think you probably want a raw pointer rather than optional:
> > > - reduce the size of the struct when it's absent
> > > - make it inheritance-friendly so we can hang index-specific info off it
> > > (raw pointer rather than unique_ptr because it's owned by a slab not by
> > > malloc, but unique_ptr is ok for now)
> > >
> > This is not easy for now with `unique_ptr` because of this line :(
> > https://github.com/llvm-mirror/clang-tools-extra/blob/3565d1a1a692fc9f5c21e634b470535da2bb4d25/clangd/index/SymbolYAML.cpp#L141).
> >
> >
> > This shouldn't be an issue when we have the optimized symbol slab, where we
> > store raw pointers. And we would probably want to serialize the whole slab
> > instead of the individual symbols anyway.
> >
> > > reduce the size of the struct when it's absent
> > `llvm::Optional` doesn't take much more space, so the size should be fine.
> >
> > > make it inheritance-friendly so we can hang index-specific info off it
> > Could you elaborate on `index-specific info`? It's unclear to me how this
> > would be used.
> > This is not easy for now with unique_ptr because of this line
> Oh no, somehow i missed this during review.
> We shouldn't be relying on symbols being copyable. I'll try to work out how
> to fix this and delete the copy constructor.
>
> > This shouldn't be an issue when we have the optimized symbol slab, where we
> > store raw pointers.
> Sure. That's not a big monolithic/mysterous thing though, storing the details
> in the slab can be done in this patch... If you think it'll be easier once
> strings are arena-based, then maybe we should delay this patch until that's
> done, rather than make that work bigger.
>
> > And we would probably want to serialize the whole slab instead of the
> > individual symbols anyway.
> This i'm less sure about, but I don't think it matters.
>
> > llvm::Optional doesn't take much more space, so the size should be fine.
> Optional takes the same size as the details itself (plus one bool). This is
> fairly small for now, but I think a major point of Details is to expand it in
> the future?
>
> > Could you elaborate on index-specific info? It's unclear to me how this
> > would be used.
> Yeah, this is something we talked about in the meeting with Marc-Andre but
> it's not really obvious - what's the point of allowing Details to be extended
> if clangd has to consume it?
>
> It sounded like he might have use cases for using index infrastructure
> outside clangd. We might also have google-internal index features we want
> (matching generated code to proto fields?). I'm not really sure how
> compelling this argument is.
Thanks for the allocator change!
`Details` now contains just stringrefs. I wonder how much we want it to be
inherit-friendly at this point, as the size is relatively small now. If you
think this is a better way to go, I'll make the structure contain strings and
store the whole structure in arena. This would require some tweaks for yamls
tho but shouldn't be hard.
Comment at: clangd/index/SymbolCollector.h:25
// changed.
class SymbolCollector : public index::IndexDataConsumer {
public:
sammccall wrote:
> can you add a comment to the class indicating that it needs to be used for
> one TU and then thrown away? This seems unfortunate but is probably simpler
> than the alternative. It also seems to be a new restriction with this patch.
How about re-initializing the allocator for each new AST in `intialize`?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41345: [clangd] Add more symbol information for code completion.
ioeric updated this revision to Diff 128505.
ioeric added a comment.
- Merge with origin/master. Use Arena for symbol details.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/CodeComplete.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
clangd/index/SymbolYAML.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -9,7 +9,6 @@
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
#include "clang/Basic/VirtualFileSystem.h"
@@ -26,11 +25,21 @@
#include
#include
+using testing::AllOf;
using testing::Eq;
using testing::Field;
using testing::UnorderedElementsAre;
// GMock helpers for matching Symbol.
+MATCHER_P(Labeled, Label, "") { return arg.CompletionLabel == Label; }
+MATCHER_P(Detail, D, "") { return arg.Detail.CompletionDetail == D; }
+MATCHER_P(Doc, D, "") { return arg.Detail.Documentation == D; }
+MATCHER_P(Plain, Text, "") {
+ return arg.CompletionPlainInsertText == Text;
+}
+MATCHER_P(Snippet, S, "") {
+ return arg.CompletionSnippetInsertText == S;
+}
MATCHER_P(QName, Name, "") {
return (arg.Scope + (arg.Scope.empty() ? "" : "::") + arg.Name).str() == Name;
}
@@ -109,6 +118,38 @@
QName("f1"), QName("f2")));
}
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+ const std::string Main = R"(
+namespace nx {
+/// Foo comment.
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("nx"),
+ AllOf(QName("nx::ff"),
+ Labeled("ff(int x, double y)"),
+ Detail("int"), Doc("Foo comment.";
+}
+
+TEST_F(SymbolCollectorTest, PlainAndSnippet) {
+ const std::string Main = R"(
+namespace nx {
+void f() {}
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ QName("nx"),
+ AllOf(QName("nx::f"), Labeled("f()"), Plain("f"), Snippet("f()")),
+ AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Plain("ff"),
+Snippet("ff(${1:int x}, ${2:double y})";
+}
+
TEST_F(SymbolCollectorTest, YAMLConversions) {
const std::string YAML1 = R"(
---
@@ -122,6 +163,12 @@
StartOffset: 0
EndOffset: 1
FilePath:/path/foo.h
+CompletionLabel:'Foo1-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+Detail:
+ Documentation:'Foo doc'
+ CompletionDetail:'int'
...
)";
const std::string YAML2 = R"(
@@ -136,15 +183,21 @@
StartOffset: 10
EndOffset: 12
FilePath:/path/foo.h
+CompletionLabel:'Foo2-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+CompletionSnippetInsertText:'snippet'
...
)";
auto Symbols1 = SymbolFromYAML(YAML1);
- EXPECT_THAT(Symbols1,
- UnorderedElementsAre(QName("clang::Foo1")));
+ EXPECT_THAT(Symbols1, UnorderedElementsAre(
+AllOf(QName("clang::Foo1"), Labeled("Foo1-label"),
+ Doc("Foo doc"), Detail("int";
auto Symbols2 = SymbolFromYAML(YAML2);
- EXPECT_THAT(Symbols2,
- UnorderedElementsAre(QName("clang::Foo2")));
+ EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(QName("clang::Foo2"),
+ Labeled("Foo2-label"),
+ Doc(""), Detail("";
std::string ConcatenatedYAML =
SymbolToYAML(Symbols1) + SymbolToYAML(Symbols2);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -68,6 +68,8 @@
MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
MATCHER_P(Kind, K, "") { return arg.kind == K; }
MATCHER_P(Filter, F, "") { return arg.filterText == F; }
+MATCHER_P(Doc, D, "") { return arg.documentation == D; }
+MATCHER_P(Detail, D, "") { return arg.detail == D; }
MATCHER_P(PlainText, Text, "") {
return arg.insertTextFormat == clangd::InsertTextFormat::PlainText &&
arg.insertText == Text;
@@ -472,6 +474,7 @@
Sym.Name = QName.substr(Pos + 2);
Sym.Scope = QName.substr(0,
[PATCH] D41345: [clangd] Add more symbol information for code completion.
sammccall added inline comments.
Comment at: clangd/index/Index.h:122
+
+ llvm::Optional Detail;
+
ioeric wrote:
> sammccall wrote:
> > I think you probably want a raw pointer rather than optional:
> > - reduce the size of the struct when it's absent
> > - make it inheritance-friendly so we can hang index-specific info off it
> > (raw pointer rather than unique_ptr because it's owned by a slab not by
> > malloc, but unique_ptr is ok for now)
> >
> This is not easy for now with `unique_ptr` because of this line :(
> https://github.com/llvm-mirror/clang-tools-extra/blob/3565d1a1a692fc9f5c21e634b470535da2bb4d25/clangd/index/SymbolYAML.cpp#L141).
>
>
> This shouldn't be an issue when we have the optimized symbol slab, where we
> store raw pointers. And we would probably want to serialize the whole slab
> instead of the individual symbols anyway.
>
> > reduce the size of the struct when it's absent
> `llvm::Optional` doesn't take much more space, so the size should be fine.
>
> > make it inheritance-friendly so we can hang index-specific info off it
> Could you elaborate on `index-specific info`? It's unclear to me how this
> would be used.
> This is not easy for now with unique_ptr because of this line
Oh no, somehow i missed this during review.
We shouldn't be relying on symbols being copyable. I'll try to work out how to
fix this and delete the copy constructor.
> This shouldn't be an issue when we have the optimized symbol slab, where we
> store raw pointers.
Sure. That's not a big monolithic/mysterous thing though, storing the details
in the slab can be done in this patch... If you think it'll be easier once
strings are arena-based, then maybe we should delay this patch until that's
done, rather than make that work bigger.
> And we would probably want to serialize the whole slab instead of the
> individual symbols anyway.
This i'm less sure about, but I don't think it matters.
> llvm::Optional doesn't take much more space, so the size should be fine.
Optional takes the same size as the details itself (plus one bool). This is
fairly small for now, but I think a major point of Details is to expand it in
the future?
> Could you elaborate on index-specific info? It's unclear to me how this would
> be used.
Yeah, this is something we talked about in the meeting with Marc-Andre but it's
not really obvious - what's the point of allowing Details to be extended if
clangd has to consume it?
It sounded like he might have use cases for using index infrastructure outside
clangd. We might also have google-internal index features we want (matching
generated code to proto fields?). I'm not really sure how compelling this
argument is.
Comment at: clangd/index/SymbolCollector.h:25
// changed.
class SymbolCollector : public index::IndexDataConsumer {
public:
can you add a comment to the class indicating that it needs to be used for one
TU and then thrown away? This seems unfortunate but is probably simpler than
the alternative. It also seems to be a new restriction with this patch.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41345: [clangd] Add more symbol information for code completion.
ioeric added inline comments. Comment at: clangd/index/Index.h:105 + /// What to insert when completing this symbol (plain text version). + std::string CompletionPlainInsertText; + /// What to insert when completing this symbol (snippet version). sammccall wrote: > insert texts can be in details I think? They're not required for completion > until after resolve. Quote from https://github.com/Microsoft/language-server-protocol/blob/master/protocol.md#completion-request ``` The request can delay the computation od the detail and documentation properties. However, properties that are needed for the inital sorting and filtering, like sortText, filterText, insertText, and textEdit must be provided in the textDocument/completion request and must not be changed during resolve. ``` Comment at: clangd/index/Index.h:122 + + llvm::Optional Detail; + sammccall wrote: > I think you probably want a raw pointer rather than optional: > - reduce the size of the struct when it's absent > - make it inheritance-friendly so we can hang index-specific info off it > (raw pointer rather than unique_ptr because it's owned by a slab not by > malloc, but unique_ptr is ok for now) > This is not easy for now with `unique_ptr` because of this line :( https://github.com/llvm-mirror/clang-tools-extra/blob/3565d1a1a692fc9f5c21e634b470535da2bb4d25/clangd/index/SymbolYAML.cpp#L141). This shouldn't be an issue when we have the optimized symbol slab, where we store raw pointers. And we would probably want to serialize the whole slab instead of the individual symbols anyway. > reduce the size of the struct when it's absent `llvm::Optional` doesn't take much more space, so the size should be fine. > make it inheritance-friendly so we can hang index-specific info off it Could you elaborate on `index-specific info`? It's unclear to me how this would be used. Comment at: clangd/index/SymbolCollector.cpp:65 + CodeCompletionResult SymbolCompletion(ND, 0); + auto Allocator = std::make_shared(); + CodeCompletionTUInfo TUInfo(Allocator); sammccall wrote: > this doesn't seem like the kind of thing we should be allocating per-symbol! > You can use a single one and Reset() it, I think > > also why globalcodecompletionallocator, vs just codecompletionallocator? > You can use a single one and Reset() it, I think It's unclear how reset would affect `CodeCompletionTUInfo`, but we can simply maintain one allocator for each TU. > also why globalcodecompletionallocator, vs just codecompletionallocator? They are actually the same thing, but `CodeCompletionTUInfo` requires a global one. I don't really know why... Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41345 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41345: [clangd] Add more symbol information for code completion.
ioeric updated this revision to Diff 127781.
ioeric marked an inline comment as done.
ioeric added a comment.
- Merged with origin/master
- Addressed some more comments.
- Add new fields to YAML.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/CodeComplete.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
clangd/index/SymbolYAML.cpp
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -9,7 +9,6 @@
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
#include "clang/Basic/VirtualFileSystem.h"
@@ -26,11 +25,25 @@
#include
#include
+using testing::AllOf;
using testing::Eq;
using testing::Field;
using testing::UnorderedElementsAre;
// GMock helpers for matching Symbol.
+MATCHER_P(Labeled, Label, "") { return arg.second.CompletionLabel == Label; }
+MATCHER_P(Detail, D, "") {
+ return arg.second.Detail && arg.second.Detail->CompletionDetail == D;
+}
+MATCHER_P(Doc, D, "") {
+ return arg.second.Detail && arg.second.Detail->Documentation == D;
+}
+MATCHER_P(Plain, Text, "") {
+ return arg.second.CompletionPlainInsertText == Text;
+}
+MATCHER_P(Snippet, S, "") {
+ return arg.second.CompletionSnippetInsertText == S;
+}
MATCHER_P(QName, Name, "") {
return (arg.second.Scope + (arg.second.Scope.empty() ? "" : "::") +
arg.second.Name) == Name;
@@ -110,6 +123,38 @@
QName("f1"), QName("f2")));
}
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+ const std::string Main = R"(
+namespace nx {
+/// Foo comment.
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("nx"),
+ AllOf(QName("nx::ff"),
+ Labeled("ff(int x, double y)"),
+ Detail("int"), Doc("Foo comment.";
+}
+
+TEST_F(SymbolCollectorTest, PlainAndSnippet) {
+ const std::string Main = R"(
+namespace nx {
+void f() {}
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ QName("nx"),
+ AllOf(QName("nx::f"), Labeled("f()"), Plain("f"), Snippet("f()")),
+ AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Plain("ff"),
+Snippet("ff(${1:int x}, ${2:double y})";
+}
+
TEST_F(SymbolCollectorTest, YAMLConversions) {
const std::string YAML1 = R"(
---
@@ -123,6 +168,13 @@
StartOffset: 0
EndOffset: 1
FilePath:/path/foo.h
+CompletionLabel:'Foo1-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+CompletionSnippetInsertText:'snippet'
+Detail:
+ Documentation:'Foo doc'
+ CompletionDetail:'int'
...
)";
const std::string YAML2 = R"(
@@ -137,15 +189,22 @@
StartOffset: 10
EndOffset: 12
FilePath:/path/foo.h
+CompletionLabel:'Foo2-label'
+CompletionFilterText:'filter'
+CompletionPlainInsertText:'plain'
+CompletionSnippetInsertText:'snippet'
...
)";
auto Symbols1 = SymbolFromYAML(YAML1);
- EXPECT_THAT(Symbols1,
- UnorderedElementsAre(QName("clang::Foo1")));
+ EXPECT_THAT(Symbols1, UnorderedElementsAre(
+AllOf(QName("clang::Foo1"), Labeled("Foo1-label"),
+ Doc("Foo doc"), Detail("int";
+ EXPECT_TRUE(Symbols1.begin()->second.Detail);
auto Symbols2 = SymbolFromYAML(YAML2);
- EXPECT_THAT(Symbols2,
- UnorderedElementsAre(QName("clang::Foo2")));
+ EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf(QName("clang::Foo2"),
+ Labeled("Foo2-label";
+ EXPECT_FALSE(Symbols2.begin()->second.Detail);
std::string ConcatenatedYAML =
SymbolToYAML(Symbols1) + SymbolToYAML(Symbols2);
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -67,6 +67,8 @@
MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
MATCHER_P(Kind, K, "") { return arg.kind == K; }
MATCHER_P(Filter, F, "") { return arg.filterText == F; }
+MATCHER_P(Doc, D, "") { return arg.documentation == D; }
+MATCHER_P(Detail, D, "") { return arg.detail == D; }
[PATCH] D41345: [clangd] Add more symbol information for code completion.
sammccall added inline comments. Comment at: clangd/index/Index.h:105 + /// What to insert when completing this symbol (plain text version). + std::string CompletionPlainInsertText; + /// What to insert when completing this symbol (snippet version). insert texts can be in details I think? They're not required for completion until after resolve. Comment at: clangd/index/Index.h:122 + + llvm::Optional Detail; + I think you probably want a raw pointer rather than optional: - reduce the size of the struct when it's absent - make it inheritance-friendly so we can hang index-specific info off it (raw pointer rather than unique_ptr because it's owned by a slab not by malloc, but unique_ptr is ok for now) Comment at: clangd/index/SymbolCollector.cpp:65 + CodeCompletionResult SymbolCompletion(ND, 0); + auto Allocator = std::make_shared(); + CodeCompletionTUInfo TUInfo(Allocator); this doesn't seem like the kind of thing we should be allocating per-symbol! You can use a single one and Reset() it, I think also why globalcodecompletionallocator, vs just codecompletionallocator? Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D41345 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41345: [clangd] Add more symbol information for code completion.
ioeric added a comment.
Thanks for the review!
Logic around CodeCompletionString is pulled into a separate file in
https://reviews.llvm.org/D41450.
I also propagate the new completion info to completion code. I am happy to
split it out if it adds too much noise for the review.
Comment at: clangd/index/Index.h:92
+ // Documentation including comment for the symbol declaration.
+ std::string Documentation;
sammccall wrote:
> AFAIK this information isn't needed for retrieval/scoring, just for display.
>
> LSP has `completionItem/resolve` that adds additional info to a completion
> item. This allows us to avoid sending a bunch of bulky comments, most of
> which will never be looked at.
>
> In practice, there's nothing we particularly want to do differently for the
> memory index: we have to load the data into memory, and so including a
> pointer to it right away is no extra work.
>
> However Symbol has two roles, and being the in-memory representation for
> MemIndex is only secondary. Its primary role is defining the protocol between
> indexes and clangd, including remote indexes where returning all
> documentation *is* expensive.
>
> One option is to have Symbol just have the "core stuff" that's suitable for
> returning in bulk, and have an index method to retrieve more details that
> would be a point lookup only. (Maybe this is just the getSymbol method we've
> thought about). I'm not sure what it means for the data structure. OK if we
> discuss offline?
As discussed offline, putting non-core stuff in an optional structure.
Comment at: clangd/index/Index.h:100
+ std::string CompletionDetail;
+ // The placeholder text for function parameters in order.
+ std::vector Params;
sammccall wrote:
> How are you planning to use this?
>
> This seems to be related to the completion text/edits. We had some early
> discussions about whether we'd encode something like CodeCompletionString, or
> LSP snippets, or something else entirely.
> Honestly it would be great to have a doc describing this mapping between
> source -> index -> LSP for completion data.
As discussed offline, we now store the whole snippet in the insertion text.
Comment at: clangd/index/SymbolCollector.cpp:61
+
+std::string getDocumentation(const CodeCompletionString ) {
+ // Things like __attribute__((nonnull(1,3))) and [[noreturn]]. Present this
sammccall wrote:
> it seems we'll want to share the(some?) doc logic between hover, AST-based
> complete, and indexing... See D35894 (which is ... complicated, no idea if
> it'll land soon).
>
> Among other things:
> - we may not want to make the logic too elaborate until we're able to merge
> interfaces
> - we may want to consume AST nodes rather than CCS in the long run
I pulled `CodeCompletionString` handling logic into a separate file in `D41450`.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41345: [clangd] Add more symbol information for code completion.
ioeric updated this revision to Diff 127730.
ioeric marked 4 inline comments as done.
ioeric added a comment.
- Merge with origin/master
- Fixed an error in merge
- Make documentation etc optional in symbols
- Merge remote-tracking branch 'origin/master' into symbol
- Merge branch 'index-completion' into symbol
- Address review comments; merge with https://reviews.llvm.org/D41450.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/CodeComplete.cpp
clangd/index/FileIndex.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
unittests/clangd/CodeCompleteTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -9,7 +9,6 @@
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
#include "clang/Basic/VirtualFileSystem.h"
@@ -26,11 +25,25 @@
#include
#include
+using testing::AllOf;
using testing::Eq;
using testing::Field;
using testing::UnorderedElementsAre;
// GMock helpers for matching Symbol.
+MATCHER_P(Labeled, Label, "") { return arg.second.CompletionLabel == Label; }
+MATCHER_P(Detail, D, "") {
+ return arg.second.Detail && arg.second.Detail->CompletionDetail == D;
+}
+MATCHER_P(Doc, D, "") {
+ return arg.second.Detail && arg.second.Detail->Documentation == D;
+}
+MATCHER_P(Plain, Text, "") {
+ return arg.second.CompletionPlainInsertText == Text;
+}
+MATCHER_P(Snippet, S, "") {
+ return arg.second.CompletionSnippetInsertText == S;
+}
MATCHER_P(QName, Name, "") {
return (arg.second.Scope + (arg.second.Scope.empty() ? "" : "::") +
arg.second.Name) == Name;
@@ -110,6 +123,38 @@
QName("f1"), QName("f2")));
}
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+ const std::string Main = R"(
+namespace nx {
+/// Foo comment.
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("nx"),
+ AllOf(QName("nx::ff"),
+ Labeled("ff(int x, double y)"),
+ Detail("int"), Doc("Foo comment.";
+}
+
+TEST_F(SymbolCollectorTest, PlainAndSnippet) {
+ const std::string Main = R"(
+namespace nx {
+void f() {}
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ QName("nx"),
+ AllOf(QName("nx::f"), Labeled("f()"), Plain("f()"), Snippet("")),
+ AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Plain("ff"),
+Snippet("ff(${1:int x}, ${2:double y})";
+}
+
TEST_F(SymbolCollectorTest, YAMLConversions) {
const std::string YAML1 = R"(
---
Index: unittests/clangd/CodeCompleteTests.cpp
===
--- unittests/clangd/CodeCompleteTests.cpp
+++ unittests/clangd/CodeCompleteTests.cpp
@@ -86,6 +86,8 @@
MATCHER_P(Labeled, Label, "") { return arg.label == Label; }
MATCHER_P(Kind, K, "") { return arg.kind == K; }
MATCHER_P(Filter, F, "") { return arg.filterText == F; }
+MATCHER_P(Doc, D, "") { return arg.documentation == D; }
+MATCHER_P(Detail, D, "") { return arg.detail == D; }
MATCHER_P(PlainText, Text, "") {
return arg.insertTextFormat == clangd::InsertTextFormat::PlainText &&
arg.insertText == Text;
@@ -480,6 +482,7 @@
Sym.Name = QName.substr(Pos + 2);
Sym.Scope = QName.substr(0, Pos);
}
+Sym.CompletionPlainInsertText = Sym.Name;
Sym.SymInfo.Kind = Pair.second;
Snap->Slab.insert(std::move(Sym));
}
@@ -530,7 +533,7 @@
void f() { ns::x^ }
)cpp",
Opts);
- EXPECT_THAT(Results.items, Contains(AllOf(Named("XYZ"), Filter("x";
+ EXPECT_THAT(Results.items, Contains(Named("XYZ")));
EXPECT_THAT(Results.items, Not(Has("foo")));
}
@@ -568,13 +571,17 @@
Server
.addDocument(Context::empty(), getVirtualTestFilePath("foo.cpp"), R"cpp(
- namespace ns { class XYZ {}; void foo() {} }
+ namespace ns { class XYZ {}; void foo(int x) {} }
)cpp")
.wait();
auto File = getVirtualTestFilePath("bar.cpp");
auto Test = parseTextMarker(R"cpp(
- namespace ns { class XXX {}; void fo() {} }
+ namespace ns {
+ class XXX {};
+ /// Doooc
+ void fo() {}
+ }
void f() { ns::^ }
)cpp");
Server.addDocument(Context::empty(), File, Test.Text).wait();
@@ -587,7 +594,9 @@
[PATCH] D41345: [clangd] Add more symbol information for code completion.
sammccall added inline comments.
Comment at: clangd/index/Index.h:92
+ // Documentation including comment for the symbol declaration.
+ std::string Documentation;
AFAIK this information isn't needed for retrieval/scoring, just for display.
LSP has `completionItem/resolve` that adds additional info to a completion
item. This allows us to avoid sending a bunch of bulky comments, most of which
will never be looked at.
In practice, there's nothing we particularly want to do differently for the
memory index: we have to load the data into memory, and so including a pointer
to it right away is no extra work.
However Symbol has two roles, and being the in-memory representation for
MemIndex is only secondary. Its primary role is defining the protocol between
indexes and clangd, including remote indexes where returning all documentation
*is* expensive.
One option is to have Symbol just have the "core stuff" that's suitable for
returning in bulk, and have an index method to retrieve more details that would
be a point lookup only. (Maybe this is just the getSymbol method we've thought
about). I'm not sure what it means for the data structure. OK if we discuss
offline?
Comment at: clangd/index/Index.h:99
+ // Detail about the symbol. For example, the result type of a function.
+ std::string CompletionDetail;
+ // The placeholder text for function parameters in order.
What are you planning to put here other than the return type of a function?
It's probably OK if we explicitly want this to be "whatever should go in the
LSP detail field" but we should think about whether there's something more
specific we can say.
Comment at: clangd/index/Index.h:100
+ std::string CompletionDetail;
+ // The placeholder text for function parameters in order.
+ std::vector Params;
How are you planning to use this?
This seems to be related to the completion text/edits. We had some early
discussions about whether we'd encode something like CodeCompletionString, or
LSP snippets, or something else entirely.
Honestly it would be great to have a doc describing this mapping between source
-> index -> LSP for completion data.
Comment at: clangd/index/SymbolCollector.cpp:61
+
+std::string getDocumentation(const CodeCompletionString ) {
+ // Things like __attribute__((nonnull(1,3))) and [[noreturn]]. Present this
it seems we'll want to share the(some?) doc logic between hover, AST-based
complete, and indexing... See D35894 (which is ... complicated, no idea if
it'll land soon).
Among other things:
- we may not want to make the logic too elaborate until we're able to merge
interfaces
- we may want to consume AST nodes rather than CCS in the long run
Comment at: clangd/index/SymbolCollector.cpp:67
+ if (AnnotationCount > 0) {
+Result += "Annotation";
+if (AnnotationCount == 1) {
Should these annotations go at the end?
Comment at: clangd/index/SymbolCollector.cpp:90
+
+void ProcessChunks(const CodeCompletionString , Symbol *Sym) {
+ for (const auto : CCS) {
nit: lowercase
Comment at: clangd/index/SymbolCollector.cpp:90
+
+void ProcessChunks(const CodeCompletionString , Symbol *Sym) {
+ for (const auto : CCS) {
sammccall wrote:
> nit: lowercase
How does this relate to the code in AST-based completion?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D41345: [clangd] Add more symbol information for code completion.
ioeric created this revision.
ioeric added reviewers: hokein, sammccall.
Herald added subscribers: cfe-commits, ilya-biryukov, klimek.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41345
Files:
clangd/ClangdUnit.cpp
clangd/ClangdUnit.h
clangd/index/FileIndex.cpp
clangd/index/Index.h
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -9,7 +9,6 @@
#include "index/SymbolCollector.h"
#include "index/SymbolYAML.h"
-#include "clang/Index/IndexingAction.h"
#include "clang/Basic/FileManager.h"
#include "clang/Basic/FileSystemOptions.h"
#include "clang/Basic/VirtualFileSystem.h"
@@ -26,12 +25,17 @@
#include
#include
+using testing::AllOf;
using testing::Eq;
using testing::Field;
using testing::UnorderedElementsAre;
// GMock helpers for matching Symbol.
MATCHER_P(QName, Name, "") { return arg.second.QualifiedName == Name; }
+MATCHER_P(Labeled, Label, "") { return arg.second.CompletionLabel == Label; }
+MATCHER_P(Detail, D, "") { return arg.second.CompletionDetail == D; }
+MATCHER_P(Doc, D, "") { return arg.second.Documentation == D; }
+MATCHER_P(Params, P, "") { return arg.second.Params == P; }
namespace clang {
namespace clangd {
@@ -107,6 +111,23 @@
QName("f1"), QName("f2")));
}
+TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+ const std::string Main = R"(
+namespace nx {
+/// Foo comment.
+int ff(int x, double y) { return 0; }
+}
+ )";
+ runSymbolCollector(/*Header=*/"", Main);
+ EXPECT_THAT(
+ Symbols,
+ UnorderedElementsAre(
+ QName("nx"),
+ AllOf(QName("nx::ff"), Labeled("ff(int x, double y)"), Detail("int"),
+Doc("Foo comment."),
+Params(std::vector({"int x", "double y"});
+}
+
TEST_F(SymbolCollectorTest, YAMLConversions) {
const std::string YAML1 = R"(
---
Index: clangd/index/SymbolCollector.h
===
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -23,6 +23,12 @@
public:
SymbolCollector() = default;
+ void initialize(ASTContext ) override { ASTCtx = }
+
+ void setPreprocessor(std::shared_ptr PP) override {
+this->PP = std::move(PP);
+ }
+
bool
handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles,
ArrayRef Relations, FileID FID,
@@ -36,6 +42,8 @@
private:
// All Symbols collected from the AST.
SymbolSlab Symbols;
+ ASTContext *ASTCtx;
+ std::shared_ptr PP;
};
} // namespace clangd
Index: clangd/index/SymbolCollector.cpp
===
--- clangd/index/SymbolCollector.cpp
+++ clangd/index/SymbolCollector.cpp
@@ -14,6 +14,7 @@
#include "clang/Basic/SourceManager.h"
#include "clang/Index/IndexSymbol.h"
#include "clang/Index/USRGeneration.h"
+#include "clang/Sema/CodeCompleteConsumer.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/Path.h"
@@ -56,6 +57,79 @@
}
return AbsolutePath.str();
}
+
+std::string getDocumentation(const CodeCompletionString ) {
+ // Things like __attribute__((nonnull(1,3))) and [[noreturn]]. Present this
+ // information in the documentation field.
+ std::string Result;
+ const unsigned AnnotationCount = CCS.getAnnotationCount();
+ if (AnnotationCount > 0) {
+Result += "Annotation";
+if (AnnotationCount == 1) {
+ Result += ": ";
+} else /* AnnotationCount > 1 */ {
+ Result += "s: ";
+}
+for (unsigned I = 0; I < AnnotationCount; ++I) {
+ Result += CCS.getAnnotation(I);
+ Result.push_back(I == AnnotationCount - 1 ? '\n' : ' ');
+}
+ }
+ // Add brief documentation (if there is any).
+ if (CCS.getBriefComment() != nullptr) {
+if (!Result.empty()) {
+ // This means we previously added annotations. Add an extra newline
+ // character to make the annotations stand out.
+ Result.push_back('\n');
+}
+Result += CCS.getBriefComment();
+ }
+ return Result;
+}
+
+void ProcessChunks(const CodeCompletionString , Symbol *Sym) {
+ for (const auto : CCS) {
+// Informative qualifier chunks only clutter completion results, skip
+// them.
+if (Chunk.Kind == CodeCompletionString::CK_Informative &&
+StringRef(Chunk.Text).endswith("::"))
+ continue;
+
+switch (Chunk.Kind) {
+case CodeCompletionString::CK_TypedText:
+ // There's always exactly one CK_TypedText chunk.
+ Sym->CompletionLabel += Chunk.Text;
+ break;
+case CodeCompletionString::CK_ResultType:
+ Sym->CompletionDetail = Chunk.Text;
+ break;
+case CodeCompletionString::CK_Optional:
+ break;
+case
