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

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

[Patrick Palka]

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.

[1]: The reason we consider these reference variables value-dependent is
due to a workaround in value_dependent_expression_p:

  case VAR_DECL:
    ...
    else if (TYPE_REF_P (TREE_TYPE (expression)))
      /* FIXME cp_finish_decl doesn't fold reference initializers.  */
      return true;
    ...

added by r5-5022-g51d72abe5ea04e.  I'm not sure if this workaround
is needed anymore, but naively removing it seems safe as far as
bootstrap+regtest is concerned (the only change is that we issue more
-Wmissing-braces warnings ahead of time in cpp0x/initlist123.C), and
lets us accept the first testcase.

Unfortunately we still reject the second testcase (in which v and vt are
additionally constexpr) for the same reason (bogus value dependence) due
to the subsequent check in v_d_e_p:

      ...
      /* We have a constexpr variable and we're processing a template.  When
         there's lifetime extension involved (for which finish_compound_literal
         used to create a temporary), we'll not be able to evaluate the
         variable until instantiating, so pretend it's value-dependent.  */
      else if (DECL_DECLARED_CONSTEXPR_P (expression)
               && !TREE_CONSTANT (expression))
        return true;

And TREE_CONSTANT isn't set for v and vt<int> because of a workaround in
cp_finish_decl:

          if (decl_maybe_constant_var_p (decl)
              /* FIXME setting TREE_CONSTANT on refs breaks the back end.  */
              && !TYPE_REF_P (type))
            TREE_CONSTANT (decl) = true;

