Re: [Tarantool-patches] [PATCH luajit] Handle table unsinking in the presence of IRFL_TAB_NOMM.

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Sergey


Thanks for the patch! LGTM

See a minor comment below.


On 8/29/23 15:36, Sergey Kaplun wrote:

<snipped>

> diff --git a/test/tarantool-tests/lj-1052-unsink-with-irfl-tab-nomm.test.lua b/test/tarantool-tests/lj-1052-unsink-with-irfl-tab-nomm.test.lua
> new file mode 100644
> index 00000000..aaf5349f
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1052-unsink-with-irfl-tab-nomm.test.lua
> @@ -0,0 +1,36 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate LuaJIT's incorrect restoration of a
> +-- table from a snapshot with the presence of `IRFL_TAB_NOMM`.
> +-- See also: https://github.com/LuaJIT/LuaJIT/issues/1052.
> +
> +local test = tap.test('lj-1052-unsink-with-irfl-tab-nomm'):skipcond({
> +  ['Test requires JIT enabled'] = not jit.status(),
> +})
> +
> +test:plan(2)
> +
> +local TEST_VALUE = 'test'
> +
> +jit.opt.start('hotloop=1')
> +
> +local counter = 0
> +local slot = 'slot'
> +while true do

You are using endless loop here, but loop is limited by 3 iterations 
(break after counter > 2).

Why "while true do end" is used here? Should we left a comment here?

> +  counter = counter + 1
> +  -- Use a non-constant slot to emit `FREF` with `IRFL_TAB_NOMM`.
> +  -- After re-emitting the variant part of the loop, NEWREF will
> +  -- contain a constant key (see below).
> +  slot = {[slot] = TEST_VALUE}
> +  -- Emit exit here to be sure that the table will be restored
> +  -- from the snapshot.
> +  if counter > 2 then break end
> +  -- We need a constant reference for NEWREF. Just use the old
> +  -- value.
> +  slot = 'slot'
> +end
<snipped>