ioeric created this revision.
ioeric added reviewers: hokein, ilya-biryukov.
Herald added subscribers: cfe-commits, klimek.
o We only collect symbols in namespace or translation unit scopes.
o Add an option to only collect symbols in included headers.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D41823
Files:
clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
clangd/index/FileIndex.cpp
clangd/index/SymbolCollector.cpp
clangd/index/SymbolCollector.h
unittests/clangd/FileIndexTests.cpp
unittests/clangd/SymbolCollectorTests.cpp
Index: unittests/clangd/SymbolCollectorTests.cpp
===================================================================
--- unittests/clangd/SymbolCollectorTests.cpp
+++ unittests/clangd/SymbolCollectorTests.cpp
@@ -50,20 +50,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<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 {
@@ -76,7 +78,7 @@
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},
@@ -97,50 +99,87 @@
protected:
SymbolSlab Symbols;
+ SymbolCollector::Options CollectorOpts;
};
-TEST_F(SymbolCollectorTest, CollectSymbol) {
+TEST_F(SymbolCollectorTest, IgnoreSymbolsInMainFile) {
const std::string Header = R"(
- class Foo {
- void f();
- };
+ 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("Foo::f"),
- QName("f1"), QName("f2")));
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(QName("Foo"), QName("f1"), QName("f2")));
}
-TEST_F(SymbolCollectorTest, SymbolWithDocumentation) {
+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"),
Labeled("ff(int x, double y)"),
Detail("int"), Doc("Foo comment."))));
}
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(
Index: unittests/clangd/FileIndexTests.cpp
===================================================================
--- unittests/clangd/FileIndexTests.cpp
+++ unittests/clangd/FileIndexTests.cpp
@@ -166,15 +166,15 @@
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 { 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: clangd/index/SymbolCollector.h
===================================================================
--- clangd/index/SymbolCollector.h
+++ clangd/index/SymbolCollector.h
@@ -24,7 +24,11 @@
// changed.
class SymbolCollector : public index::IndexDataConsumer {
public:
- SymbolCollector();
+ struct Options {
+ bool IndexMainFiles = false;
+ };
+
+ SymbolCollector(Options Opts);
void initialize(ASTContext &Ctx) override;
@@ -47,6 +51,7 @@
std::shared_ptr<Preprocessor> PP;
std::shared_ptr<GlobalCodeCompletionAllocator> CompletionAllocator;
CodeCompletionTUInfo CompletionTUInfo;
+ Options Opts;
};
} // namespace clangd
Index: clangd/index/SymbolCollector.cpp
===================================================================
--- clangd/index/SymbolCollector.cpp
+++ 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,11 +66,34 @@
return {QName.substr(0, Pos), QName.substr(Pos + 2)};
}
+bool shouldFilterDecl(const Decl *D, ASTContext *ASTCtx,
+ const SymbolCollector::Options &Opts) {
+ using namespace clang::ast_matchers;
+ if (D->isImplicit())
+ 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())))),
+ *D, *ASTCtx)
+ .empty())
+ return true;
+ if (const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(D))
+ // FIXME: Should we include the internal linkage symbols?
+ if (!ND->hasExternalFormalLinkage() || ND->isInAnonymousNamespace())
+ return true;
+
+ return false;
+}
+
} // namespace
-SymbolCollector::SymbolCollector()
+SymbolCollector::SymbolCollector(Options Opts)
: CompletionAllocator(std::make_shared<GlobalCodeCompletionAllocator>()),
- CompletionTUInfo(CompletionAllocator) {}
+ CompletionTUInfo(CompletionAllocator), Opts(std::move(Opts)) {}
void SymbolCollector::initialize(ASTContext &Ctx) {
ASTCtx = &Ctx;
@@ -82,16 +106,17 @@
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)))
return true;
- if (const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(D)) {
- // FIXME: Should we include the internal linkage symbols?
- if (!ND->hasExternalFormalLinkage() || ND->isInAnonymousNamespace())
- return true;
+ if (shouldFilterDecl(D, ASTCtx, Opts))
+ return true;
+ if (const NamedDecl *ND = llvm::dyn_cast<NamedDecl>(D)) {
llvm::SmallString<128> USR;
if (index::generateUSRForDecl(ND, USR))
return true;
@@ -116,7 +141,6 @@
S.CanonicalDeclaration = Location;
// Add completion info.
- assert(ASTCtx && PP.get() && "ASTContext and Preprocessor must be set.");
CodeCompletionResult SymbolCompletion(ND, 0);
const auto *CCS = SymbolCompletion.CreateCodeCompletionString(
*ASTCtx, *PP, CodeCompletionContext::CCC_Name, *CompletionAllocator,
Index: clangd/index/FileIndex.cpp
===================================================================
--- clangd/index/FileIndex.cpp
+++ clangd/index/FileIndex.cpp
@@ -19,7 +19,9 @@
std::unique_ptr<SymbolSlab> indexAST(ASTContext &Ctx,
std::shared_ptr<Preprocessor> PP,
llvm::ArrayRef<const Decl *> Decls) {
- auto Collector = std::make_shared<SymbolCollector>();
+ auto CollectorOpts = SymbolCollector::Options();
+ CollectorOpts.IndexMainFiles = true;
+ auto Collector = std::make_shared<SymbolCollector>(std::move(CollectorOpts));
Collector->setPreprocessor(std::move(PP));
index::IndexingOptions IndexOpts;
IndexOpts.SystemSymbolFilter =
Index: clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
===================================================================
--- clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
+++ clangd/global-symbol-builder/GlobalSymbolBuilderMain.cpp
@@ -44,7 +44,7 @@
IndexOpts.SystemSymbolFilter =
index::IndexingOptions::SystemSymbolFilterKind::All;
IndexOpts.IndexFunctionLocals = false;
- Collector = std::make_shared<SymbolCollector>();
+ Collector = std::make_shared<SymbolCollector>(SymbolCollector::Options());
return index::createIndexingAction(Collector, IndexOpts, nullptr).release();
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
