[PATCH] D138942: Support: Make Wait's SecondsToWait be Optional [NFC]

[Alexandre Ganea via Phabricator via llvm-commits]

aganea added inline comments.


================
Comment at: llvm/include/llvm/Support/Program.h:209
+      const ProcessInfo &PI, ///< The child process that should be waited on.
+      Optional<unsigned> SecondsToWait, ///< If None, waits until child has
+      ///< terminated.
----------------
I think the idea is to switch to using `std::optional`. You can do it now, or someone will do it later.


================
Comment at: llvm/unittests/Support/ProgramTest.cpp:281
   int RetCode =
-      ExecuteAndWait(Executable, argv, getEnviron(), {}, /*secondsToWait=*/1, 0,
-                     &Error, &ExecutionFailed);
+      ExecuteAndWait(Executable, argv, getEnviron(), {}, /*secondsToWait=*/1,
+                     /*MemoryLimit*/ 0, &Error, &ExecutionFailed);
----------------
`/*SecondsToWait*/` like above?


================
Comment at: llvm/unittests/Support/ProgramTest.cpp:427
   ASSERT_NO_ERROR(fs::unlockFile(FD1));
-  ProcessInfo WaitResult = llvm::sys::Wait(PI2, 5 /* seconds */, true, &Error);
+  ProcessInfo WaitResult = llvm::sys::Wait(PI2, 5 /* seconds */, &Error);
   ASSERT_TRUE(Error.empty());
----------------
Perhaps `/*SecondsToWait*/` here too?


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

https://reviews.llvm.org/D138942