OK, here's my review of Robert's library. By way of something different, I've tried to not read the docs (though I ended up scanning them in the end) and just tried to use the library. Here's what I've found: 1) Using: safe<int>(-1) & 1 I get a hard assert failure at safe_base_operations.hpp:1373. I'm assuming that this is a mistake, and the usual error handlers should be called? Bitwise or has the same issue. More tests required ;) 2) I'm not sure if this is an msvc issue but in: constexpr safe_base operator~() The static assert is triggered for *unsigned types* and not for signed. Even if this is a compiler issue, it indicates a missing test case or two. 3) If I assign a float to an integer, then I get the error: "conversion of integer to float loses precision" which isn't quite what you meant to say. More particularly, for float conversions I would expect: * Conversion from a float holding an integer value that's within the range of the target type to always succeed. * Conversion from a float holding a non-integral value to conditionally succeed (with trunction) depending on the policy in effect. * Conversion *to* a float may also fail if the integer is outside the range of the float (no checking may be required if the number of bits in the integer is less than the number of bits in the float). 4) Is constexpr arithmetic supposed to be supported? I tried: constexpr boost::numeric::safe<int> j = 0; constexpr boost::numeric::safe<int> k = 3; constexpr boost::numeric::safe<int> l = j + k; Which generates an internal compiler error for msvc, and for gcc-5.3 I get: ../../../include/safe_base_operations.hpp:332:35: error: call to non-constexpr function ‘void boost::numeric::dispatch(const boost::numeric::checked_result<R>&) [with EP = boost::numeric::throw_exception; R = int]’ dispatch<exception_policy>(r); Similarly if I try to add a literal to a safe<int>. Again as the tests all pass, so I'm assuming they're missing constexpr tests? 5) What is the purpose of class safe_literal? constepxr initialization seems to work just fine without it? 6) In the docs, the example of using error handler dispatch is nonesense (top of page 41)? 7) Related to the above, I see the need to be able to define your own primitives which behave just like yours. The example I have is for bit-scan-left/right which has as precondition that the argument is non-zero, but there are other bit-fiddling operations (bit count etc) which some folks may require. A "cookbook" example would be good. 8) In the tests, you have an undocumented function "log" in the library. Not sure if this is intended to be part of the API or not, but IMO it's misnamed. ilog2 seems to a semi-standard name for this. 9) By way of testing the libraries performance I hooked it into the Millar-Rabin primality test in the multiprecision lib (test attached, it counts how many probable primes there are below 30000000), here are the times for msvc: Testing type unsigned: time = 17.6503 seconds count = 1857858 Testing type safe<unsigned>: time = 83.5055 seconds count = 1857858 Testing type 32-bit cpp_int: time = 36.9026 seconds count = 1857858 and for GCC-MINGW-5.3.0: Testing type unsigned: time = 17.1037 seconds count = 1857858 Testing type unsigned: time = 18.4377 seconds count = 1857858 Testing type unsigned: time = 21.0592 seconds count = 1857858 I'm surprised by the difference in times between msvc and gcc, also deeply impressed that there is effectively no performance penalty with GCC (I was expecting there should be some!) 10) I don't find the scope of the library over-broad, in fact I probably find it about right wrt the policies etc. 11) There were a few traits and such that it would be nice to have, the problem as ever, is what to call them and where to put them: * A trait for extracting the underlying type of a safe<> - use case for example where you enable_if a function on is_safe so don't have the template argument list to safe<>. * Do we need a function for extracting the underlying type of a safe<>? Or is casting to the underlying type optimal? Actually, maybe having a function would negate the need for the above trait, as we could just write 'auto x = mysafe.value();' ? * Do we need traits equivalent to is_signed/is_unsigned/make_signed/make_unsigned? My gut feeling is yes we do, the issue is what to call them, and where to put them - we're not allowed to specialize the type_traits versions (either boost or std), but we would need a central "one true trait" version that can be specialized by other libraries as well (Multiprecision for example). We've backed ourselves into a corner over this I fear! 12) When calling boost::integer::gcd/lcm the sub-optimal(?) non-binary versions are called for safe<> - you might want to look into seeing what traits need to be defined to ensure that the same version as the underlying type is called. Those are all the issues I can see for now... in conclusion, as for whether the library should be accepted, yes I believe it should be, modulo the issues above. Regards, John. --- This email has been checked for viruses by AVG. http://www.avg.com