New strtod for MSVC <= 2013

[JohanEngelen]

Adding David Gay's dtoa.c for a well-known strtod implementation that accepts hexadecimal floating point constants. This fixes issue #761, and fixes bunch of unittests of math.d.
Compilation of dtoa.c and the call to strtod_davidgay is only enabled for MSVC <= 2013.
Only tested on Win64, not tested on Win32.

[redstar]

Thanks for the pull request. After the hint from @smolt I implemented strtold for MSVC in terms of the the LLVM functions. May you can try it.
But your work does not vanish: I think I include the strtod into druntime to fix the problem for D applications.

[JohanEngelen]

I run into a problem with LLVM's strtod during compilation of std/string.d's unittest in debug mode:

...\>  ldc2 -main -unittest -w -d -g ..\runtime\phobos\std\string.d

results in:

   Assertion failed: (*p == 'e' || *p == 'E') && "Invalid character in significand", file ..\lib\Support\APFloat.cpp, line 272

and a long stacktrace after.

[JohanEngelen]

Minimum test case for the crash:

import std.math, std.string, std.conv;
void main() { 
    enum bool hoi = (isNumeric(to!string(real.nan)) == true);
}

See lines 3722-3729 in phobos/std/string.d

[smolt]

When you compile the minimum case with normal ldc or dmd, do you get an error message that ends like this?

testcase.d(6): called from here: to(nanL)

If so, the Assertion being hit is while preparing that message. I am looking at same problem, but in my case it is real.infinity instead of real.nan.

I think it has something to do with APFloat::toString() and convertFromString() not being compatible. In LLVM 3.5.1 which I am using right now, toString() produces "Inf" for infinity where as convertFromString() expects either "inf" or "INFINITY".

PrettyPrintVisitor::floatToBuffer() in hdrgen.c is calling:

    ld_sprint(buffer, 'g', value);     // calls APFloat::toString()
    ...
    real_t r = Port::strtold(buffer, NULL);  // calls APFloat::convertFromString()

[JohanEngelen]

(I am using LLVM 3.7 trunk from yesterday)

The ld_sprint(buffer, 'g', value); call gives "1.#QNAN". Isn't the problem that ld_sprint does not call APFloat::toString() but instead calls the system sprintf?

[JohanEngelen]

After changing the call to ld_sprint() with APFloat::toString(), the test case works with real.nan but still crashes on real.infinity. toString generates "+Inf", which crashes convertFromString.

How to proceed?
The unrecoverable errors of APFloat::convertFromString() mean that the input will have to be checked first to prevent crashes.

[smolt]

Sorry, I was looking at a different branch that @redstar has called longdouble2. That branch does have ld_sprint(() calling APFloat::toString via this function called format(). One possibility is to intercept the special values (infinity and nan) and use something that APFoat::convertFromString() can happily parse.

APFloat::convertFromString() only accepts case-sensitive "inf" or "INFINITY" and "nan" or "NaN", so something like this seems to work:

int ldc::longdouble::format (char *buf) const
{
    // check into hdrgen, what happens if inf or nan?
    const llvm::APFloat *fp = value();

    if (fp->isNaN())
        strcpy(buf, "nan");
    else if (fp->isInfinity())
        strcpy(buf, fp->isNegative() ? "-inf" : "inf");
    else
    {
        llvm::SmallString<64> str;
        fp->toString(str, 0, 0);
        strcpy(buf, str.str().str().c_str());
    }
    return strlen(buf);
}

[JohanEngelen]

Yes, I had a similar fix. Hope this lands soon in master.

[smolt]

Good. I noticed that using APFloat::toString() affects header (.di) generation because ld_sprint is used for that (in dmd2/hdrgen.c). I don't understand it all, but there are probably more tweaks to go. Anyway, nice there is some progress.

[JohanEngelen]

@redstar : See smolt's fix to ldc::longdouble::format for your longdouble2 branch

[smolt]

The fix really only needs to intercept infinity values since convertFromString accepts "NaN" and toString produces "NaN". "nan" looked better to me because that is what my printf produces.

[dnadlinger]

Windows people (@redstar, @kinke): Is this still relevant? Can you please close this PR otherwise? Trying to clean house.

[JohanEngelen]

OK, I am going to close this. I think it is no longer relevant, we use MSVC 2015.