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