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 <memory> #include <string> +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, Pos); } + Sym.CompletionPlainInsertText = Sym.Name; Sym.SymInfo.Kind = Pair.second; Slab.insert(Sym); } @@ -561,13 +564,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"); Annotations Test(R"cpp( - namespace ns { class XXX {}; void fooooo() {} } + namespace ns { + class XXX {}; + /// Doooc + void fooooo() {} + } void f() { ns::^ } )cpp"); Server.addDocument(Context::empty(), File, Test.code()).wait(); @@ -580,7 +587,9 @@ EXPECT_THAT(Results.items, Has("XYZ", CompletionItemKind::Class)); EXPECT_THAT(Results.items, Has("foo", CompletionItemKind::Function)); EXPECT_THAT(Results.items, Has("XXX", CompletionItemKind::Class)); - EXPECT_THAT(Results.items, Has("fooooo", CompletionItemKind::Function)); + EXPECT_THAT(Results.items, Contains(AllOf(Named("fooooo"), Filter("fooooo"), + Kind(CompletionItemKind::Function), + Doc("Doooc"), Detail("void")))); } } // namespace Index: clangd/index/SymbolYAML.cpp =================================================================== --- clangd/index/SymbolYAML.cpp +++ clangd/index/SymbolYAML.cpp @@ -59,6 +59,13 @@ } }; +template <> struct MappingTraits<Symbol::Details> { + static void mapping(IO &io, Symbol::Details &Detail) { + io.mapOptional("Documentation", Detail.Documentation); + io.mapOptional("CompletionDetail", Detail.CompletionDetail); + } +}; + template<> struct MappingTraits<Symbol> { static void mapping(IO &IO, Symbol &Sym) { MappingNormalization<NormalizedSymbolID, SymbolID> NSymbolID( @@ -68,6 +75,13 @@ IO.mapRequired("Scope", Sym.Scope); IO.mapRequired("SymInfo", Sym.SymInfo); IO.mapRequired("CanonicalDeclaration", Sym.CanonicalDeclaration); + IO.mapRequired("CompletionLabel", Sym.CompletionLabel); + IO.mapRequired("CompletionFilterText", Sym.CompletionFilterText); + IO.mapRequired("CompletionPlainInsertText", Sym.CompletionPlainInsertText); + + IO.mapOptional("CompletionSnippetInsertText", + Sym.CompletionSnippetInsertText); + IO.mapOptional("Detail", Sym.Detail); } }; Index: clangd/index/SymbolCollector.h =================================================================== --- clangd/index/SymbolCollector.h +++ clangd/index/SymbolCollector.h @@ -8,8 +8,11 @@ //===----------------------------------------------------------------------===// #include "Index.h" +#include "clang/AST/ASTContext.h" +#include "clang/AST/Decl.h" #include "clang/Index/IndexDataConsumer.h" #include "clang/Index/IndexSymbol.h" +#include "clang/Sema/CodeCompleteConsumer.h" namespace clang { namespace clangd { @@ -21,7 +24,13 @@ // changed. class SymbolCollector : public index::IndexDataConsumer { public: - SymbolCollector() = default; + SymbolCollector(); + + void initialize(ASTContext &Ctx) override; + + void setPreprocessor(std::shared_ptr<Preprocessor> PP) override { + this->PP = std::move(PP); + } bool handleDeclOccurence(const Decl *D, index::SymbolRoleSet Roles, @@ -34,6 +43,10 @@ private: // All Symbols collected from the AST. SymbolSlab::Builder Symbols; + ASTContext *ASTCtx; + std::shared_ptr<Preprocessor> PP; + std::shared_ptr<GlobalCodeCompletionAllocator> CompletionAllocator; + CodeCompletionTUInfo CompletionTUInfo; }; } // namespace clangd Index: clangd/index/SymbolCollector.cpp =================================================================== --- clangd/index/SymbolCollector.cpp +++ clangd/index/SymbolCollector.cpp @@ -8,8 +8,7 @@ //===----------------------------------------------------------------------===// #include "SymbolCollector.h" -#include "clang/AST/ASTContext.h" -#include "clang/AST/Decl.h" +#include "../CodeCompletionStrings.h" #include "clang/AST/DeclCXX.h" #include "clang/Basic/SourceManager.h" #include "clang/Index/IndexSymbol.h" @@ -56,7 +55,6 @@ return AbsolutePath.str(); } -// Split a qualified symbol name into scope and unqualified name, e.g. given // "a::b::c", return {"a::b", "c"}. Scope is empty if it doesn't exist. std::pair<llvm::StringRef, llvm::StringRef> splitQualifiedName(llvm::StringRef QName) { @@ -69,6 +67,16 @@ } // namespace +SymbolCollector::SymbolCollector() + : CompletionAllocator(std::make_shared<GlobalCodeCompletionAllocator>()), + CompletionTUInfo(CompletionAllocator) {} + +void SymbolCollector::initialize(ASTContext &Ctx) { + ASTCtx = &Ctx; + CompletionAllocator = std::make_shared<GlobalCodeCompletionAllocator>(); + CompletionTUInfo = CodeCompletionTUInfo(CompletionAllocator); +} + // Always return true to continue indexing. bool SymbolCollector::handleDeclOccurence( const Decl *D, index::SymbolRoleSet Roles, @@ -106,6 +114,34 @@ S.Name = ScopeAndName.second; S.SymInfo = index::getSymbolInfo(D); S.CanonicalDeclaration = Location; + + // Add completion info. + assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set."); + CodeCompletionResult SymbolCompletion(ND, 0); + const auto *CCS = SymbolCompletion.CreateCodeCompletionString( + *ASTCtx, *PP, CodeCompletionContext::CCC_Name, *CompletionAllocator, + CompletionTUInfo, + /*IncludeBriefComments*/ true); + std::string Label; + std::string SnippetInsertText; + std::string IgnoredLabel; + std::string PlainInsertText; + getLabelAndInsertText(*CCS, &Label, &SnippetInsertText, + /*EnableSnippets=*/true); + getLabelAndInsertText(*CCS, &IgnoredLabel, &PlainInsertText, + /*EnableSnippets=*/false); + std::string FilterText = getFilterText(*CCS); + std::string Documentation = getDocumentation(*CCS); + std::string CompletionDetail = getDetail(*CCS); + + S.CompletionFilterText = FilterText; + S.CompletionLabel = Label; + S.CompletionPlainInsertText = PlainInsertText; + if (PlainInsertText != SnippetInsertText) + S.CompletionSnippetInsertText = SnippetInsertText; + S.Detail.Documentation = Documentation; + S.Detail.CompletionDetail = CompletionDetail; + Symbols.insert(S); } Index: clangd/index/Index.h =================================================================== --- clangd/index/Index.h +++ clangd/index/Index.h @@ -15,6 +15,7 @@ #include "llvm/ADT/DenseMap.h" #include "llvm/ADT/DenseSet.h" #include "llvm/ADT/Hashing.h" +#include "llvm/ADT/Optional.h" #include "llvm/ADT/StringExtras.h" #include <array> #include <string> @@ -126,10 +127,38 @@ // (not a definition), which is usually declared in ".h" file. SymbolLocation CanonicalDeclaration; + /// A brief description of the symbol that can be displayed in the completion + /// candidate list. For example, "Foo(X x, Y y) const" is a labal for a + /// function. + llvm::StringRef CompletionLabel; + /// The piece of text that the user is expected to type to match the + /// code-completion string, typically a keyword or the name of a declarator or + /// macro. + llvm::StringRef CompletionFilterText; + /// What to insert when completing this symbol (plain text version). + llvm::StringRef CompletionPlainInsertText; + /// What to insert when completing this symbol (snippet version). This is + /// empty if it is the same as the plain insert text above. + llvm::StringRef CompletionSnippetInsertText; + + /// Optional symbol details that are not required to be set. For example, an + /// index fuzzy match can return a large number of symbol candidates, and it + /// is preferable to send only core symbol information in the batched results + /// and have clients resolve full symbol information for a specific candidate + /// if needed. + struct Details { + // Documentation including comment for the symbol declaration. + llvm::StringRef Documentation; + // This is what goes into the LSP detail field in a completion item. For + // example, the result type of a function. + llvm::StringRef CompletionDetail; + }; + + Details Detail; + // FIXME: add definition location of the symbol. // FIXME: add all occurrences support. // FIXME: add extra fields for index scoring signals. - // FIXME: add code completion information. }; // An immutable symbol container that stores a set of symbols. Index: clangd/index/Index.cpp =================================================================== --- clangd/index/Index.cpp +++ clangd/index/Index.cpp @@ -55,6 +55,14 @@ Intern(S.Name); Intern(S.Scope); Intern(S.CanonicalDeclaration.FilePath); + + Intern(S.CompletionLabel); + Intern(S.CompletionFilterText); + Intern(S.CompletionPlainInsertText); + Intern(S.CompletionSnippetInsertText); + + Intern(S.Detail.Documentation); + Intern(S.Detail.CompletionDetail); } void SymbolSlab::Builder::insert(const Symbol &S) { Index: clangd/index/FileIndex.cpp =================================================================== --- clangd/index/FileIndex.cpp +++ clangd/index/FileIndex.cpp @@ -17,8 +17,10 @@ /// Retrieves namespace and class level symbols in \p Decls. std::unique_ptr<SymbolSlab> indexAST(ASTContext &Ctx, + std::shared_ptr<Preprocessor> PP, llvm::ArrayRef<const Decl *> Decls) { auto Collector = std::make_shared<SymbolCollector>(); + Collector->setPreprocessor(std::move(PP)); index::IndexingOptions IndexOpts; IndexOpts.SystemSymbolFilter = index::IndexingOptions::SystemSymbolFilterKind::All; @@ -67,7 +69,8 @@ if (!AST) { FSymbols.update(Path, nullptr); } else { - auto Slab = indexAST(AST->getASTContext(), AST->getTopLevelDecls()); + auto Slab = indexAST(AST->getASTContext(), AST->getPreprocessorPtr(), + AST->getTopLevelDecls()); FSymbols.update(Path, std::move(Slab)); } auto Symbols = FSymbols.allSymbols(); Index: clangd/CodeComplete.cpp =================================================================== --- clangd/CodeComplete.cpp +++ clangd/CodeComplete.cpp @@ -555,16 +555,17 @@ Item.kind = toCompletionItemKind(Sym.SymInfo.Kind); Item.label = Sym.Name; // FIXME(ioeric): support inserting/replacing scope qualifiers. - Item.insertText = Sym.Name; + // FIXME(ioeric): support snippets. + Item.insertText = Sym.CompletionPlainInsertText; Item.insertTextFormat = InsertTextFormat::PlainText; Item.filterText = Sym.Name; // FIXME(ioeric): sort symbols appropriately. Item.sortText = ""; - // FIXME(ioeric): use more symbol information (e.g. documentation, label) to - // populate the completion item. + Item.documentation = Sym.Detail.Documentation; + Item.detail = Sym.Detail.CompletionDetail; return Item; } Index: clangd/ClangdUnit.h =================================================================== --- clangd/ClangdUnit.h +++ clangd/ClangdUnit.h @@ -79,6 +79,7 @@ const ASTContext &getASTContext() const; Preprocessor &getPreprocessor(); + std::shared_ptr<Preprocessor> getPreprocessorPtr(); const Preprocessor &getPreprocessor() const; /// This function returns all top-level decls, including those that come Index: clangd/ClangdUnit.cpp =================================================================== --- clangd/ClangdUnit.cpp +++ clangd/ClangdUnit.cpp @@ -316,6 +316,10 @@ Preprocessor &ParsedAST::getPreprocessor() { return Clang->getPreprocessor(); } +std::shared_ptr<Preprocessor> ParsedAST::getPreprocessorPtr() { + return Clang->getPreprocessorPtr(); +} + const Preprocessor &ParsedAST::getPreprocessor() const { return Clang->getPreprocessor(); }
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits