Re: [PATCH] c++: detecting copy-init context during CTAD [PR102137]

[Jason Merrill <jason@redhat.com>]

On 3/7/22 10:47, Patrick Palka wrote:
> On Fri, 4 Mar 2022, Jason Merrill wrote:
> 
>> On 3/4/22 14:24, Patrick Palka wrote:
>>> Here we're failing to communicate to cp_finish_decl from tsubst_expr
>>> that we're in a copy-initialization context (via the LOOKUP_ONLYCONVERTING
>>> flag), which causes do_class_deduction to always consider explicit
>>> deduction guides when performing CTAD for a templated variable initializer.
>>>
>>> We could fix this by passing LOOKUP_ONLYCONVERTING appropriately when
>>> calling cp_finish_decl from tsubst_expr, but it seems do_class_deduction
>>> can determine if we're in a copy-init context by simply inspecting the
>>> initializer, and thus render its flags parameter unnecessary, which is
>>> what this patch implements.  (If we were to fix this in tsubst_expr
>>> instead, I think we'd have to inspect the initializer in the same way
>>> in order to detect a copy-init context?)
>>
>> Hmm, does this affect conversions as well?
>>
>> Looks like it does:
>>
>> struct A
>> {
>>    explicit operator int();
>> };
>>
>> template <class T> void f()
>> {
>>    T t = A();
>> }
>>
>> int main()
>> {
>>    f<int>(); // wrongly accepted
>> }
>>
>> The reverse, initializing via an explicit constructor, is caught by code in
>> build_aggr_init much like the code your patch adds to do_auto_deduction;
>> perhaps we should move/copy that code to cp_finish_decl?
> 
> Ah, makes sense.  Moving that code from build_aggr_init to
> cp_finish_decl broke things, but using it in both spots seems to work
> well.  And I suppose we might as well use it in do_class_deduction too,
> since doing so lets us remove the flags parameter.

Before removing the flags parameter please try asserting that it now 
matches is_copy_initialization and see if anything breaks.

