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

[Alexandre Ganea via Phabricator via llvm-commits]

aganea added inline comments.


================
Comment at: llvm/lib/Support/Program.cpp:35
                         ArrayRef<std::optional<StringRef>> Redirects,
                         unsigned SecondsToWait, unsigned MemoryLimit,
                         std::string *ErrMsg, bool *ExecutionFailed,
----------------
`SecondsToWait` here is not completely symmetric with `Wait()` (value 0 here has the meaning of `std::nullopt` in Wait), but maybe it's ok. Do you think it's worth changing this to `std::optional` as well?


================
Comment at: llvm/lib/Support/Unix/Program.inc:408
+    alarm(*SecondsToWait);
+  } else
     WaitPidOptions = WNOHANG;
----------------
nit: I would add braces here, after `else`, please see: https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements
//"Use braces for the `if` block to keep it uniform with the `else` block."// (and vice-versa)


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

https://reviews.llvm.org/D138942