[PATCH] Avoid lexer crash with -frewrite-includes

Alp Toker alp at nuanti.com
Thu Dec 5 21:07:01 PST 2013


Hi Lubos,

Is there anything remaining to get these in?

They look good to me with the outlined tweaks.

Alp.


On 29/11/2013 04:48, Alp Toker wrote:
>
> On 29/11/2013 04:45, Alp Toker wrote:
>>
>> On 28/11/2013 16:05, Lubos Lunak wrote:
>>> On Thursday 28 of November 2013, Alp Toker wrote:
>>>> #include "rewrite-includes-bom.h"#endif /* expanded by 
>>>> -frewrite-includes
>>>> */
>>>>
>>>> On 27/11/2013 18:24, Lubos Lunak wrote:
>>>>>    As discussed in " [PATCH] PR14795 : -frewrite-includes sometimes
>>>>> results in incorrect line number", the following leads to a crash:
>>>> I don't see a crash mentioned in PR14795. Wrong bug?
>>>   That is a reference to the mailing list thread, not to the PR itself.
>>>> Be aware however that with this change, you'll get invalid 
>>>> expansions like:
>>>>
>>>> #include "rewrite-includes-bom.h"#endif /* expanded by 
>>>> -frewrite-includes
>>>> */
>>>>
>>>> Unless this is expected behaviour, you'll need to add special handling
>>>> for end-of-directive in the include rewriter.
>>>   Rather unlikely in practice, but why not, attached.
>>>
>> It's quite usual to see this pattern with x-macros and 
>> machine-generated sources. Thanks for the fix!
>>
>> Please combine this change with the Lexer crash fix and commit them 
>> together, but instead of adding a new test case, just remove the 
>> trailing EOF
>
> /s/EOF/EOL/
>
> It'll be good to add a comment indicating that the missing EOL is 
> intentional as you did in the previous patch.
>
> Alp.
>
>
>> from the existing rewrite-includes-bom.c. That test is already set up 
>> to cover both these issues and there's not much value in adding 
>> standalone does-not-crash tests at a higher granularity.
>>
>> Cheers,
>> Alp.
>>
>

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




More information about the cfe-commits mailing list