Refactor ExecuteAndWait to take StringRefs.

This simplifies some code which had StringRefs to begin with, and
makes other code more complicated which had const char* to begin
with.

In the end, I think this makes for a more idiomatic and platform
agnostic API.  Not all platforms launch process with null terminated
c-string arrays for the environment pointer and argv, but the api
was designed that way because it allowed easy pass-through for
posix-based platforms.  There's a little additional overhead now
since on posix based platforms we'll be takign StringRefs which
were constructed from null terminated strings and then copying
them to null terminate them again, but from a readability and
usability standpoint of the API user, I think this API signature
is strictly better.

llvm-svn: 334518

2 files changed, 24 insertions, 40 deletions

diff --git a/llvm/unittests/Support/Host.cpp b/llvm/unittests/Support/Host.cpp
index 736b04c2049..e07d4151bb6 100644
--- a/llvm/unittests/Support/Host.cpp
+++ b/llvm/unittests/Support/Host.cpp
@@ -185,12 +185,12 @@ TEST_F(HostTest, getMacOSHostVersion) {
   path::append(OutputFile, "out");
 
   const char *SwVersPath = "/usr/bin/sw_vers";
-  const char *argv[] = {SwVersPath, "-productVersion", nullptr};
+  StringRef argv[] = {SwVersPath, "-productVersion"};
   StringRef OutputPath = OutputFile.str();
   const Optional<StringRef> Redirects[] = {/*STDIN=*/None,
                                            /*STDOUT=*/OutputPath,
                                            /*STDERR=*/None};
-  int RetCode = ExecuteAndWait(SwVersPath, argv, /*env=*/nullptr, Redirects);
+  int RetCode = ExecuteAndWait(SwVersPath, argv, /*env=*/llvm::None, Redirects);
   ASSERT_EQ(0, RetCode);
 
   int FD = 0;
diff --git a/llvm/unittests/Support/ProgramTest.cpp b/llvm/unittests/Support/ProgramTest.cpp
index 7a0676c7a48..ec1c85a9f25 100644
--- a/llvm/unittests/Support/ProgramTest.cpp
+++ b/llvm/unittests/Support/ProgramTest.cpp
@@ -60,7 +60,7 @@ static cl::opt<std::string>
 ProgramTestStringArg2("program-test-string-arg2");
 
 class ProgramEnvTest : public testing::Test {
-  std::vector<const char *> EnvTable;
+  std::vector<StringRef> EnvTable;
   std::vector<std::string> EnvStorage;
 
 protected:
@@ -77,7 +77,7 @@ protected:
     }();
     ASSERT_TRUE(EnvP);
 
-    auto prepareEnvVar = [this](decltype(*EnvP) Var) {
+    auto prepareEnvVar = [this](decltype(*EnvP) Var) -> StringRef {
 #if defined(_WIN32)
       // On Windows convert UTF16 encoded variable to UTF8
       auto Len = wcslen(Var);
@@ -86,10 +86,10 @@ protected:
       EnvStorage.emplace_back();
       auto convStatus = convertUTF16ToUTF8String(Ref, EnvStorage.back());
       EXPECT_TRUE(convStatus);
-      return EnvStorage.back().c_str();
+      return EnvStorage.back();
 #else
       (void)this;
-      return Var;
+      return StringRef(Var);
 #endif
     };
 
@@ -104,16 +104,9 @@ protected:
     EnvStorage.clear();
   }
 
-  void addEnvVar(const char *Var) {
-    ASSERT_TRUE(EnvTable.empty() || EnvTable.back()) << "Env table sealed";
-    EnvTable.emplace_back(Var);
-  }
+  void addEnvVar(StringRef Var) { EnvTable.emplace_back(Var); }
 
-  const char **getEnviron() {
-    if (EnvTable.back() != nullptr)
-      EnvTable.emplace_back(nullptr); // Seal table.
-    return &EnvTable[0];
-  }
+  ArrayRef<StringRef> getEnviron() const { return EnvTable; }
 };
 
 #ifdef _WIN32
