suppress non-primary errors if there is no responsible primary

Author: David Ungar

include/swift/AST/DiagnosticConsumer.h

@@ -175,10 +175,13 @@ class FileSpecificDiagnosticConsumer : public DiagnosticConsumer {
     /// diagnostics that are not in any of the other consumers' files.
     std::string inputFileName;
 
-    /// The consumer for diagnostics associated with the inputFileName.
-    /// Should never be null, because diagnostics for non-primary files are
-    /// either emitted in the corresponding primary file's .dia or are
-    /// emitted to every .dia.
+    /// The consumer (if any) for diagnostics associated with the inputFileName.
+    /// A null pointer for the DiagnosticConsumer means that this file is a
+    /// non-primary one in batch mode and we have no .dia file for it.
+    /// If there is a responsible primary when the diagnostic is handled
+    /// it will be shunted to that primary's .dia file.
+    /// Otherwise it will be suppressed, assuming that the diagnostic will
+    /// surface in another frontend job that compiles that file as a primary.
     std::unique_ptr<DiagnosticConsumer> consumer;
 
     // Has this subconsumer ever handled a diagnostic that is an error?
@@ -198,6 +201,8 @@ class FileSpecificDiagnosticConsumer : public DiagnosticConsumer {
                           ArrayRef<DiagnosticArgument> FormatArgs,
                           const DiagnosticInfo &Info,
                           const SourceLoc bufferIndirectlyCausingDiagnostic) {
+      if (!getConsumer())
+        return;
       hasAnErrorBeenConsumed |= Kind == DiagnosticKind::Error;
       getConsumer()->handleDiagnostic(SM, Loc, Kind, FormatString, FormatArgs,
                                       Info, bufferIndirectlyCausingDiagnostic);

lib/AST/DiagnosticConsumer.cpp

@@ -117,6 +117,11 @@ FileSpecificDiagnosticConsumer::subconsumerForLocation(SourceManager &SM,
   // Diagnostics with invalid locations always go to every consumer.
   if (loc.isInvalid())
     return None;
+  
+  // What if a there's a consumer for fixits but there is no fixits output path?
+  // Subconsumers will be empty in that case.
+  if (Subconsumers.empty())
+    return None;
 
   // This map is generated on first use and cached, to allow the
   // FileSpecificDiagnosticConsumer to be set up before the source files are
@@ -207,13 +212,20 @@ Optional<FileSpecificDiagnosticConsumer::Subconsumer *>
 FileSpecificDiagnosticConsumer::findSubconsumerForNonNote(
     SourceManager &SM, const SourceLoc loc,
     const SourceLoc bufferIndirectlyCausingDiagnostic) {
-  if (auto subconsumer = subconsumerForLocation(SM, loc))
+  const auto subconsumer = subconsumerForLocation(SM, loc);
+  if (!subconsumer)
+    return None; // No place to put it; might be in an imported module
+  if ((*subconsumer)->getConsumer())
     return subconsumer; // A primary file with a .dia file
-  // The diagnostic is located in a non-primary.
-  // Try to put it in the current primary file's .dia file.
-  if (bufferIndirectlyCausingDiagnostic.isValid())
-    return subconsumerForLocation(SM, bufferIndirectlyCausingDiagnostic);
-  return None;
+  // Try to put it in the responsible primary input
+  if (bufferIndirectlyCausingDiagnostic.isInvalid())
+    return None;
+  const auto currentPrimarySubconsumer =
+      subconsumerForLocation(SM, bufferIndirectlyCausingDiagnostic);
+  assert(!currentPrimarySubconsumer ||
+         (*currentPrimarySubconsumer)->getConsumer() &&
+         "current primary must have a .dia file");
+  return currentPrimarySubconsumer;
 }
 
 bool FileSpecificDiagnosticConsumer::finishProcessing() {

lib/FrontendTool/FrontendTool.cpp

@@ -1540,12 +1540,30 @@ createDispatchingDiagnosticConsumerIfNeeded(
 
   inputsAndOutputs.forEachInputProducingSupplementaryOutput(
       [&](const InputFile &input) -> bool {
-        if (auto consumer = maybeCreateConsumerForDiagnosticsFrom(input)) {
-          assert(consumer && "Consumer should always be non-null");
+        if (auto consumer = maybeCreateConsumerForDiagnosticsFrom(input))
           subconsumers.emplace_back(input.file(), std::move(consumer));
-        }
         return false;
       });
+  // For batch mode, the compiler must sometimes swallow diagnostics pertaining to
+  // non-primary files in order to avoid Xcode showing the same diagnostic
+  // multiple times. So, create a diagnostic "eater" for those non-primary
+  // files.
+  //
+  // This routine gets called for \c createJSONFixItDiagnosticConsumerIfNeeded
+  // in cases where no primary subconsumers are created.
+  // Don't bother to create non-primary subconsumers if there aren't any primary
+  // ones.
+  //
+  // To avoid introducing bugs into WMO or single-file modes, test for multiple
+  // primaries.
+  if (!subconsumers.empty() && inputsAndOutputs.hasMultiplePrimaryInputs()) {
+    inputsAndOutputs.forEachNonPrimaryInput(
+        [&](const InputFile &input) -> bool {
+          subconsumers.emplace_back(input.file(), nullptr);
+          return false;
+        });
+  }
+
   return FileSpecificDiagnosticConsumer::consolidateSubconsumers(subconsumers);
 }