labath added a comment.
In https://reviews.llvm.org/D50161#1187972, @teemperor wrote:
> StreamTee is copying it, which is expected to be copyable when we copy
> CommandObjectResult around. And then I just added the copy-constructor as
> CommandObjectResult refactoring sound time-expensive.
teemperor added a comment.
StreamTee is copying it, which is expected to be copyable when we copy
CommandObjectResult around. And then I just added the copy-constructor as
CommandObjectResult refactoring sound time-expensive.
Repository:
rL LLVM
https://reviews.llvm.org/D50161
This revision was automatically updated to reflect the committed changes.
Closed by commit rL338901: Add raw_ostream wrapper to the Stream class
(authored by teemperor, committed by ).
Herald added a subscriber: llvm-commits.
Changed prior to commit:
labath accepted this revision.
labath added a comment.
This revision is now accepted and ready to land.
looks good to me. Thanks.
Btw, have you looked at how hard would it be to actually fix the places that
are copying these streams?
https://reviews.llvm.org/D50161
teemperor updated this revision to Diff 158926.
teemperor added a reviewer: labath.
teemperor added a comment.
- Renamed m_forward_to to m_target.
- Added a getter to allow external code to use the raw_ostream API.
- Renamed m_forward to m_forwarder.
@labath Thanks, fixed the problems you
labath added a comment.
Wouldn't it be even better to actually expose the llvm class via some accessor
or something? This way we could slowly migrate existing code by changing it to
write to `stream.accessor()` instead of `stream` ? (I am not saying to do that
now, but it opens up
teemperor created this revision.
This wrapper will allow us in the future to reuse LLVM methods from within the
Stream class.
Currently no test as this is intended to be an internal class that shouldn't
have any
NFC. The test for this change will be the follow up patch that migrates LLDB's
