Subject: Re: [PATCH] describe: fix --no-exact-match

From: Junio C Hamano <[email-masked]>
Date: Wed, 09 Aug 2023 12:07:07 -0700

René Scharfe <[email-masked]> writes:

>> Here's what it looks like, for reference.
>>
>> diff --git a/builtin/describe.c b/builtin/describe.c
>> index b28a4a1f82..718b5c3073 100644
>> --- a/builtin/describe.c
>> +++ b/builtin/describe.c
>> @@ -561,9 +561,11 @@ static void describe(const char *arg, int last_one)
>>  static int option_parse_exact_match(const struct option *opt, const char *arg,
>>  				    int unset)
>>  {
>> +	int *val = opt->value;
>
> This line would assign opt->value_int instead...
>
>> +
>>  	BUG_ON_OPT_ARG(arg);
>>
>> -	max_candidates = unset ? DEFAULT_CANDIDATES : 0;
>> +	*val = unset ? DEFAULT_CANDIDATES : 0;
>>  	return 0;
>>  }
>> ...
> Thoughts?  Too much churn?

Sorry, but I am not sure what is being proposed.  opt->value_int
would presumably be defined as a pointer to an int.  The calling
side presumably defines the options[] array entry so that the
&max_candidates address is given to opt->value_int and not to
opt->value_someothertype, and if you try to pass a pointer of a
wrong type, it would be caught as a type mismatch by the compiler,
so the above covers both ends (i.e. the address &max_candidates is
passed in the correct member of the struct, and the callback takes
val out of the correct member of the struct), as long as the
callback uses the right type.  If &max_candidates is int now, and
the above code is written to use opt->value_int, anybody who needs
to update max_candidates to ulong must make sure three things match,
i.e. the type of max_candidates variable itself, the member the
caller uses to pass &max_candidates in the struct (i.e. change to
use value_ulong and not value_int), and the type of the local
variable the callback function uses.

But I am failing to imagine how the calling side actually would look
like.  Can we do something along the lines of

    #define OPT_CALLBACK(s, l, v, a, h, cb)
	.short_name = (s),
	.long_name = (l),
	.value_ ## typeof(v) = &v,
	.help = (h),
	.callback = (cb),

with a clever CPP trick?  It sounds like either too much churn or
too much magic or both, at least to me.

Thanks.