adamcz updated this revision to Diff 266240.
adamcz marked 3 inline comments as done.
adamcz added a comment.
Addressed review comments.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D80525/new/
https://reviews.llvm.org/D80525
Files:
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
clang-tools-extra/clangd/unittests/TestTU.cpp
clang-tools-extra/clangd/unittests/TestTU.h
clang/include/clang/Lex/Preprocessor.h
clang/lib/Index/IndexingAction.cpp
Index: clang/lib/Index/IndexingAction.cpp
===================================================================
--- clang/lib/Index/IndexingAction.cpp
+++ clang/lib/Index/IndexingAction.cpp
@@ -147,14 +147,24 @@
Unit.visitLocalTopLevelDecls(&IndexCtx, topLevelDeclVisitor);
}
-static void indexPreprocessorMacros(const Preprocessor &PP,
+static void indexPreprocessorMacros(Preprocessor &PP,
IndexDataConsumer &DataConsumer) {
- for (const auto &M : PP.macros())
- if (MacroDirective *MD = M.second.getLatest())
+ for (const auto &M : PP.macros()) {
+ if (MacroDirective *MD = M.second.getLatest()) {
+ auto *MI = MD->getMacroInfo();
+ if (!MI) {
+ // It is possible for MI to be nullptr if we our translation unit had
+ // only #undef of a macro defined in a module. In that case, dig deeper
+ // and use the definition from the module to yield the same indexing
+ // result whether we are using modules or not.
+ MI = PP.getMacroInfoFromModules(M.first);
+ }
+
DataConsumer.handleMacroOccurrence(
- M.first, MD->getMacroInfo(),
- static_cast<unsigned>(index::SymbolRole::Definition),
+ M.first, MI, static_cast<unsigned>(index::SymbolRole::Definition),
MD->getLocation());
+ }
+ }
}
void index::indexASTUnit(ASTUnit &Unit, IndexDataConsumer &DataConsumer,
Index: clang/include/clang/Lex/Preprocessor.h
===================================================================
--- clang/include/clang/Lex/Preprocessor.h
+++ clang/include/clang/Lex/Preprocessor.h
@@ -1115,6 +1115,28 @@
return nullptr;
}
+ // Returns MacroInfo for the latest definition of II within a module, nullptr
+ // if this macro was never defined in a module. The return MacroInfo may refer
+ // to definition that was overridden locally.
+ MacroInfo *getMacroInfoFromModules(const IdentifierInfo *II) {
+ if (!II->hasMacroDefinition())
+ return nullptr;
+ MacroState &S = CurSubmoduleState->Macros[II];
+ auto ActiveModuleMacros = S.getActiveModuleMacros(*this, II);
+ for (auto MM = ActiveModuleMacros.rbegin(); MM != ActiveModuleMacros.rend();
+ MM++) {
+ if (auto *MI = (*MM)->getMacroInfo())
+ return MI;
+ }
+ auto OverriddenMacros = S.getOverriddenMacros();
+ for (auto MM = OverriddenMacros.rbegin(); MM != OverriddenMacros.rend();
+ MM++) {
+ if (auto *MI = (*MM)->getMacroInfo())
+ return MI;
+ }
+ return nullptr;
+ }
+
/// Given an identifier, return the latest non-imported macro
/// directive for that identifier.
///
Index: clang-tools-extra/clangd/unittests/TestTU.h
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -65,6 +65,10 @@
// Simulate a header guard of the header (using an #import directive).
bool ImplicitHeaderGuard = true;
+ // Whether to use overlay the TestFS over the real filesystem. This is
+ // required for use of modules.
+ bool OverlayRealFileSystem = false;
+
// By default, build() will report Error diagnostics as GTest errors.
// Suppress this behavior by adding an 'error-ok' comment to the code.
ParsedAST build() const;
Index: clang-tools-extra/clangd/unittests/TestTU.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.cpp
+++ clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -54,7 +54,14 @@
Inputs.CompileCommand.Filename = FullFilename;
Inputs.CompileCommand.Directory = testRoot();
Inputs.Contents = Code;
- Inputs.FS = buildTestFS(Files);
+ if (OverlayRealFileSystem) {
+ IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFileSystem =
+ new llvm::vfs::OverlayFileSystem(llvm::vfs::getRealFileSystem());
+ OverlayFileSystem->pushOverlay(buildTestFS(Files));
+ Inputs.FS = OverlayFileSystem;
+ } else {
+ Inputs.FS = buildTestFS(Files);
+ }
Inputs.Opts = ParseOptions();
Inputs.Opts.BuildRecoveryAST = true;
Inputs.Opts.PreserveRecoveryASTType = true;
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1490,6 +1490,29 @@
EXPECT_THAT(Symbols, Contains(QName("operator delete")));
}
+TEST_F(SymbolCollectorTest, UndefOfModuleMacro) {
+ auto TU = TestTU::withCode(R"cpp(#include "/bar.h")cpp");
+ TU.AdditionalFiles["/bar.h"] = R"cpp(
+ #include "/foo.h"
+ #undef X
+ )cpp";
+ TU.AdditionalFiles["/foo.h"] = "#define X 1";
+ TU.AdditionalFiles["/module.map"] = R"cpp(
+ module foo {
+ header "/foo.h"
+ export *
+ }
+ )cpp";
+ TU.ExtraArgs.push_back("-fmodules");
+ TU.ExtraArgs.push_back("-fmodule-map-file=/module.map");
+ TU.OverlayRealFileSystem = true;
+
+ TU.build();
+ EXPECT_THAT(
+ TU.headerSymbols(),
+ Contains(AllOf(QName("X"), DeclURI(URI::create("/foo.h").toString()))));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
