diff mbox series

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

Message ID 20210512024503.398582-1-polacek@redhat.com
State New
Headers show
Series c++: Check attributes on friend declarations [PR99032] | expand

Commit Message

Marek Polacek May 12, 2021, 2:45 a.m. UTC
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.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

gcc/cp/ChangeLog:

	PR c++/99032
	* decl.c (grokdeclarator): Diagnose when an attribute appertains to
	a friend declaration that is not a definition.
	* parser.c (cp_parser_elaborated_type_specifier): Likewise.
	(cp_parser_member_declaration): Likewise.

gcc/testsuite/ChangeLog:

	PR c++/99032
	* g++.dg/cpp0x/friend7.C: New test.
---
 gcc/cp/decl.c                        |  4 +++
 gcc/cp/parser.c                      | 15 +++++++++-
 gcc/testsuite/g++.dg/cpp0x/friend7.C | 41 ++++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/friend7.C


base-commit: 71d38ec80008afdbb9a059253407d80598b765c0

Comments

Jason Merrill May 12, 2021, 2:37 p.m. UTC | #1
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?

Also, we should warn about similarly useless GNU attributes on friends.

> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/99032
> 	* decl.c (grokdeclarator): Diagnose when an attribute appertains to
> 	a friend declaration that is not a definition.
> 	* parser.c (cp_parser_elaborated_type_specifier): Likewise.
> 	(cp_parser_member_declaration): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/99032
> 	* g++.dg/cpp0x/friend7.C: New test.
> ---
>   gcc/cp/decl.c                        |  4 +++
>   gcc/cp/parser.c                      | 15 +++++++++-
>   gcc/testsuite/g++.dg/cpp0x/friend7.C | 41 ++++++++++++++++++++++++++++
>   3 files changed, 59 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/friend7.C
> 
> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index bc3928d7f85..687a59d49e3 100644
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -13741,6 +13741,10 @@ grokdeclarator (const cp_declarator *declarator,
>   
>   	if (friendp)
>   	  {
> +	    if (attrlist && !funcdef_flag
> +		&& cxx11_attribute_p (*attrlist))
> +	      error_at (id_loc, "attribute appertains to a friend declaration "
> +			"that is not a definition");
>   	    /* Friends are treated specially.  */
>   	    if (ctype == current_class_type)
>   	      ;  /* We already issued a permerror.  */
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 0fe29c658d2..612ca4598b9 100644
> --- a/gcc/cp/parser.c
> +++ b/gcc/cp/parser.c
> @@ -19764,11 +19764,15 @@ cp_parser_elaborated_type_specifier (cp_parser* parser,
>   	       && ! processing_explicit_instantiation)
>   	warning (OPT_Wattributes,
>   		 "attributes ignored on template instantiation");
> +      else if (is_friend && cxx11_attribute_p (attributes))
> +	error ("attribute appertains to a friend declaration that is not "
> +	       "a definition");
>         else if (is_declaration && cp_parser_declares_only_class_p (parser))
>   	cplus_decl_attributes (&type, attributes, (int) ATTR_FLAG_TYPE_IN_PLACE);
>         else
>   	warning (OPT_Wattributes,
> -		 "attributes ignored on elaborated-type-specifier that is not a forward declaration");
> +		 "attributes ignored on elaborated-type-specifier that is "
> +		 "not a forward declaration");
>       }
>   
>     if (tag_type == enum_type)
> @@ -26054,6 +26058,15 @@ cp_parser_member_declaration (cp_parser* parser)
>   		 error_at (decl_spec_token_start->location,
>   			   "friend declaration does not name a class or "
>   			   "function");
> +	       /* Give an error if an attribute cannot appear here, as per
> +		  [dcl.attr.grammar]/5.  But not when declares_class_or_enum:
> +		  we ignore attributes in elaborated-type-specifiers.  */
> +	       else if (!declares_class_or_enum
> +			&& (cxx11_attribute_p (decl_specifiers.std_attributes)
> +			    || cxx11_attribute_p (decl_specifiers.attributes)))
> +		 error_at (decl_spec_token_start->location,
> +			   "attribute appertains to a friend declaration "
> +			   "that is not a definition");
>   	       else
>   		 make_friend_class (current_class_type, type,
>   				    /*complain=*/true);
> diff --git a/gcc/testsuite/g++.dg/cpp0x/friend7.C b/gcc/testsuite/g++.dg/cpp0x/friend7.C
> new file mode 100644
> index 00000000000..4aa7b14cf7d
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/friend7.C
> @@ -0,0 +1,41 @@
> +// PR c++/99032
> +// { dg-do compile { target c++11 } }
> +
> +class X { };
> +template<typename T1, typename T2>
> +void foo (T1, T2);
> +
> +struct S {
> +  [[deprecated]] friend void f(); // { dg-error "attribute appertains" }
> +  [[deprecated]] friend void f2() { }
> +  __attribute__((deprecated)) friend void f3();
> +  friend void f3 [[deprecated]] (); // { dg-error "attribute appertains" }
> +  friend void f4 [[deprecated]] () { }
> +  [[deprecated]] friend void; // { dg-error "attribute appertains" }
> +  friend [[deprecated]] void; // { dg-error "attribute appertains" }
> +  __attribute__((deprecated)) friend int;
> +  friend __attribute__((deprecated)) int;
> +  friend int __attribute__((deprecated));
> +  [[deprecated]] friend X; // { dg-error "attribute appertains" }
> +  [[deprecated]] friend class N; // { dg-warning "attribute ignored" }
> +  friend class [[deprecated]] N2; // { dg-error "attribute appertains" }
> +  friend class __attribute__((deprecated)) N3;
> +  [[deprecated]] friend void foo<>(int, int); // { dg-error "attribute appertains" }
> +  // FIXME: Add dg error when PR100339 is resolved.
> +  //[[deprecated]] friend void ::foo(int, int);
> +};
> +
> +template<typename T>
> +class node { };
> +
> +template<typename T>
> +struct A {
> +  [[deprecated]] friend T; // { dg-error "attribute appertains" }
> +  [[deprecated]] friend class node<T>; // { dg-warning "attribute ignored" }
> +  template<typename>
> +  [[deprecated]] friend class A; // { dg-warning "attribute ignored" }
> +  template<typename>
> +  [[deprecated]] friend void bar () { }
> +  template<typename>
> +  [[deprecated]] friend void baz (); // { dg-error "attribute appertains" }
> +};
> 
> base-commit: 71d38ec80008afdbb9a059253407d80598b765c0
>
Marek Polacek May 12, 2021, 3:03 p.m. UTC | #2
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.
 
> 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;
}

So I limited the patch for [[]] only.

Marek
Jason Merrill May 12, 2021, 4:21 p.m. UTC | #3
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:
Marek Polacek May 13, 2021, 12:03 a.m. UTC | #4
On Wed, May 12, 2021 at 12:21:26PM -0400, Jason Merrill wrote:
> 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.

That works (and I need to scan all the attributes, hence the new function
any_type_req_attributes_p).

> 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.

Yeah, done.
 
> 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.

Right, it only caught the 'friend [[deprecated]] void;' case that's handled
by your patch now.  Dropped.
 
> 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:

Nice, thanks!  I've moved that line into a separate test.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
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 applies to C++11-style attributes as well as GNU
attributes with the exception that we allow GNU attributes that require
a type, such as vector_size to continue accepting code as in attrib63.C.
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.

Jason provided the cp_parser_decl_specifier_seq hunk to detect using
standard attributes in the middle of decl-specifiers, which is invalid.

Co-authored-by: Jason Merrill <jason@redhat.com>

