[clang][cas] Fix issue when we emit caching-related diagnostic after source processing is done

Introduce `FrontendOptions.MayEmitDiagnosticsAfterProcessingSourceFiles` to indicate that
`CompilerInstance::ExecuteAction` should not "finish" the diagnostic client.
This is set for caching compilations.

rdar://108014441
(cherry picked from commit ff889ff)

Author: akyrtzi

clang/include/clang/Frontend/FrontendOptions.h

@@ -375,6 +375,10 @@ class FrontendOptions {
   /// caching of compilation outputs. This is used for testing purposes.
   unsigned DisableCachedCompileJobReplay : 1;
 
+  /// Keep the diagnostic client open for receiving diagnostics after the source
+  /// files have been processed.
+  unsigned MayEmitDiagnosticsAfterProcessingSourceFiles : 1;
+
   /// Output (and read) PCM files regardless of compiler errors.
   unsigned AllowPCMWithCompilerErrors : 1;
 
@@ -570,7 +574,9 @@ class FrontendOptions {
         ASTDumpLookups(false), BuildingImplicitModule(false),
         BuildingImplicitModuleUsesLock(true), ModulesEmbedAllFiles(false),
         IncludeTimestamps(true), UseTemporary(true), CacheCompileJob(false),
-        DisableCachedCompileJobReplay(false), AllowPCMWithCompilerErrors(false),
+        DisableCachedCompileJobReplay(false),
+        MayEmitDiagnosticsAfterProcessingSourceFiles(false),
+        AllowPCMWithCompilerErrors(false),
         ModulesShareFileManager(true), TimeTraceGranularity(500) {}
 
   /// getInputKindForExtension - Return the appropriate input kind for a file

clang/lib/Frontend/CompilerInstance.cpp

@@ -1091,8 +1091,10 @@ bool CompilerInstance::ExecuteAction(FrontendAction &Act) {
   noteBottomOfStack();
 
   auto FinishDiagnosticClient = llvm::make_scope_exit([&]() {
-    // Notify the diagnostic client that all files were processed.
-    getDiagnosticClient().finish();
+    if (!getFrontendOpts().MayEmitDiagnosticsAfterProcessingSourceFiles) {
+      // Notify the diagnostic client that all files were processed.
+      getDiagnosticClient().finish();
+    }
   });
 
   raw_ostream &OS = getVerboseOutputStream();
@@ -1292,6 +1294,7 @@ compileModuleImpl(CompilerInstance &ImportingInstance, SourceLocation ImportLoc,
   // Force implicitly-built modules to hash the content of the module file.
   HSOpts.ModulesHashContent = true;
   FrontendOpts.Inputs = {Input};
+  FrontendOpts.MayEmitDiagnosticsAfterProcessingSourceFiles = false;
 
   // Don't free the remapped file buffers; they are owned by our caller.
   PPOpts.RetainRemappedFileBuffers = true;

clang/test/CAS/test-for-deterministic-outputs.c

@@ -19,19 +19,20 @@
 // CACHE-SKIPPED: remark: compile job cache skipped
 
 // RUN: env LLVM_CACHE_CAS_PATH=%t/cas %clang-cache \
-// RUN:   %clang -target x86_64-apple-macos11 -c %s -o %t/t.o -Rcompile-job-cache -Wreproducible-caching 2> %t/out.txt
-// RUN: FileCheck %s --check-prefix=CACHE-WARN --input-file=%t/out.txt
+// RUN:   %clang -target x86_64-apple-macos11 -c %s -o %t/t.o -Rcompile-job-cache -Wreproducible-caching -serialize-diagnostics %t/t.dia 2> %t/out.txt
+// RUN: FileCheck %s --check-prefix=CACHE-WARN --input-file=%t/out.txt -DREMARK=remark
+// RUN: c-index-test -read-diagnostics %t/t.dia 2>&1 | FileCheck %s --check-prefix=CACHE-WARN -DREMARK=warning
 
 /// Check still a cache miss.
 // RUN: env LLVM_CACHE_CAS_PATH=%t/cas %clang-cache \
 // RUN:   %clang -target x86_64-apple-macos11 -c %s -o %t/t.o -Rcompile-job-cache -Wreproducible-caching 2> %t/out.txt
-// RUN: FileCheck %s --check-prefix=CACHE-WARN --input-file=%t/out.txt
+// RUN: FileCheck %s --check-prefix=CACHE-WARN --input-file=%t/out.txt -DREMARK=remark
 
-// CACHE-WARN: remark: compile job cache miss
+// CACHE-WARN: [[REMARK]]: compile job cache miss
 // CACHE-WARN: warning: encountered non-reproducible token, caching will be skipped
 // CACHE-WARN: warning: encountered non-reproducible token, caching will be skipped
 // CACHE-WARN: warning: encountered non-reproducible token, caching will be skipped
-// CACHE-WARN: remark: compile job cache skipped
+// CACHE-WARN: [[REMARK]]: compile job cache skipped
 
 /// Check -Werror doesn't actually error when we use the launcher.
 // RUN: env LLVM_CACHE_CAS_PATH=%t/cas %clang-cache \

clang/tools/driver/cc1_main.cpp

@@ -1218,15 +1218,17 @@ int cc1_main(ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr) {
 
   DiagsBuffer->FlushDiagnostics(Clang->getDiagnostics());
 
-  auto FinishDiagnosticClient = llvm::make_scope_exit([&]() {
+  auto FinishDiagnosticClient = [&]() {
     // Notify the diagnostic client that all files were processed.
     Clang->getDiagnosticClient().finish();
 
     // Our error handler depends on the Diagnostics object, which we're
     // potentially about to delete. Uninstall the handler now so that any
     // later errors use the default handling behavior instead.
     llvm::remove_fatal_error_handler();
-  });
+  };
+  auto FinishDiagnosticClientScope =
+      llvm::make_scope_exit([&]() { FinishDiagnosticClient(); });
 
   if (!Success)
     return 1;
@@ -1239,8 +1241,7 @@ int cc1_main(ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr) {
   if (Optional<int> Status = JobCache.tryReplayCachedResult(*Clang))
     return *Status; // FIXME: Should write out timers before exiting!
 
-  // ExecuteAction takes responsibility.
-  FinishDiagnosticClient.release();
+  Clang->getFrontendOpts().MayEmitDiagnosticsAfterProcessingSourceFiles = true;
 
   // Execute the frontend actions.
   {
@@ -1279,6 +1280,10 @@ int cc1_main(ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr) {
     }
   }
 
+  // Call this before the Clang pointer is moved below.
+  FinishDiagnosticClient();
+  FinishDiagnosticClientScope.release();
+
   // When running with -disable-free, don't do any destruction or shutdown.
   if (Clang->getFrontendOpts().DisableFree) {
     llvm::BuryPointer(std::move(Clang));

clang/tools/driver/driver.cpp

@@ -372,8 +372,6 @@ static int ExecuteCC1Tool(SmallVectorImpl<const char *> &ArgV) {
       int RC = cc1_main(ArrayRef(ArgV).slice(1), ArgV[0], GetExecutablePathVP);
       if (RC != 0)
         return RC;
-      // FIXME: cc1_main should possibly clean up global state itself.
-      llvm::remove_fatal_error_handler();
     }
     return cc1_main(makeArrayRef(ArgV).slice(1), ArgV[0], GetExecutablePathVP);
   }