diff --git a/clang-tools-extra/clangd/CollectMacros.h b/clang-tools-extra/clangd/CollectMacros.h --- a/clang-tools-extra/clangd/CollectMacros.h +++ b/clang-tools-extra/clangd/CollectMacros.h @@ -23,9 +23,12 @@ struct MainFileMacros { llvm::StringSet<> Names; + // References ranges with a bool indicator which is true if + // the reference is a definition and false otherwise. + // // Instead of storing SourceLocation, we have to store the token range because // SourceManager from preamble is not available when we build the AST. - llvm::DenseMap> MacroRefs; + llvm::DenseMap>> MacroRefs; // Somtimes it is not possible to compute the SymbolID for the Macro, e.g. a // reference to an undefined macro. Store them separately, e.g. for semantic // highlighting. @@ -49,7 +52,7 @@ } void MacroDefined(const Token &MacroName, const MacroDirective *MD) override { - add(MacroName, MD->getMacroInfo()); + add(MacroName, MD->getMacroInfo(), /*IsDefinition=*/true); } void MacroExpands(const Token &MacroName, const MacroDefinition &MD, @@ -87,7 +90,8 @@ } private: - void add(const Token &MacroNameTok, const MacroInfo *MI); + void add(const Token &MacroNameTok, const MacroInfo *MI, + bool IsDefinition = false); const SourceManager &SM; bool InMainFile = true; MainFileMacros &Out; diff --git a/clang-tools-extra/clangd/CollectMacros.cpp b/clang-tools-extra/clangd/CollectMacros.cpp --- a/clang-tools-extra/clangd/CollectMacros.cpp +++ b/clang-tools-extra/clangd/CollectMacros.cpp @@ -13,8 +13,8 @@ namespace clang { namespace clangd { -void CollectMainFileMacros::add(const Token &MacroNameTok, - const MacroInfo *MI) { +void CollectMainFileMacros::add(const Token &MacroNameTok, const MacroInfo *MI, + bool IsDefinition) { if (!InMainFile) return; auto Loc = MacroNameTok.getLocation(); @@ -26,7 +26,7 @@ auto Range = halfOpenToRange( SM, CharSourceRange::getCharRange(Loc, MacroNameTok.getEndLoc())); if (auto SID = getSymbolID(Name, MI, SM)) - Out.MacroRefs[SID].push_back(Range); + Out.MacroRefs[SID].push_back({Range, IsDefinition}); else Out.UnknownMacros.push_back(Range); } diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -397,7 +397,7 @@ // Add highlightings for macro references. for (const auto &SIDToRefs : AST.getMacros().MacroRefs) { for (const auto &M : SIDToRefs.second) - Builder.addToken({HighlightingKind::Macro, M}); + Builder.addToken({HighlightingKind::Macro, M.first}); } for (const auto &M : AST.getMacros().UnknownMacros) Builder.addToken({HighlightingKind::Macro, M}); diff --git a/clang-tools-extra/clangd/XRefs.cpp b/clang-tools-extra/clangd/XRefs.cpp --- a/clang-tools-extra/clangd/XRefs.cpp +++ b/clang-tools-extra/clangd/XRefs.cpp @@ -1311,7 +1311,7 @@ if (Refs != IDToRefs.end()) { for (const auto &Ref : Refs->second) { Location Result; - Result.range = Ref; + Result.range = Ref.first; Result.uri = URIMainFile; Results.References.push_back(std::move(Result)); } 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 @@ -386,7 +386,9 @@ const auto MainFileURI = toURI(SM, MainFileEntry->getName(), Opts); // Add macro references. for (const auto &IDToRefs : MacroRefsToIndex.MacroRefs) { - for (const auto &Range : IDToRefs.second) { + for (const auto &RangeAndIsDefinition : IDToRefs.second) { + const auto &Range = RangeAndIsDefinition.first; + bool IsDefinition = RangeAndIsDefinition.second; Ref R; R.Location.Start.setLine(Range.start.line); R.Location.Start.setColumn(Range.start.character); @@ -394,8 +396,26 @@ R.Location.End.setColumn(Range.end.character); R.Location.FileURI = MainFileURI.c_str(); // FIXME: Add correct RefKind information to MainFileMacros. - R.Kind = RefKind::Reference; + R.Kind = IsDefinition ? RefKind::Definition : RefKind::Reference; Refs.insert(IDToRefs.first, R); + if (IsDefinition) { + Symbol S; + S.ID = IDToRefs.first; + auto StartLoc = sourceLocationInMainFile(SM, Range.start); + assert(StartLoc); + auto EndLoc = sourceLocationInMainFile(SM, Range.end); + assert(EndLoc); + S.Name = Lexer::getSourceText( + CharSourceRange::getCharRange(SourceRange(*StartLoc, *EndLoc)), SM, + clang::LangOptions()); + S.SymInfo.Kind = index::SymbolKind::Macro; + S.SymInfo.SubKind = index::SymbolSubKind::None; + S.SymInfo.Properties = index::SymbolPropertySet(); + S.SymInfo.Lang = index::SymbolLanguage::C; + S.Origin = Opts.Origin; + S.CanonicalDeclaration = R.Location; + Symbols.insert(S); + } } } } diff --git a/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp b/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp --- a/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp +++ b/clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp @@ -95,8 +95,10 @@ assert(Macro); auto SID = getSymbolID(Macro->Name, Macro->Info, SM); - EXPECT_THAT(ExpectedRefs, - UnorderedElementsAreArray(ActualMacroRefs.MacroRefs[SID])) + std::vector MacroRefs; + for (const auto &RangeAndIsDefinition : ActualMacroRefs.MacroRefs[SID]) + MacroRefs.push_back(RangeAndIsDefinition.first); + EXPECT_THAT(ExpectedRefs, UnorderedElementsAreArray(MacroRefs)) << "Annotation=" << I << ", MacroName=" << Macro->Name << ", Test = " << Test; } diff --git a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp --- a/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp @@ -52,17 +52,7 @@ return *SymbolInfos; } -// FIXME: We update two indexes during main file processing: -// - preamble index (static) -// - main-file index (dynamic) -// The macro in this test appears to be in the preamble index and not -// in the main-file index. According to our logic of indexes merging, we -// do not take this macro from the static (preamble) index, because it -// location within the file from the dynamic (main-file) index. -// -// Possible solution is to exclude main-file symbols from the preamble -// index, after that we can enable this test again. -TEST(WorkspaceSymbols, DISABLED_Macros) { +TEST(WorkspaceSymbols, Macros) { TestTU TU; TU.Code = R"cpp( #define MACRO X diff --git a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp --- a/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp +++ b/clang-tools-extra/clangd/unittests/ParsedASTTests.cpp @@ -341,7 +341,7 @@ std::vector MacroExpansionPositions; for (const auto &SIDToRefs : AST.getMacros().MacroRefs) { for (const auto &R : SIDToRefs.second) - MacroExpansionPositions.push_back(R.start); + MacroExpansionPositions.push_back(R.first.start); } for (const auto &R : AST.getMacros().UnknownMacros) MacroExpansionPositions.push_back(R.start);