Fix all the warnings in CheckedInt

RESOLVED FIXED in mozilla16



5 years ago
5 years ago


(Reporter: bjacob, Assigned: bjacob)


(Blocks: 1 bug)

Dependency tree / graph

Firefox Tracking Flags

(Not tracked)



(1 attachment)

Created attachment 636801 [details] [diff] [review]
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.
Attachment #636801 - Flags: review?(jwalden+bmo)


5 years ago
Blocks: 756397
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+

Comment 2

5 years ago
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

5 years ago
Assignee: nobody → bjacob
Target Milestone: --- → mozilla16
Last Resolved: 5 years ago
Resolution: --- → FIXED
Blocks: 828741
You need to log in before you can comment on or make changes to this bug.