Re: [PATCH v3 23/24] merge-recursive: add sanity checks for relevant merge_options

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 16, 2019, 23:15 UTC
Message-ID: <xmqq7e7czope.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <CABPp-BFE+EX9HtAqN7CeBNHDTObGVw6nT8=+xxAASPJPXAq2=g@mail.gmail.com>

Elijah Newren <newren@gmail.com> writes:

> If the only possible values are 0 and 1, I can either add assertions
> to check that at run time, or make the compiler check it for us by
> confining its value to a single bit.  A compile-time check seems more
> robust...

Sure, as long as they can catch assignments (e.g. ".field = 2", or more interestingly ".field = .field + 1" in a loop, etc.) and increments or decrements and flag them.

Show 5 quoted lines

> But the fact that you flagged the struct change -- would you prefer
> some commit message explanation of how it's related, or was it more
> the case that you felt it was a different kind of change and wanted it
> split out into a separate patch?  I'm suspecting the former but am not
> quite sure.

I do not see it as related at all, so either split it out into a separate patch, or just drop it (and have a runtime check as everybody else in this step), would be the sensible alternatives. I think the latter is easier to reason about but it may be just me.