swiftlang · davidungar · Jun 13, 2018 · Jun 12, 2018 · Jun 12, 2018
diff --git a/include/swift/AST/DiagnosticConsumer.h b/include/swift/AST/DiagnosticConsumer.h
@@ -100,7 +100,16 @@ class DiagnosticConsumer {
                                 const DiagnosticInfo &Info) = 0;
 
   /// \returns true if an error occurred while finishing-up.
-  virtual bool finishProcessing(SourceManager &) { return false; }
+  virtual bool finishProcessing() { return false; }
+
+  /// In batch mode, any error causes failure for all primary files, but
+  /// anyone consulting .dia files will only see an error for a particular
+  /// primary in that primary's serialized diagnostics file. For other
+  /// primaries' serialized diagnostics files, do something to signal the driver
+  /// what happened. This is only meaningful for SerializedDiagnosticConsumers,
+  /// so here's a placeholder.
+
+  virtual void informDriverOfIncompleteBatchModeCompilation() {}
 };
 
 /// \brief DiagnosticConsumer that discards all diagnostics.
@@ -193,13 +202,14 @@ class FileSpecificDiagnosticConsumer : public DiagnosticConsumer {
                         ArrayRef<DiagnosticArgument> FormatArgs,
                         const DiagnosticInfo &Info) override;
 
-  bool finishProcessing(SourceManager &) override;
+  bool finishProcessing() override;
 
 private:
   /// In batch mode, any error causes failure for all primary files, but
   /// Xcode will only see an error for a particular primary in that primary's
-  /// serialized diagnostics file. So, emit errors for all other primaries here.
-  void addNonSpecificErrors(SourceManager &SM);
+  /// serialized diagnostics file. So, tell the subconsumers to inform the
+  /// driver of incomplete batch mode compilation.
+  void tellSubconsumersToInformDriverOfIncompleteBatchModeCompilation() const;
 
   void computeConsumersOrderedByRange(SourceManager &SM);
 

