Skip to content

Commit 3b87270

Browse files
committed
Update to jank-test-suite-initial-run-results.md
1 parent 0a95191 commit 3b87270

File tree

1 file changed

+24
-2
lines changed

1 file changed

+24
-2
lines changed

docs/jank-test-suite-initial-run-results.md

Lines changed: 24 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -480,18 +480,40 @@ IMHO
480480

481481
## Some true breakage
482482

483+
### aset
484+
483485
My initial run of the tests indicate two places where ClojureCLR has non-trivial bugs.
484486
One is in the test for `aclone`. I have done a little investigation and can reproduce the issue with a small piece of code.
485487

486488
```clojure
487-
`(defn bad-clone [a]
489+
(defn bad-clone [a]
488490
(aset a 0 1)
489491
(let [a' (aclone a)]
490492
(aset a' 0 2)
491493
a'))
492494
```
493495

494-
The second call to `aset` throws an exception. This may be a problem with dynamic callsites. THis will not be fun.
496+
Consider calling `(bad-clone (int-array 3))`.
497+
The first call to `aset` goes fine. It resolves dynamically at run-time (no type information on `a` to avoid reflection = dynamic callsite) to a call to `RT.aset(int[],int,int)`. That works.
498+
499+
The second call to `aset` blows up with an exception relating to widening types. Actually it's a problem with narrowing. Because the `aclone` calls inlines to a call to `Array RT.aclone(Array)`, we have an `Array` type hint on `a'`. So the call to `aset` resolves to `RT.aset(Array,object,int)`. This method calls `Array.SetValue`. And that call chokes because the value being inserted into the array is an `Int64` and we have an array of `Int32`!
500+
501+
If instead we define
502+
503+
```clojure
504+
(def ok-clone [a]
505+
(aset a 0 1)
506+
(let [a' (aclone a)]
507+
(aset a' 0 (int 2))
508+
a'))
509+
```
510+
511+
the call `(ok-clone (int-array 3))` works fine.
512+
513+
I'm not sure what the best solution is here. Forcing the user to know that the conversion is necessary in one call to `aset` but not the other strikes me as bad. The easiest solution is change `RT.acline` to have return type `Object` instead of `Array`. We get a dynamic callsite in the second `aset` call, but that's probably better than the reflection that takes place in `Array.SetValue`.
514+
515+
516+
### case
495517

496518
The other problem area is with `case`. It should be noted that `case` passes all the tests that are in the test suite in the Clojure repo. There is something about the specific cases in these tests that cause `case` to fail. This will not be fun.
497519

0 commit comments

Comments
 (0)