From 4811c928f37a2a910b0c0279e108488c9710bf30 Mon Sep 17 00:00:00 2001 From: Qingyuan Wang <162230229@nuaa.edu.cn> Date: Sun, 25 Oct 2026 12:56:04 +0000 Subject: [PATCH] fix(java): round UInt64 values to the nearest float and double UInt64 widened its unsigned magnitude with `(double) (value >>> 1) * 2 + (value & 1L)`. Halving first puts the odd bit below the double grid wherever the spacing exceeds 2, so a residue just above a midpoint can only round down: 2^54 + 3 gave 1.8014398509481984E16 and 2^62 + 513 gave 4.6116860184273879E18, each one double too low. The class is one residue per two grid points in every binade from 2^53 up - 2,046 misses in the first 8,192 values of each binade, 6,157 in 5M random values - and it lands on the very accessor the class javadoc recommends "when the full unsigned magnitude must be exposed beyond the signed long range". floatValue() then narrowed that double again, which is wrong on its own account: a value just above a float midpoint can collapse onto that midpoint in the double step, where ties go to even mantissa instead of to the side the integer actually sits on. 2^53 + 2^29 + 1 came back as 9.0071993E15 rather than 9.0072003E15, and in [2^63, 2^64) the band around every float midpoint is 1,024 values wide, roughly 2^33 of the space. Both helpers now round once, straight to the target format. Non-negative longs already narrow correctly under JLS 5.1.3, and the top binade is a plain grid - 2^11 for double, 2^40 for float - whose tie is decided by the parity of the quotient. Evidence: UInt64 from the built module swept against Double/Float.parseFloat of each value's own unsigned decimal string, which the JLS requires to be correctly rounded. 524,288 consecutive values taken from every binade start: 2,046 wrong doubleValues before, 0 after. 2,389,824 values across the double and float midpoint bands: 352 wrong doubleValues and 1,050,624 wrong floatValues before, 0 after. 5,000,000 random unsigned values: 6,157 wrong doubleValues before, 0 after. The three new UnsignedTest cases keep two of those sweeps in-repo, walking the midpoint classes of every binade from 2^53 and comparing against the same decimal oracle: 640 and 108 failures before the fix, 0 after. No existing test called either accessor, and none of the parse/serialize round-trips go through them, so nothing in the suite could have seen this. Verified on JDK 17: UnsignedTest 95/95 (3 red before the fix); full fory-core suite `Tests run: 2349, Failures: 1`, the lone failure being the pre-existing ExpressionTest.testLiteralSourceRoundTrip, which also fails on main without this change; spotless:check and checkstyle:check clean, spotless run under JDK 21 as .agents/languages/java.md requires. No other Java module calls either accessor. Signed-off-by: Qingyuan Wang <162230229@nuaa.edu.cn> --- .../org/apache/fory/type/unsigned/UInt64.java | 37 ++++++++- .../fory/type/unsigned/UnsignedTest.java | 83 +++++++++++++++++++ 2 files changed, 117 insertions(+), 3 deletions(-) diff --git a/java/fory-core/src/main/java/org/apache/fory/type/unsigned/UInt64.java b/java/fory-core/src/main/java/org/apache/fory/type/unsigned/UInt64.java index d5c7267c9f..676d38a152 100644 --- a/java/fory-core/src/main/java/org/apache/fory/type/unsigned/UInt64.java +++ b/java/fory-core/src/main/java/org/apache/fory/type/unsigned/UInt64.java @@ -194,7 +194,7 @@ public long longValue() { } public float floatValue() { - return (float) toUnsignedDouble(data); + return toUnsignedFloat(data); } public double doubleValue() { @@ -225,8 +225,39 @@ public int hashCode() { return (int) (data ^ (data >>> 32)); } + /** + * Rounds the unsigned magnitude to the nearest double. The value is never shifted by one and + * scaled back: that halves the grid, so a low residue can only round down, never break the tie. + */ private static double toUnsignedDouble(long value) { - double high = (double) (value >>> 1); - return high * 2.0 + (value & 1L); + if (value >= 0) { + return (double) value; + } + // Unsigned values here are in [2^63, 2^64), where the double grid is a multiple of 2^11. + long grid = value >>> 11; + long rest = value & 2047L; + if (rest > 1024L || (rest == 1024L && (grid & 1L) != 0L)) { + grid++; + } + return (double) grid * 2048.0; + } + + /** + * Rounds the unsigned magnitude straight to the nearest float. Narrowing through {@link + * #doubleValue()} would round twice: a value just above a float midpoint can collapse onto that + * midpoint in the double step, where the tie then breaks the wrong way. + */ + private static float toUnsignedFloat(long value) { + if (value >= 0) { + return (float) value; + } + // Unsigned values here are in [2^63, 2^64), where the float grid is a multiple of 2^40. + long grid = value >>> 40; + long rest = value & ((1L << 40) - 1); + long half = 1L << 39; + if (rest > half || (rest == half && (grid & 1L) != 0L)) { + grid++; + } + return (float) grid * (1L << 40); } } diff --git a/java/fory-core/src/test/java/org/apache/fory/type/unsigned/UnsignedTest.java b/java/fory-core/src/test/java/org/apache/fory/type/unsigned/UnsignedTest.java index 78665d39e3..df47c832a8 100644 --- a/java/fory-core/src/test/java/org/apache/fory/type/unsigned/UnsignedTest.java +++ b/java/fory-core/src/test/java/org/apache/fory/type/unsigned/UnsignedTest.java @@ -162,6 +162,89 @@ public void uint64ParsingAndArithmetic() { assertEquals(UInt64.valueOf(0x0000000000000001L).shiftLeft(63).toLong(), Long.MIN_VALUE); } + @Test + public void uint64WideningRoundsToTheNearestFloatAndDouble() { + // 2^54 + 3 is one above the midpoint between the doubles 2^54 and 2^54 + 4, and 2^62 + 513 one + // above the midpoint of its own binade; both must round up. + assertEquals(UInt64.valueOf(0x40000000000003L).doubleValue(), 1.8014398509481988E16); + assertEquals(UInt64.valueOf(0x4000000000000201L).doubleValue(), 4.6116860184273889E18); + assertEquals(UInt64.valueOf(-1L).doubleValue(), 1.8446744073709552E19); + assertEquals(UInt64.valueOf(Long.MIN_VALUE).doubleValue(), 9.223372036854776E18); + // 2^53 + 2^29 + 1 is above the float midpoint 2^53 + 2^29, so it must round up to 2^53 + 2^30. + assertEquals(UInt64.valueOf(0x0020000020000001L).floatValue(), 9.0072003E15f); + } + + @Test + public void uint64DoubleValueMatchesTheCorrectlyRoundedUnsignedDecimal() { + // Double.parseDouble is correctly rounded, so a value's own unsigned decimal string is an + // independent oracle. The midpoint between two adjacent doubles, and the values on either side + // of it, are the only inputs where a widening conversion can disagree. + int wrong = 0; + String first = ""; + for (int e = 53; e <= 63; e++) { + long base = (e == 63) ? Long.MIN_VALUE : 1L << e; + long ulp = 1L << (e - 52); + for (int k = 0; k < 64; k++) { + long midpoint = base + (long) k * 2 * ulp + (ulp >> 1); + for (long d = -1; d <= 1; d++) { + UInt64 value = UInt64.valueOf(midpoint + d); + double expected = Double.parseDouble(value.toUnsignedString(10)); + if (value.doubleValue() != expected) { + wrong++; + if (first.length() == 0) { + first = + value.toUnsignedString(10) + " -> " + value.doubleValue() + ", want " + expected; + } + } + } + } + } + assertEquals( + wrong, + 0, + "uint64 doubleValue is not the nearest double " + wrong + " times; first " + first); + } + + @Test + public void uint64FloatValueIsRoundedOnceNotTwice() { + // Rounding the unsigned magnitude to double first, then to float, is wrong for every value that + // the double step lands on a float midpoint: the tie then breaks by mantissa parity instead of + // by which side of the midpoint the integer is actually on. + int wrong = 0; + String first = ""; + for (int e = 53; e <= 63; e++) { + long base = (e == 63) ? Long.MIN_VALUE : 1L << e; + long floatUlp = 1L << (e - 23); + long doubleUlp = 1L << (e - 52); + long[] offsets = { + -doubleUlp, + -(doubleUlp >> 1) - 1, + -(doubleUlp >> 1), + -1, + 1, + doubleUlp >> 1, + (doubleUlp >> 1) + 1, + doubleUlp + }; + for (int j = 0; j < 4; j++) { + long midpoint = base + (long) j * floatUlp + (floatUlp >> 1); + for (long d : offsets) { + UInt64 value = UInt64.valueOf(midpoint + d); + float expected = Float.parseFloat(value.toUnsignedString(10)); + if (value.floatValue() != expected) { + wrong++; + if (first.length() == 0) { + first = + value.toUnsignedString(10) + " -> " + value.floatValue() + ", want " + expected; + } + } + } + } + } + assertEquals( + wrong, 0, "uint64 floatValue is not the nearest float " + wrong + " times; first " + first); + } + // POJO1: Using UInt8/16/32/64 wrapper types as fields @Data @NoArgsConstructor