diff --git a/include/swift/AST/DiagnosticEngine.h b/include/swift/AST/DiagnosticEngine.h
@@ -759,7 +759,7 @@ namespace swift {
 
     /// \returns true if any diagnostic consumer gave an error while invoking
     //// \c finishProcessing.
-    bool finishProcessing(SourceManager &);
+    bool finishProcessing();
 
     /// \brief Format the given diagnostic text and place the result in the given
     /// buffer.

diff --git a/include/swift/AST/DiagnosticsFrontend.def b/include/swift/AST/DiagnosticsFrontend.def
@@ -235,8 +235,6 @@ WARNING(cannot_assign_value_to_conditional_compilation_flag,none,
 ERROR(error_optimization_remark_pattern, none, "%0 in '%1'",
       (StringRef, StringRef))
 
-ERROR(error_compilation_stopped_by_errors_in_other_files,none, "compilation stopped by errors in other files", ())
-
 #ifndef DIAG_NO_UNDEF
 # if defined(DIAG)
 #  undef DIAG

diff --git a/lib/AST/DiagnosticConsumer.cpp b/lib/AST/DiagnosticConsumer.cpp
@@ -189,41 +189,25 @@ void FileSpecificDiagnosticConsumer::handleDiagnostic(
       Kind == DiagnosticKind::Error;
 }
 
-bool FileSpecificDiagnosticConsumer::finishProcessing(SourceManager &SM) {
-  addNonSpecificErrors(SM);
+bool FileSpecificDiagnosticConsumer::finishProcessing() {
+  tellSubconsumersToInformDriverOfIncompleteBatchModeCompilation();
 
   // Deliberately don't use std::any_of here because we don't want early-exit
   // behavior.
 
   bool hadError = false;
   for (auto &subConsumer : SubConsumers)
-    hadError |= subConsumer.second && subConsumer.second->finishProcessing(SM);
+    hadError |= subConsumer.second && subConsumer.second->finishProcessing();
   return hadError;
 }
 
-static void produceNonSpecificError(
-    FileSpecificDiagnosticConsumer::ConsumerSpecificInformation &consumerInfo,
-    SourceManager &SM) {
-  Diagnostic diagnostic(
-      diag::error_compilation_stopped_by_errors_in_other_files);
-
-  // Stolen from DiagnosticEngine::emitDiagnostic
-  DiagnosticInfo Info;
-  Info.ID = diagnostic.getID();
-
-  consumerInfo.consumer->handleDiagnostic(
-      SM, consumerInfo.range.getStart(), DiagnosticKind::Error,
-      DiagnosticEngine::diagnosticStringFor(diagnostic.getID()), {}, Info);
-}
-
-void FileSpecificDiagnosticConsumer::addNonSpecificErrors(SourceManager &SM) {
+void FileSpecificDiagnosticConsumer::
+    tellSubconsumersToInformDriverOfIncompleteBatchModeCompilation() const {
   if (!HasAnErrorBeenConsumed)
     return;
   for (auto &info : ConsumersOrderedByRange) {
-    if (!info.hasAnErrorBeenEmitted && info.consumer) {
-      produceNonSpecificError(info, SM);
-      info.hasAnErrorBeenEmitted = true;
-    }
+    if (!info.hasAnErrorBeenEmitted && info.consumer)
+      info.consumer->informDriverOfIncompleteBatchModeCompilation();
   }
 }
 

diff --git a/lib/AST/DiagnosticEngine.cpp b/lib/AST/DiagnosticEngine.cpp
@@ -251,10 +251,10 @@ bool DiagnosticEngine::isDiagnosticPointsToFirstBadToken(DiagID ID) const {
   return storedDiagnosticInfos[(unsigned) ID].pointsToFirstBadToken;
 }
 
-bool DiagnosticEngine::finishProcessing(SourceManager &SM) {
+bool DiagnosticEngine::finishProcessing() {
   bool hadError = false;
   for (auto &Consumer : Consumers) {
-    hadError |= Consumer->finishProcessing(SM);
+    hadError |= Consumer->finishProcessing();
   }
   return hadError;
 }

diff --git a/lib/Frontend/SerializedDiagnosticConsumer.cpp b/lib/Frontend/SerializedDiagnosticConsumer.cpp
@@ -130,6 +130,8 @@ class SerializedDiagnosticConsumer : public DiagnosticConsumer {
   /// \brief State shared among the various clones of this diagnostic consumer.
   llvm::IntrusiveRefCntPtr<SharedState> State;
   bool CalledFinishProcessing = false;
+  bool CompilationWasComplete = true;
+
 public:
   SerializedDiagnosticConsumer(StringRef serializedDiagnosticsPath)
       : State(new SharedState(serializedDiagnosticsPath)) {
@@ -140,7 +142,7 @@ class SerializedDiagnosticConsumer : public DiagnosticConsumer {
     assert(CalledFinishProcessing && "did not call finishProcessing()");
   }
 
-  bool finishProcessing(SourceManager &SM) override {
+  bool finishProcessing() override {
     assert(!CalledFinishProcessing &&
            "called finishProcessing() multiple times");
     CalledFinishProcessing = true;
@@ -160,19 +162,35 @@ class SerializedDiagnosticConsumer : public DiagnosticConsumer {
                                       llvm::sys::fs::F_None));
     if (EC) {
       // Create a temporary diagnostics engine to print the error to stderr.
-      DiagnosticEngine DE(SM);
+      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();
+    if (CompilationWasComplete) {
+      OS->write((char *)&State->Buffer.front(), State->Buffer.size());
+      OS->flush();
+    }
     return false;
   }
 
+  /// In batch mode, if any error occurs, no primaries can be compiled.
+  /// Some primaries will have errors in their diagnostics files and so
+  /// a client (such as Xcode) can see that those primaries failed.
+  /// Other primaries will have no errors in their diagnostics files.
+  /// In order for the driver to distinguish the two cases without parsing
+  /// the diagnostics, the frontend emits a truncated diagnostics file
+  /// for the latter case.
+  /// The unfortunate aspect is that the truncation discards warnings, etc.
+
+  void informDriverOfIncompleteBatchModeCompilation() override {
+    CompilationWasComplete = false;
+  }
+
   void handleDiagnostic(SourceManager &SM, SourceLoc Loc,
                                 DiagnosticKind Kind,
                                 StringRef FormatString,

diff --git a/lib/FrontendTool/FrontendTool.cpp b/lib/FrontendTool/FrontendTool.cpp
@@ -405,7 +405,7 @@ class JSONFixitWriter
     }
   }
 
-  bool finishProcessing(SourceManager &) override {
+  bool finishProcessing() override {
     std::error_code EC;
     std::unique_ptr<llvm::raw_fd_ostream> OS;
     OS.reset(new llvm::raw_fd_ostream(FixitsOutputPath,
@@ -1659,7 +1659,7 @@ int swift::performFrontend(ArrayRef<const char *> Args,
 
   auto finishDiagProcessing = [&](int retValue) -> int {
     FinishDiagProcessingCheckRAII.CalledFinishDiagProcessing = true;
-    bool err = Instance->getDiags().finishProcessing(Instance->getSourceMgr());
+    bool err = Instance->getDiags().finishProcessing();
     return retValue ? retValue : err;
   };
 

diff --git a/test/Misc/serialized-diagnostics-batch-mode-nonprimary-errors-only.swift b/test/Misc/serialized-diagnostics-batch-mode-nonprimary-errors-only.swift
@@ -3,13 +3,9 @@
 // RUN: rm -f %t.*
 
 // RUN: not %target-swift-frontend -typecheck  -primary-file %s  -serialize-diagnostics-path %t.main.dia -primary-file %S/../Inputs/empty.swift  -serialize-diagnostics-path %t.empty.dia %S/Inputs/serialized-diagnostics-batch-mode-suppression-helper.swift  2> %t.stderr.txt
-// RUN: c-index-test -read-diagnostics %t.main.dia 2> %t.main.txt
-// RUN: c-index-test -read-diagnostics %t.empty.dia 2> %t.empty.txt
-
-// RUN: %FileCheck -check-prefix=GENERAL-ERROR-OCCURRED %s <%t.main.txt
-// RUN: %FileCheck -check-prefix=GENERAL-ERROR-OCCURRED %s <%t.empty.txt
-// GENERAL-ERROR-OCCURRED: compilation stopped by errors in other files
 
+// RUN: test -e %t.main.dia -a ! -s %t.empty.dia
+// RUN: test -e %t.empty.dia -a ! -s %t.empty.dia
 
 func test(x: SomeType) {
 }
diff --git a/test/Misc/serialized-diagnostics-batch-mode-nonprimary-errors.swift b/test/Misc/serialized-diagnostics-batch-mode-nonprimary-errors.swift
@@ -4,10 +4,9 @@
 
 // RUN: not %target-swift-frontend -typecheck  -primary-file %s  -serialize-diagnostics-path %t.main.dia -primary-file %S/../Inputs/empty.swift  -serialize-diagnostics-path %t.empty.dia %S/Inputs/serialized-diagnostics-batch-mode-suppression-helper.swift  2> %t.stderr.txt
 // RUN: c-index-test -read-diagnostics %t.main.dia 2> %t.main.txt
-// RUN: c-index-test -read-diagnostics %t.empty.dia 2> %t.empty.txt
 
 // RUN: %FileCheck -check-prefix=NO-GENERAL-ERROR-OCCURRED %s <%t.main.txt
-// RUN: %FileCheck -check-prefix=GENERAL-ERROR-OCCURRED %s <%t.empty.txt
+// RUN: test -e %t.empty.dia -a ! -s %t.empty.dia
 // GENERAL-ERROR-OCCURRED: compilation stopped by errors in other files
 // NO-GENERAL-ERROR-OCCURRED-NOT: compilation stopped by errors in other files
 

diff --git a/test/Misc/serialized-diagnostics-batch-mode-primary-errors.swift b/test/Misc/serialized-diagnostics-batch-mode-primary-errors.swift
@@ -4,11 +4,10 @@
 
 // RUN: not %target-swift-frontend -typecheck  -primary-file %s  -serialize-diagnostics-path %t.main.dia -primary-file %S/../Inputs/empty.swift  -serialize-diagnostics-path %t.empty.dia  2> %t.stderr.txt
 // RUN: c-index-test -read-diagnostics %t.main.dia 2> %t.main.txt
-// RUN: c-index-test -read-diagnostics %t.empty.dia 2> %t.empty.txt
 
 // RUN: %FileCheck -check-prefix=ERROR %s <%t.main.txt
 // RUN: %FileCheck -check-prefix=NO-NONSPECIFIC-ERROR %s <%t.main.txt
-// RUN: %FileCheck -check-prefix=NONSPECIFIC-ERROR %s <%t.empty.txt
+// RUN: test -e %t.empty.dia -a ! -s %t.empty.dia
 
 // ERROR: error:
 // NONSPECIFIC-ERROR: error: compilation stopped by errors in other files
@@ -17,4 +16,3 @@
 func test(x: SomeType) {
   nonexistent()
 }
-
diff --git a/unittests/AST/DiagnosticConsumerTests.cpp b/unittests/AST/DiagnosticConsumerTests.cpp
@@ -43,7 +43,7 @@ namespace {
       expected.erase(expected.begin());
     }
 
-    bool finishProcessing(SourceManager &) override {
+    bool finishProcessing() override {
       EXPECT_FALSE(hasFinished);
       if (previous)
         EXPECT_TRUE(previous->hasFinished);
@@ -68,7 +68,7 @@ TEST(FileSpecificDiagnosticConsumer, SubConsumersFinishInOrder) {
   consumers.emplace_back("", std::move(consumerUnaffiliated));
 
   FileSpecificDiagnosticConsumer topConsumer(consumers);
-  topConsumer.finishProcessing(sourceMgr);
+  topConsumer.finishProcessing();
 }
 
 TEST(FileSpecificDiagnosticConsumer, InvalidLocDiagsGoToEveryConsumer) {
@@ -89,7 +89,7 @@ TEST(FileSpecificDiagnosticConsumer, InvalidLocDiagsGoToEveryConsumer) {
   FileSpecificDiagnosticConsumer topConsumer(consumers);
   topConsumer.handleDiagnostic(sourceMgr, SourceLoc(), DiagnosticKind::Error,
                                "dummy", {}, DiagnosticInfo());
-  topConsumer.finishProcessing(sourceMgr);
+  topConsumer.finishProcessing();
 }
 
 TEST(FileSpecificDiagnosticConsumer, ErrorsWithLocationsGoToExpectedConsumers) {
@@ -139,7 +139,7 @@ TEST(FileSpecificDiagnosticConsumer, ErrorsWithLocationsGoToExpectedConsumers) {
                                "back", {}, DiagnosticInfo());
   topConsumer.handleDiagnostic(sourceMgr, backOfB, DiagnosticKind::Error,
                                "back", {}, DiagnosticInfo());
-  topConsumer.finishProcessing(sourceMgr);
+  topConsumer.finishProcessing();
 }
 
 TEST(FileSpecificDiagnosticConsumer,
@@ -193,7 +193,7 @@ TEST(FileSpecificDiagnosticConsumer,
                                "back", {}, DiagnosticInfo());
   topConsumer.handleDiagnostic(sourceMgr, backOfB, DiagnosticKind::Error,
                                "back", {}, DiagnosticInfo());
-  topConsumer.finishProcessing(sourceMgr);
+  topConsumer.finishProcessing();
 }
 
 TEST(FileSpecificDiagnosticConsumer, WarningsAndRemarksAreTreatedLikeErrors) {
@@ -234,7 +234,7 @@ TEST(FileSpecificDiagnosticConsumer, WarningsAndRemarksAreTreatedLikeErrors) {
                                "remark", {}, DiagnosticInfo());
   topConsumer.handleDiagnostic(sourceMgr, frontOfB, DiagnosticKind::Remark,
                                "remark", {}, DiagnosticInfo());
-  topConsumer.finishProcessing(sourceMgr);
+  topConsumer.finishProcessing();
 }
 
 TEST(FileSpecificDiagnosticConsumer, NotesAreAttachedToErrors) {
@@ -296,7 +296,7 @@ TEST(FileSpecificDiagnosticConsumer, NotesAreAttachedToErrors) {
                                "note", {}, DiagnosticInfo());
   topConsumer.handleDiagnostic(sourceMgr, backOfA, DiagnosticKind::Note,
                                "note", {}, DiagnosticInfo());
-  topConsumer.finishProcessing(sourceMgr);
+  topConsumer.finishProcessing();
 }
 
 TEST(FileSpecificDiagnosticConsumer, NotesAreAttachedToWarningsAndRemarks) {
@@ -358,7 +358,7 @@ TEST(FileSpecificDiagnosticConsumer, NotesAreAttachedToWarningsAndRemarks) {
                                "note", {}, DiagnosticInfo());
   topConsumer.handleDiagnostic(sourceMgr, backOfA, DiagnosticKind::Note,
                                "note", {}, DiagnosticInfo());
-  topConsumer.finishProcessing(sourceMgr);
+  topConsumer.finishProcessing();
 }
 
 TEST(FileSpecificDiagnosticConsumer, NotesAreAttachedToErrorsEvenAcrossFiles) {
@@ -417,7 +417,7 @@ TEST(FileSpecificDiagnosticConsumer, NotesAreAttachedToErrorsEvenAcrossFiles) {
                                "note", {}, DiagnosticInfo());
   topConsumer.handleDiagnostic(sourceMgr, backOfA, DiagnosticKind::Note,
                                "note", {}, DiagnosticInfo());
-  topConsumer.finishProcessing(sourceMgr);
+  topConsumer.finishProcessing();
 }
 
 TEST(FileSpecificDiagnosticConsumer,
@@ -480,7 +480,7 @@ TEST(FileSpecificDiagnosticConsumer,
                                "note", {}, DiagnosticInfo());
   topConsumer.handleDiagnostic(sourceMgr, backOfA, DiagnosticKind::Note,
                                "note", {}, DiagnosticInfo());
-  topConsumer.finishProcessing(sourceMgr);
+  topConsumer.finishProcessing();
 }
 
 
@@ -529,5 +529,5 @@ TEST(FileSpecificDiagnosticConsumer,
                                "error", {}, DiagnosticInfo());
   topConsumer.handleDiagnostic(sourceMgr, SourceLoc(), DiagnosticKind::Note,
                                "note", {}, DiagnosticInfo());
-  topConsumer.finishProcessing(sourceMgr);
+  topConsumer.finishProcessing();
 }