diff mbox

Fix PR c++/46394

Message ID m3d3n7mvud.fsf@redhat.com
State New
Headers show

Commit Message

Dodji Seketeli Feb. 4, 2011, 7:40 p.m. UTC 
Hello,

Consider the line commented #0 in the example below:

    template<class T>
    struct S0
    {
      typedef T type;
    };

    template<class... X>
    struct S1
    {
      typedef int I;
    };

    struct A
    {
      template<class...U, class V=typename S1<typename S0<U>::type...>::I> //#0
      A(U...u);
    };

    int
    main()
    {
      A a(1, 2);
    }

The class... U parameter pack gets fixed up into a parameter pack
U'. U' then gets substituted into the default argument of V. Normally
the substitution of U' into the pack expansion
typename S0<U>::type... should yield typename S0<U'>::type...

In this PR I believe the problem is that the substitution of that pack
expansion fails and wrongly yields "typename S0<U'>::type" which is
not a pack expansion. From there a cascade of confusing things
happens and G++ fails to compile the example.

The root cause of the failure of that pack expansion substitution is
that tsubst_pack_expansion compares U' with U in the last statement
of:

      if (arg_pack
          && TREE_VEC_LENGTH (ARGUMENT_PACK_ARGS (arg_pack)) == 1
          && PACK_EXPANSION_P (TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack), 0)))
        {
          tree expansion = TREE_VEC_ELT (ARGUMENT_PACK_ARGS (arg_pack), 0);
          tree pattern = PACK_EXPANSION_PATTERN (expansion);
          if ((TYPE_P (pattern) && same_type_p (pattern, parm_pack))
              || (!TYPE_P (pattern) && cp_tree_equal (parm_pack, pattern))) 


and that comparison fails. It fails because U [not being fixed up yet]
is set to have zero siblings, whereas U' [the fixed up counterpart
of U] has 2. Their number of sibling parameters is different so these
two types are considered different.

So the problem is that we fail to compare a type not yet fixed up with
its fixed up alter ego. I naively thought that during type
substitution, template parameters were only compared using their
position and level [like in tsubst for template the type parameter
case] so we wouldn't need to compare a non fixed up type with its
fixed up version. Alas.

What I am proposing in the patch below is to take in account the DECL
of the template parameter -- for litigious cases like this that only
happen during type fixup. That is, U and U' have different canonical
types but they do have the same DECL (i.e TYPE_NAME). So testing for
that DECL is a way to detect that U and U' are structurally
equivalent.

To respect the invariant of the type comparison system that says that
two types having two different non-NULL canonical types must be
structurally equivalent, I am setting the canonical type of U to
NULL. The type fixup process then assigns the proper canonical type to
U'.

The part of the patch that touches cp_tree_equal is just a
factorization to increase maintainability.

Tested on x86_64-unknown-linux-gnu against trunk.

Comments

Jason Merrill Feb. 5, 2011, 4:34 p.m. UTC | #1 
It seems to me that rather than try hard to make the pre-fixup parm 
compare equal to the post-fixup parm, we could just drop the 
same_type_p/cp_tree_equal check in tsubst_pack_expansion and clear 
arg_pack if the argument is an expansion of *any* parameter pack.

Jason
diff mbox

Patch

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 934dab8..c01682f 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -5048,6 +5048,7 @@  extern void do_decl_instantiation		(tree, tree);
 extern void do_type_instantiation		(tree, tree, tsubst_flags_t);
 extern bool always_instantiate_p		(tree);
 extern tree instantiate_decl			(tree, int, bool);
+extern bool comp_template_parms_index           (tree, tree);
 extern int comp_template_parms			(const_tree, const_tree);
 extern bool uses_parameter_packs                (tree);
 extern bool template_parameter_pack_p           (const_tree);
diff --git a/gcc/cp/pt.c b/gcc/cp/pt.c
index d59f32a..e15bba8 100644
--- a/gcc/cp/pt.c
+++ b/gcc/cp/pt.c
@@ -3581,7 +3581,14 @@  process_template_parm (tree list, location_t parm_loc, tree parm,
 				     num_template_parms,
 				     decl, TREE_TYPE (parm));
       TEMPLATE_TYPE_PARAMETER_PACK (t) = is_parameter_pack;
-      TYPE_CANONICAL (t) = canonical_type_parameter (t);
+      /* Let's require structural comparison for this if we don't yet
+	 know its number of sibling parms.  This way it can
+	 successfuly be compared to its fixed-up alter ego once fixup
+	 is complete.  */
+      if (num_template_parms > 0)
+	TYPE_CANONICAL (t) = canonical_type_parameter (t);
+      else
+	SET_TYPE_STRUCTURAL_EQUALITY (t);
     }
   DECL_ARTIFICIAL (decl) = 1;
   SET_DECL_TEMPLATE_PARM_P (decl);
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index d62d242..f378889 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -2176,13 +2176,7 @@  cp_tree_equal (tree t1, tree t2)
 				BASELINK_FUNCTIONS (t2)));
 
     case TEMPLATE_PARM_INDEX:
-      if (TEMPLATE_PARM_NUM_SIBLINGS (t1)
-	  != TEMPLATE_PARM_NUM_SIBLINGS (t2))
-	return false;
-      return (TEMPLATE_PARM_IDX (t1) == TEMPLATE_PARM_IDX (t2)
-	      && TEMPLATE_PARM_LEVEL (t1) == TEMPLATE_PARM_LEVEL (t2)
-	      && (TEMPLATE_PARM_PARAMETER_PACK (t1)
-		  == TEMPLATE_PARM_PARAMETER_PACK (t2))
+      return (comp_template_parms_index (t1, t2)
 	      && same_type_p (TREE_TYPE (TEMPLATE_PARM_DECL (t1)),
 			      TREE_TYPE (TEMPLATE_PARM_DECL (t2))));
 
diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c
index c062f0f..633a02d 100644
--- a/gcc/cp/typeck.c
+++ b/gcc/cp/typeck.c
@@ -1107,29 +1107,41 @@  comp_array_types (const_tree t1, const_tree t2, bool allow_redeclaration)
   return true;
 }
 
-/* Compare the relative position of T1 and T2 into their respective
-   template parameter list.
-   T1 and T2 must be template parameter types.
-   Return TRUE if T1 and T2 have the same position, FALSE otherwise.  */
+/* Compare the relative position of INDEX1 and INDEX2 into their
+   respective template parameter list.  INDEX1 and INDEX2 must be
+   template parameter indexes.  Return TRUE if INDEX1 and INDEX2 have
+   the same position, FALSE otherwise. Note that if INDEX1
+   (resp. INDEX2) is the fixed-up alter-ego of INDEX2 (resp. INDEX1)
+   this function returns TRUE. To learn more about type fixup see
+   fixup_template_parms and fixup_template_type_parm_type.  */
 
-static bool
-comp_template_parms_position (tree t1, tree t2)
+bool
+comp_template_parms_index (tree index1, tree index2)
 {
-  tree index1, index2;
-  gcc_assert (t1 && t2
-	      && TREE_CODE (t1) == TREE_CODE (t2)
-	      && (TREE_CODE (t1) == BOUND_TEMPLATE_TEMPLATE_PARM
-		  || TREE_CODE (t1) == TEMPLATE_TEMPLATE_PARM
-		  || TREE_CODE (t1) == TEMPLATE_TYPE_PARM));
-
-  index1 = TEMPLATE_TYPE_PARM_INDEX (TYPE_MAIN_VARIANT (t1));
-  index2 = TEMPLATE_TYPE_PARM_INDEX (TYPE_MAIN_VARIANT (t2));
-
-  /* If T1 and T2 belong to template parm lists of different size,
-     let's assume they are different.  */
+
+  gcc_assert (TREE_CODE (index1) == TEMPLATE_PARM_INDEX
+	      && TREE_CODE (index2) == TEMPLATE_PARM_INDEX);
+
+  /* If INDEX1 and INDEX2 belong to template parm lists of different size,
+     let's assume they are different, in the general cases.  */
   if (TEMPLATE_PARM_NUM_SIBLINGS (index1)
       != TEMPLATE_PARM_NUM_SIBLINGS (index2))
-    return false;
+    {
+      if (TEMPLATE_PARM_DECL (index1) == TEMPLATE_PARM_DECL (index2))
+	{
+	  /* If INDEX1 and INDEX2 have different numbers of siblings
+	     but have the same DECL, that must mean that they are
+	     actually the same type but one has been fixed up while
+	     the other has not. In that case, the one that hasn't been
+	     fixed up must have zero sibling. This is a rare case that
+	     might happen during template parm type fixup, i.e in a
+	     function called from fixup_template_parms.  */
+	  gcc_assert (TEMPLATE_PARM_NUM_SIBLINGS (index1) == 0
+		      || TEMPLATE_PARM_NUM_SIBLINGS (index2) == 0);
+	}
+      else
+	return false;
+    }
 
   /* Then compare their relative position.  */
   if (TEMPLATE_PARM_IDX (index1) != TEMPLATE_PARM_IDX (index2)
@@ -1221,7 +1233,8 @@  structural_comptypes (tree t1, tree t2, int strict)
 
     case TEMPLATE_TEMPLATE_PARM:
     case BOUND_TEMPLATE_TEMPLATE_PARM:
-      if (!comp_template_parms_position (t1, t2))
+      if (!comp_template_parms_index (TEMPLATE_TYPE_PARM_INDEX (t1),
+				      TEMPLATE_TYPE_PARM_INDEX (t2)))
 	return false;
       if (!comp_template_parms
 	  (DECL_TEMPLATE_PARMS (TEMPLATE_TEMPLATE_PARM_TEMPLATE_DECL (t1)),
@@ -1285,7 +1298,8 @@  structural_comptypes (tree t1, tree t2, int strict)
     case TEMPLATE_TYPE_PARM:
       /* If T1 and T2 don't have the same relative position in their
 	 template parameters set, they can't be equal.  */
-      if (!comp_template_parms_position (t1, t2))
+      if (!comp_template_parms_index (TEMPLATE_TYPE_PARM_INDEX (t1),
+				      TEMPLATE_TYPE_PARM_INDEX (t2)))
 	return false;
       break;
 
diff --git a/gcc/testsuite/g++.dg/template/typedef38.C b/gcc/testsuite/g++.dg/template/typedef38.C
new file mode 100644
index 0000000..1c76404
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/typedef38.C
@@ -0,0 +1,27 @@ 
+// Origin: PR c++/46394
+// { dg-options "-std=c++0x" }
+// { dg-do "compile" }
+
+template<class T>
+struct S0
+{
+  typedef T type;
+};
+
+template<class... X>
+struct S1
+{
+  typedef int I;
+};
+
+struct A
+{
+  template<class...U, class V=typename S1<typename S0<U>::type...>::I>
+  A(U...u);
+};
+
+int
+main()
+{
+  A a(1, 2);
+}