gcc/cp/ChangeLog:

	PR c++/99032
	* cp-tree.h (any_type_req_attributes_p): Declare.
	* decl.c (grokdeclarator): Diagnose when an attribute appertains to
	a friend declaration that is not a definition.
	* decl2.c (any_type_req_attributes_p): New.
	* parser.c (cp_parser_decl_specifier_seq): Diagnose standard attributes
	in the middle of decl-specifiers.
	(cp_parser_elaborated_type_specifier): Diagnose when an attribute
	appertains to a friend declaration that is not a definition.
	(cp_parser_member_declaration): Likewise.

gcc/testsuite/ChangeLog:

	PR c++/99032
	* g++.dg/cpp0x/friend7.C: New test.
	* g++.dg/cpp0x/gen-attrs-4.C: Add dg-error.
	* g++.dg/cpp0x/gen-attrs-39-1.C: Likewise.
	* g++.dg/cpp0x/gen-attrs-74.C: New test.
	* g++.dg/ext/attrib63.C: New test.
---
 gcc/cp/cp-tree.h                            |  1 +
 gcc/cp/decl.c                               |  5 +++
 gcc/cp/decl2.c                              | 14 ++++++++
 gcc/cp/parser.c                             | 23 +++++++++++-
 gcc/testsuite/g++.dg/cpp0x/friend7.C        | 40 +++++++++++++++++++++
 gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C |  3 +-
 gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C    |  3 +-
 gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C   | 10 ++++++
 gcc/testsuite/g++.dg/ext/attrib63.C         | 28 +++++++++++++++
 9 files changed, 124 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/friend7.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C
 create mode 100644 gcc/testsuite/g++.dg/ext/attrib63.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 122dadf976f..f4be943139d 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6763,6 +6763,7 @@ extern tree grokbitfield (const cp_declarator *, cp_decl_specifier_seq *,
 			  tree, tree, tree);
 extern tree splice_template_attributes		(tree *, tree);
 extern bool any_dependent_type_attributes_p	(tree);
+extern bool any_type_req_attributes_p		(tree);
 extern tree cp_reconstruct_complex_type		(tree, tree);
 extern bool attributes_naming_typedef_ok	(tree);
 extern void cplus_decl_attributes		(tree *, tree, int);
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index bc3928d7f85..ef18cbe6a93 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -13741,6 +13741,11 @@ grokdeclarator (const cp_declarator *declarator,
 
 	if (friendp)
 	  {
+	    if (attrlist && *attrlist && !funcdef_flag
+		/* Hack to allow attributes like vector_size on a friend.  */
+		&& !any_type_req_attributes_p (*attrlist))
+	      error_at (id_loc, "attribute appertains to a friend "
+			"declaration that is not a definition");
 	    /* Friends are treated specially.  */
 	    if (ctype == current_class_type)
 	      ;  /* We already issued a permerror.  */
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 89f874a32cc..2bcefb619aa 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -1331,6 +1331,20 @@ any_dependent_type_attributes_p (tree attrs)
   return false;
 }
 
+/* True if ATTRS contains any attribute that requires a type.  */
+
+bool
+any_type_req_attributes_p (tree attrs)
+{
+  for (tree a = attrs; a; a = TREE_CHAIN (a))
+    {
+      const attribute_spec *as = lookup_attribute_spec (get_attribute_name (a));
+      if (as && as->type_required)
+	return true;
+    }
+  return false;
+}
+
 /* Return true iff ATTRS are acceptable attributes to be applied in-place
    to a typedef which gives a previously unnamed class or enum a name for
    linkage purposes.  */
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 41df5dd525f..c0b57955954 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.  */
@@ -19764,11 +19774,15 @@ cp_parser_elaborated_type_specifier (cp_parser* parser,
 	       && ! processing_explicit_instantiation)
 	warning (OPT_Wattributes,
 		 "attributes ignored on template instantiation");
+      else if (is_friend && attributes)
+	error ("attribute appertains to a friend declaration that is not "
+	       "a definition");
       else if (is_declaration && cp_parser_declares_only_class_p (parser))
 	cplus_decl_attributes (&type, attributes, (int) ATTR_FLAG_TYPE_IN_PLACE);
       else
 	warning (OPT_Wattributes,
-		 "attributes ignored on elaborated-type-specifier that is not a forward declaration");
+		 "attributes ignored on elaborated-type-specifier that is "
+		 "not a forward declaration");
     }
 
   if (tag_type == enum_type)
@@ -26054,6 +26068,13 @@ cp_parser_member_declaration (cp_parser* parser)
 		 error_at (decl_spec_token_start->location,
 			   "friend declaration does not name a class or "
 			   "function");
+	       /* Give an error if an attribute cannot appear here, as per
+		  [dcl.attr.grammar]/5.  But not when declares_class_or_enum:
+		  we ignore attributes in elaborated-type-specifiers.  */
+	       else if (!declares_class_or_enum && decl_specifiers.attributes)
+		 error_at (decl_spec_token_start->location,
+			   "attribute appertains to a friend declaration "
+			   "that is not a definition");
 	       else
 		 make_friend_class (current_class_type, type,
 				    /*complain=*/true);
