[Tarantool-patches] [PATCH luajit v1] Fix BC_UCLO insertion for returns.
Sergey Kaplun
skaplun at tarantool.org
Thu Jul 13 10:57:59 MSK 2023
Hi, Sergey!
Thanks for the fixes!
Still some thoughts about the `pcall()`.
On 10.07.23, Sergey Bronnikov wrote:
> Hi, Sergey!
>
>
> thanks for review!
>
>
> On 7/9/23 16:15, Sergey Kaplun wrote:
> > Hi, Sergey!
> > Thanks for the fixes!
> > LGTM, except a few nits and rewordings below.
> >
<snipped>
> @@ -110,6 +114,7 @@ f()
> f = missing_uclo()
> local _
> _, res = pcall(f)
> -test:ok(type(res) == 'function', 'consistency on compilation: type of
> returned value is correct')
> +test:ok(type(res) == 'function',
> + 'consistency on compilation: type of returned value is correct')
>
> os.exit(test:check() and 0 or 1)
>
> > Do we need pcall here?
>
>
> I would use it to avoid breaking test due to assert.
>
> Without a pcall:
>
>
> TAP version 13
> 1..2
> not ok - VM consistency: type of returned value is correct
> filename: eval
> line: -1
> frame #1
> line: 0
> source: @test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
> filename: test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
> what: main
> namewhat:
> src: test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
> frame #2
> line: -1
> source: =[C]
> filename: eval
> what: C
> namewhat:
> src: [C]
> luajit:
> /home/sergeyb/sources/MRG/tarantool/third_party/luajit/src/lj_record.c:135:
> rec_check_slots: Assertion `((((((tr))>>24) & IRT_TYPE) -
> (TRef)(IRT_NUM) <= (TRef)(IRT_INT-IRT_NUM)))' failed.
> Aborted (core dumped)
>
> With pcall:
>
> TAP version 13
> 1..2
> not ok - VM consistency: type of returned value is correct
>
> filename: eval
> line: -1
> frame #1
> line: 0
> source: @test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
> filename: test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
> what: main
> namewhat:
> src: test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
> frame #2
> line: -1
> source: =[C]
> filename: eval
> what: C
> namewhat:
> src: [C]
> not ok - consistency on compilation: type of returned value is correct
> filename: eval
> line: -1
> frame #1
> line: 0
>
> <snipped>
>
>
> I like second output more.
Yes, but there is no trace related to the `f()` only for
`test:check()`:
| ---- TRACE 1 start tap.lua:33
| ---- TRACE 2 start 1/stitch tap.lua:34
| ---- TRACE 3 start tap.lua:16
| ---- TRACE 3 start tap.lua:80
So, with this `pcall()` we lose the JIT testing.
>
> >
> > Also, the test isn't failed with assertion failure as declared. But the
> > following one is:
> > | LUA_PATH="src/?.lua;;" src/luajit -Ohotloop=1 -e '
> > |
> > | local function missing_uclo()
> > | while true do -- luacheck: ignore
> > | local f
> > | if false then break end
> > | while true do
> > | if f then
> > | return f
> > | end
> > | f = function()
> > | return f
> > | end
> > | end
> > | end
> > | end
> > |
> > | -- Function to pollute Lua stack.
> > | local function ret_arg(f) return f end
> > |
> > | f = missing_uclo()
> > | ret_arg(f())
> > | ret_arg(f())
> > | '
> >
> >> +
> >> +os.exit(test:check() and 0 or 1)
> > [1]: https://github.com/tarantool/tarantool/wiki/Code-review-procedure#commit-message
> >
--
Best regards,
Sergey Kaplun
More information about the Tarantool-patches
mailing list