From patchwork Fri Dec 16 16:40:18 2011 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dodji Seketeli X-Patchwork-Id: 131865 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 4E9381007D7 for ; Sat, 17 Dec 2011 03:40:54 +1100 (EST) Received: (qmail 22974 invoked by alias); 16 Dec 2011 16:40:40 -0000 Received: (qmail 22696 invoked by uid 22791); 16 Dec 2011 16:40:34 -0000 X-SWARE-Spam-Status: No, hits=-4.8 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS 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, 16 Dec 2011 16:40:21 +0000 Received: from int-mx09.intmail.prod.int.phx2.redhat.com (int-mx09.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id pBGGeKuh021939 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Fri, 16 Dec 2011 11:40:21 -0500 Received: from localhost (ovpn-116-27.ams2.redhat.com [10.36.116.27]) by int-mx09.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id pBGGeJng028667 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Fri, 16 Dec 2011 11:40:20 -0500 Received: by localhost (Postfix, from userid 500) id 8E19229C13B; Fri, 16 Dec 2011 17:40:18 +0100 (CET) From: Dodji Seketeli To: Jason Merrill Cc: GCC Patches Subject: [PATCH] PR c++/51462 - ICE in cx_check_missing_mem_inits X-URL: http://www.redhat.com Date: Fri, 16 Dec 2011 17:40:18 +0100 Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.2 (gnu/linux) MIME-Version: 1.0 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, In the example of the patch below, the invalid initialization for the 'a' member in the context of the constructor of B (in expand_default_init) yields an error_mark_node, and rightfully so. But then expand_default_init doesn't generate any (expression) statement to represent that invalid initialization. So it's just like if no initialization was written by the user. Later, register_constexpr_fundef massages the body of the constexpr constructor B and calls cx_check_missing_mem_inits to see if there are some members that are missing initialization. As expand_default_init didn't generate any statement for the invalid initialization of 'a', the assert below, from cx_check_missing_mem_inits is violated[1]: if (type_has_constexpr_default_constructor (ftype)) { /* It's OK to skip a member with a trivial constexpr ctor. A constexpr ctor that isn't trivial should have been added in by now. */ gcc_checking_assert (!TYPE_HAS_COMPLEX_DFLT (ftype)); So I am thinking that maybe letting expand_default_init represent the invalid initialization would prevent us from getting into this case. For that to work I had to make build_constexpr_constructor_member_initializers try a little bit harder to represent invalid member initialization, as that code wasn't expecting to have e.g, an EXPR_STMT for a member initialization. If you think I am trying too hard, maybe I could just get out early from register_constexpr_fundef if errorcount is non-zero? Like, right before: if (DECL_CONSTRUCTOR_P (fun) && cx_check_missing_mem_inits (fun, body, !DECL_GENERATED_P (fun))) return NULL; ? The reason why I am not doing this straight away is that I am thinking that maybe G++ could be missing some diagnostic in that case. And more generally, I think that representing invalid input (as much as we can) in the semantic tree is consistent with what's done in the rest of the FE and leads to better diagnostics. FWIW, the patch below bootstraps and passes regression tests on x86_64-unknown-linux-gnu against trunk. [1]: By the way, I am just curious, why using gcc_checking_assert instead of just gcc_assert? Thanks. gcc/cp/ PR c++/51462 * init.c (expand_default_init): Pass even error_mark_node to finish_expr_stmt. * semantics.c (build_constexpr_constructor_member_initializers): call build_data_member_initialization even for invalid input. gcc/testsuite/ PR c++/51462 * g++.dg/cpp0x/constexpr-99.C: New test. --- gcc/cp/init.c | 7 ++++++- gcc/cp/semantics.c | 13 ++++++++++++- gcc/testsuite/g++.dg/cpp0x/constexpr-99.C | 13 +++++++++++++ 3 files changed, 31 insertions(+), 2 deletions(-) create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-99.C diff --git a/gcc/cp/init.c b/gcc/cp/init.c index e93e82c..c958c29 100644 --- a/gcc/cp/init.c +++ b/gcc/cp/init.c @@ -1671,7 +1671,12 @@ expand_default_init (tree binfo, tree true_exp, tree exp, tree init, int flags, } /* FIXME put back convert_to_void? */ - if (TREE_SIDE_EFFECTS (rval)) + if (TREE_SIDE_EFFECTS (rval) + /* If the initialization failed, let's generate an EXPR_STMT + containing error_mark_node, effectively telling other parts + of the compiler that there actually was an initialization, + even though it was invalid. */ + || rval == error_mark_node) finish_expr_stmt (rval); } diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 2f2a26a..a8ea996 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -5900,7 +5900,18 @@ build_constexpr_constructor_member_initializers (tree type, tree body) } } else - gcc_assert (errorcount > 0); + { + /* An error happened earlier, and some diagnostic has probably + been emitted for it already. */ + gcc_assert (errorcount > 0); + /* What else but an expression could we have at this point? */ + gcc_assert (TREE_CODE_CLASS (TREE_CODE (body)) == tcc_expression); + /* Let's build the data member initialization representation, + even if some initializers might actually contain + error_mark_node. */ + if (ok) + ok = build_data_member_initialization (body, &vec); + } if (ok) return build_constructor (type, vec); else diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-99.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-99.C new file mode 100644 index 0000000..13a5ea3 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-99.C @@ -0,0 +1,13 @@ +// Origin PR c++/51462 +// { dg-options "-std=c++11" } + +struct A +{ + int i = 0; +}; + +struct B +{ + A a; + constexpr B() : a(0) {} // { dg-error "no matching function" } +};