From patchwork Fri Apr 29 00:18:56 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Paolo Carlini X-Patchwork-Id: 616542 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 3qwvT11HYbz9ssM for ; Fri, 29 Apr 2016 10:19:15 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=gcc.gnu.org header.i=@gcc.gnu.org header.b=YgiglMqb; dkim-atps=neutral DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; q=dns; s=default; b=tJQ4Oaq9esS4/24Xy woMILzv4CEt70ta3MCDplq77zgTIlmb6mXuhFxO7dSgQWeCuBPRKCSUTsQBkfGkj X5nqD3vMk95Dwlj33J9dep6wDbt2xm6eW2UA/MC9vA0rH2aHFOOE88H8Dt3Nms8c QBiU1HOdd0VeNQFCkckUKNlaAY= 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 :subject:to:references:from:message-id:date:mime-version :in-reply-to:content-type; s=default; bh=x/5UndqtAypcbKhSjgiT5P8 XWwA=; b=YgiglMqbWqWz/iF/g0de4jju55knCMU75BHQ6H29vDF9l2BpPtniS1j X4yOTphcLvQw+RGLfzkb9nVWoD9T5Peok4gLbQW97b18/0sHzT+WguKTtSjBDyXl mtQyuQwi2fnEZDBJvJYCg7deGN8S5kBWL/z0gYYxRxuKN4zE6CmY= Received: (qmail 13212 invoked by alias); 29 Apr 2016 00:19:07 -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 13162 invoked by uid 89); 29 Apr 2016 00:19:06 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.1 required=5.0 tests=BAYES_00, KAM_ASCII_DIVIDERS, RP_MATCHES_RCVD, SPF_PASS autolearn=ham version=3.3.2 spammy=139, 7 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-GCM-SHA384 encrypted) ESMTPS; Fri, 29 Apr 2016 00:19:04 +0000 Received: from aserv0022.oracle.com (aserv0022.oracle.com [141.146.126.234]) by userp1040.oracle.com (Sentrion-MTA-4.3.2/Sentrion-MTA-4.3.2) with ESMTP id u3T0J1xU032662 (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 29 Apr 2016 00:19:01 GMT Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by aserv0022.oracle.com (8.13.8/8.13.8) with ESMTP id u3T0J1h9021906 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 29 Apr 2016 00:19:01 GMT Received: from abhmp0008.oracle.com (abhmp0008.oracle.com [141.146.116.14]) by aserv0122.oracle.com (8.13.8/8.13.8) with ESMTP id u3T0IwOB021655; Fri, 29 Apr 2016 00:18:59 GMT Received: from [192.168.1.4] (/87.7.187.5) by default (Oracle Beehive Gateway v4.0) with ESMTP ; Thu, 28 Apr 2016 17:18:58 -0700 Subject: Re: [C++ Patch] PR 66644 To: Jason Merrill , "gcc-patches@gcc.gnu.org" References: <572267EF.1060200@oracle.com> <57228472.8090005@redhat.com> From: Paolo Carlini Message-ID: <5722A870.5040403@oracle.com> Date: Fri, 29 Apr 2016 02:18:56 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.0 MIME-Version: 1.0 In-Reply-To: <57228472.8090005@redhat.com> X-IsSubscribed: yes Hi Jason, On 28/04/2016 23:45, Jason Merrill wrote: > I would expect this to cause a false negative on a union of two > anonymous structs, both of which have initialized members. > > I think better would be to have a local any_default_members rather > than passing the same pointer through all levels. > > Also, you can look at 'type' rather than DECL_CONTEXT (field). . ... and thanks a lot for your very helpful reply. Now I realize that something was wrong in the general logic here, not a tiny detail in a conditional. Thus the below tries to closely follow your advice: the idea is that any_default_members as required by check_field_decls should still compute to the same value but the logic to emit the "multiple fields in union initialized" diagnostic in check_field_decl is different: it relies on the incoming any_default_members, not on what is computed and returned at that level. Thus we can reject, eg, your case of two anonymous struct. Also, for test5, which we already correctly rejected, we do not emit an additional redundant error, and that's more evidence that something bigger was wrong in check_field_decl. Anyway, the below passes testing. How does it look? Thanks, Paolo. ///////////////////// Index: cp/class.c =================================================================== --- cp/class.c (revision 235615) +++ cp/class.c (working copy) @@ -139,7 +139,7 @@ static int count_fields (tree); static int add_fields_to_record_type (tree, struct sorted_fields_type*, int); static void insert_into_classtype_sorted_fields (tree, tree, int); static bool check_bitfield_decl (tree); -static void check_field_decl (tree, tree, int *, int *, int *); +static bool check_field_decl (tree, tree, int *, int *, bool); static void check_field_decls (tree, tree *, int *, int *); static tree *build_base_field (record_layout_info, tree, splay_tree, tree *); static void build_base_fields (record_layout_info, splay_tree, tree *); @@ -3541,14 +3541,15 @@ check_bitfield_decl (tree field) enclosing type T. Issue any appropriate messages and set appropriate flags. */ -static void +static bool check_field_decl (tree field, tree t, int* cant_have_const_ctor, int* no_const_asn_ref, - int* any_default_members) + bool any_default_members) { tree type = strip_array_types (TREE_TYPE (field)); + bool any_default_members_field = false; /* In C++98 an anonymous union cannot contain any fields which would change the settings of CANT_HAVE_CONST_CTOR and friends. */ @@ -3558,12 +3559,13 @@ check_field_decl (tree field, structs. So, we recurse through their fields here. */ else if (ANON_AGGR_TYPE_P (type)) { - tree fields; - - for (fields = TYPE_FIELDS (type); fields; fields = DECL_CHAIN (fields)) + for (tree fields = TYPE_FIELDS (type); fields; + fields = DECL_CHAIN (fields)) if (TREE_CODE (fields) == FIELD_DECL && !DECL_C_BIT_FIELD (field)) - check_field_decl (fields, t, cant_have_const_ctor, - no_const_asn_ref, any_default_members); + any_default_members_field |= check_field_decl (fields, t, + cant_have_const_ctor, + no_const_asn_ref, + any_default_members); } /* Check members with class type for constructors, destructors, etc. */ @@ -3623,10 +3625,12 @@ check_field_decl (tree field, { /* `build_class_init_list' does not recognize non-FIELD_DECLs. */ - if (TREE_CODE (t) == UNION_TYPE && *any_default_members != 0) + if (TREE_CODE (t) == UNION_TYPE && any_default_members) error ("multiple fields in union %qT initialized", t); - *any_default_members = 1; + any_default_members_field = true; } + + return any_default_members_field; } /* Check the data members (both static and non-static), class-scoped @@ -3662,7 +3666,7 @@ check_field_decls (tree t, tree *access_decls, tree *field; tree *next; bool has_pointers; - int any_default_members; + bool any_default_members; int cant_pack = 0; int field_access = -1; @@ -3672,7 +3676,7 @@ check_field_decls (tree t, tree *access_decls, has_pointers = false; /* Assume none of the members of this class have default initializations. */ - any_default_members = 0; + any_default_members = false; for (field = &TYPE_FIELDS (t); *field; field = next) { @@ -3868,10 +3872,10 @@ check_field_decls (tree t, tree *access_decls, /* We set DECL_C_BIT_FIELD in grokbitfield. If the type and width are valid, we'll also set DECL_BIT_FIELD. */ if (! DECL_C_BIT_FIELD (x) || ! check_bitfield_decl (x)) - check_field_decl (x, t, - cant_have_const_ctor_p, - no_const_asn_ref_p, - &any_default_members); + any_default_members |= check_field_decl (x, t, + cant_have_const_ctor_p, + no_const_asn_ref_p, + any_default_members); /* Now that we've removed bit-field widths from DECL_INITIAL, anything left in DECL_INITIAL is an NSDMI that makes the class Index: testsuite/g++.dg/cpp0x/nsdmi-anon-struct1.C =================================================================== --- testsuite/g++.dg/cpp0x/nsdmi-anon-struct1.C (revision 0) +++ testsuite/g++.dg/cpp0x/nsdmi-anon-struct1.C (working copy) @@ -0,0 +1,48 @@ +// PR c++/66644 +// { dg-do compile { target c++11 } } +// { dg-options "-Wno-pedantic" } + +struct test1 +{ + union + { + struct { char a=0, b=0; }; + char buffer[16]; + }; +}; + +struct test2 +{ + union + { + struct { char a=0, b; }; + char buffer[16]; + }; +}; + +struct test3 +{ + union + { + struct { char a, b; } test2{0,0}; + char buffer[16]; + }; +}; + +struct test4 +{ + union + { // { dg-error "multiple fields" } + struct { char a=0, b=0; }; + struct { char c=0, d; }; + }; +}; + +struct test5 +{ + union + { + union { char a=0, b=0; }; // { dg-error "multiple fields" } + char buffer[16]; + }; +};