diff --git a/gcc/testsuite/g++.dg/cpp0x/friend7.C b/gcc/testsuite/g++.dg/cpp0x/friend7.C
new file mode 100644
index 00000000000..0308cb49be0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/friend7.C
@@ -0,0 +1,40 @@
+// PR c++/99032
+// { dg-do compile { target c++11 } }
+
+class X { };
+template<typename T1, typename T2>
+void foo (T1, T2);
+
+struct S {
+  [[deprecated]] friend void f(); // { dg-error "attribute appertains" }
+  [[deprecated]] friend void f2() { }
+  __attribute__((deprecated)) friend void f3(); // { dg-error "attribute appertains" }
+  friend void f3 [[deprecated]] (); // { dg-error "attribute appertains" }
+  friend void f4 [[deprecated]] () { }
+  [[deprecated]] friend void; // { dg-error "attribute appertains" }
+  __attribute__((deprecated)) friend int; // { dg-error "attribute appertains" }
+  friend __attribute__((deprecated)) int; // { dg-error "attribute appertains" }
+  friend int __attribute__((deprecated)); // { dg-error "attribute appertains" }
+  [[deprecated]] friend X; // { dg-error "attribute appertains" }
+  [[deprecated]] friend class N; // { dg-warning "attribute ignored" }
+  friend class [[deprecated]] N2; // { dg-error "attribute appertains" }
+  friend class __attribute__((deprecated)) N3; // { dg-error "attribute appertains" }
+  [[deprecated]] friend void foo<>(int, int); // { dg-error "attribute appertains" }
+  // FIXME: Add dg error when PR100339 is resolved.
+  //[[deprecated]] friend void ::foo(int, int);
+};
+
+template<typename T>
+class node { };
+
+template<typename T>
+struct A {
+  [[deprecated]] friend T; // { dg-error "attribute appertains" }
+  [[deprecated]] friend class node<T>; // { dg-warning "attribute ignored" }
+  template<typename>
+  [[deprecated]] friend class A; // { dg-warning "attribute ignored" }
+  template<typename>
+  [[deprecated]] friend void bar () { }
+  template<typename>
+  [[deprecated]] friend void baz (); // { dg-error "attribute appertains" }
+};
diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C
index 453fc01a2e9..4010ba7724c 100644
--- a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C
+++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C
@@ -2,7 +2,8 @@
 
 int fragile_block(void) {
   typedef 
-  [[gnu::aligned (16)]] // { dg-warning "ignored" }
+  [[gnu::aligned (16)]] // { dg-error "standard attributes in middle of decl-specifiers" }
+// { dg-warning "attribute ignored" "" { target *-*-* } .-1 }
   struct  {
     int i;
   } XmmUint16;
diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C
index b401c6908e3..c120aeddf95 100644
--- a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C
+++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C
@@ -13,7 +13,8 @@ int one_third [[noreturn]] [[gnu::unused]] (void);
 int [[gnu::unused]] one_half(); // { dg-warning "ignored" }
 
 static
-[[noreturn]] // { dg-warning "ignored" }
+[[noreturn]] // { dg-error "standard attributes in middle of decl-specifiers" }
+// { dg-warning "attribute ignored" "" { target *-*-* } .-1 }
 void two [[gnu::unused]] (void) {}
 
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C
new file mode 100644
index 00000000000..7e17bc8b661
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C
@@ -0,0 +1,10 @@
+// { dg-do compile { target c++11 } }
+// 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).
+
+struct S {
+  friend [[deprecated]] void; // { dg-error "standard attributes in middle of decl-specifiers" }
+  friend [[deprecated]] int fn(); // { dg-error "standard attributes in middle of decl-specifiers" }
+  // { dg-warning "attribute ignored" "" { target *-*-* } .-1 }
+};
diff --git a/gcc/testsuite/g++.dg/ext/attrib63.C b/gcc/testsuite/g++.dg/ext/attrib63.C
new file mode 100644
index 00000000000..783f80d0044
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attrib63.C
@@ -0,0 +1,28 @@
+// { dg-do compile }
+
+#define vector __attribute__((vector_size(16)))
+class A {
+  friend vector float f();
+  __attribute__((deprecated)) friend void f2(); // { dg-error "attribute appertains" }
+  friend __attribute__((deprecated, vector_size(16))) float f3();
+  friend __attribute__((vector_size(16), deprecated)) float f4();
+};
+
+vector float vf;
+vector float
+f ()
+{
+  return vf;
+}
+
+vector float
+f3 ()
+{
+  return vf;
+}
+
+vector float
+f4 ()
+{
+  return vf;
+}

base-commit: d21963ce7a87db3d4a6921a0fa98b72ea6f4e7f5
Jason Merrill May 13, 2021, 12:27 a.m. UTC | #5
On 5/12/21 8:03 PM, Marek Polacek wrote:
> On Wed, May 12, 2021 at 12:21:26PM -0400, Jason Merrill wrote:
>> 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.
> 
> That works (and I need to scan all the attributes, hence the new function
> any_type_req_attributes_p).
> 
>> 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.
> 
> Yeah, done.
>   
>> 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.
> 
> Right, it only caught the 'friend [[deprecated]] void;' case that's handled
> by your patch now.  Dropped.
>   
>> 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:
> 
> Nice, thanks!  I've moved that line into a separate test.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> -- >8 --
> 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 applies to C++11-style attributes as well as GNU
> attributes with the exception that we allow GNU attributes that require
> a type, such as vector_size to continue accepting code as in attrib63.C.
> 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.
> 
> Jason provided the cp_parser_decl_specifier_seq hunk to detect using
> standard attributes in the middle of decl-specifiers, which is invalid.
> 
> Co-authored-by: Jason Merrill <jason@redhat.com>
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/99032
> 	* cp-tree.h (any_type_req_attributes_p): Declare.
> 	* decl.c (grokdeclarator): Diagnose when an attribute appertains to
> 	a friend declaration that is not a definition.
> 	* decl2.c (any_type_req_attributes_p): New.
> 	* parser.c (cp_parser_decl_specifier_seq): Diagnose standard attributes
> 	in the middle of decl-specifiers.
> 	(cp_parser_elaborated_type_specifier): Diagnose when an attribute
> 	appertains to a friend declaration that is not a definition.
> 	(cp_parser_member_declaration): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/99032
> 	* g++.dg/cpp0x/friend7.C: New test.
> 	* g++.dg/cpp0x/gen-attrs-4.C: Add dg-error.
> 	* g++.dg/cpp0x/gen-attrs-39-1.C: Likewise.
> 	* g++.dg/cpp0x/gen-attrs-74.C: New test.
> 	* g++.dg/ext/attrib63.C: New test.
> ---
>   gcc/cp/cp-tree.h                            |  1 +
>   gcc/cp/decl.c                               |  5 +++
>   gcc/cp/decl2.c                              | 14 ++++++++
>   gcc/cp/parser.c                             | 23 +++++++++++-
>   gcc/testsuite/g++.dg/cpp0x/friend7.C        | 40 +++++++++++++++++++++
>   gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C |  3 +-
>   gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C    |  3 +-
>   gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C   | 10 ++++++
>   gcc/testsuite/g++.dg/ext/attrib63.C         | 28 +++++++++++++++
>   9 files changed, 124 insertions(+), 3 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/friend7.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C
>   create mode 100644 gcc/testsuite/g++.dg/ext/attrib63.C
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 122dadf976f..f4be943139d 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -6763,6 +6763,7 @@ extern tree grokbitfield (const cp_declarator *, cp_decl_specifier_seq *,
>   			  tree, tree, tree);
>   extern tree splice_template_attributes		(tree *, tree);
>   extern bool any_dependent_type_attributes_p	(tree);
> +extern bool any_type_req_attributes_p		(tree);
>   extern tree cp_reconstruct_complex_type		(tree, tree);
>   extern bool attributes_naming_typedef_ok	(tree);
>   extern void cplus_decl_attributes		(tree *, tree, int);
> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index bc3928d7f85..ef18cbe6a93 100644
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -13741,6 +13741,11 @@ grokdeclarator (const cp_declarator *declarator,
>   
>   	if (friendp)
>   	  {
> +	    if (attrlist && *attrlist && !funcdef_flag
> +		/* Hack to allow attributes like vector_size on a friend.  */
> +		&& !any_type_req_attributes_p (*attrlist))
> +	      error_at (id_loc, "attribute appertains to a friend "
> +			"declaration that is not a definition");
>   	    /* Friends are treated specially.  */
>   	    if (ctype == current_class_type)
>   	      ;  /* We already issued a permerror.  */
> diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
> index 89f874a32cc..2bcefb619aa 100644
> --- a/gcc/cp/decl2.c
> +++ b/gcc/cp/decl2.c
> @@ -1331,6 +1331,20 @@ any_dependent_type_attributes_p (tree attrs)
>     return false;
>   }
>   
> +/* True if ATTRS contains any attribute that requires a type.  */

Let's invert this to check if ATTRS contains any attribute that does 
*not* require a type, and would therefore apply to the decl.

> +bool
> +any_type_req_attributes_p (tree attrs)
> +{
> +  for (tree a = attrs; a; a = TREE_CHAIN (a))
> +    {
> +      const attribute_spec *as = lookup_attribute_spec (get_attribute_name (a));
> +      if (as && as->type_required)
> +	return true;
> +    }
> +  return false;
> +}
> +
>   /* Return true iff ATTRS are acceptable attributes to be applied in-place
>      to a typedef which gives a previously unnamed class or enum a name for
>      linkage purposes.  */
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 41df5dd525f..c0b57955954 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.  */
> @@ -19764,11 +19774,15 @@ cp_parser_elaborated_type_specifier (cp_parser* parser,
>   	       && ! processing_explicit_instantiation)
>   	warning (OPT_Wattributes,
>   		 "attributes ignored on template instantiation");
> +      else if (is_friend && attributes)
> +	error ("attribute appertains to a friend declaration that is not "
> +	       "a definition");
>         else if (is_declaration && cp_parser_declares_only_class_p (parser))
>   	cplus_decl_attributes (&type, attributes, (int) ATTR_FLAG_TYPE_IN_PLACE);
>         else
>   	warning (OPT_Wattributes,
> -		 "attributes ignored on elaborated-type-specifier that is not a forward declaration");
> +		 "attributes ignored on elaborated-type-specifier that is "
> +		 "not a forward declaration");
>       }
>   
>     if (tag_type == enum_type)
> @@ -26054,6 +26068,13 @@ cp_parser_member_declaration (cp_parser* parser)
>   		 error_at (decl_spec_token_start->location,
>   			   "friend declaration does not name a class or "
>   			   "function");
> +	       /* Give an error if an attribute cannot appear here, as per
> +		  [dcl.attr.grammar]/5.  But not when declares_class_or_enum:
> +		  we ignore attributes in elaborated-type-specifiers.  */
> +	       else if (!declares_class_or_enum && decl_specifiers.attributes)
> +		 error_at (decl_spec_token_start->location,
> +			   "attribute appertains to a friend declaration "
> +			   "that is not a definition");
>   	       else
>   		 make_friend_class (current_class_type, type,
>   				    /*complain=*/true);
> diff --git a/gcc/testsuite/g++.dg/cpp0x/friend7.C b/gcc/testsuite/g++.dg/cpp0x/friend7.C
> new file mode 100644
> index 00000000000..0308cb49be0
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/friend7.C
> @@ -0,0 +1,40 @@
> +// PR c++/99032
> +// { dg-do compile { target c++11 } }
> +
> +class X { };
> +template<typename T1, typename T2>
> +void foo (T1, T2);
> +
> +struct S {
> +  [[deprecated]] friend void f(); // { dg-error "attribute appertains" }
> +  [[deprecated]] friend void f2() { }
> +  __attribute__((deprecated)) friend void f3(); // { dg-error "attribute appertains" }
> +  friend void f3 [[deprecated]] (); // { dg-error "attribute appertains" }
> +  friend void f4 [[deprecated]] () { }
> +  [[deprecated]] friend void; // { dg-error "attribute appertains" }
> +  __attribute__((deprecated)) friend int; // { dg-error "attribute appertains" }
> +  friend __attribute__((deprecated)) int; // { dg-error "attribute appertains" }
> +  friend int __attribute__((deprecated)); // { dg-error "attribute appertains" }
> +  [[deprecated]] friend X; // { dg-error "attribute appertains" }
> +  [[deprecated]] friend class N; // { dg-warning "attribute ignored" }
> +  friend class [[deprecated]] N2; // { dg-error "attribute appertains" }
> +  friend class __attribute__((deprecated)) N3; // { dg-error "attribute appertains" }
> +  [[deprecated]] friend void foo<>(int, int); // { dg-error "attribute appertains" }
> +  // FIXME: Add dg error when PR100339 is resolved.
> +  //[[deprecated]] friend void ::foo(int, int);
> +};
> +
> +template<typename T>
> +class node { };
> +
> +template<typename T>
> +struct A {
> +  [[deprecated]] friend T; // { dg-error "attribute appertains" }
> +  [[deprecated]] friend class node<T>; // { dg-warning "attribute ignored" }
> +  template<typename>
> +  [[deprecated]] friend class A; // { dg-warning "attribute ignored" }
> +  template<typename>
> +  [[deprecated]] friend void bar () { }
> +  template<typename>
> +  [[deprecated]] friend void baz (); // { dg-error "attribute appertains" }
> +};
> diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C
> index 453fc01a2e9..4010ba7724c 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C
> @@ -2,7 +2,8 @@
>   
>   int fragile_block(void) {
>     typedef
> -  [[gnu::aligned (16)]] // { dg-warning "ignored" }
> +  [[gnu::aligned (16)]] // { dg-error "standard attributes in middle of decl-specifiers" }
> +// { dg-warning "attribute ignored" "" { target *-*-* } .-1 }
>     struct  {
>       int i;
>     } XmmUint16;
> diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C
> index b401c6908e3..c120aeddf95 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C
> @@ -13,7 +13,8 @@ int one_third [[noreturn]] [[gnu::unused]] (void);
>   int [[gnu::unused]] one_half(); // { dg-warning "ignored" }
>   
>   static
> -[[noreturn]] // { dg-warning "ignored" }
> +[[noreturn]] // { dg-error "standard attributes in middle of decl-specifiers" }
> +// { dg-warning "attribute ignored" "" { target *-*-* } .-1 }
>   void two [[gnu::unused]] (void) {}
>   
>   
> diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C
> new file mode 100644
> index 00000000000..7e17bc8b661
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C
> @@ -0,0 +1,10 @@
> +// { dg-do compile { target c++11 } }
> +// 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).
> +
> +struct S {
> +  friend [[deprecated]] void; // { dg-error "standard attributes in middle of decl-specifiers" }
> +  friend [[deprecated]] int fn(); // { dg-error "standard attributes in middle of decl-specifiers" }
> +  // { dg-warning "attribute ignored" "" { target *-*-* } .-1 }
> +};
> diff --git a/gcc/testsuite/g++.dg/ext/attrib63.C b/gcc/testsuite/g++.dg/ext/attrib63.C
> new file mode 100644
> index 00000000000..783f80d0044
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ext/attrib63.C
> @@ -0,0 +1,28 @@
> +// { dg-do compile }
> +
> +#define vector __attribute__((vector_size(16)))
> +class A {
> +  friend vector float f();
> +  __attribute__((deprecated)) friend void f2(); // { dg-error "attribute appertains" }
> +  friend __attribute__((deprecated, vector_size(16))) float f3();
> +  friend __attribute__((vector_size(16), deprecated)) float f4();
> +};
> +
> +vector float vf;
> +vector float
> +f ()
> +{
> +  return vf;
> +}
> +
> +vector float
> +f3 ()
> +{
> +  return vf;
> +}
> +
> +vector float
> +f4 ()
> +{
> +  return vf;
> +}
> 
> base-commit: d21963ce7a87db3d4a6921a0fa98b72ea6f4e7f5
>
Jason Merrill May 13, 2021, 12:34 a.m. UTC | #6
On 5/12/21 8:03 PM, Marek Polacek wrote:
> +  // FIXME: Add dg error when PR100339 is resolved.
> +  //[[deprecated]] friend void ::foo(int, int);

This could be an xfailed dg-error.
Marek Polacek May 13, 2021, 10:08 p.m. UTC | #7
On Wed, May 12, 2021 at 08:27:18PM -0400, Jason Merrill wrote:
> On 5/12/21 8:03 PM, Marek Polacek wrote:
> > diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
> > index 89f874a32cc..2bcefb619aa 100644
> > --- a/gcc/cp/decl2.c
> > +++ b/gcc/cp/decl2.c
> > @@ -1331,6 +1331,20 @@ any_dependent_type_attributes_p (tree attrs)
> >     return false;
> >   }
> > +/* True if ATTRS contains any attribute that requires a type.  */
> 
> Let's invert this to check if ATTRS contains any attribute that does *not*
> require a type, and would therefore apply to the decl.

