From patchwork Tue Apr 8 11:06:38 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Carlini X-Patchwork-Id: 337622 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id D99B51400BB for ; Tue, 8 Apr 2014 21:08:12 +1000 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:content-type; q=dns; s=default; b=nS1Wa0TAdpxLGmv46+t5gq8+pHStXFoN9suMbjNal3Z 7CaOKL763lbyfy+r9DDa3iuKwCLEO+NuBXSwW3uoGITCNEbJdpuy2NHvgLZfXh4U pYXWODCzUwmsOUQ8aij2p/xacXsx483iwab9DDlw+7VN6qLNNbZoG9Z0YAN4oPFk = DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:content-type; s=default; bh=nb5TYmjGAGwo2+B4/PThAWc5lXU=; b=eDNrukdV3f3+3gEFa YG43saVoB0kc4GGpyqKHYybV0GSiYyAvA+Nqsd7NqUuW05H2lHfTZ7bH6hyIy3ts HJA4J+DFvZXNfBiD+4ydPUttLBtHTsaFj3W7NMnuB/itWezQoON8BBunEZKNJ8oB 3IJz+kLQpyGNJcMcMHuos2bbjg= Received: (qmail 29574 invoked by alias); 8 Apr 2014 11:08:05 -0000 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 Received: (qmail 29563 invoked by uid 89); 8 Apr 2014 11:08:04 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.5 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 X-HELO: userp1040.oracle.com Received: from userp1040.oracle.com (HELO userp1040.oracle.com) (156.151.31.81) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Tue, 08 Apr 2014 11:08:02 +0000 Received: from acsinet22.oracle.com (acsinet22.oracle.com [141.146.126.238]) by userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id s38B7xvL013343 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Tue, 8 Apr 2014 11:08:00 GMT Received: from userz7021.oracle.com (userz7021.oracle.com [156.151.31.85]) by acsinet22.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id s38B7xa8028270 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=FAIL); Tue, 8 Apr 2014 11:07:59 GMT Received: from abhmp0010.oracle.com (abhmp0010.oracle.com [141.146.116.16]) by userz7021.oracle.com (8.14.4+Sun/8.14.4) with ESMTP id s38B7woD013933; Tue, 8 Apr 2014 11:07:58 GMT Received: from [192.168.1.4] (/79.36.197.244) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Tue, 08 Apr 2014 04:07:58 -0700 Message-ID: <5343D83E.3010700@oracle.com> Date: Tue, 08 Apr 2014 13:06:38 +0200 From: Paolo Carlini User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 MIME-Version: 1.0 To: "gcc-patches@gcc.gnu.org" CC: Jason Merrill Subject: [C++ Patch/RFC] PR 59115 X-IsSubscribed: yes Hi, I have been working on this issue, a minor regression, looking for the safest fix. What I have got so far seems neat enough to me, but maybe not totally safe (from the diagnostic quality point of view only, of course). The ICE happens in unify, in: /* Check for mixed types and values. */ if ((TREE_CODE (parm) == TEMPLATE_TYPE_PARM && TREE_CODE (tparm) != TYPE_DECL) || (TREE_CODE (parm) == TEMPLATE_TEMPLATE_PARM && TREE_CODE (tparm) != TEMPLATE_DECL)) gcc_unreachable (); because, after the meaningful error message about the float non-type template parameter, unify wrongly handles, for parm == U, tparm == int. Obviously something is wrong with the indexing. This is because process_template_parm, upon error, early returns a chainon (list, err_parm_list), where TREE_VALUE (err_parm_list) == error_mark_node, thus no index information (build_template_parm_index is not used at all). This means that, afterward, when process_template_parm processes the next template parameters, the code at the beginning isn't really able to figure out the index of tree_last (list) and all the following template parameters have it off by one (*). Thus my idea of returning to the changes in r116473, which included the addition of the early returns, to not append simply an error_mark_node as parm, but the actual parm with its TREE_TYPE replaced by an error_mark_node. This way the indexes are computed normally and unify doesn't get confused. Besides that, I went through the rest of r116473 and made sure we use error_operand_p, to catch the TREE_TYPEs (I also added 3/4 more besides those in that commit, which seemed obviously correct). What do you think? Tested x86_64-linux. Thanks, Paolo. (*) Interestingly, replacing the block at beginning of process_template_parm with something simply using list_length would **almost** work: the only regressions are -std=c++1y, due to the calls by synthesize_implicit_template_parm, which tracks the end of the template parameters list for compile-time performance reasons, thus list_length doesn't match the actual length of the whole list as stored as an index in tree_last. Looking forward, we should probably consider handling consistently both real and synthesized parameters. ////////////////// Index: cp/pt.c =================================================================== --- cp/pt.c (revision 209212) +++ cp/pt.c (working copy) @@ -414,7 +414,7 @@ push_inline_template_parms_recursive (tree parmlis { tree parm = TREE_VALUE (TREE_VEC_ELT (parms, i)); - if (parm == error_mark_node) + if (error_operand_p (parm)) continue; gcc_assert (DECL_P (parm)); @@ -2829,7 +2829,7 @@ comp_template_parms (const_tree parms1, const_tree /* If either of the template parameters are invalid, assume they match for the sake of error recovery. */ - if (parm1 == error_mark_node || parm2 == error_mark_node) + if (error_operand_p (parm1) || error_operand_p (parm2)) return 1; if (TREE_CODE (parm1) != TREE_CODE (parm2)) @@ -3640,11 +3640,7 @@ reduce_template_parm_level (tree index, tree type, to the LIST being built. This new parameter is a non-type parameter iff IS_NON_TYPE is true. This new parameter is a parameter pack iff IS_PARAMETER_PACK is true. The location of PARM - is in PARM_LOC. NUM_TEMPLATE_PARMS is the size of the template - parameter list PARM belongs to. This is used used to create a - proper canonical type for the type of PARM that is to be created, - iff PARM is a type. If the size is not known, this parameter shall - be set to 0. */ + is in PARM_LOC. */ tree process_template_parm (tree list, location_t parm_loc, tree parm, @@ -3652,7 +3648,6 @@ process_template_parm (tree list, location_t parm_ { tree decl = 0; tree defval; - tree err_parm_list; int idx = 0; gcc_assert (TREE_CODE (parm) == TREE_LIST); @@ -3673,8 +3668,6 @@ process_template_parm (tree list, location_t parm_ ++idx; } - else - idx = 0; if (is_non_type) { @@ -3682,39 +3675,29 @@ process_template_parm (tree list, location_t parm_ SET_DECL_TEMPLATE_PARM_P (parm); - if (TREE_TYPE (parm) == error_mark_node) - { - err_parm_list = build_tree_list (defval, parm); - TREE_VALUE (err_parm_list) = error_mark_node; - return chainon (list, err_parm_list); - } - else - { - /* [temp.param] + if (TREE_TYPE (parm) != error_mark_node) + { + /* [temp.param] - The top-level cv-qualifiers on the template-parameter are - ignored when determining its type. */ - TREE_TYPE (parm) = TYPE_MAIN_VARIANT (TREE_TYPE (parm)); - if (invalid_nontype_parm_type_p (TREE_TYPE (parm), 1)) - { - err_parm_list = build_tree_list (defval, parm); - TREE_VALUE (err_parm_list) = error_mark_node; - return chainon (list, err_parm_list); - } + The top-level cv-qualifiers on the template-parameter are + ignored when determining its type. */ + TREE_TYPE (parm) = TYPE_MAIN_VARIANT (TREE_TYPE (parm)); + if (invalid_nontype_parm_type_p (TREE_TYPE (parm), 1)) + TREE_TYPE (parm) = error_mark_node; + else if (uses_parameter_packs (TREE_TYPE (parm)) + && !is_parameter_pack + /* If we're in a nested template parameter list, the template + template parameter could be a parameter pack. */ + && processing_template_parmlist == 1) + { + /* This template parameter is not a parameter pack, but it + should be. Complain about "bare" parameter packs. */ + check_for_bare_parameter_packs (TREE_TYPE (parm)); - if (uses_parameter_packs (TREE_TYPE (parm)) && !is_parameter_pack - /* If we're in a nested template parameter list, the template - template parameter could be a parameter pack. */ - && processing_template_parmlist == 1) - { - /* This template parameter is not a parameter pack, but it - should be. Complain about "bare" parameter packs. */ - check_for_bare_parameter_packs (TREE_TYPE (parm)); - - /* Recover by calling this a parameter pack. */ - is_parameter_pack = true; - } - } + /* Recover by calling this a parameter pack. */ + is_parameter_pack = true; + } + } /* A template parameter is not modifiable. */ TREE_CONSTANT (parm) = 1; @@ -5127,7 +5110,7 @@ redeclare_class_template (tree type, tree parms) continue; tmpl_parm = TREE_VALUE (TREE_VEC_ELT (tmpl_parms, i)); - if (tmpl_parm == error_mark_node) + if (error_operand_p (tmpl_parm)) return false; parm = TREE_VALUE (TREE_VEC_ELT (parms, i)); @@ -6087,8 +6070,8 @@ coerce_template_template_parm (tree parm, tree in_decl, tree outer_args) { - if (arg == NULL_TREE || arg == error_mark_node - || parm == NULL_TREE || parm == error_mark_node) + if (arg == NULL_TREE || error_operand_p (arg) + || parm == NULL_TREE || error_operand_p (parm)) return 0; if (TREE_CODE (arg) != TREE_CODE (parm)) @@ -6181,7 +6164,7 @@ coerce_template_template_parms (tree parm_parms, { parm = TREE_VALUE (TREE_VEC_ELT (parm_parms, nparms - 1)); - if (parm == error_mark_node) + if (error_operand_p (parm)) return 0; switch (TREE_CODE (parm)) @@ -17517,7 +17500,7 @@ unify (tree tparms, tree targs, tree parm, tree ar case TEMPLATE_TEMPLATE_PARM: case BOUND_TEMPLATE_TEMPLATE_PARM: tparm = TREE_VALUE (TREE_VEC_ELT (tparms, 0)); - if (tparm == error_mark_node) + if (error_operand_p (tparm)) return unify_invalid (explain_p); if (TEMPLATE_TYPE_LEVEL (parm) @@ -17535,7 +17518,7 @@ unify (tree tparms, tree targs, tree parm, tree ar idx = TEMPLATE_TYPE_IDX (parm); targ = TREE_VEC_ELT (INNERMOST_TEMPLATE_ARGS (targs), idx); tparm = TREE_VALUE (TREE_VEC_ELT (tparms, idx)); - if (tparm == error_mark_node) + if (error_operand_p (tparm)) return unify_invalid (explain_p); /* Check for mixed types and values. */ @@ -17718,7 +17701,7 @@ unify (tree tparms, tree targs, tree parm, tree ar case TEMPLATE_PARM_INDEX: tparm = TREE_VALUE (TREE_VEC_ELT (tparms, 0)); - if (tparm == error_mark_node) + if (error_operand_p (tparm)) return unify_invalid (explain_p); if (TEMPLATE_PARM_LEVEL (parm) Index: testsuite/g++.dg/template/crash119.C =================================================================== --- testsuite/g++.dg/template/crash119.C (revision 0) +++ testsuite/g++.dg/template/crash119.C (working copy) @@ -0,0 +1,8 @@ +// PR c++/59115 + +template void foo(T, U) {} // { dg-error "valid type" } + +void bar() +{ + foo(0, 0); // { dg-error "matching" } +}