rkoeninger / ShenSharp

Shen for the Common Language Runtime
BSD 3-Clause "New" or "Revised" License
33 stars 2 forks source link

Run the 39.0 kernel on .NET 8.0. #9

Open jaccarmac opened 10 months ago

jaccarmac commented 10 months ago

Cleanup remains:

jaccarmac commented 5 months ago

Bugs remain, but a REPL can start based on the GitHub source distribution. It does require some patches not yet documented. The KL->F# compiler does not handle forward or duplicate definitions.

There are a few leftover symbols which create lambda forms (see https://groups.google.com/g/qilang/c/2ep0cbcDe44/m/DKgwYGwaBgAJ). Add the following lines to the top of init.kl.

(defun shen.interror (X Y) skip)
(defun foreign (X) skip)
(defun list (X) skip)
(defun shen.pprint (X) skip)

There are duplicate definitions of internal-to-shen? and semicolon?. Delete the KL equivalents to the following.

https://github.com/Shen-Language/shen-sources/blob/shen-38.2/sources/reader.shen#L559

https://github.com/Shen-Language/shen-sources/blob/shen-38.2/sources/yacc.shen#L74

The test suite fails during the prime suite, which relies on mutual definitions of even and odd functions. There is similarly bad behavior in the REPL.

(0-) (define even*? 1 -> false X -> (odd*? (- X 1)))
(fn even*?)

(1-) (define odd*? 1 -> true X -> (even*? (- X 1)))
(fn odd*?)

(2-) (even*? 1)
false

(3-) (even*? 2)
fn: odd*? is undefined

(4-) (odd*? 1)
fn: odd*? is undefined

(5-) (exit 0)
fn: exit is undefined

(6-) %

The weird behavior of exit is here to note; No root cause yet.

jaccarmac commented 5 months ago

It may not be mutuality, though. There seems to be divergent behavior based on very specific names of functions.

I can define even*? or odd or odd? but not odd* or odd*?.

jaccarmac commented 5 months ago

The mask of list from above is still necessary, but the other symbols are fixed in this release.

jaccarmac commented 5 months ago

I've been using a block something like the below to debug runRepl (right after newRuntime).

let ast = kl_read globals [pipeString "(define odd*? X -> true)"]
eval globals ast |> printfn "%A"
``kl_shen.shen->kl`` globals [
``kl_shen.process-applications`` globals [kl_macroexpand globals [ast]
                                          ``kl_shen.find-types`` globals [ast]]
]
|> printfn "%A"
evalSyntax globals "(clr.int 1)" |> printfn "%A"
[kl_read globals [pipeString "(clr.int 1)"]] |> kl_eval globals |> printfn "%A"

That's basically https://github.com/Shen-Language/shen-sources/blob/shen-38.3/sources/sys.shen#L11 ported to F#. The clr.int test is a leftover, led to the last commit, and revealed our handling of lambda-form attributes is lacking. I'm not sure where the breakage for odd*?/even*? is yet.

jaccarmac commented 5 months ago

I found another opportunity for a refactor. The custom range loc no longer seems necessary. Range has Zero and range0, and each of the trivia types has a Zero static member as well. The comment mentions an ArrayIndexOutOfBoundsException but I have not noticed anything yet.

jaccarmac commented 5 months ago

Prolog is currently broken and I have a small reproduction case. The following returns Var3 but should return true.

(0-) (defprolog simple X X <--;)
(fn simple)

(1-) (prolog? (simple 1 X))
Var3

I generated the code for that prolog? invocation.

(2-) (head (read-from-string "(prolog? (simple 1 X))"))
[[[[[lambda V34 [lambda L35 [lambda K36 [lambda C37 [let X [shen.newpv V34] [shen.gc V34 [do [shen.incinfs] [simple 1 X V34 L35 K36 C37]]]]]]]] [shen.prolog-vector]] [@v true [@v 0 [vector 0]]]] 0] [freeze true]]

In a friendlier form (pretty and with short names), that's roughly this, which dutifully returns the incorrect value.

(((((lambda A (lambda B (lambda C (lambda D
      (let X (shen.newpv A)
           (shen.gc A (do (shen.incinfs) (simple 1 X A B C D))))))))
    (shen.prolog-vector)) (@v true (@v 0 (vector 0)))) 0) (freeze true))

However, tweaking the nested lambdas to be either a let or a multi-parameter function returns the correct result.

(4-) (let A (shen.prolog-vector)
     B (@v true (@v 0 (vector 0)))
     C 0
     D (freeze true)
     X (shen.newpv A)
     (shen.gc A (do (shen.incinfs) (simple 1 X A B C D))))
true

(5-) (define wrapper
  A B C D -> (let X (shen.newpv A)
                  (shen.gc A (do (shen.incinfs) (simple 1 X A B C D)))))
(fn wrapper)

(6-) (wrapper (shen.prolog-vector) (@v true (@v 0 (vector 0))) 0 (freeze true))
true
jaccarmac commented 5 months ago
((/. X (let Y 1 (+ X Y))) 1)

evaluates to 1, when it should be 2.

A let inside lambda appears to return the first bound value without evaluating the remaining bindings or body.

tizoc commented 5 months ago

@jaccarmac just to discard the most basic case, what do you get from this:

(0-) ((+ 1) 2)
3
jaccarmac commented 5 months ago

3

jaccarmac commented 5 months ago

There's an unrelated test failure in the Shen test suite. As symbol? is implemented in the source, https://github.com/jaccarmac/ShenSharp/blob/71592824a318fd31b30a6b2da4d804880bed6f70/src/Shen.Tests/Symbols.fs#L37 should in fact fail as it is not alphanumeric. However, we can successfully intern it, as can Shen/Scheme, and Shen/Scheme overrides analyse-symbol? to use the function from Scheme itself.

jaccarmac commented 5 months ago
passed ... 129
failed ... 5
pass rate ... 96.2686567164179104%

ok
0

run time: 239.9877038 secs
loaded

run time: 240.338632 secs

00:04:00.3456812
jaccarmac commented 5 months ago
passed ... 134
failed ... 0
pass rate ... 100%

ok
0

run time: 212.4933304 secs
loaded

run time: 212.7807741 secs

00:03:32.7836744
rkoeninger commented 5 days ago

@jaccarmac Sorry I haven't looked at this in a long time. Excellent work here, I admire your dedication.

Could you merge in my changes to the master branch and push? That will trigger the new build.

jaccarmac commented 5 days ago

No worries, it was a fun start-of-unemployment project that I keep meaning and failing to come back to. This is the sign! The merge fired off perfectly, but I will take a closer look tomorrow.

jaccarmac commented 4 days ago

Ah, that's right, my changes to shen-sources did not make it to a release tarball, and I never tried when the S39 changes percolated to there. @tizoc, I see that the removed arities were added back as comments in Shen-Language/shen-sources@e943e63cee79527189c8d6d0a19ee298c7d24d82 and added fully in Shen-Language/shen-sources@319baf2c728ba79e03120db1a598c1ac6bfe64a3. Are they necessary for something in S39?

tizoc commented 4 days ago

@jaccarmac arity was added back but as you can see in that commit those two symbols were special-cased so that the init function doesn't create lambdas for them, which solves the issue you had back then. As for them being necessary, , reason to add them back was probably to keep things as close as the official release but I don't remember now.

jaccarmac commented 4 days ago

I skipped right over that diff; Mea culpa and thanks for the clarification.

jaccarmac commented 20 hours ago

https://github.com/rkoeninger/ShenSharp/pull/9#issuecomment-2111503953 is biting; I'm not sure how to fix it as no fix was necessary before...

Since I did not leave myself a code trail in May, I'm not even confident about what changed. It might be the new kernel's preference for partial application? (type) of anything shows up as partial application now. I suppose I need to either add an override or change that test to reflect the Shen sources' behavior.

jaccarmac commented 19 hours ago

I suspect I just left that test to fail as it does not affect the Shen test suite. I updated it to pass CI and match the SBCL implementation, as detailed in that commit message.