[PATCH] c++: implicit guides should inherit class constraints [PR104873]

[PATCH] c++: implicit guides should inherit class constraints [PR104873]

[Patrick Palka]

An implicit guide already inherits the (rewritten) constraints of the
constructor.  Thus it seems natural that the guide must also inherit
the constraints of the class template, since a constructor's constraints
might assume the class's constraints are satisfied, and therefore
checking these two sets of constraints "out of order" may result in hard
errors as in the first testcase below.

This patch makes implicit guides inherit the constraints of the class
template (even for unconstrained constructors, and even for the copy
deduction candidate).

In passing, this patch gives implicit guides a trailing return type
since that's how they're depicted in the standard (e.g.
[over.match.class.deduct]/6); this changes the order of substitution
into implicit guides in a probably negligible way, especially now that
they inherit the class constraints.

The parameter_mapping_equivalent_p change is to avoid an ICE in the last
testcase below (described within), reduced from a cmcstl2 testsuite ICE.

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look like
the right approach?

	PR c++/104873

gcc/cp/ChangeLog:

	* constraint.cc (parameter_mapping_equivalent_p): Relax assert
	to expect equivalence not identity of template parameters.
	* pt.cc (build_deduction_guide): Propagate the class's
	constraints to the deduction guide.  Set TYPE_HAS_LATE_RETURN_TYPE
	on the function type.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp2a/concepts-ctad5.C: New test.
	* g++.dg/cpp2a/concepts-ctad6.C: New test.
	* g++.dg/cpp2a/concepts-ctad6a.C: New test.
	* g++.dg/cpp2a/concepts-ctad7.C: New test.
