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 @@ -29,7 +29,7 @@ /// - Library-specific private declarations (e.g. private declaration generated /// by protobuf compiler) /// -/// See also shouldFilterDecl(). +/// See also shouldCollectSymbol(...). /// /// Clients (e.g. clangd) can use SymbolCollector together with /// index::indexTopLevelDecls to retrieve all symbols when the source file is @@ -56,6 +56,11 @@ SymbolCollector(Options Opts); + /// Returns true is \p ND should be collected. + /// AST matchers require non-const ASTContext. + static bool shouldCollectSymbol(const NamedDecl &ND, ASTContext &ASTCtx, + const Options &Opts); + void initialize(ASTContext &Ctx) override; void setPreprocessor(std::shared_ptr PP) override { 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 @@ -130,51 +130,6 @@ std::any_of(Name.begin(), Name.end(), islower); } -bool shouldFilterDecl(const NamedDecl *ND, ASTContext *ASTCtx, - const SymbolCollector::Options &Opts) { - using namespace clang::ast_matchers; - if (ND->isImplicit()) - return true; - // Skip anonymous declarations, e.g (anonymous enum/class/struct). - if (ND->getDeclName().isEmpty()) - return true; - - // FIXME: figure out a way to handle internal linkage symbols (e.g. static - // variables, function) defined in the .cc files. Also we skip the symbols - // in anonymous namespace as the qualifier names of these symbols are like - // `foo::::bar`, which need a special handling. - // In real world projects, we have a relatively large set of header files - // that define static variables (like "static const int A = 1;"), we still - // want to collect these symbols, although they cause potential ODR - // violations. - if (ND->isInAnonymousNamespace()) - return true; - - // 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()), InNonLocalContext, - unless(IsSpecialization))), - *ND, *ASTCtx) - .empty()) - return true; - - // Avoid indexing internal symbols in protobuf generated headers. - if (isPrivateProtoDecl(*ND)) - return true; - return false; -} - // We only collect #include paths for symbols that are suitable for global code // completion, except for namespaces since #include path for a namespace is hard // to define. @@ -283,6 +238,52 @@ llvm::make_unique(CompletionAllocator); } +bool SymbolCollector::shouldCollectSymbol(const NamedDecl &ND, + ASTContext &ASTCtx, + const Options &Opts) { + using namespace clang::ast_matchers; + if (ND.isImplicit()) + return false; + // Skip anonymous declarations, e.g (anonymous enum/class/struct). + if (ND.getDeclName().isEmpty()) + return false; + + // FIXME: figure out a way to handle internal linkage symbols (e.g. static + // variables, function) defined in the .cc files. Also we skip the symbols + // in anonymous namespace as the qualifier names of these symbols are like + // `foo::::bar`, which need a special handling. + // In real world projects, we have a relatively large set of header files + // that define static variables (like "static const int A = 1;"), we still + // want to collect these symbols, although they cause potential ODR + // violations. + if (ND.isInAnonymousNamespace()) + return false; + + // 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()), InNonLocalContext, + unless(IsSpecialization))), + ND, ASTCtx) + .empty()) + return false; + + // Avoid indexing internal symbols in protobuf generated headers. + if (isPrivateProtoDecl(ND)) + return false; + return true; +} + // Always return true to continue indexing. bool SymbolCollector::handleDeclOccurence( const Decl *D, index::SymbolRoleSet Roles, @@ -319,7 +320,7 @@ if (!(Roles & static_cast(index::SymbolRole::Declaration) || Roles & static_cast(index::SymbolRole::Definition))) return true; - if (shouldFilterDecl(ND, ASTCtx, Opts)) + if (!shouldCollectSymbol(*ND, *ASTCtx, Opts)) return true; llvm::SmallString<128> USR; 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 @@ -9,6 +9,7 @@ #include "Annotations.h" #include "TestFS.h" +#include "TestTU.h" #include "index/SymbolCollector.h" #include "index/SymbolYAML.h" #include "clang/Basic/FileManager.h" @@ -75,6 +76,86 @@ namespace clangd { namespace { + +class ShouldCollectSymbolTest : public ::testing::Test { +public: + void build(StringRef HeaderCode, StringRef Code = "") { + File.HeaderFilename = HeaderName; + File.Filename = FileName; + File.HeaderCode = HeaderCode; + File.Code = Code; + AST = File.build(); + } + + // build() must have been called. + bool shouldCollect(StringRef Name, bool Qualified = true) { + assert(AST.hasValue()); + return SymbolCollector::shouldCollectSymbol( + Qualified ? findDecl(*AST, Name) : findAnyDecl(*AST, Name), + AST->getASTContext(), SymbolCollector::Options()); + } + +protected: + std::string HeaderName = "f.h"; + std::string FileName = "f.cpp"; + TestTU File; + Optional AST; // Initialized after build. +}; + +TEST_F(ShouldCollectSymbolTest, ShouldCollectSymbol) { + build(R"( + namespace nx { + class X{} + void f() { int Local; } + struct { int x } var; + namespace { class InAnonymous {}; } + } + )", + "class InMain {};"); + auto AST = File.build(); + EXPECT_TRUE(shouldCollect("nx")); + EXPECT_TRUE(shouldCollect("nx::X")); + EXPECT_TRUE(shouldCollect("nx::f")); + + EXPECT_FALSE(shouldCollect("InMain")); + EXPECT_FALSE(shouldCollect("Local", /*Qualified=*/false)); + EXPECT_FALSE(shouldCollect("InAnonymous", /*Qualified=*/false)); +} + +TEST_F(ShouldCollectSymbolTest, NoPrivateProtoSymbol) { + HeaderName = "f.proto.h"; + build( + R"(// Generated by the protocol buffer compiler. DO NOT EDIT! + namespace nx { + class Top_Level {}; + class TopLevel {}; + enum Kind { + KIND_OK, + Kind_Not_Ok, + }; + })"); + EXPECT_TRUE(shouldCollect("nx::TopLevel")); + EXPECT_TRUE(shouldCollect("nx::Kind::KIND_OK")); + EXPECT_TRUE(shouldCollect("nx::Kind")); + + EXPECT_FALSE(shouldCollect("nx::Top_Level")); + EXPECT_FALSE(shouldCollect("nx::Kind::Kind_Not_Ok")); +} + +TEST_F(ShouldCollectSymbolTest, DoubleCheckProtoHeaderComment) { + HeaderName = "f.proto.h"; + build(R"( + namespace nx { + class Top_Level {}; + enum Kind { + Kind_Fine + }; + } + )"); + EXPECT_TRUE(shouldCollect("nx::Top_Level")); + EXPECT_TRUE(shouldCollect("nx::Kind_Fine")); +} + class SymbolIndexActionFactory : public tooling::FrontendActionFactory { public: SymbolIndexActionFactory(SymbolCollector::Options COpts, @@ -865,42 +946,6 @@ AllOf(QName("pörk"), DeclRange(Header.range())))); } -TEST_F(SymbolCollectorTest, FilterPrivateProtoSymbols) { - TestHeaderName = testPath("x.proto.h"); - const std::string Header = - R"(// Generated by the protocol buffer compiler. DO NOT EDIT! - namespace nx { - class Top_Level {}; - class TopLevel {}; - enum Kind { - KIND_OK, - Kind_Not_Ok, - }; - bool operator<(const TopLevel &, const TopLevel &); - })"; - runSymbolCollector(Header, /*Main=*/""); - EXPECT_THAT(Symbols, - UnorderedElementsAre(QName("nx"), QName("nx::TopLevel"), - QName("nx::Kind"), QName("nx::KIND_OK"), - QName("nx::operator<"))); -} - -TEST_F(SymbolCollectorTest, DoubleCheckProtoHeaderComment) { - TestHeaderName = testPath("x.proto.h"); - const std::string Header = R"( - namespace nx { - class Top_Level {}; - enum Kind { - Kind_Fine - }; - } - )"; - runSymbolCollector(Header, /*Main=*/""); - EXPECT_THAT(Symbols, - UnorderedElementsAre(QName("nx"), QName("nx::Top_Level"), - QName("nx::Kind"), QName("nx::Kind_Fine"))); -} - TEST_F(SymbolCollectorTest, DoNotIndexSymbolsInFriendDecl) { Annotations Header(R"( namespace nx {