Re: Git compile warnings (under mac/clang)

On Fri, Jan 23, 2015 at 12:48:29PM +0100, Johannes Schindelin wrote:

I agree. It is something we as the programmers cannot possibly know (the compiler is free to decide which type however it likes) and its decision does not impact the correctness of the code (the check is either useful or tautological, and we cannot know which, so we are being warned about being too careful!).

I guess you could argue that the standard defines enum-numbering to start at 0, and increment by 1. Therefore we should know that no valid enum value is less than 0. IOW, "msg_id < 0" being true must be the result of a bug somewhere else in the program, where we assigned a value outside of the enum range to the enum.

It may be useful to reference the exact commit (3ce3ffb8) to help people digging in the history (e.g., if we decide there is a better way to shut up this warning and we need to find all the places to undo the brain-damage).

Ugh. It is really a shame how covering up this warning requires polluting so many places. I don't think we have a better way, though, aside from telling people to use -Wno-tautological-compare (and I can believe that it _is_ a useful warning in some other circumstances, so it seems a shame to lose it).

Unless we are willing to drop the ">= 0" check completely. I think it is valid to do so regardless of the compiler's representation decision due to the numbering rules I mentioned above. It kind-of serves as a cross-check that we haven't cast some random int into the enum, but I think we would do better to find those callsites (since they are not guaranteed to work, anyway; in addition to signedness, it might choose a much smaller representation).

I do not see either side of the bounds check here:

as really doing anything. Any code which triggers it must already cause undefined behavior, I think (with the exception of "msg_id == FSCK_MSG_MAX", but presumably that is something we never expect to happen, either).

-Peff