diff mbox series

[1/2] c++: requires-exprs and partial constraint subst [PR112769]

Message ID 20240202194104.317982-1-ppalka@redhat.com
State New
Headers show
Series [1/2] c++: requires-exprs and partial constraint subst [PR112769] | expand

Commit Message

Patrick Palka Feb. 2, 2024, 7:41 p.m. UTC
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this
look OK for trunk?

-- >8 --

In r11-3261-gb28b621ac67bee we made tsubst_requires_expr never partially
substitute into a requires-expression so as to avoid checking its
requirements out of order during e.g. generic lambda regeneration.

Unfortunately we still do need to partially substitute into a
requires-expression in rare cases, in particular when it's used in
associated constraints that we are directly substituting for sake of
declaration matching or dguide constraint rewriting.  We can identify
this situation by checking processing_constraint_expression_p, so this
patch uses this predicate to control whether we defer substitution or
partially substitute.  The entering_scope=true change in tsubst_baselink
is needed to avoid ICEing from tsubst_baselink during name lookup when
rewriting std::ranges::ref_view's dguide constraints.

	PR c++/112769
	PR c++/110006

gcc/cp/ChangeLog:

	* constraint.cc (tsubst_simple_requirement): Return a
	substituted _REQ node when processing_template_decl.
	(tsubst_type_requirement): Likewise.
	(tsubst_compound_requirement): Likewise.
	(tsubst_nested_requirement): Likewise.
	(tsubst_requires_expr): Don't defer partial substitution
	when processing_constraint_expression_p is true, in which
	case return a substituted REQUIRES_EXPR.
	* pt.cc (tsubst_baselink): Use tsubst_aggr_type with
	entring_scope=true instead of tsubst to substitute
	qualifying_scope.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/class-deduction-alias18.C: New test.
	* g++.dg/cpp2a/concepts-friend16.C: New test.
---
 gcc/cp/constraint.cc                          | 38 +++++++++++++++++--
 gcc/cp/pt.cc                                  |  3 +-
 .../g++.dg/cpp2a/class-deduction-alias18.C    | 13 +++++++
 .../g++.dg/cpp2a/concepts-friend16.C          | 25 ++++++++++++
 4 files changed, 74 insertions(+), 5 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/class-deduction-alias18.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend16.C

Comments

Jason Merrill Feb. 2, 2024, 7:55 p.m. UTC | #1
On 2/2/24 14:41, Patrick Palka wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this
> look OK for trunk?

OK.

