Re: [PATCH] c++: reference variable as default targ [PR101463]

[Patrick Palka <ppalka@redhat.com>]

On Tue, 22 Oct 2024, Patrick Palka wrote:

> On Tue, 9 Jan 2024, Jason Merrill wrote:
> 
> > On 1/5/24 15:01, Patrick Palka wrote:
> > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this
> > > look OK for trunk?
> > > 
> > > -- >8 --
> > > 
> > > Here during default template argument substitution we wrongly consider
> > > the (substituted) default arguments v and vt<int> as value-dependent[1]
> > > which ultimately leads to deduction failure for the calls.
> > > 
> > > The bogus value_dependent_expression_p result aside, I noticed
> > > type_unification_real during default targ substitution keeps track of
> > > whether all previous targs are known and non-dependent, as is the case
> > > for these calls.  And in such cases it should be safe to avoid checking
> > > dependence of the substituted default targ and just assume it's not.
> > > This patch implements this optimization, which lets us accept both
> > > testcases by sidestepping the value_dependent_expression_p issue
> > > altogether.
> > 
> > Hmm, maybe instead of substituting and asking if it's dependent, we should
> > specifically look for undeduced parameters.
> 
> We took this approach with r15-3038-g5348e3cb9bc99d for GCC 15, but it
> might be too risky to backport.  The original approach of avoiding the
> dependence check when we know all previous arguments are non-dependent
> seems safer, and we can simplify it a bit even.
> 
> What do you think about backporting the following in order to fix 101463
> on the release branches?  Bootstrappped and regtested on
> x86_64-pc-linux-gnu on the 14 branch.

Ping.

> 
> -- >8 --
> 
> Subject: [PATCH] c++: reference variable as default targ [PR101463]
> 
> Here during default template argument substitution we wrongly consider
> the (substituted) default arguments v and vt<int> as value-dependent[1]
> which ultimately leads to deduction failure for the calls.
> 
> The bogus value_dependent_expression_p result aside, I noticed
> type_unification_real during default targ substitution keeps track of
> whether all previous targs are known and non-dependent, as is the case
> for these calls.  And in such cases it should be safe to avoid checking
> dependence of the substituted default targ and just assume it's not.
> This patch implements this optimization, which lets us accept both
> testcases by sidestepping the value_dependent_expression_p issue
> altogether.
> 
> Note that for GCC 15 we fixed this differently, see
> r15-3038-g5348e3cb9bc99d.
> 
> 	PR c++/101463
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.cc (type_unification_real): Avoid checking dependence of
> 	a substituted default template argument if we can assume it's
> 	non-dependent.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp1z/nontype6.C: New test.
> 	* g++.dg/cpp1z/nontype6a.C: New test.
> ---
>  gcc/cp/pt.cc                           |  4 +++-
>  gcc/testsuite/g++.dg/cpp1z/nontype6.C  | 24 ++++++++++++++++++++++++
>  gcc/testsuite/g++.dg/cpp1z/nontype6a.C | 25 +++++++++++++++++++++++++
>  3 files changed, 52 insertions(+), 1 deletion(-)
>  create mode 100644 gcc/testsuite/g++.dg/cpp1z/nontype6.C
>  create mode 100644 gcc/testsuite/g++.dg/cpp1z/nontype6a.C
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 818d5b65edc..4f83426fb56 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -23651,12 +23651,14 @@ type_unification_real (tree tparms,
>  		    /* We replaced all the tparms, substitute again out of
>  		       template context.  */
>  		    substed = NULL_TREE;
> +		  else
> +		    processing_template_decl = 1;
>  		}
>  	      if (!substed)
>  		substed = tsubst_template_arg (arg, full_targs, complain,
>  					       NULL_TREE);
>  
> -	      if (!uses_template_parms (substed))
> +	      if (!processing_template_decl || !uses_template_parms (substed))
>  		arg = convert_template_argument (parm, substed, full_targs,
>  						 complain, i, NULL_TREE);
>  	      else if (saw_undeduced == 1)
> diff --git a/gcc/testsuite/g++.dg/cpp1z/nontype6.C b/gcc/testsuite/g++.dg/cpp1z/nontype6.C
> new file mode 100644
> index 00000000000..06cd234cc61
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1z/nontype6.C
> @@ -0,0 +1,24 @@
> +// PR c++/101463
> +// { dg-do compile { target c++17 } }
> +
> +int a;
> +
> +int& v = a;
> +
> +template<const int& = v>
> +void f(int) { }
> +
> +template<class T, int& = v>
> +void g(T) { }
> +
> +template<class T>
> +int& vt = a;
> +
> +template<class T, int& = vt<T>>
> +void h(T) { }
> +
> +int main() {
> +  f(0);
> +  g(0);
> +  h(0);
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp1z/nontype6a.C b/gcc/testsuite/g++.dg/cpp1z/nontype6a.C
> new file mode 100644
> index 00000000000..8bc40a0505c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1z/nontype6a.C
> @@ -0,0 +1,25 @@
> +// PR c++/101463
> +// A version of nontype6.C where v and vt are constexpr.
> +// { dg-do compile { target c++17 } }
> +
> +int a;
> +
> +constexpr int& v = a;
> +
> +template<const int& = v>
> +void f(int) { }
> +
> +template<class T, const int& = v>
> +void g(T) { }
> +
> +template<class T>
> +constexpr int& vt = a;
> +
> +template<class T, const int& = vt<T>>
> +void h(T) { }
> +
> +int main() {
> +  f(0);
> +  g(0);
> +  h(0);
> +}
> -- 
> 2.47.0.107.g34b6ce9b30
> 
>