Mercurial > hg > shenandoah-preopenjdk-archive > openjdk8 > jdk
changeset 10203:147843e7006a
8039915: Wrong NumberFormat.format() HALF_UP rounding when last digit exactly at rounding position greater than 5
Summary: Fixes erroneous rounding in DigitList for corner cases uncovered previously. Adds dedicated unit tests to TieRoundingTest
Reviewed-by: bpb, darcy
Contributed-by: Olivier Lagneau <olivier.lagneau@oracle.com>
| author | bpb |
|---|---|
| date | Fri, 17 Oct 2014 11:45:59 -0700 |
| parents | 8027d51f791d |
| children | 202d1d06b2c1 |
| files | src/share/classes/java/text/DigitList.java test/java/text/Format/DecimalFormat/TieRoundingTest.java |
| diffstat | 2 files changed, 79 insertions(+), 57 deletions(-) [+] |
line wrap: on
line diff
--- a/src/share/classes/java/text/DigitList.java Fri Oct 17 11:33:22 2014 -0700 +++ b/src/share/classes/java/text/DigitList.java Fri Oct 17 11:45:59 2014 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 1996, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 1996, 2014, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -290,25 +290,26 @@ FloatingDecimal.BinaryToASCIIConverter fdConverter = FloatingDecimal.getBinaryToASCIIConverter(source); boolean hasBeenRoundedUp = fdConverter.digitsRoundedUp(); - boolean allDecimalDigits = fdConverter.decimalDigitsExact(); + boolean valueExactAsDecimal = fdConverter.decimalDigitsExact(); assert !fdConverter.isExceptional(); String digitsString = fdConverter.toJavaFormatString(); set(isNegative, digitsString, - hasBeenRoundedUp, allDecimalDigits, + hasBeenRoundedUp, valueExactAsDecimal, maximumDigits, fixedPoint); } /** * Generate a representation of the form DDDDD, DDDDD.DDDDD, or * DDDDDE+/-DDDDD. - * @param roundedUp Boolean value indicating if the s digits were rounded-up. - * @param allDecimalDigits Boolean value indicating if the digits in s are - * an exact decimal representation of the double that was passed. + * @param roundedUp whether or not rounding up has already happened. + * @param valueExactAsDecimal whether or not collected digits provide + * an exact decimal representation of the value. */ private void set(boolean isNegative, String s, - boolean roundedUp, boolean allDecimalDigits, + boolean roundedUp, boolean valueExactAsDecimal, int maximumDigits, boolean fixedPoint) { + this.isNegative = isNegative; int len = s.length(); char[] source = getDataChars(len); @@ -361,7 +362,7 @@ } else if (-decimalAt == maximumDigits) { // If we round 0.0009 to 3 fractional digits, then we have to // create a new one digit in the least significant location. - if (shouldRoundUp(0, roundedUp, allDecimalDigits)) { + if (shouldRoundUp(0, roundedUp, valueExactAsDecimal)) { count = 1; ++decimalAt; digits[0] = '1'; @@ -381,25 +382,26 @@ // Eliminate digits beyond maximum digits to be displayed. // Round up if appropriate. round(fixedPoint ? (maximumDigits + decimalAt) : maximumDigits, - roundedUp, allDecimalDigits); - } + roundedUp, valueExactAsDecimal); + + } /** * Round the representation to the given number of digits. * @param maximumDigits The maximum number of digits to be shown. - * @param alreadyRounded Boolean indicating if rounding up already happened. - * @param allDecimalDigits Boolean indicating if the digits provide an exact - * representation of the value. + * @param alreadyRounded whether or not rounding up has already happened. + * @param valueExactAsDecimal whether or not collected digits provide + * an exact decimal representation of the value. * * Upon return, count will be less than or equal to maximumDigits. */ private final void round(int maximumDigits, boolean alreadyRounded, - boolean allDecimalDigits) { + boolean valueExactAsDecimal) { // Eliminate digits beyond maximum digits to be displayed. // Round up if appropriate. if (maximumDigits >= 0 && maximumDigits < count) { - if (shouldRoundUp(maximumDigits, alreadyRounded, allDecimalDigits)) { + if (shouldRoundUp(maximumDigits, alreadyRounded, valueExactAsDecimal)) { // Rounding up involved incrementing digits from LSD to MSD. // In most cases this is simple, but in a worst case situation // (9999..99) we have to adjust the decimalAt value. @@ -440,6 +442,9 @@ * <code>count-1</code>. If 0, then all digits are rounded away, and * this method returns true if a one should be generated (e.g., formatting * 0.09 with "#.#"). + * @param alreadyRounded whether or not rounding up has already happened. + * @param valueExactAsDecimal whether or not collected digits provide + * an exact decimal representation of the value. * @exception ArithmeticException if rounding is needed with rounding * mode being set to RoundingMode.UNNECESSARY * @return true if digit <code>maximumDigits-1</code> should be @@ -447,7 +452,7 @@ */ private boolean shouldRoundUp(int maximumDigits, boolean alreadyRounded, - boolean allDecimalDigits) { + boolean valueExactAsDecimal) { if (maximumDigits < count) { /* * To avoid erroneous double-rounding or truncation when converting @@ -460,7 +465,7 @@ * account what FloatingDecimal has done in the binary to decimal * conversion. * - * Considering the tie cases, FloatingDecimal may round-up the + * Considering the tie cases, FloatingDecimal may round up the * value (returning decimal digits equal to tie when it is below), * or "truncate" the value to the tie while value is above it, * or provide the exact decimal digits when the binary value can be @@ -490,7 +495,7 @@ * * - For other numbers that are always converted to exact digits * (like BigInteger, Long, ...), the passed alreadyRounded boolean - * have to be set to false, and allDecimalDigits has to be set to + * have to be set to false, and valueExactAsDecimal has to be set to * true in the upper DigitList call stack, providing the right state * for those situations.. */ @@ -520,42 +525,31 @@ } break; case HALF_UP: - if (digits[maximumDigits] >= '5') { - // We should not round up if the rounding digits position is - // exactly the last index and if digits were already rounded. - if ((maximumDigits == (count - 1)) && - (alreadyRounded)) - return false; - - // Value was exactly at or was above tie. We must round up. - return true; - } - break; case HALF_DOWN: if (digits[maximumDigits] > '5') { + // Value is above tie ==> must round up return true; - } else if (digits[maximumDigits] == '5' ) { - if (maximumDigits == (count - 1)) { - // The rounding position is exactly the last index. - if (allDecimalDigits || alreadyRounded) - /* FloatingDecimal rounded up (value was below tie), - * or provided the exact list of digits (value was - * an exact tie). We should not round up, following - * the HALF_DOWN rounding rule. - */ - return false; - else - // Value was above the tie, we must round up. - return true; - } - - // We must round up if it gives a non null digit after '5'. - for (int i=maximumDigits+1; i<count; ++i) { - if (digits[i] != '0') { - return true; + } else if (digits[maximumDigits] == '5') { + // Digit at rounding position is a '5'. Tie cases. + if (maximumDigits != (count - 1)) { + // There are remaining digits. Above tie => must round up + return true; + } else { + // Digit at rounding position is the last one ! + if (valueExactAsDecimal) { + // Exact binary representation. On the tie. + // Apply rounding given by roundingMode. + return roundingMode == RoundingMode.HALF_UP; + } else { + // Not an exact binary representation. + // Digit sequence either rounded up or truncated. + // Round up only if it was truncated. + return !alreadyRounded; } } } + // Digit at rounding position is < '5' ==> no round up. + // Just let do the default, which is no round up (thus break). break; case HALF_EVEN: // Implement IEEE half-even rounding @@ -569,7 +563,7 @@ // then we should not round up again. return false; - if (!allDecimalDigits) + if (!valueExactAsDecimal) // Otherwise if the digits don't represent exact value, // value was above tie and FloatingDecimal truncated // digits to tie. We must round up.
--- a/test/java/text/Format/DecimalFormat/TieRoundingTest.java Fri Oct 17 11:33:22 2014 -0700 +++ b/test/java/text/Format/DecimalFormat/TieRoundingTest.java Fri Oct 17 11:45:59 2014 -0700 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012, 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2012, 2014, Oracle and/or its affiliates. All rights reserved. * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. * * This code is free software; you can redistribute it and/or modify it @@ -23,7 +23,7 @@ /* @test * - * @bug 7131459 + * @bug 7131459 8039915 * @summary test various situations of NumberFormat rounding when close to tie * @author Olivier Lagneau * @run main TieRoundingTest @@ -56,7 +56,7 @@ if (!result.equals(expectedOutput)) { System.out.println(); System.out.println("========================================"); - System.out.println("***Error formatting double value from string : " + + System.out.println("***Failure : error formatting value from string : " + inputDigits); System.out.println("NumberFormat pattern is : " + ((DecimalFormat ) nf).toPattern()); @@ -103,7 +103,7 @@ if (!result.equals(expectedOutput)) { System.out.println(); System.out.println("========================================"); - System.out.println("***Error formatting double value from string : " + + System.out.println("***Failure : error formatting value from string : " + inputDigits); System.out.println("NumberFormat pattern is : " + ((DecimalFormat ) nf).toPattern()); @@ -144,7 +144,7 @@ if (!result.equals(expectedOutput)) { System.out.println(); System.out.println("========================================"); - System.out.println("***Error formatting number value from string : " + + System.out.println("***Failure : error formatting value from string : " + inputDigits); System.out.println("NumberFormat pattern is : " + ((DecimalFormat ) nf).toPattern()); @@ -174,7 +174,7 @@ public static void main(String[] args) { - // Only the 3 rounding modes below may be impacted by bug 7131459. + // The 3 HALF_* rounding modes are impacted by bugs 7131459, 8039915. // So we do not test the other rounding modes. RoundingMode[] roundingModes = { RoundingMode.HALF_DOWN, @@ -183,10 +183,14 @@ }; // Precise the relative position of input value against its closest tie. + // The double values tested below for 3 and 5 fractional digits must follow + // this scheme (position toward tie). String[] tieRelativePositions = { "below", "exact", "above", "below", "exact", "above", "below", "exact", "above", + "below", "above", "above", + "below", "below", "above", "below", "exact", "above" }; @@ -196,9 +200,13 @@ double[] values3FractDigits = { // unimpacting values close to tie, with less than 3 input fract digits 1.115d, 1.125d, 1.135d, - // impacting close to tie values covering all 6 cases + // HALF_* impacting close to tie values covering all 6 tie cases 0.3115d, 0.3125d, 0.3135d, 0.6865d, 0.6875d, 0.6885d, + // specific HALF_UP close to tie values + 0.3124d, 0.3126d, 0.3128d, + // specific HALF_DOWN close to tie values + 0.6864d, 0.6865d, 0.6868d, // unimpacting values close to tie, with more than 3 input fract digits 1.46885d, 2.46875d, 1.46865d }; @@ -207,6 +215,8 @@ "1.115d", "1.125d", "1.135d", "0.3115d", "0.3125d", "0.3135d", "0.6865d", "0.6875d", "0.6885d", + "0.3124d", "0.3126d", "0.3128d", + "0.6864d", "0.6865d", "0.6868d", "1.46885d", "2.46875d", "1.46865d" }; @@ -214,16 +224,22 @@ {"1.115", "1.125", "1.135", "0.311", "0.312", "0.314", "0.686", "0.687", "0.689", + "0.312", "0.313", "0.313", + "0.686", "0.686", "0.687", "1.469", "2.469", "1.469" }, {"1.115", "1.125", "1.135", "0.311", "0.312", "0.314", "0.686", "0.688", "0.689", + "0.312", "0.313", "0.313", + "0.686", "0.686", "0.687", "1.469", "2.469", "1.469" }, {"1.115", "1.125", "1.135", "0.311", "0.313", "0.314", "0.686", "0.688", "0.689", + "0.312", "0.313", "0.313", + "0.686", "0.686", "0.687", "1.469", "2.469", "1.469" }, }; @@ -250,9 +266,13 @@ double[] values5FractDigits = { // unimpacting values close to tie, with less than 5 input fract digits 1.3135d, 1.3125d, 1.3115d, - // impacting values close to tie, covering all 6 cases + // HALF_* impacting values close to tie, covering all 6 cases 1.328115d, 1.328125d, 1.328135d, 1.796865d, 1.796875d, 1.796885d, + // specific HALF_UP close to tie values + 1.328124d, 1.798876d, 1.796889d, + // specific HALF_DOWN close to tie values + 1.328114d, 1.796865d, 1.328138d, // unimpacting values close to tie, with more than 5 input fract digits 1.3281149999999d, 1.75390625d, 1.7968750000001d }; @@ -261,6 +281,8 @@ "1.3135d", "1.3125d", "1.3115d", "1.328115d", "1.328125d", "1.328135d", "1.796865d", "1.796875d", "1.796885d", + "1.328124d", "1.798876d", "1.796889d", + "1.328114d", "1.796865d", "1.328138d", "1.3281149999999d", "1.75390625d", "1.7968750000001d" }; @@ -268,16 +290,22 @@ {"1.3135", "1.3125", "1.3115", "1.32811", "1.32812", "1.32814", "1.79686", "1.79687", "1.79689", + "1.32812", "1.79888", "1.79689", + "1.32811", "1.79686", "1.32814", "1.32811", "1.75391", "1.79688" }, {"1.3135", "1.3125", "1.3115", "1.32811", "1.32812", "1.32814", "1.79686", "1.79688", "1.79689", + "1.32812", "1.79888", "1.79689", + "1.32811", "1.79686", "1.32814", "1.32811", "1.75391", "1.79688" }, {"1.3135", "1.3125", "1.3115", "1.32811", "1.32813", "1.32814", "1.79686", "1.79688", "1.79689", + "1.32812", "1.79888", "1.79689", + "1.32811", "1.79686", "1.32814", "1.32811", "1.75391", "1.79688" } };