On 10/8/19 6:32 PM, SZEDER Gábor wrote:
>>> No, they tend to show (PROGRESS | REGRESS), at least both gdb and lldb
>>> do.
>
> I was wrong here, gdb does this, but lldb, unfortunately, doesn't; see
> my other reply in this thread.
>
>> What I was worried about is that the constants that are used to
>> represent something that are *NOT* set of bits (hence "PROGRESS * 3"
>> may be perfectly a reasonable thing for such an application)
>
> I don't really see how that could be reasonable, it's prone to break
> when changing the values of the enum constants.
>
>> may be
>> mistaken by an overly clever debugger and "3" may end up getting
>> shown as "PROGRESS | REGRESS". When there are only two constants
>> (PROGRESS=1 and REGRESS=2), we humans nor debuggers can tell if that
>> is to represent two bits that can be or'ed together, or it is an
>> enumeration.
>>
>> Until we gain the third constant, that is, at which time the third
>> one may likely be 3 (if enumeration) or 4 (if bits).
>
> Humans benefit from context: they understand the name of the enum type
> (e.g. does it end with "_flags"?), the name of the enum constants, and
> the comment above the enum's definition (if any), and can then infer
> whether those constants represent OR-able bits or not. If they can't
> find this out, then that enum is poorly named and/or documented, which
> should be fixed. As for the patch that I originally commented on, I
> would expect the enum to be called e.g. 'midx_flags', and thus already
> with that single constant in there it'll be clear that it is intended
> as a collection of related OR-able bits.
>
> As for the debugger, if it sees a variable of an enum type whose value
> doesn't match any of the enum constants, then there are basically
> three possibilities:
>
> - All constants in that enum have power-of-two values. In this case
> it's reasonable from the debugger to assume that those constants
> are OR'ed together, and is extremely helpful to display the value
> that way.
>
> - The constants are just a set of values (1, 2, 3, 42, etc). In
> this case the variable shouldn't have a value that doesn't match
> one of the constants in the first place, and I would first suspect
> that the program might be buggy.
>
> - A "mostly" power-of-two enum might contain shorthand constants for
> combinations of a set of other constants, e.g.:
>
> enum flags {
> BIT0 = (1 << 0),
> BIT1 = (1 << 1),
> BIT2 = (1 << 2),
>
> FIRST_TWO = (BIT0 | BIT1),
> };
> enum flags f0 = BIT0;
> enum flags f1 = BIT0 | BIT1;
> enum flags f2 = BIT0 | BIT2;
> enum flags f3 = BIT0 | BIT1 | BIT2;
>
> In this case, sadly, gdb shows only matching constants:
>
> (gdb) p f0
> $1 = BIT0
> (gdb) p f1
> $2 = FIRST_TWO
> (gdb) p f2
> $3 = 5
> (gdb) p f3
> $4 = 7
>
I believe this is the last open thread/discussion on the "midx: add
MIDX_PROGRESS
flag" patch series.
A quick summary of where the discussion stands:
- The patch series currently uses #define for the new MIDX_PROGRESS flag.
- The git codebase uses both #defines and enums for flags.
- The debugger always shows the enum value's name if there is a match, if values
are OR-ed together there a few possibilities (see discussion above and earlier
in the thread).
- There are concerns regarding misinterpreting constants that are not a set of
bits (e.g. "PROGRESS * 3").
Are there any other details I can provide that would help in reaching a
conclusion as to how to proceed?
At this time there are no other MIDX_* flags and so there's always the option
to revisit the best way to handle multiple MIDX_* flags if/when additional
flags are added.
Thanks,
William
