[PATCH] 4511638: Double.toString(double) sometimes produces incorrect results

[Raffaello Giulietti]

Hi Brian,

On 2018-09-27 01:53, Brian Burkhalter wrote:
> There was a compilation error on Linux in one of the tests:
> 
> test/jdk/java/lang/Floating/DoubleToDecString.java:133: error: unmappable character (0x93) for encoding US-ASCII
>      Paxson V, "A Program for Testing IEEE Decimal\ufffd\ufffd\ufffdBinary Conversion"
> 
> This was only in one of the comments where it looks like some errant character leaked in. I updated the webrev (with accompanying patch) in place [1] after verifying that the change to the test fixes the problem.
> 

I can confirm that in my original source there is a 'EN DASH' (U+2013) 
Unicode character, which visually looks similar to 'HYPHEN-MINUS' 
(U+002D). I use UTF-8 on all my source files, so it didn't stand out as 
something strange in the IDE.

This is certainly due to some copy&paste operation from a source found 
on the internet. U+2013 is the correct variant, both semantically and 
typographically, but it surely causes problems with tools that expect 
US-ASCII. Frankly, there should be no such restrictions in tools, as 
Java supports Unicode source code since day 0, but that's another story.

For this project, I will switch to US-ASCII in my IDE.



> Also, there were a couple of failures observed in test [2] at lines 172 and 173. If at line 172 "foo1.17549435E-38” is changed to "foo1.1754944E-38” then the evaluation at that line succeeds but then the one at line 173 fails. Making a similar change on this line does not help. I suspect that this is due to a difference between the new code and that in jdk.internal.math.FloatingDecimal which is used by java.lang.AbstractStringBuilder and java.lang.invoke.StringConcatFactory but I’ve not actually investigated this as yet. I had actually wondered whether some or all of the FloatingDecimal code could be superseded by the updated logic.
> 

"1.1754944E-38" is shorter than "1.17549435E-38" and can still recover 
Float.MIN_NORMAL. That's why the new code returns the shorter variant.

Why this works when the replacement is made in line 172 but not in line 
173 is really counter-intuitive. The only superficial difference is that 
FLOAT_MIN_NORM_1 is declared final while FLOAT_MIN_NORM_2 is not, 
although both are initialized with the same value.

The black-box conclusion is that it seems that string concatenation 
performed at compile time and at runtime manages to produce different 
results when given the same values. This is of course a problem outside 
the scope of the Double/Float conversions. As you point out, a more 
white-box analysis might reveal the culprit in the deadly embrace 
between string building and FloatingDecimal.

Unfortunately, I won't have time to investigate this interesting issue 
further before the weekend.


Greetings
Raffaello




> Brian
> 
> [1] http://cr.openjdk.java.net/~bpb/4511638/webrev.00/
> [2] java/lang/String/concat/ImplicitStringConcatBoundaries.java
>