[Driver] Better separation of concerns in the Tool hierarchy.

No intended functionality change.

Swift SVN r29666

include/swift/Driver/Job.h

@@ -149,10 +149,10 @@ private:
 public:
   Job(const Action &Source, const Tool &Creator,
       std::unique_ptr<JobList> Inputs, std::unique_ptr<CommandOutput> Output,
-      const char *Executable, llvm::opt::ArgStringList &Arguments)
+      const char *Executable, llvm::opt::ArgStringList Arguments)
       : Source(Source), CreatorAndCondition(&Creator, Condition::Always),
         Inputs(std::move(Inputs)), Output(std::move(Output)),
-        Executable(Executable), Arguments(Arguments) {}
+        Executable(Executable), Arguments(std::move(Arguments)) {}
 
   const Action &getSource() const { return Source; }
   const Tool &getCreator() const { return *CreatorAndCondition.getPointer(); }

include/swift/Driver/Tool.h

@@ -15,6 +15,7 @@
 
 #include "swift/Basic/LLVM.h"
 #include "swift/Driver/Util.h"
+#include "llvm/Option/Option.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 
@@ -70,11 +71,23 @@ public:
   /// \param Args The argument list for this tool chain.
   /// \param OI information about the output which the driver will create,
   /// which may influence the creation of this Job.
-  virtual Job *constructJob(const JobAction &JA,
+  std::unique_ptr<Job> constructJob(const JobAction &JA,
                                     std::unique_ptr<JobList> Inputs,
                                     std::unique_ptr<CommandOutput> Output,
                                     const ActionList &InputActions,
                                     const llvm::opt::ArgList &Args,
+                                    const OutputInfo &OI) const;
+
+protected:
+  virtual llvm::opt::ArgStringList
+  constructArgumentList(const JobAction &JA,
+                        const JobList *Inputs,
+                        const CommandOutput *Output,
+                        const ActionList &InputActions,
+                        const llvm::opt::ArgList &Args,
+                        const OutputInfo &OI) const = 0;
+
+  virtual const char *getPath(const llvm::opt::ArgList &Args,
                               const OutputInfo &OI) const = 0;
 };
 } // end namespace driver

lib/Driver/Driver.cpp

@@ -1828,20 +1828,21 @@ Job *Driver::buildJobsForAction(Compilation &C, const Action *A,
   }
 
   // 5. Construct a Job which produces the right CommandOutput.
