Merge pull request #17231 from graydon/rdar-40526328-emit-SIGINT-for-cancelled-batch-constituents-swift-4.2-branch

Author: swift-ci

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);
 

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.

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

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 &info,
-    SourceManager &SM) {
-  Diagnostic diagnostic(
-      diag::error_compilation_stopped_by_errors_in_other_files);
-
-  // Stolen from DiagnosticEngine::emitDiagnostic
-  DiagnosticInfo Info;
-  Info.ID = diagnostic.getID();
-
-  info.consumer->handleDiagnostic(
-      SM, info.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();
   }
 }
 

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;
 }

lib/Driver/Compilation.cpp

@@ -45,6 +45,8 @@
 
 #include "CompilationRecord.h"
 
+#include <signal.h>
+
 #define DEBUG_TYPE "batch-mode"
 
 // Batch-mode has a sub-mode for testing that randomizes batch partitions,
@@ -459,6 +461,35 @@ namespace driver {
       }
     }
 
+    /// Check to see if a job produced a zero-length serialized diagnostics
+    /// file, which is used to indicate batch-constituents that were batched
+    /// together with a failing constituent but did not, themselves, produce any
+    /// errors.
+    bool jobWasBatchedWithFailingJobs(const Job *J) const {
+      auto DiaPath =
+        J->getOutput().getAnyOutputForType(file_types::TY_SerializedDiagnostics);
+      if (DiaPath.empty())
+        return false;
+      if (!llvm::sys::fs::is_regular_file(DiaPath))
+        return false;
+      uint64_t Size;
+      auto EC = llvm::sys::fs::file_size(DiaPath, Size);
+      if (EC)
+        return false;
+      return Size == 0;
+    }
+
+    /// If a batch-constituent job happens to be batched together with a job
+    /// that exits with an error, the batch-constituent may be considered
+    /// "cancelled".
+    bool jobIsCancelledBatchConstituent(int ReturnCode,
+                                        const Job *ContainerJob,
+                                        const Job *ConstituentJob) {
+      return ReturnCode != 0 &&
+        isBatchJob(ContainerJob) &&
+        jobWasBatchedWithFailingJobs(ConstituentJob);
+    }
+
     /// Unpack a \c BatchJob that has finished into its constituent \c Job
     /// members, and call \c taskFinished on each, propagating any \c
     /// TaskFinishedResponse other than \c
@@ -482,6 +513,26 @@ namespace driver {
       return res;
     }
 
+    void
+    emitParseableOutputForEachFinishedJob(ProcessId Pid, int ReturnCode,
+                                          StringRef Output,
+                                          const Job *FinishedCmd) {
+      FinishedCmd->forEachContainedJobAndPID(Pid, [&](const Job *J,
+                                                      Job::PID P) {
+        if (jobIsCancelledBatchConstituent(ReturnCode, FinishedCmd, J)) {
+          // Simulate SIGINT-interruption to parseable-output consumer for any
+          // constituent of a failing batch job that produced no errors of its
+          // own.
+          parseable_output::emitSignalledMessage(llvm::errs(), *J, P,
+                                                 "cancelled batch constituent",
+                                                 "", SIGINT);
+        } else {
+          parseable_output::emitFinishedMessage(llvm::errs(), *J, P, ReturnCode,
+                                                Output);
+        }
+      });
+    }
+
     /// Callback which will be called immediately after a task has finished
     /// execution. Determines if execution should continue, and also schedule
     /// any additional Jobs which we now know we need to run.
@@ -508,12 +559,8 @@ namespace driver {
             llvm::errs() << Output;
           break;
         case OutputLevel::Parseable:
-          // Parseable output was requested.
-          FinishedCmd->forEachContainedJobAndPID(
-              Pid, [&](const Job *J, Job::PID P) {
-                parseable_output::emitFinishedMessage(llvm::errs(), *J, P,
-                                                      ReturnCode, Output);
-              });
+          emitParseableOutputForEachFinishedJob(Pid, ReturnCode, Output,
+                                                FinishedCmd);
           break;
         }
       }

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,

lib/FrontendTool/FrontendTool.cpp

@@ -404,7 +404,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,
@@ -1651,7 +1651,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;
   };
 

test/Driver/batch_mode_parseable_output_cancellation.swift

@@ -0,0 +1,11 @@
+// RUN: %empty-directory(%t)
+// RUN: touch %t/file-01.swift
+// RUN: echo 'public func main() { help_an_error_happened() }' >%t/main.swift
+//
+// RUN: not %swiftc_driver -enable-batch-mode -parseable-output -serialize-diagnostics -c -emit-module -module-name main -j 1 %t/file-01.swift %t/main.swift 2>&1 | %FileCheck %s
+//
+//      CHECK:   "kind": "signalled",
+// CHECK-NEXT:   "name": "compile",
+// CHECK-NEXT:   "pid": -{{[1-9][0-9]*}},
+// CHECK-NEXT:   "error-message": "{{.*}}",
+// CHECK-NEXT:   "signal": 2

test/Misc/serialized-diagnostics-batch-mode-nonprimary-errors-only.swift

@@ -0,0 +1,11 @@
+// Ensure that an error in a non-primary causes an error in the errorless primary.
+//
+// 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: 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) {
+}

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
 

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()
 }
-