JDK 14 RFR of JDK-8233452: java.math.BigDecimal.sqrt() with RoundingMode.FLOOR results in incorrect result

[Joe Darcy]

Hi Brian,

Pushed with recommend changes; webrev of final version:

     http://cr.openjdk.java.net/~darcy/8233452.5

Thanks,

-Joe

On 1/14/2020 1:59 PM, Brian Burkhalter wrote:
> Hi Joe,
>
> This looks good modulo a few picayune things I noticed in the 
> implementation and test. Line numbers refer to the updated version of 
> each file.
>
> 1. Implementation
>
> 2: Newer copyright year 2020, of course.

Will do.

> 2150: s/much many/many/.
> 2185: I suppose that this is in case targetPrecision overflowed.

Right; I'll add a comment to make that explicit.

The code as written is probably not fully overflow-hardened, but I at 
least wanted to avoid certain infinite loops from coming up.


> 2231: Why ulp = ulp?


Just a typo; will fix before pushing.

> 2376: s/larger than/larger/
>
> 2. Test
>
> 2: Newer copyright year 2020, of course.
> 39: Perhaps for symmetry with BigInteger 
> (https://bugs.openjdk.java.net/browse/JDK-8152237) we should add 
> BigDecimal.TWO at a later date.
> 112: s/valueOf(2)/TWO/
> 117: Alignment of RoundingMode.DOWN
> 137-138:
>
> Probe inputs with two digits of precision, 10 … 99 and those
> values scaled by 10^-1, 1.0, … 9.9, and by 10^-2, 0.1, … 0.99.


Corrected.


>
> 167: Two spaces after return
> 282, 303: “doesn’t not improperly” sounds awkward: should be “does not 
> improperly”?
> 289, 310: “Sqrt root” also sounds awkward
> 296, 317: Extra space before “:”
> 316: s/down/up/ ?

Yep; cut and paste issue.


> 337: s/rounding down/half even/ ?
> 362: s/near 1 half even/“near 1 “ + mc.toString()/ ?
> 397: delete commented out line?
> 520: s/+1)/+ 1)/
> 607: Introduce a square() method in BigSquareRoot also for parity with 
> BigDecimal?

Addressed.