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

[David Blaikie via Phabricator via llvm-commits]

dblaikie added a comment.

In D80803#2065028 <https://reviews.llvm.org/D80803#2065028>, @MaskRay wrote:

> 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.


reportError doesn't seem to have the flushing (& also I'm not sure if it needs to use an extra raw_string_ostream - could write out to errs() directly, I think) & reportCmdLineWarning, reportCmdLineError (hmm, why are there different reportCmdLine* distinct from report*?)

> It may be possible that some code path does not consistently flush outs(). We can fix them if someone observes problems.

Yeah, not the end of the world/fixing at least fewer places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D80803