Boost logo

Boost :

Subject: Re: [boost] [safe_numerics] Formal review starts today
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2017-03-04 10:34:10

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:
> Online docs:
> master/doc/html/index.html

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.


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);

         class R,
         typename std::enable_if<
>::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

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

     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
         // 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.
         "Fails to fulfill requirements for an integer type"

The same for Numeric. Have you considered to define traits (usable with

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?

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?


Boost list run by bdawes at, gregod at, cpdaniel at, john at