AMDG All comments are as of 3bcfabe1cafb8ef6edf08d3bd8730a1555b1c45d automatic.hpp 15: // policy which creates results types equal to that of C++ promotions. // Using the policy will permit the program to build and run in release // mode which is identical to that in debug mode except for the fact // that errors aren't trapped. Copy/paste from native? 83: // clause 2 - otherwise if the rank of the unsigned type exceeds // the rank of the of the maximum signed type How is this possible? The rank of std::intmax_t is the maximum possible rank. If the unsigned type is larger, then rank will return void which gives a compile error when accessing ::value. Actually it looks like calculate_max_t was originally written to return T or U, but was then altered to use [u]intmax_t. 97: // clause 4 - otherwise use unsigned version of the signed type std::uintmax_t It isn't clear to me why uintmax_t is preferable to intmax_t when neither is able to represent every possible result. 124: using t_base_type = typename base_type<T>::type; This is called from safe_base_operations:259, which has already called base_type. From the documentation, I had assumed that automatic used the range from safe_base. However this range is not actually passed to the PromotionPolicy, so automatic always bumps the type to the next largest integer type, until it reaches [u]intmax_t. This means than any expressions with more than a few terms will always be carried out using [u]intmax_t, which is potentially inefficient, e.g. intmax_t may require software implementation if it's larger than the hardware register size. A greater problem arises from the fact that mixed signed/unsigned arithmetic eventually chooses unsigned when the range is too large. If we have a slightly complex expression with both signed and unsigned terms, the result type will eventually be selected as uintmax_t, which may be incapable of representing the result, thus causing an error. (I know that this scenario can also fail with builtin integer types, but it seems to me that the whole point of 'automatic' is to make things "just work" without having to worry about pesky things like signedness) 247: using result_base_type = typename boost::mpl::if_c< std::numeric_limits::is_signed || std::numeric_limits::is_signed, std::intmax_t, std::uintmax_t >::type; Why division doesn't use calculate_max_t like multiplication deserves some explanation. Division can overflow too (numeric_limits::max() / 1). 275: static constexpr divide( This function appears to be necessary, but is not documented as part of the PromotionPolicy concept. The same goes for modulus. 328: template struct left_shift_result Inconsistent with the documantation of PromotionPolicy which says: PP::left_shift_result<T>::type. Same for right_shift_result, and bitwise_result. 362: using type = typename boost::mpl::if_c< (sizeof(t_base_type) > sizeof(u_base_type)), t_base_type, u_base_type >::type; Is it actually correct to combine &, |, and ^ in the same class? The result of & will always fit in the smaller type, but | and ^ require the larger type. checked.hpp: 90: // INT32-C Ensure that operations on signed // integers do not overflow This is the unsigned case. 220: template constexpr checked_result<R> add( Consider the following (assuming 2's complement): checked::add<int>(INT_MIN, UINT_MAX) The result should be INT_MAX, but your implementation will generate an error when converting UINT_MAX to int. 227: if(! rt.no_exception() ) The double negative makes this confusing to read. 230: return detail::add<R>(t, u); This relies on implicit conversion and may generate spurious warnings. 301: Everything I said about add also applies to subtract. 411: exception_type::underflow_error, This should be overflow_error. 467: Again as with add. This time the problematic case is multiply<unsigned>(-1, -1) 539: return detail::divide<R>(tx.m_r, ux.m_r); Just because m_r is public, doesn't mean that you should access it directly. 583: constexpr divide_automatic( The difference between divide and divide_automatic could use some explanation. It looks like the only difference is that divide_automatic doesn't cast the divisor, but it isn't clear why. Also, why do you need two checked divide functions in the first place? There should really just be one that always works correctly. 622: return cast<R>(abs(t) % abs(u)); This likely differs from the builtin % and should be documented as such. Also I prefer % to have the property that t / u * u + t % u = t, which this implementation does not satisfy. 639: // INT34-C C++ standard paragraph 5.8 if(u > std::numeric_limits<T>::digits){ You're off-by-one "The behavior is undefined if the right operand is negative, or greater than *or equal to* the length in bits of the promoted left operand" (emphasis added) left_shift and right_shift (625-792) have several problems: - 640: You're checking the width of T even though the actual shift is done after casting to R. - 696, 708: These overloads serve no purpose. The only difference between them is checks that are already handled by check_shift. - 785: This test duplicates work done be check_shift checked_result.hpp: 36: // can't select constructor based on the current status of another // checked_result object. So no copy constructor Saying that there is no copy constructor is misleading, as the default cctor exists and is used. 67: //assert(no_exception()); Why is this commented out? reading a not-currently-active member of a union is undefined behavior. 99: constexpr boost::logic::tribool operator<=(const checked_result<T> & t) const { return ! operator>(t) && ! operator<(t); } This is opertor==. The correct way is just ! operator>(t). cpp.hpp: 54: using rank = Rank is only used for comparisons. I don't see any reason to use instead of using sizeof directly. This applies to automatic::rank as well. (Note: rank matters for builtin integer promotion, because types with the same size can have different ranks. Your implementation of rank, however, is strictly based on size, making it essentially useless.) exception.hpp: 15: // contains operations for doing checked aritmetic on NATIVE // C++ types. Wrong file. Also s/aritmetic/arithmetic/ for wherever this was pasted from. exception_policies.hpp: 1: #ifndef BOOST_NUMERIC_POLICIES_HPP #define BOOST_NUMERIC_POLICIES_HPP Make this BOOST_NUMERIC_EXCEPTION_POLICIES_HPP? interval.hpp: 18: #include <cstdlib> // quick_exit I don't see quick_exit anywhere in this file. 87: // account for the fact that for floats and doubles There's also long double, you know. 101: namespace { Please don't use the unnamed namespace in headers. 133: template<typename R> constexpr bool less_than( const checked_result<R> & lhs, const checked_result<R> & rhs Why is this here and not in checked_result.hpp? Also this can be implemented easily using operator<: return lhs < rhs; (The implicit conversion from tribool to bool does exactly what you want here.) 257: checked::modulus<R>(t.l, u.l), checked::modulus<R>(t.l, u.u), checked::modulus<R>(t.u, u.l), checked::modulus<R>(t.u, u.u) This appears to be copied from divide, but it's totally wrong. modulus is not monotonic, so you can't get away with checking only the boundaries. 339: const R rl = safe_compare::greater_than(t.l, u.l) ? t.l : u.l; The implicit cast to R may not be safe. (same at 340, 356, and 357) 359: if(safe_compare::greater_than(rl, ru)){ This check isn't necessary. The union of two non-empty intervals can't be empty. 453: template<> std::ostream & operator<<(std::ostream & os, const boost::numeric::interval<unsigned char> & i) - This specialization is not a template, and can only appear in one translation unit. - The implementation requires <ostream>, but you only #include <iosfwd> native.hpp: 27: // Standard C++ type promotion for expressions doesn't depend // on the operation being performed so we can just as well // use any operation to determine it. We choose + for this // purpose. Comment out-dated. The code (somewhat) uses decltype on the correct operators. safe_base.hpp: 240: safe_base operator++(){ // pre increment pre-increment and pre-decrement should return by reference. post-increment and post-decrement should return by value. You currently have everything returning by value except post-decrement (which returns a dangling reference) 258: constexpr safe_base operator-() const { // unary minus should unary minus allow unsigned to become signed? (subject to the PromotionPolicy, of course). 261: constexpr safe_base operator~() const { This will fail if Min/Max are anything other than the full range of the Stored type. safe_base_operations.hpp: 82: indeterminate(t_interval < this_interval), This works, but it would be a bit more intuitive if interval had an 'intersects' function. 244: // Note: the following global operators will be only found via // argument dependent lookup. So they won't conflict any // other global operators for types in namespaces other than // boost::numeric Unfortunately, ADL casts a very wide net. What makes these operators (mostly) safe is the use of enable_if. 267: // Use base_value(T) ( which equals MIN ) to create a new interval. Same // for MAX. Now Now ... what? 288: constexpr static const interval type_interval = exception_possible() ? interval{} This can overestimate the result interval if the interval only overflows on one side. 313: using type_helper = typename boost::mpl::if_c< std::numeric_limits::is_integer, safe_type, unsafe_type >::type; This is what? Support for floating point in the future? 531: // when we add the temporary intervals above, we'll get a new interval // with the correct range for the sum ! I think you mean multiply and product, not add and sum. 676: constexpr static const interval type_interval = exception_possible() ? At this point, you lose any benefit of divide_nz over divide. You should check r_interval.exception() instead of exception_possible(). The same goes for modulus. 748: // argument dependent lookup should guarentee that we only get here I don't understand this comment. 867: static_cast(base_value(t)) % static_cast(base_value(u)) Okay, we have a problem here: checked::modulus does /not/ have the same behavior as the builtin % when the divisor is negative, which means that treating them as interchangable here will result in weird and inconsistent behavior (not to mention silently producing values outside the expected interval.) 907: typename boost::lazy_enable_if_c< ..., boost::mpl::identity<bool> enable_if_c with bool should work just as well. 932: // if the ranges don't overlap (! boost::logic::indeterminate(r)) ? // values in those ranges can't be equal false This is operator<, not operator==. You should return r, not false here. Same at 970 in operator>. Also, you're already implementing operator<= and >= in terms of < and >. Is there a reason to implement > separately? (t > u) == (u < t) 1221: // handle safe<T> << int, int << safe<U>, safe<T> << safe<U> // exclude std::ostream << ... Copy/paste. Should be >> and istream. 1276: base_value(std::numeric_limits<T>::max()) This doesn't take the input range into account and can drastically overestimate the result range. 1401: using bwr = bitwise_or_result; I think it would be slightly better to create an alias template using bitwise_xor_result = bitwise_or_result; instead of using bitwise_or_result directly in the implementation of xor. (Note that the range of ^ can be larger than that of | if the lower bound of the parameters is greater than 0.) 1432, 1474: class P, // promotion polic s/polic/policy/ 1435: std::ostream & operator<<( std::ostream & os, Should this use std::basic_ostream? safe_common.hpp: safe_compare.hpp: safe_integer.hpp: safe_literal.hpp: 111: constexpr safe_literal_impl operator-() const { // unary minus Err, what? This will work iff. N is 0. 140: #define safe_literal(n) \ This is not acceptable. It totally violates the naming rules for macros. safe_range.hpp: utility.hpp: 26: constexpr log(T x){ A function with a common name, that is not in a private namespace, and can match almost anything is a really bad idea. concept/exception_policy.hpp: The reason that the check is commented out deserves explanation. (Missing functions are acceptible and will cause a compile-time error instead of checking at runtime.) concept/*.hpp: no comments In Christ, Steven Watanabe

