Preserve Float(double) rounding when replacing wrapper constructors - #970
Draft
martinfrancois wants to merge 3 commits into
Draft
Preserve Float(double) rounding when replacing wrapper constructors#970martinfrancois wants to merge 3 commits into
Float(double) rounding when replacing wrapper constructors#970martinfrancois wants to merge 3 commits into
Conversation
…nding
`new Float(<double literal>)` was retyped to a String and emitted as
`Float.valueOf("<decimal>")`. `Float(double)` is specified as
`(float) value`, so it rounds binary64 to binary32, while
`Float.valueOf(String)` rounds the decimal straight to binary32, and the
two can differ in the last bit: `(float) 1.0000000596046448` is `1.0f`
(bits 0x3f800000) while `Float.valueOf("1.0000000596046448")` is one ulp
higher (0x3f800001). Drop the String path so a literal argument goes
through `Float.valueOf((float) <literal>)` like every other primitive
double argument.
That cast template placed the argument directly in the cast operand. A
cast binds tighter than binary, ternary and assignment operators, so a
compound argument was only partly covered and the output either failed
to compile (`Float.valueOf((float) a + b)` passes a `double`) or rounded
one step too early (`(float) huge * 0` is `NaN` rather than `0.0f` for
`double huge = 1e39`). Parenthesize the argument for those expression
kinds; identifiers, field accesses and method invocations keep the form
they already had.
The existing `doubleToFloat` test expected `Float.valueOf("2.0")` for
`new Float(2.0d)` and now expects `Float.valueOf((float) 2.0d)`.
This was referenced Aug 11, 2026
martinfrancois
marked this pull request as draft
August 16, 2026 01:10
Float(double) rounding when replacing wrapper constructors
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Suggested review order: 29 of 52 (Score: 3.5)
Review first: openrewrite/rewrite#8444
What's changed?
When the argument of
new Float(...)has the primitive typedouble,PrimitiveWrapperClassConstructorToValueOfnow always emitsFloat.valueOf((float) X), whereXis the original argument with its source text unchanged, in parentheses of its own when it is aJ.Binary,J.Ternary,J.AssignmentorJ.AssignmentOperation, thedouble-typed expression shapes that bind less tightly than a cast. Adoubleliteral is no longer rewritten into aStringliteral: it now takes the same branch adoublevariable already takes, keeping its source text, its suffix and its radix.Before
In which
d1andd2are variables of the primitive typedouble.Actual after the recipe
Using main today.
Expected after the recipe
The corrected result is shown below.
An argument typed as the boxed class
java.lang.Doublestill becomesFloat.valueOf(boxed.floatValue()), exactly as on main. The implementation change is ten added and seven removed lines in one method; the other 123 added lines are tests.What's your motivation?
Recipe:
org.openrewrite.staticanalysis.PrimitiveWrapperClassConstructorToValueOf.For the compound arguments covered here whose static type remains
double, today's output does not compile:javac 21reportsno suitable method found for valueOf(double)ford1 + d2andd1 > d2 ? d1 : d2, andrequired: variablefor an assignment argument. Where the cast in front of the first operand alone leaves the whole argument with the static typefloatit does compile instead, and then computes a different value. Withdouble huge = 1e39,new Float(huge * 0)computes0.0f, becausehuge * 0is thedoublevalue0.0and the constructor narrows it. Main's output,Float.valueOf((float) huge * 0), computesNaN, because(float) hugeoverflows toInfinityandInfinity * 0isNaN; it compiles because afloatmultiplied by aninthas the static typefloat. This branch emitsFloat.valueOf((float) (huge * 0)), which computes0.0fagain.A
doubleliteral argument changes the computed value too.Float(double value)is specified asthis.value = (float) value, so the literal is rounded to adoubleand then narrowed to afloat, whereasFloat.valueOf(String)rounds the text straight to afloat. For1.0000000596046448,Float.floatToRawIntBitsof the constructor's result is0x3f800000and of today's output0x3f800001, one ulp higher. TheStringform is also rebuilt from the parsed value, so a hexadecimal literal, aDsuffix or digit separators all become plain decimal: on mainnew Float(0x1.0000002p0)becomesFloat.valueOf("1.0000000074505806"). Reproduced on 2.40.0 and on 2.41.0-SNAPSHOT built from main5785534a.Confirmed real-world execution
XMLPrinter.javaate6c32a6.org.openrewrite.recipe:rewrite-static-analysis:2.41.0.The released recipe places the float cast around only the left operand of a multiplication by the
doubleliteral100.0. Binary numeric promotion makes the complete argument adouble, so the generatedFloat.valueOf(...)call does not compile.Anything in particular you'd like reviewers to focus on?
doubleToFloatalready existed on main, and this change alters an expectation that used to hold there. Its inputFloat f = new Float(2.0d);is unchanged; the expected output wasFloat.valueOf("2.0")and is nowFloat.valueOf((float) 2.0d). Its other three assertions, forf2,f3andf4, are untouched.Three limits this change does not address, all the same on main and on this branch:
doubleargument is not resolved, so that argument gets no cast.valueOfmay return a cached instance, so==on the result can behave differently from==on a constructor result. The recipe description already spells out this caching.Have you considered any alternatives or workarounds?
Parenthesizing in every case would be simpler: one template instead of a conditional, about six implementation lines, deleting the branch that rewrites a
doubleliteral into aStringliteral and changing the template"Float.valueOf((float) #{any(double)})"to"Float.valueOf((float) (#{any(double)}))". I kept the form without parentheses for a simple argument because main already produces it for adoublevariable, it came in with #476 (closed), the issue that led to the(float)cast this change extends, anddoubleToFloatasserts it inFloat f4 = Float.valueOf((float) d2);, one of the three assertions this change leaves alone. That test is also the only existing one the alternative would touch:Float fandFloat f4would becomeFloat.valueOf((float) (2.0d))andFloat.valueOf((float) (d2)). Say so in review and I will switch.Any additional context
Pre-existing tests changed:
PrimitiveWrapperClassConstructorToValueOfTest.java.doubleToFloat(updated).This change adds 5 tests to
PrimitiveWrapperClassConstructorToValueOfTest. Without the code change in this pull request, these 4 tests fail:compoundDoubleExpressionToFloatIsParenthesizeddoubleLiteralToFloatKeepsBinary64RoundingdoubleLiteralToFloatKeepsSourceFormdoubleToFloat, with its updated expectationThese 2 new tests pass either way:
doubleExpressionToFloatUsesCastfloatLiteralUnchangedByDoubleHandlingThis change was prepared with AI assistance (Claude Code). I reviewed the code, the tests and this description.
Checklist
./gradlew buildlocally, and committed any resulting changes torecipes.csv