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

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 2 08:55:24 PST 2022


arsenm 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,
----------------
aganea wrote:
> `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?
That would expose both the std::nullopt and SecondsToWait == 0 possibilities, which I'm not sure makes any sense here. If you pass a 0, you're expecting to launch a process and have it immediately complete?


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

https://reviews.llvm.org/D138942



More information about the llvm-commits mailing list