Le 02/03/2017 à 00:36, Andrzej Krzemienski via Boost a écrit :
Hi Everyone, Today, on March 2nd (or tomorrow, depending on your timezone), is the start of the formal review of safe_numerics library by Robert Ramey. The review will run till March 11th. I will be serving as review manager.
safe_numerics is a set of drop-in replacements for the built-in integer types, which make the guarantee that the results of operations are either correct according to the rules of integer arithmetic or report a diagnosable error (rather than offering modulo arithmetic results, or resulting in undefined behavior, or returning incorrect results as in the case of signed vs. unsigned comparisons).
The library can be found on GitHub: https://github.com/robertramey/safe_numerics/
Online docs: http://htmlpreview.github.io/?https://github.com/robertramey/safe_numerics/ master/doc/html/index.html
Hi, let me start with some comments and questions. I was wondering if the requirements of Numeric don't need some kind of conversions between other numeric types. https://htmlpreview.github.io/?https://raw.githubusercontent.com/robertramey... T(u) T{u} How can you implement the conversion from safe<U> to T if you can not convert from U to T? We have implicit conversion from T to Safe and from Safe to T template<class T> constexpr /*explicit*/ safe_base(const T & t); template< class R, typename std::enable_if< !boost::numeric::is_safe<R>::value, int >::type = 0 > constexpr operator R () const; constexpr operator Stored () const; I believed that the conversion from Safe to T needed to be explicit. Having implicit conversion in both directions is suspect. Why the explicit conversion between safe<T> and safe<U> are not allowed when T and U are not the same? IMO, safe_base<T> is either not trivial default constructible and we check the validity of 0, if we don't check it, we should declare it =default. The current definition doesn't adds nothing and makes the type a non-POD and non trivial_default_constructible, which forbids its use on binary messages. constexpr explicit safe_base() { // this permits creating of invalid instances. This is inline // with C++ built-in but violates the premises of the whole library // choice are: // do nothing - violates premise of he library that all safe objects // are valid // initialize to valid value - violates C++ behavior of types. // add "initialized" flag. Preserves fixes the above, but doubles // "overhead" // still pending on this. } I'll suggest to use constexpr safe_base() = default; BTW, why the default constructor is declared explicit? As you say, could we talk of safe<short> when we can have one uninitialized? I believe this needs to appear clearly on the documentation if not already in. Has safe<const T> a sense? Integer<T> are not traits as there is a compiler error when T is not an integer :( template <class T> class Integer : public Numeric<T> { // integer types must have the corresponding numeric trait. static_assert( std::numeric_limits<T>::is_integer, "Fails to fulfill requirements for an integer type" ); }; The same for Numeric. Have you considered to define traits (usable with SFINAE)? I don't see a concept SafeNumeric. Have you considerd it? BTW, these concept classes check only for a single requirement. Shouldn't you specialize numeric_limits lowest()? Have you considered to define basic algorithms managing with the valid conversion between Numeric with the different policies, as Lawrence Crowl is proposing for the standard? http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0103r1.html http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2017/p0105r1.html I find weird the way you are including the files #include "concept/integer.hpp" #include "concept/exception_policy.hpp" #include "concept/promotion_policy.hpp" #include "safe_common.hpp" #include "exception_policies.hpp" #include "boost/concept/assert.hpp" I believed we used to inlcude boost files using <> and the absolute path. Is this a good practice? Does it allows the compiler to perform better? Best, Vicente