From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 33741 invoked by alias); 20 Jan 2016 18:12:20 -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 33727 invoked by uid 89); 20 Jan 2016 18:12:19 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.2 required=5.0 tests=BAYES_00,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=no version=3.3.2 spammy=sk:string_, sk:excepti, 11914, brackets X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 20 Jan 2016 18:12:18 +0000 Received: by simark.ca (Postfix, from userid 112) id 05C5B1E6C2; Wed, 20 Jan 2016 13:12:17 -0500 (EST) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id E5A331E6BF; Wed, 20 Jan 2016 13:12:14 -0500 (EST) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Wed, 20 Jan 2016 18:12:00 -0000 From: Simon Marchi To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 1/2] Fix sorting of enum values in FlagEnumerationPrinter In-Reply-To: <54be6c5227572a85b0df4f081ec3900e@simark.ca> References: <1453177390-13881-1-git-send-email-simon.marchi@polymtl.ca> <569E17C5.6080909@redhat.com> <35bba9e534e14532c11ad7c0a5c1db2b@simark.ca> <569F9C9A.7090307@redhat.com> <54be6c5227572a85b0df4f081ec3900e@simark.ca> Message-ID: <9b688a880ac187ab99d50ef790fe7c54@simark.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.1.3 X-SW-Source: 2016-01/txt/msg00486.txt.bz2 On 2016-01-20 13:03, Simon Marchi wrote: > On 2016-01-20 09:41, Pedro Alves wrote: >> OK, I think that makes sense for cases like: >> >> enum flag_enum >> { >> FOO_MASK = 0x07, >> FOO_1 = 0x01, >> FOO_2 = 0x02, >> FOO_3 = 0x04, >> >> BAR_MASK = 0x70, >> BAR_1 = 0x10, >> BAR_2 = 0x20, >> BAR_3 = x040, >> }; >> >> Would you mind augmenting the testsuite with something >> like this, then? >> >> Thanks, >> Pedro Alves > > Here is a v2: > > > From 5d7a3227fa50594c1f5541550a07481583e027df Mon Sep 17 00:00:00 2001 > From: Simon Marchi > Date: Mon, 18 Jan 2016 21:35:18 -0500 > Subject: [PATCH] Fix sorting of enum values in FlagEnumerationPrinter > > The lambda function used to sort the enumerator list does not work > properly. This list consists of tuples, (enum label, enum value). The > key function returns x.enumval. enumval not being defined for a tuple, > we see this exception in the test log: > > Python Exception 'tuple' object has no > attribute 'enumval' > > The function should return the second item of the tuple, which is the > enumval. > > The pretty-printer still worked mostly correctly, except that the > enumeration values were not sorted. The test still passed because the > enumeration values are already sorted where they are defined. The test > also passed despite the exception being printed, because the right > output > was printed after the exception: > > print (enum flag_enum) (FLAG_1) > Python Exception 'tuple' > objecthas no attribute 'enumval':M > $7 = 0x1 [FLAG_1] > (gdb) PASS: gdb.python/py-pp-maint.exp: print FLAG_1 > > New in v2: > > - Improved test case, I stole Pedro's example directly. It verifies > that the sorting of enumerators by value works, by checking that > printing FOO_MASK appears as FOO_1 | FOO_2 | FOO_3. > > I noticed that I could change the regexps to almost anything and the > tests would still pass. I think it was because of the | in there. I > made them more robust by using string_to_regexp. I used curly braces > { } instead of quoting marks " " for strings, so that I could use > square brackets [ ] in them without having to escape them all. I > also > removed the "message" part of the tests, since they are redundant > with > the command, and it's just more maintenance to have to update them. > > Tested with Python 2.7 and 3.5. > > gdb/ChangeLog: > > * python/lib/gdb/printing.py (FlagEnumerationPrinter.__call__): > Fix enumerators sort key function. > > gdb/testsuite/ChangeLog: > > * gdb.python/py-pp-maint.exp: Change/add enum flag tests. > * gdb.python/py-pp-maint.c (enum flag_enum): Use more complex > enum flag values. > --- > gdb/python/lib/gdb/printing.py | 2 +- > gdb/testsuite/gdb.python/py-pp-maint.c | 16 ++++++++++++---- > gdb/testsuite/gdb.python/py-pp-maint.exp | 27 > ++++++++++++++++++--------- > 3 files changed, 31 insertions(+), 14 deletions(-) > > diff --git a/gdb/python/lib/gdb/printing.py > b/gdb/python/lib/gdb/printing.py > index 5160581..63c3aeb 100644 > --- a/gdb/python/lib/gdb/printing.py > +++ b/gdb/python/lib/gdb/printing.py > @@ -263,7 +263,7 @@ class FlagEnumerationPrinter(PrettyPrinter): > self.enumerators.append((field.name, field.enumval)) > # Sorting the enumerators by value usually does the right > # thing. > - self.enumerators.sort(key = lambda x: x.enumval) > + self.enumerators.sort(key = lambda x: x[1]) > > if self.enabled: > return _EnumInstance(self.enumerators, val) > diff --git a/gdb/testsuite/gdb.python/py-pp-maint.c > b/gdb/testsuite/gdb.python/py-pp-maint.c > index 657dfd7..d750496 100644 > --- a/gdb/testsuite/gdb.python/py-pp-maint.c > +++ b/gdb/testsuite/gdb.python/py-pp-maint.c > @@ -17,12 +17,20 @@ > > #include > > + > enum flag_enum > { > - FLAG_1 = 1, > - FLAG_2 = 2, > - FLAG_3 = 4, > - ALL = FLAG_1 | FLAG_2 | FLAG_3 > + /* Define the enumeration values in an unsorted manner to verify > that we > + effectively sort them by value. */ > + FOO_MASK = 0x07, > + FOO_1 = 0x01, > + FOO_2 = 0x02, > + FOO_3 = 0x04, > + > + BAR_MASK = 0x70, > + BAR_1 = 0x10, > + BAR_2 = 0x20, > + BAR_3 = 0x40, > }; > > enum flag_enum fval; > diff --git a/gdb/testsuite/gdb.python/py-pp-maint.exp > b/gdb/testsuite/gdb.python/py-pp-maint.exp > index db0768f..9dbe19f 100644 > --- a/gdb/testsuite/gdb.python/py-pp-maint.exp > +++ b/gdb/testsuite/gdb.python/py-pp-maint.exp > @@ -119,14 +119,23 @@ 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) (FOO_1)" \ > + [string_to_regexp { = 0x1 [FOO_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) (BAR_3)" \ > + [string_to_regexp { = 0x40 [BAR_3]}] > > -gdb_test "print (enum flag_enum) (4 + 8)" \ > - " = 0xc .FLAG_1 | ." \ > - "print FLAG_1 | 8" > +gdb_test "print (enum flag_enum) (BAR_2 | FOO_2)" \ > + [string_to_regexp { = 0x22 [FOO_2 | BAR_2]}] > + > +gdb_test "print (enum flag_enum) (FOO_1 | FOO_2 | FOO_3)" \ > + [string_to_regexp { = 0x7 [FOO_1 | FOO_2 | FOO_3]}] > + > +gdb_test "print (enum flag_enum) (FOO_MASK)" \ > + [string_to_regexp { = 0x7 [FOO_1 | FOO_2 | FOO_3]}] > + > +gdb_test "print (enum flag_enum) (FOO_MASK | (BAR_MASK & ~BAR_2))" \ > + [string_to_regexp { = 0x57 [FOO_1 | FOO_2 | FOO_3 | BAR_1 | > BAR_3]}] > + > +gdb_test "print (enum flag_enum) (0x4 + 0x8)" \ > + [string_to_regexp { = 0xc [FOO_3 | ]}] I tried to apply my patch from here, and it says it's corrupt (I was using a web mail client). Please look at this version sent with git send-email instead. https://sourceware.org/ml/gdb-patches/2016-01/msg00485.html Thanks, Simon