Sounds good, done.  Now I don't need to check *attrlist.
I've also fixed up the xfail thing in my new test.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

-- >8 --
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 applies to C++11-style attributes as well as GNU
attributes with the exception that we allow GNU attributes that require
a type, such as vector_size to continue accepting code as in attrib63.C.
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.

Jason provided the cp_parser_decl_specifier_seq hunk to detect using
standard attributes in the middle of decl-specifiers, which is invalid.

Co-authored-by: Jason Merrill <jason@redhat.com>

gcc/cp/ChangeLog:

	PR c++/99032
	* cp-tree.h (any_non_type_attribute_p): Declare.
	* decl.c (grokdeclarator): Diagnose when an attribute appertains to
	a friend declaration that is not a definition.
	* decl2.c (any_non_type_attribute_p): New.
	* parser.c (cp_parser_decl_specifier_seq): Diagnose standard attributes
	in the middle of decl-specifiers.
	(cp_parser_elaborated_type_specifier): Diagnose when an attribute
	appertains to a friend declaration that is not a definition.
	(cp_parser_member_declaration): Likewise.

gcc/testsuite/ChangeLog:

	PR c++/99032
	* g++.dg/cpp0x/friend7.C: New test.
	* g++.dg/cpp0x/gen-attrs-4.C: Add dg-error.
	* g++.dg/cpp0x/gen-attrs-39-1.C: Likewise.
	* g++.dg/cpp0x/gen-attrs-74.C: New test.
	* g++.dg/ext/attrib63.C: New test.