2017-03-07 18:32 GMT+01:00 Robert Ramey via Boost : 247:

...

using result_base_type = typename boost::mpl::if_c< std::numeric_limits::is_signed || std::numeric_limits::is_signed, std::intmax_t, std::uintmax_t >::type; Why division doesn't use calculate_max_t like multiplication deserves some explanation. Division can overflow too (numeric_limits::max() / 1).

Hmmm - I'm sure it's because I've presumed that the result can aways be contained in the same size type as the number being divide. That t / x <= t - and divide by zero is checked separately. Am I wrong about this?

I am not sure if this is what Steve meant, but: int x = INT_MIN; int y = -1; int a = x / y; // overflows! safe<int> does a check for this condition, but I find it quite surprising that it throws a domain_error rather than overflow_error. How do you make a call whether it is an overflow or a domain error? For mathematical ints dividing -2147483648 by -1 is well defined. Regards, &rzej;

AMDG On 03/07/2017 10:32 AM, Robert Ramey via Boost wrote:

On 3/6/17 7:35 AM, Steven Watanabe via Boost wrote:

...

automatic.hpp

97: // clause 4 - otherwise use unsigned version of the signed type std::uintmax_t It isn't clear to me why uintmax_t is preferable to intmax_t when neither is able to represent every possible result.

