[PATCH] Refactor Compilation::ExecuteCommand into Command::Execute

[Hans Wennborg]

Hi rnk,

I think it makes sense that a Command knows how to execute itself.

There's no functionality change but i rewrote the code to avoid the manual memory management of Argv.

My motivation for this is that I plan to subclass Command to build fall-back functionality into clang-cl.

http://llvm-reviews.chandlerc.com/D1654

Files:
  include/clang/Driver/Job.h
  lib/Driver/Compilation.cpp
  lib/Driver/Job.cpp

Index: include/clang/Driver/Job.h
===================================================================
--- include/clang/Driver/Job.h
+++ include/clang/Driver/Job.h
@@ -76,14 +76,15 @@
   virtual void Print(llvm::raw_ostream &OS, const char *Terminator,
                      bool Quote, bool CrashReport = false) const;
 
+  int Execute(const StringRef **Redirects, std::string *ErrMsg,
+              bool *ExecutionFailed) const;
+
   /// getSource - Return the Action which caused the creation of this job.
   const Action &getSource() const { return Source; }
 
   /// getCreator - Return the Tool which caused the creation of this job.
   const Tool &getCreator() const { return Creator; }
 
-  const char *getExecutable() const { return Executable; }
-
   const llvm::opt::ArgStringList &getArguments() const { return Arguments; }
 
   static bool classof(const Job *J) {
Index: lib/Driver/Compilation.cpp
===================================================================
--- lib/Driver/Compilation.cpp
+++ lib/Driver/Compilation.cpp
@@ -16,7 +16,6 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Option/ArgList.h"
 #include "llvm/Support/FileSystem.h"
-#include "llvm/Support/Program.h"
 #include "llvm/Support/raw_ostream.h"
 #include <errno.h>
 #include <sys/stat.h>
@@ -129,12 +128,6 @@
 
 int Compilation::ExecuteCommand(const Command &C,
                                 const Command *&FailingCommand) const {
-  std::string Prog(C.getExecutable());
-  const char **Argv = new const char*[C.getArguments().size() + 2];
-  Argv[0] = C.getExecutable();
-  std::copy(C.getArguments().begin(), C.getArguments().end(), Argv+1);
-  Argv[C.getArguments().size() + 1] = 0;
-
   if ((getDriver().CCPrintOptions ||
        getArgs().hasArg(options::OPT_v)) && !getDriver().CCGenDiagnostics) {
     raw_ostream *OS = &llvm::errs();
@@ -165,18 +158,15 @@
 
   std::string Error;
   bool ExecutionFailed;
-  int Res = llvm::sys::ExecuteAndWait(Prog, Argv, /*env*/ 0, Redirects,
-                                      /*secondsToWait*/ 0, /*memoryLimit*/ 0,
-                                      &Error, &ExecutionFailed);
+  int Res = C.Execute(Redirects, &Error, &ExecutionFailed);
   if (!Error.empty()) {
     assert(Res && "Error string set with 0 result code!");
     getDriver().Diag(clang::diag::err_drv_command_failure) << Error;
   }
 
   if (Res)
     FailingCommand = &C;
 
-  delete[] Argv;
   return ExecutionFailed ? 1 : Res;
 }
 
Index: lib/Driver/Job.cpp
===================================================================
--- lib/Driver/Job.cpp
+++ lib/Driver/Job.cpp
@@ -11,6 +11,7 @@
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSwitch.h"
+#include "llvm/Support/Program.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 using namespace clang::driver;
@@ -112,6 +113,19 @@
   OS << Terminator;
 }
 
+int Command::Execute(const llvm::StringRef **Redirects, std::string *ErrMsg,
+                     bool *ExecutionFailed) const {
+  SmallVector<const char*, 128> Argv;
+  Argv.push_back(Executable);
+  for (size_t i = 0, e = Arguments.size(); i != e; ++i)
+    Argv.push_back(Arguments[i]);
+  Argv.push_back(0);
+
+  return llvm::sys::ExecuteAndWait(Executable, Argv.data(), /*env*/ 0,
+                                   Redirects, /*secondsToWait*/ 0,
+                                   /*memoryLimit*/ 0, ErrMsg, ExecutionFailed);
+}
+
 JobList::JobList() : Job(JobListClass) {}
 
 JobList::~JobList() {
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D1654.1.patch
Type: text/x-patch
Size: 3496 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130911/3b2a9ad0/attachment.bin>