Re: [Newcomer PATCH] log-tree.c: Supress Wsign-compare-warning

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> I'm still not of the opinion that it is garbage. We have tons of
> locations where we mismatch integer types only because we never got a
> warning from the compiler, and these have caused multiple stack
> overflows in the past.

I know we spotted many possible overflows and wraparound in the
past, but -Wsign-compare being not about sizes but signedness, I'd
consider them more as happy accidents, rather than intended outcome.

If the code had 'a < (int)b' comparison where 'a' is 'int' and 'b'
is 'size_t' [*], the code would still be wrong, but the compiler
would not have said anything.

	[*] which is what a typical "I've suppressed the compiler
	warning that was annoying me" patch would do if the original
	were written 'a < b'.

'a -= b' can be equally bad depending on the value range of 'b', but
it is not about -Wsign-compare and would go unreported, right?

So I think noise from -Wsign-compare are certainly not "false
positives" (in the sense that the comparisons are between signed and
unsigned---the warning option is reporting what it was asked to
report), but are not-false-but-useless positives; what they try to
catch is somehow different from what they could catch to help us.
And that is why I have been skeptical.

> I do agree though this not a good project for newcomers, as fixing those
> bugs is quite intricate overall. So we should definitely remove this
> project from the microprojects page.

Yeah, that is something I am quite certain about.