768570 - Fix all the warnings in CheckedInt

Status: RESOLVED FIXED

(Core :: MFBT, defect)

Description

[Benoit Jacob [:bjacob] (mostly away)]

Attachment: fix CheckedInt warnings

Attached patch fixes all the warnings reported by g++-4.6 -Wall -Wextra -pedantic. I have actually seen those while building Firefox, so it matters.

There are 2 fixes:

- In CheckedInt.h, DoesRangeContainRange helper introduce to avoid warnings about >= and <= comparisons that are always true due to the types at hand.

- In TestCheckedInt.cpp, we had a warning about integer overflow at compile time. 'Fixed' it by replacing a literal 1 by one.value().

Mo: you'll probably want this as well in WebKit.

Comment 1

[Jeff Walden [:Waldo]]

Comment on attachment 636801 [details] [diff] [review]
fix CheckedInt warnings

Review of attachment 636801 [details] [diff] [review]:
-----------------------------------------------------------------

You don't have to sell me on fixing the in-range warning.  We've seen this before in the JS engine, actually; I wonder if this might even plausibly be worth generalizing at all, so people can do templaty range tests a particular way and never have to worry about in-rangeness.  Probably not.  (The JS engine cases involved integers and doubles, too, and your tricks don't generalize to floating point types anyway.)

Comment 2

[Benoit Jacob [:bjacob] (mostly away)]

Yeah, it would have to be written differently to accomodate floating point types. If I had to write it with such generality in mind, here is how I would proceed. I would write template helpers that return, for a given type, an integer telling how big the max value is (and how big negative the min value is). Then the range comparison would boil down to a >= comparison on these integers. For example you could return this for the max value:

   int8 ->   1
   uint8 ->  2
   int16 ->  3
   uint16 -> 4
   int32 ->  5
   uint32 -> 6
   int64 ->  7
   uint64 -> 8
   float ->  9
   double -> 10

Of course if by 'range' you rather mean the range of exactly representable integers, then the ordering is different:

   int8 ->   1
   uint8 ->  2
   int16 ->  3
   uint16 -> 4
   float ->  5
   int32 ->  6
   uint32 -> 7
   double -> 8
   int64 ->  9
   uint64 -> 10

And ditto for the min values (this time you have repeated values, should return 0 for all unsigned types).

Comment 3

[Benoit Jacob [:bjacob] (mostly away)]

http://hg.mozilla.org/integration/mozilla-inbound/rev/2b9bea1ee9e9

Comment 4

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/2b9bea1ee9e9