Index: clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp =================================================================== --- clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp +++ clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp @@ -72,8 +72,9 @@ IndexOpts.SystemSymbolFilter = index::IndexingOptions::SystemSymbolFilterKind::All; IndexOpts.IndexFunctionLocals = false; - return new WrappedIndexAction(std::make_shared(), - IndexOpts, Ctx); + return new WrappedIndexAction( + std::make_shared(SymbolCollector::Options()), + IndexOpts, Ctx); } tooling::ExecutionContext *Ctx; Index: clang-tools-extra/trunk/clangd/index/FileIndex.cpp =================================================================== --- clang-tools-extra/trunk/clangd/index/FileIndex.cpp +++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp @@ -19,7 +19,9 @@ std::unique_ptr indexAST(ASTContext &Ctx, std::shared_ptr PP, llvm::ArrayRef Decls) { - auto Collector = std::make_shared(); + SymbolCollector::Options CollectorOpts; + CollectorOpts.IndexMainFiles = true; + auto Collector = std::make_shared(std::move(CollectorOpts)); Collector->setPreprocessor(std::move(PP)); index::IndexingOptions IndexOpts; IndexOpts.SystemSymbolFilter = 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 @@ -17,13 +17,23 @@ namespace clang { namespace clangd { -// Collect all symbols from an AST. -// -// Clients (e.g. clangd) can use SymbolCollector together with -// index::indexTopLevelDecls to retrieve all symbols when the source file is -// changed. +/// \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. +/// +/// Clients (e.g. clangd) can use SymbolCollector together with +/// index::indexTopLevelDecls to retrieve all symbols when the source file is +/// changed. class SymbolCollector : public index::IndexDataConsumer { public: + struct Options { + /// Whether to collect symbols in main files (e.g. the source file + /// corresponding to a TU). + bool IndexMainFiles = false; + }; + + SymbolCollector(Options Opts); + void initialize(ASTContext &Ctx) override; void setPreprocessor(std::shared_ptr PP) override { @@ -45,6 +55,7 @@ std::shared_ptr PP; std::shared_ptr CompletionAllocator; std::unique_ptr CompletionTUInfo; + Options Opts; }; } // namespace clangd 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 @@ -10,6 +10,7 @@ #include "SymbolCollector.h" #include "../CodeCompletionStrings.h" #include "clang/AST/DeclCXX.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" #include "clang/Basic/SourceManager.h" #include "clang/Index/IndexSymbol.h" #include "clang/Index/USRGeneration.h" @@ -65,8 +66,39 @@ return {QName.substr(0, Pos), QName.substr(Pos + 2)}; } +bool shouldFilterDecl(const NamedDecl *ND, ASTContext *ASTCtx, + const SymbolCollector::Options &Opts) { + using namespace clang::ast_matchers; + if (ND->isImplicit()) + 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 only want symbols in namespaces or translation unit scopes (e.g. no + // class members). + if (match(decl(allOf( + Opts.IndexMainFiles ? decl() + : decl(unless(isExpansionInMainFile())), + hasDeclContext(anyOf(namespaceDecl(), translationUnitDecl())))), + *ND, *ASTCtx) + .empty()) + return true; + + return false; +} + } // namespace +SymbolCollector::SymbolCollector(Options Opts) : Opts(std::move(Opts)) {} + void SymbolCollector::initialize(ASTContext &Ctx) { ASTCtx = &Ctx; CompletionAllocator = std::make_shared(); @@ -79,6 +111,8 @@ const Decl *D, index::SymbolRoleSet Roles, ArrayRef Relations, FileID FID, unsigned Offset, index::IndexDataConsumer::ASTNodeInfo ASTNode) { + assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set."); + // FIXME: collect all symbol references. if (!(Roles & static_cast(index::SymbolRole::Declaration) || Roles & static_cast(index::SymbolRole::Definition))) @@ -87,17 +121,8 @@ assert(CompletionAllocator && CompletionTUInfo); if (const NamedDecl *ND = llvm::dyn_cast(D)) { - // 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()) + if (shouldFilterDecl(ND, ASTCtx, Opts)) return true; - llvm::SmallString<128> USR; if (index::generateUSRForDecl(ND, USR)) return true; 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 @@ -166,15 +166,16 @@ EXPECT_THAT(match(M, FuzzyFindRequest()), UnorderedElementsAre()); } -TEST(FileIndexTest, ClassMembers) { +TEST(FileIndexTest, IgnoreClassMembers) { FileIndex M; auto Ctx = Context::empty(); M.update(Ctx, "f1", - build("f1", "class X { static int m1; int m2;};").getPointer()); + build("f1", "class X { static int m1; int m2; static void f(); };") + .getPointer()); FuzzyFindRequest Req; Req.Query = ""; - EXPECT_THAT(match(M, Req), UnorderedElementsAre("X", "X::m1", "X::m2")); + EXPECT_THAT(match(M, Req), UnorderedElementsAre("X")); } } // namespace 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 @@ -53,20 +53,22 @@ namespace { class SymbolIndexActionFactory : public tooling::FrontendActionFactory { public: - SymbolIndexActionFactory() = default; + SymbolIndexActionFactory(SymbolCollector::Options COpts) + : COpts(std::move(COpts)) {} clang::FrontendAction *create() override { index::IndexingOptions IndexOpts; IndexOpts.SystemSymbolFilter = index::IndexingOptions::SystemSymbolFilterKind::All; IndexOpts.IndexFunctionLocals = false; - Collector = std::make_shared(); + Collector = std::make_shared(COpts); FrontendAction *Action = index::createIndexingAction(Collector, IndexOpts, nullptr).release(); return Action; } std::shared_ptr Collector; + SymbolCollector::Options COpts; }; class SymbolCollectorTest : public ::testing::Test { @@ -79,7 +81,7 @@ const std::string FileName = "symbol.cc"; const std::string HeaderName = "symbols.h"; - auto Factory = llvm::make_unique(); + auto Factory = llvm::make_unique(CollectorOpts); tooling::ToolInvocation Invocation( {"symbol_collector", "-fsyntax-only", "-std=c++11", FileName}, @@ -100,9 +102,11 @@ protected: SymbolSlab Symbols; + SymbolCollector::Options CollectorOpts; }; -TEST_F(SymbolCollectorTest, CollectSymbol) { +TEST_F(SymbolCollectorTest, CollectSymbols) { + CollectorOpts.IndexMainFiles = true; const std::string Header = R"( class Foo { void f(); @@ -144,7 +148,6 @@ EXPECT_THAT(Symbols, UnorderedElementsAreArray( {QName("Foo"), - QName("Foo::f"), QName("f1"), QName("f2"), QName("KInt"), @@ -158,14 +161,70 @@ QName("foo::baz")})); } -TEST_F(SymbolCollectorTest, SymbolWithDocumentation) { +TEST_F(SymbolCollectorTest, IgnoreSymbolsInMainFile) { + CollectorOpts.IndexMainFiles = false; + const std::string Header = R"( + class Foo {}; + void f1(); + inline void f2() {} + )"; + const std::string Main = R"( + namespace { + void ff() {} // ignore + } + void main_f() {} // ignore + void f1() {} + )"; + runSymbolCollector(Header, Main); + EXPECT_THAT(Symbols, + UnorderedElementsAre(QName("Foo"), QName("f1"), QName("f2"))); +} + +TEST_F(SymbolCollectorTest, IncludeSymbolsInMainFile) { + CollectorOpts.IndexMainFiles = true; + const std::string Header = R"( + class Foo {}; + void f1(); + inline void f2() {} + )"; + const std::string Main = R"( + namespace { + void ff() {} // ignore + } + void main_f() {} + void f1() {} + )"; + runSymbolCollector(Header, Main); + EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("f1"), + QName("f2"), QName("main_f"))); +} + +TEST_F(SymbolCollectorTest, IgnoreClassMembers) { + const std::string Header = R"( + class Foo { + void f() {} + void g(); + static void sf() {} + static void ssf(); + static int x; + }; + )"; const std::string Main = R"( + void Foo::g() {} + void Foo::ssf() {} + )"; + runSymbolCollector(Header, Main); + EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"))); +} + +TEST_F(SymbolCollectorTest, SymbolWithDocumentation) { + const std::string Header = R"( namespace nx { /// Foo comment. int ff(int x, double y) { return 0; } } )"; - runSymbolCollector(/*Header=*/"", Main); + runSymbolCollector(Header, /*Main=*/""); EXPECT_THAT(Symbols, UnorderedElementsAre(QName("nx"), AllOf(QName("nx::ff"), @@ -174,13 +233,13 @@ } TEST_F(SymbolCollectorTest, PlainAndSnippet) { - const std::string Main = R"( + const std::string Header = R"( namespace nx { void f() {} int ff(int x, double y) { return 0; } } )"; - runSymbolCollector(/*Header=*/"", Main); + runSymbolCollector(Header, /*Main=*/""); EXPECT_THAT( Symbols, UnorderedElementsAre(