[cfe-commits] [Patch] Skip attributes

[Richard Smith]

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/20131014/8018d960/attachment.html>