From patchwork Fri Feb 4 19:40:58 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 81941 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id 25BA3B7131 for ; Sat, 5 Feb 2011 06:41:15 +1100 (EST) Received: (qmail 18819 invoked by alias); 4 Feb 2011 19:41:12 -0000 Received: (qmail 18790 invoked by uid 22791); 4 Feb 2011 19:41:10 -0000 X-SWARE-Spam-Status: No, hits=-5.6 required=5.0 tests=AWL, BAYES_00, RCVD_IN_DNSWL_HI, SPF_HELO_PASS, T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Fri, 04 Feb 2011 19:41:03 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p14Jf1Xv022619 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 4 Feb 2011 14:41:01 -0500 Received: from adjoa.redhat.com (ovpn-113-98.phx2.redhat.com [10.3.113.98]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id p14JexoP021539; Fri, 4 Feb 2011 14:41:00 -0500 From: Dodji Seketeli To: Jason Merrill Cc: GCC Patches Subject: [PATCH] Fix PR c++/46394 X-URL: http://www.redhat.com Date: Fri, 04 Feb 2011 20:40:58 +0100 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org Hello, Consider the line commented #0 in the example below: template struct S0 { typedef T type; }; template struct S1 { typedef int I; }; struct A { template::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::type... should yield typename S0::type... In this PR I believe the problem is that the substitution of that pack expansion fails and wrongly yields "typename S0::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. 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 +struct S0 +{ + typedef T type; +}; + +template +struct S1 +{ + typedef int I; +}; + +struct A +{ + template::type...>::I> + A(U...u); +}; + +int +main() +{ + A a(1, 2); +}