[PATCH] replace ICE with error for failed template deduction (PR 84355)

[PATCH] replace ICE with error for failed template deduction (PR 84355)

[Martin Sebor]

[-- Attachment #1: Type: text/plain, Size: 326 bytes --]

A failed template deduction in template member of a template
triggers an ICE with -std=c++17 due to what seems like
a missing handling of invalid input.  Replacing
the gcc_unreachable() call that causes the ICE with a return
statement indicating the deduction failure eliminates the ICE
and restores sane diagnostics.

Martin

[-- Attachment #2: gcc-84355.diff --]
[-- Type: text/x-patch, Size: 1388 bytes --]

PR c++/84355 - [7/8 Regression] ICE with failing template argument deduction

gcc/cp/ChangeLog:

	PR c++/84355
	* pt.c (unify): Return failure instead of asserting.

gcc/testsuite/ChangeLog:

	PR c++/84355
	* g++.dg/cpp1z/class-deduction48.C: New test.

Index: gcc/cp/pt.c
===================================================================
--- gcc/cp/pt.c	(revision 257713)
+++ gcc/cp/pt.c	(working copy)
@@ -20918,7 +20918,7 @@ unify (tree tparms, tree targs, tree parm, tree ar
 	   && TREE_CODE (tparm) != TYPE_DECL)
 	  || (TREE_CODE (parm) == TEMPLATE_TEMPLATE_PARM
 	      && TREE_CODE (tparm) != TEMPLATE_DECL))
-	gcc_unreachable ();
+	return unify_invalid (explain_p);
 
       if (TREE_CODE (parm) == BOUND_TEMPLATE_TEMPLATE_PARM)
 	{
Index: gcc/testsuite/g++.dg/cpp1z/class-deduction48.C
===================================================================
--- gcc/testsuite/g++.dg/cpp1z/class-deduction48.C	(nonexistent)
+++ gcc/testsuite/g++.dg/cpp1z/class-deduction48.C	(working copy)
@@ -0,0 +1,20 @@
+// PR c++/84355 - ICE with failing template argument deduction
+// { dg-do compile }
+// { dg-options "-std=c++17" }
+
+template <typename T>
+struct A
+{
+  template <int>
+  struct B
+  {
+    B (T);
+  };
+
+  A () {
+    B b (0);   // { dg-error "deduction failed" }
+               // { dg-error "no matching function" "" { target *-*-* } .-1 }
+  }
+};
+
+A<int> a;

Re: [PATCH] replace ICE with error for failed template deduction (PR 84355)

[Jason Merrill]

On Thu, Feb 15, 2018 at 6:36 PM, Martin Sebor <msebor@gmail.com> wrote:
> A failed template deduction in template member of a template
> triggers an ICE with -std=c++17 due to what seems like
> a missing handling of invalid input.  Replacing
> the gcc_unreachable() call that causes the ICE with a return
> statement indicating the deduction failure eliminates the ICE
> and restores sane diagnostics.

Hmm, we really shouldn't have gotten there; that assert is checking
that when we see a TEMPLATE_*_PARM node in the template signature, it
corresponds to one of the actual parms of the template.  Sounds like
something is going wrong in build_deduction_guide.

Jason

Re: [PATCH] replace ICE with error for failed template deduction (PR 84355)

[Jason Merrill]

84015 seems likely to be related.

On Fri, Feb 16, 2018 at 9:04 AM, Jason Merrill <jason@redhat.com> wrote:
> On Thu, Feb 15, 2018 at 6:36 PM, Martin Sebor <msebor@gmail.com> wrote:
>> A failed template deduction in template member of a template
>> triggers an ICE with -std=c++17 due to what seems like
>> a missing handling of invalid input.  Replacing
>> the gcc_unreachable() call that causes the ICE with a return
>> statement indicating the deduction failure eliminates the ICE
>> and restores sane diagnostics.
>
> Hmm, we really shouldn't have gotten there; that assert is checking
> that when we see a TEMPLATE_*_PARM node in the template signature, it
> corresponds to one of the actual parms of the template.  Sounds like
> something is going wrong in build_deduction_guide.
>
> Jason

Re: [PATCH] replace ICE with error for failed template deduction (PR 84355)

[Martin Sebor]

On 02/16/2018 07:04 AM, Jason Merrill wrote:
> On Thu, Feb 15, 2018 at 6:36 PM, Martin Sebor <msebor@gmail.com> wrote:
>> A failed template deduction in template member of a template
>> triggers an ICE with -std=c++17 due to what seems like
>> a missing handling of invalid input.  Replacing
>> the gcc_unreachable() call that causes the ICE with a return
>> statement indicating the deduction failure eliminates the ICE
>> and restores sane diagnostics.
>
> Hmm, we really shouldn't have gotten there; that assert is checking
> that when we see a TEMPLATE_*_PARM node in the template signature, it
> corresponds to one of the actual parms of the template.  Sounds like
> something is going wrong in build_deduction_guide.

Are you suggesting that build_deduction_guide should fail somehow
(it's not expected to fail right now) or that the guide it creates
is wrong?  It returns this for the test case in the bug (below):

   template<int I> B(A<T>::B<I>)-> A<T>::B<I>

If I make B a template with a type argument it returns this:

   template<class U> B(A<T>::B<U>)-> A<T>::B<U>

The test case is accepted then.

Martin

template<typename T> struct A
{
   template<int> struct B
   {
     B(T);
   };

   A() { B b(0); }
};

A<int> a;

Re: [PATCH] replace ICE with error for failed template deduction (PR 84355)

[Jason Merrill]

On Fri, Feb 16, 2018 at 4:33 PM, Martin Sebor <msebor@gmail.com> wrote:
> On 02/16/2018 07:04 AM, Jason Merrill wrote:
>>
>> On Thu, Feb 15, 2018 at 6:36 PM, Martin Sebor <msebor@gmail.com> wrote:
>>>
>>> A failed template deduction in template member of a template
>>> triggers an ICE with -std=c++17 due to what seems like
>>> a missing handling of invalid input.  Replacing
>>> the gcc_unreachable() call that causes the ICE with a return
>>> statement indicating the deduction failure eliminates the ICE
>>> and restores sane diagnostics.
>>
>>
>> Hmm, we really shouldn't have gotten there; that assert is checking
>> that when we see a TEMPLATE_*_PARM node in the template signature, it
>> corresponds to one of the actual parms of the template.  Sounds like
>> something is going wrong in build_deduction_guide.
>
>
> Are you suggesting that build_deduction_guide should fail somehow
> (it's not expected to fail right now) or that the guide it creates
> is wrong?

The latter.  Maybe we're handling T wrong somehow?  We shouldn't be
trying to deduce it.  In fact, we probably shouldn't be trying to
deduce arguments for 'b' until we instantiate A.

Jason

Re: [PATCH] replace ICE with error for failed template deduction (PR 84355)

[Jason Merrill]

On Sun, Feb 18, 2018 at 11:39 PM, Jason Merrill <jason@redhat.com> wrote:
> On Fri, Feb 16, 2018 at 4:33 PM, Martin Sebor <msebor@gmail.com> wrote:
>> On 02/16/2018 07:04 AM, Jason Merrill wrote:
>>>
>>> On Thu, Feb 15, 2018 at 6:36 PM, Martin Sebor <msebor@gmail.com> wrote:
>>>>
>>>> A failed template deduction in template member of a template
>>>> triggers an ICE with -std=c++17 due to what seems like
>>>> a missing handling of invalid input.  Replacing
>>>> the gcc_unreachable() call that causes the ICE with a return
>>>> statement indicating the deduction failure eliminates the ICE
>>>> and restores sane diagnostics.
>>>
>>>
>>> Hmm, we really shouldn't have gotten there; that assert is checking
>>> that when we see a TEMPLATE_*_PARM node in the template signature, it
>>> corresponds to one of the actual parms of the template.  Sounds like
>>> something is going wrong in build_deduction_guide.
>>
>>
>> Are you suggesting that build_deduction_guide should fail somehow
>> (it's not expected to fail right now) or that the guide it creates
>> is wrong?
>
> The latter.  Maybe we're handling T wrong somehow?  We shouldn't be
> trying to deduce it.  In fact, we probably shouldn't be trying to
> deduce arguments for 'b' until we instantiate A.

Looks like the problem is that when we substitute into the
TEMPLATE_TYPE_PARM representing 'B' in the function, we don't touch
CLASS_PLACEHOLDER_TEMPLATE:

                    else if (tree pl = CLASS_PLACEHOLDER_TEMPLATE (t))
                      {
                        if (DECL_TEMPLATE_TEMPLATE_PARM_P (pl))
                          pl = tsubst (pl, args, complain, in_decl);
                        CLASS_PLACEHOLDER_TEMPLATE (r) = pl;
                      }

This code is failing to replace A<T>::B with A<int>::B.

Jason

Re: [PATCH] replace ICE with error for failed template deduction (PR 84355)

[Martin Sebor]

On 02/23/2018 07:32 PM, Jason Merrill wrote:
> On Sun, Feb 18, 2018 at 11:39 PM, Jason Merrill <jason@redhat.com> wrote:
>> On Fri, Feb 16, 2018 at 4:33 PM, Martin Sebor <msebor@gmail.com> wrote:
>>> On 02/16/2018 07:04 AM, Jason Merrill wrote:
>>>>
>>>> On Thu, Feb 15, 2018 at 6:36 PM, Martin Sebor <msebor@gmail.com> wrote:
>>>>>
>>>>> A failed template deduction in template member of a template
>>>>> triggers an ICE with -std=c++17 due to what seems like
>>>>> a missing handling of invalid input.  Replacing
>>>>> the gcc_unreachable() call that causes the ICE with a return
>>>>> statement indicating the deduction failure eliminates the ICE
>>>>> and restores sane diagnostics.
>>>>
>>>>
>>>> Hmm, we really shouldn't have gotten there; that assert is checking
>>>> that when we see a TEMPLATE_*_PARM node in the template signature, it
>>>> corresponds to one of the actual parms of the template.  Sounds like
>>>> something is going wrong in build_deduction_guide.
>>>
>>>
>>> Are you suggesting that build_deduction_guide should fail somehow
>>> (it's not expected to fail right now) or that the guide it creates
>>> is wrong?
>>
>> The latter.  Maybe we're handling T wrong somehow?  We shouldn't be
>> trying to deduce it.  In fact, we probably shouldn't be trying to
>> deduce arguments for 'b' until we instantiate A.
>
> Looks like the problem is that when we substitute into the
> TEMPLATE_TYPE_PARM representing 'B' in the function, we don't touch
> CLASS_PLACEHOLDER_TEMPLATE:
>
>                     else if (tree pl = CLASS_PLACEHOLDER_TEMPLATE (t))
>                       {
>                         if (DECL_TEMPLATE_TEMPLATE_PARM_P (pl))
>                           pl = tsubst (pl, args, complain, in_decl);
>                         CLASS_PLACEHOLDER_TEMPLATE (r) = pl;
>                       }
>
> This code is failing to replace A<T>::B with A<int>::B.

I don't know what to do here/what you're suggesting.  Before
the call to tsubst() pl is a TEMPLATE_DECL of struct A<T>::B.
Calling tsubst() works and replaces the ICE with a reasonable
error but then causes an ICE in cpp1z/class-deduction19.C.
There, pl is also TEMPLATE_DECL, and I'm not sure how to
differentiate between the two at this level.

I was hoping I could fix this ICE quickly but I've spent too
much time on it with no progress so unless I'm just being
dense by missing your hint I think I'm going to have to give
up on this bug.

Martin

Re: [PATCH] replace ICE with error for failed template deduction (PR 84355)

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 2548 bytes --]

On Mon, Mar 5, 2018 at 7:05 PM, Martin Sebor <msebor@gmail.com> wrote:
> On 02/23/2018 07:32 PM, Jason Merrill wrote:
>> On Sun, Feb 18, 2018 at 11:39 PM, Jason Merrill <jason@redhat.com> wrote:
>>> On Fri, Feb 16, 2018 at 4:33 PM, Martin Sebor <msebor@gmail.com> wrote:
>>>> On 02/16/2018 07:04 AM, Jason Merrill wrote:
>>>>>
>>>>> On Thu, Feb 15, 2018 at 6:36 PM, Martin Sebor <msebor@gmail.com> wrote:
>>>>>>
>>>>>>
>>>>>> A failed template deduction in template member of a template
>>>>>> triggers an ICE with -std=c++17 due to what seems like
>>>>>> a missing handling of invalid input.  Replacing
>>>>>> the gcc_unreachable() call that causes the ICE with a return
>>>>>> statement indicating the deduction failure eliminates the ICE
>>>>>> and restores sane diagnostics.
>>>>>
>>>>>
>>>>>
>>>>> Hmm, we really shouldn't have gotten there; that assert is checking
>>>>> that when we see a TEMPLATE_*_PARM node in the template signature, it
>>>>> corresponds to one of the actual parms of the template.  Sounds like
>>>>> something is going wrong in build_deduction_guide.
>>>>
>>>>
>>>>
>>>> Are you suggesting that build_deduction_guide should fail somehow
>>>> (it's not expected to fail right now) or that the guide it creates
>>>> is wrong?
>>>
>>>
>>> The latter.  Maybe we're handling T wrong somehow?  We shouldn't be
>>> trying to deduce it.  In fact, we probably shouldn't be trying to
>>> deduce arguments for 'b' until we instantiate A.
>>
>>
>> Looks like the problem is that when we substitute into the
>> TEMPLATE_TYPE_PARM representing 'B' in the function, we don't touch
>> CLASS_PLACEHOLDER_TEMPLATE:
>>
>>                     else if (tree pl = CLASS_PLACEHOLDER_TEMPLATE (t))
>>                       {
>>                         if (DECL_TEMPLATE_TEMPLATE_PARM_P (pl))
>>                           pl = tsubst (pl, args, complain, in_decl);
>>                         CLASS_PLACEHOLDER_TEMPLATE (r) = pl;
>>                       }
>>
>> This code is failing to replace A<T>::B with A<int>::B.
>
> I don't know what to do here/what you're suggesting.  Before
> the call to tsubst() pl is a TEMPLATE_DECL of struct A<T>::B.
> Calling tsubst() works and replaces the ICE with a reasonable
> error but then causes an ICE in cpp1z/class-deduction19.C.
> There, pl is also TEMPLATE_DECL, and I'm not sure how to
> differentiate between the two at this level.

The problem is that tsubst calls tsubst_decl, which tries to generate
a new instantiated declaration; since this is a use, we need to use
tsubst_copy instead.

Jason

[-- Attachment #2: 84355.diff --]
[-- Type: text/x-patch, Size: 1421 bytes --]

commit 5e194d897e76c8a54a14a50b3a6cb94a21e6810d
Author: Jason Merrill <jason@redhat.com>
Date:   Sun Mar 11 21:43:32 2018 -0400

            PR c++/84355 - ICE with deduction for member class template.
    
            * pt.c (tsubst) [TEMPLATE_TYPE_PARM]: Always substitute into
            CLASS_PLACEHOLDER_TEMPLATE.

diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index a92b36a6031..4640ca08ce0 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -14125,8 +14125,7 @@ tsubst (tree t, tree args, tsubst_flags_t complain, tree in_decl)
 			= tsubst_constraint (constr, args, complain, in_decl);
 		    else if (tree pl = CLASS_PLACEHOLDER_TEMPLATE (t))
 		      {
-			if (DECL_TEMPLATE_TEMPLATE_PARM_P (pl))
-			  pl = tsubst (pl, args, complain, in_decl);
+			pl = tsubst_copy (pl, args, complain, in_decl);
 			CLASS_PLACEHOLDER_TEMPLATE (r) = pl;
 		      }
 		  }
diff --git a/gcc/testsuite/g++.dg/cpp1z/class-deduction50.C b/gcc/testsuite/g++.dg/cpp1z/class-deduction50.C
new file mode 100644
index 00000000000..e8cdd8c710f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/class-deduction50.C
@@ -0,0 +1,22 @@
+// PR c++/84355
+// { dg-additional-options -std=c++17 }
+
+template <class, class> struct same;
+template <class T> struct same<T,T> {};
+
+template<typename T> struct A
+{
+  template<class U> struct B
+  {
+    B(U);
+  };
+
+  A() {
+    B b(0);
+    same<decltype(b),B<int>>{};
+  }
+};
+
+struct C {};
+
+A<C> a;