> -- >8 --
> 
> In r11-3261-gb28b621ac67bee we made tsubst_requires_expr never partially
> substitute into a requires-expression so as to avoid checking its
> requirements out of order during e.g. generic lambda regeneration.
> 
> Unfortunately we still do need to partially substitute into a
> requires-expression in rare cases, in particular when it's used in
> associated constraints that we are directly substituting for sake of
> declaration matching or dguide constraint rewriting.  We can identify
> this situation by checking processing_constraint_expression_p, so this
> patch uses this predicate to control whether we defer substitution or
> partially substitute.  The entering_scope=true change in tsubst_baselink
> is needed to avoid ICEing from tsubst_baselink during name lookup when
> rewriting std::ranges::ref_view's dguide constraints.
> 
> 	PR c++/112769
> 	PR c++/110006
> 
> gcc/cp/ChangeLog:
> 
> 	* constraint.cc (tsubst_simple_requirement): Return a
> 	substituted _REQ node when processing_template_decl.
> 	(tsubst_type_requirement): Likewise.
> 	(tsubst_compound_requirement): Likewise.
> 	(tsubst_nested_requirement): Likewise.
> 	(tsubst_requires_expr): Don't defer partial substitution
> 	when processing_constraint_expression_p is true, in which
> 	case return a substituted REQUIRES_EXPR.
> 	* pt.cc (tsubst_baselink): Use tsubst_aggr_type with
> 	entring_scope=true instead of tsubst to substitute
> 	qualifying_scope.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/class-deduction-alias18.C: New test.
> 	* g++.dg/cpp2a/concepts-friend16.C: New test.
> ---
>   gcc/cp/constraint.cc                          | 38 +++++++++++++++++--
>   gcc/cp/pt.cc                                  |  3 +-
>   .../g++.dg/cpp2a/class-deduction-alias18.C    | 13 +++++++
>   .../g++.dg/cpp2a/concepts-friend16.C          | 25 ++++++++++++
>   4 files changed, 74 insertions(+), 5 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/class-deduction-alias18.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend16.C
> 
> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> index fef68cf7ab2..450ae548f9a 100644
> --- a/gcc/cp/constraint.cc
> +++ b/gcc/cp/constraint.cc
> @@ -2028,6 +2028,8 @@ tsubst_simple_requirement (tree t, tree args, sat_info info)
>     tree expr = tsubst_valid_expression_requirement (t0, args, info);
>     if (expr == error_mark_node)
>       return error_mark_node;
> +  if (processing_template_decl)
> +    return finish_simple_requirement (EXPR_LOCATION (t), expr);
>     return boolean_true_node;
>   }
>   
> @@ -2068,6 +2070,8 @@ tsubst_type_requirement (tree t, tree args, sat_info info)
>     tree type = tsubst_type_requirement_1 (t0, args, info, EXPR_LOCATION (t));
>     if (type == error_mark_node)
>       return error_mark_node;
> +  if (processing_template_decl)
> +    return finish_type_requirement (EXPR_LOCATION (t), type);
>     return boolean_true_node;
>   }
>   
> @@ -2182,6 +2186,9 @@ tsubst_compound_requirement (tree t, tree args, sat_info info)
>   	}
>       }
>   
> +  if (processing_template_decl)
> +    return finish_compound_requirement (EXPR_LOCATION (t),
> +					expr, type, noexcept_p);
>     return boolean_true_node;
>   }
>   
> @@ -2190,6 +2197,15 @@ tsubst_compound_requirement (tree t, tree args, sat_info info)
>   static tree
>   tsubst_nested_requirement (tree t, tree args, sat_info info)
>   {
> +  if (processing_template_decl)
> +    {
> +      tree req = TREE_OPERAND (t, 0);
> +      req = tsubst_constraint (req, args, info.complain, info.in_decl);
> +      if (req == error_mark_node)
> +	return error_mark_node;
> +      return finish_nested_requirement (EXPR_LOCATION (t), req);
> +    }
> +
>     sat_info quiet (tf_none, info.in_decl);
>     tree result = constraint_satisfaction_value (t, args, quiet);
>     if (result == boolean_true_node)
> @@ -2330,18 +2346,25 @@ tsubst_requires_expr (tree t, tree args, sat_info info)
>   
>     args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args,
>   			 info.complain, info.in_decl);
> -  if (processing_template_decl)
> +  if (processing_template_decl
> +      && !processing_constraint_expression_p ())
>       {
>         /* We're partially instantiating a generic lambda.  Substituting into
>   	 this requires-expression now may cause its requirements to get
>   	 checked out of order, so instead just remember the template
> -	 arguments and wait until we can substitute them all at once.  */
> +	 arguments and wait until we can substitute them all at once.
> +
> +	 Except if this requires-expr is part of associated constraints
> +	 that we're substituting into directly (for e.g. declaration
> +	 matching or dguide constraint rewriting), in which case we need
> +	 to partially substitute.  */
>         t = copy_node (t);
>         REQUIRES_EXPR_EXTRA_ARGS (t) = build_extra_args (t, args, info.complain);
>         return t;
>       }
>   
> -  if (tree parms = REQUIRES_EXPR_PARMS (t))
> +  tree parms = REQUIRES_EXPR_PARMS (t);
> +  if (parms)
>       {
>         parms = tsubst_constraint_variables (parms, args, info);
>         if (parms == error_mark_node)
> @@ -2349,10 +2372,13 @@ tsubst_requires_expr (tree t, tree args, sat_info info)
>       }
>   
>     tree result = boolean_true_node;
> +  if (processing_template_decl)
> +    result = NULL_TREE;
>     for (tree reqs = REQUIRES_EXPR_REQS (t); reqs; reqs = TREE_CHAIN (reqs))
>       {
>         tree req = TREE_VALUE (reqs);
> -      if (tsubst_requirement (req, args, info) == error_mark_node)
> +      req = tsubst_requirement (req, args, info);
> +      if (req == error_mark_node)
>   	{
>   	  result = boolean_false_node;
>   	  if (info.diagnose_unsatisfaction_p ())
> @@ -2360,7 +2386,11 @@ tsubst_requires_expr (tree t, tree args, sat_info info)
>   	  else
>   	    break;
>   	}
> +      else if (processing_template_decl)
> +	result = tree_cons (NULL_TREE, req, result);
>       }
> +  if (processing_template_decl && result != boolean_false_node)
> +    result = finish_requires_expr (EXPR_LOCATION (t), parms, nreverse (result));
>     return result;
>   }
>   
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 9d30a271713..17ecd4b9c5a 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -17065,7 +17065,8 @@ tsubst_baselink (tree baselink, tree object_type,
>   {
>     bool qualified_p = BASELINK_QUALIFIED_P (baselink);
>     tree qualifying_scope = BINFO_TYPE (BASELINK_ACCESS_BINFO (baselink));
> -  qualifying_scope = tsubst (qualifying_scope, args, complain, in_decl);
> +  qualifying_scope = tsubst_aggr_type (qualifying_scope, args, complain, in_decl,
> +				       /*entering_scope=*/true);
>   
>     tree optype = BASELINK_OPTYPE (baselink);
>     optype = tsubst (optype, args, complain, in_decl);
> diff --git a/gcc/testsuite/g++.dg/cpp2a/class-deduction-alias18.C b/gcc/testsuite/g++.dg/cpp2a/class-deduction-alias18.C
> new file mode 100644
> index 00000000000..0bb11bb944c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/class-deduction-alias18.C
> @@ -0,0 +1,13 @@
> +// PR c++/112769
> +// { dg-do compile { target c++20 } }
> +
> +template<int I, typename T>
> +struct type
> +{
> +    type(T) requires requires { T{0}; };
> +};
> +
> +template <typename T>
> +using alias = type<0, T>;
> +
> +alias foo{123};
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend16.C b/gcc/testsuite/g++.dg/cpp2a/concepts-friend16.C
> new file mode 100644
> index 00000000000..18974eeb172
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend16.C
> @@ -0,0 +1,25 @@
> +// PR c++/110006
> +// { dg-do compile { target c++20 } }
> +
> +template<typename T>
> +class s;
> +
> +template<typename T>
> +void constraint(s<T> const&, int&);
> +
> +template<typename U, typename T2>
> +U function(s<T2> const x)
> +	requires requires (U& u) { constraint(x, u); };
> +
> +template<typename T>
> +class s
> +{
> +	template<typename U, typename T2>
> +	friend U function(s<T2> const x)
> +		requires requires (U& u) { constraint(x, u); };
> +};
> +
> +int f(s<int> q)
> +{
> +	return function<int>(q); // { dg-bogus "ambiguous" }
> +}
Jason Merrill Feb. 2, 2024, 8:01 p.m. UTC | #2
On 2/2/24 14:41, Patrick Palka wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this
> look OK for trunk?
> 
> -- >8 --
> 
> In r11-3261-gb28b621ac67bee we made tsubst_requires_expr never partially
> substitute into a requires-expression so as to avoid checking its
> requirements out of order during e.g. generic lambda regeneration.
> 
> Unfortunately we still do need to partially substitute into a
> requires-expression in rare cases, in particular when it's used in
> associated constraints that we are directly substituting for sake of
> declaration matching or dguide constraint rewriting.  We can identify
> this situation by checking processing_constraint_expression_p, so this
> patch uses this predicate to control whether we defer substitution or
> partially substitute.  The entering_scope=true change in tsubst_baselink
> is needed to avoid ICEing from tsubst_baselink during name lookup when
> rewriting std::ranges::ref_view's dguide constraints.

Actually, I don't think we want to enter the scope when rewriting 
constraints.  Would tsubst_scope work instead?

Jason
Patrick Palka Feb. 2, 2024, 8:36 p.m. UTC | #3
On Fri, 2 Feb 2024, Jason Merrill wrote:

> On 2/2/24 14:41, Patrick Palka wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this
> > look OK for trunk?
> > 
> > -- >8 --
> > 
> > In r11-3261-gb28b621ac67bee we made tsubst_requires_expr never partially
> > substitute into a requires-expression so as to avoid checking its
> > requirements out of order during e.g. generic lambda regeneration.
> > 
> > Unfortunately we still do need to partially substitute into a
> > requires-expression in rare cases, in particular when it's used in
> > associated constraints that we are directly substituting for sake of
> > declaration matching or dguide constraint rewriting.  We can identify
> > this situation by checking processing_constraint_expression_p, so this
> > patch uses this predicate to control whether we defer substitution or
> > partially substitute.  The entering_scope=true change in tsubst_baselink
> > is needed to avoid ICEing from tsubst_baselink during name lookup when
> > rewriting std::ranges::ref_view's dguide constraints.
> 
> Actually, I don't think we want to enter the scope when rewriting constraints.
> Would tsubst_scope work instead?

Oops yes, because of the maybe_dependent_member_ref stuff, the handling
for which doesn't trigger here for some reason.  Ah, I think it's
because the tf_dguide flag gets dropped during tsubst_requires_expr
since it uses tf_none rather than complain & ~tf_warning_or_error
which would preserve special tsubst flags such as tf_dguide.  I'll fix...

> 
> Jason
> 
>
Patrick Palka Feb. 2, 2024, 8:57 p.m. UTC | #4
On Fri, 2 Feb 2024, Patrick Palka wrote:

> On Fri, 2 Feb 2024, Jason Merrill wrote:
> 
> > On 2/2/24 14:41, Patrick Palka wrote:
> > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this
> > > look OK for trunk?
> > > 
> > > -- >8 --
> > > 
> > > In r11-3261-gb28b621ac67bee we made tsubst_requires_expr never partially
> > > substitute into a requires-expression so as to avoid checking its
> > > requirements out of order during e.g. generic lambda regeneration.
> > > 
> > > Unfortunately we still do need to partially substitute into a
> > > requires-expression in rare cases, in particular when it's used in
> > > associated constraints that we are directly substituting for sake of
> > > declaration matching or dguide constraint rewriting.  We can identify
> > > this situation by checking processing_constraint_expression_p, so this
> > > patch uses this predicate to control whether we defer substitution or
> > > partially substitute.  The entering_scope=true change in tsubst_baselink
> > > is needed to avoid ICEing from tsubst_baselink during name lookup when
> > > rewriting std::ranges::ref_view's dguide constraints.
> > 
> > Actually, I don't think we want to enter the scope when rewriting constraints.
> > Would tsubst_scope work instead?
> 
> Oops yes, because of the maybe_dependent_member_ref stuff, the handling
> for which doesn't trigger here for some reason.  Ah, I think it's
> because the tf_dguide flag gets dropped during tsubst_requires_expr
> since it uses tf_none rather than complain & ~tf_warning_or_error
> which would preserve special tsubst flags such as tf_dguide.  I'll fix...

Like so?  Lightly tested so far, bootstrap+regtest in progress.

-- >8 --

Subject: [PATCH v2] c++: requires-exprs and partial constraint subst [PR112769]

In r11-3261-gb28b621ac67bee we made tsubst_requires_expr never partially
substitute into a requires-expression so as to avoid checking its
requirements out of order during e.g. generic lambda regeneration.

Unfortunately we still do need to partially substitute into a
requires-expression in rare cases, in particular when it's used in
associated constraints that we are directly substituting for sake of
declaration matching or dguide constraint rewriting.  We can identify
this situation by checking processing_constraint_expression_p, so this
patch uses this predicate to control whether we defer substitution or
partially substitute.

In turn, tsubst_requires_expr now needs to propagate semantic tsubst flags
rather than just using tf_none, in particular for dguide constraint rewriting
which sets tf_dguide.

	PR c++/112769
	PR c++/110006

gcc/cp/ChangeLog:

	* constraint.cc (subst_info::quiet): Accomodate non-diagnostic
	tsubst flags.
	(tsubst_valid_expression_requirement): Likewise.
	(tsubst_simple_requirement): Likewise.  Return a substituted _REQ
	node when processing_template_decl.
	(tsubst_type_requirement_1): Accomodate non-diagnostic tsubst
	flags.
	(tsubst_type_requirement): Return a substituted _REQ node when
	processing_template_decl.
	(tsubst_compound_requirement): Likewise.  Accomodate non-diagnostic
	tsubst flags.
	(tsubst_nested_requirement): Likewise.
	(tsubst_requires_expr): Don't defer partial substitution when
	processing_constraint_expression_p is true, in which case return
	a substituted REQUIRES_EXPR.
	* pt.cc (tsubst_expr) <case REQUIRES_EXPR>: Accomodate
	non-diagnostic tsubst flags.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/class-deduction-alias18.C: New test.
	* g++.dg/cpp2a/concepts-friend16.C: New test.
---
 gcc/cp/constraint.cc                          | 56 +++++++++++++++----
 gcc/cp/pt.cc                                  |  3 +-
 .../g++.dg/cpp2a/class-deduction-alias18.C    | 13 +++++
 .../g++.dg/cpp2a/concepts-friend16.C          | 25 +++++++++
 4 files changed, 84 insertions(+), 13 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/class-deduction-alias18.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend16.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index fef68cf7ab2..d9569013bd3 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -85,7 +85,7 @@ struct subst_info
   /* True if we should not diagnose errors.  */
   bool quiet() const
   {
-    return complain == tf_none;
+    return !(complain & tf_warning_or_error);
   }
 
   /* True if we should diagnose errors.  */
@@ -1991,8 +1991,9 @@ hash_placeholder_constraint (tree c)
 static tree
 tsubst_valid_expression_requirement (tree t, tree args, sat_info info)
 {
-  tree r = tsubst_expr (t, args, tf_none, info.in_decl);
-  if (convert_to_void (r, ICV_STATEMENT, tf_none) != error_mark_node)
+  tsubst_flags_t quiet = info.complain & ~tf_warning_or_error;
+  tree r = tsubst_expr (t, args, quiet, info.in_decl);
+  if (convert_to_void (r, ICV_STATEMENT, quiet) != error_mark_node)
     return r;
 
   if (info.diagnose_unsatisfaction_p ())
@@ -2028,6 +2029,8 @@ tsubst_simple_requirement (tree t, tree args, sat_info info)
   tree expr = tsubst_valid_expression_requirement (t0, args, info);
   if (expr == error_mark_node)
     return error_mark_node;
+  if (processing_template_decl)
+    return finish_simple_requirement (EXPR_LOCATION (t), expr);
   return boolean_true_node;
 }
 
@@ -2037,7 +2040,8 @@ tsubst_simple_requirement (tree t, tree args, sat_info info)
 static tree
 tsubst_type_requirement_1 (tree t, tree args, sat_info info, location_t loc)
 {
-  tree r = tsubst (t, args, tf_none, info.in_decl);
+  tsubst_flags_t quiet = info.complain & ~tf_warning_or_error;
+  tree r = tsubst (t, args, quiet, info.in_decl);
   if (r != error_mark_node)
     return r;
 
@@ -2068,6 +2072,8 @@ tsubst_type_requirement (tree t, tree args, sat_info info)
   tree type = tsubst_type_requirement_1 (t0, args, info, EXPR_LOCATION (t));
   if (type == error_mark_node)
     return error_mark_node;
+  if (processing_template_decl)
+    return finish_type_requirement (EXPR_LOCATION (t), type);
   return boolean_true_node;
 }
 
@@ -2124,9 +2130,11 @@ tsubst_compound_requirement (tree t, tree args, sat_info info)
 
   location_t loc = cp_expr_loc_or_input_loc (expr);
 
+  subst_info quiet (info.complain & ~tf_warning_or_error, info.in_decl);
+
   /* Check the noexcept condition.  */
   bool noexcept_p = COMPOUND_REQ_NOEXCEPT_P (t);
-  if (noexcept_p && !expr_noexcept_p (expr, tf_none))
+  if (noexcept_p && !expr_noexcept_p (expr, quiet.complain))
     {
       if (info.diagnose_unsatisfaction_p ())
 	inform (loc, "%qE is not %<noexcept%>", expr);
@@ -2139,8 +2147,6 @@ tsubst_compound_requirement (tree t, tree args, sat_info info)
   if (type == error_mark_node)
     return error_mark_node;
 
-  subst_info quiet (tf_none, info.in_decl);
-
   /* Check expression against the result type.  */
   if (type)
     {
@@ -2182,6 +2188,9 @@ tsubst_compound_requirement (tree t, tree args, sat_info info)
 	}
     }
 
+  if (processing_template_decl)
+    return finish_compound_requirement (EXPR_LOCATION (t),
+					expr, type, noexcept_p);
   return boolean_true_node;
 }
 
@@ -2190,7 +2199,16 @@ tsubst_compound_requirement (tree t, tree args, sat_info info)
 static tree
 tsubst_nested_requirement (tree t, tree args, sat_info info)
 {
-  sat_info quiet (tf_none, info.in_decl);
+  if (processing_template_decl)
+    {
+      tree req = TREE_OPERAND (t, 0);
+      req = tsubst_constraint (req, args, info.complain, info.in_decl);
+      if (req == error_mark_node)
+	return error_mark_node;
+      return finish_nested_requirement (EXPR_LOCATION (t), req);
+    }
+
+  sat_info quiet (info.complain & ~tf_warning_or_error, info.in_decl);
   tree result = constraint_satisfaction_value (t, args, quiet);
   if (result == boolean_true_node)
     return boolean_true_node;
@@ -2330,18 +2348,25 @@ tsubst_requires_expr (tree t, tree args, sat_info info)
 
   args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args,
 			 info.complain, info.in_decl);
-  if (processing_template_decl)
+  if (processing_template_decl
+      && !processing_constraint_expression_p ())
     {
       /* We're partially instantiating a generic lambda.  Substituting into
 	 this requires-expression now may cause its requirements to get
 	 checked out of order, so instead just remember the template
-	 arguments and wait until we can substitute them all at once.  */
+	 arguments and wait until we can substitute them all at once.
+
+	 Except if this requires-expr is part of associated constraints
+	 that we're substituting into directly (for e.g. declaration
+	 matching or dguide constraint rewriting), in which case we need
+	 to partially substitute.  */
       t = copy_node (t);
       REQUIRES_EXPR_EXTRA_ARGS (t) = build_extra_args (t, args, info.complain);
       return t;
     }
 
