Skip to content

Fix env for lua5.1/luajit#347

Open
espkk wants to merge 1 commit intoactboy168:masterfrom
espkk:master
Open

Fix env for lua5.1/luajit#347
espkk wants to merge 1 commit intoactboy168:masterfrom
espkk:master

Conversation

@espkk
Copy link
Copy Markdown

@espkk espkk commented Apr 10, 2026

Basically adds setfenv/getfenv support for lua5.1/luajit

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces support for Lua 5.1 function environments (fenv) by updating the evaluation scripts, variable handling, and the C++ backend's reference value system. Feedback suggests refactoring the environment detection logic in variables.lua to eliminate redundant assignments for the default global environment.

Comment on lines +149 to +161
if LUAVERSION == 51 then
local fenv = rdebug.getfenv(info.func)
if fenv and not rdebug.equal(fenv, rdebug._G) then
eval = nil
value = fenv
else
eval = "_G"
value = rdebug._G
end
else
eval = "_G"
value = rdebug._G
end
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for setting the default global environment (_G) is duplicated in both the else branch of the Lua 5.1 check and the else branch of the custom environment check. This can be simplified by setting the defaults first and then overriding them only if the specific Lua 5.1 condition is met, which improves maintainability.

        eval = "_G"
        value = rdebug._G
        if LUAVERSION == 51 then
            local fenv = rdebug.getfenv(info.func)
            if fenv and not rdebug.equal(fenv, rdebug._G) then
                eval = nil
                value = fenv
            end
        end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant