[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