-  if (tree parms = REQUIRES_EXPR_PARMS (t))
+  tree parms = REQUIRES_EXPR_PARMS (t);
+  if (parms)
     {
       parms = tsubst_constraint_variables (parms, args, info);
       if (parms == error_mark_node)
@@ -2349,10 +2374,13 @@ tsubst_requires_expr (tree t, tree args, sat_info info)
     }
 
   tree result = boolean_true_node;
+  if (processing_template_decl)
+    result = NULL_TREE;
   for (tree reqs = REQUIRES_EXPR_REQS (t); reqs; reqs = TREE_CHAIN (reqs))
     {
       tree req = TREE_VALUE (reqs);
-      if (tsubst_requirement (req, args, info) == error_mark_node)
+      req = tsubst_requirement (req, args, info);
+      if (req == error_mark_node)
 	{
 	  result = boolean_false_node;
 	  if (info.diagnose_unsatisfaction_p ())
@@ -2360,7 +2388,11 @@ tsubst_requires_expr (tree t, tree args, sat_info info)
 	  else
 	    break;
 	}
+      else if (processing_template_decl)
+	result = tree_cons (NULL_TREE, req, result);
     }
+  if (processing_template_decl && result != boolean_false_node)
+    result = finish_requires_expr (EXPR_LOCATION (t), parms, nreverse (result));
   return result;
 }
 
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 9d30a271713..7dcdb5aaf7d 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -21705,7 +21705,8 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 
     case REQUIRES_EXPR:
       {
-	tree r = tsubst_requires_expr (t, args, tf_none, in_decl);
+	complain &= ~tf_warning_or_error;
+	tree r = tsubst_requires_expr (t, args, complain, in_decl);
 	RETURN (r);
       }
 
