This revision was not accepted when it landed; it landed in state "Needs
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rG53b9199a5cdb: [clangd] Fix crash-bug in preamble indexing
when using modules. (authored by adamcz).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D85923/new/
https://reviews.llvm.org/D85923
Files:
clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
clang-tools-extra/clangd/unittests/TestFS.h
clang-tools-extra/clangd/unittests/TestTU.cpp
clang-tools-extra/clangd/unittests/TestTU.h
clang/lib/Index/IndexingAction.cpp
Index: clang/lib/Index/IndexingAction.cpp
===================================================================
--- clang/lib/Index/IndexingAction.cpp
+++ clang/lib/Index/IndexingAction.cpp
@@ -165,11 +165,20 @@
static void indexPreprocessorMacros(const Preprocessor &PP,
IndexDataConsumer &DataConsumer) {
for (const auto &M : PP.macros())
- if (MacroDirective *MD = M.second.getLatest())
+ if (MacroDirective *MD = M.second.getLatest()) {
+ auto *MI = MD->getMacroInfo();
+ // When using modules, it may happen that we find #undef of a macro that
+ // was defined in another module. In such case, MI may be nullptr, since
+ // we only look for macro definitions in the current TU. In that case,
+ // there is nothing to index.
+ if (!MI)
+ continue;
+
DataConsumer.handleMacroOccurrence(
M.first, MD->getMacroInfo(),
static_cast<unsigned>(index::SymbolRole::Definition),
MD->getLocation());
+ }
}
void index::indexASTUnit(ASTUnit &Unit, IndexDataConsumer &DataConsumer,
Index: clang-tools-extra/clangd/unittests/TestTU.h
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -66,6 +66,16 @@
// 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 implicit modules.where the module file is written to
+ // disk and later read back.
+ // FIXME: Change the way reading/writing modules work to allow us to keep them
+ // in memory across multiple clang invocations, at least in tests, to
+ // eliminate the need for real file system here.
+ // Please avoid using this for things other than implicit modules. The plan is
+ // to eliminate this option some day.
+ bool OverlayRealFileSystemForModules = 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
@@ -16,6 +16,7 @@
#include "clang/Basic/Diagnostic.h"
#include "clang/Frontend/CompilerInvocation.h"
#include "clang/Frontend/Utils.h"
+#include "llvm/ADT/ScopeExit.h"
namespace clang {
namespace clangd {
@@ -54,6 +55,8 @@
Inputs.CompileCommand.Filename = FullFilename;
Inputs.CompileCommand.Directory = testRoot();
Inputs.Contents = Code;
+ if (OverlayRealFileSystemForModules)
+ FS.OverlayRealFileSystemForModules = true;
Inputs.TFS = &FS;
Inputs.Opts = ParseOptions();
Inputs.Opts.BuildRecoveryAST = true;
@@ -66,12 +69,31 @@
return Inputs;
}
+void initializeModuleCache(CompilerInvocation &CI) {
+ llvm::SmallString<128> ModuleCachePath;
+ ASSERT_FALSE(
+ llvm::sys::fs::createUniqueDirectory("module-cache", ModuleCachePath));
+ CI.getHeaderSearchOpts().ModuleCachePath = ModuleCachePath.c_str();
+ llvm::errs() << "MC: " << ModuleCachePath << "\n";
+ llvm::errs().flush();
+}
+
+void deleteModuleCache(const std::string ModuleCachePath) {
+ if (!ModuleCachePath.empty()) {
+ ASSERT_FALSE(llvm::sys::fs::remove_directories(ModuleCachePath));
+ }
+}
+
std::shared_ptr<const PreambleData> TestTU::preamble() const {
MockFS FS;
auto Inputs = inputs(FS);
IgnoreDiagnostics Diags;
auto CI = buildCompilerInvocation(Inputs, Diags);
assert(CI && "Failed to build compilation invocation.");
+ if (OverlayRealFileSystemForModules)
+ initializeModuleCache(*CI);
+ auto ModuleCacheDeleter = llvm::make_scope_exit(
+ std::bind(deleteModuleCache, CI->getHeaderSearchOpts().ModuleCachePath));
return clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs,
/*StoreInMemory=*/true,
/*PreambleCallback=*/nullptr);
@@ -83,6 +105,11 @@
StoreDiags Diags;
auto CI = buildCompilerInvocation(Inputs, Diags);
assert(CI && "Failed to build compilation invocation.");
+ if (OverlayRealFileSystemForModules)
+ initializeModuleCache(*CI);
+ auto ModuleCacheDeleter = llvm::make_scope_exit(
+ std::bind(deleteModuleCache, CI->getHeaderSearchOpts().ModuleCachePath));
+
auto Preamble = clang::clangd::buildPreamble(testPath(Filename), *CI, Inputs,
/*StoreInMemory=*/true,
/*PreambleCallback=*/nullptr);
Index: clang-tools-extra/clangd/unittests/TestFS.h
===================================================================
--- clang-tools-extra/clangd/unittests/TestFS.h
+++ clang-tools-extra/clangd/unittests/TestFS.h
@@ -34,12 +34,21 @@
class MockFS : public ThreadsafeFS {
public:
IntrusiveRefCntPtr<llvm::vfs::FileSystem> viewImpl() const override {
- return buildTestFS(Files, Timestamps);
+ auto MemFS = buildTestFS(Files, Timestamps);
+ if (!OverlayRealFileSystemForModules)
+ return MemFS;
+ llvm::IntrusiveRefCntPtr<llvm::vfs::OverlayFileSystem> OverlayFileSystem =
+ new llvm::vfs::OverlayFileSystem(llvm::vfs::getRealFileSystem());
+ OverlayFileSystem->pushOverlay(MemFS);
+ return OverlayFileSystem;
}
// If relative paths are used, they are resolved with testPath().
llvm::StringMap<std::string> Files;
llvm::StringMap<time_t> Timestamps;
+ // If true, real file system will be used as fallback for the in-memory one.
+ // This is useful for testing module support.
+ bool OverlayRealFileSystemForModules = false;
};
// A Compilation database that returns a fixed set of compile flags.
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1623,6 +1623,31 @@
EXPECT_THAT(Symbols,
UnorderedElementsAre(AllOf(QName("X"), ForCodeCompletion(true))));
}
+
+// Regression test for a crash-bug we used to have.
+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=" + testPath("module.map"));
+ TU.OverlayRealFileSystemForModules = true;
+
+ TU.build();
+ // We mostly care about not crashing, but verify that we didn't insert garbage
+ // about X too.
+ EXPECT_THAT(TU.headerSymbols(), Not(Contains(QName("X"))));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
