Convenience/safety fix for llvm::sys::Execute(And|No)Wait

[alexfh]

Summary

Change the type of the Redirects parameter of llvm::sys::ExecuteAndWait,
ExecuteNoWait and other APIs that wrap them from const StringRef ** to
ArrayRef<Optional<StringRef>>, which is safer and simplifies the use of these
APIs (no more local StringRef variables just to get a pointer to).

Corresponding clang changes will be posted as a separate patch.

[alexfh]

Make default arguments consistent (None -> {}).

[vsk]

It'd be nice to add in "expected 3 redirects" to the assert bodies.

tools/llvm-cov/CodeCoverage.cpp

460 ↗

(On Diff #114159)

You've used llvm::None for the stderr redirect, and I think it would be clearer / more consistent to use llvm::None here too.

[vsk]

tools/llvm-cov/CodeCoverage.cpp

460 ↗

(On Diff #114159)

Sorry, I meant to write: 'you've used None stderr redirect earlier in lib/Support/Signals.cpp'. I'm not sure what the difference is here.

[alexfh]

tools/llvm-cov/CodeCoverage.cpp

460 ↗

(On Diff #114159)

The difference is that None would disable redirection of the corresponding file descriptor (so stderr would be output to the parent's stderr) and an empty string redirects the file descriptor to nowhere (so stderr would be just dropped). This behavior is the same as before the patch (a nullptr was used instead of llvm::None). Here the original code wanted a redirect to nowhere, which the new code does as well.

[bkramer]

lgtm

[alexfh]

In D37563#865541, @vsk wrote:

It'd be nice to add in "expected 3 redirects" to the assert bodies.

Sorry, missed this comment somehow. If we add a message, it should also cover the case of zero redirects. Something along the lines of "expected 0 or 3 redirects". I have a slight concern that the message would basically repeat the condition resulting in a sort of tautology (assert((Redirects.empty() || Redirects.size() == 3) && "expected 0 or 3 redirects");). But if you still find this useful, I can add a message.

[vsk]

In D37563#870647, @alexfh wrote:

In D37563#865541, @vsk wrote:

It'd be nice to add in "expected 3 redirects" to the assert bodies.

Sorry, missed this comment somehow. If we add a message, it should also cover the case of zero redirects. Something along the lines of "expected 0 or 3 redirects". I have a slight concern that the message would basically repeat the condition resulting in a sort of tautology (assert((Redirects.empty() || Redirects.size() == 3) && "expected 0 or 3 redirects");). But if you still find this useful, I can add a message.

I see, it looks fine as-is.

[vsk]

Actually could you add a blurb about this to docs/ReleaseNotes? I had to fix some old code in lldb and stuff we have internally. It'd be useful to add a sentence or two about how to upgrade old callsites.

[alexfh]

In D37563#871083, @vsk wrote:

Actually could you add a blurb about this to docs/ReleaseNotes? I had to fix some old code in lldb and stuff we have internally. It'd be useful to add a sentence or two about how to upgrade old callsites.

Done in r313356.

llvm/trunk/include/llvm/Support/Program.h

 // This file declares the llvm::sys::Program class.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_SUPPORT_PROGRAM_H
 #define LLVM_SUPPORT_PROGRAM_H
 
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/Support/ErrorOr.h"
 #include <system_error>
 
 namespace llvm {
 class StringRef;
 
 namespace sys {
 
#endif
   /// \param Name name of the executable to find. If it contains any system
   ///   slashes, it will be returned as is.
   /// \param Paths optional list of paths to search for \p Name. If empty it
   ///   will use the system PATH environment instead.
   ///
   /// \returns The fully qualified path to the first \p Name in \p Paths if it
   ///   exists. \p Name if \p Name has slashes in it. Otherwise an error.
   ErrorOr<std::string>
-  findProgramByName(StringRef Name, ArrayRef<StringRef> Paths = None);
+  findProgramByName(StringRef Name, ArrayRef<StringRef> Paths = {});
 
   // These functions change the specified standard stream (stdin or stdout) to
   // binary mode. They return errc::success if the specified stream
   // was changed. Otherwise a platform dependent error is returned.
   std::error_code ChangeStdinToBinary();
   std::error_code ChangeStdoutToBinary();
 
   /// This function executes the program using the arguments provided.  The
   /// invoked program will inherit the stdin, stdout, and stderr file
   /// descriptors, the environment and other configuration settings of the
   /// invoking program.
   /// This function waits for the program to finish, so should be avoided in
   /// library functions that aren't expected to block. Consider using
   /// ExecuteNoWait() instead.
-  /// @returns an integer result code indicating the status of the program.
+  /// \returns an integer result code indicating the status of the program.
   /// A zero or positive value indicates the result code of the program.
   /// -1 indicates failure to execute
   /// -2 indicates a crash during execution or timeout
   int ExecuteAndWait(
       StringRef Program, ///< Path of the program to be executed. It is
       ///< presumed this is the result of the findProgramByName method.
       const char **Args, ///< A vector of strings that are passed to the
       ///< program.  The first element should be the name of the program.
       ///< The list *must* be terminated by a null char* entry.
       const char **Env = nullptr, ///< An optional vector of strings to use for
       ///< the program's environment. If not provided, the current program's
       ///< environment will be used.
-      const StringRef **Redirects = nullptr, ///< An optional array of pointers
-      ///< to paths. If the array is null, no redirection is done. The array
-      ///< should have a size of at least three. The inferior process's
-      ///< stdin(0), stdout(1), and stderr(2) will be redirected to the
-      ///< corresponding paths.
-      ///< When an empty path is passed in, the corresponding file
-      ///< descriptor will be disconnected (ie, /dev/null'd) in a portable
-      ///< way.
+      ArrayRef<Optional<StringRef>> Redirects = {}, ///<
+      ///< An array of optional paths. Should have a size of zero or three.
+      ///< If the array is empty, no redirections are performed.
+      ///< Otherwise, the inferior process's stdin(0), stdout(1), and stderr(2)
+      ///< will be redirected to the corresponding paths, if the optional path
+      ///< is present (not \c llvm::None).
+      ///< When an empty path is passed in, the corresponding file descriptor
+      ///< will be disconnected (ie, /dev/null'd) in a portable way.
       unsigned SecondsToWait = 0, ///< If non-zero, this specifies the amount
       ///< of time to wait for the child process to exit. If the time
       ///< expires, the child is killed and this call returns. If zero,
       ///< this function will wait until the child finishes or forever if
       ///< it doesn't.
       unsigned MemoryLimit = 0, ///< If non-zero, this specifies max. amount
       ///< of memory can be allocated by process. If memory usage will be
       ///< higher limit, the child is killed and this call returns. If zero
       ///< - no memory limit.
       std::string *ErrMsg = nullptr, ///< If non-zero, provides a pointer to a
       ///< string instance in which error messages will be returned. If the
       ///< string is non-empty upon return an error occurred while invoking the
       ///< program.
       bool *ExecutionFailed = nullptr);
 
   /// Similar to ExecuteAndWait, but returns immediately.
   /// @returns The \see ProcessInfo of the newly launced process.
-  /// \note On Microsoft Windows systems, users will need to either call \see
-  /// Wait until the process finished execution or win32 CloseHandle() API on
-  /// ProcessInfo.ProcessHandle to avoid memory leaks.
+  /// \note On Microsoft Windows systems, users will need to either call
+  /// \see Wait until the process finished execution or win32 CloseHandle() API
+  /// on ProcessInfo.ProcessHandle to avoid memory leaks.
   ProcessInfo ExecuteNoWait(StringRef Program, const char **Args,
                             const char **Env = nullptr,
-                            const StringRef **Redirects = nullptr,
+                            ArrayRef<Optional<StringRef>> Redirects = {},
                             unsigned MemoryLimit = 0,
                             std::string *ErrMsg = nullptr,
                             bool *ExecutionFailed = nullptr);
 
   /// Return true if the given arguments fit within system-specific
   /// argument length limits.
   bool commandLineFitsWithinSystemLimits(StringRef Program,
                                          ArrayRef<const char *> Args);

llvm/trunk/lib/Support/GraphWriter.cpp

 }
 
 // Execute the graph viewer. Return true if there were errors.
 static bool ExecGraphViewer(StringRef ExecPath, std::vector<const char *> &args,
                             StringRef Filename, bool wait,
                             std::string &ErrMsg) {
   assert(args.back() == nullptr);
   if (wait) {
-    if (sys::ExecuteAndWait(ExecPath, args.data(), nullptr, nullptr, 0, 0,
+    if (sys::ExecuteAndWait(ExecPath, args.data(), nullptr, {}, 0, 0,
                             &ErrMsg)) {
       errs() << "Error: " << ErrMsg << "\n";
       return true;
     }
     sys::fs::remove(Filename);
     errs() << " done. \n";
   } else {
-    sys::ExecuteNoWait(ExecPath, args.data(), nullptr, nullptr, 0, &ErrMsg);
+    sys::ExecuteNoWait(ExecPath, args.data(), nullptr, {}, 0, &ErrMsg);
     errs() << "Remember to erase graph file: " << Filename << "\n";
   }
   return false;
 }
 
 namespace {
 
 struct GraphSession {

llvm/trunk/lib/Support/Program.cpp

 using namespace sys;
 
 //===----------------------------------------------------------------------===//
 //=== WARNING: Implementation here must contain only TRULY operating system
 //===          independent code.
 //===----------------------------------------------------------------------===//
 
 static bool Execute(ProcessInfo &PI, StringRef Program, const char **Args,
-                    const char **Env, const StringRef **Redirects,
+                    const char **Env, ArrayRef<Optional<StringRef>> Redirects,
                     unsigned MemoryLimit, std::string *ErrMsg);
 
 int sys::ExecuteAndWait(StringRef Program, const char **Args, const char **Envp,
-                        const StringRef **Redirects, unsigned SecondsToWait,
-                        unsigned MemoryLimit, std::string *ErrMsg,
-                        bool *ExecutionFailed) {
+                        ArrayRef<Optional<StringRef>> Redirects,
+                        unsigned SecondsToWait, unsigned MemoryLimit,
+                        std::string *ErrMsg, bool *ExecutionFailed) {
+  assert(Redirects.empty() || Redirects.size() == 3);
   ProcessInfo PI;
   if (Execute(PI, Program, Args, Envp, Redirects, MemoryLimit, ErrMsg)) {
     if (ExecutionFailed)
       *ExecutionFailed = false;
     ProcessInfo Result = Wait(
         PI, SecondsToWait, /*WaitUntilTerminates=*/SecondsToWait == 0, ErrMsg);
     return Result.ReturnCode;
   }
 
   if (ExecutionFailed)
     *ExecutionFailed = true;
 
   return -1;
 }
 
 ProcessInfo sys::ExecuteNoWait(StringRef Program, const char **Args,
-                               const char **Envp, const StringRef **Redirects,
+                               const char **Envp,
+                               ArrayRef<Optional<StringRef>> Redirects,
                                unsigned MemoryLimit, std::string *ErrMsg,
                                bool *ExecutionFailed) {
+  assert(Redirects.empty() || Redirects.size() == 3);
   ProcessInfo PI;
   if (ExecutionFailed)
     *ExecutionFailed = false;
   if (!Execute(PI, Program, Args, Envp, Redirects, MemoryLimit, ErrMsg))
     if (ExecutionFailed)
       *ExecutionFailed = true;
 
   return PI;

llvm/trunk/lib/Support/Signals.cpp

static bool printSymbolizedStackTrace(StringRef Argv0,
   {
     raw_fd_ostream Input(InputFD, true);
     for (int i = 0; i < Depth; i++) {
       if (Modules[i])
         Input << Modules[i] << " " << (void*)Offsets[i] << "\n";
     }
   }
 
-  StringRef InputFileStr(InputFile);
-  StringRef OutputFileStr(OutputFile);
-  StringRef StderrFileStr;
-  const StringRef *Redirects[] = {&InputFileStr, &OutputFileStr,
-                                  &StderrFileStr};
+  Optional<StringRef> Redirects[] = {InputFile.str(), OutputFile.str(), llvm::None};
   const char *Args[] = {"llvm-symbolizer", "--functions=linkage", "--inlining",
 #ifdef LLVM_ON_WIN32
                         // Pass --relative-address on Windows so that we don't
                         // have to add ImageBase from PE file.
                         // FIXME: Make this the default for llvm-symbolizer.
                         "--relative-address",
 #endif
                         "--demangle", nullptr};

llvm/trunk/lib/Support/Unix/Program.inc

for (auto Path : Paths) {
     SmallString<128> FilePath(Path);
     sys::path::append(FilePath, Name);
     if (sys::fs::can_execute(FilePath.c_str()))
       return std::string(FilePath.str()); // Found the executable!
   }
   return errc::no_such_file_or_directory;
 }
 
-static bool RedirectIO(const StringRef *Path, int FD, std::string* ErrMsg) {
+static bool RedirectIO(Optional<StringRef> Path, int FD, std::string* ErrMsg) {
   if (!Path) // Noop
     return false;
   std::string File;
   if (Path->empty())
     // Redirect empty paths to /dev/null
     File = "/dev/null";
   else
     File = *Path;
#ifdef RLIMIT_RSS
   setrlimit (RLIMIT_RSS, &r);
 #endif
 #endif
 }
 
 }
 
 static bool Execute(ProcessInfo &PI, StringRef Program, const char **Args,
-                    const char **Envp, const StringRef **Redirects,
+                    const char **Envp, ArrayRef<Optional<StringRef>> Redirects,
                     unsigned MemoryLimit, std::string *ErrMsg) {
   if (!llvm::sys::fs::exists(Program)) {
     if (ErrMsg)
       *ErrMsg = std::string("Executable \"") + Program.str() +
                 std::string("\" doesn't exist!");
     return false;
   }
 
   // If this OS has posix_spawn and there is no memory limit being implied, use
   // posix_spawn.  It is more efficient than fork/exec.
 #ifdef HAVE_POSIX_SPAWN
   if (MemoryLimit == 0) {
     posix_spawn_file_actions_t FileActionsStore;
     posix_spawn_file_actions_t *FileActions = nullptr;
 
     // If we call posix_spawn_file_actions_addopen we have to make sure the
     // c strings we pass to it stay alive until the call to posix_spawn,
     // so we copy any StringRefs into this variable.
     std::string RedirectsStorage[3];
 
-    if (Redirects) {
+    if (!Redirects.empty()) {
+      assert(Redirects.size() == 3);
       std::string *RedirectsStr[3] = {nullptr, nullptr, nullptr};
       for (int I = 0; I < 3; ++I) {
         if (Redirects[I]) {
           RedirectsStorage[I] = *Redirects[I];
           RedirectsStr[I] = &RedirectsStorage[I];
         }
       }
 
       FileActions = &FileActionsStore;
       posix_spawn_file_actions_init(FileActions);
 
       // Redirect stdin/stdout.
       if (RedirectIO_PS(RedirectsStr[0], 0, ErrMsg, FileActions) ||
           RedirectIO_PS(RedirectsStr[1], 1, ErrMsg, FileActions))
         return false;
-      if (Redirects[1] == nullptr || Redirects[2] == nullptr ||
-          *Redirects[1] != *Redirects[2]) {
+      if (!Redirects[1] || !Redirects[2] || *Redirects[1] != *Redirects[2]) {
         // Just redirect stderr
         if (RedirectIO_PS(RedirectsStr[2], 2, ErrMsg, FileActions))
           return false;
       } else {
         // If stdout and stderr should go to the same place, redirect stderr
         // to the FD already open for stdout.
         if (int Err = posix_spawn_file_actions_adddup2(FileActions, 1, 2))
           return !MakeErrMsg(ErrMsg, "Can't redirect stderr to stdout", Err);
switch (child) {
     // An error occurred:  Return to the caller.
     case -1:
       MakeErrMsg(ErrMsg, "Couldn't fork");
       return false;
 
     // Child process: Execute the program.
     case 0: {
       // Redirect file descriptors...
-      if (Redirects) {
+      if (!Redirects.empty()) {
         // Redirect stdin
         if (RedirectIO(Redirects[0], 0, ErrMsg)) { return false; }
         // Redirect stdout
         if (RedirectIO(Redirects[1], 1, ErrMsg)) { return false; }
         if (Redirects[1] && Redirects[2] && *Redirects[1] == *Redirects[2]) {
           // If stdout and stderr should go to the same place, redirect stderr
           // to the FD already open for stdout.
           if (-1 == dup2(1,2)) {

llvm/trunk/lib/Support/Windows/Program.inc

ErrorOr<std::string> sys::findProgramByName(StringRef Name,
   SmallVector<char, MAX_PATH> U8Result;
   if (std::error_code EC =
           windows::UTF16ToUTF8(U16Result.data(), U16Result.size(), U8Result))
     return EC;
 
   return std::string(U8Result.begin(), U8Result.end());
 }
 
-static HANDLE RedirectIO(const StringRef *Path, int fd, std::string* ErrMsg) {
+static HANDLE RedirectIO(Optional<StringRef> Path, int fd,
+                         std::string *ErrMsg) {
   HANDLE h;
-  if (Path == 0) {
+  if (!Path) {
     if (!DuplicateHandle(GetCurrentProcess(), (HANDLE)_get_osfhandle(fd),
                          GetCurrentProcess(), &h,
                          0, TRUE, DUPLICATE_SAME_ACCESS))
       return INVALID_HANDLE_VALUE;
     return h;
   }
 
   std::string fname;
for (unsigned i = 0; Args[i]; i++) {
     *p++ = ' ';
   }
 
   *p = 0;
   return command;
 }
 
 static bool Execute(ProcessInfo &PI, StringRef Program, const char **Args,
-                    const char **Envp, const StringRef **Redirects,
+                    const char **Envp, ArrayRef<Optional<StringRef>> Redirects,
                     unsigned MemoryLimit, std::string *ErrMsg) {
   if (!sys::fs::can_execute(Program)) {
     if (ErrMsg)
       *ErrMsg = "program not executable";
     return false;
   }
 
   // can_execute may succeed by looking at Program + ".exe". CreateProcessW
static bool Execute(ProcessInfo &PI, StringRef Program, const char **Args,
   // Create a child process.
   STARTUPINFOW si;
   memset(&si, 0, sizeof(si));
   si.cb = sizeof(si);
   si.hStdInput = INVALID_HANDLE_VALUE;
   si.hStdOutput = INVALID_HANDLE_VALUE;
   si.hStdError = INVALID_HANDLE_VALUE;
 
-  if (Redirects) {
+  if (!Redirects.empty()) {
     si.dwFlags = STARTF_USESTDHANDLES;
 
     si.hStdInput = RedirectIO(Redirects[0], 0, ErrMsg);
     if (si.hStdInput == INVALID_HANDLE_VALUE) {
       MakeErrMsg(ErrMsg, "can't redirect stdin");
       return false;
     }
     si.hStdOutput = RedirectIO(Redirects[1], 1, ErrMsg);

llvm/trunk/tools/bugpoint/OptimizerDriver.cpp

for (unsigned i = 0; i < NumExtraArgs; ++i)
     Args.push_back(*ExtraArgs);
   Args.push_back(nullptr);
 
   DEBUG(errs() << "\nAbout to run:\t";
         for (unsigned i = 0, e = Args.size() - 1; i != e; ++i) errs()
         << " " << Args[i];
         errs() << "\n";);
 
-  // Redirect stdout and stderr to nowhere if SilencePasses is given
-  StringRef Nowhere;
-  const StringRef *Redirects[3] = {nullptr, &Nowhere, &Nowhere};
+  Optional<StringRef> Redirects[3] = {None, None, None};
+  // Redirect stdout and stderr to nowhere if SilencePasses is given.
+  if (SilencePasses) {
+    Redirects[1] = "";
+    Redirects[2] = "";
+  }
 
   std::string ErrMsg;
-  int result = sys::ExecuteAndWait(Prog, Args.data(), nullptr,
-                                   (SilencePasses ? Redirects : nullptr),
+  int result = sys::ExecuteAndWait(Prog, Args.data(), nullptr, Redirects,
                                    Timeout, MemoryLimit, &ErrMsg);
 
   // If we are supposed to delete the bitcode file or if the passes crashed,
   // remove it now.  This may fail if the file was never created, but that's ok.
   if (DeleteOutput || result != 0)
     sys::fs::remove(OutputFilename);
 
   // Remove the temporary input file as well

llvm/trunk/tools/bugpoint/ToolRunner.cpp

 /// RunProgramWithTimeout - This function provides an alternate interface
 /// to the sys::Program::ExecuteAndWait interface.
 /// @see sys::Program::ExecuteAndWait
 static int RunProgramWithTimeout(StringRef ProgramPath, const char **Args,
                                  StringRef StdInFile, StringRef StdOutFile,
                                  StringRef StdErrFile, unsigned NumSeconds = 0,
                                  unsigned MemoryLimit = 0,
                                  std::string *ErrMsg = nullptr) {
-  const StringRef *Redirects[3] = {&StdInFile, &StdOutFile, &StdErrFile};
+  Optional<StringRef> Redirects[3] = {StdInFile, StdOutFile, StdErrFile};
   return sys::ExecuteAndWait(ProgramPath, Args, nullptr, Redirects, NumSeconds,
                              MemoryLimit, ErrMsg);
 }
 
 /// RunProgramRemotelyWithTimeout - This function runs the given program
 /// remotely using the given remote client and the sys::Program::ExecuteAndWait.
 /// Returns the remote program exit code or reports a remote client error if it
 /// fails. Remote client is required to return 255 if it failed or program exit
 /// code otherwise.
 /// @see sys::Program::ExecuteAndWait
 static int RunProgramRemotelyWithTimeout(StringRef RemoteClientPath,
                                          const char **Args, StringRef StdInFile,
                                          StringRef StdOutFile,
                                          StringRef StdErrFile,
                                          unsigned NumSeconds = 0,
                                          unsigned MemoryLimit = 0) {
-  const StringRef *Redirects[3] = {&StdInFile, &StdOutFile, &StdErrFile};
+  Optional<StringRef> Redirects[3] = {StdInFile, StdOutFile, StdErrFile};
 
   // Run the program remotely with the remote client
   int ReturnCode = sys::ExecuteAndWait(RemoteClientPath, Args, nullptr,
                                        Redirects, NumSeconds, MemoryLimit);
 
   // Has the remote client fail?
   if (255 == ReturnCode) {
     std::ostringstream OS;

llvm/trunk/tools/dsymutil/MachOUtils.cpp

static bool runLipo(StringRef SDKPath, SmallVectorImpl<const char *> &Args) {
 
   if (!Path) {
     errs() << "error: lipo: " << Path.getError().message() << "\n";
     return false;
   }
 
   std::string ErrMsg;
   int result =
-      sys::ExecuteAndWait(*Path, Args.data(), nullptr, nullptr, 0, 0, &ErrMsg);
+      sys::ExecuteAndWait(*Path, Args.data(), nullptr, {}, 0, 0, &ErrMsg);
   if (result) {
     errs() << "error: lipo: " << ErrMsg << "\n";
     return false;
   }
 
   return true;
 }
 

llvm/trunk/tools/llvm-cov/CodeCoverage.cpp

void CodeCoverageTool::demangleSymbols(const CoverageMapping &Coverage) {
   tool_output_file OutputTOF{OutputPath, OutputFD};
   OutputTOF.os().close();
 
   // Invoke the demangler.
   std::vector<const char *> ArgsV;
   for (const std::string &Arg : ViewOpts.DemanglerOpts)
     ArgsV.push_back(Arg.c_str());
   ArgsV.push_back(nullptr);
-  StringRef InputPathRef = InputPath.str();
-  StringRef OutputPathRef = OutputPath.str();
-  StringRef StderrRef;
-  const StringRef *Redirects[] = {&InputPathRef, &OutputPathRef, &StderrRef};
+  Optional<StringRef> Redirects[] = {InputPath.str(), OutputPath.str(), {""}};
   std::string ErrMsg;
   int RC = sys::ExecuteAndWait(ViewOpts.DemanglerOpts[0], ArgsV.data(),
                                /*env=*/nullptr, Redirects, /*secondsToWait=*/0,
                                /*memoryLimit=*/0, &ErrMsg);
   if (RC) {
     error(ErrMsg, ViewOpts.DemanglerOpts[0]);
     return;
   }

llvm/trunk/unittests/Support/ProgramTest.cpp

TEST_F(ProgramEnvTest, CreateProcessLongPath) {
   // Redirect stdout to a long path.
   SmallString<128> TestDirectory;
   ASSERT_NO_ERROR(
     fs::createUniqueDirectory("program-redirect-test", TestDirectory));
   SmallString<256> LongPath(TestDirectory);
   LongPath.push_back('\\');
   // MAX_PATH = 260
   LongPath.append(260 - TestDirectory.size(), 'a');
-  StringRef LongPathRef(LongPath);
 
   std::string Error;
   bool ExecutionFailed;
-  const StringRef *Redirects[] = { nullptr, &LongPathRef, nullptr };
+  Optional<StringRef> Redirects[] = {None, LongPath.str(), None};
   int RC = ExecuteAndWait(MyExe, ArgV, getEnviron(), Redirects,
     /*secondsToWait=*/ 10, /*memoryLimit=*/ 0, &Error,
     &ExecutionFailed);
   EXPECT_FALSE(ExecutionFailed) << Error;
   EXPECT_EQ(0, RC);
 
   // Remove the long stdout.
   ASSERT_NO_ERROR(fs::remove(Twine(LongPath)));
TEST_F(ProgramEnvTest, CreateProcessTrailingSlash) {
   std::string error;
   bool ExecutionFailed;
   // Redirect stdout and stdin to NUL, but let stderr through.
 #ifdef LLVM_ON_WIN32
   StringRef nul("NUL");
 #else
   StringRef nul("/dev/null");
 #endif
-  const StringRef *redirects[] = { &nul, &nul, nullptr };
+  Optional<StringRef> redirects[] = { nul, nul, None };
   int rc = ExecuteAndWait(my_exe, argv, getEnviron(), redirects,
                           /*secondsToWait=*/ 10, /*memoryLimit=*/ 0, &error,
                           &ExecutionFailed);
   EXPECT_FALSE(ExecutionFailed) << error;
   EXPECT_EQ(0, rc);
 }
 
 TEST_F(ProgramEnvTest, TestExecuteNoWait) {
const char *argv[] = {
     nullptr
   };
 
   // Add LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT to the environment of the child.
   addEnvVar("LLVM_PROGRAM_TEST_EXECUTE_NO_WAIT=1");
 
   std::string Error;
   bool ExecutionFailed;
-  ProcessInfo PI1 = ExecuteNoWait(Executable, argv, getEnviron(), nullptr, 0,
-                                  &Error, &ExecutionFailed);
+  ProcessInfo PI1 = ExecuteNoWait(Executable, argv, getEnviron(), {}, 0, &Error,
+                                  &ExecutionFailed);
   ASSERT_FALSE(ExecutionFailed) << Error;
   ASSERT_NE(PI1.Pid, ProcessInfo::InvalidPid) << "Invalid process id";
 
   unsigned LoopCount = 0;
 
   // Test that Wait() with WaitUntilTerminates=true works. In this case,
   // LoopCount should only be incremented once.
   while (true) {
     ++LoopCount;
     ProcessInfo WaitResult = llvm::sys::Wait(PI1, 0, true, &Error);
     ASSERT_TRUE(Error.empty());
     if (WaitResult.Pid == PI1.Pid)
       break;
   }
 
   EXPECT_EQ(LoopCount, 1u) << "LoopCount should be 1";
 
-  ProcessInfo PI2 = ExecuteNoWait(Executable, argv, getEnviron(), nullptr, 0,
-                                  &Error, &ExecutionFailed);
+  ProcessInfo PI2 = ExecuteNoWait(Executable, argv, getEnviron(), {}, 0, &Error,
+                                  &ExecutionFailed);
   ASSERT_FALSE(ExecutionFailed) << Error;
   ASSERT_NE(PI2.Pid, ProcessInfo::InvalidPid) << "Invalid process id";
 
   // Test that Wait() with SecondsToWait=0 performs a non-blocking wait. In this
   // cse, LoopCount should be greater than 1 (more than one increment occurs).
   while (true) {
     ++LoopCount;
     ProcessInfo WaitResult = llvm::sys::Wait(PI2, 0, false, &Error);
TEST_F(ProgramEnvTest, TestExecuteAndWaitTimeout) {
   };
 
   // Add LLVM_PROGRAM_TEST_TIMEOUT to the environment of the child.
  addEnvVar("LLVM_PROGRAM_TEST_TIMEOUT=1");
 
   std::string Error;
   bool ExecutionFailed;
   int RetCode =
-      ExecuteAndWait(Executable, argv, getEnviron(), nullptr, /*secondsToWait=*/1, 0,
+      ExecuteAndWait(Executable, argv, getEnviron(), {}, /*secondsToWait=*/1, 0,
                      &Error, &ExecutionFailed);
   ASSERT_EQ(-2, RetCode);
 }
 
 TEST(ProgramTest, TestExecuteNegative) {
   std::string Executable = "i_dont_exist";
   const char *argv[] = { Executable.c_str(), nullptr };
 
   {
     std::string Error;
     bool ExecutionFailed;
-    int RetCode = ExecuteAndWait(Executable, argv, nullptr, nullptr, 0, 0,
-                                 &Error, &ExecutionFailed);
+    int RetCode = ExecuteAndWait(Executable, argv, nullptr, {}, 0, 0, &Error,
+                                 &ExecutionFailed);
     ASSERT_TRUE(RetCode < 0) << "On error ExecuteAndWait should return 0 or "
                                 "positive value indicating the result code";
     ASSERT_TRUE(ExecutionFailed);
     ASSERT_FALSE(Error.empty());
   }
 
   {
     std::string Error;
     bool ExecutionFailed;
-    ProcessInfo PI = ExecuteNoWait(Executable, argv, nullptr, nullptr, 0,
-                                   &Error, &ExecutionFailed);
+    ProcessInfo PI = ExecuteNoWait(Executable, argv, nullptr, {}, 0, &Error,
+                                   &ExecutionFailed);
     ASSERT_EQ(PI.Pid, ProcessInfo::InvalidPid)
         << "On error ExecuteNoWait should return an invalid ProcessInfo";
     ASSERT_TRUE(ExecutionFailed);
     ASSERT_FALSE(Error.empty());
   }
 
 }
 

llvm/trunk/utils/not/not.cpp

int main(int argc, const char **argv) {
   auto Program = sys::findProgramByName(argv[0]);
   if (!Program) {
     errs() << "Error: Unable to find `" << argv[0]
            << "' in PATH: " << Program.getError().message() << "\n";
     return 1;
   }
 
   std::string ErrMsg;
-  int Result = sys::ExecuteAndWait(*Program, argv, nullptr, nullptr, 0, 0,
-                                   &ErrMsg);
+  int Result = sys::ExecuteAndWait(*Program, argv, nullptr, {}, 0, 0, &ErrMsg);
 #ifdef _WIN32
   // Handle abort() in msvcrt -- It has exit code as 3.  abort(), aka
   // unreachable, should be recognized as a crash.  However, some binaries use
   // exit code 3 on non-crash failure paths, so only do this if we expect a
   // crash.
   if (ExpectCrash && Result == 3)
     Result = -3;
 #endif