diff --git a/gcc/testsuite/g++.dg/cpp2a/class-deduction-alias18.C b/gcc/testsuite/g++.dg/cpp2a/class-deduction-alias18.C
new file mode 100644
index 00000000000..0bb11bb944c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/class-deduction-alias18.C
@@ -0,0 +1,13 @@
+// PR c++/112769
+// { dg-do compile { target c++20 } }
+
+template<int I, typename T>
+struct type
+{
+    type(T) requires requires { T{0}; };
+};
+
+template <typename T>
+using alias = type<0, T>;
+
+alias foo{123};
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend16.C b/gcc/testsuite/g++.dg/cpp2a/concepts-friend16.C
new file mode 100644
index 00000000000..18974eeb172
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend16.C
@@ -0,0 +1,25 @@
+// PR c++/110006
+// { dg-do compile { target c++20 } }
+
+template<typename T>
+class s;
+
+template<typename T>
+void constraint(s<T> const&, int&);
+
+template<typename U, typename T2>
+U function(s<T2> const x)
+	requires requires (U& u) { constraint(x, u); };
+
+template<typename T>
+class s
+{
+	template<typename U, typename T2>
+	friend U function(s<T2> const x)
+		requires requires (U& u) { constraint(x, u); };
+};
+
+int f(s<int> q)
+{
+	return function<int>(q); // { dg-bogus "ambiguous" }
+}
Jason Merrill Feb. 2, 2024, 9:05 p.m. UTC | #5
On 2/2/24 15:57, Patrick Palka wrote:
> On Fri, 2 Feb 2024, Patrick Palka wrote:
> 
>> On Fri, 2 Feb 2024, Jason Merrill wrote:
>>
>>> On 2/2/24 14:41, Patrick Palka wrote:
>>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this
>>>> look OK for trunk?
>>>>
>>>> -- >8 --
>>>>
>>>> In r11-3261-gb28b621ac67bee we made tsubst_requires_expr never partially
>>>> substitute into a requires-expression so as to avoid checking its
>>>> requirements out of order during e.g. generic lambda regeneration.
>>>>
>>>> Unfortunately we still do need to partially substitute into a
>>>> requires-expression in rare cases, in particular when it's used in
>>>> associated constraints that we are directly substituting for sake of
>>>> declaration matching or dguide constraint rewriting.  We can identify
>>>> this situation by checking processing_constraint_expression_p, so this
>>>> patch uses this predicate to control whether we defer substitution or
>>>> partially substitute.  The entering_scope=true change in tsubst_baselink
>>>> is needed to avoid ICEing from tsubst_baselink during name lookup when
>>>> rewriting std::ranges::ref_view's dguide constraints.
>>>
>>> Actually, I don't think we want to enter the scope when rewriting constraints.
>>> Would tsubst_scope work instead?
>>
>> Oops yes, because of the maybe_dependent_member_ref stuff, the handling
>> for which doesn't trigger here for some reason.  Ah, I think it's
>> because the tf_dguide flag gets dropped during tsubst_requires_expr
>> since it uses tf_none rather than complain & ~tf_warning_or_error
>> which would preserve special tsubst flags such as tf_dguide.  I'll fix...
> 
> Like so?  Lightly tested so far, bootstrap+regtest in progress.

