Fork me on GitHub
#clojure-dev
<
2022-11-16
>
dmiller18:11:51

I am a bit mystified by an oddity I ran into in Clojure-generated JVM bytecode. Consider:

dmiller19:11:57

Well, first let's remember shift-return Here's the code: (defn yikes [i & [opts]] (if (<= i 0) 7 (recur (dec i) [opts]))) ;; This pattern is seen in tools.analyzer.jvm.passes/schedule

ghadi19:11:04

what's the bytecode?

Alex Miller (Clojure team)19:11:24

public final class foo$yikes extends clojure.lang.RestFn {
  public static final java.lang.Object const__3;

  public foo$yikes();
    Code:
       0: aload_0
       1: invokespecial #9                  // Method clojure/lang/RestFn."<init>":()V
       4: return

  public static java.lang.Object invokeStatic(java.lang.Object, clojure.lang.ISeq);
    Code:
       0: aload_1
       1: aconst_null
       2: astore_1
       3: astore_2
       4: aload_2
       5: aconst_null
       6: astore_2
       7: lconst_0
       8: invokestatic  #17                 // Method clojure/lang/RT.intCast:(J)I
      11: aconst_null
      12: invokestatic  #21                 // Method clojure/lang/RT.nth:(Ljava/lang/Object;ILjava/lang/Object;)Ljava/lang/Object;
      15: astore_3
      16: aload_0
      17: lconst_0
      18: invokestatic  #27                 // Method clojure/lang/Numbers.lte:(Ljava/lang/Object;J)Z
      21: ifeq          31
      24: getstatic     #31                 // Field const__3:Ljava/lang/Object;
      27: goto          48
      30: athrow
      31: aload_0
      32: aconst_null
      33: astore_0
      34: invokestatic  #35                 // Method clojure/lang/Numbers.dec:(Ljava/lang/Object;)Ljava/lang/Number;
      37: aload_3
      38: aconst_null
      39: astore_3
      40: invokestatic  #41                 // Method clojure/lang/Tuple.create:(Ljava/lang/Object;)Lclojure/lang/IPersistentVector;
      43: astore_1
      44: astore_0
      45: goto          0
      48: areturn

  public java.lang.Object doInvoke(java.lang.Object, java.lang.Object);
    Code:
       0: aload_1
       1: aconst_null
       2: astore_1
       3: aload_2
       4: checkcast     #54                 // class clojure/lang/ISeq
       7: aconst_null
       8: astore_2
       9: invokestatic  #56                 // Method invokeStatic:(Ljava/lang/Object;Lclojure/lang/ISeq;)Ljava/lang/Object;
      12: areturn

  public int getRequiredArity();
    Code:
       0: iconst_1
       1: ireturn

  public static {};
    Code:
       0: ldc2_w        #60                 // long 7l
       3: invokestatic  #67                 // Method java/lang/Long.valueOf:(J)Ljava/lang/Long;
       6: putstatic     #31                 // Field const__3:Ljava/lang/Object;
       9: return
}

Alex Miller (Clojure team)19:11:42

I don't know what the question is, but there's the bytecode :)

1
dmiller19:11:05

Thanks. I'm too tired to hit shift apparently. in invokeStatic, instruction 40, a call is made to Tuple.create(Object), returning an IPersistentVector. This is then stored in a local of type ISeq. These types are not compatible. How is this possible? (In ClojureCLR, I have to insert a coercion to ISeq to get the IL to compile. Then I get an exception at runtime about not being able to convert a PersistentVector to an ISeq.)

hiredman19:11:53

the jvm spec talks about locals having types as either a primitive or just the type "reference", so the jvm might not actually care about the type of the local other than that it is not primitive

Alex Miller (Clojure team)19:11:44

I believe that's correct here

hiredman19:11:21

the spec about class file verification is a ton of prolog but contains sentence "For assignments, interfaces are treated like Object." as well

dmiller19:11:40

Well that's not fair. The code works because the JVM is sloppy? I have no idea how to make this work on the CLR. If it is valid Clojure, than I have to compile it somehow.

dmiller19:11:30

Can someone quickly think of a Clojure function that takes an ISeq but chokes on a PersistentVector?

Alex Miller (Clojure team)19:11:34

most functions that work on seqs coerce via seq

dmiller20:11:40

Yeah, that's why I'm having trouble thinking of one. Trying to find a way to catch this code in a trap.

Alex Miller (Clojure team)20:11:43

I'm trying to think of this would be "corrected" and I guess it's kind of missing the checkcast to ISeq you see in doInvoke() around this (and the construction of an ArraySeq for the restargs)

dmiller20:11:54

If I call out to something in RT that takes an ISeq, it can be caught:

(defn yikes-again [i & [opts :as all]]
  (if (<= i 0)
     (clojure.lang.RT/seqToArray all)
	(recur (dec i) [opts])))
This fails at runtime.

Alex Miller (Clojure team)20:11:40

a call from the outside would have made an ArraySeq, but the recur is not doing that. but the main thing that happens on those varargs is nth, which works on either

dmiller20:11:35

Yes, but if you were to pass to anything taking an ISeq, it fails. I'm not sure that is correct behavior.

Alex Miller (Clojure team)20:11:12

I'll see if Rich has any thoughts

Alex Miller (Clojure team)21:11:57

I'll just summarize to say that Rich was unconcerned :) I think it would not be wrong to seqify that restargs on the recur though.

dmiller23:11:26

I do have to do something. What this code demonstrates actually occurs in tools.analyzer.passes/schedule. In codegen, easy enough to substitute a call to RT.seq for the typecast. I just need to figure out how tightly I can constrain the circumstances. Thanks for your quick response.

dmiller17:11:34

A few lines of code. The call to RT.seq() replaces the cast in fifteen places in core.clj &co. (I don't have a good way to determine those places are fine with the cast.) Just for the record, I will note that the signature of tools.analyzer.passes/schedule is a bit odd, given how the parameters are used.

(defn schedule [passes & [opts]] ... (recur v [opts]) ...)
opts is checked for being equal to :debug and that is only after the iteration is complete. So we allow an arbitrary number of optional arguments, but only ever look at the first one, and only care if it is equal to :debug. If there were other arguments supplied, they are lost after the first iteration. Not that this function cares. Having a one-arg and a two-arg version would work (second arg a bool). Even if you wanted to allow for more options in the future, this code would lose them. Plus, you do an unnecessary function call and allocation on every iteration. This could be avoided (and I would never has spent five hours tracking this down) if one wrote
(defn schedule [passes & [opt1 :as all]] ... (recur v all) ... )
Then again, I'm not very expert at writing Clojure code. I was just thinking about this style of coding. I'm not suggesting this needs changing. I write this only because apparently I have not yet spent enough time on this problem. :)

Alex Miller (Clojure team)18:11:00

I think the whole intent here is to have either a 1-arg function or a two arg with an option map, so perhaps doing that directly would be better :)

dmiller18:11:39

I really was just trying to figure out the intent. I really don't spend much time coding Clojure, just reading other people's code. (I hope my previous remarks did not seem snarky.) Over and out.

Alex Miller (Clojure team)18:11:16

this style of api is also affected by the new "trailing map for varargs" support in 1.11 where you could actually using varargs and still take the opts map directly if desired (so you don't need to build the [opts] at all here)