Re: [PATCH] c++: Check attributes on friend declarations [PR99032]

[Jason Merrill <jason@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2425 bytes --]

On 5/12/21 11:03 AM, Marek Polacek wrote:
> On Wed, May 12, 2021 at 10:37:50AM -0400, Jason Merrill wrote:
>> On 5/11/21 10:45 PM, Marek Polacek wrote:
>>> This patch implements [dcl.attr.grammar]/5: "If an attribute-specifier-seq
>>> appertains to a friend declaration ([class.friend]), that declaration shall
>>> be a definition."
>>>
>>> This restriction only applies to C++11-style attributes.  There are
>>> various forms of friend declarations, we have friend templates, C++11
>>> extended friend declarations, and so on.  In some cases we already
>>> ignore the attribute and warn that it was ignored.  But certain cases
>>> weren't diagnosed, and with this patch we'll give a hard error.  I tried
>>> hard not to emit both a warning and error and I think it worked out.
>>
>> Hmm, why error in this case rather than the usual warning?
> 
> I thought such usage (C++11 attribute on a friend) is so rare (no occurrences
> in the testsuite) that it'd be better to go straight for an error.  Though
> I do see the argument that generally we ignore unknown/misplaced attributes.
> clang gives errors, FWIW.

Up to you.

>> Also, we should warn about similarly useless GNU attributes on friends.
> 
> That's unfortunately more complicated: we should(?) accept
> 
> #define vector __attribute__((vector_size(16)))
> class A {
>    friend vector float f();
> };
> 
> vector float vf;
> vector float
> f ()
> {
>    return vf;
> }

Ah, sure.  Because of the DWIM binding of GNU attributes, that attribute 
applies to 'float' rather than to the friend.

I think we could work around this issue in the grokdeclarator change by 
allowing attributes that have the type_required flag.

I don't think this issue affects the other two hunks of the patch, so we 
should be able to remove the cxx11_attribute_p checks in those.

In the cp_parser_member_declaration change, I don't think we need to 
look at decl_specifiers.std_attributes, as those appertain to the type.

Incidentally, I think one of the lines in your testcase is a syntax 
error that we fail to diagnose:

+  friend [[deprecated]] void; // { dg-error "attribute appertains" }

I think a C++11 attribute cannot appear in the middle of the 
decl-specifier-seq, only before it (in which case it appertains to the 
declaration) or at the end (in which case it appertains to the type). 
So we should give an error for this line, but not this error.  With 
something like:

[-- Attachment #2: 0001-reject-mid-declspec-attrs.patch --]
[-- Type: text/x-patch, Size: 1046 bytes --]

From 623392bfbcc7a9037403b0f98948915c90dd0c33 Mon Sep 17 00:00:00 2001
From: Jason Merrill <jason@redhat.com>
Date: Wed, 12 May 2021 12:10:45 -0400
Subject: [PATCH] reject-mid-declspec-attrs
To: gcc-patches@gcc.gnu.org

---
 gcc/cp/parser.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 0fe29c658d2..0d3deabd8dd 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -15146,6 +15146,16 @@ cp_parser_decl_specifier_seq (cp_parser* parser,
       if (!found_decl_spec)
 	break;
 
+      if (decl_specs->std_attributes)
+	{
+	  error_at (decl_specs->locations[ds_std_attribute],
+		    "standard attributes in middle of decl-specifiers");
+	  inform (decl_specs->locations[ds_std_attribute],
+		  "standard attributes must precede the decl-specifiers to "
+		  "apply to the declaration, or follow them to apply to "
+		  "the type");
+	}
+
       decl_specs->any_specifiers_p = true;
       /* After we see one decl-specifier, further decl-specifiers are
 	 always optional.  */
-- 
2.27.0