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

[Jason Merrill <jason@redhat.com>]

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, 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.

> +      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" }
> +}