[PATCH] D81078: [Support] Use outs() in ToolOutputFile
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Wed Jun 3 04:53:59 PDT 2020
jhenderson added a comment.
Thanks for looking at this. Having given it more thought, any side-effects this fix will have must be in the realm of bug fixes - if people were mixing printing to stdout from two different sources, they wouldn't expect potential stream interleaving after all.
Perhaps it's worth a simple unit test that shows that the result of `ToolOutputFile.os()` is the same as `outs()`, by comparing addresses? Not sure though.
================
Comment at: llvm/include/llvm/Support/raw_ostream.h:501
+/// like: outs() << "foo" << "bar";
+raw_fd_ostream &outs();
----------------
I'm not sure I'm following the reason for changing from the base class here? I'm guessing it's to fit in well with `ToolOutputFile`'s usage, but coudl that be changed to store a pointer to and return a `raw_ostream` easily?
================
Comment at: llvm/lib/Support/ToolOutputFile.cpp:23
// Arrange for the file to be deleted if the process is killed.
- if (Filename != "-")
+ if (isStdout(Filename))
sys::RemoveFileOnSignal(Filename);
----------------
This logic seems to have been inverted?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81078/new/
https://reviews.llvm.org/D81078
More information about the llvm-commits
mailing list