Index: clang-tools-extra/trunk/clangd/CodeComplete.h =================================================================== --- clang-tools-extra/trunk/clangd/CodeComplete.h +++ clang-tools-extra/trunk/clangd/CodeComplete.h @@ -25,6 +25,7 @@ #include "clang/Tooling/CompilationDatabase.h" namespace clang { +class NamedDecl; class PCHContainerOperations; namespace clangd { @@ -82,6 +83,17 @@ IntrusiveRefCntPtr VFS, std::shared_ptr PCHs); +// For index-based completion, we only consider: +// * symbols in namespaces or translation unit scopes (e.g. no class +// members, no locals) +// * enum constants in unscoped enum decl (e.g. "red" in "enum {red};") +// * primary templates (no specializations) +// For the other cases, we let Clang do the completion because it does not +// need any non-local information and it will be much better at following +// lookup rules. Other symbols still appear in the index for other purposes, +// like workspace/symbols or textDocument/definition, but are not used for code +// completion. +bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx); } // namespace clangd } // namespace clang Index: clang-tools-extra/trunk/clangd/CodeComplete.cpp =================================================================== --- clang-tools-extra/trunk/clangd/CodeComplete.cpp +++ clang-tools-extra/trunk/clangd/CodeComplete.cpp @@ -25,6 +25,7 @@ #include "Trace.h" #include "URI.h" #include "index/Index.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Basic/LangOptions.h" #include "clang/Format/Format.h" #include "clang/Frontend/CompilerInstance.h" @@ -949,6 +950,7 @@ if (Opts.Limit) Req.MaxCandidateCount = Opts.Limit; Req.Query = Filter->pattern(); + Req.RestrictForCodeCompletion = true; Req.Scopes = getQueryScopes(Recorder->CCContext, Recorder->CCSema->getSourceManager()); log(llvm::formatv("Code complete: fuzzyFind(\"{0}\", scopes=[{1}])", @@ -1089,5 +1091,16 @@ return Result; } +bool isIndexedForCodeCompletion(const NamedDecl &ND, ASTContext &ASTCtx) { + using namespace clang::ast_matchers; + auto InTopLevelScope = hasDeclContext( + anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl())); + return !match(decl(anyOf(InTopLevelScope, + hasDeclContext( + enumDecl(InTopLevelScope, unless(isScoped()))))), + ND, ASTCtx) + .empty(); +} + } // namespace clangd } // namespace clang Index: clang-tools-extra/trunk/clangd/index/Index.h =================================================================== --- clang-tools-extra/trunk/clangd/index/Index.h +++ clang-tools-extra/trunk/clangd/index/Index.h @@ -149,9 +149,11 @@ // The number of translation units that reference this symbol from their main // file. This number is only meaningful if aggregated in an index. unsigned References = 0; - + /// Whether or not this symbol is meant to be used for the code completion. + /// See also isIndexedForCodeCompletion(). + bool IsIndexedForCodeCompletion = false; /// 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 + /// candidate list. For example, "Foo(X x, Y y) const" is a label for a /// function. llvm::StringRef CompletionLabel; /// The piece of text that the user is expected to type to match the @@ -267,6 +269,8 @@ /// \brief The number of top candidates to return. The index may choose to /// return more than this, e.g. if it doesn't know which candidates are best. size_t MaxCandidateCount = UINT_MAX; + /// If set to true, only symbols for completion support will be considered. + bool RestrictForCodeCompletion = false; }; struct LookupRequest { Index: clang-tools-extra/trunk/clangd/index/MemIndex.cpp =================================================================== --- clang-tools-extra/trunk/clangd/index/MemIndex.cpp +++ clang-tools-extra/trunk/clangd/index/MemIndex.cpp @@ -45,6 +45,8 @@ // Exact match against all possible scopes. if (!Req.Scopes.empty() && !llvm::is_contained(Req.Scopes, Sym->Scope)) continue; + if (Req.RestrictForCodeCompletion && !Sym->IsIndexedForCodeCompletion) + continue; if (auto Score = Filter.match(Sym->Name)) { Top.emplace(-*Score * quality(*Sym), Sym); Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.h =================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolCollector.h +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.h @@ -18,13 +18,18 @@ namespace clang { namespace clangd { -/// \brief Collect top-level symbols from an AST. These are symbols defined -/// immediately inside a namespace or a translation unit scope. For example, -/// symbols in classes or functions are not collected. Note that this only -/// collects symbols that declared in at least one file that is not a main -/// file (i.e. the source file corresponding to a TU). These are symbols that -/// can be imported by other files by including the file where symbols are -/// declared. +/// \brief Collect declarations (symbols) from an AST. +/// It collects most declarations except: +/// - Implicit declarations +/// - Anonymous declarations (anonymous enum/class/struct, etc) +/// - Declarations in anonymous namespaces +/// - Local declarations (in function bodies, blocks, etc) +/// - Declarations in main files +/// - Template specializations +/// - Library-specific private declarations (e.g. private declaration generated +/// by protobuf compiler) +/// +/// See also shouldFilterDecl(). /// /// Clients (e.g. clangd) can use SymbolCollector together with /// index::indexTopLevelDecls to retrieve all symbols when the source file is Index: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp =================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp @@ -9,6 +9,7 @@ #include "SymbolCollector.h" #include "../AST.h" +#include "../CodeComplete.h" #include "../CodeCompletionStrings.h" #include "../Logger.h" #include "../SourceCode.h" @@ -149,21 +150,20 @@ if (ND->isInAnonymousNamespace()) return true; - // We only want: - // * symbols in namespaces or translation unit scopes (e.g. no class - // members) - // * enum constants in unscoped enum decl (e.g. "red" in "enum {red};") - auto InTopLevelScope = hasDeclContext( - anyOf(namespaceDecl(), translationUnitDecl(), linkageSpecDecl())); - // Don't index template specializations. + // We want most things but not "local" symbols such as symbols inside + // FunctionDecl, BlockDecl, ObjCMethodDecl and OMPDeclareReductionDecl. + // FIXME: Need a matcher for ExportDecl in order to include symbols declared + // within an export. + auto InNonLocalContext = hasDeclContext(anyOf( + translationUnitDecl(), namespaceDecl(), linkageSpecDecl(), recordDecl(), + enumDecl(), objcProtocolDecl(), objcInterfaceDecl(), objcCategoryDecl(), + objcCategoryImplDecl(), objcImplementationDecl())); + // Don't index template specializations and expansions in main files. auto IsSpecialization = anyOf(functionDecl(isExplicitTemplateSpecialization()), cxxRecordDecl(isExplicitTemplateSpecialization()), varDecl(isExplicitTemplateSpecialization())); - if (match(decl(allOf(unless(isExpansionInMainFile()), - anyOf(InTopLevelScope, - hasDeclContext(enumDecl(InTopLevelScope, - unless(isScoped())))), + if (match(decl(allOf(unless(isExpansionInMainFile()), InNonLocalContext, unless(IsSpecialization))), *ND, *ASTCtx) .empty()) @@ -377,6 +377,8 @@ Symbol S; S.ID = std::move(ID); std::tie(S.Scope, S.Name) = splitQualifiedName(QName); + + S.IsIndexedForCodeCompletion = isIndexedForCodeCompletion(ND, Ctx); S.SymInfo = index::getSymbolInfo(&ND); std::string FileURI; if (auto DeclLoc = Index: clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp =================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp +++ clang-tools-extra/trunk/clangd/index/SymbolYAML.cpp @@ -108,6 +108,8 @@ SymbolLocation()); IO.mapOptional("Definition", Sym.Definition, SymbolLocation()); IO.mapOptional("References", Sym.References, 0u); + IO.mapOptional("IsIndexedForCodeCompletion", Sym.IsIndexedForCodeCompletion, + false); IO.mapRequired("CompletionLabel", Sym.CompletionLabel); IO.mapRequired("CompletionFilterText", Sym.CompletionFilterText); IO.mapRequired("CompletionPlainInsertText", Sym.CompletionPlainInsertText); Index: clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/CodeCompleteTests.cpp @@ -32,6 +32,7 @@ using ::testing::Each; using ::testing::ElementsAre; using ::testing::Field; +using ::testing::IsEmpty; using ::testing::Not; using ::testing::UnorderedElementsAre; @@ -153,6 +154,7 @@ Sym.CompletionSnippetInsertText = Sym.Name; Sym.CompletionLabel = Sym.Name; Sym.SymInfo.Kind = Kind; + Sym.IsIndexedForCodeCompletion = true; return Sym; } Symbol func(StringRef Name) { // Assumes the function has no args. @@ -684,6 +686,20 @@ Contains(AllOf(Named("baz"), Doc("Multi-line\nblock comment")))); } +TEST(CompletionTest, GlobalCompletionFiltering) { + + Symbol Class = cls("XYZ"); + Class.IsIndexedForCodeCompletion = false; + Symbol Func = func("XYZ::foooo"); + Func.IsIndexedForCodeCompletion = false; + + auto Results = completions(R"(// void f() { + XYZ::foooo^ + })", + {Class, Func}); + EXPECT_THAT(Results.items, IsEmpty()); +} + TEST(CodeCompleteTest, DisableTypoCorrection) { auto Results = completions(R"cpp( namespace clang { int v; } Index: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp @@ -145,13 +145,14 @@ EXPECT_THAT(match(M, FuzzyFindRequest()), UnorderedElementsAre()); } -TEST(FileIndexTest, IgnoreClassMembers) { +TEST(FileIndexTest, ClassMembers) { FileIndex M; update(M, "f1", "class X { static int m1; int m2; static void f(); };"); FuzzyFindRequest Req; Req.Query = ""; - EXPECT_THAT(match(M, Req), UnorderedElementsAre("X")); + EXPECT_THAT(match(M, Req), + UnorderedElementsAre("X", "X::m1", "X::m2", "X::f")); } TEST(FileIndexTest, NoIncludeCollected) { Index: clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp @@ -120,7 +120,10 @@ EXPECT_THAT(getSymbols("UnnamedStruct"), ElementsAre(AllOf(Named("UnnamedStruct"), WithKind(SymbolKind::Variable)))); - EXPECT_THAT(getSymbols("InUnnamed"), IsEmpty()); + EXPECT_THAT( + getSymbols("InUnnamed"), + ElementsAre(AllOf(Named("InUnnamed"), InContainer("(anonymous struct)"), + WithKind(SymbolKind::Field)))); } TEST_F(WorkspaceSymbolsTest, InMainFile) { @@ -223,6 +226,44 @@ EXPECT_THAT(getSymbols(""), IsEmpty()); } +TEST_F(WorkspaceSymbolsTest, Enums) { + addFile("foo.h", R"cpp( + enum { + Red + }; + enum Color { + Green + }; + enum class Color2 { + Yellow + }; + namespace ns { + enum { + Black + }; + enum Color3 { + Blue + }; + enum class Color4 { + White + }; + } + )cpp"); + addFile("foo.cpp", R"cpp( + #include "foo.h" + )cpp"); + EXPECT_THAT(getSymbols("Red"), ElementsAre(Named("Red"))); + EXPECT_THAT(getSymbols("::Red"), ElementsAre(Named("Red"))); + EXPECT_THAT(getSymbols("Green"), ElementsAre(Named("Green"))); + EXPECT_THAT(getSymbols("Green"), ElementsAre(Named("Green"))); + EXPECT_THAT(getSymbols("Color2::Yellow"), ElementsAre(Named("Yellow"))); + EXPECT_THAT(getSymbols("Yellow"), ElementsAre(Named("Yellow"))); + + EXPECT_THAT(getSymbols("ns::Black"), ElementsAre(Named("Black"))); + EXPECT_THAT(getSymbols("ns::Blue"), ElementsAre(Named("Blue"))); + EXPECT_THAT(getSymbols("ns::Color4::White"), ElementsAre(Named("White"))); +} + TEST_F(WorkspaceSymbolsTest, WithLimit) { addFile("foo.h", R"cpp( int foo; Index: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp @@ -67,6 +67,9 @@ Pos.end.character); } MATCHER_P(Refs, R, "") { return int(arg.References) == R; } +MATCHER_P(ForCodeCompletion, IsIndexedForCodeCompletion, "") { + return arg.IsIndexedForCodeCompletion == IsIndexedForCodeCompletion; +} namespace clang { namespace clangd { @@ -132,9 +135,13 @@ CollectorOpts, PragmaHandler.get()); std::vector Args = { - "symbol_collector", "-fsyntax-only", "-xc++", "-std=c++11", - "-include", TestHeaderName, TestFileName}; + "symbol_collector", "-fsyntax-only", "-xc++", + "-std=c++11", "-include", TestHeaderName}; Args.insert(Args.end(), ExtraArgs.begin(), ExtraArgs.end()); + // This allows to override the "-xc++" with something else, i.e. + // -xobjective-c++. + Args.push_back(TestFileName); + tooling::ToolInvocation Invocation( Args, Factory->create(), Files.get(), @@ -163,8 +170,20 @@ TEST_F(SymbolCollectorTest, CollectSymbols) { const std::string Header = R"( class Foo { + Foo() {} + Foo(int a) {} void f(); + friend void f1(); + friend class Friend; + Foo& operator=(const Foo&); + ~Foo(); + class Nested { + void f(); + }; }; + class Friend { + }; + void f1(); inline void f2() {} static const int KInt = 2; @@ -200,23 +219,78 @@ runSymbolCollector(Header, /*Main=*/""); EXPECT_THAT(Symbols, UnorderedElementsAreArray( - {QName("Foo"), QName("f1"), QName("f2"), QName("KInt"), - QName("kStr"), QName("foo"), QName("foo::bar"), - QName("foo::int32"), QName("foo::int32_t"), QName("foo::v1"), - QName("foo::bar::v2"), QName("foo::baz")})); + {AllOf(QName("Foo"), ForCodeCompletion(true)), + AllOf(QName("Foo::Foo"), ForCodeCompletion(false)), + AllOf(QName("Foo::Foo"), ForCodeCompletion(false)), + AllOf(QName("Foo::f"), ForCodeCompletion(false)), + AllOf(QName("Foo::~Foo"), ForCodeCompletion(false)), + AllOf(QName("Foo::operator="), ForCodeCompletion(false)), + AllOf(QName("Foo::Nested"), ForCodeCompletion(false)), + AllOf(QName("Foo::Nested::f"), ForCodeCompletion(false)), + + AllOf(QName("Friend"), ForCodeCompletion(true)), + AllOf(QName("f1"), ForCodeCompletion(true)), + AllOf(QName("f2"), ForCodeCompletion(true)), + AllOf(QName("KInt"), ForCodeCompletion(true)), + AllOf(QName("kStr"), ForCodeCompletion(true)), + AllOf(QName("foo"), ForCodeCompletion(true)), + AllOf(QName("foo::bar"), ForCodeCompletion(true)), + AllOf(QName("foo::int32"), ForCodeCompletion(true)), + AllOf(QName("foo::int32_t"), ForCodeCompletion(true)), + AllOf(QName("foo::v1"), ForCodeCompletion(true)), + AllOf(QName("foo::bar::v2"), ForCodeCompletion(true)), + AllOf(QName("foo::baz"), ForCodeCompletion(true))})); } TEST_F(SymbolCollectorTest, Template) { Annotations Header(R"( // Template is indexed, specialization and instantiation is not. - template struct [[Tmpl]] {T x = 0;}; + template struct [[Tmpl]] {T $xdecl[[x]] = 0;}; template <> struct Tmpl {}; extern template struct Tmpl; template struct Tmpl; )"); runSymbolCollector(Header.code(), /*Main=*/""); - EXPECT_THAT(Symbols, UnorderedElementsAreArray({AllOf( - QName("Tmpl"), DeclRange(Header.range()))})); + EXPECT_THAT(Symbols, + UnorderedElementsAreArray( + {AllOf(QName("Tmpl"), DeclRange(Header.range())), + AllOf(QName("Tmpl::x"), DeclRange(Header.range("xdecl")))})); +} + +TEST_F(SymbolCollectorTest, ObjCSymbols) { + const std::string Header = R"( + @interface Person + - (void)someMethodName:(void*)name1 lastName:(void*)lName; + @end + + @implementation Person + - (void)someMethodName:(void*)name1 lastName:(void*)lName{ + int foo; + ^(int param){ int bar; }; + } + @end + + @interface Person (MyCategory) + - (void)someMethodName2:(void*)name2; + @end + + @implementation Person (MyCategory) + - (void)someMethodName2:(void*)name2 { + int foo2; + } + @end + + @protocol MyProtocol + - (void)someMethodName3:(void*)name3; + @end + )"; + TestFileName = "test.m"; + runSymbolCollector(Header, /*Main=*/"", {"-fblocks", "-xobjective-c++"}); + EXPECT_THAT(Symbols, + UnorderedElementsAre( + QName("Person"), QName("Person::someMethodName:lastName:"), + QName("MyCategory"), QName("Person::someMethodName2:"), + QName("MyProtocol"), QName("MyProtocol::someMethodName3:"))); } TEST_F(SymbolCollectorTest, Locations) { @@ -334,7 +408,7 @@ Green }; enum class Color2 { - Yellow // ignore + Yellow }; namespace ns { enum { @@ -343,20 +417,26 @@ } )"; runSymbolCollector(Header, /*Main=*/""); - EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Red"), QName("Color"), - QName("Green"), QName("Color2"), - QName("ns"), QName("ns::Black"))); + EXPECT_THAT(Symbols, + UnorderedElementsAre( + AllOf(QName("Red"), ForCodeCompletion(true)), + AllOf(QName("Color"), ForCodeCompletion(true)), + AllOf(QName("Green"), ForCodeCompletion(true)), + AllOf(QName("Color2"), ForCodeCompletion(true)), + AllOf(QName("Color2::Yellow"), ForCodeCompletion(false)), + AllOf(QName("ns"), ForCodeCompletion(true)), + AllOf(QName("ns::Black"), ForCodeCompletion(true)))); } -TEST_F(SymbolCollectorTest, IgnoreNamelessSymbols) { +TEST_F(SymbolCollectorTest, NamelessSymbols) { const std::string Header = R"( struct { int a; } Foo; )"; runSymbolCollector(Header, /*Main=*/""); - EXPECT_THAT(Symbols, - UnorderedElementsAre(QName("Foo"))); + EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), + QName("(anonymous struct)::a"))); } TEST_F(SymbolCollectorTest, SymbolFormedFromMacro) { @@ -417,7 +497,7 @@ UnorderedElementsAre(QName("Foo"), QName("f1"), QName("f2"))); } -TEST_F(SymbolCollectorTest, IgnoreClassMembers) { +TEST_F(SymbolCollectorTest, ClassMembers) { const std::string Header = R"( class Foo { void f() {} @@ -432,7 +512,10 @@ void Foo::ssf() {} )"; runSymbolCollector(Header, Main); - EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"))); + EXPECT_THAT(Symbols, + UnorderedElementsAre(QName("Foo"), QName("Foo::f"), + QName("Foo::g"), QName("Foo::sf"), + QName("Foo::ssf"), QName("Foo::x"))); } TEST_F(SymbolCollectorTest, Scopes) { @@ -531,6 +614,7 @@ End: Line: 1 Column: 1 +IsIndexedForCodeCompletion: true CompletionLabel: 'Foo1-label' CompletionFilterText: 'filter' CompletionPlainInsertText: 'plain' @@ -555,6 +639,7 @@ End: Line: 1 Column: 1 +IsIndexedForCodeCompletion: false CompletionLabel: 'Foo2-label' CompletionFilterText: 'filter' CompletionPlainInsertText: 'plain' @@ -567,11 +652,13 @@ EXPECT_THAT(Symbols1, UnorderedElementsAre(AllOf( QName("clang::Foo1"), Labeled("Foo1-label"), Doc("Foo doc"), - Detail("int"), DeclURI("file:///path/foo.h")))); + Detail("int"), DeclURI("file:///path/foo.h"), + ForCodeCompletion(true)))); auto Symbols2 = SymbolsFromYAML(YAML2); - EXPECT_THAT(Symbols2, UnorderedElementsAre(AllOf( - QName("clang::Foo2"), Labeled("Foo2-label"), - Not(HasDetail()), DeclURI("file:///path/bar.h")))); + EXPECT_THAT(Symbols2, + UnorderedElementsAre(AllOf( + QName("clang::Foo2"), Labeled("Foo2-label"), Not(HasDetail()), + DeclURI("file:///path/bar.h"), ForCodeCompletion(false)))); std::string ConcatenatedYAML; { @@ -741,23 +828,27 @@ // Canonical declarations. class $cdecl[[C]] {}; struct $sdecl[[S]] {}; - union $udecl[[U]] {int x; bool y;}; + union $udecl[[U]] {int $xdecl[[x]]; bool $ydecl[[y]];}; )"); runSymbolCollector(Header.code(), /*Main=*/""); - EXPECT_THAT(Symbols, - UnorderedElementsAre( - AllOf(QName("C"), DeclURI(TestHeaderURI), - DeclRange(Header.range("cdecl")), - IncludeHeader(TestHeaderURI), DefURI(TestHeaderURI), - DefRange(Header.range("cdecl"))), - AllOf(QName("S"), DeclURI(TestHeaderURI), - DeclRange(Header.range("sdecl")), - IncludeHeader(TestHeaderURI), DefURI(TestHeaderURI), - DefRange(Header.range("sdecl"))), - AllOf(QName("U"), DeclURI(TestHeaderURI), - DeclRange(Header.range("udecl")), - IncludeHeader(TestHeaderURI), DefURI(TestHeaderURI), - DefRange(Header.range("udecl"))))); + EXPECT_THAT( + Symbols, + UnorderedElementsAre( + AllOf(QName("C"), DeclURI(TestHeaderURI), + DeclRange(Header.range("cdecl")), IncludeHeader(TestHeaderURI), + DefURI(TestHeaderURI), DefRange(Header.range("cdecl"))), + AllOf(QName("S"), DeclURI(TestHeaderURI), + DeclRange(Header.range("sdecl")), IncludeHeader(TestHeaderURI), + DefURI(TestHeaderURI), DefRange(Header.range("sdecl"))), + AllOf(QName("U"), DeclURI(TestHeaderURI), + DeclRange(Header.range("udecl")), IncludeHeader(TestHeaderURI), + DefURI(TestHeaderURI), DefRange(Header.range("udecl"))), + AllOf(QName("U::x"), DeclURI(TestHeaderURI), + DeclRange(Header.range("xdecl")), DefURI(TestHeaderURI), + DefRange(Header.range("xdecl"))), + AllOf(QName("U::y"), DeclURI(TestHeaderURI), + DeclRange(Header.range("ydecl")), DefURI(TestHeaderURI), + DefRange(Header.range("ydecl"))))); } TEST_F(SymbolCollectorTest, ClassForwardDeclarationIsCanonical) {