From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 67865 invoked by alias); 29 Oct 2015 17:31:49 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 66386 invoked by uid 89); 29 Oct 2015 17:31:49 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.9 required=5.0 tests=AWL,BAYES_00,RP_MATCHES_RCVD,SPF_HELO_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Thu, 29 Oct 2015 17:31:48 +0000 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) by mx1.redhat.com (Postfix) with ESMTPS id 05D8392466; Thu, 29 Oct 2015 17:31:47 +0000 (UTC) Received: from [127.0.0.1] (ovpn01.gateway.prod.ext.ams2.redhat.com [10.39.146.11]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t9THViUT011653; Thu, 29 Oct 2015 13:31:45 -0400 Message-ID: <56325800.3070907@redhat.com> Date: Thu, 29 Oct 2015 17:51:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-Version: 1.0 To: Simon Marchi CC: gdb-patches@sourceware.org Subject: Re: [PATCH] gnu-v2-abi.c: Add casts References: <1446057972-21579-1-git-send-email-palves@redhat.com> In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2015-10/txt/msg00720.txt.bz2 On 10/28/2015 08:50 PM, Simon Marchi wrote: > On 28 October 2015 at 14:46, Pedro Alves wrote: >> I looked at changing these is_destructor_name/is_constructor_name >> interfaces in order to detangle the boolean result from the ctor/dtor >> kind return, but then realized that this design goes all the way down >> to the libiberty demangler interfaces. E.g, include/demangle.h: >> >> ~~~ >> /* Return non-zero iff NAME is the mangled form of a constructor name >> in the G++ V3 ABI demangling style. Specifically, return an `enum >> gnu_v3_ctor_kinds' value indicating what kind of constructor >> it is. */ >> extern enum gnu_v3_ctor_kinds >> is_gnu_v3_mangled_ctor (const char *name); >> >> >> enum gnu_v3_dtor_kinds { >> gnu_v3_deleting_dtor = 1, >> gnu_v3_complete_object_dtor, >> gnu_v3_base_object_dtor, >> /* These are not part of the V3 ABI. Unified destructors are generated >> as a speed-for-space optimization when the -fdeclone-ctor-dtor option >> is used, and are always internal symbols. */ >> gnu_v3_unified_dtor, >> gnu_v3_object_dtor_group >> }; >> ~~~ >> >> libiberty/cp-demangle.c: >> >> ~~~ >> enum gnu_v3_ctor_kinds >> is_gnu_v3_mangled_ctor (const char *name) >> { >> enum gnu_v3_ctor_kinds ctor_kind; >> enum gnu_v3_dtor_kinds dtor_kind; >> >> if (! is_ctor_or_dtor (name, &ctor_kind, &dtor_kind)) >> return (enum gnu_v3_ctor_kinds) 0; >> return ctor_kind; >> } >> ~~~ >> >> etc. >> >> gdb/ChangeLog: >> 2015-10-27 Pedro Alves >> >> * gnu-v2-abi.c (gnuv2_is_destructor_name) >> (gnuv2_is_constructor_name): Add casts. >> --- >> gdb/gnu-v2-abi.c | 4 ++-- >> 1 file changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/gdb/gnu-v2-abi.c b/gdb/gnu-v2-abi.c >> index c508b55..6c2b92a 100644 >> --- a/gdb/gnu-v2-abi.c >> +++ b/gdb/gnu-v2-abi.c >> @@ -40,7 +40,7 @@ gnuv2_is_destructor_name (const char *name) >> || startswith (name, "__dt__")) >> return complete_object_dtor; >> else >> - return 0; >> + return (enum dtor_kinds) 0; >> } >> >> static enum ctor_kinds >> @@ -51,7 +51,7 @@ gnuv2_is_constructor_name (const char *name) >> || startswith (name, "__ct__")) >> return complete_object_ctor; >> else >> - return 0; >> + return (enum ctor_kinds) 0; >> } >> >> static int >> -- >> 1.9.3 >> > > I am ok with the cast, OK, thanks. I'm holding you to that. :-) I pushed the cast version in as is. > but just to note that we could do another way. > I think it would be safe to add a "not_a_ctor = 0" enum value to > ctor_kinds (which is defined in gdb), and return that in > gnuv2_is_constructor_name. Meanwhile, the function in libiberty for > gnu-abi-v3 (is_gnu_v3_mangled_ctor) will still return a casted 0, > which will map to not_a_ctor. > > Of course, the comment > > /* Kinds of constructors. All these values are guaranteed to be > non-zero. */ > > would need to change. Idem for destructors. Yeah. I'm not a big fan of the "not_really_a_thing" enum values. Feels like 'is_constructor_name (enum ctor_kind *kind_if_true);' would be a better API. And then ctor_kinds is by design compatible with the v3 enum as defined by libiberty, and libiberty does the same cast. (I should also note that I haven't found any place in gdb that actually cares about of different enum values of is_constructor_name, etc. Everywhere seems to care only about the boolean result.) Thanks, Pedro Alves