public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH] c++: Fix deduction from the type of an NTTP
@ 2021-01-04 22:50 Patrick Palka
  2021-01-05 16:30 ` Jason Merrill
  0 siblings, 1 reply; 2+ messages in thread
From: Patrick Palka @ 2021-01-04 22:50 UTC (permalink / raw)
  To: gcc-patches

In the testcase nontype-auto17.C below, the calls to f and g are invalid
because neither deduction nor defaulting of the template parameter T
yields a valid specialization.  Deducing T doesn't work because T is
only used in a non-deduced context, and defaulting T doesn't work
because its default argument makes the type of M invalid.

But with -std=c++17 or later, we incorrectly accept both calls.  With
C++17 (specifically P0127R2), we're allowed to try to deduce T from
the argument 42 that's been tentatively deduced for M.  The problem is
that when unify walks into the type of M, it immediately gives up on
the TYPENAME_TYPE and doesn't register any new unifications (so the type
of M is still unknown) -- and then we go on to unify M with 42 anyway.
Later in type_unification_real, we blindly use the default argument for
T to complete the template argument vector, and we end up with the bogus
specializations f<void, 42> and g<S, 42>.

This patch fixes this issue by checking whether the type of a NTTP is
still dependent after walking into the type.  If it is, it means we
couldn't deduce all the template parameters used in its type, and so we
shouldn't yet unify the NTTP.

(The new testcase ttp33.C demonstrates the need for the TEMPLATE_PARM_LEVEL
check; without it, we would ICE on this testcase from the call to tsubst.)

Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?

gcc/cp/ChangeLog:

	* pt.c (unify) <case TEMPLATE_PARM_INDEX>: After walking into
	the type of the TEMPLATE_PARM_INDEX, substitute into the type a
	second time.  If the type is still dependent, don't unify it.

gcc/testsuite/ChangeLog:

	* g++.dg/template/partial5.C: Adjust directives to expect the
	same errors across all dialects.
	* g++.dg/cpp1z/nontype-auto17.C: New test.
	* g++.dg/cpp1z/nontype-auto18.C: New test.
	* g++.dg/template/ttp33.C: New test.
---
 gcc/cp/pt.c                                 | 10 +++++++++-
 gcc/testsuite/g++.dg/cpp1z/nontype-auto17.C | 10 ++++++++++
 gcc/testsuite/g++.dg/cpp1z/nontype-auto18.C |  6 ++++++
 gcc/testsuite/g++.dg/template/partial5.C    |  2 +-
 gcc/testsuite/g++.dg/template/ttp33.C       | 10 ++++++++++
 5 files changed, 36 insertions(+), 2 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/nontype-auto17.C
 create mode 100644 gcc/testsuite/g++.dg/cpp1z/nontype-auto18.C
 create mode 100644 gcc/testsuite/g++.dg/template/ttp33.C

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index 19fd4c1d8a4..f1e8b01bc01 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -23581,13 +23581,21 @@ unify (tree tparms, tree targs, tree parm, tree arg, int strict,
 	  /* We haven't deduced the type of this parameter yet.  */
 	  if (cxx_dialect >= cxx17
 	      /* We deduce from array bounds in try_array_deduction.  */
-	      && !(strict & UNIFY_ALLOW_INTEGER))
+	      && !(strict & UNIFY_ALLOW_INTEGER)
+	      && TEMPLATE_PARM_LEVEL (parm) <= TMPL_ARGS_DEPTH (targs))
 	    {
 	      /* Deduce it from the non-type argument.  */
 	      tree atype = TREE_TYPE (arg);
 	      RECUR_AND_CHECK_FAILURE (tparms, targs,
 				       tparm, atype,
 				       UNIFY_ALLOW_NONE, explain_p);
+	      /* Now check whether the type of this parameter is still
+		 dependent, and give up if so.  */
+	      ++processing_template_decl;
+	      tparm = tsubst (tparm, targs, tf_none, NULL_TREE);
+	      --processing_template_decl;
+	      if (uses_template_parms (tparm))
+		return unify_success (explain_p);
 	    }
 	  else
 	    /* Try again later.  */
diff --git a/gcc/testsuite/g++.dg/cpp1z/nontype-auto17.C b/gcc/testsuite/g++.dg/cpp1z/nontype-auto17.C
new file mode 100644
index 00000000000..509eb0e98e3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/nontype-auto17.C
@@ -0,0 +1,10 @@
+// { dg-do compile { target c++11 } }
+
+template <int> struct K { };
+
+template <class T = void, typename T::type M> int f(K<M>); // { dg-error "void" }
+int a = f(K<42>{}); // { dg-error "no match" }
+
+struct S { using type = void; };
+template <class T = S, typename T::type M> int g(K<M>); // { dg-message "deduction" }
+int b = g(K<42>{}); // { dg-error "no match" }
diff --git a/gcc/testsuite/g++.dg/cpp1z/nontype-auto18.C b/gcc/testsuite/g++.dg/cpp1z/nontype-auto18.C
new file mode 100644
index 00000000000..46873672714
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/nontype-auto18.C
@@ -0,0 +1,6 @@
+// { dg-do compile { target c++11 } }
+
+template <int> struct K { };
+struct S { using type = int; };
+template <class T = S, typename T::type M> int f(K<M>);
+int c = f(K<42>{});
diff --git a/gcc/testsuite/g++.dg/template/partial5.C b/gcc/testsuite/g++.dg/template/partial5.C
index a56229770f4..40d8c45b087 100644
--- a/gcc/testsuite/g++.dg/template/partial5.C
+++ b/gcc/testsuite/g++.dg/template/partial5.C
@@ -14,7 +14,7 @@ template<typename T, typename T::foo V>
 struct Y { };
 
 template<typename T, typename U, U v>
-struct Y<T, v> { }; // { dg-error "" "" { target { ! c++17 } } }
+struct Y<T, v> { }; // { dg-error "" }
 
 
 template<typename T, T V>
diff --git a/gcc/testsuite/g++.dg/template/ttp33.C b/gcc/testsuite/g++.dg/template/ttp33.C
new file mode 100644
index 00000000000..cd0de8ca641
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/ttp33.C
@@ -0,0 +1,10 @@
+// A slight variation of ttp31.C.
+// { dg-do compile { target c++11 } }
+
+template<class TA,
+	 template<typename TA::type...> class TTA, TA... VA>
+struct A { };
+
+template<class UC, class TC,
+	 template<typename TC::type...> class TTC, TC... VC>
+struct C : A<TC, TTC, VC...> { };
-- 
2.30.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] c++: Fix deduction from the type of an NTTP
  2021-01-04 22:50 [PATCH] c++: Fix deduction from the type of an NTTP Patrick Palka
@ 2021-01-05 16:30 ` Jason Merrill
  0 siblings, 0 replies; 2+ messages in thread
