[Tarantool-patches] [PATCH luajit v2] Fix narrowing of unary minus.
Sergey Kaplun
skaplun at tarantool.org
Wed Sep 21 11:48:47 MSK 2022
Hi, Maxim!
Thanks for the fixes!
LGTM, except a few nits below.
On 20.09.22, Maksim Kokryashkin wrote:
> From: Mike Pall <mike>
>
<snipped>
> src/lj_opt_narrow.c | 9 +++-
> .../gh-6976-narrowing-of-unary-minus.test.lua | 51 +++++++++++++++++++
> 2 files changed, 58 insertions(+), 2 deletions(-)
> create mode 100644 test/tarantool-tests/gh-6976-narrowing-of-unary-minus.test.lua
>
> diff --git a/src/lj_opt_narrow.c b/src/lj_opt_narrow.c
> index cd96ca4b..bb61f97b 100644
> --- a/src/lj_opt_narrow.c
> +++ b/src/lj_opt_narrow.c
<snipped>
> diff --git a/test/tarantool-tests/gh-6976-narrowing-of-unary-minus.test.lua b/test/tarantool-tests/gh-6976-narrowing-of-unary-minus.test.lua
> new file mode 100644
> index 00000000..18778a55
> --- /dev/null
> +++ b/test/tarantool-tests/gh-6976-narrowing-of-unary-minus.test.lua
> @@ -0,0 +1,51 @@
> +local tap = require('tap')
> +local test = tap.test('gh-6976-narrowing-of-unary-minus')
> +test:plan(2)
> +
> +jit.opt.start('hotloop=1', 'hotexit=1')
As far as we use `table.new` there is no table resizing, so there are no
exits from traces to compile and 'hotexit=1' can be removed.
> +
> +local function check(routine)
> + jit.off()
> + jit.flush()
> + local interp_res = routine()
> + jit.on()
> + local jit_res = routine()
> +
> + for i = 1, #interp_res do
> + if interp_res[i] ~= jit_res[i] then
> + return false
> + end
> + end
> +
> + return true
> +end
> +
> +test:ok(
Minor: I suggest the following indentation to make this chunk more
readable:
| test:ok(check(function()
| local res = require('table.new')(3, 0)
| for _ = 1, 3 do
| local zero = 0
| zero = -zero
| table.insert(res, tostring(zero))
| end
| return res
| end), 'incorrect recording for zero')
Feel free to ignore.
> + check(
> + function()
> + local res = require('table.new')(3, 0)
Please, add a comment why do we use `table.new` here.
> + for _ = 1, 3 do
> + local zero = 0
> + zero = -zero
> + table.insert(res, tostring(zero))
Minor: we need `tostring()` routine to get different strings for -0 and
0, as far as 0 == -0 as numbers. Please, add the corresponding comment.
> + end
> + return res
> + end
> + ),
> + 'incorrect recording for zero'
> +)
> +
Ditto, about indentation.
Feel free to ignore.
> +test:ok(
> + check(
> + function()
> + local res = require('table.new')(3, 0)
> + for i = 2, 0, -1 do
> + table.insert(res, tostring(-i))
> + end
> + return res
> + end
> + ),
> + 'assertion guard fail'
> +)
> +
> +os.exit(test:check() and 0 or 1)
> --
> 2.32.1 (Apple Git-133)
>
--
Best regards,
Sergey Kaplun
More information about the Tarantool-patches
mailing list