[PATCH] c++: unexpanded pack in requires-expr parm list [PR107417]

[PATCH] c++: unexpanded pack in requires-expr parm list [PR107417]

[Patrick Palka]

Here find_parameter_packs_r isn't recognizing the unexpanded pack T
inside the requires-expr's parameter list ultimately because
cp_walk_trees avoids walking the parameters, for good reason: in

  requires (Ts... ts) { }

walking 'ts' would trigger a false positive in the unexpanded pack
checker since it's, well, an unexpanded pack.  But we might as well walk
the TREE_TYPE of each parameter still, which for_each_template_parm_r
already does.

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

	PR c++/107417

gcc/cp/ChangeLog:

	* pt.cc (for_each_template_parm_r) <case REQUIRES_EXPR>: Move
	walking of the TREE_TYPE of each parameter to ...
	* tree.cc (cp_walk_subtrees) <case REQUIRES_EXPR>: ... here.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/concepts-requires33.C: New test.
---
 gcc/cp/pt.cc                                  | 15 --------------
 gcc/cp/tree.cc                                | 20 ++++++++++---------
 .../g++.dg/cpp2a/concepts-requires33.C        | 11 ++++++++++
 3 files changed, 22 insertions(+), 24 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-requires33.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index bc8ea06ceae..80110da2d8a 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -10573,21 +10573,6 @@ for_each_template_parm_r (tree *tp, int *walk_subtrees, void *d)
 	return error_mark_node;
       break;
 
-    case REQUIRES_EXPR:
-      {
-	if (!fn)
-	  return error_mark_node;
-
-	/* Recursively walk the type of each constraint variable.  */
-	tree p = TREE_OPERAND (t, 0);
-	while (p)
-	  {
-	    WALK_SUBTREE (TREE_TYPE (p));
-	    p = TREE_CHAIN (p);
-	  }
-      }
-      break;
-
     default:
       break;
     }
diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
index 04a055d9d77..e9b6921df6a 100644
--- a/gcc/cp/tree.cc
+++ b/gcc/cp/tree.cc
@@ -5603,15 +5603,17 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
       break;
  
     case REQUIRES_EXPR:
-      // Only recurse through the nested expression. Do not
-      // walk the parameter list. Doing so causes false
-      // positives in the pack expansion checker since the
-      // requires parameters are introduced as pack expansions.
-      ++cp_unevaluated_operand;
-      result = cp_walk_tree (&REQUIRES_EXPR_REQS (*tp), func, data, pset);
-      --cp_unevaluated_operand;
-      *walk_subtrees_p = 0;
-      break;
+      {
+	cp_unevaluated u;
+	for (tree parm = REQUIRES_EXPR_PARMS (*tp); parm; parm = DECL_CHAIN (parm))
+	  /* Walk the types of each parameter, but not the parameter itself.
+	     Doing so would cause false positives in the unexpanded parameter
+	     pack checker since a introduced parameter pack is bare.  */
+	  WALK_SUBTREE (TREE_TYPE (parm));
+	WALK_SUBTREE (REQUIRES_EXPR_REQS (*tp));
+	*walk_subtrees_p = 0;
+	break;
+      }
 
     case DECL_EXPR:
       /* User variables should be mentioned in BIND_EXPR_VARS
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-requires33.C b/gcc/testsuite/g++.dg/cpp2a/concepts-requires33.C
new file mode 100644
index 00000000000..d07ae6dee7b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-requires33.C
@@ -0,0 +1,11 @@
+// PR c++/107417
+// { dg-do compile { target c++20 } }
+
+template<typename... T>
+requires (requires (T x) { true; } && ...)
+void f();
+
+int main() {
+  f<int>();
+  f<int, void>(); // { dg-error "no match" }
+}
-- 
2.39.0.rc0.49.g083e01275b

Re: [PATCH] c++: unexpanded pack in requires-expr parm list [PR107417]

[Jason Merrill]

On 12/2/22 14:42, Patrick Palka wrote:
> Here find_parameter_packs_r isn't recognizing the unexpanded pack T
> inside the requires-expr's parameter list ultimately because
> cp_walk_trees avoids walking the parameters, for good reason: in
> 
>    requires (Ts... ts) { }
> 
> walking 'ts' would trigger a false positive in the unexpanded pack
> checker since it's, well, an unexpanded pack.  But we might as well walk
> the TREE_TYPE of each parameter still, which for_each_template_parm_r
> already does.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gne, does this look OK for
> trunk/12?

OK.

> 	PR c++/107417
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.cc (for_each_template_parm_r) <case REQUIRES_EXPR>: Move
> 	walking of the TREE_TYPE of each parameter to ...
> 	* tree.cc (cp_walk_subtrees) <case REQUIRES_EXPR>: ... here.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/concepts-requires33.C: New test.
> ---
>   gcc/cp/pt.cc                                  | 15 --------------
>   gcc/cp/tree.cc                                | 20 ++++++++++---------
>   .../g++.dg/cpp2a/concepts-requires33.C        | 11 ++++++++++
>   3 files changed, 22 insertions(+), 24 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-requires33.C
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index bc8ea06ceae..80110da2d8a 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -10573,21 +10573,6 @@ for_each_template_parm_r (tree *tp, int *walk_subtrees, void *d)
>   	return error_mark_node;
>         break;
>   
> -    case REQUIRES_EXPR:
> -      {
> -	if (!fn)
> -	  return error_mark_node;
> -
> -	/* Recursively walk the type of each constraint variable.  */
> -	tree p = TREE_OPERAND (t, 0);
> -	while (p)
> -	  {
> -	    WALK_SUBTREE (TREE_TYPE (p));
> -	    p = TREE_CHAIN (p);
> -	  }
> -      }
> -      break;
> -
>       default:
>         break;
>       }
> diff --git a/gcc/cp/tree.cc b/gcc/cp/tree.cc
> index 04a055d9d77..e9b6921df6a 100644
> --- a/gcc/cp/tree.cc
> +++ b/gcc/cp/tree.cc
> @@ -5603,15 +5603,17 @@ cp_walk_subtrees (tree *tp, int *walk_subtrees_p, walk_tree_fn func,
>         break;
>    
>       case REQUIRES_EXPR:
> -      // Only recurse through the nested expression. Do not
> -      // walk the parameter list. Doing so causes false
> -      // positives in the pack expansion checker since the
> -      // requires parameters are introduced as pack expansions.
> -      ++cp_unevaluated_operand;
> -      result = cp_walk_tree (&REQUIRES_EXPR_REQS (*tp), func, data, pset);
> -      --cp_unevaluated_operand;
> -      *walk_subtrees_p = 0;
> -      break;
> +      {
> +	cp_unevaluated u;
> +	for (tree parm = REQUIRES_EXPR_PARMS (*tp); parm; parm = DECL_CHAIN (parm))
> +	  /* Walk the types of each parameter, but not the parameter itself.
> +	     Doing so would cause false positives in the unexpanded parameter
> +	     pack checker since a introduced parameter pack is bare.  */
> +	  WALK_SUBTREE (TREE_TYPE (parm));
> +	WALK_SUBTREE (REQUIRES_EXPR_REQS (*tp));
> +	*walk_subtrees_p = 0;
> +	break;
> +      }
>   
>       case DECL_EXPR:
>         /* User variables should be mentioned in BIND_EXPR_VARS
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-requires33.C b/gcc/testsuite/g++.dg/cpp2a/concepts-requires33.C
> new file mode 100644
> index 00000000000..d07ae6dee7b
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-requires33.C
> @@ -0,0 +1,11 @@
> +// PR c++/107417
> +// { dg-do compile { target c++20 } }
> +
> +template<typename... T>
> +requires (requires (T x) { true; } && ...)
> +void f();
> +
> +int main() {
> +  f<int>();
> +  f<int, void>(); // { dg-error "no match" }
> +}