janet-lang / janet

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

recent change kinda breaks lexical scope with multiple modules #1447

Closed ianthehenry closed 1 month ago

ianthehenry commented 1 month ago

I think that this commit has some really confusing unintended consequences:

https://github.com/janet-lang/janet/commit/0350834cd3dab0491d79e90c473f678959bd97ce

As a completely contrived example:

# one.janet
(def + -)
(import ./two)
(print two/export)
# two.janet
(def export (+ 100 50))

Before that commit, this printed 150. Afterwards, it prints 50.

I definitely assume that I'm starting from the vanilla root-env when I make a new file, and even though I usually import/use at the top of a file this makes the order of those imports really significant.

As a less fake example, I have a package called pat that shadows the built-in match, but if I (use pat) and then import something else, I don't know if it will succeed or fail with pat's semantics.

sogaiu commented 1 month ago

FWIW, bakpakin made the following comment here:

EDIT: As a work around for all recent Janet versions, you can also do (import ./other :env (make-env (curenv))). The default was that import, require, and dofile start with a pristine environment, including dynamic bindings when imported. We might want to go back to that.

I didn't quite follow, but I got the sense he thought some kind of reversion might be in order.

bakpakin commented 1 month ago

This change has been reverted and replaced with a better tool *module-make-env* which is used to dynamically construct the default environment when using require, import, and dofile.

On latest master, new usage would be like so:

main.janet


(def new-env (make-env))
(put new-env *module-cache* @{})
(put new-env *module-loading* @{})
(put new-env *module-make-env*
  (fn get-new-derived-env [&]
    (def e (make-env new-env))
    (put e :extra-stuff "hello, world")
    e))
(dofile "./my-custom-plugin.janet" :env new-env)

my-custom-plugin.janet

(import ./lib) # lib will be loaded with environment table created by get-new-derived-env

lib.janet

(print (dyn :extra-stuff)) # prints "hello, world"