Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: jruby/jruby
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: ff5333105c71
Choose a base ref
...
head repository: jruby/jruby
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: a1a99aaf88c0
Choose a head ref
  • 10 commits
  • 4 files changed
  • 1 contributor

Commits on Jul 21, 2015

  1. Copy the full SHA
    b0dcd91 View commit details
  2. tune proc-to-iface callable matching to do less work

    ... and handle proc's with arity >
    
    than the functional-interface methods parameter count
    kares committed Jul 21, 2015
    Copy the full SHA
    581a815 View commit details
  3. re-arrange proc arity matching

    avoids incorrect ambiguous warning e.g. in case of :
    `java.io.File.new('.').listFiles { |pathname| ... }`
    kares committed Jul 21, 2015
    Copy the full SHA
    7d24ec4 View commit details
  4. Copy the full SHA
    e7fcfd6 View commit details
  5. [ji] fix missing local assing + do not match duckables when primitiva…

    …ble/assignable found
    
    this avoids an incorrect ambiguous warning when matching proc-to-iface methods e.g. : 
    `executor_spec.rb:22 warning: ambiguous Java methods found, using submit(java.util.concurrent.Callable)`
    
    ... also exploded the 3 level nested for loop to be only 2 levels
    kares committed Jul 21, 2015
    Copy the full SHA
    9599674 View commit details
  6. forgot to initialize the mostSpecificArity for the first list element…

    … (thus not matching)
    kares committed Jul 21, 2015
    Copy the full SHA
    120e0ab View commit details
  7. a unit test since we can not influence getMethods order (closer to fa…

    …ilure at travis-ci)
    kares committed Jul 21, 2015
    Copy the full SHA
    623a6f1 View commit details
  8. [ji] travis showcased negative proc-to-iface arity (still) dependent …

    …on getMethods order
    kares committed Jul 21, 2015
    Copy the full SHA
    3d7ac8f View commit details
  9. travis showcased we did not handle all negative proc-to-iface arity c…

    …ases correctly
    
    ... they were still dependent on getMethods order (travis-ci failures in **spec:ji**)
    kares committed Jul 21, 2015
    Copy the full SHA
    e20d6c7 View commit details
  10. Copy the full SHA
    a1a99aa View commit details
