Re: [PATCH] describe: fix --no-exact-match
From: Jeff King <hidden>
Date: 2023-08-11 15:11:09
Possibly related (same subject, not in this thread)
- 2023-08-10 · Re: [PATCH] describe: fix --no-exact-match · René Scharfe <hidden>
- 2023-08-10 · Re: [PATCH] describe: fix --no-exact-match · Junio C Hamano <hidden>
- 2023-08-10 · Re: [PATCH] describe: fix --no-exact-match · Jeff King <hidden>
- 2023-08-09 · Re: [PATCH] describe: fix --no-exact-match · Junio C Hamano <hidden>
- 2023-08-08 · Re: [PATCH] describe: fix --no-exact-match · Jeff King <hidden>
On Thu, Aug 10, 2023 at 09:10:33PM +0200, René Scharfe wrote:
quoted
I'm not sure there's a portable and non-insane way of doing what we want here. At least at compile-time.We need a wrapper with the correct signature. The wrapper is plugged into struct option. The typed callback is called by the wrapper and can be used for a type check in the struct macro. Demo patch below.
OK, clever. So we have two functions, one with a real body, and the other which is used with the void pointer. How do we make sure that the real-body one matches the type passed to OPT_CALLBACK(), if it is only seeing the void wrapper? I guess that is this bit in short_name:
+#define OPT_CALLBACK_F_T(s, l, v, a, h, f, cb) { \
+ .type = OPTION_CALLBACK, \
+ .short_name = (s) + (0 ? cb(NULL, NULL, 0, (v)) : 0), \which would cause the compiler to barf, and presumably eliminate the dead code (or at the very least never call it at runtime). So I think that works. Though...
+#define DEFINE_PARSE_OPT_CB(name) \
+static inline int name ## __void(const struct option *opt, \
+ const char *arg, int unset) \
+{ \
+ return name(opt, arg, unset, opt->value); \
+} \we are defining an inline function with the explicit goal of passing it as a function pointer. I don't remember all of the standard's rules here. Are we guaranteed that it will create a linkable version if necessary? We could probably drop the "inline" (and perhaps would need to add MAYBE_UNUSED in such a case).
quoted hunk ↗ jump to hunk
diff --git a/builtin/describe.c b/builtin/describe.c index b28a4a1f82..ce16c36de2 100644 --- a/builtin/describe.c +++ b/builtin/describe.c@@ -558,15 +558,17 @@ static void describe(const char *arg, int last_one) strbuf_release(&sb); } -static int option_parse_exact_match(const struct option *opt, const char *arg, - int unset) +static int option_parse_exact_match(const struct option *opt UNUSED, + const char *arg, int unset, int *value) { BUG_ON_OPT_ARG(arg); - max_candidates = unset ? DEFAULT_CANDIDATES : 0; + *value = unset ? DEFAULT_CANDIDATES : 0; return 0; } +DEFINE_PARSE_OPT_CB(option_parse_exact_match);
I wondered about combining these, like:
DEFINE_PARSE_OPT_CB(option_parse_exact_match, int) {
...the real body here...
}
But I guess that may confuse non-compiler parsers, and it doesn't leave
room for annotations like the UNUSED above (which ironically is still
needed, since now we pass opt->value as its own parameter).
So I dunno. Clever, for sure, and I think it would work. I'm not sure if
the extra code merits the return or not.
-Peff