---
 gcc/cp/cp-tree.h                            |  1 +
 gcc/cp/decl.c                               |  5 +++
 gcc/cp/decl2.c                              | 14 ++++++++
 gcc/cp/parser.c                             | 23 +++++++++++-
 gcc/testsuite/g++.dg/cpp0x/friend7.C        | 40 +++++++++++++++++++++
 gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C |  3 +-
 gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C    |  3 +-
 gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C   | 10 ++++++
 gcc/testsuite/g++.dg/ext/attrib63.C         | 16 +++++++++
 9 files changed, 112 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/friend7.C
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C
 create mode 100644 gcc/testsuite/g++.dg/ext/attrib63.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 122dadf976f..580db914d40 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6763,6 +6763,7 @@ extern tree grokbitfield (const cp_declarator *, cp_decl_specifier_seq *,
 			  tree, tree, tree);
 extern tree splice_template_attributes		(tree *, tree);
 extern bool any_dependent_type_attributes_p	(tree);
+extern bool any_non_type_attribute_p		(tree);
 extern tree cp_reconstruct_complex_type		(tree, tree);
 extern bool attributes_naming_typedef_ok	(tree);
 extern void cplus_decl_attributes		(tree *, tree, int);
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index bc3928d7f85..17511f09e79 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -13741,6 +13741,11 @@ grokdeclarator (const cp_declarator *declarator,
 
 	if (friendp)
 	  {
+	    if (attrlist && !funcdef_flag
+		/* Hack to allow attributes like vector_size on a friend.  */
+		&& any_non_type_attribute_p (*attrlist))
+	      error_at (id_loc, "attribute appertains to a friend "
+			"declaration that is not a definition");
 	    /* Friends are treated specially.  */
 	    if (ctype == current_class_type)
 	      ;  /* We already issued a permerror.  */
diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
index 89f874a32cc..8e4dd6b544a 100644
--- a/gcc/cp/decl2.c
+++ b/gcc/cp/decl2.c
@@ -1331,6 +1331,20 @@ any_dependent_type_attributes_p (tree attrs)
   return false;
 }
 
+/* True if ATTRS contains any attribute that does not require a type.  */
+
+bool
+any_non_type_attribute_p (tree attrs)
+{
+  for (tree a = attrs; a; a = TREE_CHAIN (a))
+    {
+      const attribute_spec *as = lookup_attribute_spec (get_attribute_name (a));
+      if (as && !as->type_required)
+	return true;
+    }
+  return false;
+}
+
 /* Return true iff ATTRS are acceptable attributes to be applied in-place
    to a typedef which gives a previously unnamed class or enum a name for
    linkage purposes.  */
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 41df5dd525f..c0b57955954 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.  */
@@ -19764,11 +19774,15 @@ cp_parser_elaborated_type_specifier (cp_parser* parser,
 	       && ! processing_explicit_instantiation)
 	warning (OPT_Wattributes,
 		 "attributes ignored on template instantiation");
+      else if (is_friend && attributes)
+	error ("attribute appertains to a friend declaration that is not "
+	       "a definition");
       else if (is_declaration && cp_parser_declares_only_class_p (parser))
 	cplus_decl_attributes (&type, attributes, (int) ATTR_FLAG_TYPE_IN_PLACE);
       else
 	warning (OPT_Wattributes,
-		 "attributes ignored on elaborated-type-specifier that is not a forward declaration");
+		 "attributes ignored on elaborated-type-specifier that is "
+		 "not a forward declaration");
     }
 
   if (tag_type == enum_type)
@@ -26054,6 +26068,13 @@ cp_parser_member_declaration (cp_parser* parser)
 		 error_at (decl_spec_token_start->location,
 			   "friend declaration does not name a class or "
 			   "function");
+	       /* Give an error if an attribute cannot appear here, as per
+		  [dcl.attr.grammar]/5.  But not when declares_class_or_enum:
+		  we ignore attributes in elaborated-type-specifiers.  */
+	       else if (!declares_class_or_enum && decl_specifiers.attributes)
+		 error_at (decl_spec_token_start->location,
+			   "attribute appertains to a friend declaration "
+			   "that is not a definition");
 	       else
 		 make_friend_class (current_class_type, type,
 				    /*complain=*/true);
diff --git a/gcc/testsuite/g++.dg/cpp0x/friend7.C b/gcc/testsuite/g++.dg/cpp0x/friend7.C
new file mode 100644
index 00000000000..734b367cd2b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/friend7.C
@@ -0,0 +1,40 @@
+// PR c++/99032
+// { dg-do compile { target c++11 } }
+
+class X { };
+template<typename T1, typename T2>
+void foo (T1, T2);
+
+struct S {
+  [[deprecated]] friend void f(); // { dg-error "attribute appertains" }
+  [[deprecated]] friend void f2() { }
+  __attribute__((deprecated)) friend void f3(); // { dg-error "attribute appertains" }
+  friend void f3 [[deprecated]] (); // { dg-error "attribute appertains" }
+  friend void f4 [[deprecated]] () { }
+  [[deprecated]] friend void; // { dg-error "attribute appertains" }
+  __attribute__((deprecated)) friend int; // { dg-error "attribute appertains" }
+  friend __attribute__((deprecated)) int; // { dg-error "attribute appertains" }
+  friend int __attribute__((deprecated)); // { dg-error "attribute appertains" }
+  [[deprecated]] friend X; // { dg-error "attribute appertains" }
+  [[deprecated]] friend class N; // { dg-warning "attribute ignored" }
+  friend class [[deprecated]] N2; // { dg-error "attribute appertains" }
+  friend class __attribute__((deprecated)) N3; // { dg-error "attribute appertains" }
+  [[deprecated]] friend void foo<>(int, int); // { dg-error "attribute appertains" }
+  [[deprecated]] friend void ::foo(int, int); // { dg-error "attribute appertains" }
+  // { dg-bogus "should have" "PR100339" { xfail *-*-* } .-1 }
+};
+
+template<typename T>
+class node { };
+
+template<typename T>
+struct A {
+  [[deprecated]] friend T; // { dg-error "attribute appertains" }
+  [[deprecated]] friend class node<T>; // { dg-warning "attribute ignored" }
+  template<typename>
+  [[deprecated]] friend class A; // { dg-warning "attribute ignored" }
+  template<typename>
+  [[deprecated]] friend void bar () { }
+  template<typename>
+  [[deprecated]] friend void baz (); // { dg-error "attribute appertains" }
+};
diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C
index 453fc01a2e9..4010ba7724c 100644
--- a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C
+++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C
@@ -2,7 +2,8 @@
 
 int fragile_block(void) {
   typedef 
-  [[gnu::aligned (16)]] // { dg-warning "ignored" }
+  [[gnu::aligned (16)]] // { dg-error "standard attributes in middle of decl-specifiers" }
+// { dg-warning "attribute ignored" "" { target *-*-* } .-1 }
   struct  {
     int i;
   } XmmUint16;
diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C
index b401c6908e3..c120aeddf95 100644
--- a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C
+++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C
@@ -13,7 +13,8 @@ int one_third [[noreturn]] [[gnu::unused]] (void);
 int [[gnu::unused]] one_half(); // { dg-warning "ignored" }
 
 static
-[[noreturn]] // { dg-warning "ignored" }
+[[noreturn]] // { dg-error "standard attributes in middle of decl-specifiers" }
+// { dg-warning "attribute ignored" "" { target *-*-* } .-1 }
 void two [[gnu::unused]] (void) {}
 
 
diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C
new file mode 100644
index 00000000000..7e17bc8b661
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C
@@ -0,0 +1,10 @@
+// { dg-do compile { target c++11 } }
+// 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).
+
+struct S {
+  friend [[deprecated]] void; // { dg-error "standard attributes in middle of decl-specifiers" }
+  friend [[deprecated]] int fn(); // { dg-error "standard attributes in middle of decl-specifiers" }
+  // { dg-warning "attribute ignored" "" { target *-*-* } .-1 }
+};
diff --git a/gcc/testsuite/g++.dg/ext/attrib63.C b/gcc/testsuite/g++.dg/ext/attrib63.C
new file mode 100644
index 00000000000..e515a2bf6ee
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attrib63.C
@@ -0,0 +1,16 @@
+// { dg-do compile }
+
+#define vector __attribute__((vector_size(16)))
+class A {
+  friend vector float f();
+  __attribute__((deprecated)) friend void f2(); // { dg-error "attribute appertains" }
+  friend __attribute__((deprecated, vector_size(16))) float f3(); // { dg-error "attribute appertains" }
+  friend __attribute__((vector_size(16), deprecated)) float f4(); // { dg-error "attribute appertains" }
+};
+
+vector float vf;
+vector float
+f ()
+{
+  return vf;
+}

base-commit: 1f6fc2826d19136bb5ab97a4bdac07e6736b6869
Jason Merrill May 14, 2021, 12:36 a.m. UTC | #8
On 5/13/21 6:08 PM, Marek Polacek wrote:
> On Wed, May 12, 2021 at 08:27:18PM -0400, Jason Merrill wrote:
>> On 5/12/21 8:03 PM, Marek Polacek wrote:
>>> diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
>>> index 89f874a32cc..2bcefb619aa 100644
>>> --- a/gcc/cp/decl2.c
>>> +++ b/gcc/cp/decl2.c
>>> @@ -1331,6 +1331,20 @@ any_dependent_type_attributes_p (tree attrs)
>>>      return false;
>>>    }
>>> +/* True if ATTRS contains any attribute that requires a type.  */
>>
>> Let's invert this to check if ATTRS contains any attribute that does *not*
>> require a type, and would therefore apply to the decl.
> 
> Sounds good, done.  Now I don't need to check *attrlist.
> I've also fixed up the xfail thing in my new test.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

