[clang] Make serialized diagnostics more reliable

clang/include/clang/Basic/Diagnostic.h

@@ -1776,8 +1776,12 @@ class DiagnosticConsumer {
   /// BeginSourceFile() are inaccessible.
   virtual void EndSourceFile() {}
 
-  /// Callback to inform the diagnostic client that processing of all
-  /// source files has ended.
+  /// Callback to inform the diagnostic client that processing of all source
+  /// files has ended, and that no more diagnostics will be emitted.
+  ///
+  /// This is only called when the DiagnosticConsumer's destructor would not
+  /// otherwise be called. Subclasses that require this to be called must ensure
+  /// that in their destructor.
   virtual void finish() {}
 
   /// Indicates whether the diagnostics handled by this

clang/lib/Frontend/CompilerInstance.cpp

@@ -1016,12 +1016,6 @@ bool CompilerInstance::ExecuteAction(FrontendAction &Act) {
   // better. We generally expect frontend actions to be invoked with (nearly)
   // DesiredStackSpace available.
   noteBottomOfStack();
-
-  auto FinishDiagnosticClient = llvm::make_scope_exit([&]() {
-    // Notify the diagnostic client that all files were processed.
-    getDiagnosticClient().finish();
-  });
-
   raw_ostream &OS = getVerboseOutputStream();
 
   if (!Act.PrepareToExecute(*this))

clang/lib/Frontend/SerializedDiagnosticPrinter.cpp

@@ -129,7 +129,7 @@ class SDiagsMerger : SerializedDiagnosticReader {
   void writeRecordWithBlob(unsigned ID, RecordData &Record, StringRef Blob);
 };
 
-class SDiagsWriter : public DiagnosticConsumer {
+class SDiagsWriter final : public DiagnosticConsumer {
   friend class SDiagsRenderer;
   friend class SDiagsMerger;
 
@@ -149,7 +149,13 @@ class SDiagsWriter : public DiagnosticConsumer {
     EmitPreamble();
   }
 
-  ~SDiagsWriter() override {}
+  ~SDiagsWriter() override {
+    // Not all uses of DiagnosticConsumer call finish, and not all uses invoke
+    // the destructor. This makes calling finish optional for cases where it
+    // is properly destructed.
+    if (!IsFinishing)
+      finish();
+  }
 
   void HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
                         const Diagnostic &Info) override;

clang/tools/driver/cc1_main.cpp

@@ -26,6 +26,7 @@
 #include "clang/Frontend/Utils.h"
 #include "clang/FrontendTool/Utils.h"
 #include "clang/Serialization/ObjectFilePCHContainerReader.h"
+#include "llvm/ADT/ScopeExit.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Config/llvm-config.h"
@@ -274,10 +275,21 @@ int cc1_main(ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr) {
                                   static_cast<void*>(&Clang->getDiagnostics()));
 
   DiagsBuffer->FlushDiagnostics(Clang->getDiagnostics());
-  if (!Success) {
+
+  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;
-  }
 
   // Execute the frontend actions.
   {
@@ -314,10 +326,9 @@ int cc1_main(ArrayRef<const char *> Argv, const char *Argv0, void *MainAddr) {
     }
   }
 
-  // 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();
+  // 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) {