Re: [PATCH 2/9] commit-graph: stop using signed integers to count bloom filters

[Patrick Steinhardt <ps@pks.im>]

On Thu, Aug 07, 2025 at 03:41:38PM -0700, Junio C Hamano wrote:
> Patrick Steinhardt <ps@pks.im> writes:
> 
> > Yup, fully agreed, and this is a good reason why it should be signed. In
> > the case at hand though we never use such sentinel values, I think
> > making that explicit by using an unsigned type is a good thing as it
> > tells the reader that "Yup, no sentinels involved, it's a plain counter
> > from 0 to $NUM_ENTRIES".
> 
> I do not think such a "statement" has much values, especially the
> right $NUM_ENTRIES is different for specific cases and is not
> expressed anywhere.
> 
> Also, by making it explicit, such a move is also making it explicit
> that we want to close the door for certain future evolution of the
> code paths involved.  I.e. anything that starts to require the
> member or the variable to use a sentinel value is unwelcome.

I think that's actually a good thing. If we wanted to start using a
sentinel value we'd have to change the type to be signed. Combined with
-Wsign-compare this would then alert us of cases where we compare this
counter with an unsigned index, which are exactly all the sites where we
might have to adjust the code to take into account the new sentinel
value.

Patrick