public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Patrick Palka <ppalka@redhat.com>
To: Patrick Palka <ppalka@redhat.com>
Cc: Jason Merrill <jason@redhat.com>, gcc-patches@gcc.gnu.org
Subject: Re: [PATCH 1/2] c++: requires-exprs and partial constraint subst [PR112769]
Date: Fri, 2 Feb 2024 15:57:16 -0500 (EST)	[thread overview]
Message-ID: <720eaacc-cd6a-8a66-e057-752c6a21183d@idea> (raw)
In-Reply-To: <5f06be77-2314-65e4-a242-b84059bebfbe@idea>

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" }
+}
-- 
2.43.0.493.gbc7ee2e5e1


  reply	other threads:[~2024-02-02 20:57 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-02-02 19:41 Patrick Palka
2024-02-02 19:41 ` [PATCH 2/2] c++: remove lookup_template_class's entering_scope flag Patrick Palka
2024-05-01 19:41   ` Patrick Palka
2024-05-01 20:22     ` Jason Merrill
2024-05-01 20:40       ` Patrick Palka
2024-05-01 20:52         ` Jason Merrill
2024-05-02 17:49           ` Patrick Palka
2024-05-02 18:11             ` Jason Merrill
2024-02-02 19:55 ` [PATCH 1/2] c++: requires-exprs and partial constraint subst [PR112769] Jason Merrill
2024-02-02 20:01 ` Jason Merrill
2024-02-02 20:36   ` Patrick Palka
2024-02-02 20:57     ` Patrick Palka [this message]
2024-02-02 21:05       ` Jason Merrill

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=720eaacc-cd6a-8a66-e057-752c6a21183d@idea \
    --to=ppalka@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jason@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).