I think "alignas(4) friend int; ", or "[[]] friend class C" is captured as free standing declaration specifier, so I add a check there with updated test cases. <br><br>Committed as r168826 with minor tweaks, thanks for the review!<br>
<br>Cheers<br>Michael<br><br><div class="gmail_quote">On Wed, Nov 28, 2012 at 2:07 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 class="im">On Wed, Nov 28, 2012 at 1:38 PM, Michael Han <span dir="ltr"><<a href="mailto:fragmentshaders@gmail.com" target="_blank">fragmentshaders@gmail.com</a>></span> wrote:<br>
</div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi Richard,<br><br><div class="im">I am not sure if it is possible to reject the attributes before finishing parse of declarators, because we should only reject attributes appertain to declarations, not to definitions, and I believe we don't know if a declaration is a definition or not until the parsing of the declarator is finished. Thus, it seems reasonable to put the checks after function declarators are fully parsed.<br>
<br>Attach tweaked patch that coveres all cases you mentioned. <br></div></blockquote><div><br></div><div>OK.</div><div><br></div><div>+ // C++11 [dcl.attr.grammar] p4: If an attribute-specifier-seq appertains </div><div>
+ // to a friend declaration, that declaration shall be a definition.</div>
<div>+ if (DeclaratorInfo.isFunctionDeclarator() && </div><div>+ DefinitionKind != FDK_Definition && DS.isFriendSpecified()) {</div><div>+ // Diagnose attributes that appear before decl specifier:</div>
<div>+ // [[]] friend int foo();</div><div>+ ProhibitAttributes(FnAttrs);</div><div>+ }</div><div><br></div><div>It looks like this will not catch cases like "alignas(4) friend int;". Maybe just remove the isFunctionDeclarator() check?</div>
<div><br></div><div><div>+ void getCXX11AttributesRanges(SmallVector<SourceRange, 4> &Ranges) {</div></div><div><br></div><div>Drop one of the 's's: getCXX11AttributeRanges.</div><div><br></div><div>Otherwise, this looks fine.</div>
<div class="im">
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Cheers<span><font color="#888888"><br>Michael</font></span><div><div>
<br><br><div class="gmail_quote">On Mon, Nov 26, 2012 at 8:42 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>On Mon, Nov 26, 2012 at 6:43 PM, Michael Han <span dir="ltr"><<a href="mailto:fragmentshaders@gmail.com" target="_blank">fragmentshaders@gmail.com</a>></span> wrote:<br>
</div><div class="gmail_quote"><div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
Hi Richard,<br><br>I haven't looked into check it in Sema yet. I had the impression that this type of checks should be performed in parser if possible. Here is what I am thinking.<br><br>There are two cases to deal with here<br>
- attributes appear before decl specifier of function:<br>
[[]] friend into foo();<br> This case can already be detected in parser using ParsedAttributesWithRange.<br><br>- attributes appear after function declarator id<br> friend int foo [[]] ();<br> Attributes in this case are parsed as part of parsing function declarator, so we need some way to pass the parsed attributes to caller where the syntactic checks are handled. I was using AttributeList for this. Without using it, it seems there are two additional options:<br>
Update Declarator so it can return a list of source ranges of C++11 attributes that appertain to the declarator, as the source ranges are what's required to emit the "attributes not allowed here" type of diagnostic. <br>
<br>Or<br><br>Update ParseDeclarator so it takes an additional parameter of ParsedAttributesWithRange. It's probably not worth doing so as ParseDeclarator is invoked in a lot of places and this second parameter will only be useful once.<br>
<br>What do you think? I can look into sema check if that is the right direction to go.<br></blockquote><div><br></div></div><div>Hmm, how about rejecting the attributes in the first case after parsing the decl-specifier-seq and before parsing any declarators, and rejecting the attributes in the second case by checking whether you have a friend declaration inside ParseDirectDeclarator? We should only produce one diagnostic for this:</div>
<div><br></div><div>[[noreturn]] friend void f(), g();</div><div><br></div><div>... but we should produce two diagnostics for this:</div><div><br></div><div>friend void f [[noreturn]] (), g [[noreturn]] ();</div></div>
</blockquote></div><br>
</div><div><br></div></div></blockquote><div> </div></div></div>
</blockquote></div><br>