[cfe-commits] [Patch] Skip attributes

[Michael Han]

Thank you for getting this patch in :)


On Mon, Oct 14, 2013 at 6:38 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> Rebased, reworked a little, and submitted as r192666. Thanks! (And sorry
> for the delay. Ahem.)
>
>
> On Mon, Dec 10, 2012 at 9:50 PM, Michael Han <fragmentshaders at gmail.com>wrote:
>
>> Thanks Richard. Patch updated, please take a look.
>>
>> I am not sure of passing Disambiguate = true to isCXX11AttributeSpecifier
>> would make recover better, because the skip logic in this patch explicitly
>> detects missing brackets (which isCXX11AttributeSpecifier will do when set
>> to disambiguate mode), and the diagnostic is not emitted based on the
>> return value of isCXX11AttributeSpecifier.
>>
>> For error recovery, I noted there is no existing test cases that covers
>> ParseCXX11Attributes as well. When an error occurs while parsing
>> attributes, this patch will skip to next semicolon (and
>> ParseCXX11Attributes would skip to end of file, which sounds like a bug).
>> This will disrupt the parser state by eating up tokens which are supposed
>> to be parsed later. Do you think is it worth to have more heuristics here
>> to cut off parsing in cases which an attribute is not closed?
>>
>> Michael
>>
>>
>> On Fri, Dec 7, 2012 at 4:09 PM, Richard Smith <richard at metafoo.co.uk>wrote:
>>
>>> On Fri, Dec 7, 2012 at 3:08 PM, Michael Han <fragmentshaders at gmail.com>
>>> wrote:
>>> > Hi,
>>> >
>>> > There are a couple of places in parser where we parse attributes, then
>>> > discard them immediately because these attributes are not allowed to
>>> appear
>>> > on certain locations. This patch kills the unnecessary parsing by
>>> simply
>>> > skipping these attributes.
>>> >
>>> > The existing attribute tests should cover all cases related to this
>>> change,
>>> > with one tweaked test case to cover different combination of C++11
>>> attribute
>>> > syntax ([[ and alignas).
>>>
>>> Please use BalancedDelimiterTracker instead of the manual
>>> ConsumeBracket / ExpectAndConsume dance. Also, please add some test
>>> cases for your error recovery (missing ')', missing ']', tokens
>>> between ']' characters).
>>>
>>> Since the presence of attributes in these cases is a hard error, we
>>> could afford to pass Disambiguate = true to isCXX11AttributeSpecifier.
>>> Are there any cases in which that would allow us to recover better?
>>>
>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20131016/3c824d00/attachment.html>