OK.

> -- >8 --
> 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 applies to C++11-style attributes as well as GNU
> attributes with the exception that we allow GNU attributes that require
> a type, such as vector_size to continue accepting code as in attrib63.C.
> 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.
> 
> Jason provided the cp_parser_decl_specifier_seq hunk to detect using
> standard attributes in the middle of decl-specifiers, which is invalid.
> 
> Co-authored-by: Jason Merrill <jason@redhat.com>
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/99032
> 	* cp-tree.h (any_non_type_attribute_p): Declare.
> 	* decl.c (grokdeclarator): Diagnose when an attribute appertains to
> 	a friend declaration that is not a definition.
> 	* decl2.c (any_non_type_attribute_p): New.
> 	* parser.c (cp_parser_decl_specifier_seq): Diagnose standard attributes
> 	in the middle of decl-specifiers.
> 	(cp_parser_elaborated_type_specifier): Diagnose when an attribute
> 	appertains to a friend declaration that is not a definition.
> 	(cp_parser_member_declaration): Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/99032
> 	* g++.dg/cpp0x/friend7.C: New test.
> 	* g++.dg/cpp0x/gen-attrs-4.C: Add dg-error.
> 	* g++.dg/cpp0x/gen-attrs-39-1.C: Likewise.
> 	* g++.dg/cpp0x/gen-attrs-74.C: New test.
> 	* g++.dg/ext/attrib63.C: New test.
> ---
>   gcc/cp/cp-tree.h                            |  1 +
>   gcc/cp/decl.c                               |  5 +++
>   gcc/cp/decl2.c                              | 14 ++++++++
>   gcc/cp/parser.c                             | 23 +++++++++++-
>   gcc/testsuite/g++.dg/cpp0x/friend7.C        | 40 +++++++++++++++++++++
>   gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C |  3 +-
>   gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C    |  3 +-
>   gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C   | 10 ++++++
>   gcc/testsuite/g++.dg/ext/attrib63.C         | 16 +++++++++
>   9 files changed, 112 insertions(+), 3 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/friend7.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C
>   create mode 100644 gcc/testsuite/g++.dg/ext/attrib63.C
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index 122dadf976f..580db914d40 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -6763,6 +6763,7 @@ extern tree grokbitfield (const cp_declarator *, cp_decl_specifier_seq *,
>   			  tree, tree, tree);
>   extern tree splice_template_attributes		(tree *, tree);
>   extern bool any_dependent_type_attributes_p	(tree);
> +extern bool any_non_type_attribute_p		(tree);
>   extern tree cp_reconstruct_complex_type		(tree, tree);
>   extern bool attributes_naming_typedef_ok	(tree);
>   extern void cplus_decl_attributes		(tree *, tree, int);
> diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
> index bc3928d7f85..17511f09e79 100644
> --- a/gcc/cp/decl.c
> +++ b/gcc/cp/decl.c
> @@ -13741,6 +13741,11 @@ grokdeclarator (const cp_declarator *declarator,
>   
>   	if (friendp)
>   	  {
> +	    if (attrlist && !funcdef_flag
> +		/* Hack to allow attributes like vector_size on a friend.  */
> +		&& any_non_type_attribute_p (*attrlist))
> +	      error_at (id_loc, "attribute appertains to a friend "
> +			"declaration that is not a definition");
>   	    /* Friends are treated specially.  */
>   	    if (ctype == current_class_type)
>   	      ;  /* We already issued a permerror.  */
> diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
> index 89f874a32cc..8e4dd6b544a 100644
> --- a/gcc/cp/decl2.c
> +++ b/gcc/cp/decl2.c
> @@ -1331,6 +1331,20 @@ any_dependent_type_attributes_p (tree attrs)
>     return false;
>   }
>   
> +/* True if ATTRS contains any attribute that does not require a type.  */
> +
> +bool
> +any_non_type_attribute_p (tree attrs)
> +{
> +  for (tree a = attrs; a; a = TREE_CHAIN (a))
> +    {
> +      const attribute_spec *as = lookup_attribute_spec (get_attribute_name (a));
> +      if (as && !as->type_required)
> +	return true;
> +    }
> +  return false;
> +}
> +
>   /* Return true iff ATTRS are acceptable attributes to be applied in-place
>      to a typedef which gives a previously unnamed class or enum a name for
>      linkage purposes.  */
> diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
> index 41df5dd525f..c0b57955954 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.  */
> @@ -19764,11 +19774,15 @@ cp_parser_elaborated_type_specifier (cp_parser* parser,
>   	       && ! processing_explicit_instantiation)
>   	warning (OPT_Wattributes,
>   		 "attributes ignored on template instantiation");
> +      else if (is_friend && attributes)
> +	error ("attribute appertains to a friend declaration that is not "
> +	       "a definition");
>         else if (is_declaration && cp_parser_declares_only_class_p (parser))
>   	cplus_decl_attributes (&type, attributes, (int) ATTR_FLAG_TYPE_IN_PLACE);
>         else
>   	warning (OPT_Wattributes,
> -		 "attributes ignored on elaborated-type-specifier that is not a forward declaration");
> +		 "attributes ignored on elaborated-type-specifier that is "
> +		 "not a forward declaration");
>       }
>   
>     if (tag_type == enum_type)
> @@ -26054,6 +26068,13 @@ cp_parser_member_declaration (cp_parser* parser)
>   		 error_at (decl_spec_token_start->location,
>   			   "friend declaration does not name a class or "
>   			   "function");
> +	       /* Give an error if an attribute cannot appear here, as per
> +		  [dcl.attr.grammar]/5.  But not when declares_class_or_enum:
> +		  we ignore attributes in elaborated-type-specifiers.  */
> +	       else if (!declares_class_or_enum && decl_specifiers.attributes)
> +		 error_at (decl_spec_token_start->location,
> +			   "attribute appertains to a friend declaration "
> +			   "that is not a definition");
>   	       else
>   		 make_friend_class (current_class_type, type,
>   				    /*complain=*/true);
> diff --git a/gcc/testsuite/g++.dg/cpp0x/friend7.C b/gcc/testsuite/g++.dg/cpp0x/friend7.C
> new file mode 100644
> index 00000000000..734b367cd2b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/friend7.C
> @@ -0,0 +1,40 @@
> +// PR c++/99032
> +// { dg-do compile { target c++11 } }
> +
> +class X { };
> +template<typename T1, typename T2>
> +void foo (T1, T2);
> +
> +struct S {
> +  [[deprecated]] friend void f(); // { dg-error "attribute appertains" }
> +  [[deprecated]] friend void f2() { }
> +  __attribute__((deprecated)) friend void f3(); // { dg-error "attribute appertains" }
> +  friend void f3 [[deprecated]] (); // { dg-error "attribute appertains" }
> +  friend void f4 [[deprecated]] () { }
> +  [[deprecated]] friend void; // { dg-error "attribute appertains" }
> +  __attribute__((deprecated)) friend int; // { dg-error "attribute appertains" }
> +  friend __attribute__((deprecated)) int; // { dg-error "attribute appertains" }
> +  friend int __attribute__((deprecated)); // { dg-error "attribute appertains" }
> +  [[deprecated]] friend X; // { dg-error "attribute appertains" }
> +  [[deprecated]] friend class N; // { dg-warning "attribute ignored" }
> +  friend class [[deprecated]] N2; // { dg-error "attribute appertains" }
> +  friend class __attribute__((deprecated)) N3; // { dg-error "attribute appertains" }
> +  [[deprecated]] friend void foo<>(int, int); // { dg-error "attribute appertains" }
> +  [[deprecated]] friend void ::foo(int, int); // { dg-error "attribute appertains" }
> +  // { dg-bogus "should have" "PR100339" { xfail *-*-* } .-1 }
> +};
> +
> +template<typename T>
> +class node { };
> +
> +template<typename T>
> +struct A {
> +  [[deprecated]] friend T; // { dg-error "attribute appertains" }
> +  [[deprecated]] friend class node<T>; // { dg-warning "attribute ignored" }
> +  template<typename>
> +  [[deprecated]] friend class A; // { dg-warning "attribute ignored" }
> +  template<typename>
> +  [[deprecated]] friend void bar () { }
> +  template<typename>
> +  [[deprecated]] friend void baz (); // { dg-error "attribute appertains" }
> +};
> diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C
> index 453fc01a2e9..4010ba7724c 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-39-1.C
> @@ -2,7 +2,8 @@
>   
>   int fragile_block(void) {
>     typedef
> -  [[gnu::aligned (16)]] // { dg-warning "ignored" }
> +  [[gnu::aligned (16)]] // { dg-error "standard attributes in middle of decl-specifiers" }
> +// { dg-warning "attribute ignored" "" { target *-*-* } .-1 }
>     struct  {
>       int i;
>     } XmmUint16;
> diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C
> index b401c6908e3..c120aeddf95 100644
> --- a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C
> +++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-4.C
> @@ -13,7 +13,8 @@ int one_third [[noreturn]] [[gnu::unused]] (void);
>   int [[gnu::unused]] one_half(); // { dg-warning "ignored" }
>   
>   static
> -[[noreturn]] // { dg-warning "ignored" }
> +[[noreturn]] // { dg-error "standard attributes in middle of decl-specifiers" }
> +// { dg-warning "attribute ignored" "" { target *-*-* } .-1 }
>   void two [[gnu::unused]] (void) {}
>   
>   
> diff --git a/gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C
> new file mode 100644
> index 00000000000..7e17bc8b661
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/gen-attrs-74.C
> @@ -0,0 +1,10 @@
> +// { dg-do compile { target c++11 } }
> +// 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).
> +
> +struct S {
> +  friend [[deprecated]] void; // { dg-error "standard attributes in middle of decl-specifiers" }
> +  friend [[deprecated]] int fn(); // { dg-error "standard attributes in middle of decl-specifiers" }
> +  // { dg-warning "attribute ignored" "" { target *-*-* } .-1 }
> +};
> diff --git a/gcc/testsuite/g++.dg/ext/attrib63.C b/gcc/testsuite/g++.dg/ext/attrib63.C
> new file mode 100644
> index 00000000000..e515a2bf6ee
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/ext/attrib63.C
> @@ -0,0 +1,16 @@
> +// { dg-do compile }
> +
> +#define vector __attribute__((vector_size(16)))
> +class A {
> +  friend vector float f();
> +  __attribute__((deprecated)) friend void f2(); // { dg-error "attribute appertains" }
> +  friend __attribute__((deprecated, vector_size(16))) float f3(); // { dg-error "attribute appertains" }
> +  friend __attribute__((vector_size(16), deprecated)) float f4(); // { dg-error "attribute appertains" }
> +};
> +
> +vector float vf;
> +vector float
> +f ()
> +{
> +  return vf;
> +}
> 
> base-commit: 1f6fc2826d19136bb5ab97a4bdac07e6736b6869
>
Franz Sirl May 18, 2021, 5:35 p.m. UTC | #9
Am 2021-05-14 um 00:08 schrieb Marek Polacek via Gcc-patches:
> On Wed, May 12, 2021 at 08:27:18PM -0400, Jason Merrill wrote:
>> On 5/12/21 8:03 PM, Marek Polacek wrote:
>>> diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
>>> index 89f874a32cc..2bcefb619aa 100644
>>> --- a/gcc/cp/decl2.c
>>> +++ b/gcc/cp/decl2.c
>>> @@ -1331,6 +1331,20 @@ any_dependent_type_attributes_p (tree attrs)
>>>      return false;
>>>    }
>>> +/* True if ATTRS contains any attribute that requires a type.  */
>>
>> Let's invert this to check if ATTRS contains any attribute that does *not*
>> require a type, and would therefore apply to the decl.
> 
> Sounds good, done.  Now I don't need to check *attrlist.
> I've also fixed up the xfail thing in my new test.
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> -- >8 --
> 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 applies to C++11-style attributes as well as GNU
> attributes with the exception that we allow GNU attributes that require
> a type, such as vector_size to continue accepting code as in attrib63.C.
> 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.
> 
> Jason provided the cp_parser_decl_specifier_seq hunk to detect using
> standard attributes in the middle of decl-specifiers, which is invalid.
> 
> Co-authored-by: Jason Merrill <jason@redhat.com>
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/99032
> 	* cp-tree.h (any_non_type_attribute_p): Declare.
> 	* decl.c (grokdeclarator): Diagnose when an attribute appertains to
> 	a friend declaration that is not a definition.
> 	* decl2.c (any_non_type_attribute_p): New.
> 	* parser.c (cp_parser_decl_specifier_seq): Diagnose standard attributes
> 	in the middle of decl-specifiers.
> 	(cp_parser_elaborated_type_specifier): Diagnose when an attribute
> 	appertains to a friend declaration that is not a definition.
> 	(cp_parser_member_declaration): Likewise.
> 

