Modules: Add LangOptions::CacheGeneratedPCH

[dexonsmith]

Summary

Add an option to cache the generated PCH in the ModuleCache when
emitting it. This protects clients that build PCHs and read them in the
same process, allowing them to avoid race conditions between parallel
jobs the same way that Clang's implicit module build system does.

[jordan_rose]

This commit by itself doesn't change any behavior, right?

clang/lib/Frontend/FrontendActions.cpp

115

What's the + for?

184

Why is this the condition, as opposed to just "do this for all modules, don't do it for PCHs"? And doesn't BuildingImplicitModule imply isCompilingModule()? (Note that BuildingImplicitModule probably isn't the same as the original condition ImplicitModules.)

[dexonsmith]

In D59176#1424864, @jordan_rose wrote:

This commit by itself doesn't change any behavior, right?

Correct, it just exposes an option.

clang/lib/Frontend/FrontendActions.cpp

115

Heh, I was trying to figure out those +s as well until I added this without one and it didn't compile. The problem is that bitfields can't be forwarded through the llvm::make_unique variadic. The + creates a local temporary with the same value.

184

(Note that BuildingImplicitModule probably isn't the same as the original condition ImplicitModules.)

Nice catch; I ported the logic from ASTWriter incorrectly. I'll fix this.

Why is this the condition, as opposed to just "do this for all modules, don't do it for PCHs"? And doesn't BuildingImplicitModule imply isCompilingModule()?

I was cargo-culting the logic for how PCHGenerator::HandleTranslationUnit sets the Module parameter to ASTWriter::WriteAST. I think that if I fix the above to match the original condition I'll need to check isCompilingModule()... but maybe BuildingImplicitModule is already &&-ing these together? I'll check.

[dexonsmith]

Updated the constructor call to PCHGenerator in GenerateModuleAction::CreateASTConsumer to use BuildingImplicitModule on its own. Checking where it's set (only in compileModuleImpl), it's exactly the condition we want here.

[dexonsmith]

clang/lib/Frontend/FrontendActions.cpp

184

Updated the patch to just use BuildingImplicitModule. The previous condition in PCHGenerator::HandleTranslationUnit seems to have been equivalent.

• Previously in PCHGenerator::HandleTranslationUnit, WritingModule would be non-null only if we're currently building a module, as opposed to writing out a PCH. The logic to write to the in-memory cache also checked if LangOptions::ImplicitModules was set.
• Now in GenerateModuleAction::CreateASTConsumer we check BuildingImplicitModule which is set in compileModuleImpl before executing the action.

I'm choosing this because it better matches the code around.

[dexonsmith]

Committed in r355950. Thanks for the review.

clang/include/clang/Basic/LangOptions.def

 LANGOPT(MathErrno         , 1, 1, "errno in math functions")
 BENIGN_LANGOPT(HeinousExtensions , 1, 0, "extensions that we really don't like and may be ripped out at any time")
 LANGOPT(Modules           , 1, 0, "modules extension to C")
 COMPATIBLE_LANGOPT(ModulesTS  , 1, 0, "C++ Modules TS")
 BENIGN_ENUM_LANGOPT(CompilingModule, CompilingModuleKind, 2, CMK_None,
                     "compiling a module interface")
 BENIGN_LANGOPT(CompilingPCH, 1, 0, "building a pch")
 BENIGN_LANGOPT(BuildingPCHWithObjectFile, 1, 0, "building a pch which has a corresponding object file")
+BENIGN_LANGOPT(CacheGeneratedPCH, 1, 0, "cache generated PCH files in memory")
 COMPATIBLE_LANGOPT(ModulesDeclUse    , 1, 0, "require declaration of module uses")
 BENIGN_LANGOPT(ModulesSearchAll  , 1, 1, "searching even non-imported modules to find unresolved references")
 COMPATIBLE_LANGOPT(ModulesStrictDeclUse, 1, 0, "requiring declaration of module uses and all headers to be in modules")
 BENIGN_LANGOPT(ModulesErrorRecovery, 1, 1, "automatically importing modules as needed when performing error recovery")
 BENIGN_LANGOPT(ImplicitModules, 1, 1, "building modules that are not specified via -fmodule-file")
 COMPATIBLE_LANGOPT(ModulesLocalVisibility, 1, 0, "local submodule visibility")
 COMPATIBLE_LANGOPT(Optimize          , 1, 0, "__OPTIMIZE__ predefined macro")
 COMPATIBLE_LANGOPT(OptimizeSize      , 1, 0, "__OPTIMIZE_SIZE__ predefined macro")

clang/include/clang/Serialization/ASTWriter.h

public:
   /// \param isysroot if non-empty, write a relocatable file whose headers
   /// are relative to the given system root. If we're writing a module, its
   /// build directory will be used in preference to this if both are available.
   ///
   /// \return the module signature, which eventually will be a hash of
   /// the module but currently is merely a random 32-bit number.
   ASTFileSignature WriteAST(Sema &SemaRef, const std::string &OutputFile,
                             Module *WritingModule, StringRef isysroot,
-                            bool hasErrors = false);
+                            bool hasErrors = false,
+                            bool ShouldCacheASTInMemory = false);
 
   /// Emit a token.
   void AddToken(const Token &Tok, RecordDataImpl &Record);
 
   /// Emit a source location.
   void AddSourceLocation(SourceLocation Loc, RecordDataImpl &Record);
 
   /// Emit a source range.
