In Lua (5.2+), when a variable named _ENV is present, all attempts to read globals are redirected to reads in the _ENV table. This means that the allowed-globals list isn't relevant, and shouldn't be tested.
This change will enable fennel users to more easily write confusing and bad fennel code. For example:
(local my-html-dsl (require :my-html-dsl))
(my-html-dsl
(fn [_ENV]
(html
(body
(h1 "my document")
(p "this is my document")))))
I don't think we can always guarantee that an "_ENV" sym always demangles to "_ENV"
I think this one is pretty straightforward? I just pushed a fix in ff9fd5f. Curious what you think still needs design.
Oh, maybe "needs design" is about the mangling issue? I would rather treat that as a separate bug.
The needs-design tag is about
- the mangling issue. We don't support nested _ENV because you sometimes get the mangling
_ENV0
instead.- I want to think about what to do with
__fnl_global_foo_2dbar
style manglings. Should we emit_ENV["foo-bar"]
instead?Design issues aside, I'm not sure if we want to take on support of _ENV or not. To me, _ENV is like
global
andrequire-macros
in that it (sort of) breaks the rules of lexical scoping and makes code harder to understand. Fennel already asks users to use_G.field
for accessing globals. It seems backward to let users skip_ENV.field
when the behavior is more confusing than regular globals.
Hm... I think if we required people to explicitly call
_ENV.field
then it defeats the purpose of_ENV
; you might as well just let-bind a regular table at that point.Thinking it thru more, the problem with this change is that
_ENV
uses dynamic scope and as such it is incompatible with the entire mechanism of strict globals checking. This patch is only a half-measure because it disables strict globals checking when_ENV
is lexically bound, which isn't how_ENV
actually works! It gives the impression of supporting it while in reality only supporting a limited subset of it, kind of like how operators support multivals but only when the final arg is avalues
form. (and we all know how THAT goes)With that in mind, maybe it's better to say that because it's impossible to statically determine which code will run under the "real"
_G
and which will run under a modified_ENV
that you need to set--globals
to a union of both. In that case, it's probably better to revert my change.
The mangling issues remain legitimate problems we need to fix in the compiler.
- We should never mangle
-ENV
to_ENV
- We should never mangle
_ENV
to anything elseThese seem pretty clear to me.
I want to think about what to do with __fnl_global_foo_2dbar style manglings. Should we emit _ENV["foo-bar"] instead?
I think implementing global mangling in the first place was a mistake, but I'm concerned that it's now impossible to improve without introducing a breaking change. Because
_ENV
is dynamically scoped, emitting_ENV["foo-bar"]
isn't really viable because we can't know where it needs to be done. Ideally we would have emitted_G["foo-bar"]
from the beginning probably. Maybe we can fix that in 2.0?
for lua 5.2+, _ENV["foo-bar"] always does what we want, but it would cause problems in lua5.1/luajit.
OK I was getting confused between
_ENV
andsetfenv
because I mostly only use the latter. Turns out_ENV
isn't dynamically scoped at all and the comparison to_G
is apt in terms of how it actually functions, tho the intent is different.To me, _ENV is like global and require-macros in that it (sort of) breaks the rules of lexical scoping and makes code harder to understand. Fennel already asks users to use _G.field for accessing globals.
It really depends; you can use it to expand the set of globals, in which case I agree it makes code harder to understand, or you can use it to reduce the set of globals, in which case it makes the code easier to understand. (when capabilities are passed on the stack) But this change of skipping global checks when
_ENV
is in scope really only relevant in the case of expanding globals, so yeah... probably not worth encouraging? I think I will revert it.It would be really cool if we could detect a table literal being bound to
_ENV
and temporarily resetallowedGlobals
based on the keys it has. But that's probably more trouble than it's worth.