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

[Jason Merrill via Gcc-patches]

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 000..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 
+concept Callable = requires(Function func, Args... args) { func(args...); };
+
+static_assert(Callable); // { dg-error "failed" }
+// { dg-message {Function = int \(\*\)\(\)} "" { target *-*-* } 5 }
+
+static_assert(Callable); // { dg-error "failed" }
+// { 

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

[Patrick Palka via Gcc-patches]

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

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 000..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 
+concept Callable = requires(Function func, Args... args) { func(args...); };
+
+static_assert(Callable); // { dg-error "failed" }
+// { dg-message {Function = int \(\*\)\(\)} "" { target *-*-* } 5 }
+
+static_assert(Callable); // { dg-error "failed" }
+// { dg-message {Function = char \(\*\)\(int\*\)} "" { target *-*-* } 5 }
+