On Fri, Dec 8, 2017 at 9:30 AM, Richard Smith via cfe-commits <cfe-commits@lists.llvm.org> wrote: > On 8 Dec 2017 08:54, "Hans Wennborg via cfe-commits" > <cfe-commits@lists.llvm.org> wrote: > > Author: hans > Date: Fri Dec 8 08:54:08 2017 > New Revision: 320162 > > URL: http://llvm.org/viewvc/llvm-project?rev=320162&view=rev > Log: > Revert "Unify implementation of our two different flavours of > -Wtautological-compare." > >> Unify implementation of our two different flavours of >> -Wtautological-compare. >> >> In so doing, fix a handful of remaining bugs where we would report false >> positives or false negatives if we promote a signed value to an unsigned >> type >> for the comparison. > > This caused a new warning in Chromium: > > ../../base/trace_event/trace_log.cc:1545:29: error: comparison of constant > 64 > with expression of type 'unsigned int' is always true > [-Werror,-Wtautological-constant-out-of-range-compare] > DCHECK(handle.event_index < TraceBufferChunk::kTraceBufferChunkSize); > ~~~~~~~~~~~~~~~~~~ ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > The 'unsigned int' is really a 6-bit bitfield, which is why it's always > less than 64. > > I thought we didn't use to warn (with out-of-range-compare) when comparing > against the boundaries of a type? > > > This isn't a "boundary of the type" case, though -- 64 is out of range for a > 6 bit bitfield. Your CHECK does nothing. Do you think that it's not > reasonable to warn on this bug, or that it's not a bug?
I'm not sure it's a bug; and the CHECK will fire in case someone widens that bitfield and stores a larger value into it. I suppose I should rewrite the code as "handle.event_index <= TraceBufferChunk::kTraceBufferChunkSize -1"? The reason I thought this was unintentional is because I thought we don't warn in cases like: void f(int x) { if (x <= INT_MAX) { foo(); } } and that's essentially what the code is doing, except it's using < instead of <=. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits