Address Jordan's code review comments

Author: nkcsgexi

include/swift/Driver/Driver.h

@@ -24,7 +24,6 @@
 #include "swift/Basic/OutputFileMap.h"
 #include "swift/Basic/Sanitizers.h"
 #include "swift/Driver/Util.h"
-#include "swift/Option/Options.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
@@ -355,12 +354,10 @@ class Driver {
                                       StringRef workingDirectory,
                                       CommandOutput *Output) const;
 
-  void choosePrivateOutputFilePath(Compilation &C,
-                                   const TypeToPathMap *OutputMap,
-                                   StringRef workingDirectory,
-                                   CommandOutput *Output,
-                                   file_types::ID fileID,
-                                   Optional<options::ID> DriverOptForPath = llvm::None) const;
+  void chooseSwiftSourceInfoOutputPath(Compilation &C,
+                                       const TypeToPathMap *OutputMap,
+                                       StringRef workingDirectory,
+                                       CommandOutput *Output) const;
 
   void chooseModuleInterfacePath(Compilation &C, const JobAction *JA,
                                  StringRef workingDirectory,

lib/Driver/Driver.cpp

@@ -2544,9 +2544,7 @@ Job *Driver::buildJobsForAction(Compilation &C, const JobAction *JA,
   if (OI.ShouldGenerateModule &&
       (isa<CompileJobAction>(JA) || isa<MergeModuleJobAction>(JA))) {
     chooseSwiftModuleDocOutputPath(C, OutputMap, workingDirectory, Output.get());
-    choosePrivateOutputFilePath(C, OutputMap, workingDirectory, Output.get(),
-                                file_types::TY_SwiftSourceInfoFile,
-                                options::OPT_emit_module_source_info_path);
+    chooseSwiftSourceInfoOutputPath(C, OutputMap, workingDirectory, Output.get());
   }
 
   if (C.getArgs().hasArg(options::OPT_emit_module_interface,
@@ -2790,12 +2788,13 @@ void Driver::chooseSwiftModuleOutputPath(Compilation &C,
   }
 }
 
-void Driver::choosePrivateOutputFilePath(Compilation &C,
-                                         const TypeToPathMap *OutputMap,
-                                         StringRef workingDirectory,
-                                         CommandOutput *Output,
-                                         file_types::ID fileID,
-                                         Optional<options::ID> optId) const {
+static void chooseModuleAuxiliaryOutputFilePath(Compilation &C,
+                                                const TypeToPathMap *OutputMap,
+                                                StringRef workingDirectory,
+                                                CommandOutput *Output,
+                                                file_types::ID fileID,
+                                                bool isPrivate,
+                                                Optional<options::ID> optId = llvm::None) {
   if (hasExistingAdditionalOutput(*Output, fileID))
     return;
   // Honor driver option for this path if it's given
@@ -2820,10 +2819,12 @@ void Driver::choosePrivateOutputFilePath(Compilation &C,
     bool isTempFile = C.isTemporaryFile(ModulePath);
     auto ModuleName = llvm::sys::path::filename(ModulePath);
     llvm::SmallString<128> Path(llvm::sys::path::parent_path(ModulePath));
-    llvm::sys::path::append(Path, "Private");
-    // If the build system has created a Private dir for us to include the file, use it.
-    if (!llvm::sys::fs::exists(Path)) {
-      llvm::sys::path::remove_filename(Path);
+    if (isPrivate) {
+      llvm::sys::path::append(Path, "Private");
+      // If the build system has created a Private dir for us to include the file, use it.
+      if (!llvm::sys::fs::exists(Path)) {
+        llvm::sys::path::remove_filename(Path);
+      }
     }
     llvm::sys::path::append(Path, ModuleName);
     llvm::sys::path::replace_extension(Path, file_types::getExtension(fileID));
@@ -2833,35 +2834,21 @@ void Driver::choosePrivateOutputFilePath(Compilation &C,
   }
 }
 
+void Driver::chooseSwiftSourceInfoOutputPath(Compilation &C,
+                                             const TypeToPathMap *OutputMap,
+                                             StringRef workingDirectory,
+                                             CommandOutput *Output) const {
+  chooseModuleAuxiliaryOutputFilePath(C, OutputMap, workingDirectory, Output,
+                                      file_types::TY_SwiftSourceInfoFile,
+                                      /*isPrivate*/true, options::OPT_emit_module_source_info_path);
+}
+
 void Driver::chooseSwiftModuleDocOutputPath(Compilation &C,
                                             const TypeToPathMap *OutputMap,
                                             StringRef workingDirectory,
                                             CommandOutput *Output) const {
-
-  if (hasExistingAdditionalOutput(*Output, file_types::TY_SwiftModuleDocFile))
-    return;
-
-  StringRef OFMModuleDocOutputPath;
-  if (OutputMap) {
-    auto iter = OutputMap->find(file_types::TY_SwiftModuleDocFile);
-    if (iter != OutputMap->end())
-      OFMModuleDocOutputPath = iter->second;
-  }
-  if (!OFMModuleDocOutputPath.empty()) {
-    // Prefer a path from the OutputMap.
-    Output->setAdditionalOutputForType(file_types::TY_SwiftModuleDocFile,
-                                       OFMModuleDocOutputPath);
-  } else if (Output->getPrimaryOutputType() != file_types::TY_Nothing) {
-    // Otherwise, put it next to the swiftmodule file.
-    llvm::SmallString<128> Path(
-        Output->getAnyOutputForType(file_types::TY_SwiftModuleFile));
-    bool isTempFile = C.isTemporaryFile(Path);
-    llvm::sys::path::replace_extension(
-        Path, file_types::getExtension(file_types::TY_SwiftModuleDocFile));
-    Output->setAdditionalOutputForType(file_types::TY_SwiftModuleDocFile, Path);
-    if (isTempFile)
-      C.addTemporaryFile(Path);
-  }
+  chooseModuleAuxiliaryOutputFilePath(C, OutputMap, workingDirectory, Output,
+                                      file_types::TY_SwiftModuleDocFile, /*isPrivate*/false);
 }
 
 void Driver::chooseRemappingOutputPath(Compilation &C,

lib/Driver/ToolChains.cpp

@@ -913,7 +913,8 @@ ToolChain::constructInvocation(const MergeModuleJobAction &job,
   addOutputsOfType(Arguments, context.Output, context.Args,
                    file_types::TY_SwiftModuleDocFile, "-emit-module-doc-path");
   addOutputsOfType(Arguments, context.Output, context.Args,
-                   file_types::TY_SwiftSourceInfoFile, "-emit-module-source-info-path");
+                   file_types::TY_SwiftSourceInfoFile,
+                   "-emit-module-source-info-path");
   addOutputsOfType(Arguments, context.Output, context.Args,
                    file_types::ID::TY_SwiftModuleInterfaceFile,
                    "-emit-module-interface-path");

lib/Frontend/FrontendInputsAndOutputs.cpp

@@ -437,9 +437,9 @@ bool FrontendInputsAndOutputs::hasModuleDocOutputPath() const {
 }
 bool FrontendInputsAndOutputs::hasModuleSourceInfoOutputPath() const {
   return hasSupplementaryOutputPath(
-  [](const SupplementaryOutputPaths &outs) -> const std::string & {
-    return outs.ModuleSourceInfoOutputPath;
-  });
+      [](const SupplementaryOutputPaths &outs) -> const std::string & {
+        return outs.ModuleSourceInfoOutputPath;
+      });
 }
 bool FrontendInputsAndOutputs::hasModuleInterfaceOutputPath() const {
   return hasSupplementaryOutputPath(