OK, thanks.

> -- >8 --
> 
> Subject: [PATCH v2] c++: requires-exprs and partial constraint subst [PR112769]
> 
> In r11-3261-gb28b621ac67bee we made tsubst_requires_expr never partially
> substitute into a requires-expression so as to avoid checking its
> requirements out of order during e.g. generic lambda regeneration.
> 
> Unfortunately we still do need to partially substitute into a
> requires-expression in rare cases, in particular when it's used in
> associated constraints that we are directly substituting for sake of
> declaration matching or dguide constraint rewriting.  We can identify
> this situation by checking processing_constraint_expression_p, so this
> patch uses this predicate to control whether we defer substitution or
> partially substitute.
> 
> In turn, tsubst_requires_expr now needs to propagate semantic tsubst flags
> rather than just using tf_none, in particular for dguide constraint rewriting
> which sets tf_dguide.
> 
> 	PR c++/112769
> 	PR c++/110006
> 
> gcc/cp/ChangeLog:
> 
> 	* constraint.cc (subst_info::quiet): Accomodate non-diagnostic
> 	tsubst flags.
> 	(tsubst_valid_expression_requirement): Likewise.
> 	(tsubst_simple_requirement): Likewise.  Return a substituted _REQ
> 	node when processing_template_decl.
> 	(tsubst_type_requirement_1): Accomodate non-diagnostic tsubst
> 	flags.
> 	(tsubst_type_requirement): Return a substituted _REQ node when
> 	processing_template_decl.
> 	(tsubst_compound_requirement): Likewise.  Accomodate non-diagnostic
> 	tsubst flags.
> 	(tsubst_nested_requirement): Likewise.
> 	(tsubst_requires_expr): Don't defer partial substitution when
> 	processing_constraint_expression_p is true, in which case return
> 	a substituted REQUIRES_EXPR.
> 	* pt.cc (tsubst_expr) <case REQUIRES_EXPR>: Accomodate
> 	non-diagnostic tsubst flags.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/class-deduction-alias18.C: New test.
> 	* g++.dg/cpp2a/concepts-friend16.C: New test.
> ---
>   gcc/cp/constraint.cc                          | 56 +++++++++++++++----
>   gcc/cp/pt.cc                                  |  3 +-
>   .../g++.dg/cpp2a/class-deduction-alias18.C    | 13 +++++
>   .../g++.dg/cpp2a/concepts-friend16.C          | 25 +++++++++
>   4 files changed, 84 insertions(+), 13 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/class-deduction-alias18.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-friend16.C
> 
> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> index fef68cf7ab2..d9569013bd3 100644
> --- a/gcc/cp/constraint.cc
> +++ b/gcc/cp/constraint.cc
> @@ -85,7 +85,7 @@ struct subst_info
>     /* True if we should not diagnose errors.  */
>     bool quiet() const
>     {
> -    return complain == tf_none;
> +    return !(complain & tf_warning_or_error);
>     }
>   
>     /* True if we should diagnose errors.  */
> @@ -1991,8 +1991,9 @@ hash_placeholder_constraint (tree c)
>   static tree
>   tsubst_valid_expression_requirement (tree t, tree args, sat_info info)
>   {
> -  tree r = tsubst_expr (t, args, tf_none, info.in_decl);
> -  if (convert_to_void (r, ICV_STATEMENT, tf_none) != error_mark_node)
> +  tsubst_flags_t quiet = info.complain & ~tf_warning_or_error;
> +  tree r = tsubst_expr (t, args, quiet, info.in_decl);
> +  if (convert_to_void (r, ICV_STATEMENT, quiet) != error_mark_node)
>       return r;
>   
>     if (info.diagnose_unsatisfaction_p ())
> @@ -2028,6 +2029,8 @@ tsubst_simple_requirement (tree t, tree args, sat_info info)
>     tree expr = tsubst_valid_expression_requirement (t0, args, info);
>     if (expr == error_mark_node)
>       return error_mark_node;
> +  if (processing_template_decl)
> +    return finish_simple_requirement (EXPR_LOCATION (t), expr);
>     return boolean_true_node;
>   }
>   
> @@ -2037,7 +2040,8 @@ tsubst_simple_requirement (tree t, tree args, sat_info info)
>   static tree
>   tsubst_type_requirement_1 (tree t, tree args, sat_info info, location_t loc)
>   {
> -  tree r = tsubst (t, args, tf_none, info.in_decl);
> +  tsubst_flags_t quiet = info.complain & ~tf_warning_or_error;
> +  tree r = tsubst (t, args, quiet, info.in_decl);
>     if (r != error_mark_node)
>       return r;
>   
> @@ -2068,6 +2072,8 @@ tsubst_type_requirement (tree t, tree args, sat_info info)
>     tree type = tsubst_type_requirement_1 (t0, args, info, EXPR_LOCATION (t));
>     if (type == error_mark_node)
>       return error_mark_node;
> +  if (processing_template_decl)
> +    return finish_type_requirement (EXPR_LOCATION (t), type);
>     return boolean_true_node;
>   }
>   
> @@ -2124,9 +2130,11 @@ tsubst_compound_requirement (tree t, tree args, sat_info info)
>   
>     location_t loc = cp_expr_loc_or_input_loc (expr);
>   
> +  subst_info quiet (info.complain & ~tf_warning_or_error, info.in_decl);
> +
>     /* Check the noexcept condition.  */
>     bool noexcept_p = COMPOUND_REQ_NOEXCEPT_P (t);
> -  if (noexcept_p && !expr_noexcept_p (expr, tf_none))
> +  if (noexcept_p && !expr_noexcept_p (expr, quiet.complain))
>       {
>         if (info.diagnose_unsatisfaction_p ())
>   	inform (loc, "%qE is not %<noexcept%>", expr);
> @@ -2139,8 +2147,6 @@ tsubst_compound_requirement (tree t, tree args, sat_info info)
>     if (type == error_mark_node)
>       return error_mark_node;
>   
> -  subst_info quiet (tf_none, info.in_decl);
> -
>     /* Check expression against the result type.  */
>     if (type)
>       {
> @@ -2182,6 +2188,9 @@ tsubst_compound_requirement (tree t, tree args, sat_info info)
>   	}
>       }
>   
> +  if (processing_template_decl)
> +    return finish_compound_requirement (EXPR_LOCATION (t),
> +					expr, type, noexcept_p);
>     return boolean_true_node;
>   }
>   
> @@ -2190,7 +2199,16 @@ tsubst_compound_requirement (tree t, tree args, sat_info info)
>   static tree
>   tsubst_nested_requirement (tree t, tree args, sat_info info)
>   {
> -  sat_info quiet (tf_none, info.in_decl);
> +  if (processing_template_decl)
> +    {
> +      tree req = TREE_OPERAND (t, 0);
> +      req = tsubst_constraint (req, args, info.complain, info.in_decl);
> +      if (req == error_mark_node)
> +	return error_mark_node;
> +      return finish_nested_requirement (EXPR_LOCATION (t), req);
> +    }
> +
> +  sat_info quiet (info.complain & ~tf_warning_or_error, info.in_decl);
>     tree result = constraint_satisfaction_value (t, args, quiet);
>     if (result == boolean_true_node)
>       return boolean_true_node;
> @@ -2330,18 +2348,25 @@ tsubst_requires_expr (tree t, tree args, sat_info info)
>   
>     args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args,
>   			 info.complain, info.in_decl);
> -  if (processing_template_decl)
> +  if (processing_template_decl
> +      && !processing_constraint_expression_p ())
>       {
>         /* We're partially instantiating a generic lambda.  Substituting into
>   	 this requires-expression now may cause its requirements to get
>   	 checked out of order, so instead just remember the template
> -	 arguments and wait until we can substitute them all at once.  */
> +	 arguments and wait until we can substitute them all at once.
> +
> +	 Except if this requires-expr is part of associated constraints
> +	 that we're substituting into directly (for e.g. declaration
> +	 matching or dguide constraint rewriting), in which case we need
> +	 to partially substitute.  */
>         t = copy_node (t);
>         REQUIRES_EXPR_EXTRA_ARGS (t) = build_extra_args (t, args, info.complain);
>         return t;
>       }
>   
> -  if (tree parms = REQUIRES_EXPR_PARMS (t))
> +  tree parms = REQUIRES_EXPR_PARMS (t);
> +  if (parms)
>       {
>         parms = tsubst_constraint_variables (parms, args, info);
>         if (parms == error_mark_node)
> @@ -2349,10 +2374,13 @@ tsubst_requires_expr (tree t, tree args, sat_info info)
>       }
>   
>     tree result = boolean_true_node;
> +  if (processing_template_decl)
> +    result = NULL_TREE;
>     for (tree reqs = REQUIRES_EXPR_REQS (t); reqs; reqs = TREE_CHAIN (reqs))
>       {
>         tree req = TREE_VALUE (reqs);
> -      if (tsubst_requirement (req, args, info) == error_mark_node)
> +      req = tsubst_requirement (req, args, info);
> +      if (req == error_mark_node)
>   	{
>   	  result = boolean_false_node;
>   	  if (info.diagnose_unsatisfaction_p ())
> @@ -2360,7 +2388,11 @@ tsubst_requires_expr (tree t, tree args, sat_info info)
>   	  else
>   	    break;
>   	}
> +      else if (processing_template_decl)
> +	result = tree_cons (NULL_TREE, req, result);
>       }
> +  if (processing_template_decl && result != boolean_false_node)
> +    result = finish_requires_expr (EXPR_LOCATION (t), parms, nreverse (result));
>     return result;
>   }
>   
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 9d30a271713..7dcdb5aaf7d 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -21705,7 +21705,8 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>   
>       case REQUIRES_EXPR:
>         {
> -	tree r = tsubst_requires_expr (t, args, tf_none, in_decl);
> +	complain &= ~tf_warning_or_error;
> +	tree r = tsubst_requires_expr (t, args, complain, in_decl);
>   	RETURN (r);
>         }
>   
> diff --git a/gcc/testsuite/g++.dg/cpp2a/class-deduction-alias18.C b/gcc/testsuite/g++.dg/cpp2a/class-deduction-alias18.C
> new file mode 100644
> index 00000000000..0bb11bb944c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/class-deduction-alias18.C
> @@ -0,0 +1,13 @@
> +// PR c++/112769
> +// { dg-do compile { target c++20 } }
> +
> +template<int I, typename T>
> +struct type
> +{
> +    type(T) requires requires { T{0}; };
> +};
> +
> +template <typename T>
> +using alias = type<0, T>;
> +
> +alias foo{123};
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend16.C b/gcc/testsuite/g++.dg/cpp2a/concepts-friend16.C
> new file mode 100644
> index 00000000000..18974eeb172
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend16.C
> @@ -0,0 +1,25 @@
> +// PR c++/110006
> +// { dg-do compile { target c++20 } }
> +
> +template<typename T>
> +class s;
> +
> +template<typename T>
> +void constraint(s<T> const&, int&);
> +
> +template<typename U, typename T2>
> +U function(s<T2> const x)
> +	requires requires (U& u) { constraint(x, u); };
> +
> +template<typename T>
> +class s
> +{
> +	template<typename U, typename T2>
> +	friend U function(s<T2> const x)
> +		requires requires (U& u) { constraint(x, u); };
> +};
> +
> +int f(s<int> q)
> +{
> +	return function<int>(q); // { dg-bogus "ambiguous" }
> +}
diff mbox series

