Re: [boost] [safe numerics] comment

Subject: Re: [boost] [safe numerics] comment
From: Vicente J. Botet Escriba (vicente.botet_at_[hidden])
Date: 2017-03-09 07:08:55

Le 09/03/2017 à 06:46, Robert Ramey via Boost a écrit :
> On 3/8/17 1:40 PM, Vicente J. Botet Escriba via Boost wrote:
>> Le 08/03/2017 à 19:52, Robert Ramey via Boost a écrit :
>
>>>> How can you implement the conversion from safe<U> to T if you can not
>>>> convert from U to T?
>>> I believe this would fail - probably with some totally confusing error
>>> message.  I think the best would be:
>>>
>>> a) Improve implementation of Numeric and other concepts with
>>> static_asserts.
>>> b) Use these implementations to check types when used.
>
>> I believe you should start by stating clearly the requirements. The
>> conversion between Numerics is needed if you want to support conversion
>> from safe<U> to T.
>
> The short form is that converting from a safe numeric type to any 
> other type will work if the arithmetic values of both types are the 
> same.  By the same token, converting any type to a safe numeric type 
> will work if
> the arithmetic value is preserved.  Any other conversion will invoke 
> error at compile or runtime.
>
I see. So your approach is to maintain the implicit conversion and check 
them at run-time.

I see that you have considered to have a more static safe interface that 
avoids common pitfall and require the user to state explicitly that she 
want run-time checks, and you don't want this.

We don't agree here on what we want, and there is no problem. It is just 
not the way I will do it, as I prefer to have compile errors than 
run-time errors.

Note the the review manager: Even if don't share the approach of the 
library I'm not saying the approach is not good for the Boost community. 
Others could take advantage of it.
>
>>>
>>>> We have implicit conversion from T to Safe and from Safe to T
>>>> template<class T>
>>>> constexpr /*explicit*/ safe_base(const T & t);
>>>> template<
>>>>          class R,
>>>>          typename std::enable_if<
>>>>              !boost::numeric::is_safe<R>::value,
>>>>              int
>>>>> ::type = 0
>>>>>
>>>>      constexpr operator R () const;
>>>> constexpr operator Stored () const;
>>>>
>>>> I believed that the conversion from Safe to T needed to be explicit.
>
> we disagree here.
No problem. There is no single solution to a problem.
See your code with an /*explicit*/ here
https://github.com/robertramey/safe_numerics/blob/master/include/safe_base.hpp#L204

     template<class T>
     constexpr /*explicit*/ safe_base(const T & t);

>>>
>>> I'm convinced of this.
> LOL I said this - but my brain must have been asleep.
>
> The concern is that implicit conversion would
>>> break the value checking.
>
>> I don't follow you here.
>
> By value checking I mean verification that the resulting value is the
> same as the original value.
I see.
>
>>> But I don't see that as a concern as
>>> value checking is implemented by overloading the binary assignment
>>> operator and the casting operator.  So
>>>
>>> void f(short x){
>>>     ...
>>> }
>>> safe<long> l = 42;
>
>> Here you have an implicit conversion int -> safe<long> and this is safe.
>
>>> short i = l; // will be checked at runtime and be OK
>> Here there is an implicit conversion int -> safe<long> -> short that is
>> not safe.
>
> yes it is.  if l is greater than numeric_limits<short>::max() then an
> error condition will be invoked.  It may fail - but it can never
> go undetected.
My bad. I used safe, when I meant static safe.
>
>> I believe that for a safe class, this should be forbidden at compile
>> time. If the user wants to do the conversion she must be explicit
>
> This would break just about all existing programs.  It would make the 
> folling code illegal
>
> safe<int> i = 0
> ...
> ++i
>
> The intention of the library is to make current program safe.  It is 
> not a new programming idiom
I understand but I don't adhere.
It maybe better than nothing, but I want more :)
>
>>
>>     short i = safe<short>(l);
>> or
>>     short i(l);
>>
>> Think about how std::duration works. When we can loss information,
>> implicit conversion are dangerous and should be avoided. This is what
>> you are trying to fix with your library, isn't it?
>
> Right - and I have.  But implicit conversion is not the culprit. It's 
> unchecked conversion which causes problems.
>
>>> f(l); // conversion to short will be checked at runtime and be OK
>> I will expect an explicit conversion here.
>
> again - we disagree.
No problem.
>
>
>>> l = 123412312321123;
>>> i = l; // will throw at runtime
>>> f(l); // will throw at runtime
>>>
>> You say that you are convinced, but your conversions/constructors are
>> implicit.
> Hmmm- Right - I was mixed up.
>>
>>
>>>> Having implicit conversion in both directions is suspect.
>
>>>> Why the explicit conversion between safe<T> and safe<U> are not 
>>>> allowed
>
> they are allowed.  as are implicit conversions.
>
>>> I believe they are.  implicit conversions as well.
>> Could you show how?
>
> safe<short> i = 1024;
> safe<char> j;
> j = i; // invokes  runtime error
>
> i = j; // can never invoke error and no runtime checking is invoked.

I meant to show the signature of the functions that are called during 
these assignments.
I had the impression that two user defined conversions were needed.

     safe<short> -> short -> char -> safe<char>

But I missed that you have an direct assignmet from

     safe<short> -> safe<char>

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

I was looking for a copy construction ** from     safe<short> -> safe<char>

safe<short> i = 1024;
safe<char> j(i);  // **

Wondering if as the copy constructor is not defined (Rule of Five), you 
haven't missed this conversion.

>>>> when T and U are not the same?
>>> they are.  as long as the arithmetic value doesn't change.
>> What do you mean?
>
> I think the above illustrates my point.
The difference is that with the explicit approach, the user must use 
explicit conversion when there is a possibility to loos data.
>
>
>> Can you consider adding what the library supports in the documentation.
>
> of course.
>
>>> I didn't need them.  What I did need and used is to create some traits
>>> "is_safe" and maybe a couple of others - off hand I don't remember.
>
>> Well, I believe that soon or later some one  would need a traits that
>> can be used using SFINAE.
>
> certainly not now.  I'm not sure it's there's a need or use for it.  
> But once concepts is available, it wouldn't be hard to upgrade the 
> current concept code to use concepts.  Then the the concepts could be 
> used as traits for dispatch.
>
>> I was looking fro it here and it is not
>> https://github.com/robertramey/safe_numerics/tree/master/include/concept
>>
> it's here - safe_numerics/include/concept/safe_numeric.hpp .  It's 
> based on BCC.  But I didn't use it in code.
>
My bad. I need to check my view :(

Best,
Vicente