Message ID | 1446057972-21579-1-git-send-email-palves@redhat.com |
---|---|
State | New, archived |
Headers |
Received: (qmail 64507 invoked by alias); 28 Oct 2015 18:46:17 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: <gdb-patches.sourceware.org> List-Unsubscribe: <mailto:gdb-patches-unsubscribe-##L=##H@sourceware.org> List-Subscribe: <mailto:gdb-patches-subscribe@sourceware.org> List-Archive: <http://sourceware.org/ml/gdb-patches/> List-Post: <mailto:gdb-patches@sourceware.org> List-Help: <mailto:gdb-patches-help@sourceware.org>, <http://sourceware.org/ml/#faqs> Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 64494 invoked by uid 89); 28 Oct 2015 18:46:17 -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; Wed, 28 Oct 2015 18:46:15 +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 314BEAACEF; Wed, 28 Oct 2015 18:46:14 +0000 (UTC) Received: from brno.lan (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 t9SIkCcE021426; Wed, 28 Oct 2015 14:46:13 -0400 From: Pedro Alves <palves@redhat.com> To: gdb-patches@sourceware.org Cc: Simon Marchi <simon.marchi@polymtl.ca> Subject: [PATCH] gnu-v2-abi.c: Add casts Date: Wed, 28 Oct 2015 18:46:12 +0000 Message-Id: <1446057972-21579-1-git-send-email-palves@redhat.com> |
Commit Message
Pedro Alves
Oct. 28, 2015, 6:46 p.m. UTC
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 <palves@redhat.com> * 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(-)
Comments
On 28 October 2015 at 14:46, Pedro Alves <palves@redhat.com> 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 <palves@redhat.com> > > * 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, 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.
On 10/28/2015 08:50 PM, Simon Marchi wrote: > On 28 October 2015 at 14:46, Pedro Alves <palves@redhat.com> 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 <palves@redhat.com> >> >> * 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
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