Re: [lmi] Is bourn_cast demonstrably correct?

[Greg Chicares]

On 2017-03-21 13:07, Vadim Zeitlin wrote:
> On Mon, 20 Mar 2017 23:56:18 +0000 Greg Chicares <address@hidden> wrote:
[...]
> GC> Now I believe the flaw was in the documentation, not the code.
> 
>  I've tried to read your entire reply carefully, but I still can't find
> what exactly motivated your change of mind.

It keeps changing still. It's not immediately obvious to me that the
arguments on either side should be dismissed, and a final decision
doesn't need to be made yet.

> My position is that bourn_cast<> looks like a safe cast and I wouldn't
> expect it to silently truncate values, especially as there doesn't seem to
> be any situation in which this would be useful -- but it's easy to imagine
> a situation in which it could happen unexpectedly/accidentally. And there
> is also another facility in lmi which can, and should, be used when we
> really need to round a floating point number to an integer, so why should
> this cast do it too?

Yes, my thinking is moving in that direction.

> GC> I think the best answer is to keep bourn_cast as a "guarded" wrapper
> GC> for static_cast, which throws only when the behavior of static_cast
> GC> is surprising (-1U --> FFFFFFFF) or undefined (DBL_MAX --> int); and
> GC> to provide a wrapper-wrapper like numeric_value_cast() above if value
> GC> preservation is important.

We can either accept operations that lose precision, or throw. Actually,
we don't have to make that decision yet: right now, we use
  boost::numeric_cast, which may lose precision; and
  numeric_value_cast, which throws if the value is not preserved.
The latter is a wrapper around the former. My first goal is to replace
the boost library without breaking lmi. But I'm quite sure lmi never
lets boost::numeric_cast truncate: the uses found by this command
  $grep numeric_cast *.?pp
are confined to
 - implementing numeric_value_cast
 - demonstrating boost's loss of value, in a unit test
 - converting integral results of std::strtol() and std::strtoul()
     in order to detect counterintuitive (narrowing) conversions
Therefore, I think we'll conclude that we really want only strict
value-preserving semantics. I even believe I can prove that. But I'm
an applied rather than a theoretical mathematician, so I don't believe
my own proofs until I've validated them with unit tests, and also run
regression tests to make assurance doubly sure.

I think the rest is just implementation detail. If it makes good
design sense to have something equivalent to boost::numeric_cast as an
implementation detail, and expose only the safer cast, that's okay. Or
we might want separate casts for integral and floating types. Or we may
find that a single template function is the best design--that's where I
now think we'll end up, because I suspect I can accomplish that by
adding this:

    if(!to_traits::is_integer)
        {
        static_assert(to_traits::is_integer || to_traits::is_iec559, "");
        // Here's where we'd test whether value is preserved.
        return static_cast<To>(from);
        }

to 'bourn_cast.hpp' in HEAD, right after the pragma that turns off
certain warnings.

BTW...doesn't that static_assert look weird? If I instead write

    if(!to_traits::is_integer)
        {
//      static_assert(to_traits::is_integer || to_traits::is_iec559, "");
        static_assert(to_traits::is_iec559, "");
        }

then I get

/opt/lmi/src/lmi/bourn_cast.hpp: In instantiation of 'To bourn_cast(From) \
  [with To = long unsigned int; From = long int]':
/opt/lmi/src/lmi/bourn_cast.hpp:94:9: error: static assertion failed: 
         static_assert(to_traits::is_iec559, "");        
         ^

but I thought a block-level static_assert would apply only inside its
block--and this block shouldn't be entered with "To = long unsigned int".
It compiles if I write the assertion with the redundant condition above.

>  I'd prefer to turn it around and use bourn_cast<> for integers only and
> have some round_cast<> for the floating point numbers if necessary (right
> now it doesn't seem to be needed at all, however, unless I'm missing
> something).

Consider the calendar_date example recently discussed. A root-finding
function gives us a JDN as a variable of type double that has been
rounded to an exact integral value, and we want to convert that value
to type int. We'd like to have a general-purpose routine to do that,
which will throw if the JDN is outside [INT_MIN, INT_MAX] or if we
somehow failed to round it correctly.

And I do want lmi's value_cast to remain a Grand Unified Cast that makes
everything interconvertible. Right now, it uses numeric_value_cast<>()
for all arithmetic <-> arithmetic conversions; we'd need to design
something to take its place if bourn_cast is restricted to integers.