Skip to content

Commit

Permalink
Handle method binding better for arrays
Browse files Browse the repository at this point in the history 
If we have two methods which take arrays then we want to be smarter
about the binding semantics since the Python side could potentially bind
to both of them.
  • Loading branch information
@iamsrp-deshaw
iamsrp-deshaw committed Aug 29, 2023
1 parent d8ce0fe commit cf4e15f
Show file tree
Hide file tree
Showing 2 changed files with 147 additions and 0 deletions.
82 changes: 82 additions & 0 deletions java/src/main/java/com/deshaw/pjrmi/MethodUtil.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,16 @@ private static int compareBySpecificity(final CallableWrapper w1,
// f(b, d); <-- unique and unambiguous
// So, in all cases, no specificity ordering exists.
//
// An extra quirk here is that the Python side of PJRmi will allow
// assignment-by-argument passing to array types. I.e. a list of Python
// floats can be rendered as a Java double[] or an Object[]. That means
// that overloaded methods which take such array types can both bind to
// the given Python argument. We dictate that similar binding semantics
// exist for the arrays, based on their element types. I.e. in the above
// example the double[] version of the method is more specific than the
// Object[] one, since you can assign doubles to Objects, but not
// Objects to doubles.
//
// Finally, note that it's possible for the arguments to _exactly_ match
// if one class overrides a method in another class but has a more
// specific return type. As such, we need to know if that happens too
Expand All @@ -318,6 +328,8 @@ private static int compareBySpecificity(final CallableWrapper w1,
// Same class is not more or less specific
}
else if (isAssignableFrom(c1, c2)) {
// c2 is more specific than c1 since we can assign a c2 value to
// a c1 one. (E.g. c1 is a long and c2 is a short.)
if (cmp < 0) {
// We have one argument which is more specific and one
// argument which is less specific; therefore neither method
Expand All @@ -333,6 +345,7 @@ else if (isAssignableFrom(c1, c2)) {
}
}
else if (isAssignableFrom(c2, c1)) {
// c1 is more specific
if (cmp > 0) {
// The same one-more/one-less case, like above.
return 0;
Expand All @@ -343,6 +356,31 @@ else if (isAssignableFrom(c2, c1)) {
match = false;
}
}
else if (c1.isArray() && c2.isArray() &&
isArrayMoreSpecific(c2, c1)) // <-- note order
{
// c2 is an array which is more specific than c1. See the
// similar case above for comments.
if (cmp < 0) {
return 0;
}
else {
cmp = 1;
match = false;
}
}
else if (c1.isArray() && c2.isArray() &&
isArrayMoreSpecific(c1, c2)) // <-- note order
{
// c1 is an array which is more specific than c2
if (cmp > 0) {
return 0;
}
else {
cmp = -1;
match = false;
}
}
else {
// The two types have no relation and so they are uncomparable.
// This renders the entire method uncomparable also. Per Java
Expand Down Expand Up @@ -599,4 +637,48 @@ else if (!c1.isPrimitive() && c2.isPrimitive()) {
throw new IllegalStateException("Unreachable statement");
}
}

/**
* Whether the array represented by class {@code c1} is more specific than
* the array represented by {@code c2}.
*
* <p>This code assumes that the arguments are non-{@code null} and that
* they represent arrays; no checking will be done.
*/
private static boolean isArrayMoreSpecific(final Class<?> c1, final Class<?> c2)
{
// Determine the element types
final Class<?> t1 = c1.getComponentType();
final Class<?> t2 = c2.getComponentType();

// If these are multi-dimensional array types then the elements will
// themselves be arrays
if (t1.isArray() && t2.isArray()) {
// We just recurse down, this will all depend on the ultimate
// element types
return isArrayMoreSpecific(t1, t2);
}
else if (t1.isPrimitive() && !t2.isPrimitive() && !t2.isArray() &&
!isAssignableFrom(t1, t2))
{
// If c1 is primitive and c2 is any form of non-array Object which
// we can't assign to it (i.e. not its boxing type) then it must be
// more specific. We have the !t2.isArray() check above because, if
// t2 is an array, then t1 isn't more specific than it (and it's not
// less specific either), so we want to conform to the semantics of
// this function.
return true;
}
else {
// At this point c1 will be more specific if we can assign its
// elements into c2, but not the other way around. Note the reversed
// ordering: if c1 is an array of ints and c2 is an array of longs
// then c1 is more specific since we can assign all the elements of
// c1 to c2 but not the other way around; c2 is more general. This
// can only be true if we can't assign in both directions (i.e. one
// is not the boxed type of the other).
return isAssignableFrom(t2, t1) &&
!isAssignableFrom(t1, t2);
}
}
}
65 changes: 65 additions & 0 deletions java/src/test/java/com/deshaw/pjrmi/MethodUtilTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,15 @@ public static class Methods
public static final Method v_h_i;
public static final Method v_h_I;

public static final Method v_j_L_d;
public static final Method v_j_L_D;
public static final Method v_j_L_O;
public static final Method v_j_L_A;
public static final Method v_j_LL_d;
public static final Method v_j_LL_O;
public static final Method v_j_L_d_i;
public static final Method v_j_L_O_i;

static {
try {
o_f_aa = Methods.class.getMethod("f", A.class, A.class);
Expand Down Expand Up @@ -116,6 +125,17 @@ public static class Methods

v_h_i = Methods.class.getMethod("h", int .class);
v_h_I = Methods.class.getMethod("h", Integer.class);

v_j_L_d = Methods.class.getMethod("j", double[] .class);
v_j_L_D = Methods.class.getMethod("j", Double[] .class);
v_j_L_O = Methods.class.getMethod("j", Object[] .class);
v_j_L_A = Methods.class.getMethod("j", A [] .class);
v_j_LL_d = Methods.class.getMethod("j", double[][].class);
v_j_LL_O = Methods.class.getMethod("j", Object[][].class);
v_j_L_d_i = Methods.class.getMethod("j", double[] .class,
int .class);
v_j_L_O_i = Methods.class.getMethod("j", Object[] .class,
int .class);
}
catch (Exception e) {
throw new RuntimeException(e);
Expand Down Expand Up @@ -143,6 +163,15 @@ public static class Methods

assertNotNull(v_h_i);
assertNotNull(v_h_I);

assertNotNull(v_j_L_d);
assertNotNull(v_j_L_D);
assertNotNull(v_j_L_O);
assertNotNull(v_j_L_A);
assertNotNull(v_j_LL_d);
assertNotNull(v_j_LL_O);
assertNotNull(v_j_L_d_i);
assertNotNull(v_j_L_O_i);
}

// Least to most specific
Expand Down Expand Up @@ -174,6 +203,16 @@ public void g(Double x) { }
// These are effectively equal for the purposes of specificity
public void h(int x) { }
public void h(Integer x) { }

// An array of primitives should bind tighter than an array of Objects
public void j(double[] x) { }
public void j(Double[] x) { }
public void j(Object[] x) { }
public void j(A[] x) { }
public void j(double[][] x) { }
public void j(Object[][] x) { }
public void j(double[] x, int i) { }
public void j(Object[] x, int i) { }
}

/**
Expand Down Expand Up @@ -260,5 +299,31 @@ public void testCompare()
assertTrue(compareBySpecificity(Methods.v_g_n, Methods.v_g_d) > 0);
assertTrue(compareBySpecificity(Methods.v_h_I, Methods.v_h_i) == 0);
assertTrue(compareBySpecificity(Methods.v_h_i, Methods.v_h_I) == 0);

// Now compare methods taking arrays. These are a little tricky but we
// essentially want to make sure that the Python side can't bind to two
// different methods (taking arrays) where one is obviously more
// specific than the other. This differs from Java semantics since you
// can't lazily cast arrays in such a manner.
assertTrue(compareBySpecificity(Methods.v_j_L_d, Methods.v_j_L_d ) == 0);
assertTrue(compareBySpecificity(Methods.v_j_L_D, Methods.v_j_L_D ) == 0);
assertTrue(compareBySpecificity(Methods.v_j_L_A, Methods.v_j_L_A ) == 0);
assertTrue(compareBySpecificity(Methods.v_j_L_O, Methods.v_j_L_O ) == 0);
assertTrue(compareBySpecificity(Methods.v_j_L_d, Methods.v_j_L_D ) == 0);
assertTrue(compareBySpecificity(Methods.v_j_L_D, Methods.v_j_L_d ) == 0);
assertTrue(compareBySpecificity(Methods.v_j_L_D, Methods.v_j_L_A ) == 0);
assertTrue(compareBySpecificity(Methods.v_j_L_d, Methods.v_j_L_A ) < 0);
assertTrue(compareBySpecificity(Methods.v_j_L_d, Methods.v_j_L_O ) < 0);
assertTrue(compareBySpecificity(Methods.v_j_L_O, Methods.v_j_L_d ) > 0);
assertTrue(compareBySpecificity(Methods.v_j_LL_d, Methods.v_j_LL_d ) == 0);
assertTrue(compareBySpecificity(Methods.v_j_L_d, Methods.v_j_LL_d ) == 0);
assertTrue(compareBySpecificity(Methods.v_j_LL_d, Methods.v_j_L_d ) == 0);
assertTrue(compareBySpecificity(Methods.v_j_LL_O, Methods.v_j_LL_O ) == 0);
assertTrue(compareBySpecificity(Methods.v_j_LL_d, Methods.v_j_LL_O ) < 0);
assertTrue(compareBySpecificity(Methods.v_j_LL_O, Methods.v_j_LL_d ) > 0);
assertTrue(compareBySpecificity(Methods.v_j_L_d_i, Methods.v_j_L_d_i) == 0);
assertTrue(compareBySpecificity(Methods.v_j_L_O_i, Methods.v_j_L_O_i) == 0);
assertTrue(compareBySpecificity(Methods.v_j_L_d_i, Methods.v_j_L_O_i) < 0);
assertTrue(compareBySpecificity(Methods.v_j_L_O_i, Methods.v_j_L_d_i) > 0);
}
}

0 comments on commit cf4e15f

Please sign in to comment.