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

From: Jeff King <peff@peff.net>
Date: Sep 7, 2023, 20:40 UTC
Message-ID: <20230907204027.GA941945@coredump.intra.peff.net>
In-Reply-To: <724641d2-1cba-3768-6008-01e8a1cdca4e@web.de>

On Thu, Sep 07, 2023 at 10:20:53PM +0200, René Scharfe wrote:

Show 16 quoted lines

> 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).

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).

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).

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.

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)
   #define OPT_BOOL_OPTARG_F(s, l, v, h, arg) OPT_BOOL_ALL(s, l, v, h, f | PARSE_OPT_OPTARG, arg)
   #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.

-Peff