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,11 @@ 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> ReferencedMacros; + llvm::DenseMap> DeclRefs; // 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" @@ -365,23 +366,19 @@ if (SM.isWrittenInBuiltinFile(DefLoc)) return true; - // 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()) - ReferencedMacros.insert(Name); + auto ID = getSymbolID(Name->getName(), MI, SM); + if (!ID) + return true; + + if (SM.getFileID(SM.getSpellingLoc(Loc)) == SM.getMainFileID()) + ReferencedMacros[*ID].push_back({Loc, Roles}); + // 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; @@ -469,26 +466,11 @@ IncRef(*ID); } } - 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()) - if (auto ID = getSymbolID(II->getName(), MI, PP->getSourceManager())) - if (MI->isUsedForHeaderGuard()) - Symbols.erase(*ID); - } - // Now increment refcounts. - for (const IdentifierInfo *II : ReferencedMacros) { - if (const auto *MI = PP->getMacroDefinition(II).getMacroInfo()) - if (auto ID = getSymbolID(II->getName(), MI, PP->getSourceManager())) - 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 +500,54 @@ } 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); + } + }; + 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()) + if (auto ID = getSymbolID(II->getName(), MI, PP->getSourceManager())) + if (MI->isUsedForHeaderGuard()) + Symbols.erase(*ID); + } + // Now collect refs and increment refcounts. + for (const auto &MacroRef : ReferencedMacros) { + SymbolID ID = MacroRef.first; + bool IsRefFromMainFile = false; + for (const auto &LocAndRole : MacroRef.second) { + IsRefFromMainFile |= + (LocAndRole.second & + static_cast(index::SymbolRole::Reference)); + if (LocAndRole.second & static_cast(Opts.RefFilter)) + CollectRef(ID, LocAndRole); + } + if (Opts.CountReferences && IsRefFromMainFile) + IncRef(ID); + } + } // 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; @@ -577,15 +578,16 @@ TEST_F(SymbolCollectorTest, Refs) { Annotations Header(R"( - class $foo[[Foo]] { + #define $macro[[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 +600,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 +621,72 @@ EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "func").ID, HaveRanges(Main.ranges("func"))))); EXPECT_THAT(Refs, Not(Contains(Pair(findSymbol(Symbols, "NS").ID, _)))); + EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "MACRO").ID, + HaveRanges(Main.ranges("macro"))))); // Symbols *only* in the main file (a, b, c) 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, MacroRef) { + Annotations Main(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.CollectMacro = true; + CollectorOpts.CollectMainFileSymbols = true; + + runSymbolCollector("", Main.code()); + + EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "FOO").ID, + HaveRanges(Main.ranges("foo"))))); + EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "BAR").ID, + HaveRanges(Main.ranges("bar"))))); + EXPECT_THAT(Refs, Contains(Pair(_, + HaveRanges(Main.ranges("ud1"))))); + EXPECT_THAT(Refs, Contains(Pair(_, + HaveRanges(Main.ranges("ud2"))))); + EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "CONCAT").ID, + HaveRanges(Main.ranges("concat"))))); + EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "PREPEND").ID, + HaveRanges(Main.ranges("prepend"))))); + EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "MACROA").ID, + HaveRanges(Main.ranges("macroa"))))); +} + +TEST_F(SymbolCollectorTest, RefsWithoutMacros) { + Annotations Header("#define $macro[[MACRO]](X) (X + 1)"); + Annotations Main("void foo() { int x = $macro[[MACRO]](1); }"); + CollectorOpts.RefFilter = RefKind::All; + CollectorOpts.CollectMacro = false; + runSymbolCollector(Header.code(), Main.code()); + auto HeaderSymbols = TestTU::withHeaderCode(Header.code()).headerSymbols(); + EXPECT_THAT(Refs, IsEmpty()); } TEST_F(SymbolCollectorTest, NameReferences) { @@ -675,10 +738,11 @@ 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) {