From: Jeff King Date: Sat, 29 Aug 2026 11:14:18 GMT Subject: Re: [PATCH 2/2] die_for_incompatible_opts(): accept more than four options Message-ID: <20260829111418.GA40814@coredump.intra.peff.net> In-Reply-To: On Thu, Aug 27, 2026 at 07:35:38AM -0700, Junio C Hamano wrote: > > So that makes sense. Of course the follow-on question is whether any > > callers actually want to pass more than 4 options. I don't see any > > patches adding new calls. > > There isn't. While I was writing [*], I wondered if the two calls > next to each other for opt3 and opt4 want to be combined to opt7. OK. I wonder if we're approaching churn here, but I don't have a strong feeling. > I think I can do without [1/2], by the way. > > - die_for_incompatible_optN() (2 <= N <= 4) will keep accepting N > pairs of > > - die_for_incompatible_opts() will take pairs of , > expects "int" to be 0 (not set), 1 (set), or EOF==-1 (sentinel). > > - static inline void die_for_incompatible_opt2() emulation layer > will call die_for_incompatible_opts(!!opt1, opt1_name, !!opt2, > opt2_name, EOF). Similarly for opt3() and opt4() variants. Yeah, but then you can't get good compiler support, since I don't think there is an integer equivalent to LAST_ARG_MUST_BE_NULL. So the varargs interface feels less safe (and strictly worse since we are not actually helping any case that has more than 4 items). If we're not actually exposing the varargs version and expect people to use the counted wrappers, then it's not as big a risk. But then I wonder what the value of the patch is. > We could switch to dynamic allocations immediately after we see > option[] filled, as we are committed to die() at that point and can > afford to waste cycles. That way, for die_for_incompatible_opt10() > when the end-user uses 7 of them, we can fill option[4], switch to > dynamic allocation to collect all 7 of them and report. > > The reason I chose not to is primarily because we cannot use the > existing message templates in that case, hurting i18n/l10n. Yeah, that makes sense. -Peff