Re: [lmi] [PATCH] Fix value_cast defect shown by the unit test

[Greg Chicares]

On 2017-01-06 22:53, Vadim Zeitlin wrote:
> 

[...pointers (and therefore arrays) infelicitously convert to bool, so
  value_cast<bool>("0");
converts to true because the array is implicitly converted to a pointer
that is not null...]

>  The simplest fix I can propose is just testing for !is_array<>, however
> this still leaves us with the first warning, from Boost type traits
> library. I don't see any simple way to avoid it and I definitely don't want
> to disable the useful -Waddress warning, so instead I propose to just
> switch to the standard C++11 <type_traits> which doesn't suffer from this
> problem.

Switching from boost to std is always good, especially when it removes
a defect. I was tempted to replace type_traits globally throughout lmi,
but feared that might collide with work you've already done. Then again,
should I do it anyway? If you've done it separately, then comparison
would show whether I make a mistake that you avoided.

And maybe it's also time for me to do this:

http://lists.nongnu.org/archive/html/lmi/2008-06/msg00061.html
| I'm thinking of writing a numeric_value_cast<T>() that doesn't
| use boost at all. The original boost::numeric_cast documentation
| seemed to say value preservation was an asserted postcondition:
|   "An exception is thrown when a runtime value-preservation
|   check fails."
| but that's not true of either the old or the new boost version.

[where "new" meant boost-1.34 or -1.35], thus rendering this whole file
boost-free. The archetypal version of this code was something like

    template<typename FROM, typename TO>
    TO number_cast(FROM z)
        {
        std::static_assert(std::is_arithmetic(FROM));
        std::static_assert(std::is_arithmetic(TO));
        if((z < 0) && std::is_unsigned(TO))       throw ...
        if(z < std::numeric_limits<TO>::lowest()) throw ...
        if(std::numeric_limits<TO>::max() < z)    throw ...
        // THIS IS WHERE I ADDED SOMETHING
        return static_cast<TO>(z);
        }

and I added a something like

        TO r = static_cast<TO>(z);
        if(r == z) return z; else throw ...

to make it fulfill its value-preservation guarantee.

>  The patch at 
> 
>       https://github.com/vadz/lmi/pull/50
> 
> contains both changes: the switch to standard functions in the first commit
> and the fix for the problem in the second one. I've tested this with gcc
> 6.3, clang (pre-4.0) and MinGW 4.9.1 used for the official builds, the test
> now passes with all of them, including the newly re-enabled test.

Where you had proposed:

-        convertible = std::is_convertible<From,To>::value
+        convertible =
+               !std::is_array<From>::value
+            &&  std::is_convertible<From,To>::value

I did this instead:

         convertible =
                 std::is_convertible<From,To>::value
             &&!(std::is_array   <From>::value && std::is_same<bool,To>::value)
             &&!(std::is_pointer <From>::value && std::is_same<bool,To>::value)

I think it accomplishes all you intended, but more selectively, by
disregarding undesirable conversions specifically to bool (only).
I'll push this changeset in a moment.