Hi,

I haven't investigated it in detail yet, but it seems this change breaks 
building Qt-based (tested with Qt-5.12.7) applications. Sample error 
output with trunk@r12+876 -std=gnu++14:

/usr/include/qt5/QtCore/qvariant.h:470:33: error: attribute appertains 
to a friend declaration that is not a definition
      friend Q_CORE_EXPORT QDebug operator<<(QDebug, const QVariant &);
                                  ^~~~~~~~

For GCC Q_CORE_EXPORT is defined (via Q_DECL_EXPORT) to 
__attribute__((visibility("default"))) AFAICS.

As this error seemingly cannot be turned into a warning, it's probably 
quite a problem for many people.

regards,
Franz
Marek Polacek May 18, 2021, 5:46 p.m. UTC | #10
On Tue, May 18, 2021 at 07:35:52PM +0200, Franz Sirl wrote:
> Am 2021-05-14 um 00:08 schrieb Marek Polacek via Gcc-patches:
> > On Wed, May 12, 2021 at 08:27:18PM -0400, Jason Merrill wrote:
> > > On 5/12/21 8:03 PM, Marek Polacek wrote:
> > > > diff --git a/gcc/cp/decl2.c b/gcc/cp/decl2.c
> > > > index 89f874a32cc..2bcefb619aa 100644
> > > > --- a/gcc/cp/decl2.c
> > > > +++ b/gcc/cp/decl2.c
> > > > @@ -1331,6 +1331,20 @@ any_dependent_type_attributes_p (tree attrs)
> > > >      return false;
> > > >    }
> > > > +/* True if ATTRS contains any attribute that requires a type.  */
> > > 
> > > Let's invert this to check if ATTRS contains any attribute that does *not*
> > > require a type, and would therefore apply to the decl.
> > 
> > Sounds good, done.  Now I don't need to check *attrlist.
> > I've also fixed up the xfail thing in my new test.
> > 
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > 
> > -- >8 --
> > 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 applies to C++11-style attributes as well as GNU
> > attributes with the exception that we allow GNU attributes that require
> > a type, such as vector_size to continue accepting code as in attrib63.C.
> > 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.
> > 
> > Jason provided the cp_parser_decl_specifier_seq hunk to detect using
> > standard attributes in the middle of decl-specifiers, which is invalid.
> > 
> > Co-authored-by: Jason Merrill <jason@redhat.com>
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	PR c++/99032
> > 	* cp-tree.h (any_non_type_attribute_p): Declare.
> > 	* decl.c (grokdeclarator): Diagnose when an attribute appertains to
> > 	a friend declaration that is not a definition.
> > 	* decl2.c (any_non_type_attribute_p): New.
> > 	* parser.c (cp_parser_decl_specifier_seq): Diagnose standard attributes
> > 	in the middle of decl-specifiers.
> > 	(cp_parser_elaborated_type_specifier): Diagnose when an attribute
> > 	appertains to a friend declaration that is not a definition.
> > 	(cp_parser_member_declaration): Likewise.
> > 
> 
> Hi,
> 
> I haven't investigated it in detail yet, but it seems this change breaks
> building Qt-based (tested with Qt-5.12.7) applications. Sample error output
> with trunk@r12+876 -std=gnu++14:
> 
> /usr/include/qt5/QtCore/qvariant.h:470:33: error: attribute appertains to a
> friend declaration that is not a definition
>      friend Q_CORE_EXPORT QDebug operator<<(QDebug, const QVariant &);
>                                  ^~~~~~~~
> 
> For GCC Q_CORE_EXPORT is defined (via Q_DECL_EXPORT) to
> __attribute__((visibility("default"))) AFAICS.
> 
> As this error seemingly cannot be turned into a warning, it's probably quite
> a problem for many people.