Naively removing this workaround lets us accept the second testcase, but
it re-introduces an ICE in g++.dg/opt/pr78373.C.

	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                           |  9 +++++++--
 gcc/testsuite/g++.dg/cpp1z/nontype6.C  | 24 ++++++++++++++++++++++++
 gcc/testsuite/g++.dg/cpp1z/nontype6a.C | 25 +++++++++++++++++++++++++
 3 files changed, 56 insertions(+), 2 deletions(-)
 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 7208c721b0b..b801ce1f18c 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -23304,6 +23304,7 @@ type_unification_real (tree tparms,
 		 might be instantiation-dependent like access (87480).  */
 	      processing_template_decl_sentinel s (!any_dependent_targs);
 	      tree substed = NULL_TREE;
+	      tristate dependent_p = tristate::unknown ();
 	      if (saw_undeduced == 1 && !any_dependent_targs)
 		{
 		  /* First instatiate in template context, in case we still
@@ -23312,8 +23313,9 @@ type_unification_real (tree tparms,
 		  substed = tsubst_template_arg (arg, full_targs, complain,
 						 NULL_TREE);
 		  --processing_template_decl;
+		  dependent_p = uses_template_parms (substed);
 		  if (substed != error_mark_node
-		      && !uses_template_parms (substed))
+		      && dependent_p.is_false ())
 		    /* We replaced all the tparms, substitute again out of
 		       template context.  */
 		    substed = NULL_TREE;
@@ -23321,8 +23323,11 @@ type_unification_real (tree tparms,
 	      if (!substed)
 		substed = tsubst_template_arg (arg, full_targs, complain,
 					       NULL_TREE);
+	      if (dependent_p.is_unknown ())
+		dependent_p = (processing_template_decl
+			       && uses_template_parms (substed));
 
-	      if (!uses_template_parms (substed))
+	      if (dependent_p.is_false ())
 		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.43.0.254.ga26002b628

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

[Jason Merrill]

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.

Jason

[PATCH] c++: default targ eligibility refinement [PR101463]

[Patrick Palka]

> > 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.

Makes sense, like so?  Bootstrapped and regtested on x86_64-pc-linux-gnu.

	PR c++/101463

gcc/cp/ChangeLog:

	* pt.cc (type_unification_real): Directly look for undeduced
	parameters in the default argument instead of substituting
	and asking if it's dependent.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp1z/nontype6.C: New test.
	* g++.dg/cpp1z/nontype6a.C: New test.
---
 gcc/cp/pt.cc                           | 41 ++++++++++++++------------
 gcc/testsuite/g++.dg/cpp1z/nontype6.C  | 24 +++++++++++++++
 gcc/testsuite/g++.dg/cpp1z/nontype6a.C | 25 ++++++++++++++++
 3 files changed, 71 insertions(+), 19 deletions(-)
 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 8725a5eeb3f..ad0f73c2f43 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -23607,28 +23607,31 @@ type_unification_real (tree tparms,
 		 is important if the default argument contains something that
 		 might be instantiation-dependent like access (87480).  */
 	      processing_template_decl_sentinel s (!any_dependent_targs);
-	      tree substed = NULL_TREE;
-	      if (saw_undeduced == 1 && !any_dependent_targs)
+
+	      tree used_tparms = NULL_TREE;
+	      if (saw_undeduced == 1)
 		{
-		  /* First instatiate in template context, in case we still
-		     depend on undeduced template parameters.  */
-		  ++processing_template_decl;
-		  substed = tsubst_template_arg (arg, full_targs, complain,
-						 NULL_TREE);
-		  --processing_template_decl;
-		  if (substed != error_mark_node
-		      && !uses_template_parms (substed))
-		    /* We replaced all the tparms, substitute again out of
-		       template context.  */
-		    substed = NULL_TREE;
+		  tree tparms_list = build_tree_list (size_int (1), tparms);
+		  used_tparms = find_template_parameters (arg, tparms_list);
+		  for (; used_tparms; used_tparms = TREE_CHAIN (used_tparms))
+		    {
+		      int level, index;
+		      template_parm_level_and_index (TREE_VALUE (used_tparms),
+						     &level, &index);
+		      if (TREE_VEC_ELT (targs, index) == NULL_TREE)
+			break;
+		    }
 		}
-	      if (!substed)
-		substed = tsubst_template_arg (arg, full_targs, complain,
-					       NULL_TREE);
 
-	      if (!uses_template_parms (substed))
-		arg = convert_template_argument (parm, substed, full_targs,
-						 complain, i, NULL_TREE);
+	      if (!used_tparms)
+		{
+		  /* All template parameters used within this default argument
+		     are deduced, so we can use it.  */
+		  arg = tsubst_template_arg (arg, full_targs, complain,
+					     NULL_TREE);
+		  arg = convert_template_argument (parm, arg, full_targs,
+						   complain, i, NULL_TREE);
+		}
 	      else if (saw_undeduced == 1)
 		arg = NULL_TREE;
 	      else if (!any_dependent_targs)
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.46.0.164.g477ce5ccd6

Re: [PATCH] c++: default targ eligibility refinement [PR101463]

[Jason Merrill]

On 8/16/24 12:00 PM, Patrick Palka wrote:
>>> 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.
> 
> Makes sense, like so?  Bootstrapped and regtested on x86_64-pc-linux-gnu.

OK.

> 	PR c++/101463
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.cc (type_unification_real): Directly look for undeduced
> 	parameters in the default argument instead of substituting
> 	and asking if it's dependent.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp1z/nontype6.C: New test.
> 	* g++.dg/cpp1z/nontype6a.C: New test.
> ---
>   gcc/cp/pt.cc                           | 41 ++++++++++++++------------
>   gcc/testsuite/g++.dg/cpp1z/nontype6.C  | 24 +++++++++++++++
>   gcc/testsuite/g++.dg/cpp1z/nontype6a.C | 25 ++++++++++++++++
>   3 files changed, 71 insertions(+), 19 deletions(-)
>   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 8725a5eeb3f..ad0f73c2f43 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -23607,28 +23607,31 @@ type_unification_real (tree tparms,
>   		 is important if the default argument contains something that
>   		 might be instantiation-dependent like access (87480).  */
>   	      processing_template_decl_sentinel s (!any_dependent_targs);
> -	      tree substed = NULL_TREE;
> -	      if (saw_undeduced == 1 && !any_dependent_targs)
> +
> +	      tree used_tparms = NULL_TREE;
> +	      if (saw_undeduced == 1)
>   		{
> -		  /* First instatiate in template context, in case we still
> -		     depend on undeduced template parameters.  */
> -		  ++processing_template_decl;
> -		  substed = tsubst_template_arg (arg, full_targs, complain,
> -						 NULL_TREE);
> -		  --processing_template_decl;
> -		  if (substed != error_mark_node
> -		      && !uses_template_parms (substed))
> -		    /* We replaced all the tparms, substitute again out of
> -		       template context.  */
> -		    substed = NULL_TREE;
> +		  tree tparms_list = build_tree_list (size_int (1), tparms);
> +		  used_tparms = find_template_parameters (arg, tparms_list);
> +		  for (; used_tparms; used_tparms = TREE_CHAIN (used_tparms))
> +		    {
> +		      int level, index;
> +		      template_parm_level_and_index (TREE_VALUE (used_tparms),
> +						     &level, &index);
> +		      if (TREE_VEC_ELT (targs, index) == NULL_TREE)
> +			break;
> +		    }
>   		}
> -	      if (!substed)
> -		substed = tsubst_template_arg (arg, full_targs, complain,
> -					       NULL_TREE);
>   
> -	      if (!uses_template_parms (substed))
> -		arg = convert_template_argument (parm, substed, full_targs,
> -						 complain, i, NULL_TREE);
> +	      if (!used_tparms)
> +		{
> +		  /* All template parameters used within this default argument
> +		     are deduced, so we can use it.  */
> +		  arg = tsubst_template_arg (arg, full_targs, complain,
> +					     NULL_TREE);
> +		  arg = convert_template_argument (parm, arg, full_targs,
> +						   complain, i, NULL_TREE);
> +		}
>   	      else if (saw_undeduced == 1)
>   		arg = NULL_TREE;
>   	      else if (!any_dependent_targs)
> 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);
> +}

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

[Patrick Palka]

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.

-- >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

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

[Patrick Palka]

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
> 
> 

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

[Jason Merrill]

On 10/22/24 1:54 PM, 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.

OK.

> -- >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);
> +}