[PATCH] Avoid lexer crash with -frewrite-includes

Alp Toker alp at nuanti.com
Fri Dec 13 09:12:31 PST 2013


Landed a rewrite of these patches in r197245.

Alp.


On 13/12/2013 16:31, Alp Toker wrote:
> Hi Lubos,
>
> I didn't hear from you and will be committing the crash and rewriter 
> fixes shortly, hope you are well.
>
> Alp.
>
>
> On 06/12/2013 05:07, Alp Toker wrote:
>> 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