<!DOCTYPE html>
<html data-lt-installed="true">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body style="padding-bottom: 1px;">
<p>Hi, Sergey,</p>
<p>fixed and force-pushed.</p>
<p>Sergey<br>
</p>
<div class="moz-cite-prefix">On 10.12.2024 12:32, Sergey Kaplun
wrote:<br>
</div>
<blockquote type="cite" cite="mid:Z1gKrIGcjQlD-wJX@root">
<pre class="moz-quote-pre" wrap="">Hi, Sergey!
Thanks for the fixes!
LGTM, after fixing a bunch of comments below.
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">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.
+-- <a class="moz-txt-link-freetext" href="https://github.com/LuaJIT/LuaJIT/issues/1279">https://github.com/LuaJIT/LuaJIT/issues/1279</a>
+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 metatable
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/metatable/the metatable/</pre>
</blockquote>
---
a/test/tarantool-tests/lj-1279-incorrect-recording-getmetatable.test.lua<br>
+++
b/test/tarantool-tests/lj-1279-incorrect-recording-getmetatable.test.lua<br>
@@ -21,8 +21,9 @@ local function rec_getmetatable(obj)<br>
return res<br>
end<br>
<br>
--- The testcase to demonstrate a problem by comparing metatable<br>
--- returned by two versions of `getmetatable()`: compiled and not.<br>
+-- The testcase to demonstrate a problem by comparing the<br>
+-- metatable returned by two versions of `getmetatable()`:<br>
+-- compiled and not.<br>
<br>
local mt_orig = debug.getmetatable(ud_io_file)<br>
assert(type(mt_orig) == 'table')<br>
<blockquote type="cite" cite="mid:Z1gKrIGcjQlD-wJX@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- 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')
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
This restoring is excess since we don't change the metatable yet.</pre>
</blockquote>
<p>Ok, updated:</p>
<p>@@ -36,17 +37,13 @@ mt_rec[5] = mt_orig<br>
test:ok(true, 'getmetatable() recording is correct')<br>
test:samevalues(mt_rec, 'metatables are the same')<br>
<br>
--- Restore metatable.<br>
-debug.setmetatable(ud_io_file, mt_orig)<br>
-assert(type(mt_orig) == 'table')<br>
-<br>
</p>
<blockquote type="cite" cite="mid:Z1gKrIGcjQlD-wJX@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+
+-- The testcase to demonstrate a problem by setting metatable for
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/metatable/the metatable/</pre>
</blockquote>
<p>Fixed:</p>
<p><br>
</p>
@@ -40,8 +41,8 @@ test:samevalues(mt_rec, 'metatables are the same')<br>
debug.setmetatable(ud_io_file, mt_orig)<br>
assert(type(mt_orig) == 'table')<br>
<br>
--- The testcase to demonstrate a problem by setting metatable for<br>
--- `io.stdout` to a string.<br>
+-- The testcase to demonstrate a problem by setting the metatable<br>
+-- for `io.stdout` to a string.<br>
<br>
-- Compile `getmetatable()`, it is expected metatable has<br>
-- a `table` type.<br>
<blockquote type="cite" cite="mid:Z1gKrIGcjQlD-wJX@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- `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.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Minor: I would rephrase this like the following:
| Set IO metatable to a string.
"Custom" isn't clear for me.
</pre>
</blockquote>
<p>Fixed:</p>
<p> -- Compile `getmetatable()`, it is expected metatable has<br>
-- a `table` type.<br>
rec_getmetatable(ud_io_file)<br>
--- Set a custom metatable to a string.<br>
+-- Set IO metatable to a string.<br>
local mt = 'IO metatable'<br>
getmetatable(ud_io_file).__metatable = mt<br>
test:is(getmetatable(ud_io_file), mt, 'getmetatable() is
correct')<br>
</p>
<blockquote type="cite" cite="mid:Z1gKrIGcjQlD-wJX@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+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')
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
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).
</pre>
</blockquote>
<p>Added:</p>
<p><br>
</p>
<p>@@ -55,9 +56,11 @@ test:is(rec_getmetatable(ud_io_file), mt,
'compiled getmetatable() is correct')<br>
-- Restore metatable.<br>
debug.setmetatable(ud_io_file, mt_orig)<br>
assert(type(mt_orig) == 'table')<br>
+jit.flush()<br>
+jit.opt.start('hotloop=1')<br>
<br>
</p>
<blockquote type="cite" cite="mid:Z1gKrIGcjQlD-wJX@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+
+-- The testcase to demonstrate a problem by removing metatable for
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/metatable/the metatable/
</pre>
</blockquote>
<p>Fixed:</p>
<p><br>
</p>
@@ -56,8 +57,8 @@ test:is(rec_getmetatable(ud_io_file), mt,
'compiled getmetatable() is correct')<br>
debug.setmetatable(ud_io_file, mt_orig)<br>
assert(type(mt_orig) == 'table')<br>
<br>
--- The testcase to demonstrate a problem by removing metatable for<br>
--- `io.stdout` and calling garbage collector.<br>
+-- The testcase to demonstrate a problem by removing the metatable<br>
+-- for `io.stdout` and calling garbage collector.<br>
<br>
-- Compile `getmetatable()`, it is expected metatable has<br>
-- a `table` type.<br>
<blockquote type="cite" cite="mid:Z1gKrIGcjQlD-wJX@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+-- `io.stdout` and calling garbage collector.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Typo: s/garbage collector/the garbage collector/</pre>
</blockquote>
<p>Fixed:</p>
<p><br>
</p>
@@ -56,8 +57,8 @@ test:is(rec_getmetatable(ud_io_file), mt,
'compiled getmetatable() is correct')<br>
debug.setmetatable(ud_io_file, mt_orig)<br>
assert(type(mt_orig) == 'table')<br>
<br>
--- The testcase to demonstrate a problem by removing metatable for<br>
--- `io.stdout` and calling garbage collector.<br>
+-- The testcase to demonstrate a problem by removing the metatable<br>
+-- for `io.stdout` and calling the garbage collector.<br>
<br>
-- Compile `getmetatable()`, it is expected metatable has<br>
-- a `table` type.<br>
<blockquote type="cite" cite="mid:Z1gKrIGcjQlD-wJX@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+
+-- 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)
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
On 22.11.24, Sergey Bronnikov wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">Hi, Sergey,
please see below.
Updated version has been force-pushed.
Sergey
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
<snipped>
</pre>
<blockquote type="cite">
<blockquote type="cite">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">[1]:<a class="moz-txt-link-freetext" href="https://github.com/LuaJIT/LuaJIT/issues/1279#issuecomment-2382392847">https://github.com/LuaJIT/LuaJIT/issues/1279#issuecomment-2382392847</a>
</pre>
</blockquote>
</blockquote>
<pre class="moz-quote-pre" wrap="">
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>