Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The 'bel' function fails on many inputs #5

Open
masak opened this issue Aug 21, 2020 · 6 comments
Open

The 'bel' function fails on many inputs #5

masak opened this issue Aug 21, 2020 · 6 comments

Comments

@masak
Copy link

masak commented Aug 21, 2020

Using the Windows executable of the 0.4.0 release:

$ ./chime.exe
> (bel '(+ 2 2))
overargs

Expecting the above to work fine and return 4.

At a guess, something is off because + is implemented natively? That's just a guess, though.

@masak masak changed the title Calling Calling 'bel' with '+' fails Aug 21, 2020
@jeremyschlatter
Copy link
Owner

jeremyschlatter commented Aug 24, 2020

Thanks for the report!

The 'bel' macro function fails on a wide range of inputs.

There are at least two things that need to be fixed here:

  1. Fix the definition of bquote in bel.bel so that it is not infinitely recursive (it doesn't directly call "bquote", but it does use backquotes, which has the same effect).
  2. Optimize chime's expression evaluation so the 'bel' macro function doesn't run intolerably slowly.

Alternatively, the 'bel' macro function could be implemented natively. This would actually be pretty straightforward -- basically just hook it up to chime's internal evaluate function -- but I don't want to do this yet, because the userland version defined in bel.bel serves as a nice test case.

(Also, incidentally, glad to hear that the Windows executable is working for you at all 😅. I have done very little testing on Windows.)

@jeremyschlatter jeremyschlatter changed the title Calling 'bel' with '+' fails The 'bel' macro fails on many inputs Aug 24, 2020
@masak
Copy link
Author

masak commented Aug 24, 2020

  1. Fix the definition of bquote in bel.bel so that it is not infinitely recursive (it doesn't directly call "bquote", but it does use backquotes, which has the same effect).

Oh, you noticed that one too! I walked right into that one in my implementation, all the way up to where it just started hanging. (Stalled PR here: masak/bel#175 )

I'm curious what happened with that one. pg has done a good job avoiding that kind of non-well-founded recursion in the language definition. I wrote to him to ask, but I haven't received a reply. My best personal guess is that bquote evolved during the design of the language from being a part of the reader (which is a common design in Lisps) to being a regular macro. As part of the change, the circularity wasn't noticed.

the 'bel' macro

A little bit confused at it being described as a macro. At least in bel.bel and in chime.exe 0.4.0, it's a function:

> (function bel)
clo

But maybe you mean something in the implementation that I don't get yet.

Alternatively, the 'bel' macro could be implemented natively. This would actually be pretty straightforward -- basically just hook it up to chime's internal evaluate function -- but I don't want to do this yet, because the userland version defined in bel.bel serves as a nice test case.

Aye. I'll probably get back to this point, because there be dragons there, and I'm not even in a position where I can express them well yet. To a first approximation, "There should be no observable difference between evaluating expression χ and evaluating (bel 'χ)" seems both excessively sane, and a potential source of lots and lots of "fun" corner cases. The thought has occurred to me to let loose a property tester on that property; should help find some things.

(Also, incidentally, glad to hear that the Windows executable is working for you at all 😅. I have done very little testing on Windows.)

It works, but I have one or two separate issues to submit about things that could be improved. (Ctrl+C/eof behaving differently, and ANSI colors showing up as mojibake in some configurations. Stay tuned.)

@jeremyschlatter jeremyschlatter changed the title The 'bel' macro fails on many inputs The 'bel' function fails on many inputs Aug 24, 2020
@jeremyschlatter
Copy link
Owner

A little bit confused at it being described as a macro.

Whoops, that was just a mistake on my part. Agreed that it's a function.

@jeremyschlatter
Copy link
Owner

re: recursive bquote, here's a non-recursive version. I think it works, but haven't tested it extensively yet.

diff --git a/reference/bel.bel b/reference/bel.bel
index 3ab4784..6fc9036 100644
--- a/reference/bel.bel
+++ b/reference/bel.bel
@@ -1316,56 +1316,57 @@
       (or (parsenum cs base) (sym cs))))
 
 (mac bquote (e)
-  (let (sub change) (bqex e nil)
-    (if change sub (list 'quote e))))
+  ((list 'lit 'clo scope '((sub change))
+     '(if change sub (list 'quote e)))
+   (bqex e nil)))
 
 (def bqex (e n)
   (if (no e)   (list nil nil)
       (atom e) (list (list 'quote e) nil)
-               (case (car e)
-                 bquote   (bqthru e (list n) 'bquote)
-                 comma    (if (no n)
-                              (list (cadr e) t)
-                              (bqthru e (car n) 'comma))
-                 comma-at (if (no n)
-                              (list (list 'splice (cadr e)) t)
-                              (bqthru e (car n) 'comma-at))
-                          (bqexpair e n))))
+               (if (id (car e) 'bquote)   (bqthru e (list n) 'bquote)
+                   (id (car e) 'comma)    (if (no n)
+                                              (list (cadr e) t)
+                                              (bqthru e (car n) 'comma))
+                   (id (car e) 'comma-at) (if (no n)
+                                              (list (list 'splice (cadr e)) t)
+                                              (bqthru e (car n) 'comma-at))
+                                          (bqexpair e n))))
 
 (def bqthru (e n op)
-  (let (sub change) (bqex (cadr e) n)
-    (if change
-        (list (if (caris sub 'splice)
-                  `(cons ',op ,(cadr sub))
-                  `(list ',op ,sub))
-              t)
-        (list (list 'quote e) nil))))
+  ((list 'lit 'clo scope '((sub change))
+     '(if change
+          (list (if (caris sub 'splice id)
+                    (list 'cons (list 'quote op) (cadr sub))
+                    (list 'list (list 'quote op) sub))
+                t)
+          (list (list 'quote e) nil)))
+   (bqex (cadr e) n)))
 
 (def bqexpair (e n)
-  (with ((a achange) (bqex (car e) n)
-         (d dchange) (bqex (cdr e) n))
-    (if (or achange dchange)
-        (list (if (caris d 'splice)
-                  (if (caris a 'splice)
-                      `(apply append (spa ,(cadr a)) (spd ,(cadr d)))
-                      `(apply cons ,a (spd ,(cadr d))))
-                  (caris a 'splice)
-                  `(append (spa ,(cadr a)) ,d)
-                  `(cons ,a ,d))
-              t)
-        (list (list 'quote e) nil))))
+  ((list 'lit 'clo scope '((a achange) (d dchange))
+     '(if (if achange t dchange)
+          (list (if (caris d 'splice id)
+                    (if (caris a 'splice id)
+                        (list 'apply 'append (list 'spa (cadr a)) (list 'spd (cadr d)))
+                        (list 'apply 'cons a (list 'spd (cadr d))))
+                    (caris a 'splice id)
+                    (list 'append (list 'spa (cadr a)) d)
+                    (list 'cons a d))
+                t)
+          (list (list 'quote e) nil)))
+   (bqex (car e) n)
+   (bqex (cdr e) n)))
 
 (def spa (x)
-  (if (and x (atom x))
+  (if (if x (atom x))
       (err 'splice-atom)
       x))
 
 (def spd (x)
-  (pcase x
-    no   (err 'splice-empty-cdr)
-    atom (err 'splice-atom)
-    cdr  (err 'splice-multiple-cdrs)
-         x))
+  (if (no x)   (err 'splice-empty-cdr)
+      (atom x) (err 'splice-atom)
+      (cdr x)  (err 'splice-multiple-cdrs)
+               x))
 
 (mac comma args
   '(err 'comma-outside-backquote))

@masak
Copy link
Author

masak commented Aug 24, 2020

Thanks! My solution thus far was more complicated — I will try yours in my implementation and report back.

Relatedly, however, I think I will need to make an "errata" list of places where the implementation is deliberately deviating from the spec, with clear reasoning as to why. (See also #9.) I see you did something similar with tabref — I will need to look more closely at why; haven't hit that one yet. Anyway, I feel collecting/documenting all the deviations-from-spec in one list is a good idea.

@masak
Copy link
Author

masak commented Aug 27, 2020

There should be no observable difference between evaluating expression χ and evaluating (bel 'χ)

This lodestar (together with things like #11), I think it leads to the REPL using bel outright as its evaluator. There still needs to be a host evaluator interpreting it, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants