r195877 - strip UTF-8 BOM in -frewrite-includes (PR#15664)

[Alp Toker]

On 27/11/2013 21:51, Alp Toker wrote:
>
> On 27/11/2013 21:14, Lubos Lunak wrote:
>> Author: llunak
>> Date: Wed Nov 27 15:14:43 2013
>> New Revision: 195877
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=195877&view=rev
>> Log:
>> strip UTF-8 BOM in -frewrite-includes (PR#15664)
>>
>>
>> Added:
>>      cfe/trunk/test/Frontend/Inputs/rewrite-includes-bom.h
>>      cfe/trunk/test/Frontend/rewrite-includes-bom.c
>> Modified:
>>      cfe/trunk/lib/Rewrite/Frontend/InclusionRewriter.cpp
>>
>> Modified: cfe/trunk/lib/Rewrite/Frontend/InclusionRewriter.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Rewrite/Frontend/InclusionRewriter.cpp?rev=195877&r1=195876&r2=195877&view=diff
>> ============================================================================== 
>>
>> --- cfe/trunk/lib/Rewrite/Frontend/InclusionRewriter.cpp (original)
>> +++ cfe/trunk/lib/Rewrite/Frontend/InclusionRewriter.cpp Wed Nov 27 
>> 15:14:43 2013
>> @@ -367,6 +367,11 @@ bool InclusionRewriter::Process(FileID F
>>     unsigned NextToWrite = 0;
>>     int Line = 1; // The current input file line number.
>>   +  // Ignore UTF-8 BOM, otherwise it'd end up somewhere else than 
>> the start
>> +  // of the resulting file.
>> +  if (FromFile.getBuffer().startswith("\xEF\xBB\xBF"))
>> +    NextToWrite = 3;
>> +
>>     Token RawToken;
>>     RawLex.LexFromRawLexer(RawToken);
>>
>> Added: cfe/trunk/test/Frontend/Inputs/rewrite-includes-bom.h
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/Inputs/rewrite-includes-bom.h?rev=195877&view=auto
>> ============================================================================== 
>>
>> --- cfe/trunk/test/Frontend/Inputs/rewrite-includes-bom.h (added)
>> +++ cfe/trunk/test/Frontend/Inputs/rewrite-includes-bom.h Wed Nov 27 
>> 15:14:43 2013
>> @@ -0,0 +1 @@
>> +// This file starts with UTF-8 BOM marker.
>>
>> Added: cfe/trunk/test/Frontend/rewrite-includes-bom.c
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Frontend/rewrite-includes-bom.c?rev=195877&view=auto
>> ============================================================================== 
>>
>> --- cfe/trunk/test/Frontend/rewrite-includes-bom.c (added)
>> +++ cfe/trunk/test/Frontend/rewrite-includes-bom.c Wed Nov 27 
>> 15:14:43 2013
>> @@ -0,0 +1,4 @@
>> +// RUN: %clang -E -frewrite-includes -I %S/Inputs %s -o - | %clang 
>> -fsyntax-only -Xclang -verify -x c -
>> +// expected-no-diagnostics
>> +
>> +#include "rewrite-includes-bom.h"
>
> Hi Lubos,
>
> I don't think it's reasonable to check in this test in its current 
> state, given that it passes with or without the BOM. If the invisible 
> BOM happens to get removed by automatic encoding transformations in 
> VCS or accidental code cleanup, it would give a false sense that the 
> feature is working when it could in fact have regressed.
>
> Leaving the feature untested until you get round to writing a better 
> test, perhaps checking output for presence of BOM with something like 
> grep, is preferable for that reason.

Also, your commit email address is unreachable. Please update this so we 
can contact you for post-commit review:

|This is the Postfix program at host mx2.suse.de.||
||
||I'm sorry to have to inform you that your message could not be||
||be delivered to one or more recipients. It's attached below.||
||
||l.lunak at suse.cz: User has moved to <unknown>|

Alp.


>
> Thanks
>
> Alp.
>
>
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>

-- 
http://www.nuanti.com
the browser experts