[clang] 73f0772 - [clangd] Revert "[clangd] Fix crash-bug in preamble indexing when using modules."
Adam Czachorowski via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 13 08:10:54 PDT 2020
Author: Adam Czachorowski
Date: 2020-08-13T17:09:54+02:00
New Revision: 73f0772c0baf1c7cac2995341c11d83c4d7a37f4
URL: https://github.com/llvm/llvm-project/commit/73f0772c0baf1c7cac2995341c11d83c4d7a37f4
DIFF: https://github.com/llvm/llvm-project/commit/73f0772c0baf1c7cac2995341c11d83c4d7a37f4.diff
LOG: [clangd] Revert "[clangd] Fix crash-bug in preamble indexing when using modules."
This reverts commit 4061d9e42cff621462931ac7df9666806c77a237.
Tests are failing in some configuration, likely due to not cleaning up
module cache path before running the test.
Differential Revision: https://reviews.llvm.org/D85907
Added:
Modified:
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
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
index 48a6952b2610..70a8e6832d02 100644
--- a/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1610,31 +1610,6 @@ TEST_F(SymbolCollectorTest, MacrosInHeaders) {
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
diff --git a/clang-tools-extra/clangd/unittests/TestFS.h b/clang-tools-extra/clangd/unittests/TestFS.h
index 82de319d96cf..7972fe37c7fe 100644
--- a/clang-tools-extra/clangd/unittests/TestFS.h
+++ b/clang-tools-extra/clangd/unittests/TestFS.h
@@ -34,21 +34,12 @@ buildTestFS(llvm::StringMap<std::string> const &Files,
class MockFS : public ThreadsafeFS {
public:
IntrusiveRefCntPtr<llvm::vfs::FileSystem> viewImpl() const override {
- 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;
+ return buildTestFS(Files, Timestamps);
}
// 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.
diff --git a/clang-tools-extra/clangd/unittests/TestTU.cpp b/clang-tools-extra/clangd/unittests/TestTU.cpp
index 16305400307a..7d5c29f23393 100644
--- a/clang-tools-extra/clangd/unittests/TestTU.cpp
+++ b/clang-tools-extra/clangd/unittests/TestTU.cpp
@@ -54,8 +54,6 @@ ParseInputs TestTU::inputs(MockFS &FS) const {
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;
diff --git a/clang-tools-extra/clangd/unittests/TestTU.h b/clang-tools-extra/clangd/unittests/TestTU.h
index dd0cecc406ac..06abd64dc623 100644
--- a/clang-tools-extra/clangd/unittests/TestTU.h
+++ b/clang-tools-extra/clangd/unittests/TestTU.h
@@ -66,16 +66,6 @@ struct TestTU {
// 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;
diff --git a/clang/lib/Index/IndexingAction.cpp b/clang/lib/Index/IndexingAction.cpp
index 4986303cac47..e698c07133a9 100644
--- a/clang/lib/Index/IndexingAction.cpp
+++ b/clang/lib/Index/IndexingAction.cpp
@@ -165,20 +165,11 @@ static void indexTranslationUnit(ASTUnit &Unit, IndexingContext &IndexCtx) {
static void indexPreprocessorMacros(const Preprocessor &PP,
IndexDataConsumer &DataConsumer) {
for (const auto &M : PP.macros())
- 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;
-
+ if (MacroDirective *MD = M.second.getLatest())
DataConsumer.handleMacroOccurrence(
M.first, MD->getMacroInfo(),
static_cast<unsigned>(index::SymbolRole::Definition),
MD->getLocation());
- }
}
void index::indexASTUnit(ASTUnit &Unit, IndexDataConsumer &DataConsumer,
More information about the cfe-commits
mailing list