[frontend] Add 'finishProcessing()' method for DiagnosticConsumers to do their finalization (e.g. writing to a file)

Use this to avoid creating an empty serialized diagnostics file when the compiler crashes.

Author: akyrtzi

include/swift/AST/DiagnosticConsumer.h

@@ -96,6 +96,9 @@ class DiagnosticConsumer {
                                 StringRef FormatString,
                                 ArrayRef<DiagnosticArgument> FormatArgs,
                                 const DiagnosticInfo &Info) = 0;
+
+  /// \returns true if an error occurred while finishing-up.
+  virtual bool finishProcessing() { return false; }
 };
 
 /// \brief DiagnosticConsumer that discards all diagnostics.

include/swift/AST/DiagnosticEngine.h

@@ -741,6 +741,10 @@ namespace swift {
     /// \returns true if diagnostic is marked with PointsToFirstBadToken
     /// option.
     bool isDiagnosticPointsToFirstBadToken(DiagID id) const;
+
+    /// \returns true if any diagnostic consumer gave an error while invoking
+    //// \c finishProcessing.
+    bool finishProcessing();
 
     /// \brief Format the given diagnostic text and place the result in the given
     /// buffer.

include/swift/Frontend/SerializedDiagnosticConsumer.h

@@ -21,7 +21,7 @@
 #include <memory>
 
 namespace llvm {
-  class raw_ostream;
+  class StringRef;
 }
 
 namespace swift {
@@ -30,13 +30,12 @@ namespace swift {
 
   namespace serialized_diagnostics {
     /// \brief Create a DiagnosticConsumer that serializes diagnostics to a
-    ///        stream.
+    ///        file.
     ///
-    /// \param OS the stream to emit the diagnostics.  The consumer takes
-    ///        ownership of the stream.
+    /// \param serializedDiagnosticsPath the file path to write the diagnostics.
     ///
     /// \returns A new diagnostic consumer that serializes diagnostics.
-    DiagnosticConsumer *createConsumer(std::unique_ptr<llvm::raw_ostream> OS);
+    DiagnosticConsumer *createConsumer(llvm::StringRef serializedDiagnosticsPath);
   }
 }
 

lib/AST/DiagnosticEngine.cpp

@@ -247,6 +247,14 @@ bool DiagnosticEngine::isDiagnosticPointsToFirstBadToken(DiagID ID) const {
   return storedDiagnosticInfos[(unsigned) ID].pointsToFirstBadToken;
 }
 
+bool DiagnosticEngine::finishProcessing() {
+  bool hadError = false;
+  for (auto &Consumer : Consumers) {
+    hadError =  Consumer->finishProcessing() || hadError;
+  }
+  return hadError;
+}
+
 /// \brief Skip forward to one of the given delimiters.
 ///
 /// \param Text The text to search through, which will be updated to point

lib/Frontend/SerializedDiagnosticConsumer.cpp

@@ -16,12 +16,15 @@
 
 #include "swift/Frontend/SerializedDiagnosticConsumer.h"
 #include "swift/AST/DiagnosticConsumer.h"
+#include "swift/AST/DiagnosticsFrontend.h"
 #include "swift/Basic/LLVM.h"
 #include "swift/Basic/SourceManager.h"
+#include "swift/Frontend/PrintingDiagnosticConsumer.h"
 #include "swift/Parse/Lexer.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/Support/FileSystem.h"
 #include "llvm/Support/raw_ostream.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/SmallString.h"
@@ -84,17 +87,19 @@ typedef SmallVector<uint64_t, 64> RecordData;
 typedef SmallVectorImpl<uint64_t> RecordDataImpl;
 
 struct SharedState : llvm::RefCountedBase<SharedState> {
-  SharedState(std::unique_ptr<raw_ostream> OS)
-      : Stream(Buffer), OS(std::move(OS)), EmittedAnyDiagBlocks(false) { }
+  SharedState(StringRef serializedDiagnosticsPath)
+      : Stream(Buffer),
+        SerializedDiagnosticsPath(serializedDiagnosticsPath),
+        EmittedAnyDiagBlocks(false) {}
 
   /// \brief The byte buffer for the serialized content.
   llvm::SmallString<1024> Buffer;
 
   /// \brief The BitStreamWriter for the serialized diagnostics.
   llvm::BitstreamWriter Stream;
 
-  /// \brief The name of the diagnostics file.
-  std::unique_ptr<raw_ostream> OS;
+  /// \brief The path of the diagnostics file.
+  std::string SerializedDiagnosticsPath;
 
   /// \brief The set of constructed record abbreviations.
   AbbreviationMap Abbrevs;
@@ -124,15 +129,21 @@ struct SharedState : llvm::RefCountedBase<SharedState> {
 class SerializedDiagnosticConsumer : public DiagnosticConsumer {
   /// \brief State shared among the various clones of this diagnostic consumer.
   llvm::IntrusiveRefCntPtr<SharedState> State;
+  bool CalledFinishProcessing = false;
 public:
-  SerializedDiagnosticConsumer(std::unique_ptr<raw_ostream> OS)
-      : State(new SharedState(std::move(OS))) {
+  SerializedDiagnosticConsumer(StringRef serializedDiagnosticsPath)
+      : State(new SharedState(serializedDiagnosticsPath)) {
     emitPreamble();
   }
 
-  ~SerializedDiagnosticConsumer() override {
-    // FIXME: we may not wish to put this in a destructor.
-    // That's not what clang does.
+  ~SerializedDiagnosticConsumer() {
+    assert(CalledFinishProcessing && "did not call finishProcessing()");
+  }
+
+  bool finishProcessing() override {
+    assert(!CalledFinishProcessing &&
+           "called finishProcessing() multiple times");
+    CalledFinishProcessing = true;
 
     // NOTE: clang also does check for shared instances.  We don't
     // have these yet in Swift, but if we do we need to add an extra
@@ -142,10 +153,25 @@ class SerializedDiagnosticConsumer : public DiagnosticConsumer {
     if (State->EmittedAnyDiagBlocks)
       exitDiagBlock();
 
-    // Write the generated bitstream to "Out".
-    State->OS->write((char *)&State->Buffer.front(), State->Buffer.size());
-    State->OS->flush();
-    State->OS.reset(nullptr);
+    // Write the generated bitstream to the file.
+    std::error_code EC;
+    std::unique_ptr<llvm::raw_fd_ostream> OS;
+    OS.reset(new llvm::raw_fd_ostream(State->SerializedDiagnosticsPath, EC,
+                                      llvm::sys::fs::F_None));
+    if (EC) {
+      // Create a temporary diagnostics engine to print the error to stderr.
+      SourceManager dummyMgr;
+      DiagnosticEngine DE(dummyMgr);
+      PrintingDiagnosticConsumer PDC;
+      DE.addConsumer(PDC);
+      DE.diagnose(SourceLoc(), diag::cannot_open_serialized_file,
+                  State->SerializedDiagnosticsPath, EC.message());
+      return true;
+    }
+
+    OS->write((char *)&State->Buffer.front(), State->Buffer.size());
+    OS->flush();
+    return false;
   }
 
   void handleDiagnostic(SourceManager &SM, SourceLoc Loc,
@@ -197,8 +223,8 @@ class SerializedDiagnosticConsumer : public DiagnosticConsumer {
 } // end anonymous namespace
 
 namespace swift { namespace serialized_diagnostics {
-  DiagnosticConsumer *createConsumer(std::unique_ptr<llvm::raw_ostream> OS) {
-    return new SerializedDiagnosticConsumer(std::move(OS));
+  DiagnosticConsumer *createConsumer(StringRef serializedDiagnosticsPath) {
+    return new SerializedDiagnosticConsumer(serializedDiagnosticsPath);
   }
 } // namespace serialized_diagnostics
 } // namespace swift

lib/FrontendTool/FrontendTool.cpp

@@ -305,19 +305,17 @@ namespace {
 /// format.
 class JSONFixitWriter
   : public DiagnosticConsumer, public migrator::FixitFilter {
+  std::string FixitsOutputPath;
   std::unique_ptr<llvm::raw_ostream> OSPtr;
   bool FixitAll;
   std::vector<SingleEdit> AllEdits;
 
 public:
-  JSONFixitWriter(std::unique_ptr<llvm::raw_ostream> OS,
+  JSONFixitWriter(std::string fixitsOutputPath,
                   const DiagnosticOptions &DiagOpts)
-    : OSPtr(std::move(OS)),
+    : FixitsOutputPath(fixitsOutputPath),
       FixitAll(DiagOpts.FixitCodeForAllDiagnostics) {}
 
-  ~JSONFixitWriter() override {
-    swift::writeEditsInJson(llvm::makeArrayRef(AllEdits), *OSPtr);
-  }
 private:
   void handleDiagnostic(SourceManager &SM, SourceLoc Loc,
                         DiagnosticKind Kind,
@@ -330,6 +328,27 @@ class JSONFixitWriter
       AllEdits.push_back({SM, Fix.getRange(), Fix.getText()});
     }
   }
+
+  bool finishProcessing() override {
+    std::error_code EC;
+    std::unique_ptr<llvm::raw_fd_ostream> OS;
+    OS.reset(new llvm::raw_fd_ostream(FixitsOutputPath,
+                                      EC,
+                                      llvm::sys::fs::F_None));
+    if (EC) {
+      // Create a temporary diagnostics engine to print the error to stderr.
+      SourceManager dummyMgr;
+      DiagnosticEngine DE(dummyMgr);
+      PrintingDiagnosticConsumer PDC;
+      DE.addConsumer(PDC);
+      DE.diagnose(SourceLoc(), diag::cannot_open_serialized_file,
+                  FixitsOutputPath, EC.message());
+      return true;
+    }
+
+    swift::writeEditsInJson(llvm::makeArrayRef(AllEdits), *OS);
+    return false;
+  }
 };
 
 } // anonymous namespace
@@ -1026,9 +1045,23 @@ int swift::performFrontend(ArrayRef<const char *> Args,
     llvm::make_unique<CompilerInstance>();
   Instance->addDiagnosticConsumer(&PDC);
 
+  struct FinishDiagProcessingCheckRAII {
+    bool CalledFinishDiagProcessing = false;
+    ~FinishDiagProcessingCheckRAII() {
+      assert(CalledFinishDiagProcessing && "returned from the function "
+        "without calling finishDiagProcessing");
+    }
+  } FinishDiagProcessingCheckRAII;
+
+  auto finishDiagProcessing = [&](int retValue) -> int {
+    FinishDiagProcessingCheckRAII.CalledFinishDiagProcessing = true;
+    bool err = Instance->getDiags().finishProcessing();
+    return retValue ? retValue : err;
+  };
+
   if (Args.empty()) {
     Instance->getDiags().diagnose(SourceLoc(), diag::error_no_frontend_args);
-    return 1;
+    return finishDiagProcessing(1);
   }
 
   CompilerInvocation Invocation;
@@ -1041,7 +1074,7 @@ int swift::performFrontend(ArrayRef<const char *> Args,
 
   // Parse arguments.
   if (Invocation.parseArgs(Args, Instance->getDiags(), workingDirectory)) {
-    return 1;
+    return finishDiagProcessing(1);
   }
 
   // Setting DWARF Version depend on platform
@@ -1063,14 +1096,14 @@ int swift::performFrontend(ArrayRef<const char *> Args,
     Options->PrintHelp(llvm::outs(), displayName(MainExecutablePath).c_str(),
                        "Swift frontend", IncludedFlagsBitmask,
                        ExcludedFlagsBitmask);
-    return 0;
+    return finishDiagProcessing(0);
   }
 
   if (Invocation.getFrontendOptions().RequestedAction ==
         FrontendOptions::NoneAction) {
     Instance->getDiags().diagnose(SourceLoc(),
                                  diag::error_missing_frontend_action);
-    return 1;
+    return finishDiagProcessing(1);
   }
 
   // Because the serialized diagnostics consumer is initialized here,
@@ -1084,21 +1117,8 @@ int swift::performFrontend(ArrayRef<const char *> Args,
     const std::string &SerializedDiagnosticsPath =
       Invocation.getFrontendOptions().SerializedDiagnosticsPath;
     if (!SerializedDiagnosticsPath.empty()) {
-      std::error_code EC;
-      std::unique_ptr<llvm::raw_fd_ostream> OS;
-      OS.reset(new llvm::raw_fd_ostream(SerializedDiagnosticsPath,
-                                        EC,
-                                        llvm::sys::fs::F_None));
-
-      if (EC) {
-        Instance->getDiags().diagnose(SourceLoc(),
-                                     diag::cannot_open_serialized_file,
-                                     SerializedDiagnosticsPath, EC.message());
-        return 1;
-      }
-
       SerializedConsumer.reset(
-          serialized_diagnostics::createConsumer(std::move(OS)));
+          serialized_diagnostics::createConsumer(SerializedDiagnosticsPath));
       Instance->addDiagnosticConsumer(SerializedConsumer.get());
     }
   }
@@ -1108,20 +1128,7 @@ int swift::performFrontend(ArrayRef<const char *> Args,
     const std::string &FixitsOutputPath =
       Invocation.getFrontendOptions().FixitsOutputPath;
     if (!FixitsOutputPath.empty()) {
-      std::error_code EC;
-      std::unique_ptr<llvm::raw_fd_ostream> OS;
-      OS.reset(new llvm::raw_fd_ostream(FixitsOutputPath,
-                                        EC,
-                                        llvm::sys::fs::F_None));
-
-      if (EC) {
-        Instance->getDiags().diagnose(SourceLoc(),
-                                     diag::cannot_open_file,
-                                     FixitsOutputPath, EC.message());
-        return 1;
-      }
-
-      FixitsConsumer.reset(new JSONFixitWriter(std::move(OS),
+      FixitsConsumer.reset(new JSONFixitWriter(FixitsOutputPath,
                                             Invocation.getDiagnosticOptions()));
       Instance->addDiagnosticConsumer(FixitsConsumer.get());
     }
@@ -1167,7 +1174,7 @@ int swift::performFrontend(ArrayRef<const char *> Args,
   }
 
   if (Instance->setup(Invocation)) {
-    return 1;
+    return finishDiagProcessing(1);
   }
 
   if (StatsReporter) {
@@ -1212,7 +1219,7 @@ int swift::performFrontend(ArrayRef<const char *> Args,
     }
   }
 
-  return (HadError ? 1 : ReturnValue);
+  return finishDiagProcessing(HadError ? 1 : ReturnValue);
 }
 
 void FrontendObserver::parsedArgs(CompilerInvocation &invocation) {}

test/Misc/serialized-diagnostics-file.swift

@@ -0,0 +1,12 @@
+// RUN: rm -rf %t
+// RUN: mkdir -p %t
+
+// RUN: not %target-swift-frontend -typecheck -serialize-diagnostics-path %t/nonexistent/some.dia %s 2>%t.err.txt
+// RUN: %FileCheck --input-file=%t.err.txt %s -check-prefix=OPEN-FAIL
+// OPEN-FAIL: cannot open file '{{.*}}/nonexistent/some.dia' for diagnostics emission
+
+var CRASH = 0
+
+// Make sure no diagnostic file is created if the compiler crashes.
+// RUN: not --crash %target-swift-frontend -typecheck -serialize-diagnostics-path %t/some.dia %s -debug-forbid-typecheck-prefix CRASH
+// RUN: not find %t/some.dia