Re: [PATCH] c++: Don't diagnose ignoring of attributes if all ignored attributes are attribute_ignored_p

[Jason Merrill <jason@redhat.com>]

On 12/6/23 09:10, Jakub Jelinek wrote:
> On Tue, Dec 05, 2023 at 11:01:20AM -0500, Jason Merrill wrote:
>>> And there is another thing I wonder about: with -Wno-attributes= we are
>>> supposed to ignore the attributes altogether, but we are actually still
>>> warning about them when we emit these generic warnings about ignoring
>>> all attributes which appertain to this and that (perhaps with some
>>> exceptions we first remove from the attribute chain), like:
>>> void foo () { [[foo::bar]]; }
>>> with -Wattributes -Wno-attributes=foo::bar
>>> Shouldn't we call some helper function in cases like this and warn
>>> not when std_attrs (or how the attribute chain var is called) is non-NULL,
>>> but if it is non-NULL and contains at least one non-attribute_ignored_p
>>> attribute?
>>
>> Sounds good.
> 
> The following patch implements it.
> I've kept warnings for cases where the C++ standard says explicitly any
> attributes aren't ok -
> "If an attribute-specifier-seq appertains to a friend declaration, that
> declaration shall be a definition."
> 
> For some changes I haven't figured out how could I cover it in the
> testsuite.
> 
> So far tested with
> GXX_TESTSUITE_STDS=98,11,14,17,20,23,26 make check-g++ RUNTESTFLAGS="dg.exp=Wno-attributes* ubsan.exp=Wno-attributes*"
> (which is all tests that use -Wno-attributes=), ok for trunk if it passes
> full bootstrap/regtest?
> 
> Note, C uses a different strategy, it has c_warn_unused_attributes
> function which warns about all the attributes one by one unless they
> are ignored (or allowed in certain position).
> Though that is just a single diagnostic wording, while C++ FE just warns
> that there are some ignored attributes and doesn't name them individually
> (except for namespace and using namespace) and uses different wordings in
> different spots.
> 
> 2023-12-06  Jakub Jelinek  <jakub@redhat.com>
> 
> gcc/
> 	* attribs.h (any_nonignored_attribute_p): Declare.
> 	* attribs.cc (any_nonignored_attribute_p): New function.
> gcc/cp/
> 	* parser.cc (cp_parser_statement, cp_parser_expression_statement,
> 	cp_parser_declaration, cp_parser_elaborated_type_specifier,
> 	cp_parser_asm_definition): Don't diagnose ignored attributes
> 	if !any_nonignored_attribute_p.
> 	* decl.cc (grokdeclarator): Likewise.
> 	* name-lookup.cc (handle_namespace_attrs, finish_using_directive):
> 	Don't diagnose ignoring of attr_ignored_p attributes.
> gcc/testsuite/
> 	* g++.dg/warn/Wno-attributes-1.C: New test.
> 
> --- gcc/cp/parser.cc.jj	2023-12-06 12:03:27.502174967 +0100
> +++ gcc/cp/parser.cc	2023-12-06 12:36:55.704884514 +0100
> @@ -21095,14 +21094,20 @@ cp_parser_elaborated_type_specifier (cp_
>     if (attributes)
>       {
>         if (TREE_CODE (type) == TYPENAME_TYPE)
> -	warning (OPT_Wattributes,
> -		 "attributes ignored on uninstantiated type");
> +	{
> +	  if (any_nonignored_attribute_p (attributes))
> +	    warning (OPT_Wattributes,
> +		     "attributes ignored on uninstantiated type");
> +	}
>         else if (tag_type != enum_type
>   	       && TREE_CODE (type) != BOUND_TEMPLATE_TEMPLATE_PARM
>   	       && CLASSTYPE_TEMPLATE_INSTANTIATION (type)
>   	       && ! processing_explicit_instantiation)
> -	warning (OPT_Wattributes,
> -		 "attributes ignored on template instantiation");
> +	{
> +	  if (any_nonignored_attribute_p (attributes))
> +	    warning (OPT_Wattributes,
> +		     "attributes ignored on template instantiation");
> +	}
>         else if (is_friend && cxx11_attribute_p (attributes))
>   	{
>   	  if (warning (OPT_Wattributes, "attribute ignored"))
> @@ -21111,7 +21116,7 @@ cp_parser_elaborated_type_specifier (cp_
>   	}
>         else if (is_declaration && cp_parser_declares_only_class_p (parser))
>   	cplus_decl_attributes (&type, attributes, (int) ATTR_FLAG_TYPE_IN_PLACE);
> -      else
> +      else if (any_nonignored_attribute_p (attributes))
>   	warning (OPT_Wattributes,
>   		 "attributes ignored on elaborated-type-specifier that is "
>   		 "not a forward declaration");

I believe this is also prohibited by
https://eel.is/c++draft/dcl.type.elab#3

so I would leave all the warnings in this function alone.

> @@ -22672,7 +22677,7 @@ cp_parser_asm_definition (cp_parser* par
>   	symtab->finalize_toplevel_asm (string);
>       }
>   
> -  if (std_attrs)
> +  if (std_attrs && any_nonignored_attribute_p (std_attrs))
>       warning_at (asm_loc, OPT_Wattributes,
>   		"attributes ignored on %<asm%> declaration");
>   }
> --- gcc/cp/decl.cc.jj	2023-12-06 12:03:27.483175235 +0100
> +++ gcc/cp/decl.cc	2023-12-06 12:36:55.698884598 +0100
> @@ -13058,7 +13058,8 @@ grokdeclarator (const cp_declarator *dec
>         && !diagnose_misapplied_contracts (declspecs->std_attributes))
>       {
>         location_t attr_loc = declspecs->locations[ds_std_attribute];
> -      if (warning_at (attr_loc, OPT_Wattributes, "attribute ignored"))
> +      if (any_nonignored_attribute_p (declspecs->std_attributes)
> +	  && warning_at (attr_loc, OPT_Wattributes, "attribute ignored"))
>   	inform (attr_loc, "an attribute that appertains to a type-specifier "
>   		"is ignored");
>       }

This seems untested, e.g.

int [[foo::bar]] i;

Jason