Stanley Brubaker created GROOVY-12439:
-----------------------------------------
Summary: Regression: BigDecimal silently narrowed to float in
overload selection since 4.0.4
Key: GROOVY-12439
URL: https://issues.apache.org/jira/browse/GROOVY-12439
Project: Groovy
Issue Type: Bug
Components: groovy-runtime
Affects Versions: 6.0.0, 5.1.3, 4.0.4
Environment: Temurin JDK 17, 21, 25 and 27 on Linux x64 — behaviour is
identical on all four, so this is not JDK-specific. Plain dynamic Groovy: no
@CompileStatic or @TypeChecked, no metaprogramming, no third-party libraries.
Reporter: Stanley Brubaker
A call that selected a {{double}} overload up to 4.0.3 now silently selects a
narrowing {{float}} one, losing about nine significant digits -- and the same
call in varargs form throws {{GroovyRuntimeException}} instead. Code that
worked on 4.0.3 either loses precision without warning or fails at runtime,
depending only on the shape of the call.
The cause is that a {{BigDecimal}} argument became applicable to a {{float}}
parameter in 4.0.4. Where both {{float}} and {{double}} overloads exist, the
same conceptual call now gives three different answers: {{double}} for a lone
{{BigDecimal}}, an ambiguity error for varargs, and a silently narrowing
{{float}} for two scalars.
h2. Reproducer
{code:java}
public class Sel {
public static String m(float... x) { return "float..."; }
public static String m(double... x) { return "double..."; }
public static String m2(float a, float b) { return "float(2)"; }
public static String m2(double a, double b) { return "double(2)"; }
}
{code}
{code:groovy}
import static Sel.*
z = 1
d = 5.0 // untyped decimal literal, therefore BigDecimal
println m(d)
println m(z, d)
println m2(z, d)
{code}
{code}
javac --release 17 Sel.java // --release 17 so pre-4.0.33 ASM can read the
class
java -cp groovy-<version>.jar:. groovy.ui.GroovyMain repro.groovy
{code}
||call||3.0.25||4.0.3||4.0.4||5.0.0 - 6.0.0||
|{{m(d)}}|double...|double...|double...|double...|
|{{m(z, d)}}|double...|double...|*AMBIGUOUS*|*AMBIGUOUS*|
|{{m2(z, d)}}|AMBIGUOUS|AMBIGUOUS|AMBIGUOUS|*float(2)*|
Expected: {{double}} in all three, as in 4.0.3.
h2. Cause
Two independent changes, each bisected to a single release.
*4.0.4 -- BigDecimal became applicable to float.*
{{ParameterTypes.isValidVargsMethod}} swapped its applicability predicate in
ef7026317f ("GROOVY-8045: support coercion for variadic parameter"):
{code:java}
// 4.0.3
if (MetaClassHelper.isAssignableFrom(clazz, arguments[i])) continue;
// 4.0.4
if (!componentType.isAssignableFrom(argumentTypes[i])) { return false; }
{code}
{{MetaClassHelper.isAssignableFrom}} and {{CachedClass.isAssignableFrom}}
disagree about {{BigDecimal}} -> {{float}}. With a class exposing *only*
{{float...}}, so no competing candidate and no distance comparison involved,
4.0.3 raises {{MissingMethodException}} for {{f(BigDecimal)}} while 4.0.4
accepts it. So {{float...}} was previously not a candidate at all and
{{double...}} won by being the only applicable overload.
Once both are applicable, {{PRIMITIVE_DISTANCE_TABLE}} decides.
{{getPrimitiveDistance(from, to)}} returns {{TABLE\[toIndex\]\[fromIndex\]}}
and is called as {{getPrimitiveDistance(parameterClass, argument)}}, so the
lookup is {{TABLE\[argument\]\[parameter\]}}:
||argument||-> float||-> double||
|{{int}} / {{Integer}}|5|7|
|{{BigDecimal}}|5|3|
For {{(int, BigDecimal)}} the totals are 5+5=10 for {{float}} and 7+3=10 for
{{double}}: the integral argument's two-point lean toward {{float}} exactly
cancels the {{BigDecimal}}'s two-point lean toward {{double}}, producing the
tie reported as ambiguous.
*5.0.0 -- the tie started resolving to float.* The GROOVY-11258 redundant-match
pruning added to {{MetaClassImpl.doChooseMostSpecificParams}} (absent in
4.0.33, present in 5.0.0) discards one of the tied matches, so the non-varargs
form silently selects the narrowing {{float(2)}} instead of erroring.
h2. Scope
Deliberately not reported here: an integral argument also prefers {{float}}
over {{double}}, but that is long-standing (identical in 3.0.25) and javac
selects {{float}} too, since JLS 4.10.1 makes {{float}} more specific than
{{double}}. Only the {{BigDecimal}} behaviour is new, and Java has no
equivalent because the conversion does not exist implicitly there.
h2. Suggested direction
Either restore the pre-4.0.4 applicability so {{BigDecimal}} is not implicitly
narrowed to {{float}} while a {{double}} overload exists -- scoping
GROOVY-8045's coercion change to non-numeric component types -- or weight lossy
conversions so a narrowing cannot tie with, or be selected over, a widening.
h2. Impact
Found in Deephaven Community Core (Apache-2.0), whose generated
{{io.deephaven.function.Numeric}} offers {{max}}/{{min}} over the full
primitive ladder. In a Groovy console {{max(5, 2, 3, 1, 5.0)}} changed from
{{Double}} to {{Float}} and {{max(1, 5.0)}} began throwing. Libraries that do
not expose both {{float}} and {{double}} overloads are unaffected.
For us this specifically blocks JDK 27 support. We run fine on 3.0.25 under JDK
17, 21 and 25, but the two requirements for 27 are mutually exclusive: reading
JDK 27 class files needs the ASM 9.10.1 bump present only in 4.0.33+ / 5.0.7+,
while preserving the previous selection needs 4.0.3 or earlier. No Groovy
release does both, and the shaded {{groovyjarjarasm}} cannot be upgraded
independently. So we stay on 3.0.25 and cannot add JDK 27 to our test matrix
until this is resolved or we accept the precision change for our users.
Workaround: a typed literal ({{5.0d}}) or a declared {{double}}, which selects
{{double}} on every version from 3.0.25 through 6.0.0.
Discussed on [email protected]:
https://lists.apache.org/thread/79xony935rzccs93kmznzrvd4ros50x1
h2. Related
* GROOVY-1937 (fixed 1.1-beta-2) "Number conversion from BigDecimal to double
does not work for Varargs" -- established {{BigDecimal}} -> {{double}} for
varargs.
* GROOVY-5490 (open, 2012) "Double Dispatch Distance Algorithm Incorrectly
Implemented" -- the same minimum-distance algorithm, for inheritance distance
among reference types.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
