Re: [PATCH] c++: NTTP constraint depending on outer args [PR109160]

[Patrick Palka <ppalka@redhat.com>]

On Wed, 29 Mar 2023, Jason Merrill wrote:

> On 3/17/23 11:26, Patrick Palka wrote:
> > Here we're crashing during satisfaction for the NTTP 'C<B> auto' from
> > do_auto_deduction ultimately because convert_template_argument / unify
> > don't pass all outer template arguments to do_auto_deduction, and during
> > satisfaction we need to know all arguments.  While these callers do
> > pass some outer arguments, they are only sufficient to properly
> > substitute the 'auto' and are not necessarily the complete set.
> > 
> > Fortunately it seems it's possible to obtain the full set of outer
> > arguments from these callers via convert_template_argument's IN_DECL
> > parameter and unify's TPARMS parameter.  So this patch adds a TMPL
> > parameter to do_auto_deduction, used only during adc_unify deduction,
> > which contains the (partially instantiated) template corresponding to
> > this auto and from which we can obtain all outer template arguments for
> > satisfaction.
> > 
> > This patch also adjusts the IN_DECL argument passed to
> > coerce_template_parms from tsubst_decl so that we could in turn safely
> > assume convert_template_argument's IN_DECL is always a TEMPLATE_DECL,
> > and thus could pass it as-is to do_auto_deduction.  (tsubst_decl seems
> > to be the only caller that passes a non-empty non-template IN_DECL to
> > coerce_template_parms.)
> > 
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk/12?
> > 
> > 	PR c++/109160
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* cp-tree.h (do_auto_deduction): Add defaulted TMPL parameter.
> > 	* pt.cc (convert_template_argument): Pass IN_DECL as TMPL to
> > 	do_auto_deduction.
> > 	(tsubst_decl) <case VAR_/TYPE_DECL>: Pass TMPL instead of T as
> > 	IN_DECL to coerce_template_parms.
> > 	(unify) <case TEMPLATE_PARM_INDEX>: Pass the corresponding
> > 	template as TMPL to do_auto_deduction.
> > 	(do_auto_deduction): Document default arguments.  Use TMPL
> > 	to obtain a full set of template arguments for satisfaction
> > 	in the adc_unify case.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/cpp2a/concepts-placeholder12.C: New test.
> > ---
> >   gcc/cp/cp-tree.h                              |  3 +-
> >   gcc/cp/pt.cc                                  | 30 ++++++++++++++-----
> >   .../g++.dg/cpp2a/concepts-placeholder12.C     | 29 ++++++++++++++++++
> >   3 files changed, 53 insertions(+), 9 deletions(-)
> >   create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-placeholder12.C
> > 
> > diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> > index dfc1c845768..e7190c5cc62 100644
> > --- a/gcc/cp/cp-tree.h
> > +++ b/gcc/cp/cp-tree.h
> > @@ -7324,7 +7324,8 @@ extern tree do_auto_deduction                   (tree,
> > tree, tree,
> >                                                    auto_deduction_context
> >   						 = adc_unspecified,
> >   						 tree = NULL_TREE,
> > -						 int = LOOKUP_NORMAL);
> > +						 int = LOOKUP_NORMAL,
> > +						 tree = NULL_TREE);
> >   extern tree type_uses_auto			(tree);
> >   extern tree type_uses_auto_or_concept		(tree);
> >   extern void append_type_to_template_for_access_check (tree, tree, tree,
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index ddbd73371b9..6400b686a58 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -8638,7 +8638,7 @@ convert_template_argument (tree parm,
> >         else if (tree a = type_uses_auto (t))
> >   	{
> >   	  t = do_auto_deduction (t, arg, a, complain, adc_unify, args,
> > -				 LOOKUP_IMPLICIT);
> > +				 LOOKUP_IMPLICIT, in_decl);
> >   	  if (t == error_mark_node)
> >   	    return error_mark_node;
> >   	}
> > @@ -15243,7 +15243,7 @@ tsubst_decl (tree t, tree args, tsubst_flags_t
> > complain)
> >   		     the template.  */
> >   		  argvec = (coerce_template_parms
> >   			    (DECL_TEMPLATE_PARMS (gen_tmpl),
> > -			     argvec, t, complain));
> > +			     argvec, tmpl, complain));
> >   		if (argvec == error_mark_node)
> >   		  RETURN (error_mark_node);
> >   		hash = spec_hasher::hash (gen_tmpl, argvec);
> > @@ -24655,7 +24655,9 @@ 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, targs);
> > +					 complain, adc_unify, targs,
> > +					 LOOKUP_NORMAL,
> > +					 TPARMS_PRIMARY_TEMPLATE (tparms)); >
> > if (tparm == error_mark_node)
> >   		return 1;
> >   	    }
> > @@ -30643,13 +30645,20 @@ unparenthesized_id_or_class_member_access_p (tree
> > init)
> >      adc_requirement contexts to communicate the necessary template
> > arguments
> >      to satisfaction.  OUTER_TARGS is ignored in other contexts.
> >   -   For partial-concept-ids, extra args may be appended to the list of
> > deduced
> > -   template arguments prior to determining constraint satisfaction.  */
> > +   Additionally for adc_unify contexts TMPL is the template for which this
> > +   auto is a template parameter type.
> > +
> > +   For partial-concept-ids, extra args from OUTER_TARGS, TMPL and the
> > current
> > +   scope may be appended to the list of deduced template arguments prior to
> > +   determining constraint satisfaction as appropriate.  */
> >     tree
> >   do_auto_deduction (tree type, tree init, tree auto_node,
> > -                   tsubst_flags_t complain, auto_deduction_context context,
> > -		   tree outer_targs, int flags)
> > +		   tsubst_flags_t complain /* = tf_warning_or_error */,
> > +		   auto_deduction_context context /* = adc_unspecified */,
> > +		   tree outer_targs /* = NULL_TREE */,
> > +		   int flags /* = LOOKUP_NORMAL */,
> > +		   tree tmpl /* = NULL_TREE */)
> >   {
> >     if (init == error_mark_node)
> >       return error_mark_node;
> > @@ -30839,7 +30848,12 @@ do_auto_deduction (tree type, tree init, tree
> > auto_node,
> >   		}
> >   	    }
> >   -      tree full_targs = add_to_template_args (outer_targs, targs);
> > +      tree full_targs = outer_targs;
> > +
> > +      if (context == adc_unify)
> > +	full_targs = add_outermost_template_args (tmpl, full_targs);
> 
> I recently noticed that sometimes TPARMS_PRIMARY_TEMPLATE isn't set properly
> for partially instantiated template template parameters,

