diff --git a/clang-tools-extra/clangd/index/SymbolCollector.h b/clang-tools-extra/clangd/index/SymbolCollector.h --- a/clang-tools-extra/clangd/index/SymbolCollector.h +++ b/clang-tools-extra/clangd/index/SymbolCollector.h @@ -151,11 +151,12 @@ std::shared_ptr CompletionAllocator; std::unique_ptr CompletionTUInfo; Options Opts; - using DeclRef = std::pair; + using SymbolRef = std::pair; // Symbols referenced from the current TU, flushed on finish(). llvm::DenseSet ReferencedDecls; llvm::DenseSet ReferencedMacros; - llvm::DenseMap> DeclRefs; + llvm::DenseMap> DeclRefs; + llvm::DenseMap> MacroRefs; // Maps canonical declaration provided by clang to canonical declaration for // an index symbol, if clangd prefers a different declaration than that // provided by clang. For example, friend declaration might be considered diff --git a/clang-tools-extra/clangd/index/SymbolCollector.cpp b/clang-tools-extra/clangd/index/SymbolCollector.cpp --- a/clang-tools-extra/clangd/index/SymbolCollector.cpp +++ b/clang-tools-extra/clangd/index/SymbolCollector.cpp @@ -16,6 +16,7 @@ #include "SourceCode.h" #include "SymbolLocation.h" #include "URI.h" +#include "index/SymbolID.h" #include "clang/AST/Decl.h" #include "clang/AST/DeclBase.h" #include "clang/AST/DeclCXX.h" @@ -345,43 +346,52 @@ const MacroInfo *MI, index::SymbolRoleSet Roles, SourceLocation Loc) { - if (!Opts.CollectMacro) - return true; assert(PP.get()); const auto &SM = PP->getSourceManager(); auto DefLoc = MI->getDefinitionLoc(); + auto SpellingLoc = SM.getSpellingLoc(Loc); + bool IsMainFileSymbol = SM.isInMainFile(SM.getExpansionLoc(DefLoc)); // Builtin macros don't have useful locations and aren't needed in completion. if (MI->isBuiltinMacro()) return true; - // Skip main-file symbols if we are not collecting them. - bool IsMainFileSymbol = SM.isInMainFile(SM.getExpansionLoc(DefLoc)); - if (IsMainFileSymbol && !Opts.CollectMainFileSymbols) - return false; - // Also avoid storing predefined macros like __DBL_MIN__. if (SM.isWrittenInBuiltinFile(DefLoc)) return true; + auto ID = getSymbolID(Name->getName(), MI, SM); + if (!ID) + return true; + + // Do not store references to main-file symbols. + if ((static_cast(Opts.RefFilter) & Roles) && !IsMainFileSymbol && + (Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID())) + MacroRefs[*ID].push_back({Loc, Roles}); + + // Collect symbols. + if (!Opts.CollectMacro) + return true; + + // Skip main-file symbols if we are not collecting them. + if (IsMainFileSymbol && !Opts.CollectMainFileSymbols) + return false; + // Mark the macro as referenced if this is a reference coming from the main // file. The macro may not be an interesting symbol, but it's cheaper to check // at the end. if (Opts.CountReferences && (Roles & static_cast(index::SymbolRole::Reference)) && - SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID()) + SM.getFileID(SpellingLoc) == SM.getMainFileID()) ReferencedMacros.insert(Name); + // Don't continue indexing if this is a mere reference. // FIXME: remove macro with ID if it is undefined. if (!(Roles & static_cast(index::SymbolRole::Declaration) || Roles & static_cast(index::SymbolRole::Definition))) return true; - auto ID = getSymbolID(Name->getName(), MI, SM); - if (!ID) - return true; - // Only collect one instance in case there are multiple. if (Symbols.find(*ID) != nullptr) return true; @@ -471,6 +481,7 @@ } if (Opts.CollectMacro) { assert(PP); + // First, drop header guards. We can't identify these until EOF. for (const IdentifierInfo *II : IndexedMacros) { if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo()) @@ -485,10 +496,10 @@ IncRef(*ID); } } - // Fill in IncludeHeaders. // We delay this until end of TU so header guards are all resolved. - // Symbols in slabs aren' mutable, so insert() has to walk all the strings :-( + // Symbols in slabs aren' mutable, so insert() has to walk all the strings + // :-( llvm::SmallString<256> QName; for (const auto &Entry : IncludeFiles) if (const Symbol *S = Symbols.find(Entry.first)) { @@ -518,24 +529,36 @@ } return Found->second; }; + auto CollectRef = + [&](SymbolID ID, + const std::pair &LocAndRole) { + auto FileID = SM.getFileID(LocAndRole.first); + // FIXME: use the result to filter out references. + shouldIndexFile(FileID); + if (auto FileURI = GetURI(FileID)) { + auto Range = + getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts()); + Ref R; + R.Location.Start = Range.first; + R.Location.End = Range.second; + R.Location.FileURI = FileURI->c_str(); + R.Kind = toRefKind(LocAndRole.second); + Refs.insert(ID, R); + } + }; + // Populate Refs slab from MacroRefs. + for (const auto &MacroRef : MacroRefs) { + const SymbolID &ID = MacroRef.first; + for (const auto &LocAndRole : MacroRef.second) { + CollectRef(ID, LocAndRole); + } + } // Populate Refs slab from DeclRefs. if (auto MainFileURI = GetURI(SM.getMainFileID())) { for (const auto &It : DeclRefs) { if (auto ID = getSymbolID(It.first)) { for (const auto &LocAndRole : It.second) { - auto FileID = SM.getFileID(LocAndRole.first); - // FIXME: use the result to filter out references. - shouldIndexFile(FileID); - if (auto FileURI = GetURI(FileID)) { - auto Range = - getTokenRange(LocAndRole.first, SM, ASTCtx->getLangOpts()); - Ref R; - R.Location.Start = Range.first; - R.Location.End = Range.second; - R.Location.FileURI = FileURI->c_str(); - R.Kind = toRefKind(LocAndRole.second); - Refs.insert(*ID, R); - } + CollectRef(*ID, LocAndRole); } } } diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp --- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp +++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp @@ -39,6 +39,7 @@ using ::testing::Each; using ::testing::ElementsAre; using ::testing::Field; +using ::testing::IsEmpty; using ::testing::Not; using ::testing::Pair; using ::testing::UnorderedElementsAre; @@ -214,7 +215,8 @@ CreateASTConsumer(CompilerInstance &CI, llvm::StringRef InFile) override { if (PragmaHandler) CI.getPreprocessor().addCommentHandler(PragmaHandler); - return createIndexingASTConsumer(DataConsumer, Opts, CI.getPreprocessorPtr()); + return createIndexingASTConsumer(DataConsumer, Opts, + CI.getPreprocessorPtr()); } bool BeginInvocation(CompilerInstance &CI) override { @@ -577,15 +579,16 @@ TEST_F(SymbolCollectorTest, Refs) { Annotations Header(R"( - class $foo[[Foo]] { + #define MACRO(X) (X + 1) + class Foo { public: - $foo[[Foo]]() {} - $foo[[Foo]](int); + Foo() {} + Foo(int); }; - class $bar[[Bar]]; - void $func[[func]](); + class Bar; + void func(); - namespace $ns[[NS]] {} // namespace ref is ignored + namespace NS {} // namespace ref is ignored )"); Annotations Main(R"( class $bar[[Bar]] {}; @@ -598,19 +601,20 @@ $func[[func]](); int abc = 0; $foo[[Foo]] foo2 = abc; + abc = $macro[[MACRO]](1); } )"); Annotations SymbolsOnlyInMainCode(R"( + #define FUNC(X) (X+1) int a; void b() {} - static const int c = 0; + static const int c = FUNC(1); class d {}; )"); CollectorOpts.RefFilter = RefKind::All; + CollectorOpts.CollectMacro = true; runSymbolCollector(Header.code(), (Main.code() + SymbolsOnlyInMainCode.code()).str()); - auto HeaderSymbols = TestTU::withHeaderCode(Header.code()).headerSymbols(); - EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID, HaveRanges(Main.ranges("foo"))))); EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Bar").ID, @@ -618,12 +622,84 @@ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "func").ID, HaveRanges(Main.ranges("func"))))); EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _)))); - // Symbols *only* in the main file (a, b, c) had no refs collected. + EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "MACRO").ID, + HaveRanges(Main.ranges("macro"))))); + // Symbols *only* in the main file (a, b, c, FUNC) had no refs collected. auto MainSymbols = TestTU::withHeaderCode(SymbolsOnlyInMainCode.code()).headerSymbols(); EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "a").ID, _)))); EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "b").ID, _)))); EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "c").ID, _)))); + EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(MainSymbols, "FUNC").ID, _)))); +} + +TEST_F(SymbolCollectorTest, MacroRefInHeader) { + Annotations Header(R"( + #define $foo[[FOO]](X) (X + 1) + #define $bar[[BAR]](X) (X + 2) + + // Macro defined multiple times. + #define $ud1[[UD]] 1 + int ud_1 = $ud1[[UD]]; + #undef UD + + #define $ud2[[UD]] 2 + int ud_2 = $ud2[[UD]]; + #undef UD + + // Macros from token concatenations not included. + #define $concat[[CONCAT]](X) X##A() + #define $prepend[[PREPEND]](X) MACRO##X() + #define $macroa[[MACROA]]() 123 + int B = $concat[[CONCAT]](MACRO); + int D = $prepend[[PREPEND]](A); + + void fff() { + int abc = $foo[[FOO]](1) + $bar[[BAR]]($foo[[FOO]](1)); + } + )"); + CollectorOpts.RefFilter = RefKind::All; + CollectorOpts.RefsInHeaders = true; + // Need this to get the SymbolID for macros for tests. + CollectorOpts.CollectMacro = true; + + runSymbolCollector(Header.code(), ""); + + EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "FOO").ID, + HaveRanges(Header.ranges("foo"))))); + EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "BAR").ID, + HaveRanges(Header.ranges("bar"))))); + EXPECT_THAT(Refs, Contains(Pair(_, HaveRanges(Header.ranges("ud1"))))); + EXPECT_THAT(Refs, Contains(Pair(_, HaveRanges(Header.ranges("ud2"))))); + EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "CONCAT").ID, + HaveRanges(Header.ranges("concat"))))); + EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "PREPEND").ID, + HaveRanges(Header.ranges("prepend"))))); + EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "MACROA").ID, + HaveRanges(Header.ranges("macroa"))))); +} + +TEST_F(SymbolCollectorTest, MacroRefWithoutCollectingSymbol) { + Annotations Header(R"( + #define $foo[[FOO]](X) (X + 1) + int abc = $foo[[FOO]](1); + )"); + CollectorOpts.RefFilter = RefKind::All; + CollectorOpts.RefsInHeaders = true; + CollectorOpts.CollectMacro = false; + + runSymbolCollector(Header.code(), ""); + + EXPECT_THAT(Refs, Contains(Pair(_, HaveRanges(Header.ranges("foo"))))); +} + +TEST_F(SymbolCollectorTest, MacrosWithRefFilter) { + Annotations Header("#define $macro[[MACRO]](X) (X + 1)"); + Annotations Main("void foo() { int x = $macro[[MACRO]](1); }"); + CollectorOpts.RefFilter = RefKind::Unknown; + runSymbolCollector(Header.code(), Main.code()); + auto HeaderSymbols = TestTU::withHeaderCode(Header.code()).headerSymbols(); + EXPECT_THAT(Refs, IsEmpty()); } TEST_F(SymbolCollectorTest, NameReferences) { @@ -675,21 +751,26 @@ TestFileName = testPath("foo.hh"); runSymbolCollector("", Header.code()); EXPECT_THAT(Symbols, UnorderedElementsAre(QName("Foo"), QName("Func"))); - EXPECT_THAT(Refs, UnorderedElementsAre(Pair(findSymbol(Symbols, "Foo").ID, - HaveRanges(Header.ranges("Foo"))), - Pair(findSymbol(Symbols, "Func").ID, - HaveRanges(Header.ranges("Func"))))); + EXPECT_THAT(Refs, + UnorderedElementsAre(Pair(findSymbol(Symbols, "Foo").ID, + HaveRanges(Header.ranges("Foo"))), + Pair(findSymbol(Symbols, "Func").ID, + HaveRanges(Header.ranges("Func"))))); } TEST_F(SymbolCollectorTest, RefsInHeaders) { CollectorOpts.RefFilter = RefKind::All; CollectorOpts.RefsInHeaders = true; + CollectorOpts.CollectMacro = true; Annotations Header(R"( - class [[Foo]] {}; + #define $macro[[MACRO]](x) (x+1) + class $foo[[Foo]] {}; )"); runSymbolCollector(Header.code(), ""); EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "Foo").ID, - HaveRanges(Header.ranges())))); + HaveRanges(Header.ranges("foo"))))); + EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "MACRO").ID, + HaveRanges(Header.ranges("macro"))))); } TEST_F(SymbolCollectorTest, Relations) { @@ -704,7 +785,7 @@ Contains(Relation{Base.ID, RelationKind::BaseOf, Derived.ID})); } -TEST_F(SymbolCollectorTest, References) { +TEST_F(SymbolCollectorTest, CountReferences) { const std::string Header = R"( class W; class X {};