When converting some of the old Migrator automation to the new Migrator,

I had set up the driver to invoke a separate frontend invocation with
the "update code" mode. We sort of did this last release, except we
forked to the swift-update binary instead. This is causing problems with
testing in Xcode.

Instead, let's perform a single compile and add the remap file as an
additional output during normal compiles. The driver, seeing
-update-code, will add -emit-remap-file-path $PATH to the -c frontend
invocation.

rdar://problem/31857580

Author: bitjammer

include/swift/Driver/Driver.h

@@ -94,7 +94,7 @@ class OutputInfo {
 
   /// Whether the compiler picked the current module name, rather than the user.
   bool ModuleNameIsFallback = false;
-  
+
   /// The number of threads for multi-threaded compilation.
   unsigned numThreads = 0;
 

include/swift/Frontend/FrontendOptions.h

@@ -172,8 +172,6 @@ class FrontendOptions {
     EmitIR, ///< Emit LLVM IR
     EmitBC, ///< Emit LLVM BC
     EmitObject, ///< Emit object file
-
-    UpdateCode, ///< Update Swift code
   };
 
   /// Indicates the action the user requested that the frontend perform.

include/swift/Migrator/Migrator.h

@@ -27,15 +27,18 @@ namespace migrator {
 
 /// Run the migrator on the compiler invocation's input file and emit a
 /// "replacement map" describing the requested changes to the source file.
-bool updateCodeAndEmitRemap(const CompilerInvocation &Invocation);
+bool updateCodeAndEmitRemap(CompilerInstance &Instance,
+                            const CompilerInvocation &Invocation);
 
 class Migrator {
+  CompilerInstance &StartInstance;
   const CompilerInvocation &StartInvocation;
   SourceManager SrcMgr;
   std::vector<RC<MigrationState>> States;
 
 public:
-  Migrator(const CompilerInvocation &StartInvocation);
+  Migrator(CompilerInstance &StartInstance,
+           const CompilerInvocation &StartInvocation);
 
   /// The maximum number of times to run the compiler over the input to get
   /// fix-its. Nullability changes may expose other fix-its in subsequent

include/swift/Migrator/MigratorOptions.h

@@ -31,6 +31,11 @@ struct MigratorOptions {
   /// Whether to print each USR we query the api change data store about.
   bool DumpUsr = false;
 
+  /// If non-empty, print a replacement map describing changes to get from
+  /// the first MigrationState's output text to the last MigrationState's
+  /// output text.
+  std::string EmitRemapFilePath = "";
+
   /// If non-empty, print the last MigrationState's output text to the given
   /// file path.
   std::string EmitMigratedFilePath = "";
@@ -40,6 +45,11 @@ struct MigratorOptions {
 
   /// If non-empty, use the api change data serialized to this path.
   std::string APIDigesterDataStorePath = "";
+
+  bool shouldRunMigrator() const {
+    return !(EmitRemapFilePath.empty() && EmitMigratedFilePath.empty() &&
+             DumpMigrationStatesDir.empty());
+  }
 };
 
 } // end namespace swift

include/swift/Option/Options.td

@@ -415,10 +415,20 @@ def line_range : Separate<["-"], "line-range">, Group<code_formatting_Group>,
            "Can only be used with one input file.">, MetaVarName<"<n:n>">;
 
 // Migrator Options
+
+def update_code : Flag<["-"], "update-code">,
+  HelpText<"Update Swift code">,
+  Flags<[FrontendOption, HelpHidden, NoInteractiveOption, DoesNotAffectIncrementalBuild]>;
+
 def disable_migrator_fixits: Flag<["-"], "disable-migrator-fixits">,
   Flags<[FrontendOption, NoInteractiveOption]>,
   HelpText<"Disable the Migrator phase which automatically applies fix-its">;
 
+def emit_remap_file_path: Separate<["-"], "emit-remap-file-path">,
+  Flags<[FrontendOption, NoDriverOption, NoInteractiveOption, DoesNotAffectIncrementalBuild]>,
+  HelpText<"Emit the replacement map describing Swift Migrator changes to <path>">,
+  MetaVarName<"<path>">;
+
 def emit_migrated_file_path: Separate<["-"], "emit-migrated-file-path">,
   Flags<[FrontendOption, NoDriverOption, NoInteractiveOption, DoesNotAffectIncrementalBuild]>,
   HelpText<"Emit the migrated source file to <path>">,
@@ -508,10 +518,6 @@ def c : Flag<["-"], "c">, Alias<emit_object>,
 def S: Flag<["-"], "S">, Alias<emit_assembly>,
   Flags<[FrontendOption, NoInteractiveOption]>, ModeOpt;
 
-def update_code : Flag<["-"], "update-code">,
-  HelpText<"Update Swift code">, ModeOpt,
-  Flags<[FrontendOption, HelpHidden, NoInteractiveOption, DoesNotAffectIncrementalBuild]>;
-
 def fixit_all : Flag<["-"], "fixit-all">,
   HelpText<"Apply all fixits from diagnostics without any filtering">,
   Flags<[FrontendOption, NoInteractiveOption, DoesNotAffectIncrementalBuild]>;

lib/Driver/Driver.cpp

@@ -494,8 +494,6 @@ std::unique_ptr<Compilation> Driver::buildCompilation(
     ArgList->hasArg(options::OPT_driver_show_incremental);
   bool ShowJobLifecycle =
     ArgList->hasArg(options::OPT_driver_show_job_lifecycle);
-  bool UpdateCode =
-    ArgList->hasArg(options::OPT_update_code);
 
   bool Incremental = ArgList->hasArg(options::OPT_incremental);
   if (ArgList->hasArg(options::OPT_whole_module_optimization)) {
@@ -671,10 +669,9 @@ std::unique_ptr<Compilation> Driver::buildCompilation(
 
   buildJobs(Actions, OI, OFM.get(), *TC, *C);
 
-  // For updating code we need to go through all the files and pick up changes,
-  // even if they have compiler errors. Also for getting bulk fixits, or for when
-  // users explicitly request to continue building despite errors.
-  if (UpdateCode || ContinueBuildingAfterErrors)
+  // For getting bulk fixits, or for when users explicitly request to continue
+  // building despite errors.
+  if (ContinueBuildingAfterErrors)
     C->setContinueBuildingAfterErrors();
 
   if (ShowIncrementalBuildDecisions || ShowJobLifecycle)
@@ -1126,11 +1123,6 @@ void Driver::buildOutputInfo(const ToolChain &TC, const DerivedArgList &Args,
       OI.CompilerMode = OutputInfo::Mode::SingleCompile;
       break;
 
-    case options::OPT_update_code:
-      OI.CompilerOutputType = types::TY_Remapping;
-      OI.LinkAction = LinkKind::None;
-      break;
-
     case options::OPT_parse:
     case options::OPT_typecheck:
     case options::OPT_dump_parse:
@@ -2068,6 +2060,27 @@ Job *Driver::buildJobsForAction(Compilation &C, const JobAction *JA,
     }
   }
 
+  if (C.getArgs().hasArg(options::OPT_update_code) &&
+      isa<CompileJobAction>(JA)) {
+    StringRef OFMFixitsOutputPath;
+    if (OutputMap) {
+      auto iter = OutputMap->find(types::TY_Remapping);
+      if (iter != OutputMap->end())
+        OFMFixitsOutputPath = iter->second;
+    }
+    if (!OFMFixitsOutputPath.empty()) {
+      Output->setAdditionalOutputForType(types::ID::TY_Remapping,
+                                         OFMFixitsOutputPath);
+    } else {
+      llvm::SmallString<128> Path(Output->getPrimaryOutputFilenames()[0]);
+      bool isTempFile = C.isTemporaryFile(Path);
+      llvm::sys::path::replace_extension(Path, "remap");
+      Output->setAdditionalOutputForType(types::ID::TY_Remapping, Path);
+      if (isTempFile)
+        C.addTemporaryFile(Path);
+    }
+  }
+
   if (isa<CompileJobAction>(JA) || isa<GeneratePCHJobAction>(JA)) {
     // Choose the serialized diagnostics output path.
     if (C.getArgs().hasArg(options::OPT_serialize_diagnostics)) {

lib/Driver/ToolChains.cpp

@@ -402,7 +402,7 @@ ToolChain::constructInvocation(const CompileJobAction &job,
   const std::string &FixitsPath =
     context.Output.getAdditionalOutputForType(types::TY_Remapping);
   if (!FixitsPath.empty()) {
-    Arguments.push_back("-emit-fixits-path");
+    Arguments.push_back("-emit-remap-file-path");
     Arguments.push_back(FixitsPath.c_str());
   }
 

lib/Frontend/CompilerInvocation.cpp

@@ -323,8 +323,6 @@ static bool ParseFrontendArgs(FrontendOptions &Opts, ArgList &Args,
       Action = FrontendOptions::REPL;
     } else if (Opt.matches(OPT_interpret)) {
       Action = FrontendOptions::Immediate;
-    } else if (Opt.matches(OPT_update_code)) {
-      Action = FrontendOptions::UpdateCode;
     } else {
       llvm_unreachable("Unhandled mode option");
     }
@@ -562,10 +560,6 @@ static bool ParseFrontendArgs(FrontendOptions &Opts, ArgList &Args,
         Suffix = "importedmodules";
       break;
 
-    case FrontendOptions::UpdateCode:
-      Suffix = REMAP_EXTENSION;
-      break;
-
     case FrontendOptions::EmitTBD:
       if (Opts.OutputFilenames.empty())
         Opts.setSingleOutputFilename("-");
@@ -716,7 +710,6 @@ static bool ParseFrontendArgs(FrontendOptions &Opts, ArgList &Args,
     case FrontendOptions::DumpTypeRefinementContexts:
     case FrontendOptions::Immediate:
     case FrontendOptions::REPL:
-    case FrontendOptions::UpdateCode:
       Diags.diagnose(SourceLoc(), diag::error_mode_cannot_emit_dependencies);
       return true;
     case FrontendOptions::Parse:
@@ -749,7 +742,6 @@ static bool ParseFrontendArgs(FrontendOptions &Opts, ArgList &Args,
     case FrontendOptions::DumpTypeRefinementContexts:
     case FrontendOptions::Immediate:
     case FrontendOptions::REPL:
-    case FrontendOptions::UpdateCode:
       Diags.diagnose(SourceLoc(), diag::error_mode_cannot_emit_header);
       return true;
     case FrontendOptions::Parse:
@@ -781,7 +773,6 @@ static bool ParseFrontendArgs(FrontendOptions &Opts, ArgList &Args,
     case FrontendOptions::DumpTypeRefinementContexts:
     case FrontendOptions::Immediate:
     case FrontendOptions::REPL:
-    case FrontendOptions::UpdateCode:
       Diags.diagnose(SourceLoc(),
                      diag::error_mode_cannot_emit_loaded_module_trace);
       return true;
@@ -818,7 +809,6 @@ static bool ParseFrontendArgs(FrontendOptions &Opts, ArgList &Args,
     case FrontendOptions::EmitSILGen:
     case FrontendOptions::Immediate:
     case FrontendOptions::REPL:
-    case FrontendOptions::UpdateCode:
       if (!Opts.ModuleOutputPath.empty())
         Diags.diagnose(SourceLoc(), diag::error_mode_cannot_emit_module);
       else
@@ -1598,6 +1588,10 @@ bool ParseMigratorArgs(MigratorOptions &Opts, llvm::Triple &Triple,
     Opts.EnableMigratorFixits = false;
   }
 
+  if (auto RemapFilePath = Args.getLastArg(OPT_emit_remap_file_path)) {
+    Opts.EmitRemapFilePath = RemapFilePath->getValue();
+  }
+
   if (auto MigratedFilePath = Args.getLastArg(OPT_emit_migrated_file_path)) {
     Opts.EmitMigratedFilePath = MigratedFilePath->getValue();
   }
@@ -1625,6 +1619,7 @@ bool ParseMigratorArgs(MigratorOptions &Opts, llvm::Triple &Triple,
     if (Supported)
       Opts.APIDigesterDataStorePath = dataPath.str();
   }
+
   return false;
 }
 

lib/Frontend/FrontendOptions.cpp

@@ -34,7 +34,6 @@ bool FrontendOptions::actionHasOutput() const {
   case EmitSIBGen:
   case EmitSIB:
   case EmitModuleOnly:
-  case UpdateCode:
     return true;
   case Immediate:
   case REPL:
@@ -67,7 +66,6 @@ bool FrontendOptions::actionIsImmediate() const {
   case EmitSIBGen:
   case EmitSIB:
   case EmitModuleOnly:
-  case UpdateCode:
     return false;
   case Immediate:
   case REPL:

lib/FrontendTool/FrontendTool.cpp

@@ -460,16 +460,17 @@ static bool performCompile(std::unique_ptr<CompilerInstance> &Instance,
 
   ASTContext &Context = Instance->getASTContext();
 
+  if (!Context.hadError() &&
+      Invocation.getMigratorOptions().shouldRunMigrator()) {
+    migrator::updateCodeAndEmitRemap(*Instance, Invocation);
+  }
+
   if (Action == FrontendOptions::REPL) {
     runREPL(*Instance, ProcessCmdLine(Args.begin(), Args.end()),
             Invocation.getParseStdlib());
     return Context.hadError();
   }
 
-  if (Action == FrontendOptions::UpdateCode) {
-    return migrator::updateCodeAndEmitRemap(Invocation);
-  }
-
   SourceFile *PrimarySourceFile = Instance->getPrimarySourceFile();
 
   // We've been told to dump the AST (either after parsing or type-checking,