Re: [PATCH] c++: optimize tsubst_template_decl for function templates

[Jason Merrill <jason@redhat.com>]

On 9/17/23 15:13, Patrick Palka wrote:
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
> 
> -- >8 --
> 
> r14-2655-g92d1425ca78040 made instantiate_template avoid redundantly
> performing a specialization lookup when instantiating a function or
> alias template.  This patch applies the same optimization to
> tsubst_template_decl when (partially) instantiating a function template,
> which allows us to remove a check from register_specialization since
> tsubst_function_decl no longer calls register_specialization for
> a function template partial instantiation.
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.cc (register_specialization): Remove now-unnecessary
> 	early exit for FUNCTION_DECL partial instantiation.
> 	(tsubst_template_decl): Pass use_spec_table=false to
> 	tsubst_function_decl.  Set DECL_TI_ARGS of a non-lambda
> 	FUNCTION_DECL specialization to the full set of arguments.
> 	Simplify register_specialization call accordingly.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/template/nontype12.C: Expect two instead of three
> 	duplicate diagnostics for A<double>::bar() specialization.
> ---
>   gcc/cp/pt.cc                              | 29 +++++++----------------
>   gcc/testsuite/g++.dg/template/nontype12.C |  1 -
>   2 files changed, 9 insertions(+), 21 deletions(-)
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index c311a6b88f5..a0296a1ea16 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -1507,21 +1507,6 @@ register_specialization (tree spec, tree tmpl, tree args, bool is_friend,
>   	      || (TREE_CODE (tmpl) == FIELD_DECL
>   		  && TREE_CODE (spec) == NONTYPE_ARGUMENT_PACK));
>   
> -  if (TREE_CODE (spec) == FUNCTION_DECL
> -      && uses_template_parms (DECL_TI_ARGS (spec)))
> -    /* This is the FUNCTION_DECL for a partial instantiation.  Don't
> -       register it; we want the corresponding TEMPLATE_DECL instead.
> -       We use `uses_template_parms (DECL_TI_ARGS (spec))' rather than
> -       the more obvious `uses_template_parms (spec)' to avoid problems
> -       with default function arguments.  In particular, given
> -       something like this:
> -
> -	  template <class T> void f(T t1, T t = T())
> -
> -       the default argument expression is not substituted for in an
> -       instantiation unless and until it is actually needed.  */
> -    return spec;
> -
>     spec_entry elt;
>     elt.tmpl = tmpl;
>     elt.args = args;
> @@ -14663,7 +14648,7 @@ tsubst_template_decl (tree t, tree args, tsubst_flags_t complain,
>     tree in_decl = t;
>     tree spec;
>     tree tmpl_args;
> -  tree full_args;
> +  tree full_args = NULL_TREE;
>     tree r;
>     hashval_t hash = 0;
>   
> @@ -14754,7 +14739,8 @@ tsubst_template_decl (tree t, tree args, tsubst_flags_t complain,
>     tree inner = decl;
>     ++processing_template_decl;
>     if (TREE_CODE (inner) == FUNCTION_DECL)
> -    inner = tsubst_function_decl (inner, args, complain, lambda_fntype);
> +    inner = tsubst_function_decl (inner, args, complain, lambda_fntype,
> +				  /*use_spec_table=*/false);
>     else
>       {
>         if (TREE_CODE (inner) == TYPE_DECL && !TYPE_DECL_ALIAS_P (inner))
> @@ -14792,6 +14778,11 @@ tsubst_template_decl (tree t, tree args, tsubst_flags_t complain,
>       }
>     else
>       {
> +      if (TREE_CODE (inner) == FUNCTION_DECL)
> +	/* Set DECL_TI_ARGS to the full set of template arguments, which
> +	   tsubst_function_decl didn't do due to use_spec_table=false.  */
> +	DECL_TI_ARGS (inner) = full_args;
> +
>         DECL_TI_TEMPLATE (inner) = r;
>         DECL_TI_ARGS (r) = DECL_TI_ARGS (inner);
>       }
> @@ -14822,9 +14813,7 @@ tsubst_template_decl (tree t, tree args, tsubst_flags_t complain,
>   
>     if (TREE_CODE (decl) == FUNCTION_DECL && !lambda_fntype)
>       /* Record this non-type partial instantiation.  */
> -    register_specialization (r, t,
> -			     DECL_TI_ARGS (DECL_TEMPLATE_RESULT (r)),
> -			     false, hash);
> +    register_specialization (r, t, full_args, false, hash);
>   
>     return r;
>   }
> diff --git a/gcc/testsuite/g++.dg/template/nontype12.C b/gcc/testsuite/g++.dg/template/nontype12.C
> index 9a9c3ac1e66..e36a9f16f94 100644
> --- a/gcc/testsuite/g++.dg/template/nontype12.C
> +++ b/gcc/testsuite/g++.dg/template/nontype12.C
> @@ -5,7 +5,6 @@ template<typename T> struct A
>   {
>     template<T> int foo();                        // { dg-error "double" "" { target c++17_down } }
>     template<template<T> class> int bar();        // { dg-bogus {double[^\n]*\n[^\n]*C:7:[^\n]*double} "" { xfail c++17_down } }
> -  // { dg-error "double" "" { target c++17_down } .-1 }

Hmm, I thought this line was to check that we get one error even if we 
don't want two?

Jason