The major things I see are:
- The generic code should support at least the range of signed integral types (that's a very straight-forward change).
- The naming conventions and general structure should more closely align with what we're already doing with the CheckedNumeric code (this might require some discussion).
- We need to resolve and document the semantics for corner cases like pegging saturation, saturated_max * 0, or saturated_max + saturated_min (the last two are NaN for IEEE floating points, but they're normal math in this code, which feels a bit scary to me).
- This really should be C++11 constexpr clean, which can't be done with the way the assembler is implemented today (although we should be able to work it with __builtin_constant_p and some wrapper hackery).
- Blink specific code like SaturatedSet should be moved back into blink, along with the corresponding tests.
- In keeping with the rest of the library, this should have no external dependencies to base or other Chrome code (it looks like we just have some compiler-specific stuff, which is easy enough to fix).
I probably should have been a reviewer on the CL that landed this. I was fine with pulling the math in from blink, but I've put a lot of effort in keeping this library consistent and portable, because it's pulled into a lot of other code. Unfortunately, this addition kinda breaks that. However, I might make it my X-mas project to take care of this for fun.
Comment 1 by danakj@chromium.org
, Dec 8 2016