From b121bb38bb03fc20c109c9a221d6e9755be93797 Mon Sep 17 00:00:00 2001 From: Karl Nelson Date: Fri, 16 Oct 2020 09:51:13 -0700 Subject: [PATCH 1/2] Fixed variadic method resolution --- doc/CHANGELOG.rst | 3 + .../org/jpype/manager/MethodResolution.java | 94 ++++++++++++++++++- test/harness/jpype/varargs/VarArgs.java | 33 +++++++ test/jpypetest/test_varargs.py | 13 +++ 4 files changed, 138 insertions(+), 5 deletions(-) diff --git a/doc/CHANGELOG.rst b/doc/CHANGELOG.rst index 888710ed2..34830881a 100644 --- a/doc/CHANGELOG.rst +++ b/doc/CHANGELOG.rst @@ -5,6 +5,9 @@ This changelog *only* contains changes from the *first* pypi release (0.5.4.3) o Latest Changes: - **1.0.3_dev0 - unreleased** + + - Correct bug resulting in reporting ambiguous overloads when resolving + methods with variadic arguments. - Ctrl+C behavior is switchable with interrupt flag to startJVM. If True, process will halt on Ctrl-C. If False, the process diff --git a/native/java/org/jpype/manager/MethodResolution.java b/native/java/org/jpype/manager/MethodResolution.java index 33437ea89..b5dd0e0f0 100644 --- a/native/java/org/jpype/manager/MethodResolution.java +++ b/native/java/org/jpype/manager/MethodResolution.java @@ -119,7 +119,7 @@ static Class[] of(Class... l) } static HashMap CONVERSION = new HashMap<>(); - + { CONVERSION.put(Byte.TYPE, of(Byte.TYPE, Byte.class, Short.TYPE, Short.class, @@ -172,15 +172,99 @@ public static boolean isMoreSpecificThan(Executable method1, Executable method2) List> param1 = new ArrayList<>(Arrays.asList(method1.getParameterTypes())); List> param2 = new ArrayList<>(Arrays.asList(method2.getParameterTypes())); - // This line prevents ambiguity resolution both static and method forms. -// if (Modifier.isStatic(method1.getModifiers()) -// != Modifier.isStatic(method2.getModifiers())) -// return false; if (!Modifier.isStatic(method1.getModifiers())) param1.add(0, method1.getDeclaringClass()); if (!Modifier.isStatic(method2.getModifiers())) param2.add(0, method2.getDeclaringClass()); + // Special handling is needed for varargs as it may chop or expand. + // we have 4 cases for a varargs methods + // foo(Arg0, Arg1...) as + // foo(Arg0) + // foo(Arg0, Arg1) + // foo(Arg0, Arg1[]) + // foo(Arg0, Arg1, Arg1+) + if (method1.isVarArgs() && method2.isVarArgs()) + { + // Punt on this as there are too many different cases + return isMoreSpecificThan(param1, param2); + } + + if (method1.isVarArgs()) + { + int n1 = param1.size(); + int n2 = param2.size(); + + // Last element is an array + Class cls = param1.get(n1 - 1); + Class cls2 = cls.getComponentType(); + + // Less arguments, chop the list + if (n1 - 1 == n2) + return isMoreSpecificThan(param1.subList(0, n2), param2); + + // Same arguments + if (n1 == n2) + { + List> q = new ArrayList<>(param1); + q.set(n1 - 1, cls2); + + // Check both ways + return isMoreSpecificThan(param1, param2) || isMoreSpecificThan(q, param2); + } + + // More arguments + if (n1 < n2) + { + // Grow the list + List> q = new ArrayList<>(param1); + q.set(n1 - 1, cls2); + for (int i = n1; i < n2; ++i) + q.add(cls2); + return isMoreSpecificThan(q, param2); + } + } + + if (method2.isVarArgs()) + { + int n1 = param1.size(); + int n2 = param2.size(); + + // Last element is an array + Class cls = param2.get(n2 - 1); + Class cls2 = cls.getComponentType(); + + // Less arguments, chop the list + if (n2 - 1 == n1) + return isMoreSpecificThan(param1, param2.subList(0, n2)); + + // Same arguments + if (n1 == n2) + { + List> q = new ArrayList<>(param2); + q.set(n2 - 1, cls2); + + // Compare both ways + return isMoreSpecificThan(param1, param2) || isMoreSpecificThan(param1, q); + } + + // More arguments + if (n2 < n1) + { + // Grow the list + List> q = new ArrayList<>(param2); + q.set(n2 - 1, cls2); + for (int i = n2; i < n1; ++i) + q.add(cls2); + return isMoreSpecificThan(param1, q); + } + } + + return isMoreSpecificThan(param1, param2); + } + + public static boolean isMoreSpecificThan(List> param1, List> param2) + { // FIXME need to consider resolving mixing of static and non-static // Methods here. if (param1.size() != param2.size()) diff --git a/test/harness/jpype/varargs/VarArgs.java b/test/harness/jpype/varargs/VarArgs.java index e013bc739..a04e123cd 100644 --- a/test/harness/jpype/varargs/VarArgs.java +++ b/test/harness/jpype/varargs/VarArgs.java @@ -15,6 +15,8 @@ **************************************************************************** */ package jpype.varargs; +import java.util.Map; + class VarArgs { @@ -63,4 +65,35 @@ public String[] method(String s, String... rest) { return rest; } + + public int conflict1(Object... j) + { + return 1; + } + + public int conflict1(Map j) + { + return 2; + } + + public int conflict2(Object... j) + { + return 1; + } + + public int conflict2(Map j, Map k) + { + return 2; + } + + + public int conflict3(char... j) + { + return 1; + } + + public int conflict3(String j) + { + return 2; + } } diff --git a/test/jpypetest/test_varargs.py b/test/jpypetest/test_varargs.py index fc62777ad..29784afcf 100644 --- a/test/jpypetest/test_varargs.py +++ b/test/jpypetest/test_varargs.py @@ -39,6 +39,19 @@ def setUp(self): self.String = jpype.JClass('java.lang.String') self.StringA = jpype.JArray(self.String) + def testConflict(self): + m = jpype.java.util.LinkedHashMap({"a":1,"b":2,"c":3}) + test = self.VarArgs() + self.assertEqual(test.conflict2([1,2,3]),1) + self.assertEqual(test.conflict2(m,m),2) + self.assertEqual(test.conflict3(['h','e']),1) + self.assertEqual(test.conflict3('h'),2) + self.assertEqual(test.conflict3('hello'),2) + self.assertEqual(test.conflict1(1),1) + self.assertEqual(test.conflict1([1,2,3]),1) + self.assertEqual(test.conflict1({"a":1,"b":2,"c":3}),2) + self.assertEqual(test.conflict1(m),2) + def testVarArgsCtor(self): va0 = self.VarArgs('1') va1 = self.VarArgs('1', 'a') From 936381762f6cf7bde7f3ff8daa2fcd8746ab2e70 Mon Sep 17 00:00:00 2001 From: Karl Nelson Date: Fri, 16 Oct 2020 10:02:19 -0700 Subject: [PATCH 2/2] Reinstall autopep8 prehook --- test/jpypetest/test_varargs.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/test/jpypetest/test_varargs.py b/test/jpypetest/test_varargs.py index 29784afcf..b1b55c90a 100644 --- a/test/jpypetest/test_varargs.py +++ b/test/jpypetest/test_varargs.py @@ -40,17 +40,17 @@ def setUp(self): self.StringA = jpype.JArray(self.String) def testConflict(self): - m = jpype.java.util.LinkedHashMap({"a":1,"b":2,"c":3}) + m = jpype.java.util.LinkedHashMap({"a": 1, "b": 2, "c": 3}) test = self.VarArgs() - self.assertEqual(test.conflict2([1,2,3]),1) - self.assertEqual(test.conflict2(m,m),2) - self.assertEqual(test.conflict3(['h','e']),1) - self.assertEqual(test.conflict3('h'),2) - self.assertEqual(test.conflict3('hello'),2) - self.assertEqual(test.conflict1(1),1) - self.assertEqual(test.conflict1([1,2,3]),1) - self.assertEqual(test.conflict1({"a":1,"b":2,"c":3}),2) - self.assertEqual(test.conflict1(m),2) + self.assertEqual(test.conflict2([1, 2, 3]), 1) + self.assertEqual(test.conflict2(m, m), 2) + self.assertEqual(test.conflict3(['h', 'e']), 1) + self.assertEqual(test.conflict3('h'), 2) + self.assertEqual(test.conflict3('hello'), 2) + self.assertEqual(test.conflict1(1), 1) + self.assertEqual(test.conflict1([1, 2, 3]), 1) + self.assertEqual(test.conflict1({"a": 1, "b": 2, "c": 3}), 2) + self.assertEqual(test.conflict1(m), 2) def testVarArgsCtor(self): va0 = self.VarArgs('1')