ArcsinX created this revision.
Herald added subscribers: usaxena95, kadircet, arphaman.
ArcsinX requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang.
This patch is a try to fix `WorkspaceSymbols.Macros` test after D93796
<https://reviews.llvm.org/D93796>.
If a macro definition is in the preamble section, then it appears to be in the
preamble (static) index and not in the main-file (dynamic) index.
Thus, a such macro could not be found at a symbol search according to the logic
that we skip symbols from the static index if the location of these symbols is
inside the dynamic index files.
To fix this behavior this patch adds main file macros into the main-file
(dynamic) index.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D94477
Files:
clang-tools-extra/clangd/CollectMacros.cpp
clang-tools-extra/clangd/CollectMacros.h
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
Index: clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ParsedASTTests.cpp
@@ -341,7 +341,7 @@
std::vector<Position> 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);
Index: clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/FindSymbolsTests.cpp
+++ 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
Index: clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/CollectMacrosTests.cpp
+++ 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<Range> 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;
}
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ 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);
+ }
}
}
}
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ 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));
}
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===================================================================
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ 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});
Index: clang-tools-extra/clangd/CollectMacros.h
===================================================================
--- clang-tools-extra/clangd/CollectMacros.h
+++ 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<SymbolID, std::vector<Range>> MacroRefs;
+ llvm::DenseMap<SymbolID, std::vector<std::pair<Range, bool>>> 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;
Index: clang-tools-extra/clangd/CollectMacros.cpp
===================================================================
--- clang-tools-extra/clangd/CollectMacros.cpp
+++ 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);
}
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
