8039915: Wrong NumberFormat.format() HALF_UP rounding when last digit exactly...

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: NOlivier Lagneau <olivier.lagneau@oracle.com>

src/share/classes/java/text/DigitList.java

 /*
- * 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 @@ final class DigitList implements Cloneable {
 
         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 @@ final class DigitList implements Cloneable {
             } 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 @@ final class DigitList implements Cloneable {
         // 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 @@ final class DigitList implements Cloneable {
      * <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 @@ final class DigitList implements Cloneable {
      */
     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 @@ final class DigitList implements Cloneable {
              *   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 @@ final class DigitList implements Cloneable {
              *
              * - 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 @@ final class DigitList implements Cloneable {
                 }
                 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') {
+                } 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 @@ final class DigitList implements Cloneable {
                             // 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.

test/java/text/Format/DecimalFormat/TieRoundingTest.java

 /*
- * 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 @@ public class TieRoundingTest {
         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 @@ public class TieRoundingTest {
         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 @@ public class TieRoundingTest {
         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 class TieRoundingTest {
 
     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 @@ public class TieRoundingTest {
         };
 
         // 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 @@ public class TieRoundingTest {
         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 @@ public class TieRoundingTest {
             "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 @@ public class TieRoundingTest {
             {"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 @@ public class TieRoundingTest {
         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 @@ public class TieRoundingTest {
             "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 @@ public class TieRoundingTest {
             {"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"
             }
         };