[4/5] gdb: print unknown part of flag enum in hex

  When we print the "unknown" part of a flag enum, it is printed in
decimal.  I think it would be more useful if it was printed in hex, as
it helps to determine which bits are set more than a decimal value.

gdb/ChangeLog:

	* valprint.c (generic_val_print_enum_1): Print unknown part of
	flag enum in hex.

gdb/testsuite/ChangeLog:

	* gdb.base/printcmds.exp (test_print_enums): Expect hex values
	for "unknown".
---
 gdb/testsuite/gdb.base/printcmds.exp | 4 ++--
 gdb/valprint.c                       | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)
  

On 2/13/20 5:30 PM, Simon Marchi wrote:
> When we print the "unknown" part of a flag enum, it is printed in
> decimal.  I think it would be more useful if it was printed in hex, as
> it helps to determine which bits are set more than a decimal value.
> 

Would it be better to mention the offending bit position explicitly? The 
hex value could be displayed along with it as well.

I mean, in both decimal and hex you'd need to do some calculations to 
figure out the bit position anyway. Might as well make it easier for 
developers by displaying the information.

> gdb/ChangeLog:
> 
> 	* valprint.c (generic_val_print_enum_1): Print unknown part of
> 	flag enum in hex.
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/printcmds.exp (test_print_enums): Expect hex values
> 	for "unknown".
> ---
>   gdb/testsuite/gdb.base/printcmds.exp | 4 ++--
>   gdb/valprint.c                       | 4 ++--
>   2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
> index 6afb965af066..d6f5c75650bf 100644
> --- a/gdb/testsuite/gdb.base/printcmds.exp
> +++ b/gdb/testsuite/gdb.base/printcmds.exp
> @@ -743,10 +743,10 @@ proc test_print_enums {} {
>       gdb_test "print (enum flag_enum) 0x0" [string_to_regexp " = FE_NONE"]
>   
>       # Print a flag enum with value 0, where no enumerator has value 0.
> -    gdb_test "print flag_enum_without_zero" [string_to_regexp " = (unknown: 0)"]
> +    gdb_test "print flag_enum_without_zero" [string_to_regexp " = (unknown: 0x0)"]
>   
>       # Print a flag enum with unknown bits set.
> -    gdb_test "print (enum flag_enum) 0xf1" [string_to_regexp " = (FE_ONE | unknown: 240)"]
> +    gdb_test "print (enum flag_enum) 0xf1" [string_to_regexp " = (FE_ONE | unknown: 0xf0)"]
>   
>       # Test printing an enum not considered a "flag enum" (because one of its
>       # enumerators has multiple bits set).
> diff --git a/gdb/valprint.c b/gdb/valprint.c
> index 77b9a4993d79..bd21be69e1bf 100644
> --- a/gdb/valprint.c
> +++ b/gdb/valprint.c
> @@ -659,8 +659,8 @@ generic_val_print_enum_1 (struct type *type, LONGEST val,
>   	{
>   	  if (!first)
>   	    fputs_filtered (" | ", stream);
> -	  fputs_filtered ("unknown: ", stream);
> -	  print_longest (stream, 'd', 0, val);
> +	  fputs_filtered ("unknown: 0x", stream);
> +	  print_longest (stream, 'x', 0, val);
>   	}
>   
>         fputs_filtered (")", stream);
>
  

On 2020-02-17 6:04 a.m., Luis Machado wrote:
> On 2/13/20 5:30 PM, Simon Marchi wrote:
>> When we print the "unknown" part of a flag enum, it is printed in
>> decimal.  I think it would be more useful if it was printed in hex, as
>> it helps to determine which bits are set more than a decimal value.
>>
> 
> Would it be better to mention the offending bit position explicitly? The hex value could be displayed along with it as well.
> 
> I mean, in both decimal and hex you'd need to do some calculations to figure out the bit position anyway. Might as well make it easier for developers by displaying the information.

It's much easier to convert between hex and bit positions than decimal and
bit positions, so I wouldn't put decimal and hex on the same level here.

Also, there could be many bits that are set and unknown, so if we mention them
explicitly, it could take a lot of space and become actually less readable.  I
think that the hex notation is the most convenient, it's compact and people are
used to converting between hex and binary on the spot.

Simon
  

>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:

Simon> When we print the "unknown" part of a flag enum, it is printed in
Simon> decimal.  I think it would be more useful if it was printed in hex, as
Simon> it helps to determine which bits are set more than a decimal value.

This seems fine to me.

Tom