[Tarantool-patches] [PATCH v2 luajit 21/30] test: disable test for getfenv in closure tailcall

Sergey Kaplun skaplun at tarantool.org
Fri Apr 2 10:40:41 MSK 2021


Igor,

Thanks for the review!

On 31.03.21, Igor Munkin wrote:
> Sergey,
> 
> Thanks for the patch! LGTM, except the comments related to the wording.
> 
> On 26.03.21, Sergey Kaplun wrote:
> > LuaJIT doesn't take into account tail calls for call-level counting, so
> 
> Minor: This is a good example, when the passive voice makes the sense
> clearer. Consider the following:
> | Tail calls are not taken into account for call-level counting.
> 
> Otherwise it seems like LuaJIT doesn't take into account *tail calls for
> call-level counting* (i.e. tail calls that are not used for call-level
> counting are taken into). Or simply try to use another preposition.

Yes, see it. Thanks! Fixed.

> 
> > getfenv() behaviour is different from Lua 5.1 in tail calls.
> 
> Minor: You use both "tailcall" and "tail call" within this commit.
> Please choose one and use it everywhere.

Fixed.

> 
> > 
> > This patch disables test for the return result of tail call getfenv().
> 
> Typo: s/tail call getfenv()/getfenv() tail call/.

Fixed.

> 
> > Default value (equals 1) of getfenv() function's argument (function
> 
> Typo: s/equals 1/equals to 1/.

Fixed.

> 
> > level) is invalid for this tail call -- LuaJIT can't provide necessary
> > debug information for the frame.
> 
> Minor: I would explicitly mention, that there is no a separate frame for
> tail call. LuaJIT simply uses the existing one created for the caller.

Fixed.

> 
> > 
> > Relates to tarantool/tarantool#5713
> > Part of tarantool/tarantool#5845
> > Part of tarantool/tarantool#4473
> > ---
> >  test/PUC-Lua-5.1-tests/closure.lua | 9 ++++++++-
> >  1 file changed, 8 insertions(+), 1 deletion(-)
> > 
> 
> <snipped>
> 
> > -- 
> > 2.31.0
> > 
> 
> -- 
> Best regards,
> IM

-- 
Best regards,
Sergey Kaplun


More information about the Tarantool-patches mailing list