Re: RFC: enum pretty-printing support

[Jan Kratochvil <jan.kratochvil@redhat.com>]

Hi Tom,

On Wed, 11 Jan 2012 17:45:16 +0100, Tom Tromey wrote:
> Tom> I think the C code has to be a bit more careful because it
> Tom> cannot readily be turned off.
> Tom> 
> Tom> One idea is to have both.
> 
> Jan> I agree.
> 
> Here's a new patch.  Let me know what you think.

That's great now, thanks.  FYI not reviewed the Python part.


> --- a/gdb/NEWS
> +++ b/gdb/NEWS
> @@ -9,6 +9,18 @@
>  * The binary "gdbtui" can no longer be built or installed.
>    Use "gdb -tui" instead.
>  
> +* GDB will now print "flag" enums specially.  A flag enum is one where
> +  all the enumerator values have no bits in common when pairwise
> +  "and"ed.  When printing a value whose type is a flag enum, GDB will
> +  show all the constants, e.g., for enum E { ONE = 1, TWO = 2}:
> +  (gdb) print (enum E) 3
> +  $1 = [ONE | TWO]

I do not much understand - why not to make it C syntax compliant?
     $1 = (ONE | TWO)


> +
> +* Python scripting
> +
> +  ** A new class, gdb.printing.FlagEnumerationPrinter, can be used to
> +     apply "flag enum"-style pretty-printing to any enum.
> +
>  *** Changes in GDB 7.4
>  
>  * GDB now handles ambiguous linespecs more consistently; the existing
> diff --git a/gdb/c-valprint.c b/gdb/c-valprint.c
> index 9949015..1d44e90 100644
> --- a/gdb/c-valprint.c
> +++ b/gdb/c-valprint.c
> @@ -458,7 +458,38 @@ c_val_print (struct type *type, const gdb_byte *valaddr,
>  	}
>        else
>  	{
> -	  print_longest (stream, 'd', 0, val);
> +	  if (TYPE_FLAG_ENUM (type))

Some comment here what this new block does, please.

This "else if" can be indented left - one needless block/indentation here.


> +	    {
> +	      int first = 1;
> +
> +	      fputs_filtered ("[", stream);
> +	      for (i = 0; i < len; ++i)
> +		{
> +		  QUIT;
> +
> +		  if ((val & TYPE_FIELD_BITPOS (type, i)) != 0)
> +		    {
> +		      if (!first)
> +			fputs_filtered (" | ", stream);
> +		      first = 0;
> +
> +		      val &= ~ TYPE_FIELD_BITPOS (type, i);
                              ^
Extra space, not GDB coding style compliant.


> +		      fputs_filtered (TYPE_FIELD_NAME (type, i), stream);
> +		    }
> +		}
> +
> +	      if (first || val != 0)
> +		{
> +		  if (!first)
> +		    fputs_filtered (" | ", stream);
> +		  fputs_filtered ("unknown: ", stream);
> +		  print_longest (stream, 'd', 0, val);
> +		}
> +
> +	      fputs_filtered ("]", stream);
> +	    }
> +	  else
> +	    print_longest (stream, 'd', 0, val);
>  	}
>        break;
>  
[...]
> diff --git a/gdb/gdbtypes.h b/gdb/gdbtypes.h
> index ddad0dc..f152945 100644
> --- a/gdb/gdbtypes.h
> +++ b/gdb/gdbtypes.h
> @@ -290,6 +290,12 @@ enum type_instance_flag_value
>  
>  #define TYPE_DECLARED_CLASS(t) (TYPE_MAIN_TYPE (t)->flag_declared_class)
>  
> +/* True if this type is a "flag" enum.  A flag enum is one where all
> +   the values are pairwise disjoint when "and"ed together.  This
> +   affects how enum values are printed.  */
> +
> +#define TYPE_FLAG_ENUM(t) (TYPE_MAIN_TYPE (t)->flag_flag_enum)
> +
>  /* Constant type.  If this is set, the corresponding type has a
>     const modifier.  */
>  
> @@ -399,6 +405,9 @@ struct main_type
>    /* True if this type was declared with "class" rather than
>       "struct".  */
>    unsigned int flag_declared_class : 1;

Empty line (OK, flag_declared_class is also wrong wrt this).

> +  /* True if this is an enum type with disjoint values.  This affects
> +     how the enum is printed.  */
> +  unsigned int flag_flag_enum : 1;
>  
>    /* A discriminant telling us which field of the type_specific union
>       is being used for this type, if any.  */
[...]
> --- a/gdb/testsuite/gdb.python/py-pp-maint.exp
> +++ b/gdb/testsuite/gdb.python/py-pp-maint.exp
[...]
> @@ -127,3 +132,18 @@ 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) (FLAG_1 | FLAG_3)" \
> +    " = 0x5 .FLAG_1 | FLAG_3." \
> +    "print FLAG_1 | FLAG_3"
> +
> +gdb_test "print (enum flag_enum) (4 + 8)" \
> +    " = 0xc .FLAG_1 | <unknown: 0x8>." \
> +    "print FLAG_1 | 8"
> +
> +
> +

3 extra empty lines.



Thanks,
Jan