class PCHGenerator : public SemaConsumer {
   const Preprocessor &PP;
   std::string OutputFile;
   std::string isysroot;
   Sema *SemaPtr;
   std::shared_ptr<PCHBuffer> Buffer;
   llvm::BitstreamWriter Stream;
   ASTWriter Writer;
   bool AllowASTWithErrors;
+  bool ShouldCacheASTInMemory;
 
 protected:
   ASTWriter &getWriter() { return Writer; }
   const ASTWriter &getWriter() const { return Writer; }
   SmallVectorImpl<char> &getPCH() const { return Buffer->Data; }
 
 public:
   PCHGenerator(const Preprocessor &PP, InMemoryModuleCache &ModuleCache,
                StringRef OutputFile, StringRef isysroot,
                std::shared_ptr<PCHBuffer> Buffer,
                ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
-               bool AllowASTWithErrors = false, bool IncludeTimestamps = true);
+               bool AllowASTWithErrors = false, bool IncludeTimestamps = true,
+               bool ShouldCacheASTInMemory = false);
   ~PCHGenerator() override;
 
   void InitializeSema(Sema &S) override { SemaPtr = &S; }
   void HandleTranslationUnit(ASTContext &Ctx) override;
   ASTMutationListener *GetASTMutationListener() override;
   ASTDeserializationListener *GetASTDeserializationListener() override;
   bool hasEmittedPCH() const { return Buffer->IsComplete; }
 };

clang/lib/Frontend/FrontendActions.cpp