If both operands are signed - then uintmax_t could hold one more bit than intmax_t can. In these cases the operand result can be twice as large before runtime checking is necessary.

Right, but uintmax_t can't hold negative values which /also/ require runtime checking.

...

247: using result_base_type = typename boost::mpl::if_c< std::numeric_limits::is_signed || std::numeric_limits::is_signed, std::intmax_t, std::uintmax_t >::type; Why division doesn't use calculate_max_t like multiplication deserves some explanation. Division can overflow too (numeric_limits::max() / 1).

Hmmm - I'm sure it's because I've presumed that the result can aways be contained in the same size type as the number being divide. That t / x <= t - and divide by zero is checked separately. Am I wrong about this?

Yes. There are two specific cases that can cause problems: - the example I gave above returns intmax_t which cannot represent the maximum value of uintmax_t. - std::numeric_limits::min() / -1, which is also out of range.

...

checked.hpp:

220: template constexpr checked_result<R> add( Consider the following (assuming 2's complement): checked::add<int>(INT_MIN, UINT_MAX) The result should be INT_MAX,

I'm not seeing this. With 8 bit ints we've got INT_MIN = x80 = -128 UINT_MAX = x80 = 128 adding together x100 truncating = 0

UINT_MAX (not INT_MAX) = 0xFF = 255

which might be OK except on current machines but does violate the standard in that it undefined behavior. Given that compiler optimizers can do anything they wand on UB I don't think we can do this

...

but your implementation will generate an error when converting UINT_MAX to int.

As I believe it should -

UINT_MAX = 0x80 convert to int - nothing changes x80 interpreted as an int is -128

So this would transform an arithmetic value of 128 to a value of -128. Not a good thing.

What I've done is applied the standard. Convert each type to the result type then do the addition. On this conversion we change the value - game over - invoke error.

My mental model for the correct behavior of checked::add is: - add the values as if using infinite precision arithmetic - truncate the result to result_type - if the value doesn't fit, then return an error I notice that safe_compare does extra work to allow comparisons to give the correct mathematical result even if casts would fail. I believe the checked::xxx should do the same. In addition, *not* following this rule is precisely the reason for the problems that you've encountered with divide.

...

230: return detail::add<R>(t, u); This relies on implicit conversion and may generate spurious warnings.

conversion - I don't see it. detail::add<R>(t, u) return a checked_result<R> which is returned as another checked_result<R>. No conversion. Probably not even a copy with copy elusion.

I meant implicit conversion of t and u. (Is the PromotionPolicy forbidden from returning a type smaller than the arguments?)

...

583: constexpr divide_automatic( The difference between divide and divide_automatic could use some explanation. It looks like the only difference is that divide_automatic doesn't cast the divisor, but it isn't clear why. Also, why do you need two checked divide functions in the first place? There should really just be one that always works correctly.

LOL - that's what I thought. I couldn't make one which worked for both native and automatic promotions.

The case which is giving me fits is dividing INT_MIN / -1

INT_MIN = 0x80 = -128 -128 / -1 = + 128 = 0x80 = INT_MIN again.

So we have INT_MIN / -1 -> INT_MIN an incorrect result.

Now I forget - but this is only a problem with automatic promotions.

Try this way: if (u == 0) error; auto uresult = checked::abs(t)/checked::abs(u); if (sign(t) != sign(u)) return checked::negate<R>(uresult); else return cast<R>(uresult); This should work correctly in all cases as long as you handle all the sticky cases in checked::abs and a hypothetical checked::negate.

...

interval.hpp:

101: namespace { Please don't use the unnamed namespace in headers.

OK - but I forgot why it's not recommended

Code in the unnamed namespace will be distinct in every translation unit. Using the unnamed namespace in a header means that every translation unit that #includes it will get a copy of this code, and linker will not merge the duplicates.

...

453: template<> std::ostream & operator<<(std::ostream & os, const boost::numeric::interval<unsigned char> & i) - This specialization is not a template, and can only appear in one translation unit.

Please expand upon this

If you #include this header from multiple translation units, it will fail to link, with a multiple definition error.

...

- The implementation requires <ostream>, but you only #include <iosfwd> Right = but any other which invokes this function will aready have included ostream. That's why included <iosfwd> rather than <ostream> since this operator is used almost exclusively for debugging/examples.

This operator requires ostream to be complete /at point of definition/, not when it called. i.e. #include "interval.hpp" #include <ostream> will fail. I'm assuming that interval.hpp doesn't indirectly #include <ostream>.

...

safe_literal.hpp:

111: constexpr safe_literal_impl operator-() const { // unary minus Err, what? This will work iff. N is 0.

that's out

...

140: #define safe_literal(n) \ This is not acceptable. It totally violates the naming rules for macros.

I'm still struggling with this. What do you suggest?

I suggest leaving it out. Any name that can legitimately be used as a macro would be more cumbersome than writing out safe_signed_literal or safe_unsigned_literal. In Christ, Steven Watanabe

On 3/9/17 7:23 AM, Paul A. Bristow via Boost wrote:

...

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

Hmmm - a major design goal is that it be a no-brainer to use. I'd like to see you exand a little on this comment. and very, very complicated to write. LOL - it's worse that it looks. There are a lot of difficulties with something like this. One is that its hard to stick to the subject and keep in mind what one's idea of the original purpose of the library is. But this is common in almost all library efforts. A major difficulty in an elaborate TMP project is that the compiler does't really support it very well. Error messages are often unrelated to the mistake and very unhelpful. There is not convenient way to display messages which display type information at compile time. It would be useful to have a BOOST_STATIC_WARNING that works. These problems can only really be addressed by enhancements to the compiler itself. The fact that none of these have been proposed suggests to me that TMP is a lot like teenage sex - there's a lot more talk about it than is actually going on.

But that is a consequence of the daft design of the C language.

tl;dr; I want to defend the C language design. My first exposure to C was in 1980? I was introduced to Ratfor which was a fortran implemented C like language introduced in the Plauger book "Software Tools". After that I lusted after a C language tool. The machine I had at my disposal was a Data General Eclipse - a 16 bit architecture. I got a hold source code for a compiler for the Z80 and managed to port it to the DG machine. Then I managed to compile the standard source which my compiler. I was hooked! The C language then was the world's first "portable assembler". This was huge at the time. BTW - it can (and often is) used in this manner. The "problem" is that evolved the orignal C language into something entirely different. - A system for describing and implementing higher order ideas in an abstract and portable manner. It was done through evolution. Dozens (hundreds) of attempts to build something like that from scratch (fortran, cobol, pascal, modula, Ada) all failed for some reason or another. I believe in evolution. I believe it is the source of progress and success in all things - but it always leaves me disappointed and wanting something more perfect - which I can never get.

The language also doesn't allow use of hardware to handle overflow (and underflow)

When I started this project I was thinking that things would be better if hardware handled these things. But now that I look what I've achieved, I think I was wrong. A key feature of this library is that it allows one to select how to handle these and other similar situations. Currently one can detect and handle these at runtime, or detect potential problems and fail a compile time, promote types to larger types to avoid the problems - (Thereby "fixing" the original "daft" design of the C language - and likely creating your own). Perhaps others ideas are possible. So have been mentioned - "saturation" for example. and doesn't have arrays etc as first class citizens. I don't agree with this - but arguing about 30 year old decisions is a sign of old age.

I don't believe that it is really a idiot-proof drop-in replacement for the built-in integral types,

Well, that is what I've intended and I think I've mostly achieved it. but it will be fine to be used for new projects, especially as it needed the fancy features of C++14. tl;dr; One of the major use cases I see for this is finding errors in legacy code. I included a small example which touches on this. I actually have a larger example - the code is in there. But since its a real (and realistic example - stepping motor controller running on an 8 bit PIC microcontroller) it was a little more complex than normal and I didn't have time to finish the narrative which goes with the example. I hope get back to it in the future. I have done multiple projects with such tiny micro controllers. One big issue with these is the development environment which doesn't lend itself to unit testing. Mostly this is addressed just by skipping unit testing - using the burn (the chip) and crash (the program - but these days it might mean the car itself). But I like to factor out critical pieces into modules which can compiled and unit tested. An can create test vectors which run all the test cases and end points. When a unit test fails I can step through with the debugger. The cpp policy implements safe types and checking with the correct sizes for the microcontoller. I can compile and debug the tests in an environment which closely tracks the target one. By using the compile time trap as the exception policy I can also detect potential failures and tweak my code so that they can never happen. Then I can build and run on the target environment with much more confidence that there are no bugs. To me this is absolutly huge. But it's a very, very, very tough sell. People who work in the microcontroller world see C as portable assembler. People who work in C++/Boost world see C++ as a way of addressing general problems in a more abstract (mathematical like way). I'm hoping to bridge two worlds here. I'm not hopeful. I'm a sucker for lost causes.

I agree that explicit conversions are the Right Thing,

I used to believe that. Now my view is "not always" 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. Right - I'm thinking that that library traps at compile time when one does the following. I'll have to recheck this. safe<int> i; i = 1.0 but accepts i = static_cast<int>(1.0)

Costly at compile time because of the number of libraries included, but that's a cost worth paying.

and I don't think it's all the costly. I'm using a 3 year old mac mini with Xcode/clang for development and all tests compile in a few seconds. Not a big deal in my opinion.

I like that the infrastructure might be extended to other than integral types.

...

- What is your evaluation of the implementation?

Above my pay grade.

LOL - that makes two of us. That's why we have obi-wan-watanbi

...

- What is your evaluation of the documentation?

Good exposition of the problems and solutions.

Good examples.

Links to source of examples code would be *very* useful. easy one - I'll do this. I'll also add the output from the examples which is another idea that has been mentioned.

Starting with an example is the most common way of 'getting started'.

Will using "" file specification #include "../include/safe_range.hpp" instead of <> #include <../include/safe_range.hpp> cause trouble for users trying to use/modify the examples in Boost or their own folder position?

The version being reviewed in the incubator is meant to be built and tested outside the boost infrastructure. The boost testing infracture supports the creating of the test matrix and centralized testing. But in opinion it is an impeditment to one who just wants to test a single library not already included in boost. Boost libraries sort of "presume" the centralized setup promoted by b2 headers. I believe that this is also an obstacle to mixing/matching/testing other libraries. In any case, if this library is accepted, include file structure will have to evolve to the boost way of doing things. Not a big issue or problem.

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 ;-)) Right - This has come up. But I declined to address this for a few reasons.

a) Things are already really complex - I'm at my brain's limit b) I don't want to pollute the overriding idea: "This library provides a method for the brain dead to guarantee that his program will not produce incorrect arithmetic results." Adding more "features" dilutes the conceptual power of thee library, This makes it impossible to describe what it does in one sentence.

...

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

Very valuable to make programs that 'always work' instead of 'mostly work'.

Ahhh - this is the thing for me. Imaging how your going to feel when your microcontroller is included in a product which ships 10000 units and a bug is discovered? How are you going to feel if that product is an automobile accelerator or Mars landing probe? How, after 50 years of computer systems and computer language development, arrived at this situation. I'm mystified and disheartened.

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

LOL - I try to follow the standard - but it's not really possible to even read it. I rely on testing - known limitations. But it's the best we have. So far it's worked pretty well. I can build on Clang with no warnings and on recent versions of GCC and produce the exact same results. I would love to test on VC but I don't have such a machine. I've tried to set up appveyor.yml to do this but have been unsuccessful. I'm interested if someone want's to look into this.

A good effort at working round fundamental C language defects.

a good effort - a ringing endorsement ! It's ok one of my squash partners - a female - says I'm a "good sport". doesn't bug me though.

...

- 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

I'd be curious to see the example you tried. Did you try to run the test suite and/or examples? But this re-enforce the suggestion that I should put the concept checking - such as it is - into the library code to help detect usage mis-understandings. I'll look into this.

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::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.

I don't think it's your fault. I think that's an indication that I've fallen short of my goal that the library be idiot proof. I shouldn't have to say this but this not mean that I think your an idiot, but that if it's not simple for you, it's not going to be simple for an idiot - which is my goal. This re-enforces the suggestion that I should put the concept checking - such as it is - into the library code to help detect usage mis-understandings. I'm look into this.

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::operator =': in C++14 'constexpr' will not imply 'const'; consider explicitly specifying 'const'

This looks very easy to fix - but my compiler doesn't emit these warnings. Maybe it would with some switch. I'll look in to it.

I feel that all warnings should be avoided or supressed using push'n'pop pragmas.

My goal is to closely stick to C++14. So I shouldn't have any warnings. I believe this is possible with some minor code tweaking.

Thanks for your comments. They are very, very helpful.

--- Paul A. Bristow Prizet Farmhouse Kendal UK LA8 8AB +44 (0) 1539 561830

_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

2017-03-11 15:27 GMT+01:00 Paul A. Bristow via Boost :

...

-----Original Message----- From: Boost [mailto:boost-bounces@lists.boost.org] On Behalf Of Robert Ramey via Boost Sent: 09 March 2017 18:32 To: boost@lists.boost.org Cc: Robert Ramey Subject: Re: [boost] [safe_numerics] Formal review starts today

On 3/9/17 7:23 AM, Paul A. Bristow via Boost wrote:

...

...

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

Hmmm - a major design goal is that it be a no-brainer to use. I'd like to see you exand a little on this comment.

One needs to fully understand static_cast - it's an unintuitive name for something that has unexpectedly complicated implications.

...

...

But that is a consequence of the daft design of the C language.

tl;dr;

I want to defend the C language design. My first exposure to C was in 1980? I was introduced to Ratfor which was a fortran implemented C like language introduced in the Plauger book "Software Tools". After that I lusted after a C language tool. The machine I had at my disposal was a Data General Eclipse - a 16 bit architecture. I got a hold source code for a compiler for the Z80 and managed to port it to the DG machine. Then I managed to compile the standard source which my compiler. I was hooked! The C language then was the world's first "portable assembler". This was huge at the time.

I can't resist saying that my reaction to learning of C design philosophy was LOL :-(

But programmers liked quick'n'dirty and being trusted (the biggest mistake of all), a host of .edu starting teaching C, and like FORTRANs bottomless pit of working subroutines, it was unstoppable.

And so here we are putting Band-Aids on the C++ and STL Band-Aids*.

...

I'm hoping to bridge two worlds here. I'm not hopeful. I'm a sucker for lost causes.

I'm optimistic. I'm not the only one getting cross with 'mostly work' programs, and as you observe, people will get *really cross* with 'mostly work' killer cars. Chris Kormanyos has publicised how to use recent C++ on bare-metal in his book Real-Time C++.

...

...

Users might appreciate a list of compiler versions that really do work.

They really *must* have such a list.

...

...

...

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

...

...

Had a very brief try with VS 2015 and failed.

John Maddock has since explained why nothing I tried worked. I'm a bit shocked that it hasn't been tested on MSVC. My acceptance was on the assumption that it would work. It really must be portable over recent GCC, Clang and MSVC at the very minimum.

According to formal Boost criteria, it is sufficient for the library to work on two major compilers. These formal criteria are met by safe_numerics. Of couse, I acknoledge, that formal criteria are not the ony thing in the world.

I suggest that we should pause the review until you adopt John's patches and reissue the review code and then restart the review.

From the formal point of view, the two options for this I can see are:

- To conclude the review as rejected, and schedule a new one. - Accept the library conditionally, and make the fix a hard condition/ It'll be a bit poor to accept the library until a few people confirm it's

working on MSVC.

Accepting the library does not mean it is immediately available in the next Boost release. If the library is accepted conditionally, you would be guaranteed that the users will get MSVC support (if adding this support is doable). Regards, &rzej;

AMDG On 03/11/2017 07:27 AM, Paul A. Bristow via Boost wrote:

John Maddock has since explained why nothing I tried worked. I'm a bit shocked that it hasn't been tested on MSVC. My acceptance was on the assumption that it would work. It really must be portable over recent GCC, Clang and MSVC at the very minimum.

I suggest that we should pause the review until you adopt John's patches and reissue the review code and then restart the review.

It'll be a bit poor to accept the library until a few people confirm it's working on MSVC.

VC2017 (which is the minimum required to build the library, even with patches) wasn't released until halfway through the review. In Christ, Steven Watanabe