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

[Jason Merrill <jason@redhat.com>]

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
>