[PATCH] D67696: [tools] Mark output of tools as text if it is really text

Thu Sep 19 06:37:29 PDT 2019

Kai added a comment.

In D67696#1674254 <https://reviews.llvm.org/D67696#1674254>, @rnk wrote:

> The main effect that I'm aware of here is that this will do CRLF conversion on Windows. Are we sure we really want that? I just did the opposite to TableGen output in rL371683 <https://reviews.llvm.org/rL371683>. In general, I think the majority of tools that write text use OF_Text, so your change makes this minority of tools consistent with the other ones.

I am on a platform where the difference between text and binary streams is much greater than on Windows. Therefore I really appreciate the consistency.
Yes, I noted your commit. It had surprising effects...

================
Comment at: llvm/lib/IR/RemarkStreamer.cpp:129
   auto RemarksFile =
-      std::make_unique<ToolOutputFile>(RemarksFilename, EC, sys::fs::OF_None);
+      std::make_unique<ToolOutputFile>(RemarksFilename, EC, sys::fs::OF_Text);
   // We don't use llvm::FileError here because some diagnostics want the file
----------------
thegameg wrote:
> Depending on `RemarksFormat`, the output can also be LLVM Bitstream. Should this set `OF_Text` only if the format is YAML?
Ah - thanks. Of course it should only be set if the format is YAML.

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

https://reviews.llvm.org/D67696