Make JSON fix-it outputs be per-primary in batch mode. (swiftlang#14995)

This means moving the output path into SupplementaryOutputPaths, and
using the same sort of diagnostic dispatching that serialized
diagnostics use. This is part of what's needed to run the migrator
in batch mode.

Author: jrose-apple

include/swift/Basic/SupplementaryOutputPaths.h

@@ -49,6 +49,9 @@ struct SupplementaryOutputPaths {
   /// frontend invocation.
   std::string SerializedDiagnosticsPath;
 
+  /// The path to which we should output fix-its as source edits.
+  std::string FixItsOutputPath;
+
   /// The path to which we should output a loaded module trace file.
   /// It is currently only used with WMO, but could be generalized.
   std::string LoadedModuleTracePath;

include/swift/Frontend/FrontendOptions.h

@@ -55,9 +55,6 @@ class FrontendOptions {
   /// The name of the library to link against when using this module.
   std::string ModuleLinkName;
 
-  /// The path to which we should output fixits as source edits.
-  std::string FixitsOutputPath;
-
   /// Arguments which should be passed in immediate mode.
   std::vector<std::string> ImmediateArgv;
 

include/swift/Frontend/InputFile.h

@@ -95,6 +95,9 @@ class InputFile {
     return getPrimarySpecificPaths().SupplementaryOutputs
         .SerializedDiagnosticsPath;
   }
+  std::string fixItsOutputPath() const {
+    return getPrimarySpecificPaths().SupplementaryOutputs.FixItsOutputPath;
+  }
 };
 } // namespace swift
 

lib/Frontend/ArgsToFrontendOptionsConverter.cpp

@@ -117,9 +117,6 @@ bool ArgsToFrontendOptionsConverter::convert() {
   if (checkUnusedSupplementaryOutputPaths())
     return true;
 
-  if (const Arg *A = Args.getLastArg(OPT_emit_fixits_path))
-    Opts.FixitsOutputPath = A->getValue();
-
   if (const Arg *A = Args.getLastArg(OPT_module_link_name))
     Opts.ModuleLinkName = A->getValue();
 

lib/Frontend/ArgsToFrontendOutputsConverter.cpp

@@ -283,13 +283,15 @@ SupplementaryOutputPathsComputer::getSupplementaryOutputPathsFromArguments()
       options::OPT_emit_reference_dependencies_path);
   auto serializedDiagnostics = getSupplementaryFilenamesFromArguments(
       options::OPT_serialize_diagnostics_path);
+  auto fixItsOutput = getSupplementaryFilenamesFromArguments(
+      options::OPT_emit_fixits_path);
   auto loadedModuleTrace = getSupplementaryFilenamesFromArguments(
       options::OPT_emit_loaded_module_trace_path);
   auto TBD = getSupplementaryFilenamesFromArguments(options::OPT_emit_tbd_path);
 
   if (!objCHeaderOutput || !moduleOutput || !moduleDocOutput ||
       !dependenciesFile || !referenceDependenciesFile ||
-      !serializedDiagnostics || !loadedModuleTrace || !TBD) {
+      !serializedDiagnostics || !fixItsOutput || !loadedModuleTrace || !TBD) {
     return None;
   }
   std::vector<SupplementaryOutputPaths> result;
@@ -304,6 +306,7 @@ SupplementaryOutputPathsComputer::getSupplementaryOutputPathsFromArguments()
     sop.DependenciesFilePath = (*dependenciesFile)[i];
     sop.ReferenceDependenciesFilePath = (*referenceDependenciesFile)[i];
     sop.SerializedDiagnosticsPath = (*serializedDiagnostics)[i];
+    sop.FixItsOutputPath = (*fixItsOutput)[i];
     sop.LoadedModuleTracePath = (*loadedModuleTrace)[i];
     sop.TBDPath = (*TBD)[i];
 
@@ -356,6 +359,9 @@ SupplementaryOutputPathsComputer::computeOutputPathsForOneInput(
       OPT_serialize_diagnostics, pathsFromArguments.SerializedDiagnosticsPath,
       "dia", "", defaultSupplementaryOutputPathExcludingExtension);
 
+  // There is no non-path form of -emit-fixits-path
+  auto fixItsOutputPath = pathsFromArguments.FixItsOutputPath;
+
   auto objcHeaderOutputPath = determineSupplementaryOutputFilename(
       OPT_emit_objc_header, pathsFromArguments.ObjCHeaderOutputPath, "h", "",
       defaultSupplementaryOutputPathExcludingExtension);
@@ -391,6 +397,7 @@ SupplementaryOutputPathsComputer::computeOutputPathsForOneInput(
   sop.DependenciesFilePath = dependenciesFilePath;
   sop.ReferenceDependenciesFilePath = referenceDependenciesFilePath;
   sop.SerializedDiagnosticsPath = serializedDiagnosticsPath;
+  sop.FixItsOutputPath = fixItsOutputPath;
   sop.LoadedModuleTracePath = loadedModuleTracePath;
   sop.TBDPath = tbdPath;
   return sop;

lib/Frontend/CompilerInvocation.cpp

@@ -908,8 +908,10 @@ static bool ParseIRGenArgs(IRGenOptions &Opts, ArgList &Args,
   return false;
 }
 
-bool ParseMigratorArgs(MigratorOptions &Opts, llvm::Triple &Triple,
-                       StringRef ResourcePath, ArgList &Args,
+bool ParseMigratorArgs(MigratorOptions &Opts,
+                       const FrontendOptions &FrontendOpts,
+                       const llvm::Triple &Triple,
+                       StringRef ResourcePath, const ArgList &Args,
                        DiagnosticEngine &Diags) {
   using namespace options;
 
@@ -958,6 +960,21 @@ bool ParseMigratorArgs(MigratorOptions &Opts, llvm::Triple &Triple,
     }
   }
 
+  if (Opts.shouldRunMigrator()) {
+    assert(!FrontendOpts.InputsAndOutputs.isWholeModule());
+    // FIXME: In order to support batch mode properly, the migrator would have
+    // to support having one remap file path and one migrated file path per
+    // primary input. The easiest way to do this would be to move processing of
+    // these paths into FrontendOptions, like other supplementary outputs, and
+    // to call migrator::updateCodeAndEmitRemapIfNeeded once for each primary
+    // file.
+    //
+    // Supporting WMO would be similar, but WMO is set up to only produce one
+    // supplementary output for the whole compilation instead of one per input,
+    // so it's probably not worth it.
+    FrontendOpts.InputsAndOutputs.assertMustNotBeMoreThanOnePrimaryInput();
+  }
+
   return false;
 }
 
@@ -1022,7 +1039,7 @@ bool CompilerInvocation::parseArgs(ArrayRef<const char *> Args,
     return true;
   }
 
-  if (ParseMigratorArgs(MigratorOpts, LangOpts.Target,
+  if (ParseMigratorArgs(MigratorOpts, FrontendOpts, LangOpts.Target,
                         SearchPathOpts.RuntimeResourcePath, ParsedArgs, Diags)) {
     return true;
   }

lib/FrontendTool/FrontendTool.cpp

@@ -1509,6 +1509,41 @@ computeStatsReporter(const CompilerInvocation &Invocation, CompilerInstance *Ins
       ProfileEvents, ProfileEntities);
 }
 
+/// Creates a diagnostic consumer that handles dispatching diagnostics to
+/// multiple output files, based on the supplementary output paths specified by
+/// \p inputsAndOutputs.
+///
+/// If no output files are needed, returns null.
+static std::unique_ptr<DiagnosticConsumer>
+createDispatchingDiagnosticConsumerIfNeeded(
+    const FrontendInputsAndOutputs &inputsAndOutputs,
+    llvm::function_ref<std::unique_ptr<DiagnosticConsumer>(const InputFile &)>
+      maybeCreateSingleConsumer) {
+
+  // The "4" here is somewhat arbitrary. In practice we're going to have one
+  // sub-consumer for each diagnostic file we're trying to output, which (again
+  // in practice) is going to be 1 in WMO mode and equal to the number of
+  // primary inputs in batch mode. That in turn is going to be "the number of
+  // files we need to recompile in this build, divided by the number of jobs".
+  // So a value of "4" here means that there would be no heap allocation on a
+  // clean build of a module with up to 32 files on an 8-core machine, if the
+  // user doesn't customize anything.
+  SmallVector<FileSpecificDiagnosticConsumer::ConsumerPair, 4> subConsumers;
+
+  inputsAndOutputs.forEachInputProducingSupplementaryOutput(
+      [&](const InputFile &input) -> bool {
+    if (auto subConsumer = maybeCreateSingleConsumer(input))
+      subConsumers.emplace_back(input.file(), std::move(subConsumer));
+    return false;
+  });
+
+  if (subConsumers.empty())
+    return nullptr;
+  if (subConsumers.size() == 1)
+    return std::move(subConsumers.front()).second;
+  return llvm::make_unique<FileSpecificDiagnosticConsumer>(subConsumers);
+}
+
 /// Creates a diagnostic consumer that handles serializing diagnostics, based on
 /// the supplementary output paths specified by \p inputsAndOutputs.
 ///
@@ -1520,39 +1555,39 @@ computeStatsReporter(const CompilerInvocation &Invocation, CompilerInstance *Ins
 static std::unique_ptr<DiagnosticConsumer>
 createSerializedDiagnosticConsumerIfNeeded(
     const FrontendInputsAndOutputs &inputsAndOutputs) {
-
-  // The "4" here is somewhat arbitrary. In practice we're going to have one
-  // sub-consumer for each .dia file we're trying to output, which (again in
-  // practice) is going to be 1 in WMO mode and equal to the number of primary
-  // inputs in batch mode. That in turn is going to be "the number of files we
-  // need to recompile in this build, divided by the number of jobs". So a
-  // value of "4" here means that there would be no heap allocation on a clean
-  // build of a module with up to 32 files on an 8-core machine, if the user
-  // doesn't customize anything.
-  SmallVector<FileSpecificDiagnosticConsumer::ConsumerPair, 4>
-      serializedConsumers;
-
-  inputsAndOutputs.forEachInputProducingSupplementaryOutput(
-      [&](const InputFile &Input) -> bool {
-    std::string serializedDiagnosticsPath = Input.serializedDiagnosticsPath();
+  return createDispatchingDiagnosticConsumerIfNeeded(
+      inputsAndOutputs,
+      [](const InputFile &input) -> std::unique_ptr<DiagnosticConsumer> {
+    std::string serializedDiagnosticsPath = input.serializedDiagnosticsPath();
     if (serializedDiagnosticsPath.empty())
-      return false;
-
-    serializedConsumers.emplace_back(
-        Input.file(),
-        serialized_diagnostics::createConsumer(serializedDiagnosticsPath));
-
-    // Continue for other inputs.
-    return false;
+      return nullptr;
+    return serialized_diagnostics::createConsumer(serializedDiagnosticsPath);
   });
+}
 
-  if (serializedConsumers.empty())
-    return nullptr;
-  if (serializedConsumers.size() == 1)
-    return std::move(serializedConsumers.front()).second;
-  return llvm::make_unique<FileSpecificDiagnosticConsumer>(serializedConsumers);
+/// Creates a diagnostic consumer that handles serializing diagnostics, based on
+/// the supplementary output paths specified in \p options.
+///
+/// The returned consumer will handle producing multiple serialized diagnostics
+/// files if necessary, by using sub-consumers for each file and dispatching to
+/// the right one.
+///
+/// If no serialized diagnostics are being produced, returns null.
+static std::unique_ptr<DiagnosticConsumer>
+createJSONFixItDiagnosticConsumerIfNeeded(
+    const CompilerInvocation &invocation) {
+  return createDispatchingDiagnosticConsumerIfNeeded(
+      invocation.getFrontendOptions().InputsAndOutputs,
+      [&](const InputFile &input) -> std::unique_ptr<DiagnosticConsumer> {
+    std::string fixItsOutputPath = input.fixItsOutputPath();
+    if (fixItsOutputPath.empty())
+      return nullptr;
+    return llvm::make_unique<JSONFixitWriter>(
+        fixItsOutputPath, invocation.getDiagnosticOptions());
+  });
 }
 
+
 int swift::performFrontend(ArrayRef<const char *> Args,
                            const char *Argv0, void *MainAddr,
                            FrontendObserver *observer) {
@@ -1671,17 +1706,10 @@ int swift::performFrontend(ArrayRef<const char *> Args,
   if (SerializedConsumerDispatcher)
     Instance->addDiagnosticConsumer(SerializedConsumerDispatcher.get());
 
-  // FIXME: The fix-its need to be multiplexed like the serialized diagnostics.
-  std::unique_ptr<DiagnosticConsumer> FixitsConsumer;
-  {
-    const std::string &FixitsOutputPath =
-      Invocation.getFrontendOptions().FixitsOutputPath;
-    if (!FixitsOutputPath.empty()) {
-      FixitsConsumer.reset(new JSONFixitWriter(FixitsOutputPath,
-                                            Invocation.getDiagnosticOptions()));
-      Instance->addDiagnosticConsumer(FixitsConsumer.get());
-    }
-  }
+  std::unique_ptr<DiagnosticConsumer> FixItsConsumer =
+      createJSONFixItDiagnosticConsumerIfNeeded(Invocation);
+  if (FixItsConsumer)
+    Instance->addDiagnosticConsumer(FixItsConsumer.get());
 
   if (Invocation.getDiagnosticOptions().UseColor)
     PDC.forceColors();

test/FixCode/Inputs/batch-mode-helper.swift

@@ -0,0 +1,10 @@
+enum E2 {
+  case x
+  case y
+  case z
+}
+
+func fooHelper(_ e: E2) {
+  switch e {
+  }
+}

test/FixCode/batch-mode.swift

@@ -0,0 +1,24 @@
+// RUN: not %swift -typecheck -target %target-triple -primary-file %s  -emit-fixits-path %t.main.remap -primary-file %S/Inputs/batch-mode-helper.swift -emit-fixits-path %t.helper.remap -diagnostics-editor-mode
+// RUN: %FileCheck -check-prefix=CHECK-MAIN %s < %t.main.remap
+// RUN: %FileCheck -check-prefix=NEGATIVE-MAIN %s < %t.main.remap
+// RUN: %FileCheck -check-prefix=CHECK-HELPER %s < %t.helper.remap
+// RUN: %FileCheck -check-prefix=NEGATIVE-HELPER %s < %t.helper.remap
+
+// CHECK-MAIN: "file": "{{.+}}batch-mode.swift"
+// CHECK-MAIN: "text": "case .a:\n<#code#>\ncase .b:\n<#code#>\ncase .c:\n<#code#>\n"
+// NEGATIVE-MAIN-NOT: batch-mode-helper.swift
+
+// CHECK-HELPER: "file": "{{.+}}batch-mode-helper.swift"
+// CHECK-HELPER: "text": "case .x:\n<#code#>\ncase .y:\n<#code#>\ncase .z:\n<#code#>\n"
+// NEGATIVE-HELPER-NOT: batch-mode.swift
+
+enum E1 {
+  case a
+  case b
+  case c
+}
+
+func fooMain(_ e: E1) {
+  switch e {
+  }
+}