[PATCH] c++: variable template and targ deduction [PR108550]

[PATCH] c++: variable template and targ deduction [PR108550]

[Marek Polacek]

In this test, we get a bogus error because we failed to deduce the auto in
constexpr auto is_pointer_v = is_pointer<Tp>::value;
to bool.  Then ensure_literal_type_for_constexpr_object thinks the object
isn't literal and an error is reported.

This is another case of the interaction between tf_partial and 'auto',
where the auto was not reduced so the deduction failed.  In more detail:
we have

  Wrap1<int>()

in the code and we need to perform OR -> fn_type_unification.  The targ
list is incomplete, so we do
      tsubst_flags_t ecomplain = complain | tf_partial | tf_fndecl_type;
      fntype = tsubst (TREE_TYPE (fn), explicit_targs, ecomplain, NULL_TREE);
where TREE_TYPE (fn) is struct integral_constant <T402> (void).  Then
we substitute the return type, which results in tsubsting is_pointer_v<int>.
is_pointer_v is a variable template with a placeholder type:

  template <class Tp>
  constexpr auto is_pointer_v = is_pointer<Tp>::value;

so we find ourselves in lookup_and_finish_template_variable.  tf_partial is
still set, so finish_template_variable -> instantiate_template -> tsubst
won't reduce the level of auto.  But then we do mark_used which eventually
calls do_auto_deduction which clears tf_partial, because we want to replace
the auto now.  But we hadn't reduced auto's level so this fails.  And
since we're not in an immediate context, we emit a hard error.

I suppose that when we reach lookup_and_finish_template_variable it's
probably time to clear tf_partial.  (I added an assert and our testsuite
doesn't have a test whereby we get to lookup_and_finish_template_variable
while tf_partial is still active.)

Does this make *any* sense to you?

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

	PR c++/108550

gcc/cp/ChangeLog:

	* pt.cc (lookup_and_finish_template_variable): Clear tf_partial.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp1y/var-templ70.C: New test.
	* g++.dg/cpp1y/var-templ71.C: New test.
---
 gcc/cp/pt.cc                             |  6 ++++++
 gcc/testsuite/g++.dg/cpp1y/var-templ70.C | 25 +++++++++++++++++++++++
 gcc/testsuite/g++.dg/cpp1y/var-templ71.C | 26 ++++++++++++++++++++++++
 3 files changed, 57 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ70.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ71.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 1a071e95004..f636bac5413 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -10355,6 +10355,12 @@ lookup_and_finish_template_variable (tree templ, tree targs,
   if (TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (templ)) == 1
       && !any_dependent_template_arguments_p (targs))
     {
+      /* We may be called while doing a partial substitution, but the
+	 type of the variable template may be auto, in which case we
+	 will call do_auto_deduction in mark_used (which clears tf_partial)
+	 and the auto must be properly reduced at that time for the
+	 deduction to work.  */
+      complain &= ~tf_partial;
       var = finish_template_variable (var, complain);
       mark_used (var);
     }
diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ70.C b/gcc/testsuite/g++.dg/cpp1y/var-templ70.C
new file mode 100644
index 00000000000..1d35c38c7cc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/var-templ70.C
@@ -0,0 +1,25 @@
+// PR c++/108550
+// { dg-do compile { target c++14 } }
+
+template<class T>
+struct is_pointer
+{
+  static constexpr bool value = false;
+};
+
+template<class T, T T1>
+struct integral_constant
+{
+  static constexpr T value = T1;
+};
+
+
+template <class Tp>
+constexpr auto is_pointer_v = is_pointer<Tp>::value;
+
+template <class Tp, int = 0>
+integral_constant<bool, is_pointer_v<int>> Wrap1();
+
+int main() {
+  static_assert(!decltype(Wrap1<int>())::value, "");
+}
diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ71.C b/gcc/testsuite/g++.dg/cpp1y/var-templ71.C
new file mode 100644
index 00000000000..e0cf55230d9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/var-templ71.C
@@ -0,0 +1,26 @@
+// PR c++/108550
+// { dg-do compile { target c++14 } }
+
+template<class T, T T1>
+struct integral_constant
+{
+  static constexpr T value = T1;
+};
+
+template <typename T>
+struct S {
+  template <typename U, typename V>
+  static constexpr void foo(V) { }
+
+  constexpr bool bar () const { foo<int>(10); return false; }
+};
+
+template <class Tp>
+constexpr auto is_pointer_v = S<Tp>{}.bar();
+
+template <class Tp, int = 0>
+integral_constant<bool, is_pointer_v<int>> Wrap1();
+
+int main() {
+  static_assert(!decltype(Wrap1<int>())::value, "");
+}

base-commit: 1370014f2ea02ec185cf1199027575916f79fe63
-- 
2.39.2

Re: [PATCH] c++: variable template and targ deduction [PR108550]

[Patrick Palka]

On Wed, 22 Feb 2023, Marek Polacek wrote:

> In this test, we get a bogus error because we failed to deduce the auto in
> constexpr auto is_pointer_v = is_pointer<Tp>::value;
> to bool.  Then ensure_literal_type_for_constexpr_object thinks the object
> isn't literal and an error is reported.
> 
> This is another case of the interaction between tf_partial and 'auto',
> where the auto was not reduced so the deduction failed.  In more detail:
> we have
> 
>   Wrap1<int>()
> 
> in the code and we need to perform OR -> fn_type_unification.  The targ
> list is incomplete, so we do
>       tsubst_flags_t ecomplain = complain | tf_partial | tf_fndecl_type;
>       fntype = tsubst (TREE_TYPE (fn), explicit_targs, ecomplain, NULL_TREE);
> where TREE_TYPE (fn) is struct integral_constant <T402> (void).  Then
> we substitute the return type, which results in tsubsting is_pointer_v<int>.
> is_pointer_v is a variable template with a placeholder type:
> 
>   template <class Tp>
>   constexpr auto is_pointer_v = is_pointer<Tp>::value;
> 
> so we find ourselves in lookup_and_finish_template_variable.  tf_partial is
> still set, so finish_template_variable -> instantiate_template -> tsubst
> won't reduce the level of auto.  But then we do mark_used which eventually
> calls do_auto_deduction which clears tf_partial, because we want to replace
> the auto now.  But we hadn't reduced auto's level so this fails.  And
> since we're not in an immediate context, we emit a hard error.
> 
> I suppose that when we reach lookup_and_finish_template_variable it's
> probably time to clear tf_partial.  (I added an assert and our testsuite
> doesn't have a test whereby we get to lookup_and_finish_template_variable
> while tf_partial is still active.)
> 
> Does this make *any* sense to you?
> 
> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> 
> 	PR c++/108550
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.cc (lookup_and_finish_template_variable): Clear tf_partial.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp1y/var-templ70.C: New test.
> 	* g++.dg/cpp1y/var-templ71.C: New test.
> ---
>  gcc/cp/pt.cc                             |  6 ++++++
>  gcc/testsuite/g++.dg/cpp1y/var-templ70.C | 25 +++++++++++++++++++++++
>  gcc/testsuite/g++.dg/cpp1y/var-templ71.C | 26 ++++++++++++++++++++++++
>  3 files changed, 57 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ70.C
>  create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ71.C
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 1a071e95004..f636bac5413 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -10355,6 +10355,12 @@ lookup_and_finish_template_variable (tree templ, tree targs,
>    if (TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (templ)) == 1
>        && !any_dependent_template_arguments_p (targs))
>      {
> +      /* We may be called while doing a partial substitution, but the
> +	 type of the variable template may be auto, in which case we
> +	 will call do_auto_deduction in mark_used (which clears tf_partial)
> +	 and the auto must be properly reduced at that time for the
> +	 deduction to work.  */
> +      complain &= ~tf_partial;

LGTM.  I can't think of a reason to keep tf_partial set at this point
since we know there's only a single level of template arguments left to
substitute and the args we have are non-dependent, so the substitution
is in no way partial.

Though I wonder if ideally we should clear tf_partial more generally
from instantiate_template?  Modulo diagnostics, the result of
instantiate_template shouldn't depend on complain, in particular because
we cache the result in the specializations table is which keyed off of
only the given tmpl and args.  Not sure if that'd be a suitable change
for stage4/backports though...

>        var = finish_template_variable (var, complain);
>        mark_used (var);
>      }
> diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ70.C b/gcc/testsuite/g++.dg/cpp1y/var-templ70.C
> new file mode 100644
> index 00000000000..1d35c38c7cc
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/var-templ70.C
> @@ -0,0 +1,25 @@
> +// PR c++/108550
> +// { dg-do compile { target c++14 } }
> +
> +template<class T>
> +struct is_pointer
> +{
> +  static constexpr bool value = false;
> +};
> +
> +template<class T, T T1>
> +struct integral_constant
> +{
> +  static constexpr T value = T1;
> +};
> +
> +
> +template <class Tp>
> +constexpr auto is_pointer_v = is_pointer<Tp>::value;
> +
> +template <class Tp, int = 0>
> +integral_constant<bool, is_pointer_v<int>> Wrap1();

It might be good to have a version of this testcase that uses a
dependent is_pointer_v<Tp> here instead of is_pointer_v<int>.

