[PATCH] D80803: [DebugInfo] Add flush to ensure debug line errors are in right place

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sat May 30 13:14:47 PDT 2020


MaskRay added a comment.

In D80803#2065004 <https://reviews.llvm.org/D80803#2065004>, @dblaikie wrote:

> In D80803#2064936 <https://reviews.llvm.org/D80803#2064936>, @MaskRay wrote:
>
> > I am in favor of synchronizing stdout/stderr, i.e. when redirected to a file, making the behavior similar to a pty (line buffered). This makes stderr output in tests more relevant, i.e. we usually do `2>&1 | FileCheck`, now we can check stderr output is emitted in appropriate places. However, like other opinions, I am wary of littering `flush` all over the code base. Also note that flushing too frequently has a performance penalty when full buffering is otherwise used. (I once made `llvm-objdump -d` slow due to a change to make it line buffered)
> >
> > Can we do something similar to D64165 <https://reviews.llvm.org/D64165>? Teach the warning/error (stderr) handler to flush stdout? If there is no stderr output, usual buffering (full buffering when the output is a file) is used. There is no performance penalty.
>
>
> If we're going to do it, yeah, D64165 <https://reviews.llvm.org/D64165> (flushing outs() before the diagnostic, flushing the diagnostic stream after the diagnostic) would be the better direction. But even in that case - it seems it isn't consistently applied even within llvm-objdump.cpp (isn't done for command line warnings, nor for errors?)


The idea is that some warning/error reporting facilities need `outs().flush()`. For llvm-objdump.cpp, `reportWarning` and `reportError` are the two utilities. It may be possible that some code path does not consistently flush outs(). We can fix them if someone observes problems.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80803





More information about the llvm-commits mailing list