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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 6, 2025, 15:41 UTC
Message-ID: <xmqq4iukwj83.fsf@gitster.g>
In-Reply-To: <aJL06Nlee6CR-KK0@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

Show 7 quoted lines

>> I wrote these counters in 312cff5207 (bloom: split 'get_bloom_filter()'
>> in two, 2020-09-16) and 59f0d5073f (bloom: encode out-of-bounds filters
>> as non-empty, 2020-09-17), and I don't see a compelling reason that
>> these should be unsigned.
>
> I think that is going backwards though: the question to ask is why
> should these be signed if they cannot ever be negative?

Earlier I gave an example of allowing for a "not yet counted" sentinel value for a variable or a structure member. Another example may be for a function that counts that also needs to signal an error, and as usual in any C programs, the natural way to do so for any function whose "normal" return values are non-negative integers is to signal errors with a negative value.

Note that a structure member or a variable that does not need such a "not yet counted" sentinel value (e.g., it may have a separate "counted already" member associated with it, or the nature of the thing it counts does not have such "not yet counted" state), and it is possible for such a variable to live happily with a function that can signal an error.

It means the variable that receives the counted result from such a function may be able to use only half a range of values as its type implies, if that helper function is the only source of information that is assigned to it, though.

If the counter in question never needs to store such a sentinel value itself, then I am OK for it to be unsigned, and that is exactly why I said "not always a valid excuse". But if the counter variable or structure member has to work with functions that need to return sentinel values (like platform natural int that can use the usual "negative is an error, non-negative is a normal result"), it may have less chance to trigger the -Wsign-compare irritation, if you made it also signed.