diff mbox series

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

Message ID 786533c4-2017-86c0-8813-0bea046fc7d0@gmail.com
State New
Headers show
Series replace ICE with error for failed template deduction (PR 84355) | expand

Commit Message

Martin Sebor Feb. 15, 2018, 11:36 p.m. UTC 
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

Comments

Jason Merrill Feb. 16, 2018, 2:04 p.m. UTC | #1 
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
Jason Merrill Feb. 16, 2018, 7:30 p.m. UTC | #2 
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
Martin Sebor Feb. 16, 2018, 9:33 p.m. UTC | #3 
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;
Jason Merrill Feb. 19, 2018, 4:39 a.m. UTC | #4 
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
Jason Merrill Feb. 24, 2018, 2:32 a.m. UTC | #5 
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
Martin Sebor March 6, 2018, 12:05 a.m. UTC | #6 
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
Jason Merrill March 12, 2018, 2:39 p.m. UTC | #7 
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
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;
diff mbox series

Patch

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;