---
 gcc/cp/constraint.cc                         |  2 +-
 gcc/cp/pt.cc                                 | 26 ++++++++++++++++++
 gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C  | 29 ++++++++++++++++++++
 gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C  | 19 +++++++++++++
 gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C | 19 +++++++++++++
 gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C  | 26 ++++++++++++++++++
 6 files changed, 120 insertions(+), 1 deletion(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C
 create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C

diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
index 94f6222b436..6cbb182dda2 100644
--- a/gcc/cp/constraint.cc
+++ b/gcc/cp/constraint.cc
@@ -604,7 +604,7 @@ parameter_mapping_equivalent_p (tree t1, tree t2)
   tree map2 = ATOMIC_CONSTR_MAP (t2);
   while (map1 && map2)
     {
-      gcc_checking_assert (TREE_VALUE (map1) == TREE_VALUE (map2));
+      gcc_checking_assert (cp_tree_equal (TREE_VALUE (map1), TREE_VALUE (map2)));
       tree arg1 = TREE_PURPOSE (map1);
       tree arg2 = TREE_PURPOSE (map2);
       if (!template_args_equal (arg1, arg2))
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 75ed9a34018..966e6d90d3a 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -29261,6 +29261,10 @@ build_deduction_guide (tree type, tree ctor, tree outer_args, tsubst_flags_t com
       /* Discard the 'this' parameter.  */
       fparms = FUNCTION_ARG_CHAIN (ctor);
       fargs = TREE_CHAIN (DECL_ARGUMENTS (ctor));
+      /* The guide's constraints consist of the class template's constraints
+	 followed by the constructor's rewritten constraints.  We start
+	 with the constructor's constraints (since we need to rewrite them),
+	 and prepend the class template's constraints later.  */
       ci = get_constraints (ctor);
       loc = DECL_SOURCE_LOCATION (ctor);
       explicit_p = DECL_NONCONVERTING_P (ctor);
@@ -29362,6 +29366,27 @@ build_deduction_guide (tree type, tree ctor, tree outer_args, tsubst_flags_t com
 	return error_mark_node;
     }
 
+  /* Prepend the class template's constraints to the constructor's rewritten
+     constraints (if any).  */
+  if (tree class_ci = get_constraints (CLASSTYPE_TI_TEMPLATE (type)))
+    {
+      if (outer_args)
+	{
+	  /* FIXME: As above.  */
+	  ++processing_template_decl;
+	  class_ci = tsubst_constraint_info (class_ci, outer_args,
+					     complain, ctor);
+	  --processing_template_decl;
+	}
+      if (ci)
+	ci = build_constraints (combine_constraint_expressions
+				(CI_TEMPLATE_REQS (class_ci),
+				 CI_TEMPLATE_REQS (ci)),
+				CI_DECLARATOR_REQS (ci));
+      else
+	ci = copy_node (class_ci);
+    }
+
   if (!memtmpl)
     {
       /* Copy the parms so we can set DECL_PRIMARY_TEMPLATE.  */
@@ -29371,6 +29396,7 @@ build_deduction_guide (tree type, tree ctor, tree outer_args, tsubst_flags_t com
     }
 
   tree fntype = build_function_type (type, fparms);
+  TYPE_HAS_LATE_RETURN_TYPE (fntype) = true;
   tree ded_fn = build_lang_decl_loc (loc,
 				     FUNCTION_DECL,
 				     dguide_name (type), fntype);
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C
new file mode 100644
index 00000000000..5990088f1db
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C
@@ -0,0 +1,29 @@
+// PR c++/104873
+// { dg-do compile { target c++20 } }
+// Verify implicit guides inherit the constraints of the class template.
+
+template<class T>
+struct A {
+  static_assert(!__is_same(T, void));
+  static constexpr bool value = true;
+};
+
+template<class T> requires (!__is_same(T, void))
+struct B {
+  B(T*) requires A<T>::value; // #1
+  // implicit guide:
+  // template<class T> requires (!__is_same(T, void)) && A<T>::value
+  // B(T*) -> B<T>;
+
+  B(T);
+  // implicit guide:
+  // template<class T> requires (!__is_same(T, void))
+  // B(T) -> B<T>;
+};
+
+void* p;
+using type = decltype(B(p)); // previously hard error during dguide overload
+			     // resolution because #1's implicit guide would
+			     // inherit only the constructor's constraints and
+			     // not also the class's.
+using type = B<void*>;
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C
new file mode 100644
index 00000000000..10bb86df6a0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C
@@ -0,0 +1,19 @@
+// PR c++/104873
+// { dg-do compile { target c++20 } }
+
+template<class T> concept C = true;
+
+template<class T> requires C<T>
+struct S {
+  S(...);
+};
+// synthesized copy deduction candidate:
+// template<class T> requires C<T>
+// S(S<T>) -> S<T>;
+
+template<class T> S(S<T>) -> S<void>; // #1
+
+using type = decltype(S(S<int>())); // The deduction candidate (which inherits
+				    // the class's constraints) is preferred
+				    // over #1 because it's more constrained.
+using type = S<int>;
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C
new file mode 100644
index 00000000000..f9d1d6ec11e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C
@@ -0,0 +1,19 @@
+// PR c++/104873
+// { dg-do compile { target c++20 } }
+
+template<class T> concept C = true;
+
+template<class T> requires C<T>
+struct S {
+  S(T); // #1
+  // implicit guide:
+  // template<class T> requires C<T>
+  // S<T> -> S<T>;
+};
+
+template<class T> S(T) -> S<S<T>>; // #2
+
+using type = decltype(S(0)); // The implicit guide for #1 (which inherits the
+			     // class's constraints) is preferred over #2
+			     // because it's more constrained.
+using type = S<int>;
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C
new file mode 100644
index 00000000000..4e1b1abfb94
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C
@@ -0,0 +1,26 @@
+// { dg-do compile { target c++20 } }
+
+template<class T> requires __is_same(T, int)
+struct A;
+
+// When checking constraints on the class template, we
+// normalize __is_same(T, int) and cache the resulting atom.
+A<int> *p;
+
+template<class T> requires __is_same(T, int)
+struct A {
+  A(T);
+  // implicit guide:
+  // template<class T> requires __is_same(T, int)
+  // A(T) -> A<T>;
+};
+
+// When checking constraints on the implicit guide, we normalize
+// the same expression again with an equivalent but not identical
+// set of template parameters (those from the definition vs from
+// the forward declaration).  We should notice that that the
+// resulting atom is the same as the one we cached earlier, but
+// instead we crashed due to an overly strict assert in
+// parameter_mapping_equivalent_p that demanded identity instead
+// of equivalence of the template parameters in the mapping.
+A a = 1;
-- 
2.35.1.735.g4b6846d9dc

Re: [PATCH] c++: implicit guides should inherit class constraints [PR104873]

[Jason Merrill]

On 4/1/22 11:17, Patrick Palka wrote:
> An implicit guide already inherits the (rewritten) constraints of the
> constructor.  Thus it seems natural that the guide must also inherit
> the constraints of the class template, since a constructor's constraints
> might assume the class's constraints are satisfied, and therefore
> checking these two sets of constraints "out of order" may result in hard
> errors as in the first testcase below.

> This patch makes implicit guides inherit the constraints of the class
> template (even for unconstrained constructors, and even for the copy
> deduction candidate).
> 
> In passing, this patch gives implicit guides a trailing return type
> since that's how they're depicted in the standard (e.g.
> [over.match.class.deduct]/6); this changes the order of substitution
> into implicit guides in a probably negligible way, especially now that
> they inherit the class constraints.
> 
> The parameter_mapping_equivalent_p change is to avoid an ICE in the last
> testcase below (described within), reduced from a cmcstl2 testsuite ICE.
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look like
> the right approach?

I don't think so, given the testcases below.

Maybe fn_type_unification should check formation of the return type of a 
deduction guide before constraints?  In general, whichever order you do 
things in, it'll be wrong for some testcase or other.

The broader subject of constraints and deduction guides should be raised 
with CWG in general (https://github.com/cplusplus/CWG/issues/new/choose)

> 	PR c++/104873
> 
> gcc/cp/ChangeLog:
> 
> 	* constraint.cc (parameter_mapping_equivalent_p): Relax assert
> 	to expect equivalence not identity of template parameters.
> 	* pt.cc (build_deduction_guide): Propagate the class's
> 	constraints to the deduction guide.  Set TYPE_HAS_LATE_RETURN_TYPE
> 	on the function type.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp2a/concepts-ctad5.C: New test.
> 	* g++.dg/cpp2a/concepts-ctad6.C: New test.
> 	* g++.dg/cpp2a/concepts-ctad6a.C: New test.
> 	* g++.dg/cpp2a/concepts-ctad7.C: New test.
> ---
>   gcc/cp/constraint.cc                         |  2 +-
>   gcc/cp/pt.cc                                 | 26 ++++++++++++++++++
>   gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C  | 29 ++++++++++++++++++++
>   gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C  | 19 +++++++++++++
>   gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C | 19 +++++++++++++
>   gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C  | 26 ++++++++++++++++++
>   6 files changed, 120 insertions(+), 1 deletion(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C
> 
> diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> index 94f6222b436..6cbb182dda2 100644
> --- a/gcc/cp/constraint.cc
> +++ b/gcc/cp/constraint.cc
> @@ -604,7 +604,7 @@ parameter_mapping_equivalent_p (tree t1, tree t2)
>     tree map2 = ATOMIC_CONSTR_MAP (t2);
>     while (map1 && map2)
>       {
> -      gcc_checking_assert (TREE_VALUE (map1) == TREE_VALUE (map2));
> +      gcc_checking_assert (cp_tree_equal (TREE_VALUE (map1), TREE_VALUE (map2)));
>         tree arg1 = TREE_PURPOSE (map1);
>         tree arg2 = TREE_PURPOSE (map2);
>         if (!template_args_equal (arg1, arg2))
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 75ed9a34018..966e6d90d3a 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -29261,6 +29261,10 @@ build_deduction_guide (tree type, tree ctor, tree outer_args, tsubst_flags_t com
>         /* Discard the 'this' parameter.  */
>         fparms = FUNCTION_ARG_CHAIN (ctor);
>         fargs = TREE_CHAIN (DECL_ARGUMENTS (ctor));
> +      /* The guide's constraints consist of the class template's constraints
> +	 followed by the constructor's rewritten constraints.  We start
> +	 with the constructor's constraints (since we need to rewrite them),
> +	 and prepend the class template's constraints later.  */
>         ci = get_constraints (ctor);
>         loc = DECL_SOURCE_LOCATION (ctor);
>         explicit_p = DECL_NONCONVERTING_P (ctor);
> @@ -29362,6 +29366,27 @@ build_deduction_guide (tree type, tree ctor, tree outer_args, tsubst_flags_t com
>   	return error_mark_node;
>       }
>   
> +  /* Prepend the class template's constraints to the constructor's rewritten
> +     constraints (if any).  */
> +  if (tree class_ci = get_constraints (CLASSTYPE_TI_TEMPLATE (type)))
> +    {
> +      if (outer_args)
> +	{
> +	  /* FIXME: As above.  */
> +	  ++processing_template_decl;
> +	  class_ci = tsubst_constraint_info (class_ci, outer_args,
> +					     complain, ctor);
> +	  --processing_template_decl;
> +	}
> +      if (ci)
> +	ci = build_constraints (combine_constraint_expressions
> +				(CI_TEMPLATE_REQS (class_ci),
> +				 CI_TEMPLATE_REQS (ci)),
> +				CI_DECLARATOR_REQS (ci));
> +      else
> +	ci = copy_node (class_ci);
> +    }
> +
>     if (!memtmpl)
>       {
>         /* Copy the parms so we can set DECL_PRIMARY_TEMPLATE.  */
> @@ -29371,6 +29396,7 @@ build_deduction_guide (tree type, tree ctor, tree outer_args, tsubst_flags_t com
>       }
>   
>     tree fntype = build_function_type (type, fparms);
> +  TYPE_HAS_LATE_RETURN_TYPE (fntype) = true;
>     tree ded_fn = build_lang_decl_loc (loc,
>   				     FUNCTION_DECL,
>   				     dguide_name (type), fntype);
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C
> new file mode 100644
> index 00000000000..5990088f1db
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C
> @@ -0,0 +1,29 @@
> +// PR c++/104873
> +// { dg-do compile { target c++20 } }
> +// Verify implicit guides inherit the constraints of the class template.
> +
> +template<class T>
> +struct A {
> +  static_assert(!__is_same(T, void));
> +  static constexpr bool value = true;
> +};
> +
> +template<class T> requires (!__is_same(T, void))
> +struct B {
> +  B(T*) requires A<T>::value; // #1
> +  // implicit guide:
> +  // template<class T> requires (!__is_same(T, void)) && A<T>::value
> +  // B(T*) -> B<T>;
> +
> +  B(T);
> +  // implicit guide:
> +  // template<class T> requires (!__is_same(T, void))
> +  // B(T) -> B<T>;
> +};
> +
> +void* p;
> +using type = decltype(B(p)); // previously hard error during dguide overload
> +			     // resolution because #1's implicit guide would
> +			     // inherit only the constructor's constraints and
> +			     // not also the class's.
> +using type = B<void*>;
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C
> new file mode 100644
> index 00000000000..10bb86df6a0
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C
> @@ -0,0 +1,19 @@
> +// PR c++/104873
> +// { dg-do compile { target c++20 } }
> +
> +template<class T> concept C = true;
> +
> +template<class T> requires C<T>
> +struct S {
> +  S(...);
> +};
> +// synthesized copy deduction candidate:
> +// template<class T> requires C<T>
> +// S(S<T>) -> S<T>;
> +
> +template<class T> S(S<T>) -> S<void>; // #1
> +
> +using type = decltype(S(S<int>())); // The deduction candidate (which inherits
> +				    // the class's constraints) is preferred
> +				    // over #1 because it's more constrained.

This seems like a regression: Presumably people write deduction guides 
because they want them to be used, not to have them silently ignored 
because the class is constrained.

> +using type = S<int>;
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C
> new file mode 100644
> index 00000000000..f9d1d6ec11e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C
> @@ -0,0 +1,19 @@
> +// PR c++/104873
> +// { dg-do compile { target c++20 } }
> +
> +template<class T> concept C = true;
> +
> +template<class T> requires C<T>
> +struct S {
> +  S(T); // #1
> +  // implicit guide:
> +  // template<class T> requires C<T>
> +  // S<T> -> S<T>;
> +};
> +
> +template<class T> S(T) -> S<S<T>>; // #2
> +
> +using type = decltype(S(0)); // The implicit guide for #1 (which inherits the
> +			     // class's constraints) is preferred over #2
> +			     // because it's more constrained.

Likewise.

> +using type = S<int>;
> diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C
> new file mode 100644
> index 00000000000..4e1b1abfb94
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C
> @@ -0,0 +1,26 @@
> +// { dg-do compile { target c++20 } }
> +
> +template<class T> requires __is_same(T, int)
> +struct A;
> +
> +// When checking constraints on the class template, we
> +// normalize __is_same(T, int) and cache the resulting atom.
> +A<int> *p;
> +
> +template<class T> requires __is_same(T, int)
> +struct A {
> +  A(T);
> +  // implicit guide:
> +  // template<class T> requires __is_same(T, int)
> +  // A(T) -> A<T>;
> +};
> +
> +// When checking constraints on the implicit guide, we normalize
> +// the same expression again with an equivalent but not identical
> +// set of template parameters (those from the definition vs from
> +// the forward declaration).  We should notice that that the
> +// resulting atom is the same as the one we cached earlier, but
> +// instead we crashed due to an overly strict assert in
> +// parameter_mapping_equivalent_p that demanded identity instead
> +// of equivalence of the template parameters in the mapping.
> +A a = 1;

Re: [PATCH] c++: implicit guides should inherit class constraints [PR104873]

[Patrick Palka]

On Fri, 1 Apr 2022, Jason Merrill wrote:

> On 4/1/22 11:17, Patrick Palka wrote:
> > An implicit guide already inherits the (rewritten) constraints of the
> > constructor.  Thus it seems natural that the guide must also inherit
> > the constraints of the class template, since a constructor's constraints
> > might assume the class's constraints are satisfied, and therefore
> > checking these two sets of constraints "out of order" may result in hard
> > errors as in the first testcase below.
> 
> > This patch makes implicit guides inherit the constraints of the class
> > template (even for unconstrained constructors, and even for the copy
> > deduction candidate).
> > 
> > In passing, this patch gives implicit guides a trailing return type
> > since that's how they're depicted in the standard (e.g.
> > [over.match.class.deduct]/6); this changes the order of substitution
> > into implicit guides in a probably negligible way, especially now that
> > they inherit the class constraints.
> > 
> > The parameter_mapping_equivalent_p change is to avoid an ICE in the last
> > testcase below (described within), reduced from a cmcstl2 testsuite ICE.
> > 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look like
> > the right approach?
> 
> I don't think so, given the testcases below.
> 
> Maybe fn_type_unification should check formation of the return type of a
> deduction guide before constraints?  In general, whichever order you do things
> in, it'll be wrong for some testcase or other.

Makes sense..  Though I notice the resolution of CWG2628 goes the
direction of propagating the class's constraints and seems to have been
approved in November.  I wonder how that discussion went wrt your
alternative resolution of first checking the return type during
deduction?  Shall we follow suit with the approved resolution (and
therefore this patch) for GCC 14?

> 
> The broader subject of constraints and deduction guides should be raised with
> CWG in general (https://github.com/cplusplus/CWG/issues/new/choose)
> 
> > 	PR c++/104873
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* constraint.cc (parameter_mapping_equivalent_p): Relax assert
> > 	to expect equivalence not identity of template parameters.
> > 	* pt.cc (build_deduction_guide): Propagate the class's
> > 	constraints to the deduction guide.  Set TYPE_HAS_LATE_RETURN_TYPE
> > 	on the function type.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/cpp2a/concepts-ctad5.C: New test.
> > 	* g++.dg/cpp2a/concepts-ctad6.C: New test.
> > 	* g++.dg/cpp2a/concepts-ctad6a.C: New test.
> > 	* g++.dg/cpp2a/concepts-ctad7.C: New test.
> > ---
> >   gcc/cp/constraint.cc                         |  2 +-
> >   gcc/cp/pt.cc                                 | 26 ++++++++++++++++++
> >   gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C  | 29 ++++++++++++++++++++
> >   gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C  | 19 +++++++++++++
> >   gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C | 19 +++++++++++++
> >   gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C  | 26 ++++++++++++++++++
> >   6 files changed, 120 insertions(+), 1 deletion(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C
> > 
> > diff --git a/gcc/cp/constraint.cc b/gcc/cp/constraint.cc
> > index 94f6222b436..6cbb182dda2 100644
> > --- a/gcc/cp/constraint.cc
> > +++ b/gcc/cp/constraint.cc
> > @@ -604,7 +604,7 @@ parameter_mapping_equivalent_p (tree t1, tree t2)
> >     tree map2 = ATOMIC_CONSTR_MAP (t2);
> >     while (map1 && map2)
> >       {
> > -      gcc_checking_assert (TREE_VALUE (map1) == TREE_VALUE (map2));
> > +      gcc_checking_assert (cp_tree_equal (TREE_VALUE (map1), TREE_VALUE
> > (map2)));
> >         tree arg1 = TREE_PURPOSE (map1);
> >         tree arg2 = TREE_PURPOSE (map2);
> >         if (!template_args_equal (arg1, arg2))
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index 75ed9a34018..966e6d90d3a 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -29261,6 +29261,10 @@ build_deduction_guide (tree type, tree ctor, tree
> > outer_args, tsubst_flags_t com
> >         /* Discard the 'this' parameter.  */
> >         fparms = FUNCTION_ARG_CHAIN (ctor);
> >         fargs = TREE_CHAIN (DECL_ARGUMENTS (ctor));
> > +      /* The guide's constraints consist of the class template's
> > constraints
> > +	 followed by the constructor's rewritten constraints.  We start
> > +	 with the constructor's constraints (since we need to rewrite them),
> > +	 and prepend the class template's constraints later.  */
> >         ci = get_constraints (ctor);
> >         loc = DECL_SOURCE_LOCATION (ctor);
> >         explicit_p = DECL_NONCONVERTING_P (ctor);
> > @@ -29362,6 +29366,27 @@ build_deduction_guide (tree type, tree ctor, tree
> > outer_args, tsubst_flags_t com
> >   	return error_mark_node;
> >       }
> >   +  /* Prepend the class template's constraints to the constructor's
> > rewritten
> > +     constraints (if any).  */
> > +  if (tree class_ci = get_constraints (CLASSTYPE_TI_TEMPLATE (type)))
> > +    {
> > +      if (outer_args)
> > +	{
> > +	  /* FIXME: As above.  */
> > +	  ++processing_template_decl;
> > +	  class_ci = tsubst_constraint_info (class_ci, outer_args,
> > +					     complain, ctor);
> > +	  --processing_template_decl;
> > +	}
> > +      if (ci)
> > +	ci = build_constraints (combine_constraint_expressions
> > +				(CI_TEMPLATE_REQS (class_ci),
> > +				 CI_TEMPLATE_REQS (ci)),
> > +				CI_DECLARATOR_REQS (ci));
> > +      else
> > +	ci = copy_node (class_ci);
> > +    }
> > +
> >     if (!memtmpl)
> >       {
> >         /* Copy the parms so we can set DECL_PRIMARY_TEMPLATE.  */
> > @@ -29371,6 +29396,7 @@ build_deduction_guide (tree type, tree ctor, tree
> > outer_args, tsubst_flags_t com
> >       }
> >       tree fntype = build_function_type (type, fparms);
> > +  TYPE_HAS_LATE_RETURN_TYPE (fntype) = true;
> >     tree ded_fn = build_lang_decl_loc (loc,
> >   				     FUNCTION_DECL,
> >   				     dguide_name (type), fntype);
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C
> > b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C
> > new file mode 100644
> > index 00000000000..5990088f1db
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad5.C
> > @@ -0,0 +1,29 @@
> > +// PR c++/104873
> > +// { dg-do compile { target c++20 } }
> > +// Verify implicit guides inherit the constraints of the class template.
> > +
> > +template<class T>
> > +struct A {
> > +  static_assert(!__is_same(T, void));
> > +  static constexpr bool value = true;
> > +};
> > +
> > +template<class T> requires (!__is_same(T, void))
> > +struct B {
> > +  B(T*) requires A<T>::value; // #1
> > +  // implicit guide:
> > +  // template<class T> requires (!__is_same(T, void)) && A<T>::value
> > +  // B(T*) -> B<T>;
> > +
> > +  B(T);
> > +  // implicit guide:
> > +  // template<class T> requires (!__is_same(T, void))
> > +  // B(T) -> B<T>;
> > +};
> > +
> > +void* p;
> > +using type = decltype(B(p)); // previously hard error during dguide
> > overload
> > +			     // resolution because #1's implicit guide would
> > +			     // inherit only the constructor's constraints and
> > +			     // not also the class's.
> > +using type = B<void*>;
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C
> > b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C
> > new file mode 100644
> > index 00000000000..10bb86df6a0
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6.C
> > @@ -0,0 +1,19 @@
> > +// PR c++/104873
> > +// { dg-do compile { target c++20 } }
> > +
> > +template<class T> concept C = true;
> > +
> > +template<class T> requires C<T>
> > +struct S {
> > +  S(...);
> > +};
> > +// synthesized copy deduction candidate:
> > +// template<class T> requires C<T>
> > +// S(S<T>) -> S<T>;
> > +
> > +template<class T> S(S<T>) -> S<void>; // #1
> > +
> > +using type = decltype(S(S<int>())); // The deduction candidate (which
> > inherits
> > +				    // the class's constraints) is preferred
> > +				    // over #1 because it's more constrained.
> 
> This seems like a regression: Presumably people write deduction guides because
> they want them to be used, not to have them silently ignored because the class
> is constrained.
> 
> > +using type = S<int>;
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C
> > b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C
> > new file mode 100644
> > index 00000000000..f9d1d6ec11e
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad6a.C
> > @@ -0,0 +1,19 @@
> > +// PR c++/104873
> > +// { dg-do compile { target c++20 } }
> > +
> > +template<class T> concept C = true;
> > +
> > +template<class T> requires C<T>
> > +struct S {
> > +  S(T); // #1
> > +  // implicit guide:
> > +  // template<class T> requires C<T>
> > +  // S<T> -> S<T>;
> > +};
> > +
> > +template<class T> S(T) -> S<S<T>>; // #2
> > +
> > +using type = decltype(S(0)); // The implicit guide for #1 (which inherits
> > the
> > +			     // class's constraints) is preferred over #2
> > +			     // because it's more constrained.
> 
> Likewise.
> 
> > +using type = S<int>;
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C
> > b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C
> > new file mode 100644
> > index 00000000000..4e1b1abfb94
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-ctad7.C
> > @@ -0,0 +1,26 @@
> > +// { dg-do compile { target c++20 } }
> > +
> > +template<class T> requires __is_same(T, int)
> > +struct A;
> > +
> > +// When checking constraints on the class template, we
> > +// normalize __is_same(T, int) and cache the resulting atom.
> > +A<int> *p;
> > +
> > +template<class T> requires __is_same(T, int)
> > +struct A {
> > +  A(T);
> > +  // implicit guide:
> > +  // template<class T> requires __is_same(T, int)
> > +  // A(T) -> A<T>;
> > +};
> > +
> > +// When checking constraints on the implicit guide, we normalize
> > +// the same expression again with an equivalent but not identical
> > +// set of template parameters (those from the definition vs from
> > +// the forward declaration).  We should notice that that the
> > +// resulting atom is the same as the one we cached earlier, but
> > +// instead we crashed due to an overly strict assert in
> > +// parameter_mapping_equivalent_p that demanded identity instead
> > +// of equivalence of the template parameters in the mapping.
> > +A a = 1;
> 
> 

Re: [PATCH] c++: implicit guides should inherit class constraints [PR104873]

[Jason Merrill]

On 12/20/23 12:00, Patrick Palka wrote:
> On Fri, 1 Apr 2022, Jason Merrill wrote:
> 
>> On 4/1/22 11:17, Patrick Palka wrote:
>>> An implicit guide already inherits the (rewritten) constraints of the
>>> constructor.  Thus it seems natural that the guide must also inherit
>>> the constraints of the class template, since a constructor's constraints
>>> might assume the class's constraints are satisfied, and therefore
>>> checking these two sets of constraints "out of order" may result in hard
>>> errors as in the first testcase below.
>>
>>> This patch makes implicit guides inherit the constraints of the class
>>> template (even for unconstrained constructors, and even for the copy
>>> deduction candidate).
>>>
>>> In passing, this patch gives implicit guides a trailing return type
>>> since that's how they're depicted in the standard (e.g.
>>> [over.match.class.deduct]/6); this changes the order of substitution
>>> into implicit guides in a probably negligible way, especially now that
>>> they inherit the class constraints.
>>>
>>> The parameter_mapping_equivalent_p change is to avoid an ICE in the last
>>> testcase below (described within), reduced from a cmcstl2 testsuite ICE.
>>>
>>> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look like
>>> the right approach?
>>
>> I don't think so, given the testcases below.
>>
>> Maybe fn_type_unification should check formation of the return type of a
>> deduction guide before constraints?  In general, whichever order you do things
>> in, it'll be wrong for some testcase or other.
> 
> Makes sense..  Though I notice the resolution of CWG2628 goes the
> direction of propagating the class's constraints and seems to have been
> approved in November.  I wonder how that discussion went wrt your
> alternative resolution of first checking the return type during
> deduction?  Shall we follow suit with the approved resolution (and
> therefore this patch) for GCC 14?

Hmm, it seems like I failed to ever actually make that suggestion, so I 
suppose let's go ahead with your patch.

I wonder about adding a joust warning for this case, to help people 
understand why their deduction guide is being ignored.  But I'm not sure 
that real code would actually write deduction guides with signatures 
similar enough to constructors that we consider which one is more 
constrained, so it's probably not important.

Jason