Closed Bug 768570 Opened 7 years ago Closed 7 years ago

Fix all the warnings in CheckedInt

Categories

(Core :: MFBT, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla16

People

(Reporter: bjacob, Assigned: bjacob)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Attachment #636801 - Flags: review?(jwalden+bmo)
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.)
Attachment #636801 - Flags: review?(jwalden+bmo) → review+
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).
http://hg.mozilla.org/integration/mozilla-inbound/rev/2b9bea1ee9e9
Assignee: nobody → bjacob
Target Milestone: --- → mozilla16
https://hg.mozilla.org/mozilla-central/rev/2b9bea1ee9e9
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.