A guide to PR3402 ("[Double,Float].toString()") code reviewers

Raffaello Giulietti raffaello.giulietti at gmail.com
Tue Nov 16 20:10:21 UTC 2021


Hello,

as mentioned in [1], the Schubfach writing accompanying JBS issues [2] 
and [3] has recently undergone a thorough review by renowned researchers 
(in addition to Dmitry Nadezhin back in 2018-19 and myself on a 
continuous basis) [4].

For the *code* reviewers of PR [5] this means that there's no need for 
them to deal with the more mathematical aspects of the writing. They can 
instead concentrate in crosschecking that the Java code is in sync with 
the pseudo-code in the writing, as well as about its readability.

To facilitate this review, there are comments scattered in the code that 
refer back to the writing for easy reference.


# MathUtils

For the flog*() methods in MathUtils refer to section 9.1.1.

The large lookup table in MathUtils is defined in result 24 as well as 
in javadoc.
MathUtilsChecker contains fully commented code that tests the 
correctness of the table as well as the flog*() methods to an extent 
sufficient for the conversions of doubles.
(The table has also been checked independently by one of the article 
reviewers.)


# DoubleToDecimal

With regard to DoubleToDecimal, the fast path described in 8.3 is coded 
as part of toDecimal(double).

In DoubleToDecimal, toDecimal(int,long,int) is a Java translation of the 
Schubfach skeleton in figure 7 combined with the suggestions in figure 9.

Figure 8 is embodied in rop(long,long,long).

§10 about digits extraction is implemented in toChars(long,int) and its 
dependencies.

The rest of the code is just glue and should be easy to understand.


# FloatToDecimal

The code of FloatToDecimal is almost identical but adapted to the 32 bit 
case. Method rop(long,long) is from figure 11.


# Test classes

I hope it turns out that the test classes are either commented well 
enough to guide the reader or are kind of straightforward to understand.



There are some package private constants in the code that are not 
referenced anywhere else in src/ and could thus be removed. However, 
since there's no great harm retaining them and since they might turn out 
to be useful in later work, I would suggest not to get rid of them.


Of course, I'm at the disposal of this community for any question 
related to the PR and the writing.


HTH
Raffaello

----

[1] 
https://mail.openjdk.java.net/pipermail/core-libs-dev/2021-November/083536.html
[2] https://bugs.openjdk.java.net/browse/JDK-4511638
[3] https://bugs.openjdk.java.net/browse/JDK-8202555
[4] https://drive.google.com/file/d/1IEeATSVnEE6TkrHlCYNY2GjaraBjOT4f
[5] https://github.com/openjdk/jdk/pull/3402


More information about the core-libs-dev mailing list