c++: Reject attributes without arguments used as pack expansion [PR109756]

Hi!

The following testcase shows we silently accept (and ignore) attributes without
arguments used as pack expansions.  This is because we call
make_pack_expansion and that starts with
  if (!arg || arg == error_mark_node)
    return arg;
Now, an attribute without arguments like [[noreturn...]] is IMHO always
invalid, in this case for 2 reasons; one is that as it has no arguments,
no pack can be present and second is that the standard says that
attributes need to specially permit uses of parameter pack and doesn't
explicitly permit it for any of the standard attributes (except for alignas?
which has different syntax).
If an attribute has some arguments but doesn't contain packs in those
arguments, make_pack_expansion will already diagnose it.

My first version of the patch just added the last hunk in parser.cc and
without the && !has_args, but that unfortunately regresses
gen-attrs-65.C - TREE_VALUE (attribute) == NULL_TREE can mean a recognized
or unrecognized attribute which doesn't have any arguments, but can also
mean an unrecognized attribute which had arguments but which we didn't
parse because we didn't know how to parse those arguments.

So, the patch remembers from cp_parser_std_attribute whether an attribute
had or didn't have arguments and diagnoses it just in the latter case.

Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
I guess this isn't appropriate for backports as accepts-invalid.

2023-05-09  Jakub Jelinek  <jakub@redhat.com>

	PR c++/109756
	* parser.cc (cp_parser_std_attribute): Add HAS_ARGS argument,
	set it to true iff the attribute has arguments specified.
	(cp_parser_std_attribute_list): Adjust caller.  If ... is used
	after attribute without arguments, diagnose it and return
	error_mark_node.

	* g++.dg/cpp0x/gen-attrs-78.C: New test.


	Jakub

On 5/9/23 04:12, Jakub Jelinek wrote:
> Hi!
> 
> The following testcase shows we silently accept (and ignore) attributes without
> arguments used as pack expansions.  This is because we call
> make_pack_expansion and that starts with
>    if (!arg || arg == error_mark_node)
>      return arg;
> Now, an attribute without arguments like [[noreturn...]] is IMHO always
> invalid, in this case for 2 reasons; one is that as it has no arguments,
> no pack can be present and second is that the standard says that
> attributes need to specially permit uses of parameter pack and doesn't
> explicitly permit it for any of the standard attributes (except for alignas?
> which has different syntax).
> If an attribute has some arguments but doesn't contain packs in those
> arguments, make_pack_expansion will already diagnose it.
> 
> My first version of the patch just added the last hunk in parser.cc and
> without the && !has_args, but that unfortunately regresses
> gen-attrs-65.C - TREE_VALUE (attribute) == NULL_TREE can mean a recognized
> or unrecognized attribute which doesn't have any arguments, but can also
> mean an unrecognized attribute which had arguments but which we didn't
> parse because we didn't know how to parse those arguments.

How about changing cp_parser_std_attribute to set TREE_VALUE to 
error_mark_node if it skips arguments?

> So, the patch remembers from cp_parser_std_attribute whether an attribute
> had or didn't have arguments and diagnoses it just in the latter case.
> 
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> I guess this isn't appropriate for backports as accepts-invalid.
> 
> 2023-05-09  Jakub Jelinek  <jakub@redhat.com>
> 
> 	PR c++/109756
> 	* parser.cc (cp_parser_std_attribute): Add HAS_ARGS argument,
> 	set it to true iff the attribute has arguments specified.
> 	(cp_parser_std_attribute_list): Adjust caller.  If ... is used
> 	after attribute without arguments, diagnose it and return
> 	error_mark_node.
> 
> 	* g++.dg/cpp0x/gen-attrs-78.C: New test.
> 
> --- gcc/cp/parser.cc.jj	2023-04-25 16:40:42.010723809 +0200
> +++ gcc/cp/parser.cc	2023-05-08 12:47:58.767924380 +0200
> @@ -2634,7 +2634,7 @@ static tree cp_parser_gnu_attributes_opt
>   static tree cp_parser_gnu_attribute_list
>     (cp_parser *, bool = false);
>   static tree cp_parser_std_attribute
> -  (cp_parser *, tree);
> +  (cp_parser *, tree, bool &);
>   static tree cp_parser_std_attribute_spec
>     (cp_parser *);
>   static tree cp_parser_std_attribute_spec_seq
> @@ -29313,7 +29313,7 @@ cp_parser_omp_sequence_args (cp_parser *
>         { balanced-token-seq }.  */
>   
>   static tree
> -cp_parser_std_attribute (cp_parser *parser, tree attr_ns)
> +cp_parser_std_attribute (cp_parser *parser, tree attr_ns, bool &has_args)
>   {
>     tree attribute, attr_id = NULL_TREE, arguments;
>     cp_token *token;
> @@ -29323,6 +29323,7 @@ cp_parser_std_attribute (cp_parser *pars
>   
>     /* First, parse name of the attribute, a.k.a attribute-token.  */
>   
> +  has_args = false;
>     token = cp_lexer_peek_token (parser->lexer);
>     if (token->type == CPP_NAME)
>       attr_id = token->u.value;
> @@ -29419,6 +29420,7 @@ cp_parser_std_attribute (cp_parser *pars
>         return attribute;
>       }
>   
> +  has_args = true;
>     {
>       vec<tree, va_gc> *vec;
>       int attr_flag = normal_attr;
> @@ -29544,7 +29546,8 @@ cp_parser_std_attribute_list (cp_parser
>     while (true)
>       {
>         location_t loc = cp_lexer_peek_token (parser->lexer)->location;
> -      attribute = cp_parser_std_attribute (parser, attr_ns);
> +      bool has_args;
> +      attribute = cp_parser_std_attribute (parser, attr_ns, has_args);
>         if (attribute == error_mark_node)
>   	break;
>         if (attribute != NULL_TREE)
> @@ -29562,6 +29565,12 @@ cp_parser_std_attribute_list (cp_parser
>   	  if (attribute == NULL_TREE)
>   	    error_at (token->location,
>   		      "expected attribute before %<...%>");
> +	  else if (TREE_VALUE (attribute) == NULL_TREE && !has_args)
> +	    {
> +	      error_at (token->location, "attribute with no arguments "
> +					 "contains no parameter packs");
> +	      return error_mark_node;
> +	    }
>   	  else
>   	    {
>   	      tree pack = make_pack_expansion (TREE_VALUE (attribute));
> --- gcc/testsuite/g++.dg/cpp0x/gen-attrs-78.C.jj	2023-05-08 12:33:13.387581760 +0200
> +++ gcc/testsuite/g++.dg/cpp0x/gen-attrs-78.C	2023-05-08 12:32:23.146301128 +0200
> @@ -0,0 +1,29 @@
> +// PR c++/109756
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-Wno-attributes" }
> +
> +template <int ...args>
> +[[noreturn...]]					// { dg-error "attribute with no arguments contains no parameter packs" }
> +[[deprecated...]]				// { dg-error "attribute with no arguments contains no parameter packs" }
> +[[nodiscard...]]				// { dg-error "attribute with no arguments contains no parameter packs" }
> +int foo (int x)
> +{
> +  switch (x)
> +    {
> +    case 1:
> +      [[likely...]];				// { dg-error "attribute with no arguments contains no parameter packs" }
> +      [[fallthrough...]];			// { dg-error "attribute with no arguments contains no parameter packs" }
> +    case 2:
> +      [[unlikely...]];				// { dg-error "attribute with no arguments contains no parameter packs" }
> +
> +      break;
> +    default:
> +      break;
> +    }
> +  struct T {};
> +  struct S { [[no_unique_address...]] T t; };	// { dg-error "attribute with no arguments contains no parameter packs" }
> +  for (;;)
> +    ;
> +}
> +
> +int a = foo <1, 2, 3> (4);
> 
> 	Jakub
>