From: Jason Merrill @ 2021-01-05 16:30 UTC (permalink / raw)
  To: Patrick Palka, gcc-patches

On 1/4/21 5:50 PM, Patrick Palka wrote:
> In the testcase nontype-auto17.C below, the calls to f and g are invalid
> because neither deduction nor defaulting of the template parameter T
> yields a valid specialization.  Deducing T doesn't work because T is
> only used in a non-deduced context, and defaulting T doesn't work
> because its default argument makes the type of M invalid.
> 
> But with -std=c++17 or later, we incorrectly accept both calls.  With
> C++17 (specifically P0127R2), we're allowed to try to deduce T from
> the argument 42 that's been tentatively deduced for M.  The problem is
> that when unify walks into the type of M, it immediately gives up on
> the TYPENAME_TYPE and doesn't register any new unifications (so the type
> of M is still unknown) -- and then we go on to unify M with 42 anyway.
> Later in type_unification_real, we blindly use the default argument for
> T to complete the template argument vector, and we end up with the bogus
> specializations f<void, 42> and g<S, 42>.
> 
> This patch fixes this issue by checking whether the type of a NTTP is
> still dependent after walking into the type.  If it is, it means we
> couldn't deduce all the template parameters used in its type, and so we
> shouldn't yet unify the NTTP.
> 
> (The new testcase ttp33.C demonstrates the need for the TEMPLATE_PARM_LEVEL
> check; without it, we would ICE on this testcase from the call to tsubst.)
> 
> Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> trunk?
> 
> gcc/cp/ChangeLog:
> 
> 	* pt.c (unify) <case TEMPLATE_PARM_INDEX>: After walking into
> 	the type of the TEMPLATE_PARM_INDEX, substitute into the type a
> 	second time.  If the type is still dependent, don't unify it.
> 
> gcc/testsuite/ChangeLog:
> 
> 	* g++.dg/template/partial5.C: Adjust directives to expect the
> 	same errors across all dialects.
> 	* g++.dg/cpp1z/nontype-auto17.C: New test.
> 	* g++.dg/cpp1z/nontype-auto18.C: New test.
> 	* g++.dg/template/ttp33.C: New test.
> ---
>   gcc/cp/pt.c                                 | 10 +++++++++-
>   gcc/testsuite/g++.dg/cpp1z/nontype-auto17.C | 10 ++++++++++
>   gcc/testsuite/g++.dg/cpp1z/nontype-auto18.C |  6 ++++++
>   gcc/testsuite/g++.dg/template/partial5.C    |  2 +-
>   gcc/testsuite/g++.dg/template/ttp33.C       | 10 ++++++++++
>   5 files changed, 36 insertions(+), 2 deletions(-)
>   create mode 100644 gcc/testsuite/g++.dg/cpp1z/nontype-auto17.C
>   create mode 100644 gcc/testsuite/g++.dg/cpp1z/nontype-auto18.C
>   create mode 100644 gcc/testsuite/g++.dg/template/ttp33.C
> 
> diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
> index 19fd4c1d8a4..f1e8b01bc01 100644
> --- a/gcc/cp/pt.c
> +++ b/gcc/cp/pt.c
> @@ -23581,13 +23581,21 @@ unify (tree tparms, tree targs, tree parm, tree arg, int strict,
>   	  /* We haven't deduced the type of this parameter yet.  */
>   	  if (cxx_dialect >= cxx17
>   	      /* We deduce from array bounds in try_array_deduction.  */
> -	      && !(strict & UNIFY_ALLOW_INTEGER))
> +	      && !(strict & UNIFY_ALLOW_INTEGER)
> +	      && TEMPLATE_PARM_LEVEL (parm) <= TMPL_ARGS_DEPTH (targs))
>   	    {
>   	      /* Deduce it from the non-type argument.  */
>   	      tree atype = TREE_TYPE (arg);
>   	      RECUR_AND_CHECK_FAILURE (tparms, targs,
>   				       tparm, atype,
>   				       UNIFY_ALLOW_NONE, explain_p);
> +	      /* Now check whether the type of this parameter is still
> +		 dependent, and give up if so.  */
> +	      ++processing_template_decl;
> +	      tparm = tsubst (tparm, targs, tf_none, NULL_TREE);
> +	      --processing_template_decl;
> +	      if (uses_template_parms (tparm))
> +		return unify_success (explain_p);

Hmm, I was wondering about returning success without checking whether 
the type is still dependent, and relying on the retrying in 
type_unification_real, but that only works for function templates.

The patch is OK.

>   	    }
>   	  else
>   	    /* Try again later.  */
> diff --git a/gcc/testsuite/g++.dg/cpp1z/nontype-auto17.C b/gcc/testsuite/g++.dg/cpp1z/nontype-auto17.C
> new file mode 100644
> index 00000000000..509eb0e98e3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1z/nontype-auto17.C
> @@ -0,0 +1,10 @@
> +// { dg-do compile { target c++11 } }
> +
> +template <int> struct K { };
> +
> +template <class T = void, typename T::type M> int f(K<M>); // { dg-error "void" }
> +int a = f(K<42>{}); // { dg-error "no match" }
> +
> +struct S { using type = void; };
> +template <class T = S, typename T::type M> int g(K<M>); // { dg-message "deduction" }
> +int b = g(K<42>{}); // { dg-error "no match" }
> diff --git a/gcc/testsuite/g++.dg/cpp1z/nontype-auto18.C b/gcc/testsuite/g++.dg/cpp1z/nontype-auto18.C
> new file mode 100644
> index 00000000000..46873672714
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp1z/nontype-auto18.C
> @@ -0,0 +1,6 @@
> +// { dg-do compile { target c++11 } }
> +
> +template <int> struct K { };
> +struct S { using type = int; };
> +template <class T = S, typename T::type M> int f(K<M>);
> +int c = f(K<42>{});
> diff --git a/gcc/testsuite/g++.dg/template/partial5.C b/gcc/testsuite/g++.dg/template/partial5.C
> index a56229770f4..40d8c45b087 100644
> --- a/gcc/testsuite/g++.dg/template/partial5.C
> +++ b/gcc/testsuite/g++.dg/template/partial5.C
> @@ -14,7 +14,7 @@ template<typename T, typename T::foo V>
>   struct Y { };
>   
>   template<typename T, typename U, U v>
> -struct Y<T, v> { }; // { dg-error "" "" { target { ! c++17 } } }
> +struct Y<T, v> { }; // { dg-error "" }
>   
>   
>   template<typename T, T V>
> diff --git a/gcc/testsuite/g++.dg/template/ttp33.C b/gcc/testsuite/g++.dg/template/ttp33.C
> new file mode 100644
> index 00000000000..cd0de8ca641
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/template/ttp33.C
> @@ -0,0 +1,10 @@
> +// A slight variation of ttp31.C.
> +// { dg-do compile { target c++11 } }
> +
> +template<class TA,
> +	 template<typename TA::type...> class TTA, TA... VA>
> +struct A { };
> +
> +template<class UC, class TC,
> +	 template<typename TC::type...> class TTC, TC... VC>
> +struct C : A<TC, TTC, VC...> { };
> 


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2021-01-05 16:30 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-04 22:50 [PATCH] c++: Fix deduction from the type of an NTTP Patrick Palka
2021-01-05 16:30 ` Jason Merrill

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).