-  Job *J = T->constructJob(*JA, std::move(InputJobs), std::move(Output),
+  std::unique_ptr<Job> J = T->constructJob(*JA, std::move(InputJobs),
+                                           std::move(Output),
                                            InputActions, C.getArgs(), OI);
 
   // If we track dependencies for this job, we may be able to avoid running it.
   if (!J->getOutput().getAdditionalOutputForType(types::TY_SwiftDeps).empty()) {
     if (InputActions.size() == 1) {
       auto compileJob = cast<CompileJobAction>(A);
-      handleCompileJobCondition(J, compileJob->getInputInfo(), BaseInput);
+      handleCompileJobCondition(J.get(), compileJob->getInputInfo(), BaseInput);
     }
   }
 
   // 6. Add it to the JobCache, so we don't construct the same Job multiple
   // times.
-  JobCache[Key] = J;
+  JobCache[Key] = J.get();
 
   if (DriverPrintBindings) {
     llvm::outs() << "# \"" << T->getToolChain().getTripleString()
@@ -1876,7 +1877,7 @@ Job *Driver::buildJobsForAction(Compilation &C, const Action *A,
                },
                [] { llvm::outs() << ", "; });
 
-    types::forAllTypes([J](types::ID Ty) {
+    types::forAllTypes([&J](types::ID Ty) {
       StringRef AdditionalOutput =
         J->getOutput().getAdditionalOutputForType(Ty);
       if (!AdditionalOutput.empty()) {
@@ -1903,7 +1904,7 @@ Job *Driver::buildJobsForAction(Compilation &C, const Action *A,
     llvm::outs() << '\n';
   }
 
-  return J;
+  return J.release();
 }
 
 static unsigned printActions(const Action *A,

lib/Driver/Tool.cpp

@@ -11,3 +11,22 @@
 //===----------------------------------------------------------------------===//
 
 #include "swift/Driver/Tool.h"
+#include "swift/Driver/Action.h"
+#include "swift/Driver/Job.h"
+#include "llvm/ADT/STLExtras.h"
+
+using namespace swift;
+using namespace swift::driver;
+
+std::unique_ptr<Job>
+Tool::constructJob(const JobAction &JA,
+                   std::unique_ptr<JobList> inputs,
+                   std::unique_ptr<CommandOutput> output,
+                   const ActionList &inputActions,
+                   const llvm::opt::ArgList &args,
+                   const OutputInfo &OI) const {
+  auto arguments = constructArgumentList(JA, inputs.get(), output.get(),
+                                         inputActions, args, OI);
+  return llvm::make_unique<Job>(JA, *this, std::move(inputs), std::move(output),
+                                getPath(args, OI), std::move(arguments));
+}

lib/Driver/Tools.cpp

@@ -84,7 +84,7 @@ static void addPrimaryInputsOfType(ArgStringList &Arguments,
 /// module-merging, LLDB's REPL, etc).
 static void addCommonFrontendArgs(const ToolChain &TC,
                                   const OutputInfo &OI,
-                                  CommandOutput *output,
+                                  const CommandOutput *output,
                                   const ArgList &inputArgs,
                                   ArgStringList &arguments) {
   arguments.push_back("-target");
@@ -160,31 +160,35 @@ static void addCommonFrontendArgs(const ToolChain &TC,
 /// \returns the executable name.
 static const char *setupSwiftFrontendOrSwiftUpdate(const Tool &Tool,
                                                    const ArgList &Args,
-                                                   const OutputInfo &OI,
+                                                   const OutputInfo &OI) {
-                                                   ArgStringList &Arguments) {
+  const std::string &Exec =
-  const char *Exec =
+      Tool.getToolChain().getDriver().getSwiftProgramPath();
-    Tool.getToolChain().getDriver().getSwiftProgramPath().c_str();
 
   if (OI.CompilerMode == OutputInfo::Mode::UpdateCode) {
     SmallString<128> SwiftUpdatePath = llvm::sys::path::parent_path(Exec);
     llvm::sys::path::append(SwiftUpdatePath, "swift-update");
-    Exec = Args.MakeArgString(SwiftUpdatePath.str());
+    return Args.MakeArgString(SwiftUpdatePath.str());
-  } else {
-    // Invoke ourselves in -frontend mode.
-    Arguments.push_back("-frontend");
   }
 
-  return Exec;
+  return Exec.c_str();
 }
 
-Job *Swift::constructJob(const JobAction &JA, std::unique_ptr<JobList> Inputs,
+const char *Swift::getPath(const llvm::opt::ArgList &Args,
-                         std::unique_ptr<CommandOutput> Output,
+                           const OutputInfo &OI) const {
-                         const ActionList &InputActions, const ArgList &Args,
+  return setupSwiftFrontendOrSwiftUpdate(*this, Args, OI);
+}
+
+llvm::opt::ArgStringList
+Swift::constructArgumentList(const JobAction &JA,
+                             const JobList *Inputs,
+                             const CommandOutput *Output,
+                             const ActionList &InputActions,
+                             const llvm::opt::ArgList &Args,
                              const OutputInfo &OI) const {
   ArgStringList Arguments;
 
-  const char *Exec = setupSwiftFrontendOrSwiftUpdate(*this, Args, OI,
+  if (OI.CompilerMode != OutputInfo::Mode::UpdateCode)
-                                                     Arguments);
+    Arguments.push_back("-frontend");
 
   // Determine the frontend mode option.
   const char *FrontendModeOption = nullptr;
@@ -365,14 +369,13 @@ Job *Swift::constructJob(const JobAction &JA, std::unique_ptr<JobList> Inputs,
     // Disable all llvm IR level optimizations.
     Arguments.push_back("-disable-llvm-optzns");
 
-    return new Job(JA, *this, std::move(Inputs), std::move(Output), Exec,
+    return Arguments;
-                   Arguments);
   }
 
   if (Args.hasArg(options::OPT_parse_stdlib))
     Arguments.push_back("-disable-objc-attr-requires-foundation-module");
 
-  addCommonFrontendArgs(getToolChain(), OI, Output.get(), Args, Arguments);
+  addCommonFrontendArgs(getToolChain(), OI, Output, Args, Arguments);
 
   // Pass the optimization level down to the frontend.
   Args.AddLastArg(Arguments, options::OPT_O_Group);
@@ -464,21 +467,25 @@ Job *Swift::constructJob(const JobAction &JA, std::unique_ptr<JobList> Inputs,
   if (Args.hasArg(options::OPT_embed_bitcode_marker))
     Arguments.push_back("-embed-bitcode-marker");
 
-  return new Job(JA, *this, std::move(Inputs), std::move(Output), Exec,
+  return Arguments;
-                 Arguments);
 }
 
+const char *MergeModule::getPath(const llvm::opt::ArgList &Args,
+                                 const OutputInfo &OI) const {
+  return setupSwiftFrontendOrSwiftUpdate(*this, Args, OI);
+}
 
-Job *MergeModule::constructJob(const JobAction &JA,
+llvm::opt::ArgStringList
-                               std::unique_ptr<JobList> Inputs,
+MergeModule::constructArgumentList(const JobAction &JA,
-                               std::unique_ptr<CommandOutput> Output,
+                                   const JobList *Inputs,
+                                   const CommandOutput *Output,
                                    const ActionList &InputActions,
-                               const ArgList &Args,
+                                   const llvm::opt::ArgList &Args,
                                    const OutputInfo &OI) const {
   ArgStringList Arguments;
 
-  const char *Exec = setupSwiftFrontendOrSwiftUpdate(*this, Args, OI,
+  if (OI.CompilerMode != OutputInfo::Mode::UpdateCode)
-                                                     Arguments);
+    Arguments.push_back("-frontend");
 
   // We just want to emit a module, so pass -emit-module without any other
   // mode options.
@@ -497,7 +504,7 @@ Job *MergeModule::constructJob(const JobAction &JA,
   // serialized ASTs.
   Arguments.push_back("-parse-as-library");
 
-  addCommonFrontendArgs(getToolChain(), OI, Output.get(), Args, Arguments);
+  addCommonFrontendArgs(getToolChain(), OI, Output, Args, Arguments);
 
   Arguments.push_back("-module-name");
   Arguments.push_back(Args.MakeArgString(OI.ModuleName));
@@ -515,11 +522,18 @@ Job *MergeModule::constructJob(const JobAction &JA,
   Arguments.push_back("-o");
   Arguments.push_back(Args.MakeArgString(Output->getPrimaryOutputFilename()));
 
-  return new Job(JA, *this, std::move(Inputs), std::move(Output), Exec,
+  return Arguments;
-                 Arguments);
 }
 
-const char *ToolchainTool::getPath() const {
+const char *ToolchainTool::getPath(const ArgList &Args,
+                                   const OutputInfo &OI) const {
+  if (!Bits.DidCheckRelativeToDriver) {
+    (void)isPresentRelativeToDriver();
+  }
+  return NameOrPath.c_str();
+}
+
+bool ToolchainTool::isPresentRelativeToDriver() const {
   if (!Bits.DidCheckRelativeToDriver) {
     const Driver &D = getToolChain().getDriver();
     std::string RelativePath =
@@ -532,28 +546,22 @@ const char *ToolchainTool::getPath() const {
     }
     Bits.DidCheckRelativeToDriver = true;
   }
-  return NameOrPath.c_str();
-}
-
-bool ToolchainTool::isPresentRelativeToDriver() const {
-  if (!Bits.DidCheckRelativeToDriver) {
-    (void)getPath();
-  }
   return Bits.IsPresentRelativeToDriver;
 }
 
-Job *LLDB::constructJob(const JobAction &JA,
+llvm::opt::ArgStringList
-                        std::unique_ptr<JobList> Inputs,
+LLDB::constructArgumentList(const JobAction &JA,
-                        std::unique_ptr<CommandOutput> Output,
+                            const JobList *Inputs,
+                            const CommandOutput *Output,
                             const ActionList &InputActions,
-                        const ArgList &Args,
+                            const llvm::opt::ArgList &Args,
                             const OutputInfo &OI) const {
   assert(Inputs->empty());
   assert(InputActions.empty());
 
   // Squash important frontend options into a single argument for LLDB.
   ArgStringList FrontendArgs;
-  addCommonFrontendArgs(getToolChain(), OI, Output.get(), Args, FrontendArgs);
+  addCommonFrontendArgs(getToolChain(), OI, Output, Args, FrontendArgs);
   Args.AddAllArgs(FrontendArgs, options::OPT_l, options::OPT_framework,
                   options::OPT_L);
 
@@ -566,15 +574,15 @@ Job *LLDB::constructJob(const JobAction &JA,
   ArgStringList Arguments;
   Arguments.push_back(Args.MakeArgString(std::move(SingleArg)));
 
-  return new Job(JA, *this, std::move(Inputs), std::move(Output), getPath(),
+  return Arguments;
-                 Arguments);
 }
 
-Job *Dsymutil::constructJob(const JobAction &JA,
+llvm::opt::ArgStringList
-                            std::unique_ptr<JobList> Inputs,
+Dsymutil::constructArgumentList(const JobAction &JA,
-                            std::unique_ptr<CommandOutput> Output,
+                                const JobList *Inputs,
+                                const CommandOutput *Output,
                                 const ActionList &InputActions,
-                            const ArgList &Args,
+                                const llvm::opt::ArgList &Args,
                                 const OutputInfo &OI) const {
   assert(Inputs->size() == 1);
   assert(InputActions.empty());
@@ -588,15 +596,15 @@ Job *Dsymutil::constructJob(const JobAction &JA,
   Arguments.push_back("-o");
   Arguments.push_back(Args.MakeArgString(Output->getPrimaryOutputFilename()));
 
-  return new Job(JA, *this, std::move(Inputs), std::move(Output), getPath(),
+  return Arguments;
-                 Arguments);
 }
 
-Job *AutolinkExtract::constructJob(const JobAction &JA,
+llvm::opt::ArgStringList
-                                   std::unique_ptr<JobList> Inputs,
+AutolinkExtract::constructArgumentList(const JobAction &JA,
-                                   std::unique_ptr<CommandOutput> Output,
+                                       const JobList *Inputs,
+                                       const CommandOutput *Output,
                                        const ActionList &InputActions,
-                                   const ArgList &Args,
+                                       const llvm::opt::ArgList &Args,
                                        const OutputInfo &OI) const {
   assert(Output->getPrimaryOutputType() == types::TY_AutolinkFile);
 
@@ -607,8 +615,7 @@ Job *AutolinkExtract::constructJob(const JobAction &JA,
   Arguments.push_back("-o");
   Arguments.push_back(Args.MakeArgString(Output->getPrimaryOutputFilename()));
 
-  return new Job(JA, *this, std::move(Inputs), std::move(Output), getPath(),
+  return Arguments;
-                 Arguments);
 }
 
 /// Darwin Tools
@@ -638,11 +645,12 @@ static void addVersionString(const ArgList &inputArgs, ArgStringList &arguments,
   arguments.push_back(inputArgs.MakeArgString(os.str()));
 }
 
-Job *darwin::Linker::constructJob(const JobAction &JA,
+llvm::opt::ArgStringList
-                                  std::unique_ptr<JobList> Inputs,
+darwin::Linker::constructArgumentList(const JobAction &JA,
-                                  std::unique_ptr<CommandOutput> Output,
+                                      const JobList *Inputs,
+                                      const CommandOutput *Output,
                                       const ActionList &InputActions,
-                                  const ArgList &Args,
+                                      const llvm::opt::ArgList &Args,
                                       const OutputInfo &OI) const {
   assert(Output->getPrimaryOutputType() == types::TY_Image &&
          "Invalid linker output type.");
@@ -822,19 +830,27 @@ Job *darwin::Linker::constructJob(const JobAction &JA,
   Arguments.push_back("-o");
   Arguments.push_back(Output->getPrimaryOutputFilename().c_str());
 
-  return new Job(JA, *this, std::move(Inputs), std::move(Output), getPath(),
+  return Arguments;
-                 Arguments);
 }
 
 #if defined(SWIFT_ENABLE_TARGET_LINUX)
 
 /// Linux Tools
 
-Job *linux::Linker::constructJob(const JobAction &JA,
+const char *linux::Linker::getPath(const llvm::opt::ArgList &Args,
-                                 std::unique_ptr<JobList> Inputs,
+                                   const OutputInfo &OI) const {
-                                 std::unique_ptr<CommandOutput> Output,
+  // This avoids a bunch of issues trying to correctly guess parameters
+  // for ld for linux platforms. We know we have clang, we know it should
+  // be able to link, so use clang for now.
+  return Args.MakeArgString(getToolChain().getProgramPath("clang++"));
+}
+
+llvm::opt::ArgStringList
+linux::Linker::constructArgumentList(const JobAction &JA,
+                                   const JobList *Inputs,
+                                   const CommandOutput *Output,
                                    const ActionList &InputActions,
-                                 const ArgList &Args,
+                                   const llvm::opt::ArgList &Args,
                                    const OutputInfo &OI) const {
   const ToolChain &TC = getToolChain();
   const Driver &D = TC.getDriver();
@@ -920,13 +936,7 @@ Job *linux::Linker::constructJob(const JobAction &JA,
   Arguments.push_back("-o");
   Arguments.push_back(Output->getPrimaryOutputFilename().c_str());
 
-  // This avoids a bunch of issues trying to correctly guess parameters
+  return Arguments;
-  // for ld for linux platforms. We know we have clang, we know it should
-  // be able to link, so use clang for now.
-  std::string Exec = TC.getProgramPath("clang++");
-
-  return new Job(JA, *this, std::move(Inputs), std::move(Output),
-                 Args.MakeArgString(Exec), Arguments);
 }
 
 #endif // SWIFT_ENABLE_TARGET_LINUX

lib/Driver/Tools.h

@@ -33,14 +33,19 @@ class LLVM_LIBRARY_VISIBILITY Swift : public Tool {
 public:
   explicit Swift(const ToolChain &TC) : Tool("swift", "swift frontend", TC) {}
 
-  virtual bool hasGoodDiagnostics() const { return true; }
+  bool hasGoodDiagnostics() const override { return true; }
 
-  virtual Job *constructJob(const JobAction &JA,
+protected:
-                            std::unique_ptr<JobList> Inputs,
+  const char *getPath(const llvm::opt::ArgList &Args,
-                            std::unique_ptr<CommandOutput> Output,
+                      const OutputInfo &OI) const override;
+
+  llvm::opt::ArgStringList
+  constructArgumentList(const JobAction &JA,
+                        const JobList *Inputs,
+                        const CommandOutput *Output,
                         const ActionList &InputActions,
                         const llvm::opt::ArgList &Args,
-                            const OutputInfo &OI) const;
+                        const OutputInfo &OI) const override;
 };
 
 class LLVM_LIBRARY_VISIBILITY MergeModule : public Tool {
@@ -48,14 +53,19 @@ public:
   explicit MergeModule(const ToolChain &TC)
     : Tool("merge-module", TC) {}
 
-  virtual bool hasGoodDiagnostics() const { return true; }
+  bool hasGoodDiagnostics() const override { return true; }
 
-  virtual Job *constructJob(const JobAction &JA,
+protected:
-                            std::unique_ptr<JobList> Inputs,
+  const char *getPath(const llvm::opt::ArgList &Args,
-                            std::unique_ptr<CommandOutput> Output,
+                      const OutputInfo &OI) const override;
+
+  llvm::opt::ArgStringList
+  constructArgumentList(const JobAction &JA,
+                        const JobList *Inputs,
+                        const CommandOutput *Output,
                         const ActionList &InputActions,
                         const llvm::opt::ArgList &Args,
-                            const OutputInfo &OI) const;
+                        const OutputInfo &OI) const override;
 };
 
 /// A ToolchainTool may be installed in a location relative to the driver
@@ -75,8 +85,10 @@ protected:
   ToolchainTool(StringRef Name, const ToolChain &TC)
       : ToolchainTool(Name, Name, Name, TC) {}
 
+  const char *getPath(const llvm::opt::ArgList &Args,
+                      const OutputInfo &OI) const override;
+
 public:
-  const char *getPath() const;
   bool isPresentRelativeToDriver() const;
 };
 
@@ -85,24 +97,28 @@ public:
   explicit LLDB(const ToolChain &TC)
       : ToolchainTool("lldb", "LLDB", "LLDB REPL", TC) {}
 
-  virtual Job *constructJob(const JobAction &JA,
+protected:
-                            std::unique_ptr<JobList> Inputs,
+  llvm::opt::ArgStringList
-                            std::unique_ptr<CommandOutput> Output,
+  constructArgumentList(const JobAction &JA,
+                        const JobList *Inputs,
+                        const CommandOutput *Output,
                         const ActionList &InputActions,
                         const llvm::opt::ArgList &Args,
-                            const OutputInfo &OI) const;
+                        const OutputInfo &OI) const override;
 };
 
 class LLVM_LIBRARY_VISIBILITY Dsymutil : public ToolchainTool {
 public:
   explicit Dsymutil(const ToolChain &TC) : ToolchainTool("dsymutil", TC) {}
 
-  virtual Job *constructJob(const JobAction &JA,
+protected:
-                            std::unique_ptr<JobList> Inputs,
+  llvm::opt::ArgStringList
-                            std::unique_ptr<CommandOutput> Output,
+  constructArgumentList(const JobAction &JA,
+                        const JobList *Inputs,
+                        const CommandOutput *Output,
                         const ActionList &InputActions,
                         const llvm::opt::ArgList &Args,
-                            const OutputInfo &OI) const;
+                        const OutputInfo &OI) const override;
 };
 
 class LLVM_LIBRARY_VISIBILITY AutolinkExtract : public ToolchainTool {
@@ -110,12 +126,14 @@ public:
   explicit AutolinkExtract(const ToolChain &TC)
     : ToolchainTool("swift-autolink-extract", TC) {}
 
-  virtual Job *constructJob(const JobAction &JA,
+protected:
-                            std::unique_ptr<JobList> Inputs,
+  llvm::opt::ArgStringList
-                            std::unique_ptr<CommandOutput> Output,
+  constructArgumentList(const JobAction &JA,
+                        const JobList *Inputs,
+                        const CommandOutput *Output,
                         const ActionList &InputActions,
                         const llvm::opt::ArgList &Args,
-                            const OutputInfo &OI) const;
+                        const OutputInfo &OI) const override;
 };
 
 namespace darwin {
@@ -127,12 +145,14 @@ public:
   explicit Linker(const ToolChain &TC)
     : ToolchainTool("ld", "darwin::Linker", "linker", TC) {}
 
-  virtual Job *constructJob(const JobAction &JA,
+protected:
-                            std::unique_ptr<JobList> Inputs,
+  llvm::opt::ArgStringList
-                            std::unique_ptr<CommandOutput> Output,
+  constructArgumentList(const JobAction &JA,
+                        const JobList *Inputs,
+                        const CommandOutput *Output,
                         const ActionList &InputActions,
                         const llvm::opt::ArgList &Args,
-                            const OutputInfo &OI) const;
+                        const OutputInfo &OI) const override;
 };
 
 } // end namespace darwin
@@ -144,12 +164,17 @@ public:
   explicit Linker(const ToolChain &TC)
     : Tool("linux::Linker", "linker", TC) {}
 
-  virtual Job *constructJob(const JobAction &JA,
+protected:
-                            std::unique_ptr<JobList> Inputs,
+  const char *getPath(const llvm::opt::ArgList &Args,
-                            std::unique_ptr<CommandOutput> Output,
+                      const OutputInfo &OI) const override;
+
+  llvm::opt::ArgStringList
+  constructArgumentList(const JobAction &JA,
+                        const JobList *Inputs,
+                        const CommandOutput *Output,
                         const ActionList &InputActions,
                         const llvm::opt::ArgList &Args,
-                            const OutputInfo &OI) const;
+                        const OutputInfo &OI) const override;
 };
 
 } // end namespace linux