Hi all,

While moving a project from Groovy 3.0.25 to 5.1.3 we hit a dispatch change
that I have bisected to 4.0.4, and I would like to know whether it was
intended before I file a ticket.

Given two competing varargs overloads:

    public class Overloads {
        public static String m(float... x)  { return "float...";  }
        public static String m(double... x) { return "double..."; }
    }

and a script where the decimal literal is untyped, so a BigDecimal:

    import static Overloads.*
    z = 1
    d = 5.0
    println m(d)
    println m(z, d)
    println m(5, z, d)
    println m(5, 2, 3, z, d)

Groovy 4.0.3 and all of 3.0.x print "double..." four times. Groovy 4.0.4
through 6.0.0 print:

    m(d)             -> double...
    m(z, d)          -> GroovyRuntimeException: Ambiguous method overloading
    m(5, z, d)       -> float...
    m(5, 2, 3, z, d) -> float...

So a BigDecimal is now narrowed to float (about 7 significant digits rather
than 16) when accompanied by two or more integral arguments, and the
two-argument form fails outright where it previously worked.

The cause appears to be commit ef7026317f, "GROOVY-8045: support coercion
for
variadic parameter", which changed the predicate gating varargs
applicability
in ParameterTypes.isValidVargsMethod:

    // 4.0.3
    if (MetaClassHelper.isAssignableFrom(clazz, arguments[i])) continue;
    // 4.0.4
    if (!componentType.isAssignableFrom(argumentTypes[i])) { return false; }

MetaClassHelper.isAssignableFrom and CachedClass.isAssignableFrom disagree
about BigDecimal -> float. With a class exposing only a float... overload,
so
no competing candidate and no distance comparison involved, 4.0.3 raises
MissingMethodException for m(BigDecimal) while 4.0.4 accepts it.

So float... previously was not a candidate at all and double... won by
default. Now both are applicable and selection falls through to the distance
algorithm, which is itself unchanged -- PRIMITIVE_DISTANCE_TABLE, the
varargs
accumulation in calculateParameterDistance, and MetaClassImpl.handleMatches
are byte-identical between 3.0.25 and 5.1.3. Summing its per-argument costs,
each integral argument biases toward float by 2 while the lone BigDecimal
biases toward double by only 2, which produces exactly the arity-dependent
results above.

Since GROOVY-8045 is about implicit closure coercion, I suspect the numeric
side effect was collateral: it shipped in a patch release with no
breaking-change note, and it was not backported to 3.0.x, which still
selects
double... as of 3.0.25. But I may well be missing context, hence this mail
rather than a Jira issue.

Two questions:

1. Was widening varargs applicability to include BigDecimal -> float
   intended?
2. If so, should a lossy narrowing be able to win on summed distance against
   the lossless widenings of its accompanying arguments?

Reproduced on Temurin 17, 21, 25 and 27 on Linux x64, plain dynamic Groovy
--
no @CompileStatic or @TypeChecked, no metaprogramming, no third-party
libraries. I could not find an existing issue; the closest are GROOVY-5490
(open, same distance algorithm but inheritance distance among reference
types) and GROOVY-1937 (fixed in 1.1-beta-2, establishing BigDecimal ->
double for varargs).

Happy to file this in Jira with the full analysis, or to submit a PR with a
failing test, if that is useful.

Encountered in Deephaven Community Core (Apache-2.0), whose generated
io.deephaven.function.Numeric exposes max/min over the full primitive
varargs
ladder, so max(5, 2, 3, 1, 5.0) changed from Double to Float and max(1, 5.0)
began throwing. A typed literal (5.0d) still selects double... on every
version from 3.0.25 through 6.0.0.

Thanks,
Stan

Reply via email to