[boost] [safe_numerics] Review

Subject: [boost] [safe_numerics] Review
From: Barrett Adair (barrettellisadair_at_[hidden])
Date: 2017-03-12 00:32:31

Last minute review here. This will be short and sweet.

> What is your evaluation of the design?

I like it. The "drop-in replacement" approach is easy to understand.

> What is your evaluation of the implementation?

It's more complicated than I expected, but if it works, it works. I think the
runtime checks in this library might be good candidates for
BOOST_[UN]LIKELY.

Are there test cases for post decrement? The implementation
appears to return a reference to local:

https://github.com/robertramey/safe_numerics/blob/master/include/safe_base.hpp#L251

safe_base & operator--(int){ // post decrement
    safe_base old_t = *this;
    --(*this);
    return old_t;
}

If I'm reading correctly, this is UB (and likely a segfault).

> What is your evaluation of the documentation?

The documentation is quite thorough. A "cheatsheet" page would
be nice, because most of the documentation is rather in-depth. There
are many bells and whistles documented that end users might not want
to sift through. I'm not sure what this would look like, though.

The FAQ appears to be formatted strangely.

Notes about bitwise operators in the FAQ might be nice (signed right-shift
especially).

Very nice job overall. Much work and thought went into this.

> What is your evaluation of the potential usefulness of the library?

Useful. I'd consider using it in applications that are not latency-sensitive,
perhaps if only in debug mode. Detecting accidental unsigned underflow is
especially useful.

I do think this library might be much more useful with floating point
support, if
it is eventually added.

> Did you try to use the library? With what compiler? Did you have any
   problems?

No.

> How much effort did you put into your evaluation? A glance? A quick
   reading? In-depth study?

3 hours reading documentation, source code, and previous reviews.

> Are you knowledgeable about the problem domain?

No more than any CS grad should be.

> Do you think the library should be accepted as a Boost library?

Yes. This library would be a good addition to Boost. I trust
that Robert will address the issues raised in this review and
others (especially Steven's) before this library would be shipped
with a future Boost release, should it be accepted.