[clang-tools-extra] r322193 - [clangd] Add more filters for collected symbols.
Eric Liu via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 10 06:57:58 PST 2018
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(
More information about the cfe-commits
mailing list