diff mbox

[C++] c++/78776 fix alias template ICE

Message ID ef9e1f4c-b1e2-24f3-649a-c45d0d51eba3@acm.org
State New
Headers show

Commit Message

Nathan Sidwell Dec. 12, 2016, 7:12 p.m. UTC
This patch fixes an ICE in a checking build where structural_comptypes 
disagrees with TYPE_CANONICAL.

An (implicit) template alias has a different TYPE_TI_TEMPLATE to the 
thing its aliasing.  That make structural comparison think it's 
different.  In the testcase we end up thinking 'Loc' refers to a 
different template than 'Traits<T>'

Solved by breaking TYPE_TEMPLATE_INFO into an underlying helper that 
doesn't deal with type aliases.  While there, I noticed 
TYPE_TEMPLATE_INFO was doing more work than necessary because it checked 
twice whether DECL_LANG_SPECIFIC (TYPE_NAME (NODE)) was non-null. 
There's no need to check it again in the branch we can only get to when 
it's non-null.  I also removed some unnecessary parens.

ok?

nathan
-- 
Nathan Sidwell

Comments

Jason Merrill Dec. 12, 2016, 9:44 p.m. UTC | #1
On Mon, Dec 12, 2016 at 2:12 PM, Nathan Sidwell <nathan@acm.org> wrote:
> Solved by breaking TYPE_TEMPLATE_INFO into an underlying helper that doesn't

> deal with type aliases.


I like this idea, but I don't like the name.  Since alias templates
are generally transparent in the language, I wonder about changing
TYPE_TEMPLATE_INFO to look through aliases and creating a new macro
that also handles aliases, say TYPE_TEMPLATE_INFO_MAYBE_ALIAS?

Jason
Nathan Sidwell Dec. 12, 2016, 10:48 p.m. UTC | #2
On 12/12/2016 04:44 PM, Jason Merrill wrote:
> On Mon, Dec 12, 2016 at 2:12 PM, Nathan Sidwell <nathan@acm.org> wrote:

>> Solved by breaking TYPE_TEMPLATE_INFO into an underlying helper that doesn't

>> deal with type aliases.

>

> I like this idea, but I don't like the name.  Since alias templates

> are generally transparent in the language, I wonder about changing

> TYPE_TEMPLATE_INFO to look through aliases and creating a new macro


Do you mean '... to NOT look ...'?  If so ...

> that also handles aliases, say TYPE_TEMPLATE_INFO_MAYBE_ALIAS?


I had similar thought (down to the MAYBE name), but guessed that the 
invasiveness would be great.  I suppose SED to the rescue?

nathan

-- 
Nathan Sidwell
Jason Merrill Dec. 13, 2016, 3:28 a.m. UTC | #3
On Mon, Dec 12, 2016 at 5:48 PM, Nathan Sidwell <nathan@acm.org> wrote:
> On 12/12/2016 04:44 PM, Jason Merrill wrote:

>>

>> On Mon, Dec 12, 2016 at 2:12 PM, Nathan Sidwell <nathan@acm.org> wrote:

>>>

>>> Solved by breaking TYPE_TEMPLATE_INFO into an underlying helper that

>>> doesn't deal with type aliases.

>>

>> I like this idea, but I don't like the name.  Since alias templates

>> are generally transparent in the language, I wonder about changing

>> TYPE_TEMPLATE_INFO to look through aliases and creating a new macro

>

> Do you mean '... to NOT look ...'?  If so ...

>

>> that also handles aliases, say TYPE_TEMPLATE_INFO_MAYBE_ALIAS?

>

> I had similar thought (down to the MAYBE name), but guessed that the

> invasiveness would be great.  I suppose SED to the rescue?


I suspect that most uses don't need to change.

Jason
Andrew Pinski Dec. 13, 2016, 5:10 a.m. UTC | #4
On Mon, Dec 12, 2016 at 11:12 AM, Nathan Sidwell <nathan@acm.org> wrote:
> This patch fixes an ICE in a checking build where structural_comptypes

> disagrees with TYPE_CANONICAL.

>

> An (implicit) template alias has a different TYPE_TI_TEMPLATE to the thing

> its aliasing.  That make structural comparison think it's different.  In the

> testcase we end up thinking 'Loc' refers to a different template than

> 'Traits<T>'

>

> Solved by breaking TYPE_TEMPLATE_INFO into an underlying helper that doesn't

> deal with type aliases.  While there, I noticed TYPE_TEMPLATE_INFO was doing

> more work than necessary because it checked twice whether DECL_LANG_SPECIFIC

> (TYPE_NAME (NODE)) was non-null. There's no need to check it again in the

> branch we can only get to when it's non-null.  I also removed some

> unnecessary parens.

>

> ok?


Note this is originally PR 69481 which I found a year or so ago.  I
was trying to debug this and try to understand why typedef and using
would produce different result but I never could figure it out.  I
also could not figure out why they went down a different path either
and not treated as the same.

Thanks,
Andrew

>

> nathan

> --

> Nathan Sidwell
diff mbox

Patch

2016-12-12  Nathan Sidwell  <nathan@acm.org>

	PR c++/78776
	* cp-tree.h (TYPE_TEMPLATE_INFO_RAW): New, broken out of ...
	(TYPE_TEMPLATE_INFO): ... here. Use it.
	* typeck.c (structural_comptypes): Adjust record/union check to
	avoid alias template confusion.

	PR c++/78776
	* g++.dg/cpp0x/pr78776.C: New.

Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 243554)
+++ cp/cp-tree.h	(working copy)
@@ -3038,22 +3038,25 @@  extern void decl_shadowed_for_var_insert
    ->template_info)
 
 /* Template information for an ENUMERAL_, RECORD_, UNION_TYPE, or
-   BOUND_TEMPLATE_TEMPLATE_PARM type.  Note that if NODE is a
-   specialization of an alias template, this accessor returns the
-   template info for the alias template, not the one (if any) for the
-   template of the underlying type.  */
+   BOUND_TEMPLATE_TEMPLATE_PARM type.  This ignores any alias
+   templateness of NODE.  */
+
+#define TYPE_TEMPLATE_INFO_RAW(NODE)					\
+  (TREE_CODE (NODE) == ENUMERAL_TYPE					\
+   ? ENUM_TEMPLATE_INFO (NODE)						\
+   : (TREE_CODE (NODE) == BOUND_TEMPLATE_TEMPLATE_PARM			\
+      ? TEMPLATE_TEMPLATE_PARM_TEMPLATE_INFO (NODE)			\
+      : (CLASS_TYPE_P (NODE)						\
+	 ? CLASSTYPE_TEMPLATE_INFO (NODE)				\
+	 : NULL_TREE)))
+
+/* If NODE is a specialization of an alias template, this accessor
+   returns the template info for the alias template.  Otherwise behave
+   as TYPE_TEMPLATE_INFO_RAW.  */
 #define TYPE_TEMPLATE_INFO(NODE)					\
-  ((TYPE_ALIAS_P (NODE) && DECL_LANG_SPECIFIC (TYPE_NAME (NODE)))	\
-   ? (DECL_LANG_SPECIFIC (TYPE_NAME (NODE))				\
-      ? DECL_TEMPLATE_INFO (TYPE_NAME (NODE))				\
-      : NULL_TREE)							\
-   : ((TREE_CODE (NODE) == ENUMERAL_TYPE)				\
-      ? ENUM_TEMPLATE_INFO (NODE)					\
-      : ((TREE_CODE (NODE) == BOUND_TEMPLATE_TEMPLATE_PARM)		\
-	 ? TEMPLATE_TEMPLATE_PARM_TEMPLATE_INFO (NODE)			\
-	 : (CLASS_TYPE_P (NODE)						\
-	    ? CLASSTYPE_TEMPLATE_INFO (NODE)				\
-	    : NULL_TREE))))
+  (TYPE_ALIAS_P (NODE) && DECL_LANG_SPECIFIC (TYPE_NAME (NODE))		\
+   ? DECL_TEMPLATE_INFO (TYPE_NAME (NODE))				\
+   : TYPE_TEMPLATE_INFO_RAW (NODE))
 
 
 /* Set the template information for an ENUMERAL_, RECORD_, or
Index: cp/typeck.c
===================================================================
--- cp/typeck.c	(revision 243554)
+++ cp/typeck.c	(working copy)
@@ -1284,18 +1284,23 @@  structural_comptypes (tree t1, tree t2,
 
     case RECORD_TYPE:
     case UNION_TYPE:
-      if (TYPE_TEMPLATE_INFO (t1) && TYPE_TEMPLATE_INFO (t2)
-	  && (TYPE_TI_TEMPLATE (t1) == TYPE_TI_TEMPLATE (t2)
-	      || TREE_CODE (t1) == BOUND_TEMPLATE_TEMPLATE_PARM)
-	  && comp_template_args (TYPE_TI_ARGS (t1), TYPE_TI_ARGS (t2)))
-	break;
+      {
+	/* Ignore any alias templateness.  */
+	tree ti1 = TYPE_TEMPLATE_INFO_RAW (t1);
+	tree ti2 = TYPE_TEMPLATE_INFO_RAW (t2);
 
-      if ((strict & COMPARE_BASE) && DERIVED_FROM_P (t1, t2))
-	break;
-      else if ((strict & COMPARE_DERIVED) && DERIVED_FROM_P (t2, t1))
-	break;
+	if (ti1 && ti2
+	    && (TI_TEMPLATE (ti1) == TI_TEMPLATE (ti2)
+		|| TREE_CODE (t1) == BOUND_TEMPLATE_TEMPLATE_PARM)
+	    && comp_template_args (TI_ARGS (ti1), TI_ARGS (ti2)))
+	  break;
+	if ((strict & COMPARE_BASE) && DERIVED_FROM_P (t1, t2))
+	  break;
+	else if ((strict & COMPARE_DERIVED) && DERIVED_FROM_P (t2, t1))
+	  break;
 
-      return false;
+	return false;
+      }
 
     case OFFSET_TYPE:
       if (!comptypes (TYPE_OFFSET_BASETYPE (t1), TYPE_OFFSET_BASETYPE (t2),
Index: testsuite/g++.dg/cpp0x/pr78776.C
===================================================================
--- testsuite/g++.dg/cpp0x/pr78776.C	(revision 0)
+++ testsuite/g++.dg/cpp0x/pr78776.C	(working copy)
@@ -0,0 +1,15 @@ 
+// PR c++/78776
+// { dg-do compile { target c++11 } }
+
+// ICE with canonical type verification
+
+template <typename> struct Traits;
+
+template <typename T>
+struct Bob {
+  using Loc = Traits<T>;
+  using typename Loc::Thing;
+
+  Thing Foo (); 
+};
+