From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15061 invoked by alias); 11 Jan 2012 20:59:42 -0000 Received: (qmail 15052 invoked by uid 22791); 11 Jan 2012 20:59:41 -0000 X-SWARE-Spam-Status: No, hits=-6.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_HI,SPF_HELO_PASS,T_RP_MATCHES_RCVD 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; Wed, 11 Jan 2012 20:59:27 +0000 Received: from int-mx01.intmail.prod.int.phx2.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id q0BKxRKq009613 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 11 Jan 2012 15:59:27 -0500 Received: from host2.jankratochvil.net (ovpn-116-21.ams2.redhat.com [10.36.116.21]) by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id q0BKxLff012612 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NO); Wed, 11 Jan 2012 15:59:24 -0500 Date: Wed, 11 Jan 2012 21:03:00 -0000 From: Jan Kratochvil To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: RFC: enum pretty-printing support Message-ID: <20120111205920.GA7967@host2.jankratochvil.net> References: <20120110211713.GA1550@host2.jankratochvil.net> <20120110222424.GA4613@host2.jankratochvil.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) X-IsSubscribed: yes 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 X-SW-Source: 2012-01/txt/msg00370.txt.bz2 Hi Tom, On Wed, 11 Jan 2012 17:45:16 +0100, Tom Tromey wrote: > Tom> I think the C code has to be a bit more careful because it > Tom> cannot readily be turned off. > Tom> > Tom> One idea is to have both. > > Jan> I agree. > > Here's a new patch. Let me know what you think. That's great now, thanks. FYI not reviewed the Python part. > --- a/gdb/NEWS > +++ b/gdb/NEWS > @@ -9,6 +9,18 @@ > * The binary "gdbtui" can no longer be built or installed. > Use "gdb -tui" instead. > > +* GDB will now print "flag" enums specially. A flag enum is one where > + all the enumerator values have no bits in common when pairwise > + "and"ed. When printing a value whose type is a flag enum, GDB will > + show all the constants, e.g., for enum E { ONE = 1, TWO = 2}: > + (gdb) print (enum E) 3 > + $1 = [ONE | TWO] I do not much understand - why not to make it C syntax compliant? $1 = (ONE | TWO) > + > +* Python scripting > + > + ** A new class, gdb.printing.FlagEnumerationPrinter, can be used to > + apply "flag enum"-style pretty-printing to any enum. > + > *** Changes in GDB 7.4 > > * GDB now handles ambiguous linespecs more consistently; the existing > diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c > index 9949015..1d44e90 100644 > --- a/gdb/c-valprint.c > +++ b/gdb/c-valprint.c > @@ -458,7 +458,38 @@ c_val_print (struct type *type, const gdb_byte *valaddr, > } > else > { > - print_longest (stream, 'd', 0, val); > + if (TYPE_FLAG_ENUM (type)) Some comment here what this new block does, please. This "else if" can be indented left - one needless block/indentation here. > + { > + int first = 1; > + > + fputs_filtered ("[", stream); > + for (i = 0; i < len; ++i) > + { > + QUIT; > + > + if ((val & TYPE_FIELD_BITPOS (type, i)) != 0) > + { > + if (!first) > + fputs_filtered (" | ", stream); > + first = 0; > + > + val &= ~ TYPE_FIELD_BITPOS (type, i); ^ Extra space, not GDB coding style compliant. > + fputs_filtered (TYPE_FIELD_NAME (type, i), stream); > + } > + } > + > + if (first || val != 0) > + { > + if (!first) > + fputs_filtered (" | ", stream); > + fputs_filtered ("unknown: ", stream); > + print_longest (stream, 'd', 0, val); > + } > + > + fputs_filtered ("]", stream); > + } > + else > + print_longest (stream, 'd', 0, val); > } > break; > [...] > diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h > index ddad0dc..f152945 100644 > --- a/gdb/gdbtypes.h > +++ b/gdb/gdbtypes.h > @@ -290,6 +290,12 @@ enum type_instance_flag_value > > #define TYPE_DECLARED_CLASS(t) (TYPE_MAIN_TYPE (t)->flag_declared_class) > > +/* True if this type is a "flag" enum. A flag enum is one where all > + the values are pairwise disjoint when "and"ed together. This > + affects how enum values are printed. */ > + > +#define TYPE_FLAG_ENUM(t) (TYPE_MAIN_TYPE (t)->flag_flag_enum) > + > /* Constant type. If this is set, the corresponding type has a > const modifier. */ > > @@ -399,6 +405,9 @@ struct main_type > /* True if this type was declared with "class" rather than > "struct". */ > unsigned int flag_declared_class : 1; Empty line (OK, flag_declared_class is also wrong wrt this). > + /* True if this is an enum type with disjoint values. This affects > + how the enum is printed. */ > + unsigned int flag_flag_enum : 1; > > /* A discriminant telling us which field of the type_specific union > is being used for this type, if any. */ [...] > --- a/gdb/testsuite/gdb.python/py-pp-maint.exp > +++ b/gdb/testsuite/gdb.python/py-pp-maint.exp [...] > @@ -127,3 +132,18 @@ gdb_test "print flt" " = x=<42> y=<43>" \ > > gdb_test "print ss" " = a= b=<$hex>> b= b=<$hex>>" \ > "print ss re-enabled" > + > +gdb_test "print (enum flag_enum) (FLAG_1)" \ > + " = 0x1 .FLAG_1." \ > + "print FLAG_1" > + > +gdb_test "print (enum flag_enum) (FLAG_1 | FLAG_3)" \ > + " = 0x5 .FLAG_1 | FLAG_3." \ > + "print FLAG_1 | FLAG_3" > + > +gdb_test "print (enum flag_enum) (4 + 8)" \ > + " = 0xc .FLAG_1 | ." \ > + "print FLAG_1 | 8" > + > + > + 3 extra empty lines. Thanks, Jan