[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