Re: [PATCH] grep: use OPT_INTEGER_F for --max-depth

From: René Scharfe <l.s.r@web.de>
Date: Sep 8, 2023, 22:28 UTC
Message-ID: <cec47733-5b15-6ca7-adaf-7f3216ad178b@web.de>
In-Reply-To: <20230907204027.GA941945@coredump.intra.peff.net>

Am 07.09.23 um 22:40 schrieb Jeff King:

Show 21 quoted lines

> On Thu, Sep 07, 2023 at 10:20:53PM +0200, René Scharfe wrote:
>
>> Am 05.09.23 um 09:21 schrieb Jeff King:
>>> On Sat, Sep 02, 2023 at 08:54:54PM +0200, René Scharfe wrote:
>>>
>>> In general, I wonder how many of the results from:
>>>
>>>   git grep '{ OPTION'
>>>
>>> could be converted to use the macros and end up more readable. There are
>>> a number of OPTARG ones, which I guess can't use macros. Looks like
>>> there are a handful of others (mostly for OPT_HIDDEN).
>>
>> Indeed, and I have a semantic patch for that, but mostly because the
>> macros allow injecting a type check.
>>
>> OPTARG would need a new macro to allow specifying the default value.  Or
>> is there a variadic macro trick that we could use?
>
> Hmm, I had just assumed OPTARG was a lost cause (or we would need an
> "OPTARG" variant of each macro; yuck).

Only for OPT_INTEGER and OPT_STRING AFAICS.

Show 9 quoted lines

> I suspect variadic macros could be made to work, but you'd lose some
> compile-time safety. If I say:
>
>   OPT_BOOL('x', NULL, &v, NULL, "turn on x")
>
> now, the compiler will complain about the number of arguments. In a
> variadic world, it silently ignores the final one. I feel like I've made
> this kind of error before (e.g., when switching to/from _F variants, or
> between types).

OPT_BOOL has PARSE_OPT_NOARG.  Just saying.

It's true that a macro that accepts a variable number of arguments would accept accidental extra arguments of the right type, but I don't see how it would ignore excessive ones.

> You'd want some semantic check between what's in flags (i.e., is the
> OPTARG flag set), but I think that's beyond what the compiler itself can
> do (you could probably write a coccinelle rule for it, though).

Actually I'd want the macro to set that flag for me.

> I think it also squats on the variadic concept for the macro, so that no
> other features can use it. I.e., if you accidentally give _two_ extra
> arguments, I don't know that we can parse them out individually.

In case of an accident I'd just expect a compiler error. A cryptic one, probably, alas, but no silence.

I was thinking more about something like the solutions discussed at https://stackoverflow.com/questions/47674663/variable-arguments-inside-a-macro. It allows selecting variants based on argument count.

That could look like this (untested except on https://godbolt.org/; the EVALs are needed for MSVC for some reason):

#define OPT_INTEGER_FULL(s, l, v, h, f, d) { \
	.type = OPTION_INTEGER, \
	.short_name = (s), \
	.long_name = (l), \
	.value = (v), \
	.argh = N_("n"), \
	.help = (h), \
	.flags = (f), \
	.defval = (d), \
}
#define OPT_INTEGER_4(s, l, v, h) \
	OPT_INTEGER_FULL(s, l, v, h, 0, 0)
#define OPT_INTEGER_5(s, l, v, h, f) \
	OPT_INTEGER_FULL(s, l, v, h, f, 0)
#define OPT_INTEGER_6(s, l, v, h, f, d) \
	OPT_INTEGER_FULL(s, l, v, h, (f) | PARSE_OPT_OPTARG, d)
#define EVAL(x) x
#define SEVENTH(_1, _2, _3, _4, _5, _6, x, ...) x
#define OPT_INTEGER(...) \
	EVAL(EVAL(SEVENTH(__VA_ARGS__, OPT_INTEGER_6, OPT_INTEGER_5, OPT_INTEGER_4, 0))(__VA_ARGS__))

So OPT_INTEGER(s, l, v, h) would be the same as before. Add an argument and it becomes current OPT_INTEGER_F, add another one and it acts as your _OPTARG_F variant.

Show 7 quoted lines

> So yeah, I think you'd really want a separate macro. The combinations
> start to add up (or multiply up, if you prefer ;) ). They _could_ be
> generated mechanically, I think, as they can all be implemented in terms
> of a master macro that knows about all features:
>
>    #define OPT_BOOL_F(s, l, v, h, f) OPT_BOOL_ALL(s, l, v, h, f, 0)
>    #define OPT_BOOL(s, l, v, h, f) OPT_BOOL_F(s, l, v, h, 0)

The "f" arg needs to go...

>    #define OPT_BOOL_OPTARG_F(s, l, v, h, arg) OPT_BOOL_ALL(s, l, v, h, f | PARSE_OPT_OPTARG, arg)

... here, possibly.

>    #define OPT_BOOL_OPTARG(s, l, v, h, arg) OPT_BOOL_OPTARG_F(s, l, v, h, 0, arg)
>
> But I'm not sure if cpp is up to that, or if I'd want to see what the
> resulting code looks like.

You mean having a macro define another macro? I don't think that's possible.

René