On Fri, 5 Mar 2021, Jason Merrill wrote:
> On 3/4/21 9:55 PM, Patrick Palka wrote:
> > On Thu, 4 Mar 2021, Patrick Palka wrote:
> >
> > > On Thu, 4 Mar 2021, Patrick Palka wrote:
> > >
> > > > On Thu, 4 Mar 2021, Jason Merrill wrote:
> > > >
> > > > > On 3/4/21 11:32 AM, Patrick Palka wrote:
> > > > > > On Thu, 4 Mar 2021, Patrick Palka wrote:
> > > > > >
> > > > > > > My recent r11-7454 changed the way do_auto_deduction handles
> > > > > > > constrained
> > > > > > > placeholders during template argument deduction (context ==
> > > > > > > adc_unify)
> > > > > > > when processing_template_decl != 0.
> > > > > > >
> > > > > > > Before the patch, when processing_template_decl != 0 we would just
> > > > > > > ignore the constraints on the placeholder in this situation, and
> > > > > > > proceed
> > > > > > > with deduction:
> > > > > > >
> > > > > > > /* Check any placeholder constraints against the deduced type.
> > > > > > > */
> > > > > > > if (flag_concepts && !processing_template_decl)
> > > > > > > if (tree check = NON_ERROR (PLACEHOLDER_TYPE_CONSTRAINTS
> > > > > > > (auto_node)))
> > > > > > > {
> > > > > > > ...
> > > > > > >
> > > > > > > After the patch, we now punt and return the original placeholder
> > > > > > > type:
> > > > > > >
> > > > > > > /* Check any placeholder constraints against the deduced type.
> > > > > > > */
> > > > > > > if (flag_concepts)
> > > > > > > if (NON_ERROR (PLACEHOLDER_TYPE_CONSTRAINTS (auto_node)))
> > > > > > > {
> > > > > > > if (processing_template_decl)
> > > > > > > /* In general we can't check satisfaction until we
> > > > > > > know all
> > > > > > > template arguments. */
> > > > > > > return type;
> > > > > > > ...
> > > > > > >
> > > > > > > While this change fixed instances where we'd prematurely resolve a
> > > > > > > constrained placeholder return or variable type with non-dependent
> > > > > > > initializer at template parse time (such as PR96444), it broke the
> > > > > > > adc_unify callers that rely on this previous behavior.
> > > > > > >
> > > > > > > So this patch restores the previous behavior during adc_unify
> > > > > > > deduction
> > > > > > > while retaining the new behavior only during adc_variable_type or
> > > > > > > adc_return_type deduction.
> > > > >
> > > > > Sure, it makes sense for adc_unify to behave differently, since in
> > > > > deduction
> > > > > context constraints are checked after all template args have been
> > > > > deduced.
> > > >
> > > > I see.
> > > >
> > > > >
> > > > > > > We additionally now need to pass outer template arguments to
> > > > > > > do_auto_deduction during unify, for sake of constraint checking.
> > > > > > > But we don't want do_auto_deduction to substitute these outer
> > > > > > > arguments
> > > > > > > into type if it's already been done, hence the added
> > > > > > > TEMPLATE_TYPE_LEVEL
> > > > > > > check.
> > > > > > >
> > > > > > > This fixes partial specializations of non-nested templates with
> > > > > > > constrained 'auto' template parameters, but nested templates are
> > > > > > > still
> > > > > > > broken, ultimately because most_specialized_partial_spec passes
> > > > > > > only the
> > > > > > > innermost template arguments to get_partial_spec_bindings, and so
> > > > > > > outer_targs during do_auto_deduction (called from unify) contains
> > > > > > > only
> > > > > > > the innermost template arguments which makes satisfaction unhappy.
> > > > > > > Fixing this might be too invasive at this stage, perhaps.. (Seems
> > > > > > > we
> > > > > > > need to make most_specialized_partial_spec pass all template
> > > > > > > arguments
> > > > > > > to get_partial_spec_bindings.)
> > > > >
> > > > > How did this work before?
> > > >
> > > > Before, it would work, but only if the constraint didn't also depend on
> > > > any outer template arguments. do_auto_deduction would just "surgically"
> > > > replace the auto in the concept-id with the type we deduced and leave
> > > > the other template arguments of the concept-id alone. So if the
> > > > constraint was non-dependent, satisfaction would work regardless of the
> > > > template nesting level.
> > > >
> > > > Now that we try to do perform satisfaction properly, we're sensitive to
> > > > the template nesting level even if the constraint is otherwise
> > > > non-dependent, because the template nesting level determines the level
> > > > of the auto that appears inside the constraint. So we rely on
> > > > outer_targs to contain all levels of outer template arguments, because
> > > > we tack on another level to the end of outer_targs which needs to
> > > > map to the level of the auto for satisfaction.
> > > >
> > > > (As a hacky workaround, when outer_targs is incomplete, can probably
> > > > just augment it with empty levels until it's
> > > > TEMPLATE_TYPE_LEVEL(auto_node)-1
> > > > levels deep, which would fix the nested template case as long as the
> > > > constraint was depended only on the innermost level of template
> > > > arguments.)
> > > >
> > > > >
> > > > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look
> > > > > > > OK for
> > > > > > > trunk? Also tested on range-v3 and cmcstl2.
> > > > > >
> > > > > > Here's the same patch generated with -w which hides the noisy
> > > > > > indentation
> > > > > > changes:
> > > > > >
> > > > > > -- >8 --
> > > > > >
> > > > > > PR c++/99365
> > > > > > * pt.c (do_auto_deduction): When processing_template_decl != 0
> > > > > > and context is adc_unify and we have constraints, pretend the
> > > > > > constraints are satisfied instead of punting. Add some
> > > > > > clarifying sanity checks. Don't substitute outer_targs into
> > > > > > type if not needed.
> > > > > >
> > > > > > gcc/testsuite/ChangeLog:
> > > > > >
> > > > > > PR c++/99365
> > > > > > * g++.dg/cpp2a/concepts-partial-spec9.C: New test.
> > > > > > ---
> > > > > > gcc/cp/pt.c | 24
> > > > > > ++++++++++++++-----
> > > > > > .../g++.dg/cpp2a/concepts-partial-spec9.C | 24
> > > > > > +++++++++++++++++++
> > > > > > 2 files changed, 42 insertions(+), 6 deletions(-)
> > > > > > create mode 100644
> > > > > > gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec9.C
> > > > > >
> > > > > > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > > > > > index a4686e0affb..ce537e4529a 100644
> > > > > > --- a/gcc/cp/pt.c
> > > > > > +++ b/gcc/cp/pt.c
> > > > > > @@ -23693,7 +23693,8 @@ unify (tree tparms, tree targs, tree parm,
> > > > > > tree arg,
> > > > > > int strict,
> > > > > > if (tree a = type_uses_auto (tparm))
> > > > > > {
> > > > > > - tparm = do_auto_deduction (tparm, arg, a, complain,
> > > > > > adc_unify);
> > > > > > + tparm = do_auto_deduction (tparm, arg, a,
> > > > > > + complain, adc_unify, targs);
> > > > > > if (tparm == error_mark_node)
> > > > > > return 1;
> > > > > > }
> > > > > > @@ -29619,13 +29620,21 @@ do_auto_deduction (tree type, tree init,
> > > > > > tree
> > > > > > auto_node,
> > > > > > }
> > > > > > /* Check any placeholder constraints against the deduced
> > > > > > type. */
> > > > > > - if (flag_concepts)
> > > > > > - if (NON_ERROR (PLACEHOLDER_TYPE_CONSTRAINTS (auto_node)))
> > > > > > + if (processing_template_decl && context == adc_unify)
> > > > > > + /* Pretend constraints are satisfied. */;
> > > > > > + else if (flag_concepts
> > > > > > + && NON_ERROR (PLACEHOLDER_TYPE_CONSTRAINTS (auto_node)))
> > > > > > {
> > > > > > if (processing_template_decl)
> > > > > > - /* In general we can't check satisfaction until we know all
> > > > > > - template arguments. */
> > > > > > + {
> > > > > > + /* Even though the initializer is non-dependent, we need to
> > > > > > wait
> > > > > > until
> > > > > > + instantiation time to resolve this constrained
> > > > > > placeholder
> > > > > > variable
> > > > > > + or return type, since the constraint itself may be
> > > > > > dependent. */
> > > > >
> > > > > Can't we check whether the constraint is dependent, and check
> > > > > satisfaction if
> > > > > it isn't? That might be necessary to make later expressions
> > > > > non-dependent
> > > > > that are supposed to be.
> > > >
> > > > We'd have to check if outer_targs (and the deduced type) is dependent
> > > > too. But it seems tricky because outer_targs, during adc_unify
> > > > deduction, is usually at least partially empty which is enough to make
> > > > any_dependent_template_arguments_p return true.
> > >
> > > Ah sorry, I just realized you probably meant we should check whether the
> > > constraint is dependent during adc_variable_type and adc_return_type
> > > deduction. I think that might be straightforward; I'll try it. I'll
> > > also experiment with the hacky workaround mentioned earlier.
> >
> > Here's a patch that incorporates these two ideas:
> >
> > -- >8 --
> >
> > Subject: [PATCH] c++: adc_unify deduction with constrained auto [PR99365]
> >
> > My recent r11-7454 changed the way do_auto_deduction handles constrained
> > placeholders during template argument deduction (context == adc_unify)
> > when processing_template_decl != 0.
> >
> > Before the patch, when processing_template_decl != 0 we would just
> > ignore the constraints on the placeholder in this situation, and proceed
> > with deduction:
> >
> > /* Check any placeholder constraints against the deduced type. */
> > if (flag_concepts && !processing_template_decl)
> > if (tree check = NON_ERROR (PLACEHOLDER_TYPE_CONSTRAINTS (auto_node)))
> > {
> > ...
> >
> > After the patch, we now punt and return the original placeholder type:
> >
> > /* Check any placeholder constraints against the deduced type. */
> > if (flag_concepts)
> > if (NON_ERROR (PLACEHOLDER_TYPE_CONSTRAINTS (auto_node)))
> > {
> > if (processing_template_decl)
> > /* In general we can't check satisfaction until we know all
> > template arguments. */
> > return type;
> > ...
> >
> > While this change fixed instances where we'd prematurely resolve a
> > constrained placeholder return or variable type with non-dependent
> > initializer at template parse time (such as PR96444), it broke the
> > adc_unify callers that rely on the previous behavior.
> >
> > So this patch restores the previous behavior during adc_unify deduction,
> > while retaining the new behavior only during adc_variable_type or
> > adc_return_type deduction.
> >
> > We additionally now need to pass the outer template arguments to
> > do_auto_deduction during unify, for sake of constraint checking.
> > But we don't want do_auto_deduction to substitute these outer arguments
> > into type when the caller has already done so, so this patch adds
> > a TEMPLATE_TYPE_LEVEL check to do_auto_deduction to that effect.
> >
> > Overall, these changes fix partial specialization of non-nested
> > templates with constrained 'auto' template parameters, but nested
> > templates are still broken, ultimately because
> > most_specialized_partial_spec passes only the innermost template
> > arguments to get_partial_spec_bindings, and so outer_targs during
> > do_auto_deduction (called from unify) contains only the innermost
> > template arguments which makes satisfaction unhappy. Fixing this
> > properly is perhaps too risky at this stage, so this patch adds a hack
> > to do_auto_deduction to compensate for callers that don't supply all
> > outer template arguments. The goal of this hack is to ensure
> > placeholder type constraint checking continues to work whenever it
> > worked before r11-7454, namely whenever the constraint is non-dependent.
> >
> > Finally, this patch allows do_auto_deduction to resolve a constrained
> > placeholder type ahead of time (at template parse time), as long as the
> > constraint is non-dependent.
> >
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk? Also tested on range-v3 and cmcstl2.
> >
> > gcc/cp/ChangeLog:
> >
> > PR c++/99365
> > * pt.c (do_auto_deduction): When processing_template_decl != 0
> > and context is adc_unify and we have constraints, pretend the
> > constraints are satisfied instead of punting. Otherwise don't
> > punt unless any of the explicit arguments in the constraint are
> > dependent. Add some clarifying sanity checks. Add a hack to
> > add missing outermost template levels to outer_args before
> > checking satisfaction. Don't substitute outer_targs into type
> > if it's already been done.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR c++/99365
> > * g++.dg/cpp2a/concepts-partial-spec9.C: New test.
> > * g++.dg/cpp2a/concepts-placeholder4: New test.
> > ---
> > gcc/cp/pt.c | 44 ++++++++++++++++---
> > .../g++.dg/cpp2a/concepts-partial-spec9.C | 23 ++++++++++
> > .../g++.dg/cpp2a/concepts-placeholder4.C | 24 ++++++++++
> > 3 files changed, 85 insertions(+), 6 deletions(-)
> > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec9.C
> > create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-placeholder4.C
> >
> > diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> > index 83589101c0d..31386a74a59 100644
> > --- a/gcc/cp/pt.c
> > +++ b/gcc/cp/pt.c
> > @@ -23681,7 +23681,8 @@ unify (tree tparms, tree targs, tree parm, tree arg,
> > int strict,
> > if (tree a = type_uses_auto (tparm))
> > {
> > - tparm = do_auto_deduction (tparm, arg, a, complain, adc_unify);
> > + tparm = do_auto_deduction (tparm, arg, a,
> > + complain, adc_unify, targs);
> > if (tparm == error_mark_node)
> > return 1;
> > }
> > @@ -29611,13 +29612,24 @@ do_auto_deduction (tree type, tree init, tree
> > auto_node,
> > }
> > /* Check any placeholder constraints against the deduced type. */
> > - if (flag_concepts)
> > - if (NON_ERROR (PLACEHOLDER_TYPE_CONSTRAINTS (auto_node)))
> > + if (processing_template_decl && context == adc_unify)
> > + /* Pretend constraints are satisfied. */;
>
> Let's say "constraints will be checked after deduction".
Done.
>
> > + else if (tree constr = NON_ERROR (PLACEHOLDER_TYPE_CONSTRAINTS
> > (auto_node)))
> > {
> > if (processing_template_decl)
> > - /* In general we can't check satisfaction until we know all
> > - template arguments. */
> > + {
> > + gcc_checking_assert (context == adc_variable_type
> > + || context == adc_return_type);
> > + gcc_checking_assert (!type_dependent_expression_p (init));
>
> This assert seems redundant with the condition at the top of the function,
> though I suppose it's harmless to have it here as well.
Yeah, I figured the assert might be beneficial to the reader, as a
reminder that we're dealing with a non-dependent initializer, since the
condition is a bit far away at this point.
>
> > + /* Check if any of the explicit arguments in the constraint
> > + are dependent. If so, we need to wait until instantiation time
> > + to resolve the constriant. */
> > + tree cid = unpack_concept_check (constr);
> > + tree cargs = TREE_OPERAND (cid, 1);
> > + for (int i = 1; i < TREE_VEC_LENGTH (cargs); ++i)
> > + if (dependent_template_arg_p (TREE_VEC_ELT (cargs, i)))
> > return type;
>
> Let's factor this out; I imagine testing whether a concept-check is dependent
> (apart from the placeholder type itself) may be more broadly useful.
Done as well (into a predicate placeholder_type_constraint_dependent_p).
How does the following look? (testing in progress)
-- >8 --
gcc/cp/ChangeLog:
PR c++/99365
* pt.c (unify) <case TEMPLATE_TYPE_PARM>: Pass targs as
outer_targs to do_auto_deduction.
(placeholder_type_constraint_dependent_p): Define.
(do_auto_deduction): When processing_template_decl != 0
and context is adc_unify and we have constraints, pretend the
constraints are satisfied instead of punting. Otherwise don't
punt unless placeholder_type_constraint_dependent_p holds.
Add some clarifying sanity checks. Add a hack to add missing
outermost template levels to outer_args before checking
satisfaction. Don't substitute outer_targs into type if it's
already been done.
gcc/testsuite/ChangeLog:
PR c++/99365
* g++.dg/cpp2a/concepts-partial-spec9.C: New test.
* g++.dg/cpp2a/concepts-placeholder4: New test.
---
gcc/cp/pt.c | 57 +++++++++++++++++--
.../g++.dg/cpp2a/concepts-partial-spec9.C | 23 ++++++++
.../g++.dg/cpp2a/concepts-placeholder4.C | 24 ++++++++
3 files changed, 98 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec9.C
create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-placeholder4.C
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 8ca3dc8ec2b..b2ecd0b3213 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -23683,7 +23683,8 @@ unify (tree tparms, tree targs, tree parm, tree arg,
int strict,
if (tree a = type_uses_auto (tparm))
{
- tparm = do_auto_deduction (tparm, arg, a, complain, adc_unify);
+ tparm = do_auto_deduction (tparm, arg, a,
+ complain, adc_unify, targs);
if (tparm == error_mark_node)
return 1;
}
@@ -28205,6 +28206,23 @@ make_constrained_decltype_auto (tree con, tree args)
return make_constrained_placeholder_type (type, con, args);
}
+/* Return true if the placeholder type constraint T has any dependent
+ (explicit) template arguments. */
+
+static bool
+placeholder_type_constraint_dependent_p (tree t)
+{
+ tree id = unpack_concept_check (t);
+ tree args = TREE_OPERAND (id, 1);
+ tree first = TREE_VEC_ELT (args, 0);
+ gcc_checking_assert (TREE_CODE (first) == WILDCARD_DECL
+ || is_auto (first));
+ for (int i = 1; i < TREE_VEC_LENGTH (args); ++i)
+ if (dependent_template_arg_p (TREE_VEC_ELT (args, i)))
+ return true;
+ return false;
+}
+
/* Build and return a concept definition. Like other templates, the
CONCEPT_DECL node is wrapped by a TEMPLATE_DECL. This returns the
the TEMPLATE_DECL. */
@@ -29613,13 +29631,20 @@ do_auto_deduction (tree type, tree init, tree
auto_node,
}
/* Check any placeholder constraints against the deduced type. */
- if (flag_concepts)
- if (NON_ERROR (PLACEHOLDER_TYPE_CONSTRAINTS (auto_node)))
+ if (processing_template_decl && context == adc_unify)
+ /* Constraints will be checked after deduction. */;
+ else if (tree constr = NON_ERROR (PLACEHOLDER_TYPE_CONSTRAINTS (auto_node)))
{
if (processing_template_decl)
- /* In general we can't check satisfaction until we know all
- template arguments. */
+ {
+ gcc_checking_assert (context == adc_variable_type
+ || context == adc_return_type);
+ gcc_checking_assert (!type_dependent_expression_p (init));
+ /* If the constraint is dependent, we need to wait until
+ instantiation time to resolve the placeholder. */
+ if (placeholder_type_constraint_dependent_p (constr))
return type;
+ }
if ((context == adc_return_type || context == adc_variable_type)
&& current_function_decl
@@ -29627,6 +29652,23 @@ do_auto_deduction (tree type, tree init, tree
auto_node,
outer_targs = DECL_TI_ARGS (current_function_decl);
tree full_targs = add_to_template_args (outer_targs, targs);
+
+ /* HACK: Compensate for callers not always communicating all levels of
+ outer template arguments by filling in the outermost missing levels
+ with dummy levels before checking satisfaction. We'll still crash
+ if the constraint depends on a template argument belonging to one of
+ these missing levels, but this hack otherwise allows us to handle a
+ large subset of possible constraints (including all non-dependent
+ constraints). */
+ if (int missing_levels = (TEMPLATE_TYPE_ORIG_LEVEL (auto_node)
+ - TMPL_ARGS_DEPTH (full_targs)))
+ {
+ tree dummy_levels = make_tree_vec (missing_levels);
+ for (int i = 0; i < missing_levels; ++i)
+ TREE_VEC_ELT (dummy_levels, i) = make_tree_vec (0);
+ full_targs = add_to_template_args (dummy_levels, full_targs);
+ }
+
if (!constraints_satisfied_p (auto_node, full_targs))
{
if (complain & tf_warning_or_error)
@@ -29658,7 +29700,10 @@ do_auto_deduction (tree type, tree init, tree
auto_node,
}
}
- if (context == adc_unify)
+ if (TEMPLATE_TYPE_LEVEL (auto_node) == 1)
+ /* The outer template arguments are already substituted into type
+ (but we still may have used them for constraint checking above). */;
+ else if (context == adc_unify)
targs = add_to_template_args (outer_targs, targs);
else if (processing_template_decl)
targs = add_to_template_args (current_template_args (), targs);
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec9.C
b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec9.C
new file mode 100644
index 00000000000..3dae24d915a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec9.C
@@ -0,0 +1,23 @@
+// PR c++/99365
+// { dg-do compile { target c++20 } }
+
+template <class> concept C = true;
+template <class T, class U> concept D = C<T> && __is_same(T, U);
+
+template <class, C auto> struct A { static const int i = 0; };
+template <class T, D<T> auto V> struct A<T, V> { static const int i = 1; };
+
+static_assert(A<int, 0>::i == 1);
+static_assert(A<char, 0>::i == 0);
+static_assert(A<int, '0'>::i == 0);
+static_assert(A<char, '0'>::i == 1);
+
+template <class> struct O {
+ template <class, C auto> struct A { static const int i = 0; };
+ template <class T, D<T> auto V> struct A<T, V> { static const int i = 1; };
+};
+
+static_assert(O<void>::A<int, 0>::i == 1);
+static_assert(O<void>::A<char, 0>::i == 0);
+static_assert(O<void>::A<int, '0'>::i == 0);
+static_assert(O<void>::A<char, '0'>::i == 1);
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder4.C
b/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder4.C
new file mode 100644
index 00000000000..082c5d36cbe
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder4.C
@@ -0,0 +1,24 @@
+// { dg-do compile { target c++20 } }
+
+template <class T, class U> concept same_as = __is_same(T, U);
+
+// non-dependent initializer, always-satisfied constraint resolved at parse
time
+template <class T> same_as<bool> auto x = true;
+
+template <auto V> same_as<int> auto y = V; // { dg-error "constraint" }
+
+template <class>
+struct A {
+ template <auto V> static inline same_as<int> auto z = V; // { dg-error
"constraint" }
+};
+
+int main() {
+ x<int>; // { dg-bogus "" }
+ y<0>; // { dg-bogus "" }
+ y<'0'>; // { dg-message "required from here" }
+ A<void>::z<0>; // { dg-bogus "" }
+ A<void>::z<'0'>; // { dg-message "required from here" }
+}
+
+// non-dependent initializer, never-satisfied constraint diagnosed at parse
time
+template <class T> same_as<int> auto z = true; // { dg-error "constraint" }
--
2.31.0.rc0.75.gec125d1bc1
