Since we have created a new OF_TextWithCRLF flag, we no longer need to worry about OF_Text flag turning on CRLF translation. I can remove this workaround I added to globally open all ToolOutputFiles as binary on Windows.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Unit Tests
Time | Test | |
---|---|---|
1,790 ms | x64 debian > libarcher.races::lock-unrelated.c |
Event Timeline
This change isn't NFC, there are many places we use OF_TextWithCRLF with ToolOutputFile:
https://github.com/llvm/llvm-project/blob/main/llvm/tools/llvm-dis/llvm-dis.cpp#L206
In any case, this is my preferred future direction, so let's just make the code change and fix any fallout.
So I added this workaround in https://reviews.llvm.org/D97785. All ToolOutputFiles that were marked with OF_Text after this workaround are still OF_Text and shouldn't turn on CRLF translation. Therefore I don't think there will be any fallout on Windows. Any ToolOutputFile prior to this workaround that was marked as OF_Text was changed to OF_TextWithCRLF like the one in llvm-dis.cpp. My follow-up question is should we change those back to OF_Text or leave them as OF_TextWithCRLF?