java method calls cannot omit varargs
Description
Problem
Clojure calls to Java vararg methods require creating an object array for the final arg. This is a frequent source of confusion when doing interop.
E.g., trying to call java.util.Collections.addAll(Collection c, T... elements):
The Method class provides an isVarArg() method, which could be used to inform the compiler to process things differently.
From http://groups.google.com/group/clojure/browse_thread/thread/7d0d6cb32656a621
Latest patch: Removed because incomplete and goal not clear
Varargs in Java
As currently stated, the scope of this ticket is only to omit varargs, but this is only one case where Clojures handling of varargs differs from Java. For completeness, here is a brief survey of how Java handles vararg methods, which could hopefully inform a discussion for how Clojure could do things differently, and what the goal of this ticket should be.
Given the following setup:
VarArgs.java
Java | Possible clojure equivalent? | Comments |
|---|---|---|
VarArgs.SingleVarargMethod.m("a"); | (SingleVarargMethod/m "a") | |
VarArgs.SingleVarargMethod.m("a", "b"); | (SingleVarargMethod/m "a" "b") | |
VarArgs.SingleVarargMethod.m("a", "b", "c"); | (SingleVarargMethod/m "a" "b" "c") | |
VarArgs.SingleVarargMethod.m("a", new String[]{"b", "c"}); | (SingleVarargMethod/m "a" (object-array ["b" "c"])) | |
VarArgs.MultipleVarargMethods.m(); | (MultipleVarargMethods/m) | |
VarArgs.MultipleVarargMethods.m((String) null); | (MultipleVarargMethods/m nil) | Use type hints to disambiguate? |
VarArgs.MultipleVarargMethods.m((String[]) null); | (MultipleVarargMethods/m nil) | Use type hints to disambiguate? |
VarArgs.MultipleVarargMethods.m("a", null); | (MultipleVarargMethods/m "a" nil) | |
VarArgs.MultipleVarargMethods.m("a", new String[]{}); | (MultipleVarargMethods/m "a" (object-array 0)) | |
VarArgs.MultipleVarargMethods.m(new String[]{"a"}); | (MultipleVarargMethods/m (object-array ["a"])) | |
VarArgs.MultipleVarargMethods.m("a", new String[]{"b", "c"}); | (MultipleVarargMethods/m "a" (object-array ["b" "c"])) |
Environment
Activity
I had a stab at this, have attached an initial patch, parts of which I'm not too sure/happy about so feedback would be appreciated.
The patch takes the following approach:
Teach Reflector/getMethods how to find matching vararg methods. In addition to the current constraints, a method can also match if it is a varargs method, and the arity of the method is one more than the requested arity. That means it's a varargs method we could call, but the user hasn't provided the varargs argument.
In MethodExpr/emitTypedArgs we handle the case were there is one more argument in the method being called than there were arguments provided. The only case were that should happen is when it is a varargs method and the last argument was not provided. In that case we push a new empty object array to the stack.
I'm not to sure about my implementation of the second part. It could open up for some hard to understand bugs in the future. One option would be to be more defensive, and make sure it's really the last argument for instance, or even pass along the Method object (or a varargs flag) so we know what we can expect and need to do.
I realised my patch is missing two important cases; the interface handling in Reflector and handling multiple matching methods. I'll look into that too, but would still appreciate feedback on the approach in MethodExpr/emitTypedArgs.
I am in favor of using isVarArg() to explicitly handle this case rather than guessing if we're in this situation. We should check the behavior (and add tests where it seems needed) for calling a var args method with too few args, too many args, etc. And also double-check that non vararg cases have not changed behavior.
Also, keep in mind that as a general rule, existing AOT compiled code may rely on calling into public Reflector methods, so if you change the signatures of public Reflector methods, you should leave a version with the old arity that has some default behavior for backwards compatibility.
Any extra logic that the compiler implements will need to distinguish between:
which I don't think is possible generically, without breaking code.
Rather than omitting varargs, how about handling them without tedious array construction. An alternative is to introduce new explicit sugar that you have to opt-in to:
where ... or similar would be understood by StaticMethodExpr or InstanceMethodExpr in the compiler, and could be type-hinted to resolve ambiguity. This would not be a breaking change.
Comment made by: pbwolf
If javac can tell the methods apart without the caller providing a "..." partition, so should Clojure. (But in the specific "MultipleVarargMethods" class, javac can't distinguish:
)
