Hi, Sergey,
thanks for fixes! LGTM
One more minor update below. On 13.09.24, Sergey Kaplun via Tarantool-patches wrote:Hi, Sergey! Thanks for the review! On 13.09.24, Sergey Bronnikov wrote:Hi, Sergey, thanks for the patch! LGTM with a minor comment. See below. On 21.08.2024 11:23, Sergey Kaplun wrote: <snipped>+-- Need to use metamethod call in the concat recording. +debug.setmetatable(nil, {I propose to add a comment that explain why `nil` is used here.I've removed the nil usage to avoid confusion. Branch is rebased on the current master and force-pushed. =================================================================== diff --git a/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua b/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua index 9abaeba5..4fe02708 100644 --- a/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua +++ b/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua @@ -12,12 +12,13 @@ test:plan(2) jit.opt.start('hotloop=1') -local __concat = function(v1, v2) - return tostring(v1) .. tostring(v2) +local __concat = function() + return '' end -- Need to use metamethod call in the concat recording. -debug.setmetatable(nil, { +-- Let's use a table with the metamethod. +local concatable_t = setmetatable({}, { __concat = __concat, }) @@ -31,7 +32,7 @@ local function test_concat_p() -- `lj_record_ret()` restore the Lua stack (before the patch), -- it becomes unbalanced after the instruction recording -- attempt. - local _ = {} .. (nil .. nil) + local _ = {} .. (concatable_t .. concatable_t) end end ===================================================================Added more verbose description: =================================================================== diff --git a/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua b/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua index 4fe02708..75736e74 100644 --- a/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua +++ b/test/tarantool-tests/lj-1234-err-in-record-concat.test.lua @@ -17,7 +17,8 @@ local __concat = function() end -- Need to use metamethod call in the concat recording. --- Let's use a table with the metamethod. +-- We may use any object with a metamethod, but let's use a table +-- as the most common one. local concatable_t = setmetatable({}, { __concat = __concat, }) ===================================================================+ __concat = __concat, +}) +<snipped> -- Best regards, Sergey Kaplun