c++: Fix pretty printing of function pointer type [PR98767]

When pretty printing a function pointer type via
pp_cxx_parameter_declaration_clause, we end up always printing an empty
parameter list because the loop that's supposed to print the parameter
list iterates over 'args' instead of 'types', and 'args' is empty in
this case when a FUNCTION_TYPE is passed to this routine (as opposed
to a FUNCTION_DECL).

This patch fixes this by making the loop iterator over 'types' instead.
This patch also moves the retrofitted PARM_DECL printing from this
routine to pp_cxx_requires_expr, the only caller that uses it.  This
simplification lets us easily output the trailing '...' in the parameter
list of a variadic function, which this patch also implements.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

gcc/cp/ChangeLog:

	PR c++/98767
	* cxx-pretty-print.c (pp_cxx_parameter_declaration_clause): Fix
	loop over parameter list to iterate over 'types' instead of
	'args'.  Output the trailing '...' for a variadic function.
	Remove PARM_DECL support.
	(pp_cxx_requires_expr): Pretty print the parameter list directly
	instead of going through pp_cxx_parameter_declaration_clause.

gcc/testsuite/ChangeLog:

	PR c++/98767
	* g++.dg/concepts/diagnostic16.C: New test.
---
 gcc/cp/cxx-pretty-print.c                    | 48 ++++++++++++--------
 gcc/testsuite/g++.dg/concepts/diagnostic16.C | 17 +++++++
 2 files changed, 46 insertions(+), 19 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic16.C

On 4/16/21 6:58 PM, Patrick Palka wrote:
> When pretty printing a function pointer type via
> pp_cxx_parameter_declaration_clause, we end up always printing an empty
> parameter list because the loop that's supposed to print the parameter
> list iterates over 'args' instead of 'types', and 'args' is empty in
> this case when a FUNCTION_TYPE is passed to this routine (as opposed
> to a FUNCTION_DECL).
> 
> This patch fixes this by making the loop iterator over 'types' instead.
> This patch also moves the retrofitted PARM_DECL printing from this
> routine to pp_cxx_requires_expr, the only caller that uses it.  This
> simplification lets us easily output the trailing '...' in the parameter
> list of a variadic function, which this patch also implements.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?

OK.

