Hi, Sergey,
fixed and force-pushed.
Sergey
--- a/test/tarantool-tests/lj-1279-incorrect-recording-getmetatable.test.luaHi, Sergey! Thanks for the fixes! LGTM, after fixing a bunch of comments below.diff --git a/test/tarantool-tests/lj-1279-incorrect-recording-getmetatable.test.lua b/test/tarantool-tests/lj-1279-incorrect-recording-getmetatable.test.lua new file mode 100644 index 00000000..dc01307e --- /dev/null +++ b/test/tarantool-tests/lj-1279-incorrect-recording-getmetatable.test.lua @@ -0,0 +1,74 @@ +local tap = require('tap') +-- A test file to demonstrate an incorrect recording of +-- `getmetatable()` for I/O handlers. +-- https://github.com/LuaJIT/LuaJIT/issues/1279 +local test = tap.test('lj-1279-incorrect-recording-getmetatable'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(6) + +jit.opt.start('hotloop=1') + +local ud_io_file = io.stdout +local getmetatable = getmetatable + +local function rec_getmetatable(obj) + local res + for _ = 1, 4 do + res = getmetatable(obj) + end + return res +end + +-- The testcase to demonstrate a problem by comparing metatableTypo: s/metatable/the metatable/
+-- returned by two versions of `getmetatable()`: compiled and not. + +local mt_orig = debug.getmetatable(ud_io_file) +assert(type(mt_orig) == 'table') + +local mt_rec = {} +for i = 1, 4 do + mt_rec[i] = getmetatable(ud_io_file) +end +mt_rec[5] = mt_orig + +test:ok(true, 'getmetatable() recording is correct') +test:samevalues(mt_rec, 'metatables are the same') + +-- Restore metatable. +debug.setmetatable(ud_io_file, mt_orig) +assert(type(mt_orig) == 'table')This restoring is excess since we don't change the metatable yet.
Ok, updated:
@@ -36,17 +37,13 @@ mt_rec[5] = mt_orig
test:ok(true, 'getmetatable() recording is correct')
test:samevalues(mt_rec, 'metatables are the same')
--- Restore metatable.
-debug.setmetatable(ud_io_file, mt_orig)
-assert(type(mt_orig) == 'table')
-
+ +-- The testcase to demonstrate a problem by setting metatable forTypo: s/metatable/the metatable/
Fixed:
+-- `io.stdout` to a string. + +-- Compile `getmetatable()`, it is expected metatable has +-- a `table` type. +rec_getmetatable(ud_io_file) +-- Set a custom metatable to a string.Minor: I would rephrase this like the following: | Set IO metatable to a string. "Custom" isn't clear for me.
Fixed:
-- Compile `getmetatable()`, it is expected metatable has
-- a `table` type.
rec_getmetatable(ud_io_file)
--- Set a custom metatable to a string.
+-- Set IO metatable to a string.
local mt = 'IO metatable'
getmetatable(ud_io_file).__metatable = mt
test:is(getmetatable(ud_io_file), mt, 'getmetatable() is
correct')
+local mt = 'IO metatable' +getmetatable(ud_io_file).__metatable = mt +test:is(getmetatable(ud_io_file), mt, 'getmetatable() is correct') +test:is(rec_getmetatable(ud_io_file), mt, 'compiled getmetatable() is correct') + +-- Restore metatable. +debug.setmetatable(ud_io_file, mt_orig) +assert(type(mt_orig) == 'table')It is better also to restore the JIT state here, ie. call `jit.flush()` and reset hotcounters via `jit.opt.start('hotloop=1')` to be in sync with the code (we want to compile a trace again).
Added:
@@ -55,9 +56,11 @@ test:is(rec_getmetatable(ud_io_file), mt,
'compiled getmetatable() is correct')
-- Restore metatable.
debug.setmetatable(ud_io_file, mt_orig)
assert(type(mt_orig) == 'table')
+jit.flush()
+jit.opt.start('hotloop=1')
+ +-- The testcase to demonstrate a problem by removing metatable forTypo: s/metatable/the metatable/
Fixed:
+-- `io.stdout` and calling garbage collector.Typo: s/garbage collector/the garbage collector/
Fixed:
+ +-- Compile `getmetatable()`, it is expected metatable has +-- a `table` type. +rec_getmetatable(ud_io_file) +-- Delete metatable. +debug.setmetatable(ud_io_file, nil) +collectgarbage() +test:is(getmetatable(ud_io_file), nil, 'getmetatable() is correct') +test:is(rec_getmetatable(ud_io_file), nil, 'compiled getmetatable() is correct') + +-- Restore metatable. +debug.setmetatable(ud_io_file, mt_orig) + +test:done(true)On 22.11.24, Sergey Bronnikov wrote:Hi, Sergey, please see below. Updated version has been force-pushed. Sergey<snipped>[1]:https://github.com/LuaJIT/LuaJIT/issues/1279#issuecomment-2382392847