*nod* as well as for partially instantiated partial specializations it seems.

> so let's make this
> more robust by only doing this if tmpl is non-null, maybe with a
> checking_assert that it is.  OK with that change.

Thanks a lot.  Looks like such a checking_assert would cause us to ICE
on cpp2a/concepts-partial-spec9.C during partial ordering for the
partial specializations of O<void>::A despite being able to check
satisfaction of the NTTP constraints just fine (since they don't depend
on outermost parameters).  In light of this it seems preferable to
ignore null tmpl here and proceed with satisfaction, so that we'll ICE
(by way of segfault) only if the constraint depends on outermost
template parameters whose arguments we lack.  So I went ahead and added
a check for non-null tmpl, but no checking_assert.

> 
> > +      full_targs = add_to_template_args (full_targs, targs);
> >           /* HACK: Compensate for callers not always communicating all
> > levels of
> >   	 outer template arguments by filling in the outermost missing levels
> > diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder12.C
> > b/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder12.C
> > new file mode 100644
> > index 00000000000..3d4d138720e
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp2a/concepts-placeholder12.C
> > @@ -0,0 +1,29 @@
> > +// PR c++/109160
> > +// { dg-do compile { target c++20 } }
> > +
> > +template<class T, bool B>
> > +concept C = B;
> > +
> > +template<int> struct X { };
> > +
> > +template<bool B>
> > +struct A {
> > +  template<C<B> auto V> static void f();
> > +  template<C<B> auto V> static void g(X<V>);
> > +  template<C<B> auto V> static inline int value = V;
> > +  template<C<B> auto V> struct D { };
> > +};
> > +
> > +int main() {
> > +  A<true>::f<0>();
> > +  A<false>::f<0>(); // { dg-error "no match|constraints" }
> > +
> > +  A<true>::g(X<0>{});
> > +  A<false>::g(X<0>{}); // { dg-error "no match|constraints" }
> > +
> > +  bool v1 = A<true>::value<0>;
> > +  bool v2 = A<false>::value<0>;  // { dg-error "constraints" }
> > +
> > +  A<true>::D<0> d1;
> > +  A<false>::D<0> d2; // { dg-error "constraints" }
> > +}
> 
>