> gcc/cp/ChangeLog:
> 
> 	PR c++/98767
> 	* cxx-pretty-print.c (pp_cxx_parameter_declaration_clause): Fix
> 	loop over parameter list to iterate over 'types' instead of
> 	'args'.  Output the trailing '...' for a variadic function.
> 	Remove PARM_DECL support.
> 	(pp_cxx_requires_expr): Pretty print the parameter list directly
> 	instead of going through pp_cxx_parameter_declaration_clause.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/98767
> 	* g++.dg/concepts/diagnostic16.C: New test.
> ---
>   gcc/cp/cxx-pretty-print.c                    | 48 ++++++++++++--------
>   gcc/testsuite/g++.dg/concepts/diagnostic16.C | 17 +++++++
>   2 files changed, 46 insertions(+), 19 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/concepts/diagnostic16.C
> 
> diff --git a/gcc/cp/cxx-pretty-print.c b/gcc/cp/cxx-pretty-print.c
> index a22eea5239c..894472e26e0 100644
> --- a/gcc/cp/cxx-pretty-print.c
> +++ b/gcc/cp/cxx-pretty-print.c
> @@ -1537,34 +1537,27 @@ pp_cxx_parameter_declaration (cxx_pretty_printer *pp, tree t)
>   static void
>   pp_cxx_parameter_declaration_clause (cxx_pretty_printer *pp, tree t)
>   {
> -  tree args;
> -  tree types;
> -  bool abstract;
> -
> -  // For a requires clause or the explicit printing of a parameter list
> -  // we expect T to be a chain of PARM_DECLs. Otherwise, the list of
> -  // args and types are taken from the function decl T.
> -  if (TREE_CODE (t) == PARM_DECL)
> +  gcc_assert (FUNC_OR_METHOD_TYPE_P (t) || TREE_CODE (t) == FUNCTION_DECL);
> +  tree types, args;
> +  if (TYPE_P (t))
>       {
> -      args = t;
> -      types = t;
> -      abstract = false;
> +      types = TYPE_ARG_TYPES (t);
> +      args = NULL_TREE;
>       }
>     else
>       {
> -      bool type_p = TYPE_P (t);
> -      args = type_p ? NULL : FUNCTION_FIRST_USER_PARM (t);
> -      types = type_p ? TYPE_ARG_TYPES (t) : FUNCTION_FIRST_USER_PARMTYPE (t);
> -      abstract = args == NULL || pp->flags & pp_c_flag_abstract;
> +      types = FUNCTION_FIRST_USER_PARMTYPE (t);
> +      args = FUNCTION_FIRST_USER_PARM (t);
>       }
> -  bool first = true;
> +  bool abstract = !args || (pp->flags & pp_c_flag_abstract);
>   
>     /* Skip artificial parameter for non-static member functions.  */
>     if (TREE_CODE (t) == METHOD_TYPE)
>       types = TREE_CHAIN (types);
>   
> +  bool first = true;
>     pp_cxx_left_paren (pp);
> -  for (; args; args = TREE_CHAIN (args), types = TREE_CHAIN (types))
> +  for (; types && types != void_list_node; types = TREE_CHAIN (types))
>       {
>         if (!first)
>   	pp_cxx_separate_with (pp, ',');
> @@ -1577,6 +1570,14 @@ pp_cxx_parameter_declaration_clause (cxx_pretty_printer *pp, tree t)
>   	  pp_cxx_whitespace (pp);
>   	  pp->assignment_expression (TREE_PURPOSE (types));
>   	}
> +      if (!abstract)
> +	args = TREE_CHAIN (args);
> +    }
> +  if (!types)
> +    {
> +      if (!first)
> +	pp_cxx_separate_with (pp, ',');
> +      pp_cxx_ws_string (pp, "...");
>       }
>     pp_cxx_right_paren (pp);
>   }
> @@ -2775,9 +2776,18 @@ void
>   pp_cxx_requires_expr (cxx_pretty_printer *pp, tree t)
>   {
>     pp_string (pp, "requires");
> -  if (tree parms = TREE_OPERAND (t, 0))
> +  if (tree parms = REQUIRES_EXPR_PARMS (t))
>       {
> -      pp_cxx_parameter_declaration_clause (pp, parms);
> +      bool first = true;
> +      pp_cxx_left_paren (pp);
> +      for (; parms; parms = TREE_CHAIN (parms))
> +	{
> +	  if (!first)
> +	    pp_cxx_separate_with (pp, ',' );
> +	  first = false;
> +	  pp_cxx_parameter_declaration (pp, parms);
> +	}
> +      pp_cxx_right_paren (pp);
>         pp_cxx_whitespace (pp);
>       }
>     pp_cxx_requirement_body (pp, TREE_OPERAND (t, 1));
> diff --git a/gcc/testsuite/g++.dg/concepts/diagnostic16.C b/gcc/testsuite/g++.dg/concepts/diagnostic16.C
> new file mode 100644
> index 00000000000..49d5733faea
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/concepts/diagnostic16.C
> @@ -0,0 +1,17 @@
> +// PR c++/98767
> +// { dg-do compile { target c++20 } }
> +
> +template <typename Function, typename... Args>
> +concept Callable = requires(Function func, Args... args) { func(args...); };
> +
> +static_assert(Callable<int(*)(), bool>); // { dg-error "failed" }
> +// { dg-message {Function = int \(\*\)\(\)} "" { target *-*-* } 5 }
> +
> +static_assert(Callable<char(*)(int*), bool>); // { dg-error "failed" }
> +// { dg-message {Function = char \(\*\)\(int\*\)} "" { target *-*-* } 5 }
> +
> +static_assert(Callable<short(*)(int*, int), bool>); // { dg-error "failed" }
> +// { dg-message {Function = short int \(\*\)\(int\*, int\)} "" { target *-*-* } 5 }
> +
> +static_assert(Callable<long(*)(int*, int, ...), bool>); // { dg-error "failed" }
> +// { dg-message {Function = long int \(\*\)\(int\*, int, \.\.\.\)} "" { target *-*-* } 5 }
>