Subject: Re: [PATCH v2 02/13] msvc: avoid using minus operator on unsigned types

From: Junio C Hamano <[email-masked]>
Date: Sun, 06 Oct 2019 09:02:43 +0900

Johannes Schindelin <[email-masked]> writes:

>> IMHO, if you don't accompany insert_pos_as_negative_offset() with a
>> corresponding extract_pos_and_found_condition() and use it everywhere,
>> it is more obfuscating than necessary.
>
> I do disagree here. No overflow checking needs to be performed for `-1 -
> <int-value>`. And that's what the opposite of this function really boils
> down to.

I do not think j6t is referring to the over/underflow issues at all.

The suggestion is that, because insert-pos-as-negative-offset
abstracts away (in addition to the overflow checks) the fact that
"does not exist but here is the location it would be inserted" is
encoded in a certain way (i.e. not just the array index negated, but
also is offset by -1, because we wouldn't be able to say "at the
very beginning at index 0" without the -1 offset), the side that
consumes the encoded "pos" (i.e. "we got a negative, so we know the
element does not exist, and the index into the array we would insert
a new element is computed this way") should be abstracted away, as
it must know that the extra negative offset used when encoding is
"-1".

I think that is a reasonable thing to consider; it is not necessary
for correctness, but contributes to the conceptual clarity (iow, it
can be left as a separate clean-up step done after the series is
done).