[cfe-commits] [PATCH] forbid attributes on friend member function declaration

[Richard Smith]

On Wed, Nov 28, 2012 at 1:38 PM, Michael Han <fragmentshaders at gmail.com>wrote:

> Hi Richard,
>
> 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.
>
> Attach tweaked patch that coveres all cases you mentioned.
>

OK.

+    // C++11 [dcl.attr.grammar] p4: If an attribute-specifier-seq
appertains
+    // to a friend declaration, that declaration shall be a definition.
+    if (DeclaratorInfo.isFunctionDeclarator() &&
+        DefinitionKind != FDK_Definition && DS.isFriendSpecified()) {
+      // Diagnose attributes that appear before decl specifier:
+      // [[]] friend int foo();
+      ProhibitAttributes(FnAttrs);
+    }

It looks like this will not catch cases like "alignas(4) friend int;".
Maybe just remove the isFunctionDeclarator() check?

+  void getCXX11AttributesRanges(SmallVector<SourceRange, 4> &Ranges) {

Drop one of the 's's: getCXX11AttributeRanges.

Otherwise, this looks fine.

Cheers
> Michael
>
>
> On Mon, Nov 26, 2012 at 8:42 PM, Richard Smith <richard at metafoo.co.uk>wrote:
>
>> On Mon, Nov 26, 2012 at 6:43 PM, Michael Han <fragmentshaders at gmail.com>wrote:
>>
>>> Hi Richard,
>>>
>>> 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.
>>>
>>> There are two cases to deal with here
>>> - attributes appear before decl specifier of function:
>>>   [[]] friend into foo();
>>>   This case can already be detected in parser using
>>> ParsedAttributesWithRange.
>>>
>>> - attributes appear after function declarator id
>>>   friend int foo [[]] ();
>>>   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:
>>> 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.
>>>
>>> Or
>>>
>>> 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.
>>>
>>> What do you think? I can look into sema check if that is the right
>>> direction to go.
>>>
>>
>> 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:
>>
>> [[noreturn]] friend void f(), g();
>>
>> ... but we should produce two diagnostics for this:
>>
>> friend void f [[noreturn]] (), g [[noreturn]] ();
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20121128/d16aeccd/attachment.html>