Clojure

CLJ-850-test.patch added.

When the compiler tries to generates the call to the correct overload of invokePrim, it's failing to take the return type into account. I should be calling invokePrim(Ljava/lang/Object;J)J;.

XXX this is where I got myself confused. The invokePrim overload it's trying to invoke is the correct one. But, that apparently is no the one that's being generated. Sorry for the noise.

Here's what I think I'm seeing:

HostExpr.Parse.parse() loses track of the return type, in the final else branch where method calls are handled. This is because tagOf(form), where form is something like: (. foo invokePrim 1) returns nil. (The form itself doesn't have a :tag, but I believe foo does, though that's the name of the appropriate invokePrim interface (i.e. IFn$OLL).

new InstanceMethodExpr(...) then gets constructed with tag==null, at which point we've already lost sine InstanceMethodExpr can't correctly consider overloading on the result type if it doesn't know what it is.

It's not yet clear to me how I can get InstanceMethodExpr to consider the return type, if it knew it...

There are two things going on here. I'm not sure which is the error.

It looks like the return type of the generated invokePrim method is too specific. It's generated as returning String, though the IFn$LO interface specifies returning Object.

The caller attempts to call invokePrim returning Object, which is what the interface IFn$LO specifies, but this doesn't work because methodSL doesn't actually implement that method. Instead it implements an overload returning String.

1. methodSL.invokePrim is declared as (long)->String
2. methodSL.invoke does invokeinterface with the correct return type WRT methodSL, but the wrong return type WRT the IFn$LO interface.
3. callSL.invoke does invokeinterface with the wrong return type WRT methodSL, but the correct return type WRT IFn$LO. (This is the failure we observe in the clj-850 unit test.)

(defn methodSL  ^String [^long i] (str i))

<<1>> public final java.lang.String invokePrim(long);  <<1>>
      Code:
       0:   getstatic   #25; 
            //Field const__0:Lclojure/lang/Var;
       3:   invokevirtual   #34; 
            //Method clojure/lang/Var.getRawRoot:()Ljava/lang/Object;
       6:   checkcast   #36; 
            //class clojure/lang/IFn
       9:   lload_1
       10:  invokestatic    #42; 
            //Method clojure/lang/Numbers.num:(J)Ljava/lang/Number;
       13:  invokeinterface #46,  2; 
            //InterfaceMethod clojure/lang/IFn.invoke:(Ljava/lang/Object;)Ljava/lang/Object;
       18:  checkcast   #48; 
            //class java/lang/String
       21:  areturn

      public java.lang.Object invoke(java.lang.Object);
      Code:
       0:   aload_0
       1:   aload_1
       2:   checkcast   #54; 
            //class java/lang/Number
       5:   invokestatic    #58; 
            //Method clojure/lang/RT.longCast:(Ljava/lang/Object;)J
<<2>>  8:   invokeinterface #60,  3; 
            //InterfaceMethod clojure/lang/IFn$LO.invokePrim:(J)Ljava/lang/String;
       13:  areturn

(defn callSL ^String [] (methodSL 42))

    public java.lang.Object invoke();
      Code:
       0:   getstatic   #25; 
            //Field const__0:Lclojure/lang/Var;
       3:   invokevirtual   #43; 
            //Method clojure/lang/Var.getRawRoot:()Ljava/lang/Object;
       6:   checkcast   #45; 
            //class clojure/lang/IFn$LO
       9:   ldc2_w  #26; 
            //long 42l
<<3>>  12:  invokeinterface #49,  3; 
            //InterfaceMethod clojure/lang/IFn$LO.invokePrim:(J)Ljava/lang/Object;
       17:  areturn

Given P is some primitive type, O is type Object, and R some subclass of Object:

When Clojure generates a R invokePrim(P x), it also generates a Object invoke(Object x), which delegates to R invokePrim(P x).

R invokePrim(P x) overloads, but does not override the method of the corresponding Fn$PO interface.

If Clojure were to generate an additional O invokePrim(P x) which delegates to R invokePrim(P x), it would satisfy the requirements of the Fn$PO interface, and should fix this issue.

CLJ-850.patch fixes the issue.

I consider this patch to be pretty hackish and hope that there's a cleaner way of addressing CLJ-850. This is the first time I've tried to understand (much less change) the Clojure compiler, so don't expect genius.

The patch lies slightly:

Clojure needs to generate an additional O invokePrim(P x) method to
satisfy the interface. This also delegates to R invokePrim(P x).

It turns out that what I'm actually doing is generating a R invokePrim(P x) which is a copy of O invokePrim(P x), instead of delgating to O invokePrim(P x). This works, but the resulting class file would be smaller if the patch actually did what it says it does.

clj-850-type-hinted-fn-abstractmethoderror-patch2.txt is identical to Ben's two patches combined into one, with the small modification that the new tests are added to metadata.clj instead of creating a new test file. The patch applies cleanly to latest master as of Feb 27, 2012. One of the new tests does fail without the change to the compiler, and succeeds with it. I can't vouch for the correctness of the change myself, not knowing enough about the compiler internals to judge.

Same comments as made on Feb 27, 2012, except the patch clj-850-type-hinted-fn-abstractmethoderror-patch3.txt applies cleanly to latest master as of Mar 23, 2012. Updated because previous patch (now removed) no longer applied cleanly. git patches often fail to apply if context lines near changes are modified.

We don't support sigs taking prims and returning anything other than prim or Object. Overloading on return value only is a bad idea (and forbidden in Java). The return type of the generated method should be Object, and the String return hint should be used only as a hint.

+10 for solving this issue: it keeps biting me in 1.4 and wouuld love to see in 1.5

I'm not familiar with the Clojure compiler internals, but looking at the approach, shouldn't we produce a primitive method with a different name (since Java doesn't support overloading on return types as Rich correctly points out). Also I think there should be 4 methods:

R invokePrimExact(P x) - the actual method, used when compiler can infer
R invokePrimExact(O x) - delegates, used when compiler can't infer type of x
Object invokePrim(P x) - primitive method, conforms to IFn$PO interface, delegates
Object invoke(Object x) - general method, delegates

I think this solves all the important cases?

Still no patch incorporating my feedback, afaict. Pushing to next release.

Does this new patch address the issue and concerns? (This incorporates Ben's tests from the previous patch, wasn't sure how to attribute him on that hunk) CLJ-850-conform-to-invokePrim.diff

Presumptuously changing state from Incomplete back to Vetted after Ghadi Shayban added the patch CLJ-850-conform-to-invokePrim.diff dated Dec 19 2012 after the status was changed to Incomplete.