[PATCH] D81156: [Support] Add stream tie function and use it for errs()

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 8 13:18:31 PDT 2020


MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.

In D81156#2079144 <https://reviews.llvm.org/D81156#2079144>, @jhenderson wrote:

> In D81156#2074078 <https://reviews.llvm.org/D81156#2074078>, @MaskRay wrote:
>
> > Sorry, undo LGTM. Is it worth making tied_to an opt-in feature? i.e. for command line utilities, they can run `errs().tie(outs())`.
>
>
> The issue with that is that all tools that want it (most of them?) will have to explicitly do the work for that. One of the motivations to change `errs()` is to prevent the need to require code in llvm-dwarfdump (and other tools) to be used to prevent stream interleaving. @dblaikie/@JDevlieghere etc what are your opinions? Is it acceptable to require tools to specify the one-line change here?
>
> > We can probably even fold the TiedTo field into `raw_fd_ostream`. Just a thought.
>
> Folding it into raw_fd_ostream would be reasonable. We could have an Optional stream as the tied-to stream for this, and it would probably simplify the required code (no need to provide a template parameter etc to identify the base stream). `tie` and `untie` could be provided to set/unset the tied-to stream.
>
> If we went with that approach, we could always have `errs()` and `outs()` tied as proposed in the current patch, but with the ability for tools to opt-out if they don't want the behaviour. What do you think?


Let's make it opt-out for now. Hopefully there aren't any tool requiring `errs().tie(nullptr)`



================
Comment at: llvm/include/llvm/Support/raw_ostream.h:69
+  /// Optional stream this stream is tied to. If this stream is written to, the
+  // tied-to stream will be flushed first.
+  raw_ostream * TiedStream = nullptr;
----------------
Inconsistent comment marker. `//` -> `///`


================
Comment at: llvm/include/llvm/Support/raw_ostream.h:70
+  // tied-to stream will be flushed first.
+  raw_ostream * TiedStream = nullptr;
+
----------------
` * ` => ` *`


================
Comment at: llvm/include/llvm/Support/raw_ostream.h:307
+  /// stream. Specifying a nullptr unties the stream.
+  void tie(raw_ostream *TieTo) { TiedStream = TieTo; }
+
----------------
`TieTo` -> `TiedTo`


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