Bug 13013 – Failed unittests in std.json - does not parse doubles correctly
Status
RESOLVED
Resolution
FIXED
Severity
enhancement
Priority
P1
Component
phobos
Product
D
Version
D2
Platform
x86_64
OS
Windows
Creation time
2014-06-30T20:21:00Z
Last change time
2014-08-21T18:22:02Z
Assigned to
nobody
Creator
markisaa
Comments
Comment #0 by markisaa — 2014-06-30T20:21:00Z
When compiling with -m64 on Windows, std.json fails to pass unittests regarding parsing doubles from string json format into the JSONValue type.
Specifically:
[email protected](1027): 1.44354e-312 should be 0.23
This does not happen when I leave off -m64.
This does not happen when I use -m64 on OSX.
I compile with the line:
dmd -main -unittest -m64 json.d
I downloaded the latest version of std.json to see if this had already been patched; it is still broken. The line number above refers to the version currently on Github.
I will likely spend some time investigating this myself, as I must eventually have a solution to this problem.
Comment #1 by code — 2014-06-30T20:31:46Z
When porting LDC to Windows, I had to deal with a large number of such issues that manifested in a similar way (absurd floating point results). The root cause always was that the MSVC runtime does not support reals ("long double" in C land, i.e. x87 80 bit floats) at all. This leads to situations where D code is calling a function (say snprintf) with some floating point arguments on the x87 stack, but the called function reads them from the XMM registers or the regular stack.
If I remember correctly, DMD also uses 80 bit reals on Win64. This can't work unless it comes with its own string formatting/math/etc. routines. For LDC/MinGW, we just use the MinGW ones which do properly support long doubles, but I'm not sure what the intended solution for DMD is.
Comment #2 by markisaa — 2014-06-30T20:38:53Z
Thanks for the insight David! I added a:
version (Win64) {
alias FloatingType = double;
} else {
alias FloatingType = real;
}
to the top and changed the appropriate union type to use that alias. The unittests ran perfectly.
I doubt that this is the fix we want to ship, but it seems that your analysis is spot-on.
Comment #3 by dmitry.olsh — 2014-06-30T20:45:55Z
JavaScript uses double exclusively why use 'real' in JSON to begin with?
Comment #4 by markisaa — 2014-06-30T20:53:47Z
Dimitry - Just had the same conversation with a co-worker :). Checked the blame log; was unable to quickly find the person who initially coded it up as real to find the rationale.
Comment #5 by dmitry.olsh — 2014-06-30T21:11:21Z
(In reply to Mark Isaacson from comment #4)
> Dimitry - Just had the same conversation with a co-worker :). Checked the
> blame log; was unable to quickly find the person who initially coded it up
> as real to find the rationale.
That stuff is old...
I'd pull a patch that fixes it to double without a second thought.
It still leaves the question of ABI violation as something separate. From the bug report I guess that not every test is run on Win64 ATM.