Phillip Wood <phillip.wood123@xxxxxxxxx> writes:
>> - resume->mode = RESUME_SHOW_PATCH;
>> + resume->mode_int = RESUME_SHOW_PATCH;
>> resume->sub_mode = new_value;
>> return 0;
>> }
>
> Having "mode" and "mode_int" feels a bit fragile as only "mode_int" is
> valid while parsing the options but then we want to use "mode". I
> wonder if we could get Oswald's idea of using callbacks working in a
> reasonably ergonomic way with a couple of macros. We could add an new
> OPTION_SET_ENUM member to "enum parse_opt_type" that would take a
> setter function as well as the usual void *value. To set the value it
> would pass the value pointer and an integer value to the setter
> function. We could change OPT_CMDMODE to use OPTION_SET_ENUM and take
> the name of the enum as well as the integer value we want to set for
> that option. The name of the enum would be used to generate the name
> of the setter callback which would be defined with another macro. The
> macro to generate the setter would look like
>
> #define MAKE_CMDMODE_SETTER(name) \
> static void parse_cmdmode_ ## name (void * var, int value) {
> enum name *p = var;
> *p = value;
> }
Ah, OK. So that's how you defeat "how the size and alignment of an
enum mixes well with int is not known and depends on particular enum
type". It is a tad sad that this relies on "void *", which means
that the caller of parse_cmdmode_resume_type cannot be forced by the
compilers to pass "enum resume_type *" to the function, though. And
that is probably inevitable with the design as .enum_setter needs to
be of a single type, and the member in the "struct option" that
points at the destination variable must be "void *" as it has to
be capable of pointing at various different enum types.
> ...
> Then in builtin/am.c at the top level we'd add
>
> MAKE_CMDMODE_SETTER(resume_type)
>
> and change the option definitions to look like
>
> OPT_CMDMODE(0, "continue", resume_type, &resume.mode, ...)
Yup, that is ergonomic and corrects "The shape of a particular enum
may not match 'int'" issue nicely. I do not know how severe the
problem is that it is not quite type safe that we cannot enforce
resume_type is the same as typeof(resume.mode) here, though.
Thanks.
