[clang-tools-extra] e2d61ae - Correctly set CompilingPCH in PrecompilePreambleAction.

[Adam Czachorowski via cfe-commits]

Author: Adam Czachorowski
Date: 2020-08-10T17:49:23+02:00
New Revision: e2d61ae5733316a14783b36c84b8e7681b0e3d59

URL: https://github.com/llvm/llvm-project/commit/e2d61ae5733316a14783b36c84b8e7681b0e3d59
DIFF: https://github.com/llvm/llvm-project/commit/e2d61ae5733316a14783b36c84b8e7681b0e3d59.diff

LOG: Correctly set CompilingPCH in PrecompilePreambleAction.

This fixes a crash bug in clangd when used with modules. ASTWriter would
end up writing references to submodules into the PCH file, but upon
reading the submodules would not exists and
HeaderFileInfoTrait::ReadData would crash.

Differential Revision: https://reviews.llvm.org/D85532

Added: 
    clang-tools-extra/clangd/unittests/ModulesTests.cpp

Modified: 
    clang-tools-extra/clangd/unittests/CMakeLists.txt
    clang/lib/Frontend/PrecompiledPreamble.cpp
    clang/unittests/Frontend/ASTUnitTest.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt
index c25e2b7f8103..966fa9630852 100644
--- a/clang-tools-extra/clangd/unittests/CMakeLists.txt
+++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt
@@ -63,6 +63,7 @@ add_unittest(ClangdUnitTests ClangdTests
   IndexTests.cpp
   JSONTransportTests.cpp
   LSPClient.cpp
+  ModulesTests.cpp
   ParsedASTTests.cpp
   PathMappingTests.cpp
   PreambleTests.cpp

diff  --git a/clang-tools-extra/clangd/unittests/ModulesTests.cpp b/clang-tools-extra/clangd/unittests/ModulesTests.cpp
new file mode 100644
index 000000000000..0098a15f64bb
--- /dev/null
+++ b/clang-tools-extra/clangd/unittests/ModulesTests.cpp
@@ -0,0 +1,44 @@
+//===-- ModulesTests.cpp  ---------------------------------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "Annotations.h"
+#include "TestFS.h"
+#include "TestTU.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+
+#include <memory>
+#include <string>
+
+namespace clang {
+namespace clangd {
+namespace {
+
+TEST(Modules, TextualIncludeInPreamble) {
+  TestTU TU = TestTU::withCode(R"cpp(
+    #include "Textual.h"
+
+    void foo() {}
+)cpp");
+  TU.ExtraArgs.push_back("-fmodule-name=M");
+  TU.ExtraArgs.push_back("-fmodule-map-file=m.modulemap");
+  TU.AdditionalFiles["Textual.h"] = "void foo();";
+  TU.AdditionalFiles["m.modulemap"] = R"modulemap(
+    module M {
+      module Textual {
+        textual header "Textual.h"
+      }
+    }
+)modulemap";
+  // Test that we do not crash.
+  TU.index();
+}
+
+} // namespace
+} // namespace clangd
+} // namespace clang

diff  --git a/clang/lib/Frontend/PrecompiledPreamble.cpp b/clang/lib/Frontend/PrecompiledPreamble.cpp
index 6cdfc595dcae..3fb61f37ae2b 100644
--- a/clang/lib/Frontend/PrecompiledPreamble.cpp
+++ b/clang/lib/Frontend/PrecompiledPreamble.cpp
@@ -208,6 +208,11 @@ class PrecompilePreambleAction : public ASTFrontendAction {
     Callbacks.AfterPCHEmitted(Writer);
   }
 
+  bool BeginSourceFileAction(CompilerInstance &CI) override {
+    assert(CI.getLangOpts().CompilingPCH);
+    return ASTFrontendAction::BeginSourceFileAction(CI);
+  }
+
   bool shouldEraseOutputFiles() override { return !hasEmittedPreamblePCH(); }
   bool hasCodeCompletionSupport() const override { return false; }
   bool hasASTFileSupport() const override { return false; }
@@ -396,6 +401,8 @@ llvm::ErrorOr<PrecompiledPreamble> PrecompiledPreamble::Build(
   auto PreambleDepCollector = std::make_shared<PreambleDependencyCollector>();
   Clang->addDependencyCollector(PreambleDepCollector);
 
+  Clang->getLangOpts().CompilingPCH = true;
+
   // Remap the main source file to the preamble buffer.
   StringRef MainFilePath = FrontendOpts.Inputs[0].getFile();
   auto PreambleInputBuffer = llvm::MemoryBuffer::getMemBufferCopy(

diff  --git a/clang/unittests/Frontend/ASTUnitTest.cpp b/clang/unittests/Frontend/ASTUnitTest.cpp
index bdbc462e2bcb..c5b84e74cee9 100644
--- a/clang/unittests/Frontend/ASTUnitTest.cpp
+++ b/clang/unittests/Frontend/ASTUnitTest.cpp
@@ -13,6 +13,7 @@
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
 #include "clang/Frontend/PCHContainerOperations.h"
+#include "clang/Lex/HeaderSearch.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ToolOutputFile.h"
@@ -111,4 +112,42 @@ TEST_F(ASTUnitTest, GetBufferForFileMemoryMapping) {
             llvm::MemoryBuffer::MemoryBuffer_MMap);
 }
 
+TEST_F(ASTUnitTest, ModuleTextualHeader) {
+  llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> InMemoryFs =
+      new llvm::vfs::InMemoryFileSystem();
+  InMemoryFs->addFile("test.cpp", 0, llvm::MemoryBuffer::getMemBuffer(R"cpp(
+      #include "Textual.h"
+      void foo() {}
+    )cpp"));
+  InMemoryFs->addFile("m.modulemap", 0, llvm::MemoryBuffer::getMemBuffer(R"cpp(
+      module M {
+        module Textual {
+          textual header "Textual.h"
+        }
+      }
+    )cpp"));
+  InMemoryFs->addFile("Textual.h", 0, llvm::MemoryBuffer::getMemBuffer(R"cpp(
+      void foo();
+    )cpp"));
+
+  const char *Args[] = {"clang", "test.cpp", "-fmodule-map-file=m.modulemap",
+                        "-fmodule-name=M"};
+  Diags = CompilerInstance::createDiagnostics(new DiagnosticOptions());
+  CInvok = createInvocationFromCommandLine(Args, Diags);
+  ASSERT_TRUE(CInvok);
+
+  FileManager *FileMgr = new FileManager(FileSystemOptions(), InMemoryFs);
+  PCHContainerOps = std::make_shared<PCHContainerOperations>();
+
+  auto AU = ASTUnit::LoadFromCompilerInvocation(
+      CInvok, PCHContainerOps, Diags, FileMgr, false, CaptureDiagsKind::None, 1,
+      TU_Complete, false, false, false);
+  ASSERT_TRUE(AU);
+  auto File = AU->getFileManager().getFileRef("Textual.h", false, false);
+  ASSERT_TRUE(bool(File));
+  // Verify that we do not crash here.
+  EXPECT_TRUE(AU->getPreprocessor().getHeaderSearchInfo().getExistingFileInfo(
+      &File->getFileEntry()));
+}
+
 } // anonymous namespace