On 11/03/2017 21:35, Robert Ramey via Boost wrote:
On 3/11/17 11:37 AM, John Maddock via Boost wrote:
OK, here's my review of Robert's library.
By way of something different, I've tried to not read the docs (though I ended up scanning them in the end) and just tried to use the library. Here's what I've found:
1) Using:
safe<int>(-1) & 1
I get a hard assert failure at safe_base_operations.hpp:1373. I'm assuming that this is a mistake, and the usual error handlers should be called? Bitwise or has the same issue. More tests required ;)
bit wise operators have issues both in concept and implementation which I'm in the process of addressing.
2) I'm not sure if this is an msvc issue but in:
constexpr safe_base operator~()
The static assert is triggered for *unsigned types* and not for signed. Even if this is a compiler issue, it indicates a missing test case or two.
I've been as how to address bit wise operations on signed integers. I've been thinking that they are an error which should be trapped at compile time. First of all I don't seen a use case for them and second they have portability problems. the results are arithmetically different depending on the size of the int. I can do it either way, it's really a question about what the library is meant to do - insist on arithmetical consistence and correctness - or ... what? For now I've trapped attempts to use bit wise operations on signed integers and permitted them on unsigned integers. But as you can see, i'm conflicted here. I wish we had static_warning in C++ but we don't.
Bitwise operations on signed types are well defined *as long as the value is positive*, as soon as the sign bit is set, or you use an operation which would touch it some way then you have undefined behaviour. For positive values, bitwise ops are genuinely useful too - the usual case is the test for even/oddness with `x & 1`.
the results are arithmetically different depending on the size of the int
Huh? I don't think so, not for positive values. While we're at it, operator^ doesn't behave as expected: safe<int>(4) ^ 1 Generates a static assert: "error C2338: safe type cannot be constructed with this type" Leaving aside the fact that I don't understand the error message, the result of 4 ^1 is 5 and certainly can be constructed for that type. So I believe the correct behaviour should be: * Bitwise operations on signed types should be allowed by default as long as they don't invoke undefined behaviour by operating on (or generating) negative values - this should be a runtime check. * operator ~ always touches the sign bit so is undefined for signed types - however your static assert currently fails for *unsigned* types (at least with msvc). * It would be nice to be able to alter the behaviour via a static_assert - is it possible for the error policy to static assert on a bitwise op on a signed type - so that all bitwise ops on signed types become compiler errors? Regards, John. --- This email has been checked for viruses by AVG. http://www.avg.com