From c19356ffc19af61b98d496a29376ebf42b2b8be7 Mon Sep 17 00:00:00 2001 From: David Bernheisel Date: Mon, 19 Oct 2026 13:01:52 -0400 Subject: [PATCH] fix(compiler): capture _ENV only in closures that use a global Every nested function took _ENV as upvalue 1, even when it referenced no global. debug.getupvalue and debug.setupvalue indices were therefore off by one against PUC-Lua, and debug.setupvalue(f, 1, v) on a closure over a local overwrote _ENV instead of that local. Scope resolution no longer allocates _ENV at function entry. The first free-name reference allocates it through the existing upvalue chain, so numbering follows first use, as in PUC-Lua. --- lib/lua/compiler/instruction.ex | 4 +- lib/lua/compiler/scope.ex | 19 ------ test/language/stdlib/debug_test.exs | 20 ++++-- test/lua/compiler/upvalue_descriptor_test.exs | 66 ++++++++++++++----- test/lua/vm/display_test.exs | 5 +- test/lua/vm/env_semantics_test.exs | 5 +- 6 files changed, 67 insertions(+), 52 deletions(-) diff --git a/lib/lua/compiler/instruction.ex b/lib/lua/compiler/instruction.ex index 4a6decb5..58ab1747 100644 --- a/lib/lua/compiler/instruction.ex +++ b/lib/lua/compiler/instruction.ex @@ -42,8 +42,8 @@ defmodule Lua.Compiler.Instruction do # Field access through an upvalue-held table, fusing a `get_upvalue` with # the `get_field` / `set_field` that consumes it. Every read or write of a - # global is exactly that pair (`_ENV` is an upvalue in every function but - # the chunk), so the fused form halves their instruction count. + # global outside the chunk is exactly that pair (a nested function reaches + # `_ENV` through an upvalue), so the fused form halves their instruction count. # `Lua.Compiler.Peephole` emits these; codegen never does. def get_field_upvalue(dest, index, name, name_hint \\ nil), do: {:get_field_upvalue, dest, index, name, name_hint} diff --git a/lib/lua/compiler/scope.ex b/lib/lua/compiler/scope.ex index 3f1cd131..5baf1fa6 100644 --- a/lib/lua/compiler/scope.ex +++ b/lib/lua/compiler/scope.ex @@ -719,25 +719,6 @@ defmodule Lua.Compiler.Scope do state = %{state | functions: Map.put(state.functions, func_key, func_scope)} - # Every nested function inherits `_ENV` as an upvalue. Resolve it - # eagerly before the body is processed so codegen paths that read - # free names through `_ENV` can rely on the upvalue being present - # with a known index. A parameter or `local _ENV = ...` still takes - # precedence for free-name resolution, which is the correct Lua 5.3 - # behaviour. - state = - if Map.has_key?(state.locals, "_ENV") do - # `_ENV` is a parameter — no upvalue allocation needed at this entry - # point. Inner local rebinds will be handled by normal scope rules. - state - else - case find_upvalue("_ENV", state.parent_scopes, state) do - {:ok, _index, state} -> state - # Should not happen: chunk always defines `_ENV` as a local. - :not_found -> state - end - end - # Resolve the function body state = resolve_block(body, state) diff --git a/test/language/stdlib/debug_test.exs b/test/language/stdlib/debug_test.exs index 75213779..ce75cc15 100644 --- a/test/language/stdlib/debug_test.exs +++ b/test/language/stdlib/debug_test.exs @@ -144,31 +144,37 @@ defmodule Lua.Language.Stdlib.DebugTest do end test "debug.getupvalue names captured upvalues and bounds-checks", %{lua: lua} do - # Every nested function captures `_ENV` as upvalue 1; explicit captures - # follow in declaration order. `debug.getupvalue(_, 3)` is out of range. + # `inner` uses no global, so it has no `_ENV` upvalue and `x` is slot 1. code = """ local x = 10 local function outer() return function() return x end end local inner = outer() - local env_name = debug.getupvalue(inner, 1) - local name, val = debug.getupvalue(inner, 2) - return env_name, name, val, debug.getupvalue(inner, 3) + local name, val = debug.getupvalue(inner, 1) + return name, val, debug.getupvalue(inner, 2) """ - assert {["_ENV", "x", 10, nil], _} = Lua.eval!(lua, code) + assert {["x", 10, nil], _} = Lua.eval!(lua, code) end test "debug.setupvalue mutates an upvalue and returns its name", %{lua: lua} do code = """ local x = 1 local function getx() return x end - local name = debug.setupvalue(getx, 2, 99) + local name = debug.setupvalue(getx, 1, 99) return name, getx() """ assert {["x", 99], _} = Lua.eval!(lua, code) end + test "debug.setupvalue in a later evaluation changes the closure's variable", %{lua: lua} do + # `getc` uses no global, so it has no `_ENV` upvalue and `c` is slot 1. + {_, lua} = Lua.eval!(lua, "local c = 1 function getc() return c end") + assert {["c"], lua} = Lua.eval!(lua, "return debug.setupvalue(getc, 1, 99)") + + assert {[99], _} = Lua.eval!(lua, "return getc()") + end + test "debug.getupvalue returns nil for non-closures", %{lua: lua} do code = """ return debug.getupvalue(print, 1), debug.getupvalue(42, 1) diff --git a/test/lua/compiler/upvalue_descriptor_test.exs b/test/lua/compiler/upvalue_descriptor_test.exs index a70fc1a5..3a930b74 100644 --- a/test/lua/compiler/upvalue_descriptor_test.exs +++ b/test/lua/compiler/upvalue_descriptor_test.exs @@ -26,8 +26,8 @@ defmodule Lua.Compiler.UpvalueDescriptorTest do return f() """) - assert f.upvalue_descriptors == [{:parent_local, 0, "_ENV"}, {:parent_local, 1, "x"}] - assert f.upvalue_names == ["_ENV", "x"] + assert f.upvalue_descriptors == [{:parent_local, 1, "x"}] + assert f.upvalue_names == ["x"] end test "a global referenced many times gets one _ENV slot" do @@ -50,7 +50,7 @@ defmodule Lua.Compiler.UpvalueDescriptorTest do return fib(10) """) - assert fib.upvalue_descriptors == [{:parent_local, 0, "_ENV"}, {:parent_local, 1, "fib"}] + assert fib.upvalue_descriptors == [{:parent_local, 1, "fib"}] end test "a grandparent capture is deduped at every level" do @@ -65,8 +65,8 @@ defmodule Lua.Compiler.UpvalueDescriptorTest do [inner] = outer.prototypes - assert outer.upvalue_descriptors == [{:parent_local, 0, "_ENV"}, {:parent_local, 1, "a"}] - assert inner.upvalue_descriptors == [{:parent_upvalue, 0, "_ENV"}, {:parent_upvalue, 1, "a"}] + assert outer.upvalue_descriptors == [{:parent_local, 1, "a"}] + assert inner.upvalue_descriptors == [{:parent_upvalue, 0, "a"}] end test "shadowed captures of the same name stay in their own function's list" do @@ -86,13 +86,8 @@ defmodule Lua.Compiler.UpvalueDescriptorTest do [inner] = outer.prototypes - assert outer.upvalue_descriptors == [{:parent_local, 0, "_ENV"}, {:parent_local, 1, "x"}] - - assert inner.upvalue_descriptors == [ - {:parent_upvalue, 0, "_ENV"}, - {:parent_local, 0, "seen"}, - {:parent_local, 1, "x"} - ] + assert outer.upvalue_descriptors == [{:parent_local, 1, "x"}] + assert inner.upvalue_descriptors == [{:parent_local, 0, "seen"}, {:parent_local, 1, "x"}] assert {[5], _} = Lua.eval!(""" @@ -106,6 +101,46 @@ defmodule Lua.Compiler.UpvalueDescriptorTest do """) end + test "a closure captures _ENV only when it uses a global, in first-use order" do + [h] = + prototypes_of(""" + local c = 1 + local function h() return c, print end + return h() + """) + + assert h.upvalue_descriptors == [{:parent_local, 1, "c"}, {:parent_local, 0, "_ENV"}] + end + + test "a nested function reaches _ENV through a parent that does not use it" do + source = """ + local function outer() + return function() return type(print) end + end + return outer()() + """ + + [outer] = prototypes_of(source) + [inner] = outer.prototypes + + assert outer.upvalue_descriptors == [{:parent_local, 0, "_ENV"}] + assert inner.upvalue_descriptors == [{:parent_upvalue, 0, "_ENV"}] + assert {["function"], _} = Lua.eval!(source) + end + + test "a function with an _ENV parameter does not capture the chunk's _ENV" do + [f] = + prototypes_of(""" + local function f(_ENV) return function() return x end end + return f({x = 7})() + """) + + [inner] = f.prototypes + + assert f.upvalue_descriptors == [] + assert inner.upvalue_descriptors == [{:parent_local, 0, "_ENV"}] + end + test "no prototype in the compilable surface carries a duplicate descriptor" do sources = Path.wildcard("test/lua53_tests/*.lua") ++ Path.wildcard("test/integration/**/*.lua") @@ -149,12 +184,7 @@ defmodule Lua.Compiler.UpvalueDescriptorTest do return table.concat(out, ",") """ - # PUC-Lua prints "a,b": it only gives a closure an `_ENV` upvalue when - # the closure actually references a global. This VM currently hands - # `_ENV` to every nested function, so slot 1 is always `_ENV`. This - # assertion pins that divergence on purpose — aligning `_ENV` capture - # with PUC-Lua should update it to "a,b", not read as a regression. - assert {["_ENV,a,b"], _} = Lua.eval!(code) + assert {["a,b"], _} = Lua.eval!(code) end end diff --git a/test/lua/vm/display_test.exs b/test/lua/vm/display_test.exs index 0dc822bf..d116268d 100644 --- a/test/lua/vm/display_test.exs +++ b/test/lua/vm/display_test.exs @@ -66,13 +66,12 @@ defmodule Lua.VM.DisplayTest do describe "Lua.VM.Display.Closure inspect" do test "wraps Lua closures returned in default decode mode" do - {[c], _} = Lua.eval!(Lua.new(), "return function(a, b) return a + b end") + {[c], _} = Lua.eval!(Lua.new(), "local k = 1 return function(a, b) return a + b + k end") assert %Closure{source: "", line: 1, arity: 2, vararg?: false, ref: ref} = c assert match?({:lua_closure, _, _}, ref) or match?({:compiled_closure, _, _}, ref) - # The closure captures `_ENV` as an upvalue, surfaced in the inspect output. - assert inspect(c) == "#Lua.Closure\", line: 1, arity: 2, upvalues: [_ENV]>" + assert inspect(c) == "#Lua.Closure\", line: 1, arity: 2, upvalues: [k]>" end test "wraps Lua closures returned in decode: false mode" do diff --git a/test/lua/vm/env_semantics_test.exs b/test/lua/vm/env_semantics_test.exs index 9c74aea9..9eeb6aee 100644 --- a/test/lua/vm/env_semantics_test.exs +++ b/test/lua/vm/env_semantics_test.exs @@ -12,9 +12,8 @@ defmodule Lua.VM.EnvSemanticsTest do # In our implementation, the chunk reserves register 0 for `_ENV` and # binds it to `_G` at startup via the `:load_env` opcode. Free names in # the chunk compile to `_ENV.name` (`get_field`/`set_field` against - # register 0). Nested functions inherit `_ENV` via the standard upvalue - # chain, allocated eagerly during scope resolution so every function - # has access regardless of which free names it references. + # register 0). A nested function captures `_ENV` through the standard + # upvalue chain on its first free-name reference, as PUC-Lua does. setup do %{lua: Lua.new(sandboxed: [])}