Martin Ågren <martin.agren@xxxxxxxxx> writes:
>> enum parse_opt_option_flags {
>> - PARSE_OPT_OPTARG = 1,
>> - PARSE_OPT_NOARG = 2,
>> - PARSE_OPT_NONEG = 4,
>> - PARSE_OPT_HIDDEN = 8,
>> - PARSE_OPT_LASTARG_DEFAULT = 16,
>> - PARSE_OPT_NODASH = 32,
>> - PARSE_OPT_LITERAL_ARGHELP = 64,
>
> `PARSE_OPT_NEGHELP` is gone since acbb08c2e0b ("parse-options: remove
> PARSE_OPT_NEGHELP", 2012-02-28), which explains the jump here.
>
>> - PARSE_OPT_SHELL_EVAL = 256,
>> - PARSE_OPT_NOCOMPLETE = 512,
>> - PARSE_OPT_COMP_ARG = 1024,
>> - PARSE_OPT_CMDMODE = 2048
>> + PARSE_OPT_OPTARG = 1 << 0,
>> + PARSE_OPT_NOARG = 1 << 1,
>> + PARSE_OPT_NONEG = 1 << 2,
>> + PARSE_OPT_HIDDEN = 1 << 3,
>> + PARSE_OPT_LASTARG_DEFAULT = 1 << 4,
>> + PARSE_OPT_NODASH = 1 << 5,
>> + PARSE_OPT_LITERAL_ARGHELP = 1 << 6,
>> + PARSE_OPT_SHELL_EVAL = 1 << 7,
>> + PARSE_OPT_NOCOMPLETE = 1 << 8,
>> + PARSE_OPT_COMP_ARG = 1 << 9,
>> + PARSE_OPT_CMDMODE = 1 << 10,
>> };
>
> Those last few conversions close the gap and we end with 1024 rather
> than 2048. That "should" be ok, unless some piece of code relies on
> exact values here, or even their relations(!). Hopefully not? Might be
> worth calling out in the commit message that you're changing some
> values, if you're rerolling anyway. (Or you could leave 1<<8 unused to
> make this a true no-op, then use that value in the next patch. Anyway, I
> think this is safely in bikeshedding land.)
I think 8 vs 1<<3 is bikeshedding, but a patch that claims to
"convert bitfield flags to use binary shift" is supposed to be a
no-op, so it would highly be problematic to see such a hidden
renumbering happening at the same time. Even if the existing code
or any code in flight only relies on the fact that these bits are
distinct and non overlapping.
Thanks for a sharp set of eyes.
