[PATCH] D80803: [llvm-dwarfdump] Flush output before warning and errors are reported

[Pavel Labath via Phabricator via llvm-commits]

labath added a comment.

I think this is definitely better than having scattered flushes -- thanks for working on a more systemic solution.

I'm just going to throw one more idea out there. I came up with it when I was adding the original flushes. At the time I chose not to pursue it as I just wanted to get things done, but if we're going to discuss this problem in depth, I think it deserves consideration. If there's consensus that this is a good direction to go in, I'll be happy to create a patch for it

> While I do prefer the current approach over the first revision of this path, I'm still not convinced that dwarfdump should be flushing. If you combine the streams you accept that there is no guaranteed order.

That is not really true. std::iostreams have the concept of "tied" streams, whose (I believe) sole purpose for existence is to ensure consistent ordering between cout and cerr streams. The way it works is that whenever a stream does a read or write operation from/to the backing storage, it first flushes the "tied" stream (if it has one). Normally, cin and cerr are tied to cout, and that's why patterns like `cout << "Give me a number:" /* no newline*/; cin >> n;` work. So, combining stdout and stderr is an issue that a lot of people considered important, and indeed there are a lot of tools out there which get it "right".

llvm streams have no "tied stream" concept, and I would stipulate that this is the real reason for why we're running into the problems here. The simplest and least obtrusive way this concept could be retrofitted into the existing llvm streams is to have a new class -- let's call it `tied_raw_fd_ostream`. This class would inherit from `raw_fd_ostream` and it would take an additional constructor argument -- the "tied" stream. The only special thing it would do is that in its `write_impl` implementation, it would call `TiedStream.flush()` before doing the actual write. `llvm::errs()` would be of type `tied_raw_fd_ostream`, tied to `outs()`, everything else would remain unchanged.

Implementing this idea should be very easy. The result won't be as flexible as the std::iostream solution, but that might actually be a good thing. And as long as an application writes only/mostly to `outs()`, the performance impact should be zero. If an application combines stdout and stderr a lot, then it will suffer some performance degradation, but hopefully that is not the case we need to optimize for.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80803