[cfe-commits] [Patch] Skip attributes

Michael Han fragmentshaders at gmail.com
Mon Dec 10 21:50:07 PST 2012


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/20121210/7d091b38/attachment.html>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: skipcxxattr.patch
Type: application/octet-stream
Size: 5695 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121210/7d091b38/attachment.obj>


More information about the cfe-commits mailing list