Subject: Re: [boost] [safe_numerics] Formal review starts today
From: Paul A. Bristow (pbristow_at_[hidden])
Date: 2017-03-09 15:23:29
> -----Original Message-----
> From: Boost [mailto:boost-bounces_at_[hidden]] On Behalf Of Andrzej Krzemienski via Boost
> Sent: 01 March 2017 23:37
> To: boost-announce_at_[hidden]; boost_at_[hidden]
> Cc: Andrzej Krzemienski
> Subject: [boost] [safe_numerics] Formal review starts today
> 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:
> Formal Review comments can be posted publicly on the mailing list or Boost
> Library Incubator <http://blincubator.com>, or sent privately to the review
> manager, that is myself.
> Here are some questions you might want to answer in your review:
> - What is your evaluation of the design?
Complicated to use (compared to int), and very, very complicated to write.
But that is a consequence of the daft design of the C language.
The language also doesn't allow use of hardware to handle overflow (and underflow) and doesn't have arrays etc as first class citizens.
I don't believe that it is really a idiot-proof drop-in replacement for the built-in integral types, but it will be fine to be used for new projects, especially as it needed the fancy features of C++14.
I agree that explicit conversions are the Right Thing, but they do carry a cost to the users - the need to understand the issues and take care with construction and assignment because this is a User Defined Type (UDT) and the special-case privileges of built-in do not apply (another daft design decision). Floating-point and fixed-point UDT have proved unintuitive to use because of explicit conversions; there are big elephant traps awaiting the unwary.
Costly at compile time because of the number of libraries included, but that's a cost worth paying.
I like that the infrastructure might be extended to other than integral types.
> - What is your evaluation of the implementation?
Above my pay grade.
> - What is your evaluation of the documentation?
Good exposition of the problems and solutions.
Links to source of examples code would be *very* useful. Starting with an example is the most common way of 'getting started'.
Will using "" file specification
instead of <>
cause trouble for users trying to use/modify the examples in Boost or their own folder position?
The common need for overflow (or underflow) to become 'saturation' == max_value (or min_value) is not an option (but then it is arithmetically 'wrong', I suppose ;-))
> - What is your evaluation of the potential usefulness of the library?
Very valuable to make programs that 'always work' instead of 'mostly work'.
Users might appreciate a list of compiler versions that really do work.
Sadly 'Compliant C++14' is a bit fuzzy. (Should become clearer if accepted and test matrix visible).
A good effort at working round fundamental C language defects.
> - Did you try to use the library? With what compiler? Did you have any problems?
Had a very brief try with VS 2015 with /std:c++latest added to command line (to try to ensure C++14 compliance) but am stuck with
1>j:\cpp\safe_numeric\safe_numeric\include\interval.hpp(107): error C2737: 'boost::numeric::`anonymous-namespace'::failed_result': 'constexpr' object must be initialized
1>j:\cpp\safe_numeric\safe_numeric\include\safe_base_operations.hpp(131): error C2244: 'boost::numeric::safe_base<T,Min,Max,P,E>::operator =': unable to match function definition to an existing declaration
But I am sure that this is entirely my fault, but I'm out of my time allotted to this review.
Also - Is this warning(s) avoidable/relevant/quietable?
j:\cpp\safe_numeric\safe_numeric\include\safe_base.hpp(233): warning C4814: 'boost::numeric::safe_base<T,Min,Max,P,E>::operator =': in C++14 'constexpr' will not imply 'const'; consider explicitly specifying 'const'
I feel that all warnings should be avoided or supressed using push'n'pop pragmas.
> - How much effort did you put into your evaluation? A glance?
A quick reading.
> - Are you especially knowledgeable about the problem domain?
> Do you think the library should be accepted as a Boost library?
PS I noted a few typos in documentation (though I doubt these have escaped Steven's fine-tooth-combing ;-).
multiplication etc. . C/C++ often automatically - extraneous .
similar, We'll - should be lower case w.
the expression will yield the correct mathematical result - missing .
trap at compiler time all operations which might fail at runtime. - compile-time
(Better to use compile-time and run-time throughout? Inventing new words is unnecessary and hyphenating is good for readbility)
This solution is simple, Just replace instances - lower case j
parameters are guarenteed to meet requirements when - guaranteed
our program compiles without error - even when using the trap_exception exception policy - missing .
We only needed to change two lines of code to achieve our goal missing .
( throw exception, compile time trap, etc..) no implementation should return an arithmetically incorrect result. - extraneous space after ( and missing . and new sentence No implementation ...
C++ operations permitted on it's base type - should be its (possessive!)
--- Paul A. Bristow Prizet Farmhouse Kendal UK LA8 8AB +44 (0) 1539 561830
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk