From patchwork Wed Jan 20 18:03:44 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Simon Marchi X-Patchwork-Id: 10486 Received: (qmail 33466 invoked by alias); 20 Jan 2016 18:03:50 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Delivered-To: mailing list gdb-patches@sourceware.org Received: (qmail 33448 invoked by uid 89); 20 Jan 2016 18:03:49 -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:03:48 +0000 Received: by simark.ca (Postfix, from userid 112) id 1DAC91E6C2; Wed, 20 Jan 2016 13:03:46 -0500 (EST) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id 0D3161E6BF; Wed, 20 Jan 2016 13:03:44 -0500 (EST) MIME-Version: 1.0 Date: Wed, 20 Jan 2016 13:03:44 -0500 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: <569F9C9A.7090307@redhat.com> References: <1453177390-13881-1-git-send-email-simon.marchi@polymtl.ca> <569E17C5.6080909@redhat.com> <35bba9e534e14532c11ad7c0a5c1db2b@simark.ca> <569F9C9A.7090307@redhat.com> Message-ID: <54be6c5227572a85b0df4f081ec3900e@simark.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.1.3 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(-) + [string_to_regexp { = 0xc [FOO_3 | ]}] 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)" \