janet-lang / janet

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

Move idempotent to corelib #1195

Closed primo-ppcg closed 1 year ago

primo-ppcg commented 1 year ago

Related issue: #1192

Moves idempotent? from boot.janet to corelib.c. This is done so that var bindings and def bindings can be distinguished within macros.

(defmacro macro-idempotent
  [x]
  (idempotent? x))

(var var-x 1)
(assert (= (macro-idempotent var-x) false) "macro idempotent var")

(def def-x 1)
(assert (= (macro-idempotent def-x) true) "macro idempotent def")
sogaiu commented 1 year ago

I tried it out a bit.

The tests I mentioned here all passed at least :)

ianthehenry commented 1 year ago

This doesn't quite do what you want, because you're only looking at the symbol in the fiber's environment, which is not necessarily the scope that the macro will be expanded in. Consider:

Janet 1.28.0-dev-2e2d792e macos/aarch64/clang - '(doc)' for help
repl:1:> (idempotent? 'x)
true
repl:2:> (var x 1)
1
repl:3:> (idempotent? 'x)
false

So far so good, because x is an entry in the environment table. But when you make non-top-level bindings:

repl:4:> (let [x 1] (idempotent? 'x))
false
repl:5:> (do (var y 1) (idempotent? 'y))
true

Those don't live in the fiber's environment, and you get unexpected results.

I don't think there's a way to write the thing that you want in general. idempotent? is not really a function of a symbol by itself, it's a function of a symbol and a scope. But macros aren't aware of the lexical environment into which they're being expanded -- Janet doesn't have "first-class scopes," apart from the fiber environment, which is its own special thing.

primo-ppcg commented 1 year ago

Thanks for taking the time to review.

I think this optimization would need to happen in the compiler itself (remove def bindings of def bindings).