[PATCH] c++: naming a dependently-scoped template for CTAD [PR104641]

Jason Merrill jason@redhat.com
Fri Mar 11 23:49:39 GMT 2022


On 3/10/22 12:41, Patrick Palka wrote:
> On Wed, 9 Mar 2022, Jason Merrill wrote:
> 
>> On 3/9/22 10:39, Patrick Palka wrote:
>>> On Tue, 8 Mar 2022, Jason Merrill wrote:
>>>
>>>> On 3/2/22 14:32, Patrick Palka wrote:
>>>>> In order to be able to perform CTAD for a dependently-scoped template
>>>>> such as A<T>::B in the testcase below, we need to permit a
>>>>> typename-specifier to resolve to a template as per [dcl.type.simple]/2,
>>>>> at least when it appears in a CTAD-enabled context.
>>>>>
>>>>> This patch implements this using a new tsubst flag tf_tst_ok to control
>>>>> when a TYPENAME_TYPE is allowed to name a template, and sets this flag
>>>>> when substituting into the type of a CAST_EXPR, CONSTRUCTOR or VAR_DECL
>>>>> (each of which is a CTAD-enabled context).
>>>>
>>>> What breaks if we always allow that, or at least in -std that support
>>>> CTAD?
>>>
>>> AFAICT no significant breakage, but some accepts-invalid and diagnostic
>>> regressions crop up, e.g. accepts-invalid for
>>>
>>>     using type = typename A<T>::B; // no more diagnostic if typename resolves
>>> to a
>>>                                    // template at instantiation time
>>>
>>> and diagnostic regression for
>>>
>>>     template<class T, class = typename A<T>::B> void f();
>>>     // no more elaboration why deduction failed if typename resolves
>>>     // to a template
>>
>> Ah, sure, the cost is that we would need to check for this case in various
>> callers, rather than setting a flag in a different set of callers.  Fair
>> enough.
> 
> Yes exactly, and presumably the set of callers for which typename is
> permitted to resolve to a template is much smaller, so making the
> behavior opt-in instead of opt-out seems more desirable.  Alternatively
> we could add a new flag to TYPENAME_TYPE set carefully at parse time
> that controls this behavior, but seems overall simpler to not use a
> new tree flag if we can get away with it.
> 
>>
>>> @@ -16229,6 +16237,12 @@ tsubst (tree t, tree args, tsubst_flags_t complain,
>>> tree in_decl)
>>>   	      }
>>>   	  }
>>>   +	if (TREE_CODE (f) == TEMPLATE_DECL)
>>> +	  {
>>> +	    gcc_checking_assert (tst_ok);
>>> +	    f = make_template_placeholder (f);
>>> +	  }
>>
>> How about calling make_template_placeholder in make_typename_type?
> 
> That works nicely too, like so?
> 
> -- >8 --
> 
> Subject: [PATCH] c++: naming a dependently-scoped template for CTAD [PR104641]
> 
> In order to be able to perform CTAD for a dependently-scoped template
> (such as A<T>::B in the testcase below), we need to permit a
> typename-specifier to resolve to a template as per [dcl.type.simple]/3,
> at least when it appears in a CTAD-enabled context.
> 
> This patch implements this using a new tsubst flag tf_tst_ok to control
> when a TYPENAME_TYPE is allowed to name a template, and sets this flag
> when substituting into the type of a CAST_EXPR, CONSTRUCTOR or VAR_DECL
> (each of which is a CTAD-enabled context).
> 
> 	PR c++/104641
> 
> gcc/cp/ChangeLog:
> 
> 	* cp-tree.h (tsubst_flags::tf_tst_ok): New flag.
> 	* decl.cc (make_typename_type): Allow a typename-specifier to
> 	resolve to a template when tf_tst_ok, in which case return
> 	a CTAD placeholder for the template.
> 	* pt.cc (tsubst_decl) <case VAR_DECL>: Set tf_tst_ok when
> 	substituting the type.
> 	(tsubst): Clear tf_tst_ok and remember if it was set.
> 	<case TYPENAME_TYPE>: Pass tf_tst_ok to make_typename_type
> 	appropriately.
> 	(tsubst_copy) <case CAST_EXPR>: Set tf_tst_ok when substituting
> 	the type.
> 	(tsubst_copy_and_build) <case CAST_EXPR>: Likewise.
> 	<case CONSTRUCTOR>: Likewise.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp1z/class-deduction107.C: New test.
> ---
>   gcc/cp/cp-tree.h                              |  2 ++
>   gcc/cp/decl.cc                                | 20 ++++++++++---
>   gcc/cp/pt.cc                                  | 29 +++++++++++++++----
>   .../g++.dg/cpp1z/class-deduction107.C         | 20 +++++++++++++
>   4 files changed, 61 insertions(+), 10 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp1z/class-deduction107.C
> 
> diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
> index b71bce1ab97..b7606f22287 100644
> --- a/gcc/cp/cp-tree.h
> +++ b/gcc/cp/cp-tree.h
> @@ -5557,6 +5557,8 @@ enum tsubst_flags {
>   				(build_target_expr and friends) */
>     tf_norm = 1 << 11,		 /* Build diagnostic information during
>   				    constraint normalization.  */
> +  tf_tst_ok = 1 << 12,		 /* Allow a typename-specifier to name
> +				    a template.  */
>     /* Convenient substitution flags combinations.  */
>     tf_warning_or_error = tf_warning | tf_error
>   };
> diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
> index 992e38385c2..d2d46915068 100644
> --- a/gcc/cp/decl.cc
> +++ b/gcc/cp/decl.cc
> @@ -4204,16 +4204,28 @@ make_typename_type (tree context, tree name, enum tag_types tag_type,
>       }
>     if (!want_template && TREE_CODE (t) != TYPE_DECL)
>       {
> -      if (complain & tf_error)
> -	error ("%<typename %T::%D%> names %q#T, which is not a type",
> -	       context, name, t);
> -      return error_mark_node;
> +      if ((complain & tf_tst_ok) && DECL_TYPE_TEMPLATE_P (t))
> +	/* The caller permits this typename-specifier to name a template
> +	   (because it appears in a CTAD-enabled context).  */;
> +      else
> +	{
> +	  if (complain & tf_error)
> +	    error ("%<typename %T::%D%> names %q#T, which is not a type",
> +		   context, name, t);
> +	  return error_mark_node;
> +	}
>       }
>   
>     if (!check_accessibility_of_qualified_id (t, /*object_type=*/NULL_TREE,
>   					    context, complain))
>       return error_mark_node;
>   
> +  if (!want_template && DECL_TYPE_TEMPLATE_P (t))
> +    {
> +      gcc_assert (complain & tf_tst_ok);

I'm not sure we need this assert so close to the test above, but OK 
either way.

> +      return make_template_placeholder (t);
> +    }
> +
>     if (want_template)
>       {
>         t = lookup_template_class (t, TREE_OPERAND (fullname, 1),
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index f7ee33a6dfd..f890d92d715 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -14896,7 +14896,10 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain)
>   		&& VAR_HAD_UNKNOWN_BOUND (t)
>   		&& type != error_mark_node)
>   	      type = strip_array_domain (type);
> -	    type = tsubst (type, args, complain, in_decl);
> +	    tsubst_flags_t tcomplain = complain;
> +	    if (VAR_P (t))
> +	      tcomplain |= tf_tst_ok;
> +	    type = tsubst (type, args, tcomplain, in_decl);
>   	    /* Substituting the type might have recursively instantiated this
>   	       same alias (c++/86171).  */
>   	    if (gen_tmpl && DECL_ALIAS_TEMPLATE_P (gen_tmpl)
> @@ -15582,6 +15585,9 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>     bool fndecl_type = (complain & tf_fndecl_type);
>     complain &= ~tf_fndecl_type;
>   
> +  bool tst_ok = (complain & tf_tst_ok);
> +  complain &= ~tf_tst_ok;
> +
>     if (type
>         && code != TYPENAME_TYPE
>         && code != TEMPLATE_TYPE_PARM
> @@ -16169,8 +16175,10 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>   	      return error_mark_node;
>   	  }
>   
> -	f = make_typename_type (ctx, f, typename_type,
> -				complain | tf_keep_type_decl);
> +	tsubst_flags_t tcomplain = complain | tf_keep_type_decl;
> +	if (tst_ok)
> +	  tcomplain |= tf_tst_ok;
> +	f = make_typename_type (ctx, f, typename_type, tcomplain);
>   	if (f == error_mark_node)
>   	  return f;
>   	if (TREE_CODE (f) == TYPE_DECL)
> @@ -17077,7 +17085,10 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl)
>       case CONVERT_EXPR:
>       case NOP_EXPR:
>         {
> -	tree type = tsubst (TREE_TYPE (t), args, complain, in_decl);
> +	tsubst_flags_t tcomplain = complain;
> +	if (code == CAST_EXPR)
> +	  tcomplain |= tf_tst_ok;
> +	tree type = tsubst (TREE_TYPE (t), args, tcomplain, in_decl);
>   	tree op0 = tsubst_copy (TREE_OPERAND (t, 0), args, complain, in_decl);
>   	return build1 (code, type, op0);
>         }
> @@ -20100,7 +20111,10 @@ tsubst_copy_and_build (tree t,
>   	tree type;
>   	tree op, r = NULL_TREE;
>   
> -	type = tsubst (TREE_TYPE (t), args, complain, in_decl);
> +	tsubst_flags_t tcomplain = complain;
> +	if (TREE_CODE (t) == CAST_EXPR)
> +	  tcomplain |= tf_tst_ok;
> +	type = tsubst (TREE_TYPE (t), args, tcomplain, in_decl);
>   	if (integral_constant_expression_p
>   	    && !cast_valid_in_integral_constant_expression_p (type))
>   	  {
> @@ -21060,12 +21074,15 @@ tsubst_copy_and_build (tree t,
>   	vec<constructor_elt, va_gc> *n;
>   	constructor_elt *ce;
>   	unsigned HOST_WIDE_INT idx;
> -	tree type = tsubst (TREE_TYPE (t), args, complain, in_decl);
>   	bool process_index_p;
>           int newlen;
>           bool need_copy_p = false;
>   	tree r;
>   
> +	tsubst_flags_t tcomplain = complain;
> +	if (COMPOUND_LITERAL_P (t))
> +	  tcomplain |= tf_tst_ok;
> +	tree type = tsubst (TREE_TYPE (t), args, tcomplain, in_decl);
>   	if (type == error_mark_node)
>   	  RETURN (error_mark_node);
>   
> diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction107.C b/gcc/testsuite/g++.dg/cpp1z/class-deduction107.C
> new file mode 100644
> index 00000000000..462da1dcdc2
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction107.C
> @@ -0,0 +1,20 @@
> +// PR c++/104641
> +// { dg-do compile { target c++17 } }
> +
> +template<class T>
> +struct A {
> +  template<class U> struct B { B(U); };
> +private:
> +  template<class U> struct C { C(U); };
> +};
> +
> +template<class T>
> +void f() {
> +  typename A<T>::B x = 0;
> +  auto y = typename A<T>::B(0);
> +  auto z = typename A<T>::B{0};
> +
> +  typename A<T>::C w(0); // { dg-error "private" }
> +}
> +
> +template void f<void>();



More information about the Gcc-patches mailing list