[clang-tools-extra] r351041 - [clangd] Index main-file symbols (bug 39761)
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Mon Jan 14 02:01:18 PST 2019
Author: sammccall
Date: Mon Jan 14 02:01:17 2019
New Revision: 351041
URL: http://llvm.org/viewvc/llvm-project?rev=351041&view=rev
Log:
[clangd] Index main-file symbols (bug 39761)
Patch by Nathan Ridge!
Differential Revision: https://reviews.llvm.org/D55185
Modified:
clang-tools-extra/trunk/clangd/index/Index.h
clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp
clang-tools-extra/trunk/clangd/index/SymbolCollector.h
clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp
clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp
Modified: clang-tools-extra/trunk/clangd/index/Index.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/index/Index.h?rev=351041&r1=351040&r2=351041&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/Index.h (original)
+++ clang-tools-extra/trunk/clangd/index/Index.h Mon Jan 14 02:01:17 2019
@@ -241,6 +241,8 @@ struct Symbol {
Deprecated = 1 << 1,
// Symbol is an implementation detail.
ImplementationDetail = 1 << 2,
+ // Symbol is visible to other files (not e.g. a static helper function).
+ VisibleOutsideFile = 1 << 3,
};
SymbolFlag Flags = SymbolFlag::None;
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=351041&r1=351040&r2=351041&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.cpp Mon Jan 14 02:01:17 2019
@@ -240,22 +240,20 @@ void SymbolCollector::initialize(ASTCont
bool SymbolCollector::shouldCollectSymbol(const NamedDecl &ND,
const ASTContext &ASTCtx,
- const Options &Opts) {
+ const Options &Opts,
+ bool IsMainFileOnly) {
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::<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())
+ // Skip main-file symbols if we are not collecting them.
+ if (IsMainFileOnly && !Opts.CollectMainFileSymbols)
+ return false;
+
+ // Skip symbols in anonymous namespaces in header files.
+ if (!IsMainFileOnly && ND.isInAnonymousNamespace())
return false;
// We want most things but not "local" symbols such as symbols inside
@@ -285,10 +283,6 @@ bool SymbolCollector::shouldCollectSymbo
explicitTemplateSpecialization<VarDecl>(ND))
return false;
- const auto &SM = ASTCtx.getSourceManager();
- // Skip decls in the main file.
- if (SM.isInMainFile(SM.getExpansionLoc(ND.getBeginLoc())))
- return false;
// Avoid indexing internal symbols in protobuf generated headers.
if (isPrivateProtoDecl(ND))
return false;
@@ -335,9 +329,15 @@ bool SymbolCollector::handleDeclOccurenc
if (IsOnlyRef && !CollectRef)
return true;
- if (!shouldCollectSymbol(*ND, *ASTCtx, Opts))
+
+ // ND is the canonical (i.e. first) declaration. If it's in the main file,
+ // then no public declaration was visible, so assume it's main-file only.
+ bool IsMainFileOnly = SM.isWrittenInMainFile(SM.getExpansionLoc(
+ ND->getBeginLoc()));
+ if (!shouldCollectSymbol(*ND, *ASTCtx, Opts, IsMainFileOnly))
return true;
- if (CollectRef && !isa<NamespaceDecl>(ND) &&
+ // Do not store references to main-file symbols.
+ if (CollectRef && !IsMainFileOnly && !isa<NamespaceDecl>(ND) &&
(Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
DeclRefs[ND].emplace_back(SpellingLoc, Roles);
// Don't continue indexing if this is a mere reference.
@@ -351,13 +351,13 @@ bool SymbolCollector::handleDeclOccurenc
const NamedDecl &OriginalDecl = *cast<NamedDecl>(ASTNode.OrigD);
const Symbol *BasicSymbol = Symbols.find(*ID);
if (!BasicSymbol) // Regardless of role, ND is the canonical declaration.
- BasicSymbol = addDeclaration(*ND, std::move(*ID));
+ BasicSymbol = addDeclaration(*ND, std::move(*ID), IsMainFileOnly);
else if (isPreferredDeclaration(OriginalDecl, Roles))
// If OriginalDecl is preferred, replace the existing canonical
// declaration (e.g. a class forward declaration). There should be at most
// one duplicate as we expect to see only one preferred declaration per
// TU, because in practice they are definitions.
- BasicSymbol = addDeclaration(OriginalDecl, std::move(*ID));
+ BasicSymbol = addDeclaration(OriginalDecl, std::move(*ID), IsMainFileOnly);
if (Roles & static_cast<unsigned>(index::SymbolRole::Definition))
addDefinition(OriginalDecl, *BasicSymbol);
@@ -506,7 +506,8 @@ void SymbolCollector::finish() {
}
const Symbol *SymbolCollector::addDeclaration(const NamedDecl &ND,
- SymbolID ID) {
+ SymbolID ID,
+ bool IsMainFileOnly) {
auto &Ctx = ND.getASTContext();
auto &SM = Ctx.getSourceManager();
@@ -517,10 +518,13 @@ const Symbol *SymbolCollector::addDeclar
// FIXME: this returns foo:bar: for objective-C methods, we prefer only foo:
// for consistency with CodeCompletionString and a clean name/signature split.
- if (isIndexedForCodeCompletion(ND, Ctx))
+ // We collect main-file symbols, but do not use them for code completion.
+ if (!IsMainFileOnly && isIndexedForCodeCompletion(ND, Ctx))
S.Flags |= Symbol::IndexedForCodeCompletion;
if (isImplementationDetail(&ND))
S.Flags |= Symbol::ImplementationDetail;
+ if (!IsMainFileOnly)
+ S.Flags |= Symbol::VisibleOutsideFile;
S.SymInfo = index::getSymbolInfo(&ND);
std::string FileURI;
auto Loc = findNameLoc(&ND);
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=351041&r1=351040&r2=351041&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/index/SymbolCollector.h (original)
+++ clang-tools-extra/trunk/clangd/index/SymbolCollector.h Mon Jan 14 02:01:17 2019
@@ -28,13 +28,14 @@ namespace clangd {
/// It collects most declarations except:
/// - Implicit declarations
/// - Anonymous declarations (anonymous enum/class/struct, etc)
-/// - Declarations in anonymous namespaces
+/// - Declarations in anonymous namespaces in headers
/// - Local declarations (in function bodies, blocks, etc)
-/// - Declarations in main files
/// - Template specializations
/// - Library-specific private declarations (e.g. private declaration generated
/// by protobuf compiler)
///
+/// References to main-file symbols are not collected.
+///
/// See also shouldCollectSymbol(...).
///
/// Clients (e.g. clangd) can use SymbolCollector together with
@@ -72,6 +73,9 @@ public:
/// collect macros. For example, `indexTopLevelDecls` will not index any
/// macro even if this is true.
bool CollectMacro = false;
+ /// Collect symbols local to main-files, such as static functions
+ /// and symbols inside an anonymous namespace.
+ bool CollectMainFileSymbols = true;
/// If this is set, only collect symbols/references from a file if
/// `FileFilter(SM, FID)` is true. If not set, all files are indexed.
std::function<bool(const SourceManager &, FileID)> FileFilter = nullptr;
@@ -81,7 +85,7 @@ public:
/// Returns true is \p ND should be collected.
static bool shouldCollectSymbol(const NamedDecl &ND, const ASTContext &ASTCtx,
- const Options &Opts);
+ const Options &Opts, bool IsMainFileSymbol);
void initialize(ASTContext &Ctx) override;
@@ -105,7 +109,7 @@ public:
void finish() override;
private:
- const Symbol *addDeclaration(const NamedDecl &, SymbolID);
+ const Symbol *addDeclaration(const NamedDecl &, SymbolID, bool IsMainFileSymbol);
void addDefinition(const NamedDecl &, const Symbol &DeclSymbol);
// All Symbols collected from the AST.
Modified: clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp?rev=351041&r1=351040&r2=351041&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp Mon Jan 14 02:01:17 2019
@@ -125,7 +125,7 @@ TEST_F(BackgroundIndexTest, IndexTwoFile
ASSERT_TRUE(Idx.blockUntilIdleForTest());
EXPECT_THAT(
runFuzzyFind(Idx, ""),
- UnorderedElementsAre(Named("common"), Named("A_CC"),
+ UnorderedElementsAre(Named("common"), Named("A_CC"), Named("g"),
AllOf(Named("f_b"), Declared(), Not(Defined()))));
Cmd.Filename = testPath("root/B.cc");
@@ -135,7 +135,7 @@ TEST_F(BackgroundIndexTest, IndexTwoFile
ASSERT_TRUE(Idx.blockUntilIdleForTest());
// B_CC is dropped as we don't collect symbols from A.h in this compilation.
EXPECT_THAT(runFuzzyFind(Idx, ""),
- UnorderedElementsAre(Named("common"), Named("A_CC"),
+ UnorderedElementsAre(Named("common"), Named("A_CC"), Named("g"),
AllOf(Named("f_b"), Declared(), Defined())));
auto Syms = runFuzzyFind(Idx, "common");
@@ -198,7 +198,7 @@ TEST_F(BackgroundIndexTest, ShardStorage
auto ShardSource = MSS.loadShard(testPath("root/A.cc"));
EXPECT_NE(ShardSource, nullptr);
- EXPECT_THAT(*ShardSource->Symbols, UnorderedElementsAre());
+ EXPECT_THAT(*ShardSource->Symbols, UnorderedElementsAre(Named("g")));
EXPECT_THAT(*ShardSource->Refs, RefsAre({FileURI("unittest:///root/A.cc")}));
}
Modified: clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp?rev=351041&r1=351040&r2=351041&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp Mon Jan 14 02:01:17 2019
@@ -62,6 +62,7 @@ public:
: Server(CDB, FSProvider, DiagConsumer, optsForTests()) {
// Make sure the test root directory is created.
FSProvider.Files[testPath("unused")] = "";
+ CDB.ExtraClangFlags = {"-xc++"};
}
protected:
@@ -141,10 +142,11 @@ TEST_F(WorkspaceSymbolsTest, Unnamed) {
TEST_F(WorkspaceSymbolsTest, InMainFile) {
addFile("foo.cpp", R"cpp(
- int test() {
- }
+ int test() {}
+ static test2() {}
)cpp");
- EXPECT_THAT(getSymbols("test"), IsEmpty());
+ EXPECT_THAT(getSymbols("test"),
+ ElementsAre(QName("test"), QName("test2")));
}
TEST_F(WorkspaceSymbolsTest, Namespaces) {
@@ -184,7 +186,7 @@ TEST_F(WorkspaceSymbolsTest, AnonymousNa
addFile("foo.cpp", R"cpp(
#include "foo.h"
)cpp");
- EXPECT_THAT(getSymbols("test"), IsEmpty());
+ EXPECT_THAT(getSymbols("test"), ElementsAre(QName("test")));
}
TEST_F(WorkspaceSymbolsTest, MultiFile) {
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=351041&r1=351040&r2=351041&view=diff
==============================================================================
--- clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp (original)
+++ clang-tools-extra/trunk/unittests/clangd/SymbolCollectorTests.cpp Mon Jan 14 02:01:17 2019
@@ -87,6 +87,9 @@ MATCHER(Deprecated, "") { return arg.Fla
MATCHER(ImplementationDetail, "") {
return arg.Flags & Symbol::ImplementationDetail;
}
+MATCHER(VisibleOutsideFile, "") {
+ return static_cast<bool>(arg.Flags & Symbol::VisibleOutsideFile);
+}
MATCHER(RefRange, "") {
const Ref &Pos = testing::get<0>(arg);
const Range &Range = testing::get<1>(arg);
@@ -113,9 +116,13 @@ public:
// build() must have been called.
bool shouldCollect(llvm::StringRef Name, bool Qualified = true) {
assert(AST.hasValue());
+ const NamedDecl& ND = Qualified ? findDecl(*AST, Name)
+ : findUnqualifiedDecl(*AST, Name);
+ ASTContext& Ctx = AST->getASTContext();
+ const SourceManager& SM = Ctx.getSourceManager();
+ bool MainFile = SM.isWrittenInMainFile(SM.getExpansionLoc(ND.getBeginLoc()));
return SymbolCollector::shouldCollectSymbol(
- Qualified ? findDecl(*AST, Name) : findUnqualifiedDecl(*AST, Name),
- AST->getASTContext(), SymbolCollector::Options());
+ ND, Ctx, SymbolCollector::Options(), MainFile);
}
protected:
@@ -131,18 +138,22 @@ TEST_F(ShouldCollectSymbolTest, ShouldCo
class X{};
auto f() { int Local; } // auto ensures function body is parsed.
struct { int x; } var;
- namespace { class InAnonymous {}; }
}
)",
- "class InMain {};");
+ R"(
+ class InMain {};
+ namespace { class InAnonymous {}; }
+ static void g();
+ )");
auto AST = File.build();
EXPECT_TRUE(shouldCollect("nx"));
EXPECT_TRUE(shouldCollect("nx::X"));
EXPECT_TRUE(shouldCollect("nx::f"));
+ EXPECT_TRUE(shouldCollect("InMain"));
+ EXPECT_TRUE(shouldCollect("InAnonymous", /*Qualified=*/false));
+ EXPECT_TRUE(shouldCollect("g"));
- EXPECT_FALSE(shouldCollect("InMain"));
EXPECT_FALSE(shouldCollect("Local", /*Qualified=*/false));
- EXPECT_FALSE(shouldCollect("InAnonymous", /*Qualified=*/false));
}
TEST_F(ShouldCollectSymbolTest, NoPrivateProtoSymbol) {
@@ -347,6 +358,35 @@ TEST_F(SymbolCollectorTest, CollectSymbo
AllOf(QName("foo::baz"), ForCodeCompletion(true))}));
}
+TEST_F(SymbolCollectorTest, FileLocal) {
+ const std::string Header = R"(
+ class Foo {};
+ namespace {
+ class Ignored {};
+ }
+ void bar();
+ )";
+ const std::string Main = R"(
+ class ForwardDecl;
+ void bar() {}
+ static void a();
+ class B {};
+ namespace {
+ void c();
+ }
+ )";
+ runSymbolCollector(Header, Main);
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(
+ AllOf(QName("Foo"), VisibleOutsideFile()),
+ AllOf(QName("bar"), VisibleOutsideFile()),
+ AllOf(QName("a"), Not(VisibleOutsideFile())),
+ AllOf(QName("B"), Not(VisibleOutsideFile())),
+ AllOf(QName("c"), Not(VisibleOutsideFile())),
+ // FIXME: ForwardDecl likely *is* visible outside.
+ AllOf(QName("ForwardDecl"), Not(VisibleOutsideFile()))));
+}
+
TEST_F(SymbolCollectorTest, Template) {
Annotations Header(R"(
// Template is indexed, specialization and instantiation is not.
@@ -417,7 +457,7 @@ o]]();
void $printdef[[print]]() {}
// Declared/defined in main only.
- int Y;
+ int $ydecl[[Y]];
)cpp");
runSymbolCollector(Header.code(), Main.code());
EXPECT_THAT(Symbols,
@@ -429,7 +469,8 @@ o]]();
AllOf(QName("print"), DeclRange(Header.range("printdecl")),
DefRange(Main.range("printdef"))),
AllOf(QName("Z"), DeclRange(Header.range("zdecl"))),
- AllOf(QName("foo"), DeclRange(Header.range("foodecl")))));
+ AllOf(QName("foo"), DeclRange(Header.range("foodecl"))),
+ AllOf(QName("Y"), DeclRange(Main.range("ydecl")))));
}
TEST_F(SymbolCollectorTest, Refs) {
@@ -508,17 +549,25 @@ TEST_F(SymbolCollectorTest, References)
W* w2 = nullptr; // only one usage counts
X x();
class V;
- V* v = nullptr; // Used, but not eligible for indexing.
class Y{}; // definition doesn't count as a reference
+ V* v = nullptr;
GLOBAL_Z(z); // Not a reference to Z, we don't spell the type.
)";
CollectorOpts.CountReferences = true;
runSymbolCollector(Header, Main);
EXPECT_THAT(Symbols,
- UnorderedElementsAre(AllOf(QName("W"), RefCount(1)),
- AllOf(QName("X"), RefCount(1)),
- AllOf(QName("Y"), RefCount(0)),
- AllOf(QName("Z"), RefCount(0)), QName("y")));
+ UnorderedElementsAreArray(
+ {AllOf(QName("W"), RefCount(1)),
+ AllOf(QName("X"), RefCount(1)),
+ AllOf(QName("Y"), RefCount(0)),
+ AllOf(QName("Z"), RefCount(0)),
+ AllOf(QName("y"), RefCount(0)),
+ AllOf(QName("z"), RefCount(0)),
+ AllOf(QName("x"), RefCount(0)),
+ AllOf(QName("w"), RefCount(0)),
+ AllOf(QName("w2"), RefCount(0)),
+ AllOf(QName("V"), RefCount(1)),
+ AllOf(QName("v"), RefCount(0))}));
}
TEST_F(SymbolCollectorTest, SymbolRelativeNoFallback) {
@@ -621,22 +670,28 @@ TEST_F(SymbolCollectorTest, SymbolFormed
DeclURI(TestHeaderURI))));
}
-TEST_F(SymbolCollectorTest, IgnoreSymbolsInMainFile) {
- const std::string Header = R"(
+TEST_F(SymbolCollectorTest, SymbolsInMainFile) {
+ const std::string Main = R"(
class Foo {};
void f1();
inline void f2() {}
- )";
- const std::string Main = R"(
+
namespace {
- void ff() {} // ignore
+ void ff() {}
}
- void main_f() {} // ignore
+ namespace foo {
+ namespace {
+ class Bar {};
+ }
+ }
+ void main_f() {}
void f1() {}
)";
- runSymbolCollector(Header, Main);
+ runSymbolCollector(/*Header=*/"", Main);
EXPECT_THAT(Symbols,
- UnorderedElementsAre(QName("Foo"), QName("f1"), QName("f2")));
+ UnorderedElementsAre(QName("Foo"), QName("f1"), QName("f2"),
+ QName("ff"), QName("foo"), QName("foo::Bar"),
+ QName("main_f")));
}
TEST_F(SymbolCollectorTest, ClassMembers) {
@@ -978,7 +1033,8 @@ TEST_F(SymbolCollectorTest, ReferencesIn
CollectorOpts.CountReferences = true;
runSymbolCollector(Header, Main);
EXPECT_THAT(Symbols, UnorderedElementsAre(AllOf(QName("X"), RefCount(1)),
- AllOf(QName("Y"), RefCount(1))));
+ AllOf(QName("Y"), RefCount(1)),
+ AllOf(QName("C"), RefCount(0))));
}
TEST_F(SymbolCollectorTest, Origin) {
@@ -1005,7 +1061,7 @@ TEST_F(SymbolCollectorTest, CollectMacro
CollectorOpts.CollectMacro = true;
runSymbolCollector(Header.code(), Main);
EXPECT_THAT(Symbols,
- UnorderedElementsAre(QName("p"),
+ UnorderedElementsAre(QName("p"), QName("t"),
AllOf(QName("X"), DeclURI(TestHeaderURI),
IncludeHeader(TestHeaderURI)),
AllOf(Labeled("MAC(x)"), RefCount(0),
More information about the cfe-commits
mailing list