> +
> +int main() {
> +  static_assert(!decltype(Wrap1<int>())::value, "");
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ71.C b/gcc/testsuite/g++.dg/cpp1y/var-templ71.C
> new file mode 100644
> index 00000000000..e0cf55230d9
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/var-templ71.C
> @@ -0,0 +1,26 @@
> +// PR c++/108550
> +// { dg-do compile { target c++14 } }
> +
> +template<class T, T T1>
> +struct integral_constant
> +{
> +  static constexpr T value = T1;
> +};
> +
> +template <typename T>
> +struct S {
> +  template <typename U, typename V>
> +  static constexpr void foo(V) { }
> +
> +  constexpr bool bar () const { foo<int>(10); return false; }
> +};
> +
> +template <class Tp>
> +constexpr auto is_pointer_v = S<Tp>{}.bar();
> +
> +template <class Tp, int = 0>
> +integral_constant<bool, is_pointer_v<int>> Wrap1();
> +
> +int main() {
> +  static_assert(!decltype(Wrap1<int>())::value, "");
> +}
> 
> base-commit: 1370014f2ea02ec185cf1199027575916f79fe63
> -- 
> 2.39.2
> 
> 

Re: [PATCH] c++: variable template and targ deduction [PR108550]

[Marek Polacek]

On Thu, Feb 23, 2023 at 10:17:22AM -0500, Patrick Palka wrote:
> On Wed, 22 Feb 2023, Marek Polacek wrote:
> 
> > In this test, we get a bogus error because we failed to deduce the auto in
> > constexpr auto is_pointer_v = is_pointer<Tp>::value;
> > to bool.  Then ensure_literal_type_for_constexpr_object thinks the object
> > isn't literal and an error is reported.
> > 
> > This is another case of the interaction between tf_partial and 'auto',
> > where the auto was not reduced so the deduction failed.  In more detail:
> > we have
> > 
> >   Wrap1<int>()
> > 
> > in the code and we need to perform OR -> fn_type_unification.  The targ
> > list is incomplete, so we do
> >       tsubst_flags_t ecomplain = complain | tf_partial | tf_fndecl_type;
> >       fntype = tsubst (TREE_TYPE (fn), explicit_targs, ecomplain, NULL_TREE);
> > where TREE_TYPE (fn) is struct integral_constant <T402> (void).  Then
> > we substitute the return type, which results in tsubsting is_pointer_v<int>.
> > is_pointer_v is a variable template with a placeholder type:
> > 
> >   template <class Tp>
> >   constexpr auto is_pointer_v = is_pointer<Tp>::value;
> > 
> > so we find ourselves in lookup_and_finish_template_variable.  tf_partial is
> > still set, so finish_template_variable -> instantiate_template -> tsubst
> > won't reduce the level of auto.  But then we do mark_used which eventually
> > calls do_auto_deduction which clears tf_partial, because we want to replace
> > the auto now.  But we hadn't reduced auto's level so this fails.  And
> > since we're not in an immediate context, we emit a hard error.
> > 
> > I suppose that when we reach lookup_and_finish_template_variable it's
> > probably time to clear tf_partial.  (I added an assert and our testsuite
> > doesn't have a test whereby we get to lookup_and_finish_template_variable
> > while tf_partial is still active.)
> > 
> > Does this make *any* sense to you?
> > 
> > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > 
> > 	PR c++/108550
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	* pt.cc (lookup_and_finish_template_variable): Clear tf_partial.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	* g++.dg/cpp1y/var-templ70.C: New test.
> > 	* g++.dg/cpp1y/var-templ71.C: New test.
> > ---
> >  gcc/cp/pt.cc                             |  6 ++++++
> >  gcc/testsuite/g++.dg/cpp1y/var-templ70.C | 25 +++++++++++++++++++++++
> >  gcc/testsuite/g++.dg/cpp1y/var-templ71.C | 26 ++++++++++++++++++++++++
> >  3 files changed, 57 insertions(+)
> >  create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ70.C
> >  create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ71.C
> > 
> > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > index 1a071e95004..f636bac5413 100644
> > --- a/gcc/cp/pt.cc
> > +++ b/gcc/cp/pt.cc
> > @@ -10355,6 +10355,12 @@ lookup_and_finish_template_variable (tree templ, tree targs,
> >    if (TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (templ)) == 1
> >        && !any_dependent_template_arguments_p (targs))
> >      {
> > +      /* We may be called while doing a partial substitution, but the
> > +	 type of the variable template may be auto, in which case we
> > +	 will call do_auto_deduction in mark_used (which clears tf_partial)
> > +	 and the auto must be properly reduced at that time for the
> > +	 deduction to work.  */
> > +      complain &= ~tf_partial;
> 
> LGTM.  I can't think of a reason to keep tf_partial set at this point
> since we know there's only a single level of template arguments left to
> substitute and the args we have are non-dependent, so the substitution
> is in no way partial.

Right, once we get to finish_template_variable I suppose we're really
committed to the var tmpl.  So the fix should be safe, albeit it feels
a bit ad hoc.  FWIW, I'd tested adding alias templates to the mix and
it still worked.
 
> Though I wonder if ideally we should clear tf_partial more generally
> from instantiate_template?  Modulo diagnostics, the result of
> instantiate_template shouldn't depend on complain, in particular because
> we cache the result in the specializations table is which keyed off of
> only the given tmpl and args.  Not sure if that'd be a suitable change
> for stage4/backports though...

This makes sense; I guess anytime you have the full set of targs and are
going to instantiate, tf_partial should be cleared otherwise it can
wreak havoc.  I'd be nervous about applying such a patch now but maybe
we could experiment with that in GCC 14.

Thanks for taking a look!
 
> >        var = finish_template_variable (var, complain);
> >        mark_used (var);
> >      }
> > diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ70.C b/gcc/testsuite/g++.dg/cpp1y/var-templ70.C
> > new file mode 100644
> > index 00000000000..1d35c38c7cc
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/cpp1y/var-templ70.C
> > @@ -0,0 +1,25 @@
> > +// PR c++/108550
> > +// { dg-do compile { target c++14 } }
> > +
> > +template<class T>
> > +struct is_pointer
> > +{
> > +  static constexpr bool value = false;
> > +};
> > +
> > +template<class T, T T1>
> > +struct integral_constant
> > +{
> > +  static constexpr T value = T1;
> > +};
> > +
> > +
> > +template <class Tp>
> > +constexpr auto is_pointer_v = is_pointer<Tp>::value;
> > +
> > +template <class Tp, int = 0>
> > +integral_constant<bool, is_pointer_v<int>> Wrap1();
> 
> It might be good to have a version of this testcase that uses a
> dependent is_pointer_v<Tp> here instead of is_pointer_v<int>.

True, added (and I threw an alias template in the mix too).

-- >8 --
In this test, we get a bogus error because we failed to deduce the auto in
constexpr auto is_pointer_v = is_pointer<Tp>::value;
to bool.  Then ensure_literal_type_for_constexpr_object thinks the object
isn't literal and an error is reported.

This is another case of the interaction between tf_partial and 'auto',
where the auto was not reduced so the deduction failed.  In more detail:
we have

  Wrap1<int>()

in the code and we need to perform OR -> fn_type_unification.  The targ
list is incomplete, so we do
      tsubst_flags_t ecomplain = complain | tf_partial | tf_fndecl_type;
      fntype = tsubst (TREE_TYPE (fn), explicit_targs, ecomplain, NULL_TREE);
where TREE_TYPE (fn) is struct integral_constant <T402> (void).  Then
we substitute the return type, which results in tsubsting is_pointer_v<int>.
is_pointer_v is a variable template with a placeholder type:

  template <class Tp>
  constexpr auto is_pointer_v = is_pointer<Tp>::value;

so we find ourselves in lookup_and_finish_template_variable.  tf_partial is
still set, so finish_template_variable -> instantiate_template -> tsubst
won't reduce the level of auto.  But then we do mark_used which eventually
calls do_auto_deduction which clears tf_partial, because we want to replace
the auto now.  But we hadn't reduced auto's level so this fails.  And
since we're not in an immediate context, we emit a hard error.

I suppose that when we reach lookup_and_finish_template_variable it's
probably time to clear tf_partial.  (I added an assert and our testsuite
doesn't have a test whereby we get to lookup_and_finish_template_variable
while tf_partial is still active.)

	PR c++/108550

gcc/cp/ChangeLog:

	* pt.cc (lookup_and_finish_template_variable): Clear tf_partial.

gcc/testsuite/ChangeLog:

	* g++.dg/cpp1y/var-templ70.C: New test.
	* g++.dg/cpp1y/var-templ71.C: New test.
	* g++.dg/cpp1y/var-templ72.C: New test.
---
 gcc/cp/pt.cc                             |  6 ++++++
 gcc/testsuite/g++.dg/cpp1y/var-templ70.C | 25 ++++++++++++++++++++++
 gcc/testsuite/g++.dg/cpp1y/var-templ71.C | 26 +++++++++++++++++++++++
 gcc/testsuite/g++.dg/cpp1y/var-templ72.C | 27 ++++++++++++++++++++++++
 4 files changed, 84 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ70.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ71.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ72.C

diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 1a071e95004..f636bac5413 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -10355,6 +10355,12 @@ lookup_and_finish_template_variable (tree templ, tree targs,
   if (TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (templ)) == 1
       && !any_dependent_template_arguments_p (targs))
     {
+      /* We may be called while doing a partial substitution, but the
+	 type of the variable template may be auto, in which case we
+	 will call do_auto_deduction in mark_used (which clears tf_partial)
+	 and the auto must be properly reduced at that time for the
+	 deduction to work.  */
+      complain &= ~tf_partial;
       var = finish_template_variable (var, complain);
       mark_used (var);
     }
diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ70.C b/gcc/testsuite/g++.dg/cpp1y/var-templ70.C
new file mode 100644
index 00000000000..1d35c38c7cc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/var-templ70.C
@@ -0,0 +1,25 @@
+// PR c++/108550
+// { dg-do compile { target c++14 } }
+
+template<class T>
+struct is_pointer
+{
+  static constexpr bool value = false;
+};
+
+template<class T, T T1>
+struct integral_constant
+{
+  static constexpr T value = T1;
+};
+
+
+template <class Tp>
+constexpr auto is_pointer_v = is_pointer<Tp>::value;
+
+template <class Tp, int = 0>
+integral_constant<bool, is_pointer_v<int>> Wrap1();
+
+int main() {
+  static_assert(!decltype(Wrap1<int>())::value, "");
+}
diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ71.C b/gcc/testsuite/g++.dg/cpp1y/var-templ71.C
new file mode 100644
index 00000000000..e0cf55230d9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/var-templ71.C
@@ -0,0 +1,26 @@
+// PR c++/108550
+// { dg-do compile { target c++14 } }
+
+template<class T, T T1>
+struct integral_constant
+{
+  static constexpr T value = T1;
+};
+
+template <typename T>
+struct S {
+  template <typename U, typename V>
+  static constexpr void foo(V) { }
+
+  constexpr bool bar () const { foo<int>(10); return false; }
+};
+
+template <class Tp>
+constexpr auto is_pointer_v = S<Tp>{}.bar();
+
+template <class Tp, int = 0>
+integral_constant<bool, is_pointer_v<int>> Wrap1();
+
+int main() {
+  static_assert(!decltype(Wrap1<int>())::value, "");
+}
diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ72.C b/gcc/testsuite/g++.dg/cpp1y/var-templ72.C
new file mode 100644
index 00000000000..7426bad4a6c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/var-templ72.C
@@ -0,0 +1,27 @@
+// PR c++/108550
+// { dg-do compile { target c++14 } }
+
+template<class T>
+struct is_pointer
+{
+  static constexpr bool value = false;
+};
+
+template<class T, T T1>
+struct integral_constant
+{
+  static constexpr T value = T1;
+};
+
+template<typename T>
+using PTR_P = is_pointer<T>;
+
+template <class Tp>
+constexpr auto is_pointer_v = PTR_P<Tp>::value;
+
+template <class Tp, int = 0>
+integral_constant<bool, is_pointer_v<Tp>> Wrap1();
+
+int main() {
+  static_assert(!decltype(Wrap1<int>())::value, "");
+}

base-commit: 426b0ae103894d1f1bd82e5f049ff8a53bd82a8d
-- 
2.39.2

Re: [PATCH] c++: variable template and targ deduction [PR108550]

[Jason Merrill]

On 2/23/23 10:54, Marek Polacek wrote:
> On Thu, Feb 23, 2023 at 10:17:22AM -0500, Patrick Palka wrote:
>> On Wed, 22 Feb 2023, Marek Polacek wrote:
>>
>>> In this test, we get a bogus error because we failed to deduce the auto in
>>> constexpr auto is_pointer_v = is_pointer<Tp>::value;
>>> to bool.  Then ensure_literal_type_for_constexpr_object thinks the object
>>> isn't literal and an error is reported.
>>>
>>> This is another case of the interaction between tf_partial and 'auto',
>>> where the auto was not reduced so the deduction failed.  In more detail:
>>> we have
>>>
>>>    Wrap1<int>()
>>>
>>> in the code and we need to perform OR -> fn_type_unification.  The targ
>>> list is incomplete, so we do
>>>        tsubst_flags_t ecomplain = complain | tf_partial | tf_fndecl_type;
>>>        fntype = tsubst (TREE_TYPE (fn), explicit_targs, ecomplain, NULL_TREE);
>>> where TREE_TYPE (fn) is struct integral_constant <T402> (void).  Then
>>> we substitute the return type, which results in tsubsting is_pointer_v<int>.
>>> is_pointer_v is a variable template with a placeholder type:
>>>
>>>    template <class Tp>
>>>    constexpr auto is_pointer_v = is_pointer<Tp>::value;
>>>
>>> so we find ourselves in lookup_and_finish_template_variable.  tf_partial is
>>> still set, so finish_template_variable -> instantiate_template -> tsubst
>>> won't reduce the level of auto.  But then we do mark_used which eventually
>>> calls do_auto_deduction which clears tf_partial, because we want to replace
>>> the auto now.  But we hadn't reduced auto's level so this fails.  And
>>> since we're not in an immediate context, we emit a hard error.
>>>
>>> I suppose that when we reach lookup_and_finish_template_variable it's
>>> probably time to clear tf_partial.  (I added an assert and our testsuite
>>> doesn't have a test whereby we get to lookup_and_finish_template_variable
>>> while tf_partial is still active.)
>>>
>>> Does this make *any* sense to you?
>>>
>>> Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
>>>
>>> 	PR c++/108550
>>>
>>> gcc/cp/ChangeLog:
>>>
>>> 	* pt.cc (lookup_and_finish_template_variable): Clear tf_partial.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	* g++.dg/cpp1y/var-templ70.C: New test.
>>> 	* g++.dg/cpp1y/var-templ71.C: New test.
>>> ---
>>>   gcc/cp/pt.cc                             |  6 ++++++
>>>   gcc/testsuite/g++.dg/cpp1y/var-templ70.C | 25 +++++++++++++++++++++++
>>>   gcc/testsuite/g++.dg/cpp1y/var-templ71.C | 26 ++++++++++++++++++++++++
>>>   3 files changed, 57 insertions(+)
>>>   create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ70.C
>>>   create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ71.C
>>>
>>> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
>>> index 1a071e95004..f636bac5413 100644
>>> --- a/gcc/cp/pt.cc
>>> +++ b/gcc/cp/pt.cc
>>> @@ -10355,6 +10355,12 @@ lookup_and_finish_template_variable (tree templ, tree targs,
>>>     if (TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (templ)) == 1
>>>         && !any_dependent_template_arguments_p (targs))
>>>       {
>>> +      /* We may be called while doing a partial substitution, but the
>>> +	 type of the variable template may be auto, in which case we
>>> +	 will call do_auto_deduction in mark_used (which clears tf_partial)
>>> +	 and the auto must be properly reduced at that time for the
>>> +	 deduction to work.  */
>>> +      complain &= ~tf_partial;
>>
>> LGTM.  I can't think of a reason to keep tf_partial set at this point
>> since we know there's only a single level of template arguments left to
>> substitute and the args we have are non-dependent, so the substitution
>> is in no way partial.
> 
> Right, once we get to finish_template_variable I suppose we're really
> committed to the var tmpl.  So the fix should be safe, albeit it feels
> a bit ad hoc.  FWIW, I'd tested adding alias templates to the mix and
> it still worked.
>   
>> Though I wonder if ideally we should clear tf_partial more generally
>> from instantiate_template?  Modulo diagnostics, the result of
>> instantiate_template shouldn't depend on complain, in particular because
>> we cache the result in the specializations table is which keyed off of
>> only the given tmpl and args.  Not sure if that'd be a suitable change
>> for stage4/backports though...
> 
> This makes sense; I guess anytime you have the full set of targs and are
> going to instantiate, tf_partial should be cleared otherwise it can
> wreak havoc.  I'd be nervous about applying such a patch now but maybe
> we could experiment with that in GCC 14.

This sounds right to me; it would never make sense to have tf_partial 
set in instantiate_template, so we should be able to clear it there.  We 
might even do

  complain = complain & tf_warning_or_error

since none of the other flags are relevant at that level either.

But this patch is OK for GCC 13.

> Thanks for taking a look!
>   
>>>         var = finish_template_variable (var, complain);
>>>         mark_used (var);
>>>       }
>>> diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ70.C b/gcc/testsuite/g++.dg/cpp1y/var-templ70.C
>>> new file mode 100644
>>> index 00000000000..1d35c38c7cc
>>> --- /dev/null
>>> +++ b/gcc/testsuite/g++.dg/cpp1y/var-templ70.C
>>> @@ -0,0 +1,25 @@
>>> +// PR c++/108550
>>> +// { dg-do compile { target c++14 } }
>>> +
>>> +template<class T>
>>> +struct is_pointer
>>> +{
>>> +  static constexpr bool value = false;
>>> +};
>>> +
>>> +template<class T, T T1>
>>> +struct integral_constant
>>> +{
>>> +  static constexpr T value = T1;
>>> +};
>>> +
>>> +
>>> +template <class Tp>
>>> +constexpr auto is_pointer_v = is_pointer<Tp>::value;
>>> +
>>> +template <class Tp, int = 0>
>>> +integral_constant<bool, is_pointer_v<int>> Wrap1();
>>
>> It might be good to have a version of this testcase that uses a
>> dependent is_pointer_v<Tp> here instead of is_pointer_v<int>.
> 
> True, added (and I threw an alias template in the mix too).
> 
> -- >8 --
> In this test, we get a bogus error because we failed to deduce the auto in
> constexpr auto is_pointer_v = is_pointer<Tp>::value;
> to bool.  Then ensure_literal_type_for_constexpr_object thinks the object
> isn't literal and an error is reported.
> 
> This is another case of the interaction between tf_partial and 'auto',
> where the auto was not reduced so the deduction failed.  In more detail:
> we have
> 
>    Wrap1<int>()
> 
> in the code and we need to perform OR -> fn_type_unification.  The targ
> list is incomplete, so we do
>        tsubst_flags_t ecomplain = complain | tf_partial | tf_fndecl_type;
>        fntype = tsubst (TREE_TYPE (fn), explicit_targs, ecomplain, NULL_TREE);
> where TREE_TYPE (fn) is struct integral_constant <T402> (void).  Then
> we substitute the return type, which results in tsubsting is_pointer_v<int>.
> is_pointer_v is a variable template with a placeholder type:
> 
>    template <class Tp>
>    constexpr auto is_pointer_v = is_pointer<Tp>::value;
> 
> so we find ourselves in lookup_and_finish_template_variable.  tf_partial is
> still set, so finish_template_variable -> instantiate_template -> tsubst
> won't reduce the level of auto.  But then we do mark_used which eventually
> calls do_auto_deduction which clears tf_partial, because we want to replace
> the auto now.  But we hadn't reduced auto's level so this fails.  And
> since we're not in an immediate context, we emit a hard error.
> 
> I suppose that when we reach lookup_and_finish_template_variable it's
> probably time to clear tf_partial.  (I added an assert and our testsuite
> doesn't have a test whereby we get to lookup_and_finish_template_variable
> while tf_partial is still active.)
> 
> 	PR c++/108550
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.cc (lookup_and_finish_template_variable): Clear tf_partial.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/cpp1y/var-templ70.C: New test.
> 	* g++.dg/cpp1y/var-templ71.C: New test.
> 	* g++.dg/cpp1y/var-templ72.C: New test.
> ---
>   gcc/cp/pt.cc                             |  6 ++++++
>   gcc/testsuite/g++.dg/cpp1y/var-templ70.C | 25 ++++++++++++++++++++++
>   gcc/testsuite/g++.dg/cpp1y/var-templ71.C | 26 +++++++++++++++++++++++
>   gcc/testsuite/g++.dg/cpp1y/var-templ72.C | 27 ++++++++++++++++++++++++
>   4 files changed, 84 insertions(+)
>   create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ70.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ71.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ72.C
> 
> diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> index 1a071e95004..f636bac5413 100644
> --- a/gcc/cp/pt.cc
> +++ b/gcc/cp/pt.cc
> @@ -10355,6 +10355,12 @@ lookup_and_finish_template_variable (tree templ, tree targs,
>     if (TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (templ)) == 1
>         && !any_dependent_template_arguments_p (targs))
>       {
> +      /* We may be called while doing a partial substitution, but the
> +	 type of the variable template may be auto, in which case we
> +	 will call do_auto_deduction in mark_used (which clears tf_partial)
> +	 and the auto must be properly reduced at that time for the
> +	 deduction to work.  */
> +      complain &= ~tf_partial;
>         var = finish_template_variable (var, complain);
>         mark_used (var);
>       }
> diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ70.C b/gcc/testsuite/g++.dg/cpp1y/var-templ70.C
> new file mode 100644
> index 00000000000..1d35c38c7cc
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/var-templ70.C
> @@ -0,0 +1,25 @@
> +// PR c++/108550
> +// { dg-do compile { target c++14 } }
> +
> +template<class T>
> +struct is_pointer
> +{
> +  static constexpr bool value = false;
> +};
> +
> +template<class T, T T1>
> +struct integral_constant
> +{
> +  static constexpr T value = T1;
> +};
> +
> +
> +template <class Tp>
> +constexpr auto is_pointer_v = is_pointer<Tp>::value;
> +
> +template <class Tp, int = 0>
> +integral_constant<bool, is_pointer_v<int>> Wrap1();
> +
> +int main() {
> +  static_assert(!decltype(Wrap1<int>())::value, "");
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ71.C b/gcc/testsuite/g++.dg/cpp1y/var-templ71.C
> new file mode 100644
> index 00000000000..e0cf55230d9
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/var-templ71.C
> @@ -0,0 +1,26 @@
> +// PR c++/108550
> +// { dg-do compile { target c++14 } }
> +
> +template<class T, T T1>
> +struct integral_constant
> +{
> +  static constexpr T value = T1;
> +};
> +
> +template <typename T>
> +struct S {
> +  template <typename U, typename V>
> +  static constexpr void foo(V) { }
> +
> +  constexpr bool bar () const { foo<int>(10); return false; }
> +};
> +
> +template <class Tp>
> +constexpr auto is_pointer_v = S<Tp>{}.bar();
> +
> +template <class Tp, int = 0>
> +integral_constant<bool, is_pointer_v<int>> Wrap1();
> +
> +int main() {
> +  static_assert(!decltype(Wrap1<int>())::value, "");
> +}
> diff --git a/gcc/testsuite/g++.dg/cpp1y/var-templ72.C b/gcc/testsuite/g++.dg/cpp1y/var-templ72.C
> new file mode 100644
> index 00000000000..7426bad4a6c
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1y/var-templ72.C
> @@ -0,0 +1,27 @@
> +// PR c++/108550
> +// { dg-do compile { target c++14 } }
> +
> +template<class T>
> +struct is_pointer
> +{
> +  static constexpr bool value = false;
> +};
> +
> +template<class T, T T1>
> +struct integral_constant
> +{
> +  static constexpr T value = T1;
> +};
> +
> +template<typename T>
> +using PTR_P = is_pointer<T>;
> +
> +template <class Tp>
> +constexpr auto is_pointer_v = PTR_P<Tp>::value;
> +
> +template <class Tp, int = 0>
> +integral_constant<bool, is_pointer_v<Tp>> Wrap1();
> +
> +int main() {
> +  static_assert(!decltype(Wrap1<int>())::value, "");
> +}
> 
> base-commit: 426b0ae103894d1f1bd82e5f049ff8a53bd82a8d

Re: [PATCH] c++: variable template and targ deduction [PR108550]

[Marek Polacek]

On Mon, Feb 27, 2023 at 06:21:13PM -0500, Jason Merrill wrote:
> On 2/23/23 10:54, Marek Polacek wrote:
> > On Thu, Feb 23, 2023 at 10:17:22AM -0500, Patrick Palka wrote:
> > > On Wed, 22 Feb 2023, Marek Polacek wrote:
> > > 
> > > > In this test, we get a bogus error because we failed to deduce the auto in
> > > > constexpr auto is_pointer_v = is_pointer<Tp>::value;
> > > > to bool.  Then ensure_literal_type_for_constexpr_object thinks the object
> > > > isn't literal and an error is reported.
> > > > 
> > > > This is another case of the interaction between tf_partial and 'auto',
> > > > where the auto was not reduced so the deduction failed.  In more detail:
> > > > we have
> > > > 
> > > >    Wrap1<int>()
> > > > 
> > > > in the code and we need to perform OR -> fn_type_unification.  The targ
> > > > list is incomplete, so we do
> > > >        tsubst_flags_t ecomplain = complain | tf_partial | tf_fndecl_type;
> > > >        fntype = tsubst (TREE_TYPE (fn), explicit_targs, ecomplain, NULL_TREE);
> > > > where TREE_TYPE (fn) is struct integral_constant <T402> (void).  Then
> > > > we substitute the return type, which results in tsubsting is_pointer_v<int>.
> > > > is_pointer_v is a variable template with a placeholder type:
> > > > 
> > > >    template <class Tp>
> > > >    constexpr auto is_pointer_v = is_pointer<Tp>::value;
> > > > 
> > > > so we find ourselves in lookup_and_finish_template_variable.  tf_partial is
> > > > still set, so finish_template_variable -> instantiate_template -> tsubst
> > > > won't reduce the level of auto.  But then we do mark_used which eventually
> > > > calls do_auto_deduction which clears tf_partial, because we want to replace
> > > > the auto now.  But we hadn't reduced auto's level so this fails.  And
> > > > since we're not in an immediate context, we emit a hard error.
> > > > 
> > > > I suppose that when we reach lookup_and_finish_template_variable it's
> > > > probably time to clear tf_partial.  (I added an assert and our testsuite
> > > > doesn't have a test whereby we get to lookup_and_finish_template_variable
> > > > while tf_partial is still active.)
> > > > 
> > > > Does this make *any* sense to you?
> > > > 
> > > > Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?
> > > > 
> > > > 	PR c++/108550
> > > > 
> > > > gcc/cp/ChangeLog:
> > > > 
> > > > 	* pt.cc (lookup_and_finish_template_variable): Clear tf_partial.
> > > > 
> > > > gcc/testsuite/ChangeLog:
> > > > 
> > > > 	* g++.dg/cpp1y/var-templ70.C: New test.
> > > > 	* g++.dg/cpp1y/var-templ71.C: New test.
> > > > ---
> > > >   gcc/cp/pt.cc                             |  6 ++++++
> > > >   gcc/testsuite/g++.dg/cpp1y/var-templ70.C | 25 +++++++++++++++++++++++
> > > >   gcc/testsuite/g++.dg/cpp1y/var-templ71.C | 26 ++++++++++++++++++++++++
> > > >   3 files changed, 57 insertions(+)
> > > >   create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ70.C
> > > >   create mode 100644 gcc/testsuite/g++.dg/cpp1y/var-templ71.C
> > > > 
> > > > diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
> > > > index 1a071e95004..f636bac5413 100644
> > > > --- a/gcc/cp/pt.cc
> > > > +++ b/gcc/cp/pt.cc
> > > > @@ -10355,6 +10355,12 @@ lookup_and_finish_template_variable (tree templ, tree targs,
> > > >     if (TMPL_PARMS_DEPTH (DECL_TEMPLATE_PARMS (templ)) == 1
> > > >         && !any_dependent_template_arguments_p (targs))
> > > >       {
> > > > +      /* We may be called while doing a partial substitution, but the
> > > > +	 type of the variable template may be auto, in which case we
> > > > +	 will call do_auto_deduction in mark_used (which clears tf_partial)
> > > > +	 and the auto must be properly reduced at that time for the
> > > > +	 deduction to work.  */
> > > > +      complain &= ~tf_partial;
> > > 
> > > LGTM.  I can't think of a reason to keep tf_partial set at this point
> > > since we know there's only a single level of template arguments left to
> > > substitute and the args we have are non-dependent, so the substitution
> > > is in no way partial.
> > 
> > Right, once we get to finish_template_variable I suppose we're really
> > committed to the var tmpl.  So the fix should be safe, albeit it feels
> > a bit ad hoc.  FWIW, I'd tested adding alias templates to the mix and
> > it still worked.
> > > Though I wonder if ideally we should clear tf_partial more generally
> > > from instantiate_template?  Modulo diagnostics, the result of
> > > instantiate_template shouldn't depend on complain, in particular because
> > > we cache the result in the specializations table is which keyed off of
> > > only the given tmpl and args.  Not sure if that'd be a suitable change
> > > for stage4/backports though...
> > 
> > This makes sense; I guess anytime you have the full set of targs and are
> > going to instantiate, tf_partial should be cleared otherwise it can
> > wreak havoc.  I'd be nervous about applying such a patch now but maybe
> > we could experiment with that in GCC 14.
> 
> This sounds right to me; it would never make sense to have tf_partial set in
> instantiate_template, so we should be able to clear it there.  We might even
> do
> 
>  complain = complain & tf_warning_or_error
> 
> since none of the other flags are relevant at that level either.

I filed an internal-improvement PR and assigned to self for GCC 14:
<https://gcc.gnu.org/PR108960>.

> But this patch is OK for GCC 13.

Thanks,

Marek