Index: clang-tools-extra/trunk/clangd/index/Index.h =================================================================== --- clang-tools-extra/trunk/clangd/index/Index.h +++ clang-tools-extra/trunk/clangd/index/Index.h @@ -241,6 +241,8 @@ 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; 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 @@ -28,13 +28,14 @@ /// 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 @@ /// 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 FileFilter = nullptr; @@ -81,7 +85,7 @@ /// 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 @@ 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. 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 @@ -240,22 +240,20 @@ 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::::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 @@ explicitTemplateSpecialization(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 @@ 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(ND) && + // Do not store references to main-file symbols. + if (CollectRef && !IsMainFileOnly && !isa(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 @@ const NamedDecl &OriginalDecl = *cast(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(index::SymbolRole::Definition)) addDefinition(OriginalDecl, *BasicSymbol); @@ -506,7 +506,8 @@ } 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 @@ // 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); Index: clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/BackgroundIndexTests.cpp @@ -125,7 +125,7 @@ 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 @@ 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 @@ 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")})); } Index: clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp =================================================================== --- clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp +++ clang-tools-extra/trunk/unittests/clangd/FindSymbolsTests.cpp @@ -62,6 +62,7 @@ : 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, 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 @@ 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) { 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 @@ -87,6 +87,9 @@ MATCHER(ImplementationDetail, "") { return arg.Flags & Symbol::ImplementationDetail; } +MATCHER(VisibleOutsideFile, "") { + return static_cast(arg.Flags & Symbol::VisibleOutsideFile); +} MATCHER(RefRange, "") { const Ref &Pos = testing::get<0>(arg); const Range &Range = testing::get<1>(arg); @@ -113,9 +116,13 @@ // 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ 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),