@@ -129,11 +122,8 @@ TEST_F(ProgramEnvTest, CreateProcessLongPath) {
     MyExe.append("\\\\?\\");
   MyExe.append(MyAbsExe);
 
-  const char *ArgV[] = {
-    MyExe.c_str(),
-    "--gtest_filter=ProgramEnvTest.CreateProcessLongPath",
-    nullptr
-  };
+  StringRef ArgV[] = {MyExe,
+                      "--gtest_filter=ProgramEnvTest.CreateProcessLongPath"};
 
   // Add LLVM_PROGRAM_TEST_LONG_PATH to the environment of the child.
   addEnvVar("LLVM_PROGRAM_TEST_LONG_PATH=1");
@@ -173,13 +163,13 @@ TEST_F(ProgramEnvTest, CreateProcessTrailingSlash) {
 
   std::string my_exe =
       sys::fs::getMainExecutable(TestMainArgv0, &ProgramTestStringArg1);
-  const char *argv[] = {
-    my_exe.c_str(),
-    "--gtest_filter=ProgramEnvTest.CreateProcessTrailingSlash",
-    "-program-test-string-arg1", "has\\\\ trailing\\",
-    "-program-test-string-arg2", "has\\\\ trailing\\",
-    nullptr
-  };
+  StringRef argv[] = {
+      my_exe,
+      "--gtest_filter=ProgramEnvTest.CreateProcessTrailingSlash",
+      "-program-test-string-arg1",
+      "has\\\\ trailing\\",
+      "-program-test-string-arg2",
+      "has\\\\ trailing\\"};
 
   // Add LLVM_PROGRAM_TEST_CHILD to the environment of the child.
   addEnvVar("LLVM_PROGRAM_TEST_CHILD=1");
@@ -210,11 +200,8 @@ TEST_F(ProgramEnvTest, TestExecuteNoWait) {
 
   std::string Executable =
       sys::fs::getMainExecutable(TestMainArgv0, &ProgramTestStringArg1);
-  const char *argv[] = {
-    Executable.c_str(),
-    "--gtest_filter=ProgramEnvTest.TestExecuteNoWait",
-    nullptr
-  };
+  StringRef argv[] = {Executable,
+                      "--gtest_filter=ProgramEnvTest.TestExecuteNoWait"};
 
   // Add LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT to the environment of the child.
   addEnvVar("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT=1");
@@ -268,11 +255,8 @@ TEST_F(ProgramEnvTest, TestExecuteAndWaitTimeout) {
 
   std::string Executable =
       sys::fs::getMainExecutable(TestMainArgv0, &ProgramTestStringArg1);
-  const char *argv[] = {
-    Executable.c_str(),
-    "--gtest_filter=ProgramEnvTest.TestExecuteAndWaitTimeout",
-    nullptr
-  };
+  StringRef argv[] = {
+      Executable, "--gtest_filter=ProgramEnvTest.TestExecuteAndWaitTimeout"};
 
   // Add LLVM_PROGRAM_TEST_TIMEOUT to the environment of the child.
  addEnvVar("LLVM_PROGRAM_TEST_TIMEOUT=1");
@@ -287,12 +271,12 @@ TEST_F(ProgramEnvTest, TestExecuteAndWaitTimeout) {
 
 TEST(ProgramTest, TestExecuteNegative) {
   std::string Executable = "i_dont_exist";
-  const char *argv[] = { Executable.c_str(), nullptr };
+  StringRef argv[] = {Executable};
 
   {
     std::string Error;
     bool ExecutionFailed;
-    int RetCode = ExecuteAndWait(Executable, argv, nullptr, {}, 0, 0, &Error,
+    int RetCode = ExecuteAndWait(Executable, argv, llvm::None, {}, 0, 0, &Error,
                                  &ExecutionFailed);
     ASSERT_TRUE(RetCode < 0) << "On error ExecuteAndWait should return 0 or "
                                 "positive value indicating the result code";
@@ -303,7 +287,7 @@ TEST(ProgramTest, TestExecuteNegative) {
   {
     std::string Error;
     bool ExecutionFailed;
-    ProcessInfo PI = ExecuteNoWait(Executable, argv, nullptr, {}, 0, &Error,
+    ProcessInfo PI = ExecuteNoWait(Executable, argv, llvm::None, {}, 0, &Error,
                                    &ExecutionFailed);
     ASSERT_EQ(PI.Pid, ProcessInfo::InvalidPid)
         << "On error ExecuteNoWait should return an invalid ProcessInfo";