Thanks! Here is the updated patch that:<br>- Enable ParseImplicitInt to pass attributes to caller.<br>- Handle alignment specifiers as well.<br>- Fix comments and variable naming Sean pointed out.<br>- Update test case.<br>
<br>Cheers<br>Michael<br><br><div class="gmail_quote">On Sun, Nov 18, 2012 at 7:00 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">Index: lib/Parse/ParseDecl.cpp<br>
===================================================================<br>
--- lib/Parse/ParseDecl.cpp (revision 168292)<br>
+++ lib/Parse/ParseDecl.cpp (working copy)<br>
@@ -1923,12 +1923,13 @@<br>
<< TokenName << TagName;<br>
}<br>
<br>
+ ParsedAttributesWithRange attrs(AttrFactory);<br>
// Parse this as a tag as if the missing tag were present.<br>
if (TagKind == tok::kw_enum)<br>
ParseEnumSpecifier(Loc, DS, TemplateInfo, AS, DSC_normal);<br>
else<br>
ParseClassSpecifier(TagKind, Loc, DS, TemplateInfo, AS,<br>
- /*EnteringContext*/ false, DSC_normal);<br>
+ /*EnteringContext*/ false, DSC_normal, attrs);<br>
return true;<br>
}<br>
<br>
It looks like the attributes are being discarded here. This is an<br>
error path, but we have a fix-it so we still need to recover properly.<br>
It'd be fine to add an extra parameter to ParseImplicitInt for this.<br>
<br>
@@ -1234,12 +1235,24 @@<br>
// - If we have 'struct foo;', then this is either a forward declaration<br>
// or a friend declaration, which have to be treated differently.<br>
// - Otherwise we have something like 'struct foo xyz', a reference.<br>
+ // When start parsing from here, we also take misplaced C++11 attributes<br>
+ // specifiers into consideration by detecting the following cases:<br>
+ // - attributes follow class-name:<br>
+ // struct foo [[]] {};<br>
+ // - attributes appear before or after 'final':<br>
+ // struct foo [[]] final [[]] {};<br>
+ // This allow we produce better diagnostics.<br>
<br>
Something funny seems to have happened to the grammar in this comment.<br>
<br>
@@ -1261,6 +1274,30 @@<br>
// Okay, this is a class definition.<br>
TUK = Sema::TUK_Definition;<br>
}<br>
+ } else if (isCXX0XFinalKeyword() && (NextToken().is(tok::l_square))) {<br>
+ // We can't tell if this is a declaration or definition or reference<br>
+ // until we skipped the 'final' and attributes sequences.<br>
<br>
This comment isn't quite right: this can't be a declaration if we have<br>
the 'final' keyword here.<br>
<br>
+ do {<br>
+ // Skip the attributes without parsing them<br>
+ if (NextToken().is(tok::l_square)) {<br>
+ ConsumeBracket();<br>
+ ConsumeBracket();<br>
+ SkipUntil(tok::r_square, false);<br>
+ SkipUntil(tok::r_square, false);<br>
<br>
The indentation here is off.<br>
<br>
+ }<br>
+ } while (Tok.is(tok::l_square));<br>
<br>
It'd be good to handle alignment-specifiers (the other kind of C++11<br>
attribute-specifier) here.<br>
<div class="HOEnZb"><div class="h5"><br>
On Sun, Nov 18, 2012 at 3:52 PM, Sean Silva <<a href="mailto:silvas@purdue.edu">silvas@purdue.edu</a>> wrote:<br>
> + ParsedAttributesWithRange attrs(AttrFactory);<br>
><br>
> Variable names should be uppercase, as per the coding standards:<br>
> <<a href="http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly" target="_blank">http://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly</a>><br>
><br>
> -- Sean Silva<br>
><br>
> On Sun, Nov 18, 2012 at 5:52 PM, Michael Han <<a href="mailto:fragmentshaders@gmail.com">fragmentshaders@gmail.com</a>> wrote:<br>
>> Hi Richard,<br>
>><br>
>> Thanks for the suggestions!<br>
>><br>
>> Attached updated patch. The parser now will parse C++11 attribute<br>
>> specifiers, in the form of double squares that appear at all possible<br>
>> syntactic locations following class-name in a class specifier (also before<br>
>> or after 'final' keyword). In case of final keyword, I have not found a way<br>
>> to classify the nature of the class specifier without using tentative<br>
>> parsing. Also updated test cases.<br>
>><br>
>> I will look into fix-it in a separate patch.<br>
>><br>
>> Cheers<br>
>> Michael<br>
>><br>
>><br>
>> On Thu, Nov 15, 2012 at 10:32 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
>> wrote:<br>
>>><br>
>>> On Thu, Nov 15, 2012 at 10:33 AM, Michael Han <<a href="mailto:fragmentshaders@gmail.com">fragmentshaders@gmail.com</a>><br>
>>> wrote:<br>
>>> > Hi,<br>
>>> ><br>
>>> > Consider this code<br>
>>> > class [[foo]] c [[foo]];<br>
>>> ><br>
>>> > Clang generates diagnostic like this:<br>
>>> > error: an attribute list cannot appear here<br>
>>> > class [[foo]] c [[foo]];<br>
>>> > ^~~~<br>
>>> ><br>
>>> > I think the first attribute is conforming, and it's the second attribute<br>
>>> > that should be diagnosed.<br>
>>><br>
>>> Yes, I agree that it would be better to point the diagnostic at the<br>
>>> second attribute-specifier-seq (and ideally to say that it's in the<br>
>>> wrong place, and offer a fixit...).<br>
>>><br>
>>> > Attached the patch that fixes this.<br>
>>><br>
>>> I don't think your approach here is going to work. This is valid:<br>
>>><br>
>>> class c {};<br>
>>> class c [[ ]] x;<br>
>>><br>
>>> I believe your patch would treat 'class c' as a declaration in the<br>
>>> second line, which it is not.<br>
>>><br>
>>> In order to correctly handle this, I suggest you parse attributes<br>
>>> after the class name as well as before it, before trying to classify<br>
>>> the reference. If you then find that it's a TUK_Reference, return the<br>
>>> attributes you found after the class name to the caller, for them to<br>
>>> handle appropriately. Otherwise, produce a diagnostic indicating that<br>
>>> they were written in the wrong place (offering a fixit would be<br>
>>> awesome...). For a class definition, you should probably look for<br>
>>> attributes both before and after the optional 'final'.<br>
>>><br>
>>> Thanks for looking into this!<br>
>><br>
>><br>
>><br>
>> _______________________________________________<br>
>> cfe-commits mailing list<br>
>> <a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
>><br>
</div></div></blockquote></div><br>