> -- >8 --
> 
> Subject: [PATCH] c++: detecting copy-init context during CTAD [PR102137]
> 
> Here we're failing to communicate to cp_finish_decl from tsubst_expr
> that we're in a copy-initialization context (via the LOOKUP_ONLYCONVERTING
> flag), which causes us to always consider explicit deduction guides when
> performing CTAD for a templated variable initializer.
> 
> It turns out this bug also affects consideration of explicit conversion
> operators for the same reason.  But consideration of explicit constructors
> is unaffected and seems to work correctly thanks to code in build_aggr_init
> that sets LOOKUP_ONLYCONVERTING when the initializer represents
> copy-initialization.
> 
> This patch factors out the copy-initialization test from build_aggr_init
> and reuses it in tsubst_expr for sake of cp_finish_decl.  And we might
> as well use it in do_class_deduction too, since doing so lets us get rid
> of its flags parameter (which is used only to control whether explicit
> deduction guides are considered).
> 
> Bootstrapped and regtestd on x86_64-pc-linux-gnu, does this look OK for
> trunk?
> 
> 	PR c++/102137
> 	PR c++/87820
> 
> gcc/cp/ChangeLog:
> 
> 	* cp-tree.h (is_copy_initialization): Declare.
> 	(do_auto_deduction): Remove flags parameter.
> 	* decl.cc (cp_finish_decl): Adjust call to do_auto_deduction.
> 	* init.cc (build_aggr_init): Split out copy-initialization
> 	check into ...
> 	(is_copy_initialization): ... here.
> 	* pt.cc (convert_template_argument): Adjust call to
> 	do_auto_deduction.
> 	(tsubst_expr) <case VAR_DECL>: Pass LOOKUP_ONLYCONVERTING
> 	to cp_finish_decl when is_copy_initialization.
> 	(do_class_deduction): Remove flags parameter, and instead
> 	call is_copy_initialization to determine if we're in a copy-init
> 	context.
> 	(do_auto_deduction): Adjust call to do_class_deduction.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp0x/explicit15.C: New test.
> 	* g++.dg/cpp1z/class-deduction108.C: New test.
> ---
>   gcc/cp/cp-tree.h                              |  4 +--
>   gcc/cp/decl.cc                                |  2 +-
>   gcc/cp/init.cc                                | 20 +++++++++---
>   gcc/cp/pt.cc                                  | 23 ++++++++------
>   gcc/testsuite/g++.dg/cpp0x/explicit15.C       | 31 +++++++++++++++++++
>   .../g++.dg/cpp1z/class-deduction108.C         | 31 +++++++++++++++++++
>   6 files changed, 93 insertions(+), 18 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/explicit15.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction108.C
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index ac723901098..be556d2c441 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -7039,6 +7039,7 @@ extern void emit_mem_initializers		(tree);
>   extern tree build_aggr_init			(tree, tree, int,
>                                                    tsubst_flags_t);
>   extern int is_class_type			(tree, int);
> +extern bool is_copy_initialization		(tree);
>   extern tree build_zero_init			(tree, tree, bool);
>   extern tree build_value_init			(tree, tsubst_flags_t);
>   extern tree build_value_init_noctor		(tree, tsubst_flags_t);
> @@ -7279,8 +7280,7 @@ extern tree do_auto_deduction                   (tree, tree, tree,
>   						 = tf_warning_or_error,
>                                                    auto_deduction_context
>   						 = adc_unspecified,
> -						 tree = NULL_TREE,
> -						 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/decl.cc b/gcc/cp/decl.cc
> index 199ac768d43..152f657e9f2 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -8039,7 +8039,7 @@ cp_finish_decl (tree decl, tree init, bool init_const_expr_p,
>   	outer_targs = DECL_TI_ARGS (decl);
>         type = TREE_TYPE (decl) = do_auto_deduction (type, d_init, auto_node,
>   						   tf_warning_or_error, adc,
> -						   outer_targs, flags);
> +						   outer_targs);
>         if (type == error_mark_node)
>   	return;
>         if (TREE_CODE (type) == FUNCTION_TYPE)
> diff --git a/gcc/cp/init.cc b/gcc/cp/init.cc
> index 545d904c0f9..d5faa0dc519 100644
> --- a/gcc/cp/init.cc
> +++ b/gcc/cp/init.cc
> @@ -2019,11 +2019,7 @@ build_aggr_init (tree exp, tree init, int flags, tsubst_flags_t complain)
>         return stmt_expr;
>       }
>   
> -  if (init && init != void_type_node
> -      && TREE_CODE (init) != TREE_LIST
> -      && !(TREE_CODE (init) == TARGET_EXPR
> -	   && TARGET_EXPR_DIRECT_INIT_P (init))
> -      && !DIRECT_LIST_INIT_P (init))
> +  if (is_copy_initialization (init))
>       flags |= LOOKUP_ONLYCONVERTING;
>   
>     is_global = begin_init_stmts (&stmt_expr, &compound_stmt);
> @@ -2331,6 +2327,20 @@ is_class_type (tree type, int or_else)
>     return 1;
>   }
>   
> +/* Returns true iff the variable initializer INIT represents
> +   copy-initialization (and therefore we must set LOOKUP_ONLYCONVERTING
> +   when processing it).  */
> +
> +bool
> +is_copy_initialization (tree init)
> +{
> +  return (init && init != void_type_node
> +	  && TREE_CODE (init) != TREE_LIST
> +	  && !(TREE_CODE (init) == TARGET_EXPR
> +	       && TARGET_EXPR_DIRECT_INIT_P (init))
> +	  && !DIRECT_LIST_INIT_P (init));
> +}
> +
>   /* Build a reference to a member of an aggregate.  This is not a C++
>      `&', but really something which can have its address taken, and
>      then act as a pointer to member, for example TYPE :: FIELD can have
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index d94d4538faa..a3ff81e2c3a 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -8567,8 +8567,7 @@ convert_template_argument (tree parm,
>   	   can happen in the context of -fnew-ttp-matching.  */;
>         else if (tree a = type_uses_auto (t))
>   	{
> -	  t = do_auto_deduction (t, arg, a, complain, adc_unify, args,
> -				 LOOKUP_IMPLICIT);
> +	  t = do_auto_deduction (t, arg, a, complain, adc_unify, args);
>   	  if (t == error_mark_node)
>   	    return error_mark_node;
>   	}
> @@ -18606,7 +18605,11 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl,
>   			TREE_TYPE (asmspec_tree) = char_array_type_node;
>   		      }
>   
> -		    cp_finish_decl (decl, init, const_init, asmspec_tree, 0);
> +
> +		    int flags = LOOKUP_NORMAL;
> +		    if (VAR_P (decl) && is_copy_initialization (init))
> +		      flags |= LOOKUP_ONLYCONVERTING;
> +		    cp_finish_decl (decl, init, const_init, asmspec_tree, flags);
>   
>   		    if (ndecl != error_mark_node)
>   		      cp_finish_decomp (ndecl, first, cnt);
> @@ -29832,8 +29835,7 @@ ctad_template_p (tree tmpl)
>      type.  */
>   
>   static tree
> -do_class_deduction (tree ptype, tree tmpl, tree init,
> -		    int flags, tsubst_flags_t complain)
> +do_class_deduction (tree ptype, tree tmpl, tree init, tsubst_flags_t complain)
>   {
>     /* We should have handled this in the caller.  */
>     if (DECL_TEMPLATE_TEMPLATE_PARM_P (tmpl))
> @@ -29885,6 +29887,7 @@ do_class_deduction (tree ptype, tree tmpl, tree init,
>   
>     bool try_list_ctor = false;
>     bool list_init_p = false;
> +  bool copy_init_p = is_copy_initialization (init);
>   
>     releasing_vec rv_args = NULL;
>     vec<tree,va_gc> *&args = *&rv_args;
> @@ -29929,7 +29932,7 @@ do_class_deduction (tree ptype, tree tmpl, tree init,
>     /* Prune explicit deduction guides in copy-initialization context (but
>        not copy-list-initialization).  */
>     bool elided = false;
> -  if (!list_init_p && (flags & LOOKUP_ONLYCONVERTING))
> +  if (!list_init_p && copy_init_p)
>       {
>         for (lkp_iterator iter (cands); !elided && iter; ++iter)
>   	if (DECL_NONCONVERTING_P (STRIP_TEMPLATE (*iter)))
> @@ -30007,7 +30010,7 @@ do_class_deduction (tree ptype, tree tmpl, tree init,
>       }
>     /* [over.match.list]/1: In copy-list-initialization, if an explicit
>        constructor is chosen, the initialization is ill-formed.  */
> -  else if (flags & LOOKUP_ONLYCONVERTING)
> +  else if (copy_init_p)
>       {
>         if (DECL_NONCONVERTING_P (fndecl))
>   	{
> @@ -30045,7 +30048,7 @@ do_class_deduction (tree ptype, tree tmpl, tree init,
>   /* Replace occurrences of 'auto' in TYPE with the appropriate type deduced
>      from INIT.  AUTO_NODE is the TEMPLATE_TYPE_PARM used for 'auto' in TYPE.
>      The CONTEXT determines the context in which auto deduction is performed
> -   and is used to control error diagnostics.  FLAGS are the LOOKUP_* flags.
> +   and is used to control error diagnostics.
>   
>      OUTER_TARGS is used during template argument deduction (context == adc_unify)
>      to properly substitute the result.  It's also used in the adc_unify and
> @@ -30058,7 +30061,7 @@ do_class_deduction (tree ptype, tree tmpl, tree init,
>   tree
>   do_auto_deduction (tree type, tree init, tree auto_node,
>                      tsubst_flags_t complain, auto_deduction_context context,
> -		   tree outer_targs, int flags)
> +		   tree outer_targs)
>   {
>     if (init == error_mark_node)
>       return error_mark_node;
> @@ -30079,7 +30082,7 @@ do_auto_deduction (tree type, tree init, tree auto_node,
>   
>     if (tree tmpl = CLASS_PLACEHOLDER_TEMPLATE (auto_node))
>       /* C++17 class template argument deduction.  */
> -    return do_class_deduction (type, tmpl, init, flags, complain);
> +    return do_class_deduction (type, tmpl, init, complain);
>   
>     if (init == NULL_TREE || TREE_TYPE (init) == NULL_TREE)
>       /* Nothing we can do with this, even in deduction context.  */
> diff --git a/gcc/testsuite/g++.dg/cpp0x/explicit15.C b/gcc/testsuite/g++.dg/cpp0x/explicit15.C
> new file mode 100644
> index 00000000000..f6c584d5f88
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/explicit15.C
> @@ -0,0 +1,31 @@
> +// PR c++/87820
> +// { dg-do compile { target c++11 } }
> +
> +struct A {
> +  explicit operator int();
> +};
> +
> +template<class T>
> +void f() {
> +  A a;
> +  T t1 = a; // { dg-error "cannot convert" }
> +  T t2 = {a}; // { dg-error "cannot convert" }
> +  T t3(a);
> +  T t4{a};
> +  T t5 = T(a);
> +  T t6 = T{a};
> +}
> +
> +template<class T>
> +void g() {
> +  T t;
> +  int n1 = t; // { dg-error "cannot convert" }
> +  int n2 = {t}; // { dg-error "cannot convert" }
> +  int n3(t);
> +  int n4{t};
> +  int n5 = int(t);
> +  int n6 = int{t};
> +}
> +
> +template void f<int>();
> +template void g<A>();
> diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction108.C b/gcc/testsuite/g++.dg/cpp1z/class-deduction108.C
> new file mode 100644
> index 00000000000..9e36c53c1a4
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction108.C
> @@ -0,0 +1,31 @@
> +// PR c++/102137
> +// { dg-do compile { target c++17 } }
> +
> +template<class T>
> +struct A {
> +  A();
> +  A(int);
> +};
> +
> +explicit A(...) -> A<int>;
> +
> +template<template<class> class T>
> +void f() {
> +  T x1 = 0; // { dg-error "deduction|no match" }
> +  T x2 = {0}; // { dg-error "explicit deduction guide" }
> +  T x3(0);
> +  T x4{0};
> +  T x5;
> +}
> +
> +template<class T>
> +void g(T t) {
> +  A a1 = t; // { dg-error "deduction|no match" }
> +  A a2 = {t}; // { dg-error "explicit deduction guide" }
> +  A a3(t);
> +  A a4{t};
> +  A a5;
> +}
> +
> +template void f<A>();
> +template void g(int);