[PATCH] D81156: [Support] Add stream tie function and use it for errs()
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 10 01:03:17 PDT 2020
jhenderson added a comment.
In D81156#2084176 <https://reviews.llvm.org/D81156#2084176>, @sammccall wrote:
> This is surprising and unsafe behaviour. Can we reconsider changing the default, at least until a more careful audit of the places errs() is used?
> In particular, if errs is used from multiple threads (with a lock) and outs is used from a single thread (so no lock needed) then this introduces racy flushes to outs() concurrent with writes.
Thanks for highlighting this, it's been brought up on D81525 <https://reviews.llvm.org/D81525> as well, so rather than discuss it in two places, we'll look at it there, if that's okay?
As to why this was decided to be the default in the first place: a) it matches the default behaviour of `std::cout`/`std::cerr`, and b) in the typical case of printing an error message of some kind, it ensures we don't interleave error output in the middle of a line of other output, which seems like an almost universally good thing to me. But yet, we missed the thread-safety (or rather lack thereof) aspect of this change.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81156/new/
https://reviews.llvm.org/D81156
More information about the llvm-commits
mailing list