GeneratePCHAction::CreateASTConsumer(CompilerInstance &CI, StringRef InFile) {
 
   const auto &FrontendOpts = CI.getFrontendOpts();
   auto Buffer = std::make_shared<PCHBuffer>();
   std::vector<std::unique_ptr<ASTConsumer>> Consumers;
   Consumers.push_back(llvm::make_unique<PCHGenerator>(
       CI.getPreprocessor(), CI.getModuleCache(), OutputFile, Sysroot, Buffer,
       FrontendOpts.ModuleFileExtensions,
       CI.getPreprocessorOpts().AllowPCHWithCompilerErrors,
-      FrontendOpts.IncludeTimestamps));
+      FrontendOpts.IncludeTimestamps, +CI.getLangOpts().CacheGeneratedPCH));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
       CI, InFile, OutputFile, std::move(OS), Buffer));
 
   return llvm::make_unique<MultiplexConsumer>(std::move(Consumers));
 }
 
 bool GeneratePCHAction::ComputeASTConsumerArguments(CompilerInstance &CI,
                                                     std::string &Sysroot) {
GenerateModuleAction::CreateASTConsumer(CompilerInstance &CI,
   auto Buffer = std::make_shared<PCHBuffer>();
   std::vector<std::unique_ptr<ASTConsumer>> Consumers;
 
   Consumers.push_back(llvm::make_unique<PCHGenerator>(
       CI.getPreprocessor(), CI.getModuleCache(), OutputFile, Sysroot, Buffer,
       CI.getFrontendOpts().ModuleFileExtensions,
       /*AllowASTWithErrors=*/false,
       /*IncludeTimestamps=*/
+      +CI.getFrontendOpts().BuildingImplicitModule,
+      /*ShouldCacheASTInMemory=*/
       +CI.getFrontendOpts().BuildingImplicitModule));
   Consumers.push_back(CI.getPCHContainerWriter().CreatePCHContainerGenerator(
       CI, InFile, OutputFile, std::move(OS), Buffer));
   return llvm::make_unique<MultiplexConsumer>(std::move(Consumers));
 }
 
 bool GenerateModuleFromModuleMapAction::BeginSourceFileAction(
     CompilerInstance &CI) {
   if (!CI.getLangOpts().Modules) {
     CI.getDiagnostics().Report(diag::err_module_build_requires_fmodules);
     return false;
   }

clang/lib/Serialization/ASTWriter.cpp

 
 time_t ASTWriter::getTimestampForOutput(const FileEntry *E) const {
   return IncludeTimestamps ? E->getModificationTime() : 0;
 }
 
 ASTFileSignature ASTWriter::WriteAST(Sema &SemaRef,
                                      const std::string &OutputFile,
                                      Module *WritingModule, StringRef isysroot,
-                                     bool hasErrors) {
+                                     bool hasErrors,
+                                     bool ShouldCacheASTInMemory) {
   WritingAST = true;
 
   ASTHasCompilerErrors = hasErrors;
 
   // Emit the file header.
   Stream.Emit((unsigned)'C', 8);
   Stream.Emit((unsigned)'P', 8);
   Stream.Emit((unsigned)'C', 8);
   Stream.Emit((unsigned)'H', 8);
 
   WriteBlockInfoBlock();
 
   Context = &SemaRef.Context;
   PP = &SemaRef.PP;
   this->WritingModule = WritingModule;
   ASTFileSignature Signature =
       WriteASTCore(SemaRef, isysroot, OutputFile, WritingModule);
   Context = nullptr;
   PP = nullptr;
   this->WritingModule = nullptr;
   this->BaseDirectory.clear();
 
   WritingAST = false;
-  if (SemaRef.Context.getLangOpts().ImplicitModules && WritingModule) {
+  if (ShouldCacheASTInMemory) {
     // Construct MemoryBuffer and update buffer manager.
     ModuleCache.addBuiltPCM(OutputFile,
                             llvm::MemoryBuffer::getMemBufferCopy(
                                 StringRef(Buffer.begin(), Buffer.size())));
   }
   return Signature;
 }
 

clang/lib/Serialization/GeneratePCH.cpp

 #include "llvm/Bitcode/BitstreamWriter.h"
 
 using namespace clang;
 
 PCHGenerator::PCHGenerator(
     const Preprocessor &PP, InMemoryModuleCache &ModuleCache,
     StringRef OutputFile, StringRef isysroot, std::shared_ptr<PCHBuffer> Buffer,
     ArrayRef<std::shared_ptr<ModuleFileExtension>> Extensions,
-    bool AllowASTWithErrors, bool IncludeTimestamps)
+    bool AllowASTWithErrors, bool IncludeTimestamps,
+    bool ShouldCacheASTInMemory)
     : PP(PP), OutputFile(OutputFile), isysroot(isysroot.str()),
       SemaPtr(nullptr), Buffer(std::move(Buffer)), Stream(this->Buffer->Data),
       Writer(Stream, this->Buffer->Data, ModuleCache, Extensions,
              IncludeTimestamps),
-      AllowASTWithErrors(AllowASTWithErrors) {
+      AllowASTWithErrors(AllowASTWithErrors),
+      ShouldCacheASTInMemory(ShouldCacheASTInMemory) {
   this->Buffer->IsComplete = false;
 }
 
 PCHGenerator::~PCHGenerator() {
 }
 
 void PCHGenerator::HandleTranslationUnit(ASTContext &Ctx) {
   // Don't create a PCH if there were fatal failures during module loading.
void PCHGenerator::HandleTranslationUnit(ASTContext &Ctx) {
   }
 
   // Emit the PCH file to the Buffer.
   assert(SemaPtr && "No Sema?");
   Buffer->Signature =
       Writer.WriteAST(*SemaPtr, OutputFile, Module, isysroot,
                       // For serialization we are lenient if the errors were
                       // only warn-as-error kind.
-                      PP.getDiagnostics().hasUncompilableErrorOccurred());
+                      PP.getDiagnostics().hasUncompilableErrorOccurred(),
+                      ShouldCacheASTInMemory);
 
   Buffer->IsComplete = true;
 }
 
 ASTMutationListener *PCHGenerator::GetASTMutationListener() {
   return &Writer;
 }
 
 ASTDeserializationListener *PCHGenerator::GetASTDeserializationListener() {
   return &Writer;
 }

clang/unittests/Frontend/FrontendActionTest.cpp

 //===- unittests/Frontend/FrontendActionTest.cpp - FrontendAction tests ---===//
 //
 // 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 "clang/Frontend/FrontendAction.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/RecursiveASTVisitor.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/CompilerInvocation.h"
-#include "clang/Frontend/FrontendAction.h"
 #include "clang/Frontend/FrontendActions.h"
 #include "clang/Lex/Preprocessor.h"
 #include "clang/Lex/PreprocessorOptions.h"
 #include "clang/Sema/Sema.h"
+#include "clang/Serialization/InMemoryModuleCache.h"
 #include "llvm/ADT/Triple.h"
 #include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/ToolOutputFile.h"
 #include "gtest/gtest.h"
 
 using namespace llvm;
 using namespace clang;
 
 namespace {
 
 class TestASTFrontendAction : public ASTFrontendAction {
TEST(ASTFrontendAction, ExternalSemaSource) {
   SyntaxOnlyAction TestAction;
   ASSERT_TRUE(Compiler.ExecuteAction(TestAction));
   // There should be one error correcting to 'moo' and a note attached to it.
   EXPECT_EQ("use of undeclared identifier 'foo'; did you mean 'moo'?",
             TDC->Error.str().str());
   EXPECT_EQ("This is a note", TDC->Note.str().str());
 }
 
+TEST(GeneratePCHFrontendAction, CacheGeneratedPCH) {
+  // Create a temporary file for writing out the PCH that will be cleaned up.
+  int PCHFD;
+  llvm::SmallString<128> PCHFilename;
+  ASSERT_FALSE(
+      llvm::sys::fs::createTemporaryFile("test.h", "pch", PCHFD, PCHFilename));
+  llvm::ToolOutputFile PCHFile(PCHFilename, PCHFD);
+
+  for (bool ShouldCache : {false, true}) {
+    auto Invocation = std::make_shared<CompilerInvocation>();
+    Invocation->getLangOpts()->CacheGeneratedPCH = ShouldCache;
+    Invocation->getPreprocessorOpts().addRemappedFile(
+        "test.h",
+        MemoryBuffer::getMemBuffer("int foo(void) { return 1; }\n").release());
+    Invocation->getFrontendOpts().Inputs.push_back(
+        FrontendInputFile("test.h", InputKind::C));
+    Invocation->getFrontendOpts().OutputFile = StringRef(PCHFilename);
+    Invocation->getFrontendOpts().ProgramAction = frontend::GeneratePCH;
+    Invocation->getTargetOpts().Triple = "x86_64-apple-darwin19.0.0";
+    CompilerInstance Compiler;
+    Compiler.setInvocation(std::move(Invocation));
+    Compiler.createDiagnostics();
+
+    GeneratePCHAction TestAction;
+    ASSERT_TRUE(Compiler.ExecuteAction(TestAction));
+
+    // Check whether the PCH was cached.
+    if (ShouldCache)
+      EXPECT_EQ(InMemoryModuleCache::Final,
+                Compiler.getModuleCache().getPCMState(PCHFilename));
+    else
+      EXPECT_EQ(InMemoryModuleCache::Unknown,
+                Compiler.getModuleCache().getPCMState(PCHFilename));
+  }
+}
+
 } // anonymous namespace