Welcome! Please see the About page for a little more info on how this works.

+6 votes
in Java Interop by

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):

user=> (Collections/addAll [] (object-array 0)) false user=> (Collections/addAll []) IllegalArgumentException No matching method: addAll clojure.lang.Compiler$StaticMethodExpr.<init> (Compiler.java:1401)

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:

`
public class VarArgs {

public static class SingleVarargMethod {
    public static void m(String arg1, String... args) {}
}

public static class MultipleVarargMethods {
    public static void m(String... args) {}
    public static void m(String arg1) {}
    public static void m(String arg1, String... args) {}
}

}
`

|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"]))}} | |

13 Answers

0 votes
by

Comment made by: importer

Converted from http://www.assembla.com/spaces/clojure/tickets/440

0 votes
by

Comment made by: ataggart

Patch adds support for varargs. Builds on top of patch in CLJ-445.

0 votes
by

Comment made by: ataggart

Patch updated to current CLJ-445 patch.

0 votes
by

Comment made by: klauern

Is this ticket on hold? I find myself typing {{(.someCall arg1 arg2 (into-array SomeType nil))}} alot just to get the right method to be called. This ticket sounds like it would address that extraneous {{into-array}} arg that I use alot.

0 votes
by

Comment made by: jafingerhut

fixbug445.diff uploaded on Oct 29 2012 was written Oct 23 2010 by Alexander Taggart. I am simply copying it from the old Assembla ticket tracking system to here to make it more easily accessible. Not surprisingy, it doesn't apply cleanly to latest master. I don't know how much effort it would be to update it, but only a few hunks do not apply cleanly according to 'patch'. See the "Updating stale patches" section on the JIRA workflow page here: http://dev.clojure.org/display/design/JIRA workflow

0 votes
by

Comment made by: jafingerhut

Ugh. Deleted the attachment because it was for CLJ-445, or at least it was named that way. CLJ-445 definitely has a long comment history, so if one or more of its patches address this issue, then you can read the discussion there to see the history.

I don't know of any "on hold" status for tickets, except for one or two where Rich Hickey has explicitly said in a comment that he wants to wait a while before making the change. There are just tickets that contributors choose to work on and ones that screeners choose to screen.

0 votes
by

Comment made by: alexmiller

I would love to see an updated patch on this ticket that specifically addressed the varargs issue without building on the other mentioned ticket and patch (which is of lower priority).

0 votes
by

Comment made by: ragge

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:

  1. 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.
  2. 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.

0 votes
by

Comment made by: ragge

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}}.

0 votes
by

Comment made by: alexmiller

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.

0 votes
by

Comment made by: gshayban

Any extra logic that the compiler implements will need to distinguish between:

`

public static class MultipleVarargMethods {
    public static void m(String... args) {}
    public static void m(String arg1, String... args) {}
}

`

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:

(Whatever/varargs a b c ... x y z)

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.

0 votes
by

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:

`
public class MultipleVarargMethods {

public static void m(String... args) {}
public static void m(String arg1, String... args) {}
public static void main(String [] args) {
    m("bankruptcy");
    m("bankruptcy","in","progress");
}

}

MultipleVarargMethods.java:5: error: reference to m is ambiguous

    m("bankruptcy");
    ^

both method m(String...) in MultipleVarargMethods and method m(String,String...) in MultipleVarargMethods match
MultipleVarargMethods.java:6: error: reference to m is ambiguous

    m("bankruptcy","in","progress");
    ^

both method m(String...) in MultipleVarargMethods and method m(String,String...) in MultipleVarargMethods match
2 errors
`

)

0 votes
by
Reference: https://clojure.atlassian.net/browse/CLJ-440 (reported by ataggart)
...