Yes, I'm working on a patch that will fix this (hoping to submit it today).

Sorry about the breakage.

Marek
diff mbox series

Patch

diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index bc3928d7f85..687a59d49e3 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -13741,6 +13741,10 @@  grokdeclarator (const cp_declarator *declarator,
 
 	if (friendp)
 	  {
+	    if (attrlist && !funcdef_flag
+		&& cxx11_attribute_p (*attrlist))
+	      error_at (id_loc, "attribute appertains to a friend declaration "
+			"that is not a definition");
 	    /* Friends are treated specially.  */
 	    if (ctype == current_class_type)
 	      ;  /* We already issued a permerror.  */
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 0fe29c658d2..612ca4598b9 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -19764,11 +19764,15 @@  cp_parser_elaborated_type_specifier (cp_parser* parser,
 	       && ! processing_explicit_instantiation)
 	warning (OPT_Wattributes,
 		 "attributes ignored on template instantiation");
+      else if (is_friend && cxx11_attribute_p (attributes))
+	error ("attribute appertains to a friend declaration that is not "
+	       "a definition");
       else if (is_declaration && cp_parser_declares_only_class_p (parser))
 	cplus_decl_attributes (&type, attributes, (int) ATTR_FLAG_TYPE_IN_PLACE);
       else
 	warning (OPT_Wattributes,
-		 "attributes ignored on elaborated-type-specifier that is not a forward declaration");
+		 "attributes ignored on elaborated-type-specifier that is "
+		 "not a forward declaration");
     }
 
   if (tag_type == enum_type)
@@ -26054,6 +26058,15 @@  cp_parser_member_declaration (cp_parser* parser)
 		 error_at (decl_spec_token_start->location,
 			   "friend declaration does not name a class or "
 			   "function");
+	       /* Give an error if an attribute cannot appear here, as per
+		  [dcl.attr.grammar]/5.  But not when declares_class_or_enum:
+		  we ignore attributes in elaborated-type-specifiers.  */
+	       else if (!declares_class_or_enum
+			&& (cxx11_attribute_p (decl_specifiers.std_attributes)
+			    || cxx11_attribute_p (decl_specifiers.attributes)))
+		 error_at (decl_spec_token_start->location,
+			   "attribute appertains to a friend declaration "
+			   "that is not a definition");
 	       else
 		 make_friend_class (current_class_type, type,
 				    /*complain=*/true);
diff --git a/gcc/testsuite/g++.dg/cpp0x/friend7.C b/gcc/testsuite/g++.dg/cpp0x/friend7.C
new file mode 100644
index 00000000000..4aa7b14cf7d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/friend7.C
@@ -0,0 +1,41 @@ 
+// PR c++/99032
+// { dg-do compile { target c++11 } }
+
+class X { };
+template<typename T1, typename T2>
+void foo (T1, T2);
+
+struct S {
+  [[deprecated]] friend void f(); // { dg-error "attribute appertains" }
+  [[deprecated]] friend void f2() { }
+  __attribute__((deprecated)) friend void f3();
+  friend void f3 [[deprecated]] (); // { dg-error "attribute appertains" }
+  friend void f4 [[deprecated]] () { }
+  [[deprecated]] friend void; // { dg-error "attribute appertains" }
+  friend [[deprecated]] void; // { dg-error "attribute appertains" }
+  __attribute__((deprecated)) friend int;
+  friend __attribute__((deprecated)) int;
+  friend int __attribute__((deprecated));
+  [[deprecated]] friend X; // { dg-error "attribute appertains" }
+  [[deprecated]] friend class N; // { dg-warning "attribute ignored" }
+  friend class [[deprecated]] N2; // { dg-error "attribute appertains" }
+  friend class __attribute__((deprecated)) N3;
+  [[deprecated]] friend void foo<>(int, int); // { dg-error "attribute appertains" }
+  // FIXME: Add dg error when PR100339 is resolved.
+  //[[deprecated]] friend void ::foo(int, int);
+};
+
+template<typename T>
+class node { };
+
+template<typename T>
+struct A {
+  [[deprecated]] friend T; // { dg-error "attribute appertains" }
+  [[deprecated]] friend class node<T>; // { dg-warning "attribute ignored" }
+  template<typename>
+  [[deprecated]] friend class A; // { dg-warning "attribute ignored" }
+  template<typename>
+  [[deprecated]] friend void bar () { }
+  template<typename>
+  [[deprecated]] friend void baz (); // { dg-error "attribute appertains" }
+};