Author: ioeric Date: Wed Jan 10 06:57:58 2018 New Revision: 322193 URL: http://llvm.org/viewvc/llvm-project?rev=322193&view=rev Log: [clangd] Add more filters for collected symbols.
Summary: o We only collect symbols in namespace or translation unit scopes. o Add an option to only collect symbols in included headers. Reviewers: hokein, ilya-biryukov Reviewed By: hokein, ilya-biryukov Subscribers: klimek, cfe-commits Differential Revision: https://reviews.llvm.org/D41823 Modified: clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp clang-tools-extra/trunk/clangd/index/FileIndex.cpp clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp clang-tools-extra/trunk/clangd/index/SymbolCollector.h clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Modified: clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp?rev=322193&r1=322192&r2=322193&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp (original) +++ clang-tools-extra/trunk/clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp Wed Jan 10 06:57:58 2018 @@ -72,8 +72,9 @@ public: IndexOpts.SystemSymbolFilter = index::IndexingOptions::SystemSymbolFilterKind::All; IndexOpts.IndexFunctionLocals = false; - return new WrappedIndexAction(std::make_shared<SymbolCollector>(), - IndexOpts, Ctx); + return new WrappedIndexAction( + std::make_shared<SymbolCollector>(SymbolCollector::Options()), + IndexOpts, Ctx); } tooling::ExecutionContext *Ctx; Modified: clang-tools-extra/trunk/clangd/index/FileIndex.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/FileIndex.cpp?rev=322193&r1=322192&r2=322193&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/FileIndex.cpp (original) +++ clang-tools-extra/trunk/clangd/index/FileIndex.cpp Wed Jan 10 06:57:58 2018 @@ -19,7 +19,9 @@ namespace { std::unique_ptr<SymbolSlab> indexAST(ASTContext &Ctx, std::shared_ptr<Preprocessor> PP, llvm::ArrayRef<const Decl *> Decls) { - auto Collector = std::make_shared<SymbolCollector>(); + SymbolCollector::Options CollectorOpts; + CollectorOpts.IndexMainFiles = true; + auto Collector = std::make_shared<SymbolCollector>(std::move(CollectorOpts)); Collector->setPreprocessor(std::move(PP)); index::IndexingOptions IndexOpts; IndexOpts.SystemSymbolFilter = Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp?rev=322193&r1=322192&r2=322193&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original) +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Wed Jan 10 06:57:58 2018 @@ -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 @@ splitQualifiedName(llvm::StringRef QName 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::<anonymous>::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<GlobalCodeCompletionAllocator>(); @@ -79,6 +111,8 @@ bool SymbolCollector::handleDeclOccurenc const Decl *D, index::SymbolRoleSet Roles, ArrayRef<index::SymbolRelation> 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<unsigned>(index::SymbolRole::Declaration) || Roles & static_cast<unsigned>(index::SymbolRole::Definition))) @@ -87,17 +121,8 @@ bool SymbolCollector::handleDeclOccurenc assert(CompletionAllocator && CompletionTUInfo); if (const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(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::<anonymous>::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; Modified: clang-tools-extra/trunk/clangd/index/SymbolCollector.h URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/SymbolCollector.h?rev=322193&r1=322192&r2=322193&view=diff ============================================================================== --- clang-tools-extra/trunk/clangd/index/SymbolCollector.h (original) +++ clang-tools-extra/trunk/clangd/index/SymbolCollector.h Wed Jan 10 06:57:58 2018 @@ -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<Preprocessor> PP) override { @@ -45,6 +55,7 @@ private: std::shared_ptr<Preprocessor> PP; std::shared_ptr<GlobalCodeCompletionAllocator> CompletionAllocator; std::unique_ptr<CodeCompletionTUInfo> CompletionTUInfo; + Options Opts; }; } // namespace clangd Modified: clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp?rev=322193&r1=322192&r2=322193&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/FileIndexTests.cpp Wed Jan 10 06:57:58 2018 @@ -166,15 +166,16 @@ TEST(FileIndexTest, RemoveNonExisting) { 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 Modified: clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp?rev=322193&r1=322192&r2=322193&view=diff ============================================================================== --- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original) +++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Wed Jan 10 06:57:58 2018 @@ -53,20 +53,22 @@ namespace clangd { 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<SymbolCollector>(); + Collector = std::make_shared<SymbolCollector>(COpts); FrontendAction *Action = index::createIndexingAction(Collector, IndexOpts, nullptr).release(); return Action; } std::shared_ptr<SymbolCollector> Collector; + SymbolCollector::Options COpts; }; class SymbolCollectorTest : public ::testing::Test { @@ -79,7 +81,7 @@ public: const std::string FileName = "symbol.cc"; const std::string HeaderName = "symbols.h"; - auto Factory = llvm::make_unique<SymbolIndexActionFactory>(); + auto Factory = llvm::make_unique<SymbolIndexActionFactory>(CollectorOpts); tooling::ToolInvocation Invocation( {"symbol_collector", "-fsyntax-only", "-std=c++11", FileName}, @@ -100,9 +102,11 @@ public: 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 @@ TEST_F(SymbolCollectorTest, CollectSymbo EXPECT_THAT(Symbols, UnorderedElementsAreArray( {QName("Foo"), - QName("Foo::f"), QName("f1"), QName("f2"), QName("KInt"), @@ -158,14 +161,70 @@ TEST_F(SymbolCollectorTest, CollectSymbo 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, SymbolWithDo } 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( _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits