[PATCH] D62347: [Support] Modernize process launching API

Nico Weber via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri May 24 08:55:23 PDT 2019


thakis added a comment.
Herald added a subscriber: dexonsmith.

  $ pbpaste | diffstat
   10 files changed, 497 insertions(+), 276 deletions(-)

The "modernized" api is a good chunk wordier. Can you outline how composing launching would look like with and without this change?



================
Comment at: include/llvm/Support/Program.h:68
+  Crashed,             // process crashed
+  TimedOut             // process was forcibly terminated
+};
----------------
Do we need so many different failure types? Isn't "Failure" enough?


================
Comment at: lib/Support/Unix/Program.inc:180
+  if (!llvm::sys::fs::exists(Program))
+    return Program::makeFailureResult("Executable \"" + Program.str() + "\" doesn't exist!");
 
----------------
clang-format


================
Comment at: lib/Support/Windows/Program.inc:371
+    SetLastError(err);
+    *P = Program::makeCrashedResult(-1, MakeErrMsg("Failed getting return status for program"));
+    return false;
----------------
clang-format


================
Comment at: lib/Support/Windows/Program.inc:405
 
-ProcessInfo sys::Wait(const ProcessInfo &PI, unsigned SecondsToWait,
-                      bool WaitUntilChildTerminates, std::string *ErrMsg) {
-  assert(PI.Pid && "invalid pid to wait on, process not started?");
-  assert((PI.Process && PI.Process != INVALID_HANDLE_VALUE) &&
+void sys::Program::waitCompletion(unsigned SecondsToWait, bool WaitUntilChildTerminates) {
+  assert((Handle && Handle != INVALID_HANDLE_VALUE) &&
----------------
clang-format


================
Comment at: lib/Support/Windows/Program.inc:433
       // Non-blocking wait.
-      return ProcessInfo();
+      return;
     }
----------------
Both if and else now end in a return; can move that after the branches


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62347/new/

https://reviews.llvm.org/D62347





More information about the llvm-commits mailing list