Thanks Richard. Patch updated, please take a look.<br><br>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. <br>




<br>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?<br>




<br>Michael<br><br><div class="gmail_quote">On Fri, Dec 7, 2012 at 4:09 PM, Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">




<div><div>On Fri, Dec 7, 2012 at 3:08 PM, Michael Han <<a href="mailto:fragmentshaders@gmail.com" target="_blank">fragmentshaders@gmail.com</a>> wrote:<br>
> Hi,<br>
><br>
> There are a couple of places in parser where we parse attributes, then<br>
> discard them immediately because these attributes are not allowed to appear<br>
> on certain locations. This patch kills the unnecessary parsing by simply<br>
> skipping these attributes.<br>
><br>
> The existing attribute tests should cover all cases related to this change,<br>
> with one tweaked test case to cover different combination of C++11 attribute<br>
> syntax ([[ and alignas).<br>
<br>
</div></div>Please use BalancedDelimiterTracker instead of the manual<br>
ConsumeBracket / ExpectAndConsume dance. Also, please add some test<br>
cases for your error recovery (missing ')', missing ']', tokens<br>
between ']' characters).<br>
<br>
Since the presence of attributes in these cases is a hard error, we<br>
could afford to pass Disambiguate = true to isCXX11AttributeSpecifier.<br>
Are there any cases in which that would allow us to recover better?<br>
</blockquote></div><br>