Problem dispatching array as argument to a closure

Looked a bit into it and obviously the problem is related to how ClosureMetaClass(CMC) and ExpandoMetaClass choose the method to invoke.

1) The ClosureMetaClass chooses the method to invoke via StandardClosureChooser as it finds the following two methods burnt into the class and restricts its method selection to these two going just by the number of arguments (and selects "doCall(Object)" when "closure(myObjectArray)" call is made:

- public Object doCall()
- public Object doCall(Object it)

2) On the other hand, the EMC makes its choice from among the following methods and chooses Closure.call(Object... args) as a more exact match:

- public Object groovy.lang.Closure.call()
- public Object groovy.lang.Closure.call(java.lang.Object[])
- public Object groovy.lang.Closure.call(java.lang.Object)

I think the difference in behavior is arising because groovy.lang.Closure has an extra "call(Object...)" and its counterpart "doCall(Object...)" is missing in closure's bytecode. Shouln't these two sets of methods be in sync always? Once they are, we should have consistent behavior one way or another, whether CMC is used, or EMC.

Apart from the above mentioned inconsistency on Groovy side, there is another inconsistency on Java side that is in play - related to how the direct dispatch in it differs from a reflection based dispatch:

import java.lang.reflect.Method;

public class TryInJava {
    public static void main(String[] args) throws Exception {
        Object[] strs = new Object[] {"a", "b"};
        Object[] inputs = new Object[] {strs};
        Method m = Foo.class.getMethod("call", Object[].class);
        Foo foo = new Foo();
        foo.call(inputs); // this prints "1" as the inputs array reaches method "call" wrapped properly - as {{"a", "b"}}.
        m.invoke(foo, inputs); // this prints "2" as now the inputs array reaches method "call" unwrapped - as {"a", "b"}.
    }
}


class Foo {
    public void call(Object[] inputs) {
        System.out.println(inputs.length);
    }
}

Since we cannot remove call(Object... args)/call(Object[]) from public API of groovy.lang.Closure and since Groovy has to do a reflection based call, removing the Groovy side inconsistency may result in failure to dispatch the array to a closure in both CMC and EMC cases - unless we then treat it as some special case.

I think the solution is to make EMC doCall instead of call

call is intended to be a kind of dispatcher method and actually behaving as it should, even though that makes no sense if called directly in Grovy

Can we prioritize a fix on this? I would like to get it tested against Grails as I'm concerned about what other regressions could be caused by making a change in behavior like this. In particular I'm concerned about meta-programming logic in EMC and whether it will be impacted by this change.

For example what will be the impact on methodMissing logic like this which currently works:

https://github.com/SpringSource/grails-data-mapping/blob/44031e133d3f7d3c93f575c38b4038bee246292c/grails-datastore-gorm/src/main/groovy/org/grails/datastore/gorm/GormStaticApi.groovy#L102

Anyway regardless I think it is important we get this change in soon so we can do some appropriate testing

should be fixed now

This still seems to be an issue. Code to reproduce:

ExpandoMetaClass.enableGlobally()
def args = [1, 2] as Object[]
def c = {}
c(args)

Exception:

groovy.lang.MissingMethodException: No signature of method: ConsoleScript6$_run_closure1.doCall() is applicable for argument types: (java.lang.Integer, java.lang.Integer) values: [1, 2]
Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll()
	at org.codehaus.groovy.runtime.ScriptBytecodeAdapter.unwrap(ScriptBytecodeAdapter.java:55)
	at groovy.lang.Closure$call.call(Unknown Source)
	at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:42)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:108)
	at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:116)
	at ConsoleScript6.run(ConsoleScript6:4)
	at groovy.lang.GroovyShell.runScriptOrMainOrTestOrRunnable(GroovyShell.java:266)
	

This looks like it may be a regression introduced in 1.8.1.

ExpandoMetaClass.enableGlobally()
List l = new ArrayList()
l << ([1, 2] as Object[])

l.each {
	println "it is ${it}"
}

With 1.8.0...

grv $ groovy -version
Groovy Version: 1.8.0 JVM: 1.6.0_26
grv $ groovy doit.groovy 
it is [1, 2]
grv $ 

With 1.8.1...

grv $ groovy -version
gGroovy Version: 1.8.1 JVM: 1.6.0_26
grv $ groovy doit.groovy 
Caught: groovy.lang.MissingMethodException: No signature of method: doit$_run_closure1.doCall() is applicable for argument types: (java.lang.Integer, java.lang.Integer) values: [1, 2]
Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll()
groovy.lang.MissingMethodException: No signature of method: doit$_run_closure1.doCall() is applicable for argument types: (java.lang.Integer, java.lang.Integer) values: [1, 2]
Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll()
	at doit.run(doit.groovy:5)
grv $ 

With 1.8.2...

grv $ groovy -version
grGroovy Version: 1.8.2 JVM: 1.6.0_26
ogrv $ groovy doit.groovy 
Caught: groovy.lang.MissingMethodException: No signature of method: doit$_run_closure1.doCall() is applicable for argument types: (java.lang.Integer, java.lang.Integer) values: [1, 2]
Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll()
groovy.lang.MissingMethodException: No signature of method: doit$_run_closure1.doCall() is applicable for argument types: (java.lang.Integer, java.lang.Integer) values: [1, 2]
Possible solutions: doCall(), doCall(java.lang.Object), call(), call([Ljava.lang.Object;), call(java.lang.Object), findAll()
	at doit.run(doit.groovy:5)
grv $ 

the original example of this issue still holds, but is not ot be fixed for 1.8 or 1.7 and is not o be fixed for Grails as well

Guys are there any plans to fix this?

The problem is how to fix this. We cannot fix this without breaking code at another place. The call(Object...) could be called with call('a','b') or with def x=['a','b'] as Object[]; call(x). For the call method they are the same, but for the doCall methods they are not!

For example... let us say we add a doCall(Object...) method, then either we would let it do what call does and in the end have the same problem and nothing fixed, or we change the type of the implicit it to be an Object[] instead of being an Object. The disadvantages of this are the semantic change for the much implicit it parameter, which will break a lot of code, and that you will then never get a MissingMethodException from the closure unless you gave in the type yourself. And even then it won't be always correct... assuming you have cl=

, and then do cl('a','b') you expect this to work. So far so good... but cl(x), with x being the Object[] will still also call that one

The other possible way would be to do what ClosureMetaClass does and not even think about calling the call method, but instead directing to doCall. the result for me: 11 failures and 401 errors in the build.

this is reported as fixed in 1.8.3

reopen for now till I verified it is really fixed in 1.8.9

Should we remove 1.8.3 from the "Fix Version" field?

looks like I made a fix, but then did not apply it to 2.0.x and 1.8.x

I found the fix I did back then and that it is indeed in master and 2.1.0-RC-x, but is missing on 1.8.x and 2.0.x. So I pushed the change there as well and close the issue with that