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

[Jason Merrill <jason@redhat.com>]

On 9/18/23 08:58, Patrick Palka wrote:
> On Sun, 17 Sep 2023, Jason Merrill wrote:
> 
>> 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?
> 
> The xfailed dg-bogus directive seems to "consume" the two errors, and
> prevents the dg-error from matching either of them.  Before this patch,
> we issued three duplicate errors and so this arrangement worked out
> since the xfailed dg-bogus consumes only two of the errors.  I'm not
> sure how to express that we're currently seeing two errors but only want
> one using _both_ dg-error and dg-bogus..

Would it work to have a dg-error to consume one error followed by a 
dg-bogus for another single error?

If not, just add a comment.  OK either way.

Jason