[Issue 5232] New: [patch] std.conv.to & std.conv.roundTo report invalid overflows for very large numbers

[d-bugmail at puremagic.com]

http://d.puremagic.com/issues/show_bug.cgi?id=5232

           Summary: [patch] std.conv.to & std.conv.roundTo report invalid
                    overflows for very large numbers
           Product: D
           Version: D2
          Platform: Other
        OS/Version: Windows
            Status: NEW
          Keywords: patch
          Severity: minor
          Priority: P2
         Component: Phobos
        AssignedTo: nobody at puremagic.com
        ReportedBy: sandford at jhu.edu


--- Comment #0 from Rob Jacques <sandford at jhu.edu> 2010-11-17 22:00:45 PST ---
This comes from using roundTo!ulong with a real equal to ulong.max. Here's the
unit test:

    real r = ulong.max;
    assert(   (cast(ulong)r) == ulong.max , "Okay");
    assert(      to!ulong(r) == ulong.max , "Okay");
    assert( roundTo!ulong(r) == ulong.max , "Conversion overflow");

The reason for this is that toImpl uses casting, which implies truncation, but
tests for overflow by simple comparison and roundTo adds 0.5 to the source
value. Here's a patch for toImpl:

T toImpl(T, S)(S value)
if (!implicitlyConverts!(S, T)
        && std.traits.isNumeric!(S) && std.traits.isNumeric!(T))
{
    enum sSmallest = mostNegative!(S);
    enum tSmallest = mostNegative!(T);
    static if (sSmallest < 0) {
        // possible underflow converting from a signed
        static if (tSmallest == 0) {
            immutable good = value >= 0;
        } else {
            static assert(tSmallest < 0);
            immutable good = value >= tSmallest;
        }
        if (!good) ConvOverflowError.raise("Conversion negative overflow");
    }
    static if (S.max > T.max) {
        // possible overflow
-        if (value > T.max) ConvOverflowError.raise("Conversion overflow");
+        if (value >= T.max+1.0L) ConvOverflowError.raise("Conversion
overflow");
    }
    return cast(T) value;
}

As a note, the roundTo unit test still fails because reals can't represent
ulong.max+0.5 and thus the implementation of roundTo effectively adds 1.0
instead of 0.5 (And thus it is still broken) Also, it should probably use
template constraints instead of static asserts. Here's a patch for both issues:

template roundTo(Target) {
    Target roundTo(Source)(Source value)
        if( isFloatingPoint!Source && isIntegral!Target )
    {
        return to!(Target)( round(value) );
    }
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------