[PATCH] D15705: Adding a scripted test for PR25717

Rafael EspĂ­ndola via cfe-commits cfe-commits at lists.llvm.org
Wed Dec 23 15:23:20 PST 2015


Gosh, that is quite unfortunate. The problems I see are

* The code only work on windows, as opening a file as text on other
systems is not special.
* There is a confusion in the code about binary being "crlf
translation" or "produce a .o". The logic for creating a buffer is
there because the .o writer needs something it can seek on.

What I would suggest is:

* Check that this test fails at least on windows with your patch
reverted. If so, commit it. BTW, don't you need to drop the "|
FileCheck" to cause a  crash?
* Pass two flags to createDefaultOutputFile: TranslateCrLf and
NeedsSeek instead of just Binary.
* Stop using F_Text flag and just print the correct line ending so
this works on any system.

The last two can be just a bug report for now :-)

Cheers,
Rafael



On 23 December 2015 at 17:43, Yunzhong Gao
<Yunzhong_Gao at playstation.sony.com> wrote:
> ygao added a comment.
>
> If you take a look at
>
>   void PrintPreprocessedAction::ExecuteAction() {
>         ...
>         while (next < end) {
>         if (*cur == 0x0D) {  // CR
>           if (*next == 0x0A)  // CRLF
>             BinaryMode = false;
>         ...
>
> The value of this BinaryMode reflects whether the line ending style of the input file
> is CRLF or LF. And it is passed all the way down to the constructor of raw_fd_ostream,
>
>   std::unique_ptr<llvm::raw_pwrite_stream> CompilerInstance::createOutputFile(...) {
>       ...
>     if (!Binary || OS->supportsSeeking())
>       return std::move(OS);
>
>     auto B = llvm::make_unique<llvm::buffer_ostream>(*OS);
>     ...
>
> So I think the line ending style of the input file does affect whether the output is buffered.
>
>
> http://reviews.llvm.org/D15705
>
>
>


More information about the cfe-commits mailing list