Re: [PATCH 3/5] gdb: allow duplicate enumerators in flag enums

[Simon Marchi <simark@simark.ca>]

On 2020-02-18 3:38 p.m., Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@efficios.com> writes:
> 
> Simon> I have come across some uses cases where it would be desirable to treat
> Simon> an enum that has duplicate values as a "flag enum".  For example, this
> Simon> one here [1]:
> 
> Simon>             /* Alias for header backward compatibility. */
> Simon>             MEMBARRIER_CMD_SHARED = MEMBARRIER_CMD_GLOBAL,
> Simon>     };
> 
> Simon> The last enumerator is kept for backwards compatibility.  Without this
> Simon> patch, this enumeration wouldn't be considered a flag enum, because two
> Simon> enumerators collide.   With this patch, it would be considered a flag
> Simon> enum, and the value 3 would be printed as:
> 
> Does it always choose the first enumerator?

Yes.

generic_val_print_enum_1 iterates on the enum type's fields (so,
enumerators) and clears those bits in the value as it goes.  So
if multiple enumerators match, the first one in the enum type's
fields will be printed.  Is this list of fields guaranteed to be
in the same order as what's in the code though?

> 
> Simon>  	  if (nbits != 0 && nbits && nbits != 1)
> Simon>  	    flag_enum = 0;
> Simon> -	  else if ((mask & value) != 0)
> Simon> -	    flag_enum = 0;
> Simon> -	  else
> Simon> -	    mask |= value;
> 
> I wonder if this allows too much, though.
> 
> Maybe instead it should check for duplicate enumerator values and allow
> those, while still disallowing enums with conflicts, like:
> 
> enum x {
>   one = 0x11,
>   two = 0x10,
>   three = 0x01
> };
> 
> ... which probably isn't a sensible flag enum.

Not sure what you mean here.  With the current patch, this enum would not
bne marked as a flag enum, as `one` has multiple bits set.

Simon