Skip to content

Fix #335 Improve overloaded varargs resolution #425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jhubbardnso wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Fix #335 Improve overloaded varargs resolution #425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
c2caa28
Fix #335 Improve overloaded varargs resolution
jhubbardnso May 12, 2026
27ed9ca
Implement PR feedback
jhubbardnso May 12, 2026
b0395fd
PR Feedback
jhubbardnso May 13, 2026
95b6efd
Improve varargs overload consistency tests
jhubbardnso May 18, 2026
64f060b
Add legacy switch for varargs overload resolution
jhubbardnso Jun 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
107 changes: 106 additions & 1 deletion Lib/test/test_joverload.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from java.lang import Float, Double, Integer, Long, Boolean, Exception
from java.util import ArrayList
from javatests import JOverload, Reflection
from org.python.core import PyReflectedFunction
from org.python.core import PyReflectedFunction, ReflectedArgs

class PyReflFuncEnvl:

Expand Down Expand Up @@ -124,6 +124,12 @@ def test_scal_float_one(self):

class VarargsDispatchTests(unittest.TestCase):

def setUp(self):
ReflectedArgs.setLegacyMode(False)

def tearDown(self):
ReflectedArgs.setLegacyMode(True)

def test_strings(self):
t = Reflection.StringVarargs()
self.assertEqual(t.test("abc", "xyz"),
Expand Down Expand Up @@ -196,8 +202,102 @@ def test_booleans(self):
self.assertEqual(t.testTwoFixedArg(True, False, True, False),
"boolean arg1:true boolean arg2:false booleans...:[true, false]");

def test_mixed_varargs_overload(self):
t = Reflection.MixedVarargs()
self.assertEqual(t.insert("name", "abc", "xyz"),
"String name:[abc, xyz]")
self.assertEqual(t.insert("name", ["abc", "xyz"]),
"String name:[abc, xyz]")
self.assertEqual(t.insert("flag", True, False),
"boolean flag:[true, false]")
self.assertEqual(t.insert("flag", [True, False]),
"boolean flag:[true, false]")
self.assertEqual(t.insert("int", 1, 2, 3),
"int int:[1, 2, 3]")
self.assertEqual(t.insert("int", [1, 2, 3]),
"int int:[1, 2, 3]")
self.assertEqual(t.insert("double", 1.0, 2.0, 3.14),
"double double:[1.0, 2.0, 3.14]")
self.assertEqual(t.insert("double", [1.0, 2.0, 3.14]),
"double double:[1.0, 2.0, 3.14]")

def test_varargs_overload_resolution(self):
t = Reflection.OverloadResolution()
# Object... is available but should not steal a call that has a better primitive varargs
# match.
self.assertEqual(t.varArgs(1, 2L, 3.0), "double")
# PyFloat to boolean is deliberately expensive, so the double overload still wins even
# when a leading PyBoolean makes the boolean overload look plausible. Object... should also
# lose to the primitive overload that best preserves the rest of the numeric arguments.
self.assertEqual(t.varArgs(True, 1, 2L, 3.0), "double")

self.assertEqual(t.varArgs(1, 2, 3), "long")
self.assertEqual(t.varArgs([1, 2, 3]), "long")

self.assertEqual(t.varArgs("name", 1, 2L, 3.0), "name: double")
self.assertEqual(t.varArgs("flag", True, 1, 2L, 3.0), "flag: double")

def test_pylong_single_and_varargs_consistent(self):
# This test only asserts consistency with non-varargs overload handling. The varargs
# ranking improvement is intended to match existing fixed-arity behavior, not to make a
# claim that the fixed-arity choice is itself the most correct resolution.
t = Reflection.OverloadResolution()
expected = t.singleArg(21L)
self.assertEqual(t.varArgs(21L, 22L, 23L), expected)

def test_pyinteger_single_and_varargs_consistent(self):
# This test only asserts consistency with non-varargs overload handling. The varargs
# ranking improvement is intended to match existing fixed-arity behavior, not to make a
# claim that the fixed-arity choice is itself the most correct resolution.
t = Reflection.OverloadResolution()
expected = t.singleArg(21)
self.assertEqual(t.varArgs(21, 22, 23), expected)

def test_pydouble_single_and_varargs_consistent(self):
# This test only asserts consistency with non-varargs overload handling. The varargs
# ranking improvement is intended to match existing fixed-arity behavior, not to make a
# claim that the fixed-arity choice is itself the most correct resolution.
t = Reflection.OverloadResolution()
expected = t.singleArg(21.0)
self.assertEqual(t.varArgs(21.0, 22.0, 23.0), expected)


class LegacyVarargsDispatchTests(unittest.TestCase):

def setUp(self):
ReflectedArgs.setLegacyMode(True)

def tearDown(self):
ReflectedArgs.setLegacyMode(True)

def test_legacy_single_and_varargs_can_disagree(self):
# Legacy varargs dispatch keeps the last matching varargs overload instead of ranking
# matches by conversion quality, so it can diverge from the fixed-arity overload choice.
t = Reflection.OverloadResolution()
self.assertEqual(t.singleArg(1), "long")
self.assertEqual(t.varArgs(1, 2, 3), "Object")
self.assertEqual(t.singleArg(1L), "long")
self.assertEqual(t.varArgs(1L, 2L, 3L), "Object")
self.assertEqual(t.singleArg(1.0), "double")
self.assertEqual(t.varArgs(1.0, 2.0, 3.0), "Object")

def test_legacy_mixed_numeric_varargs_can_pick_bad_overload(self):
# With legacy last-match behavior, mixed numeric inputs can resolve to Object or boolean
# even though the improved ranking chooses the overload that preserves the floating-point
# argument.
t = Reflection.OverloadResolution()
self.assertEqual(t.varArgs(1, 2L, 3.0), "Object")
self.assertEqual(t.varArgs("name", 1, 2L, 3.0), "name: boolean")


class ComplexOverloadingTests(unittest.TestCase):

def setUp(self):
ReflectedArgs.setLegacyMode(False)

def tearDown(self):
ReflectedArgs.setLegacyMode(True)

def test_constructor_overloading(self):
self.assertEqual(Reflection.Overloaded().constructorVersion, '')
self.assertEqual(Reflection.Overloaded(1).constructorVersion, 'int')
Expand All @@ -206,6 +306,8 @@ def test_constructor_overloading(self):
self.assertEqual(Reflection.Overloaded(1, 1, 1, 1).constructorVersion, 'int, int...')

self.assertEqual(Reflection.Overloaded(1, [2,3,4]).constructorVersion, 'int, int...')
self.assertEqual(Reflection.Overloaded(Boolean(True)).constructorVersion,
'boolean, boolean...')

b = Exception("Oops")
self.assertEqual(Reflection.Overloaded("aa").constructorVersion, "String")
Expand All @@ -222,6 +324,8 @@ def test_method_overloading(self):
# Note in Java these match both foo(int,int...) and foo(int...):
self.assertEqual(over.foo(1), "int, int...")
self.assertEqual(over.foo(1, 2, 3, 4), "int, int...")
self.assertEqual(over.foo(True), "boolean, boolean...")
self.assertEqual(over.foo(Boolean(True)), "boolean, boolean...")

def test_method_most_specific(self):
over = Reflection.Overloaded()
Expand Down Expand Up @@ -254,6 +358,7 @@ def test_main():
test_support.run_unittest(
OverloadedDispatchTests,
VarargsDispatchTests,
LegacyVarargsDispatchTests,
ComplexOverloadingTests,
)

Expand Down
14 changes: 10 additions & 4 deletions src/org/python/core/PyReflectedConstructor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ public PyObject __call__(PyObject self, PyObject[] args, String[] keywords) {
args = new PyObject[allArgs.length - nkeywords];
System.arraycopy(allArgs, 0, args, 0, args.length);
Object varargMatch = null;
ReflectedArgs varargArgs = null;
ReflectedCallData varargData = null;

// Look for a constructor with no keyword args
Expand All @@ -162,11 +163,13 @@ public PyObject __call__(PyObject self, PyObject[] args, String[] keywords) {
if (!argslist[i].isVarArgs) {
method = argslist[i].method;
break;
} else {
} else if (varargMatch == null
|| argslist[i].betterVarargsMatchThan(varargArgs, null, args)) {
varargMatch = argslist[i].method;
varargArgs = argslist[i];
varargData = callData;
callData = new ReflectedCallData();
}
callData = new ReflectedCallData();
}
}
if (method == null && varargMatch != null) {
Expand All @@ -177,6 +180,7 @@ public PyObject __call__(PyObject self, PyObject[] args, String[] keywords) {
} else {
// Just look for a constructor with no keyword args
Object varargMatch = null;
ReflectedArgs varargArgs = null;
ReflectedCallData varargData = null;
int n = nargs;
for (int i = 0; i < n; i++) {
Expand All @@ -185,11 +189,13 @@ public PyObject __call__(PyObject self, PyObject[] args, String[] keywords) {
if (!argslist[i].isVarArgs) {
method = argslist[i].method;
break;
} else {
} else if (varargMatch == null
|| argslist[i].betterVarargsMatchThan(varargArgs, null, args)) {
varargMatch = argslist[i].method;
varargArgs = argslist[i];
varargData = callData;
callData = new ReflectedCallData();
}
callData = new ReflectedCallData();
}
}
if (method == null && varargMatch != null) {
Expand Down
5 changes: 4 additions & 1 deletion src/org/python/core/PyReflectedFunction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,13 @@ public PyObject __call__(PyObject self, PyObject[] args, String[] keywords) {
if (argslist[i].matches(self, args, keywords, callData)) {
if (!argslist[i].isVarArgs) {
match = argslist[i];
} else {
} else if (varargMatch == null
|| argslist[i].betterVarargsMatchThan(varargMatch, self, args)) {
varargMatch = argslist[i];
varargData = callData;
callData = new ReflectedCallData();
} else {
callData = new ReflectedCallData();
}
}
}
Expand Down
Loading