Patch

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index fef68cf7ab2..450ae548f9a 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -2028,6 +2028,8 @@  tsubst_simple_requirement (tree t, tree args, sat_info info)
   tree expr = tsubst_valid_expression_requirement (t0, args, info);
   if (expr == error_mark_node)
     return error_mark_node;
+  if (processing_template_decl)
+    return finish_simple_requirement (EXPR_LOCATION (t), expr);
   return boolean_true_node;
 }
 
@@ -2068,6 +2070,8 @@  tsubst_type_requirement (tree t, tree args, sat_info info)
   tree type = tsubst_type_requirement_1 (t0, args, info, EXPR_LOCATION (t));
   if (type == error_mark_node)
     return error_mark_node;
+  if (processing_template_decl)
+    return finish_type_requirement (EXPR_LOCATION (t), type);
   return boolean_true_node;
 }
 
@@ -2182,6 +2186,9 @@  tsubst_compound_requirement (tree t, tree args, sat_info info)
 	}
     }
 
+  if (processing_template_decl)
+    return finish_compound_requirement (EXPR_LOCATION (t),
+					expr, type, noexcept_p);
   return boolean_true_node;
 }
 
@@ -2190,6 +2197,15 @@  tsubst_compound_requirement (tree t, tree args, sat_info info)
 static tree
 tsubst_nested_requirement (tree t, tree args, sat_info info)
 {
+  if (processing_template_decl)
+    {
+      tree req = TREE_OPERAND (t, 0);
+      req = tsubst_constraint (req, args, info.complain, info.in_decl);
+      if (req == error_mark_node)
+	return error_mark_node;
+      return finish_nested_requirement (EXPR_LOCATION (t), req);
+    }
+
   sat_info quiet (tf_none, info.in_decl);
   tree result = constraint_satisfaction_value (t, args, quiet);
   if (result == boolean_true_node)
@@ -2330,18 +2346,25 @@  tsubst_requires_expr (tree t, tree args, sat_info info)
 
   args = add_extra_args (REQUIRES_EXPR_EXTRA_ARGS (t), args,
 			 info.complain, info.in_decl);
-  if (processing_template_decl)
+  if (processing_template_decl
+      && !processing_constraint_expression_p ())
     {
       /* We're partially instantiating a generic lambda.  Substituting into
 	 this requires-expression now may cause its requirements to get
 	 checked out of order, so instead just remember the template
-	 arguments and wait until we can substitute them all at once.  */
+	 arguments and wait until we can substitute them all at once.
+
+	 Except if this requires-expr is part of associated constraints
+	 that we're substituting into directly (for e.g. declaration
+	 matching or dguide constraint rewriting), in which case we need
+	 to partially substitute.  */
       t = copy_node (t);
       REQUIRES_EXPR_EXTRA_ARGS (t) = build_extra_args (t, args, info.complain);
       return t;
     }
 
