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

[Michael Han]

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.

Committed as r168826 with minor tweaks, thanks for the review!

Cheers
Michael

On Wed, Nov 28, 2012 at 2:07 PM, Richard Smith <richard at metafoo.co.uk>wrote:

> 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/5861c985/attachment.html>