181 changes: 116 additions & 65 deletions core/src/main/java/org/jruby/java/dispatch/CallableSelector.java
Original file line number Diff line number Diff line change
@@ -166,15 +166,29 @@ private static <T extends ParameterTypes> T findMatchingCallableForArgs(final Ru
Class<?>[] msTypes = mostSpecific.getParameterTypes();
boolean ambiguous = false;

final IRubyObject lastArg = args.length > 0 ? args[ args.length - 1 ] : null;
int mostSpecificArity = Integer.MIN_VALUE;

final int procArity;
if ( lastArg instanceof RubyProc ) {
final Method implMethod; final int last = msTypes.length - 1;
if ( last >= 0 && msTypes[last].isInterface() && ( implMethod = getFunctionalInterfaceMethod(msTypes[last]) ) != null ) {
mostSpecificArity = implMethod.getParameterTypes().length;
}
procArity = ((RubyProc) lastArg).getBlock().arity().getValue();
}
else {
procArity = Integer.MIN_VALUE;
}

OUTER: for ( int c = 1; c < size; c++ ) {
final T candidate = candidates.get(c);
final Class<?>[] cTypes = candidate.getParameterTypes();

for ( int i = 0; i < msTypes.length; i++ ) {
final Class<?> msType = msTypes[i], cType = cTypes[i];
if ( msType != cType && msType.isAssignableFrom(cType) ) {
mostSpecific = candidate;
msTypes = cTypes;
mostSpecific = candidate; msTypes = cTypes;
ambiguous = false; continue OUTER;
}
}
@@ -192,26 +206,68 @@ else if ( cType.isPrimitive() && msType.isAssignableFrom(getBoxType(cType)) ) {
}
}

// special handling if we're dealing with Proc#impl :
if ( procArity != Integer.MIN_VALUE ) { // lastArg instanceof RubyProc
// cases such as (both ifaces - differ in arg count) :
// java.io.File#listFiles(java.io.FileFilter) ... accept(File)
// java.io.File#listFiles(java.io.FilenameFilter) ... accept(File, String)
final Method implMethod; final int last = cTypes.length - 1;
if ( last >= 0 && cTypes[last].isInterface() && ( implMethod = getFunctionalInterfaceMethod(cTypes[last]) ) != null ) {
// we're sure to have an interface in the end - match arg count :
// NOTE: implMethod.getParameterCount() on Java 8 would do ...
final int methodArity = implMethod.getParameterTypes().length;
if ( methodArity == procArity ) {
if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity
// TODO we could try to match parameter types with arg types
else {
mostSpecific = candidate; msTypes = cTypes;
mostSpecificArity = procArity; ambiguous = false;
}
continue; /* OUTER */ // we want to check all
}
else if ( mostSpecificArity != procArity ) {
if ( methodArity < procArity ) { // not ideal but still usable
if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity
else if ( mostSpecificArity < methodArity ) { // candidate is "better" match
mostSpecific = candidate; msTypes = cTypes;
mostSpecificArity = methodArity; ambiguous = false;
}
continue; /* OUTER */
}
else if ( procArity < 0 && methodArity >= -(procArity + 1) ) { // *splat that fits
if ( mostSpecificArity == methodArity ) ambiguous = true; // 2 with same arity
else {
final int msa = mostSpecificArity + procArity;
final int ma = methodArity + procArity;
if ( ( msa < 0 && ma < 0 && msa < ma ) ||
( msa >= 0 && ma >= 0 && msa > ma ) ||
( msa > ma ) ) {
mostSpecific = candidate; msTypes = cTypes;
mostSpecificArity = methodArity; // ambiguous = false;
}
}
ambiguous = false;
continue; /* OUTER */
}
}
else { // we're not a match and if there's something else matched than it's not really ambiguous
ambiguous = false; /* continue; /* OUTER */
}
}
}

// somehow we can still decide e.g. if we got a RubyFixnum
// then (int) constructor should be preferred over (float)
if ( ambiguous ) {
// special handling if we're dealing with Proc#impl :
final IRubyObject lastArg = args.length > 0 ? args[ args.length - 1 ] : null;
final T procToIfaceMatch = matchProcToInterfaceCandidate(lastArg, candidates);
if ( procToIfaceMatch != null ) {
mostSpecific = procToIfaceMatch; ambiguous = false;
}
else {
int msPref = 0, cPref = 0;
for ( int i = 0; i < msTypes.length; i++ ) {
final Class<?> msType = msTypes[i], cType = cTypes[i];
msPref += calcTypePreference(msType, args[i]);
cPref += calcTypePreference(cType, args[i]);
}
// for backwards compatibility we do not switch to cType as
// the better fit - we seem to lack tests on this front ...
if ( msPref > cPref ) ambiguous = false; // continue OUTER;
int msPref = 0, cPref = 0;
for ( int i = 0; i < msTypes.length; i++ ) {
final Class<?> msType = msTypes[i], cType = cTypes[i];
msPref += calcTypePreference(msType, args[i]);
cPref += calcTypePreference(cType, args[i]);
}
// for backwards compatibility we do not switch to cType as
// the better fit - we seem to lack tests on this front ...
if ( msPref > cPref ) ambiguous = false; // continue OUTER;
}
}
method = mostSpecific;
@@ -224,55 +280,28 @@ else if ( cType.isPrimitive() && msType.isAssignableFrom(getBoxType(cType)) ) {

// fall back on old ways
if (method == null) {
method = findCallable(methods, Exact, args);
if (method == null) {
method = findCallable(methods, AssignableAndPrimitivable, args);
if (method == null) {
method = findCallable(methods, AssignableOrDuckable, args);
if (method == null) {
method = findCallable(methods, AssignableOrDuckable, args);
if (method == null) {
method = findCallable(methods, AssignableAndPrimitivableWithVarargs, args);
}
}
}
}
method = findMatchingCallableForArgsFallback(runtime, methods, args);
}

return method;
}

private static <T extends ParameterTypes> T matchProcToInterfaceCandidate(
final IRubyObject lastArg, final List<T> candidates) {
if ( lastArg instanceof RubyProc ) {
// cases such as (both ifaces - differ in arg count) :
// java.io.File#listFiles(java.io.FileFilter) ... accept(File)
// java.io.File#listFiles(java.io.FilenameFilter) ... accept(File, String)
final int arity = ((RubyProc) lastArg).getBlock().arity().getValue();
T match = null;
for ( int i = 0; i < candidates.size(); i++ ) {
final T method = candidates.get(i);

final Class<?>[] params = method.getParameterTypes();

if ( params.length == 0 ) return null; // can not match (no args)
final Class<?> lastParam = params[ params.length - 1 ];

if ( ! lastParam.isInterface() ) return null; // can not match

final Method implMethod = getFunctionalInterfaceMethod(lastParam);
if ( implMethod != null ) {
// we're sure to have an interface in the end - match arg count :
// NOTE: implMethod.getParameterCount() on Java 8 would do ...
if ( implMethod.getParameterTypes().length == arity ) {
if ( match != null ) return null; // 2 with same arity (can not match)
match = method; // do not break here we want to check all
private static <T extends ParameterTypes> T findMatchingCallableForArgsFallback(final Ruby runtime,
final T[] methods, final IRubyObject... args) {
T method = findCallable(methods, Exact, args);
if (method == null) {
method = findCallable(methods, AssignableAndPrimitivable, args);
if (method == null) {
method = findCallable(methods, AssignableOrDuckable, args);
if (method == null) {
method = findCallable(methods, AssignableOrDuckable, args);
if (method == null) {
method = findCallable(methods, AssignableAndPrimitivableWithVarargs, args);
}
}
}
return match;
}
return null;
return method;
}

private static <T extends ParameterTypes> T findCallable(T[] callables, CallableAcceptor acceptor, IRubyObject[] args) {
@@ -304,17 +333,42 @@ private static <T extends ParameterTypes> List<T> findCallableCandidates(final T
ParameterTypes[] incoming = callables.clone();

for ( int i = 0; i < args.length; i++ ) {
retained.clear();
for ( final Matcher matcher : NON_EXACT_MATCH_SEQUENCE ) {
retained.clear(); // non-exact match sequence :
// PRIMITIVABLE :
for ( int c = 0; c < incoming.length; c++ ) {
ParameterTypes callable = incoming[c];
if ( callable == null ) continue; // removed (matched)

Class[] types = callable.getParameterTypes();

if ( PRIMITIVABLE.match( types[i], args[i] ) ) {
retained.add((T) callable);
incoming[c] = null; // retaining - remove
}
}
// ASSIGNABLE :
for ( int c = 0; c < incoming.length; c++ ) {
ParameterTypes callable = incoming[c];
if ( callable == null ) continue; // removed (matched)

Class[] types = callable.getParameterTypes();

if ( ASSIGNABLE.match( types[i], args[i] ) ) {
retained.add((T) callable);
incoming[c] = null; // retaining - remove
}
}
if ( retained.isEmpty() ) {
// DUCKABLE :
for ( int c = 0; c < incoming.length; c++ ) {
ParameterTypes callable = incoming[c];
if ( callable == null ) continue; // removed (matched)

Class[] types = callable.getParameterTypes();

if ( matcher.match( types[i], args[i] ) ) {
if ( DUCKABLE.match( types[i], args[i] ) ) {
retained.add((T) callable);
incoming[c] = null; // retaining - remove
//incoming[c] = null; // retaining - remove
}
}
}
@@ -412,9 +466,6 @@ public boolean match(Class type, IRubyObject arg) {
@Override public String toString() { return "DUCKABLE"; } // for debugging
};

//private static final Matcher[] MATCH_SEQUENCE = new Matcher[] { EXACT, PRIMITIVABLE, ASSIGNABLE, DUCKABLE };
private static final Matcher[] NON_EXACT_MATCH_SEQUENCE = new Matcher[] { PRIMITIVABLE, ASSIGNABLE, DUCKABLE };

private static boolean exactMatch(ParameterTypes paramTypes, IRubyObject... args) {
Class[] types = paramTypes.getParameterTypes();

9 changes: 2 additions & 7 deletions core/src/main/java/org/jruby/javasupport/JavaMethod.java
Original file line number Diff line number Diff line change
@@ -46,7 +46,6 @@
import org.jruby.Ruby;
import org.jruby.RubyBoolean;
import org.jruby.RubyClass;
import org.jruby.RubyInstanceConfig;
import org.jruby.RubyModule;
import org.jruby.RubyString;
import org.jruby.anno.JRubyClass;
@@ -57,17 +56,13 @@
import org.jruby.javasupport.proxy.JavaProxyMethod;
import org.jruby.runtime.ObjectAllocator;
import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.util.log.Logger;
import org.jruby.util.log.LoggerFactory;

import static org.jruby.util.CodegenUtils.getBoxType;
import static org.jruby.util.CodegenUtils.prettyParams;

@JRubyClass(name="Java::JavaMethod")
public class JavaMethod extends JavaCallable {

//private static final Logger LOG = LoggerFactory.getLogger("JavaMethod");

//private final static boolean USE_HANDLES = RubyInstanceConfig.USE_GENERATED_HANDLES;
//private final static boolean HANDLE_DEBUG = false;

@@ -188,7 +183,7 @@ public static JavaMethod getMatchingDeclaredMethod(Ruby runtime, Class<?> javaCl

@Override
public final boolean equals(Object other) {
return other instanceof JavaMethod && this.method.equals( ((JavaMethod)other).method );
return other instanceof JavaMethod && this.method.equals( ((JavaMethod) other).method );
}

@Override
@@ -594,7 +589,7 @@ public String toGenericString() {
}

@Override
public AccessibleObject accessibleObject() {
public final AccessibleObject accessibleObject() {
return method;
}

171 changes: 171 additions & 0 deletions core/src/test/java/org/jruby/java/dispatch/CallableSelectorTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,171 @@
/*
* Copyright (c) 2015 JRuby.
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*/
package org.jruby.java.dispatch;

import java.lang.reflect.Method;
import org.jruby.Ruby;
import org.jruby.RubyProc;
import org.jruby.javasupport.JavaMethod;
import org.jruby.runtime.Arity;
import org.jruby.runtime.Binding;
import org.jruby.runtime.Block;
import org.jruby.runtime.BlockBody;
import org.jruby.runtime.Frame;
import org.jruby.runtime.NullBlockBody;
import org.jruby.runtime.backtrace.BacktraceElement;
import org.jruby.runtime.builtin.IRubyObject;
import org.jruby.util.collections.IntHashMap;

import org.junit.Test;
import static org.junit.Assert.*;

/**
* @author kares
*/
public class CallableSelectorTest {

@Test
public void testCallableProcToIfaceMatchIsNotOrderSensitive() throws Exception {
final Ruby runtime = Ruby.newInstance();

final Method list1 = java.io.File.class.getMethod("listFiles", java.io.FileFilter.class);
final Method list2 = java.io.File.class.getMethod("listFiles", java.io.FilenameFilter.class);

IntHashMap cache;
JavaMethod[] methods;
Binding binding = new Binding(new Frame(), null, null, new BacktraceElement());
JavaMethod result; IRubyObject[] args;

// arity 1 :

BlockBody body1 = new NullBlockBody() {
@Override public Arity arity() { return Arity.ONE_ARGUMENT; }
};
RubyProc dummyProc = RubyProc.newProc(runtime, new Block(body1, binding), Block.Type.PROC);

cache = IntHashMap.nullMap();
methods = new JavaMethod[] {
new JavaMethod(runtime, list1), new JavaMethod(runtime, list2)
};
result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
assertEquals(new JavaMethod(runtime, list1), result);

cache = IntHashMap.nullMap();
args = new IRubyObject[] { dummyProc };
result = CallableSelector.matchingCallableArityN(runtime, cache, methods, args);
assertEquals(new JavaMethod(runtime, list1), result);

cache = IntHashMap.nullMap();
methods = new JavaMethod[] { // "reverse" method order
new JavaMethod(runtime, list2), new JavaMethod(runtime, list1)
};
result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
assertEquals(new JavaMethod(runtime, list1), result);

cache = IntHashMap.nullMap();
args = new IRubyObject[] { dummyProc };
result = CallableSelector.matchingCallableArityN(runtime, cache, methods, args);
assertEquals(new JavaMethod(runtime, list1), result);

// arity 2 :

BlockBody body2 = new NullBlockBody() {
@Override public Arity arity() { return Arity.TWO_ARGUMENTS; }
};
dummyProc = RubyProc.newProc(runtime, new Block(body2, binding), Block.Type.PROC);

cache = IntHashMap.nullMap();
methods = new JavaMethod[] {
new JavaMethod(runtime, list1), new JavaMethod(runtime, list2)
};
result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
assertEquals(new JavaMethod(runtime, list2), result);

cache = IntHashMap.nullMap();
args = new IRubyObject[] { dummyProc };
result = CallableSelector.matchingCallableArityN(runtime, cache, methods, args);
assertEquals(new JavaMethod(runtime, list2), result);

cache = IntHashMap.nullMap();
methods = new JavaMethod[] { // "reverse" method order
new JavaMethod(runtime, list2), new JavaMethod(runtime, list1)
};
result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
assertEquals(new JavaMethod(runtime, list2), result);

cache = IntHashMap.nullMap();
args = new IRubyObject[] { dummyProc };
result = CallableSelector.matchingCallableArityN(runtime, cache, methods, args);
assertEquals(new JavaMethod(runtime, list2), result);

// arity -1 :

BlockBody body_1 = new NullBlockBody() { // arity -1
@Override public Arity arity() { return Arity.OPTIONAL; }
};
dummyProc = RubyProc.newProc(runtime, new Block(body_1, binding), Block.Type.PROC);

cache = IntHashMap.nullMap();
methods = new JavaMethod[] {
new JavaMethod(runtime, list1), new JavaMethod(runtime, list2)
};
result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
assertEquals(new JavaMethod(runtime, list1), result);

cache = IntHashMap.nullMap();
methods = new JavaMethod[] {
new JavaMethod(runtime, list2), new JavaMethod(runtime, list1)
};
result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
assertEquals(new JavaMethod(runtime, list1), result);

// arity -3 :

BlockBody body_3 = new NullBlockBody() { // arity -3
@Override public Arity arity() { return Arity.TWO_REQUIRED; }
};
dummyProc = RubyProc.newProc(runtime, new Block(body_3, binding), Block.Type.PROC);

cache = IntHashMap.nullMap();
methods = new JavaMethod[] {
new JavaMethod(runtime, list1), new JavaMethod(runtime, list2)
};
result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
assertEquals(new JavaMethod(runtime, list2), result);

cache = IntHashMap.nullMap();
methods = new JavaMethod[] {
new JavaMethod(runtime, list2), new JavaMethod(runtime, list1)
};
result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
assertEquals(new JavaMethod(runtime, list2), result);

// arity -2 :

BlockBody body_2 = new NullBlockBody() { // arity -2 (arg1, *rest) should prefer (single)
@Override public Arity arity() { return Arity.ONE_REQUIRED; }
};
dummyProc = RubyProc.newProc(runtime, new Block(body_2, binding), Block.Type.PROC);

cache = IntHashMap.nullMap();
methods = new JavaMethod[] {
new JavaMethod(runtime, list1), new JavaMethod(runtime, list2)
};
result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
assertEquals(new JavaMethod(runtime, list1), result);

cache = IntHashMap.nullMap();
methods = new JavaMethod[] {
new JavaMethod(runtime, list2), new JavaMethod(runtime, list1)
};
result = CallableSelector.matchingCallableArityOne(runtime, cache, methods, dummyProc);
assertEquals(new JavaMethod(runtime, list1), result);

}

}
36 changes: 36 additions & 0 deletions spec/java_integration/interfaces/implementation_spec.rb
Original file line number Diff line number Diff line change
@@ -218,6 +218,42 @@ def callIt
dir.should be_kind_of(java.io.File)
name.should be_kind_of(String)
end

java.io.File.new('.').listFiles do |dir, name, invalid|
# should choose FilenameFilter#accept(File, String)
dir.should be_kind_of(java.io.File)
name.should be_kind_of(String)
end

java.io.File.new('.').listFiles do |pathname, *args|
pathname.should be_kind_of(java.io.File)
args.should be_empty
end

java.io.File.new('.').listFiles do |dir, name, *args|
dir.should be_kind_of(java.io.File)
name.should be_kind_of(String)
args.should be_empty
end

java.io.File.new('.').listFiles do |*args|
args[0].should be_kind_of(java.io.File)
args.size.should eql 1
end

#executor = java.util.concurrent.Executors.newSingleThreadExecutor
#executor.execute { |*args| args.should be_empty }; sleep 0.1
#executor.shutdown

work_queue = java.util.concurrent.LinkedBlockingQueue.new
executor = java.util.concurrent.ThreadPoolExecutor.new(0, 2, 0, java.util.concurrent.TimeUnit::SECONDS, work_queue) do
|*args| # newThread(Runnable)
args[0].should be_kind_of(java.lang.Runnable)
args.size.should eql 1
java.lang.Thread.new(args[0])
end
executor.execute { |*args| args.should be_empty }; sleep 0.1
executor.shutdown
end

it "should maintain Ruby object equality when passed through Java and back" do