-  if (tree parms = REQUIRES_EXPR_PARMS (t))
+  tree parms = REQUIRES_EXPR_PARMS (t);
+  if (parms)
     {
       parms = tsubst_constraint_variables (parms, args, info);
       if (parms == error_mark_node)
@@ -2349,10 +2372,13 @@  tsubst_requires_expr (tree t, tree args, sat_info info)
     }
 
   tree result = boolean_true_node;
+  if (processing_template_decl)
+    result = NULL_TREE;
   for (tree reqs = REQUIRES_EXPR_REQS (t); reqs; reqs = TREE_CHAIN (reqs))
     {
       tree req = TREE_VALUE (reqs);
-      if (tsubst_requirement (req, args, info) == error_mark_node)
+      req = tsubst_requirement (req, args, info);
+      if (req == error_mark_node)
 	{
 	  result = boolean_false_node;
 	  if (info.diagnose_unsatisfaction_p ())
@@ -2360,7 +2386,11 @@  tsubst_requires_expr (tree t, tree args, sat_info info)
 	  else
 	    break;
 	}
+      else if (processing_template_decl)
+	result = tree_cons (NULL_TREE, req, result);
     }
+  if (processing_template_decl && result != boolean_false_node)
+    result = finish_requires_expr (EXPR_LOCATION (t), parms, nreverse (result));
   return result;
 }
 
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 9d30a271713..17ecd4b9c5a 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -17065,7 +17065,8 @@  tsubst_baselink (tree baselink, tree object_type,
 {
   bool qualified_p = BASELINK_QUALIFIED_P (baselink);
   tree qualifying_scope = BINFO_TYPE (BASELINK_ACCESS_BINFO (baselink));
-  qualifying_scope = tsubst (qualifying_scope, args, complain, in_decl);
+  qualifying_scope = tsubst_aggr_type (qualifying_scope, args, complain, in_decl,
+				       /*entering_scope=*/true);
 
   tree optype = BASELINK_OPTYPE (baselink);
   optype = tsubst (optype, args, complain, in_decl);
diff --git a/gcc/testsuite/g++.dg/cpp2a/class-deduction-alias18.C b/gcc/testsuite/g++.dg/cpp2a/class-deduction-alias18.C
new file mode 100644
index 00000000000..0bb11bb944c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/class-deduction-alias18.C
@@ -0,0 +1,13 @@ 
+// PR c++/112769
+// { dg-do compile { target c++20 } }
+
+template<int I, typename T>
+struct type
+{
+    type(T) requires requires { T{0}; };
+};
+
+template <typename T>
+using alias = type<0, T>;
+
+alias foo{123};
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-friend16.C b/gcc/testsuite/g++.dg/cpp2a/concepts-friend16.C
new file mode 100644
index 00000000000..18974eeb172
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-friend16.C
@@ -0,0 +1,25 @@ 
+// PR c++/110006
+// { dg-do compile { target c++20 } }
+
+template<typename T>
+class s;
+
+template<typename T>
+void constraint(s<T> const&, int&);
+
+template<typename U, typename T2>
+U function(s<T2> const x)
+	requires requires (U& u) { constraint(x, u); };
+
+template<typename T>
+class s
+{
+	template<typename U, typename T2>
+	friend U function(s<T2> const x)
+		requires requires (U& u) { constraint(x, u); };
+};
+
+int f(s<int> q)
+{
+	return function<int>(q); // { dg-bogus "ambiguous" }
+}