kadircet created this revision.
kadircet added a reviewer: hokein.
Herald added subscribers: cfe-commits, usaxena95, arphaman, jkorous, MaskRay,
ilya-biryukov.
Herald added a project: clang.
This was resulting in macros coming from preambles vanishing when user
have opened the source header. For example:
// test.h:
and
// test.cc
^
If user only opens test.cc, we'll get `X` as a completion candidate,
since it is indexed as part of the preamble. But if the user opens
test.h afterwards we would index it as part of the main file and lose
the symbol (as new index shard for test.h will override the existing one
in dynamic index).
Also we were not setting origins for macros correctly, this patch also
fixes it.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D84297
Files:
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1402,6 +1402,9 @@
runSymbolCollector("class Foo {};", /*Main=*/"");
EXPECT_THAT(Symbols, UnorderedElementsAre(
Field(&Symbol::Origin, SymbolOrigin::Static)));
+ runSymbolCollector("#define FOO", /*Main=*/"");
+ EXPECT_THAT(Symbols, UnorderedElementsAre(
+ Field(&Symbol::Origin, SymbolOrigin::Static)));
}
TEST_F(SymbolCollectorTest, CollectMacros) {
@@ -1504,6 +1507,13 @@
EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "PUNCT").ID, _)));
}
+TEST_F(SymbolCollectorTest, MacrosInHeaders) {
+ CollectorOpts.CollectMacro = true;
+ TestFileName = testPath("test.h");
+ runSymbolCollector("", "#define X");
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(AllOf(QName("X"),
ForCodeCompletion(true))));
+}
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -377,7 +377,10 @@
const auto &SM = PP->getSourceManager();
auto DefLoc = MI->getDefinitionLoc();
auto SpellingLoc = SM.getSpellingLoc(Loc);
- bool IsMainFileSymbol = SM.isInMainFile(SM.getExpansionLoc(DefLoc));
+ bool IsMainFileOnly =
+ SM.isInMainFile(SM.getExpansionLoc(DefLoc)) &&
+ !isHeaderFile(SM.getFileEntryForID(SM.getMainFileID())->getName(),
+ ASTCtx->getLangOpts());
// Builtin macros don't have useful locations and aren't needed in
completion.
if (MI->isBuiltinMacro())
@@ -392,7 +395,7 @@
return true;
// Do not store references to main-file macros.
- if ((static_cast<unsigned>(Opts.RefFilter) & Roles) && !IsMainFileSymbol &&
+ if ((static_cast<unsigned>(Opts.RefFilter) & Roles) && !IsMainFileOnly &&
(Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
MacroRefs[*ID].push_back({Loc, Roles});
@@ -401,7 +404,7 @@
return true;
// Skip main-file macros if we are not collecting them.
- if (IsMainFileSymbol && !Opts.CollectMainFileSymbols)
+ if (IsMainFileOnly && !Opts.CollectMainFileSymbols)
return false;
// Mark the macro as referenced if this is a reference coming from the main
@@ -425,11 +428,12 @@
Symbol S;
S.ID = std::move(*ID);
S.Name = Name->getName();
- if (!IsMainFileSymbol) {
+ if (!IsMainFileOnly) {
S.Flags |= Symbol::IndexedForCodeCompletion;
S.Flags |= Symbol::VisibleOutsideFile;
}
S.SymInfo = index::getSymbolInfoForMacro(*MI);
+ S.Origin = Opts.Origin;
std::string FileURI;
// FIXME: use the result to filter out symbols.
shouldIndexFile(SM.getFileID(Loc));
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1402,6 +1402,9 @@
runSymbolCollector("class Foo {};", /*Main=*/"");
EXPECT_THAT(Symbols, UnorderedElementsAre(
Field(&Symbol::Origin, SymbolOrigin::Static)));
+ runSymbolCollector("#define FOO", /*Main=*/"");
+ EXPECT_THAT(Symbols, UnorderedElementsAre(
+ Field(&Symbol::Origin, SymbolOrigin::Static)));
}
TEST_F(SymbolCollectorTest, CollectMacros) {
@@ -1504,6 +1507,13 @@
EXPECT_THAT(Refs, Contains(Pair(findSymbol(Symbols, "PUNCT").ID, _)));
}
+TEST_F(SymbolCollectorTest, MacrosInHeaders) {
+ CollectorOpts.CollectMacro = true;
+ TestFileName = testPath("test.h");
+ runSymbolCollector("", "#define X");
+ EXPECT_THAT(Symbols,
+ UnorderedElementsAre(AllOf(QName("X"), ForCodeCompletion(true))));
+}
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -377,7 +377,10 @@
const auto &SM = PP->getSourceManager();
auto DefLoc = MI->getDefinitionLoc();
auto SpellingLoc = SM.getSpellingLoc(Loc);
- bool IsMainFileSymbol = SM.isInMainFile(SM.getExpansionLoc(DefLoc));
+ bool IsMainFileOnly =
+ SM.isInMainFile(SM.getExpansionLoc(DefLoc)) &&
+ !isHeaderFile(SM.getFileEntryForID(SM.getMainFileID())->getName(),
+ ASTCtx->getLangOpts());
// Builtin macros don't have useful locations and aren't needed in completion.
if (MI->isBuiltinMacro())
@@ -392,7 +395,7 @@
return true;
// Do not store references to main-file macros.
- if ((static_cast<unsigned>(Opts.RefFilter) & Roles) && !IsMainFileSymbol &&
+ if ((static_cast<unsigned>(Opts.RefFilter) & Roles) && !IsMainFileOnly &&
(Opts.RefsInHeaders || SM.getFileID(SpellingLoc) == SM.getMainFileID()))
MacroRefs[*ID].push_back({Loc, Roles});
@@ -401,7 +404,7 @@
return true;
// Skip main-file macros if we are not collecting them.
- if (IsMainFileSymbol && !Opts.CollectMainFileSymbols)
+ if (IsMainFileOnly && !Opts.CollectMainFileSymbols)
return false;
// Mark the macro as referenced if this is a reference coming from the main
@@ -425,11 +428,12 @@
Symbol S;
S.ID = std::move(*ID);
S.Name = Name->getName();
- if (!IsMainFileSymbol) {
+ if (!IsMainFileOnly) {
S.Flags |= Symbol::IndexedForCodeCompletion;
S.Flags |= Symbol::VisibleOutsideFile;
}
S.SymInfo = index::getSymbolInfoForMacro(*MI);
+ S.Origin = Opts.Origin;
std::string FileURI;
// FIXME: use the result to filter out symbols.
shouldIndexFile(SM.getFileID(Loc));
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
