janet-lang / janet

A dynamic language and bytecode vm
https://janet-lang.org
MIT License
3.38k stars 217 forks source link

macex raises on () #1341

Closed ianthehenry closed 6 months ago

ianthehenry commented 6 months ago

This is valid code:

Janet 1.32.1-cc5beda0 macos/aarch64/clang - '(doc)' for help
repl:1:> (if true () ())
()

But trying to macro-expand it raises:

repl:2:> (macex '(if true () ()))
error: expected integer key for tuple in range [0, 0), got 0
  in dotup [boot.janet] on line 2126, column 12
  in macex1 [boot.janet] (tailcall) on line 2140, column 16
  in map [boot.janet] on line 998, column 3
  in expandall [boot.janet] (tailcall) on line 2083, column 15
  in macex1 [boot.janet] on line 2140, column 16
  in macex [boot.janet] on line 2237, column 16
  in _thunk [repl] (tailcall) on line 2, column 1

(I wrote a macro that calls macex on one of its arguments, and it worked fine for a while, but then I passed it an expression containing () and it raised.)

sogaiu commented 6 months ago

Looks like in the most recent janet (11d7af3f9540bf2d4ae8a45ccc1e2e0348805937), the dotup bit is this line.

iacore commented 6 months ago

(discussed before) this very footgun-ish maybe we make () invalid instead of empty list

sogaiu commented 6 months ago

Are you confident that that will have minimal to no effect on existing code?

sogaiu commented 6 months ago

I don't suppose an appropriate fix is as simple as:

diff --git a/src/boot/boot.janet b/src/boot/boot.janet
index 9c5a2bd9..354f0f14 100644
--- a/src/boot/boot.janet
+++ b/src/boot/boot.janet
@@ -2141,7 +2141,9 @@
     (case (type x)
       :tuple (if (= (tuple/type x) :brackets)
                (tuple/brackets ;(map recur x))
-               (dotup x))
+               (if (next x)
+                 (dotup x)
+                 ()))
       :array (map recur x)
       :struct (table/to-struct (dotable x recur))
       :table (dotable x recur)
$ ./build/janet
Janet 1.32.1-11d7af3f linux/x64/gcc - '(doc)' for help
repl:1:> (macex '(if true () ()))
(if true () ())
pepe commented 6 months ago

Something like that, I wanted to propose. :-).

iacore commented 6 months ago

Are you confident that that will have minimal to no effect on existing code?

very not confident

the thing is, the inconsistency that () is not a "function call" will probably end up being discovered again.

there's a way to add warnings to bare () (suggest to switch to [])

sogaiu commented 6 months ago

the inconsistency that () is not a "function call"

I think it depends on how one defines things.

There is at least one other Lisp-like that has a similar arrangement:

An empty list () evaluates to an empty list.

Non-empty Lists are considered calls to either special forms, macros, or functions. A call has the form (operator operands*).

via: https://clojure.org/reference/evaluation

By this definition, () isn't a call and at least when looking directly at such a definition, there doesn't appear to be an outright inconsistency to me.

iacore commented 6 months ago

It's inconsistent because without any prior knowledge to lisp-like languages, one would deduct () (a) (a b) (a b c) to behave similarly. That's also the simplest way to write code to handle lispy expressions. In this case (macex) the inconsistency caused the mistake to happen.

People by default don't know, so I argue to make a language self-consistent rather than follow convention (requires knowledge).