adamcz created this revision.
adamcz added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous.
Herald added a project: clang.
adamcz requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

This reverts commit 4061d9e42cff621462931ac7df9666806c77a237 
<https://reviews.llvm.org/rG4061d9e42cff621462931ac7df9666806c77a237>.
Tests are failing in some configuration, likely due to not cleaning up
module cache path before running the test.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D85907

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,20 +165,11 @@
 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,
Index: clang-tools-extra/clangd/unittests/TestTU.h
===================================================================
--- clang-tools-extra/clangd/unittests/TestTU.h
+++ clang-tools-extra/clangd/unittests/TestTU.h
@@ -66,16 +66,6 @@
   // 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
@@ -54,8 +54,6 @@
   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;
Index: clang-tools-extra/clangd/unittests/TestFS.h
===================================================================
--- clang-tools-extra/clangd/unittests/TestFS.h
+++ clang-tools-extra/clangd/unittests/TestFS.h
@@ -34,21 +34,12 @@
 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.
Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1610,31 +1610,6 @@
   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
  • [PATCH] D85907: [clangd]... Adam Czachorowski via Phabricator via cfe-commits

Reply via email to