janet-lang / janet

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

Consider making `module/paths` a dynamic variable #1434

Closed afranchuk closed 1 month ago

afranchuk commented 3 months ago

I'm trying to run some code that has a full root-env but has a different set of module paths than the main program. Right now I don't think this is possible without redefining all module-loading-related functions (e.g. require, use, etc) because any changes to module/paths will affect both the main program environment and the "isolated" version of the environment (the alternate environment to use with specific loaded code, in my case). For that matter, you could probably make the same argument for module/loaders/module/cache being dynamic variables, too.

If I'm making some poor assumption and there is a way to do this, let me know! I'm still pretty new to Janet.

Also I realize this might be a bit more complicated than it seems, because e.g. making them dynamic variables would interact poorly with fibers by default (as far as I understand).

pepe commented 2 months ago

I may be mistaken, but as dynamics are part of the environment your idea doesn't make sense to me. May you please elaborate your use case?

afranchuk commented 2 months ago

Yeah, let me be more specific.

I'm writing a program in janet which loads/evaluates other janet scripts (it uses janet as a scripting language behind another programming language). So the main interpreter program needs use/require/import/etc to work as usual. However when evaluating the scripts which underpin the language being implemented, I want a different module/paths to be defined. My problem is that module/paths is a table in root-env, so I don't know how to change it only for the environment that's used when evaluating the scripts; I don't want it to be different for the main interpreter program.

bakpakin commented 2 months ago

I actually did an experiment where module/paths, module/loaders, etc. were only defaults for dynamic bindings, and never merged in since nobody every asked for it and it seemed to make things a little more confusing. The code is simple, add some dynamic bindings, and replace all uses of module/paths in boot.janet with (dyn *module-paths* module/paths)

I could see possible issues here, but I think it should generally work

afranchuk commented 2 months ago

@bakpakin On a related note though (and maybe this should be a separate issue?), I was a little surprised that dynamic bindings aren't propagated through imports. It makes sense that e.g. the current environment isn't (so that each file has a consistent base environment when loaded, not based on the load location), however the nature/purpose of dynamic bindings made me think I could do something like

# main.janet
(setdyn :foo 'bar)
(import ./other)
(assert (= other/foo 'bar))

# other.janet
(def foo (dyn :foo))

I think this is related to this issue since, if I set the *module/paths* dynamic binding, I would expect it to work on imports within files I import, too. Though I could definitely see an argument for doing something along those lines with a custom loader instead.

bakpakin commented 1 month ago

I was a little surprised that dynamic bindings aren't propagated through imports

This now works on the latest master

@afranchuk a bit belated, but does this work for you?

(with-dyns [*module/cache* @{}
            *module/loading* @{}]
            (import ./other))

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.

bakpakin commented 1 month ago

This should be fixed as of fdaf2e1594bb3b68a9e7c179388ac8afa9025204 (since revised to *module-make-env*). I've verified that this can solve the initial issue (albeit with a bit of work) by bypassing the usual flow of (make-env) to create new environments for files when doing import, require, or dofile.

afranchuk commented 1 month ago

@bakpakin thanks, that should work great! FWIW your original change wasn't quite what I expected, as I was thinking that only dynamic bindings would be present in imports (not normal bindings) by their nature. But with *module/make-env* people can choose to implement that or other behaviors, so I think it's a good compromise.