Re: [PATCH 5/5] gdb: change print format of flag enums with value 0

[Luis Machado <luis.machado@linaro.org>]

On 2/13/20 5:30 PM, Simon Marchi wrote:
> If a flag enum has value 0 and the enumeration type does not have an
> enumerator with value 0, we currently print:
> 
>    $1 = (unknown: 0x0)
> 
> I don't like the display of "unknown" here, since for flags, 0 is a
> an expected value.  It just means that no flags are set.  This patch
> makes it so that we print it as a simple 0 in this situation:

Should we print "no flags set" alongside the 0 for this case then?

> 
>    $1 = 0
> 
> If there is an enumerator with value 0, it is still printed using that
> enumerator, for example (from the test):
> 
>    $1 = FE_NONE
> 
> gdb/ChangeLog:
> 
> 	* valprint.c (generic_val_print_enum_1): When printing a flag
> 	enum with value 0 and there is no enumerator with value 0, print
> 	just "0" instead of "(unknown: 0x0)".
> 
> gdb/testsuite/ChangeLog:
> 
> 	* gdb.base/printcmds.exp (test_print_enums): Update expected
> 	output.
> ---
>   gdb/testsuite/gdb.base/printcmds.exp |  2 +-
>   gdb/valprint.c                       | 31 +++++++++++++++++++++-------
>   2 files changed, 25 insertions(+), 8 deletions(-)
> 
> diff --git a/gdb/testsuite/gdb.base/printcmds.exp b/gdb/testsuite/gdb.base/printcmds.exp
> index d6f5c75650bf..bd2afc8696f0 100644
> --- a/gdb/testsuite/gdb.base/printcmds.exp
> +++ b/gdb/testsuite/gdb.base/printcmds.exp
> @@ -743,7 +743,7 @@ 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: 0x0)"]
> +    gdb_test "print flag_enum_without_zero" [string_to_regexp " = 0"]
>   
>       # Print a flag enum with unknown bits set.
>       gdb_test "print (enum flag_enum) 0xf1" [string_to_regexp " = (FE_ONE | unknown: 0xf0)"]
> diff --git a/gdb/valprint.c b/gdb/valprint.c
> index bd21be69e1bf..6e62d420c0f4 100644
> --- a/gdb/valprint.c
> +++ b/gdb/valprint.c
> @@ -634,7 +634,6 @@ generic_val_print_enum_1 (struct type *type, LONGEST val,
>         /* We have a "flag" enum, so we try to decompose it into
>   	 pieces as appropriate.  A flag enum has disjoint
>   	 constants by definition.  */
> -      fputs_filtered ("(", stream);
>         for (i = 0; i < len; ++i)
>   	{
>   	  QUIT;
> @@ -646,24 +645,42 @@ generic_val_print_enum_1 (struct type *type, LONGEST val,
>   
>   	  if ((val & enumval) != 0)
>   	    {
> -	      if (!first)
> +	      if (first)
> +		{
> +		  fputs_filtered ("(", stream);
> +		  first = 0;
> +		}
> +	      else
>   		fputs_filtered (" | ", stream);
> -	      first = 0;
>   
>   	      val &= ~TYPE_FIELD_ENUMVAL (type, i);
>   	      fputs_filtered (TYPE_FIELD_NAME (type, i), stream);
>   	    }
>   	}
>   
> -      if (first || val != 0)
> +      if (val != 0)
>   	{
> -	  if (!first)
> +	  /* There are leftover bits, print them.  */
> +	  if (first)
> +	    fputs_filtered ("(", stream);
> +	  else
>   	    fputs_filtered (" | ", stream);
> +
>   	  fputs_filtered ("unknown: 0x", stream);
>   	  print_longest (stream, 'x', 0, val);
> +	  fputs_filtered (")", stream);
> +	}
> +      else if (first)
> +	{
> +	  /* Nothing has been printed and the value is 0, the enum value must
> +	     have been 0.  */
> +	  fputs_filtered ("0", stream);
> +	}
> +      else
> +	{
> +	  /* Something has been printed, close the parenthesis.  */
> +	  fputs_filtered (")", stream);
>   	}
> -
> -      fputs_filtered (")", stream);
>       }
>     else
>       print_longest (stream, 'd', 0, val);
>
Otherwise LGTM.