Hi, Igor! Thanks for the review!   Here is the new commit message: ================================================================================ test: adapt test checking global environment   LuaJIT doesn't take into account tail calls for call-level counting, so `getfenv()` behaviour is different in tail calls. `getfenv()` default level is 1 which is invalid for the test case when is called from tail call (`lj_debug_frame()` returns NULL).   This commits adds local variables, so there is no tail call any more.   Resolves tarantool/tarantool#5713 Part of tarantool/tarantool#5870 ================================================================================   And here is the diff: ================================================================================ diff --git a/test/PUC-Rio-Lua-5.1-tests/closure.lua b/test/PUC-Rio-Lua-5.1-tests/closure.lua index c4491dcf..291c1ede 100644 --- a/test/PUC-Rio-Lua-5.1-tests/closure.lua +++ b/test/PUC-Rio-Lua-5.1-tests/closure.lua @@ -170,8 +170,8 @@ local function foo (a)    -- LuaJIT doesn't take into account tail calls for  -- call-level counting, so `getfenv()` behaviour is different --- in tail calls. For example, `pcall()` to this functon returns --- false in case of tailcall, because `getfenv()` default level +-- in tail calls. For example, `pcall()` to this function returns +-- false in case of tail calls, because `getfenv()` default level  -- is 1 which is invalid for this case (`lj_debug_frame()`  -- returns NULL).  -- See also https://github.com/tarantool/tarantool/issues/5713. @@ -183,7 +183,7 @@ f = coroutine.wrap(foo)  local a = {}  assert(f(a) == _G)  local a,b = pcall(f) --- Test is adapted to the behavior LuaJIT. See the comment above. +-- Test is adapted to the LuaJIT behavior. See the comment above.  assert(a and b == _G) ================================================================================   Best regards, Maxim Kokryashkin >  >>Max, >> >>Thanks for the patch! >> >>LGTM, except a couple of typos. >> >>On 18.01.22, Максим Корякшин wrote: >>> >>> Hello, Sergey! >>> Thanks for the review! >>>   >>> Here is the new commit message: >>> ================================================================================ >>> test: adapt test checking global environment >>>   >>> LuaJIT doesn't take into account tail calls for >>> call-level counting, so `getfenv()` behaviour is different >>> in tail calls. getfenv()` default level is 1 which is invalid for >> >>Typo: s/getfenv()`/`getfenv()`/. >> >>> the test case when is called from tail call (`lj_debug_frame()` >>> returns NULL). >>>   >>> This commits adds local variables, so there is no tail call any more. >>>   >>> Part of tarantool/tarantool#5870 >>> Resolves tarantool/tarantool#5713 >> >>Typo: "Resolves" should go prior to all other GitHub tags ("Part of" in >>our case). >> >>> ================================================================================ >>>   >>> And the diff for the comment fix: >>> ================================================================================ >>> diff --git a/test/PUC-Rio-Lua-5.1-tests/closure.lua b/test/PUC-Rio-Lua-5.1-tests/closure.lua >>> index 86f6ad87..c4491dcf 100644 >>> --- a/test/PUC-Rio-Lua-5.1-tests/closure.lua >>> +++ b/test/PUC-Rio-Lua-5.1-tests/closure.lua >>> @@ -170,9 +170,10 @@ local function foo (a) >>>   >>>  -- LuaJIT doesn't take into account tail calls for >>>  -- call-level counting, so `getfenv()` behaviour is different >>> --- in tail calls. For example, `pcall()` to this functon returns false >>> --- in case of tailcall, because `getfenv()` default level is 1 which >>> --- is invalid for this case (`lj_debug_frame()` returns NULL). >>> +-- in tail calls. For example, `pcall()` to this functon returns >> >>Typo: s/functon/function/. >> >>> +-- false in case of tailcall, because `getfenv()` default level >> >>Typo: "tailcall", but "tail calls". Please choose one option for >>consistency (at least within a single comment). >> >>> +-- is 1 which is invalid for this case (`lj_debug_frame()` >>> +-- returns NULL). >>>  -- See also https://github.com/tarantool/tarantool/issues/5713 . >>>    local env = getfenv() >>>    return env >>> ================================================================================ >>>   >>> Best regards, >>> Maxim Kokryashkin >>>   >>> >  >>> >>Hi, Maxim! >>> >> >>> >>Thanks for the fixes! >>> >> >>> >>LGTM, except a few nits below. >>> >> >>> >>On 06.10.21, Maxim Kokryashkin wrote: >>> >>> LuaJIT doesn't take into account tail calls for >>> >>> call-level counting, so `getfenv()` behaviour is different >>> >>> in tail calls. getfenv()` default level is 1 which is invalid for >>> >>> the test case when is called from tail call (`lj_debug_frame()` >>> >>> returns NULL). >>> >>> >>> >>> This commits adds local varibles, so there is no tail call any more. >>> >> >>> >>Typo: s/varibles/variables/ >>> >> >>> >>> >>> >>> Part of tarantool/tarantool#5870 >>> >> >>> >>It's OK to mention that this patch "Resolves tarantool/tarantool#5713". >>> >> >>> >>> --- >>> >>> Issue: https://github.com/tarantool/tarantool/issues/5713 >>> >>> GitHub branch: https://github.com/tarantool/luajit/tree/fckxorg/gh-5713-adapt-test-global-environment-PUC-Rio >>> >>> CI: https://github.com/tarantool/tarantool/tree/fckxorg/gh-5713-adapt-test-global-environment-PUC-Rio >>> >>> >>> >>> test/PUC-Rio-Lua-5.1-tests/closure.lua | 21 +++++++++++---------- >>> >>> 1 file changed, 11 insertions(+), 10 deletions(-) >>> >>> >>> >>> diff --git a/test/PUC-Rio-Lua-5.1-tests/closure.lua b/test/PUC-Rio-Lua-5.1-tests/closure.lua >>> >>> index 551fe70d..86f6ad87 100644 >>> >>> --- a/test/PUC-Rio-Lua-5.1-tests/closure.lua >>> >>> +++ b/test/PUC-Rio-Lua-5.1-tests/closure.lua >>> >>> @@ -167,22 +167,23 @@ local function foo (a) >>> >>> assert(getfenv(0) == a) >>> >>> assert(getfenv(1) == _G) >>> >>> assert(getfenv(loadstring"") == a) >>> >>> - return getfenv() >>> >>> + >>> >>> +-- LuaJIT doesn't take into account tail calls for >>> >>> +-- call-level counting, so `getfenv()` behaviour is different >>> >>> +-- in tail calls. For example, `pcall()` to this functon returns false >>> >>> +-- in case of tailcall, because `getfenv()` default level is 1 which >>> >>> +-- is invalid for this case (`lj_debug_frame()` returns NULL). >>> >>> +-- See also https://github.com/tarantool/tarantool/issues/5713 . >>> >> >>> >>Minor: linewidth for comments is 66 symbols. >>> >> >>> >>> + local env = getfenv() >>> >>> + return env >>> >>> end >>> >>> >>> >>> f = coroutine.wrap(foo) >>> >>> local a = {} >>> >>> assert(f(a) == _G) >>> >>> local a,b = pcall(f) >>> >>> --- FIXME: LuaJIT doesn't take into account tail calls for >>> >>> --- call-level counting, so `getfenv()` behaviour is different >>> >>> --- in tail calls. For example, this `pcall()` returns false, >>> >>> --- because `getfenv()` default level is 1 which is invalid for >>> >>> --- this case when is called from tail call (`lj_debug_frame()` >>> >>> --- returns NULL). >>> >>> --- See also https://github.com/tarantool/tarantool/issues/5713 . >>> >>> --- Test is disabled for LuaJIT for now. >>> >>> --- assert(a and b == _G) >>> >>> +-- Test is adapted to the behavior LuaJIT. See the comment above. >> >>Typo: either s/behavior LuaJIT/LuaJIT behavior/ or >>s/behavior LuaJIT/behavior of LuaJIT/ for this case. >> >>> >>> +assert(a and b == _G) >>> >>> >>> >>> >>> >>> -- tests for multiple yield/resume arguments >>> >>> -- >>> >>> 2.33.0 >>> >>> >>> >> >>> >>-- >>> >>Best regards, >>> >>Sergey Kaplun >>> >  >> >>-- >>Best regards, >>IM >