[v2] Fix sorting of enum values in FlagEnumerationPrinter
Commit Message
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 <class 'AttributeError'> '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 <type 'exceptions.AttributeError'> '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(-)
Comments
On 01/20/2016 06:10 PM, Simon Marchi wrote:
> 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.
You can use gdb_test_exact instead, which does the string_to_regexp
for you. OK with that change.
Thanks,
Pedro Alves
On 2016-01-20 13:20, Pedro Alves wrote:
> On 01/20/2016 06:10 PM, Simon Marchi wrote:
>
>> 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.
>
> You can use gdb_test_exact instead, which does the string_to_regexp
> for you. OK with that change.
Ok, pushed with that change.
Thanks.
@@ -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)
@@ -17,12 +17,20 @@
#include <string.h>
+
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;
@@ -119,14 +119,23 @@ gdb_test "print flt" " = x=<42> y=<43>" \
gdb_test "print ss" " = a=<a=<1> b=<$hex>> b=<a=<2> 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 | <unknown: 0x8>." \
- "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 | <unknown: 0x8>]}]