Re: [PATCH] c++: adc_unify deduction with constrained auto [PR99365]

[Patrick Palka <ppalka@redhat.com>]

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.

> 
> > +	  gcc_checking_assert (context == adc_variable_type
> > +			       || context == adc_return_type);
> > +	  gcc_checking_assert (!type_dependent_expression_p (init));
> >   	  return type;
> > +	}
> >           if ((context == adc_return_type || context == adc_variable_type)
> >   	  && current_function_decl
> > @@ -29664,7 +29673,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..b79d12b6f17
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-partial-spec9.C
> > @@ -0,0 +1,24 @@
> > +// 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);
> > +
> > +/* Partial specialization of nested class template with constrained 'auto'
> > +   template parameter still broken:
> > +
> > +template <class> struct O {
> > +  template <C auto> struct B { static const int i = 0; };
> > +  template <D auto V> struct B<V> { static const int i = 1; };
> > +};
> > +
> > +static_assert(O<void>::B<int, 0>::i == 0);
> > +static_assert(O<void>::B<int, '0'>::i == 1);  */
> > 
> 
>