[Tarantool-patches] [PATCH luajit][v2] test: limit code and comment max length
Sergey Kaplun
skaplun at tarantool.org
Thu Feb 13 16:50:11 MSK 2025
Hi, Sergey!
Thanks for the decent work!
I've checked, at a rough guess, that there are no line numbers to be
updated in these changed files.
LGTM, with an ignorable suggestion below.
On 12.02.25, Sergey Bronnikov wrote:
> The patch sets a max length with 80 symbols for lines with code
> and max length with 66 symbols for lines with comments in luacheck
> configuration file [1] and fixes files where this length is
> exceeding.
>
> 1. https://luacheck.readthedocs.io/en/stable/warnings.html#line-length-limits
> ---
> Changes v2:
> - Added fixes according to comments by Sergey Kaplun.
> - Reduced a max length of lines with comments (80 -> 66).
> - Fixed warnings triggered by reducing max limit of lines with
> comments.
>
> Branch: https://github.com/tarantool/luajit/tree/ligurio/gh-xxxx-set-max-length
>
> .luacheckrc | 3 +
> .../fix-argv-handling.test.lua | 4 ++
> .../fix-binary-number-parsing.test.lua | 2 +
> .../gh-3196-incorrect-string-length.test.lua | 3 +
> ...gh-4773-tonumber-fail-on-NUL-char.test.lua | 9 +--
> test/tarantool-tests/gh-6163-min-max.test.lua | 68 ++++++++++++-------
> .../gh-7745-oom-on-trace.test.lua | 3 +-
> .../lj-1004-oom-error-frame.test.lua | 6 +-
> .../lj-1116-redzones-checks.test.lua | 2 +
> .../lj-1149-g-number-formating-bufov.test.lua | 4 +-
> .../lj-366-strtab-correct-size.test.lua | 17 +++--
> .../lj-416-xor-before-jcc.test.lua | 28 ++++----
> .../lj-494-table-chain-infinite-loop.test.lua | 14 ++--
> ...lj-505-fold-no-strref-for-ptrdiff.test.lua | 3 +-
> .../lj-524-fold-conv-respect-src-irt.test.lua | 4 +-
> .../lj-603-err-snap-restore.test.lua | 6 +-
> ...-611-gc64-inherit-frame-slot-orig.test.lua | 2 +
> .../lj-611-gc64-inherit-frame-slot.test.lua | 2 +
> .../lj-624-jloop-snapshot-pc.test.lua | 4 +-
> .../lj-688-snap-ir-rename.test.lua | 2 +
> .../lj-737-snap-use-def-upvalues.test.lua | 3 +-
> .../lj-819-fix-missing-uclo.test.lua | 49 +++++++------
> ...-865-cross-generation-mach-o-file.test.lua | 4 ++
> ...lj-918-fma-numerical-accuracy-jit.test.lua | 4 ++
> .../lj-918-fma-numerical-accuracy.test.lua | 4 ++
> .../lj-962-stack-overflow-report.test.lua | 3 +-
> .../lj-962-stack-overflow-report/script.lua | 3 +-
> .../mark-conv-non-weak.test.lua | 2 +
> .../misclib-getmetrics-lapi.test.lua | 9 ++-
> .../or-144-gc64-asmref-l.test.lua | 4 ++
> .../or-232-unsink-64-kptr.test.lua | 24 ++++---
> .../profilers/gh-5688-tool-cli-flag.test.lua | 3 +-
> .../gh-5813-resolving-of-c-symbols.test.lua | 9 +--
> .../gh-5994-memprof-human-readable.test.lua | 3 +-
> ...17-profile-parsers-error-handling.test.lua | 3 +-
> tools/sysprof/parse.lua | 6 +-
> 36 files changed, 206 insertions(+), 113 deletions(-)
<snipped>
> diff --git a/test/tarantool-tests/mark-conv-non-weak.test.lua b/test/tarantool-tests/mark-conv-non-weak.test.lua
> index 73c24b66..fe0969cf 100644
> --- a/test/tarantool-tests/mark-conv-non-weak.test.lua
> +++ b/test/tarantool-tests/mark-conv-non-weak.test.lua
> @@ -4,6 +4,7 @@ local test = tap.test('mark-conv-non-weak'):skipcond({
> })
>
> test:plan(1)
> +-- luacheck: push no max_comment_line_length
I suggest the following patch instead to prevent the warning only for
necessary part of the IR dump:
===================================================================
diff --git a/test/tarantool-tests/mark-conv-non-weak.test.lua b/test/tarantool-tests/mark-conv-non-weak.test.lua
index 73c24b66..4396ee58 100644
--- a/test/tarantool-tests/mark-conv-non-weak.test.lua
+++ b/test/tarantool-tests/mark-conv-non-weak.test.lua
@@ -9,20 +9,22 @@ test:plan(1)
-- instruction is emitted. See `loop_unrool` in `lj_opt_loop.c`.
local data = {0, 0.1, 0, 0 / 0}
---- XXX: The sum is required to be initialized with a non-zero
--- floating point value; otherwise, `0023 + num ADD 0017 0007`
--- instruction in the IR below becomes `ADDOV` and the `CONV int.num`
--- conversion is used by it.
+-- XXX: The sum is required to be initialized with a non-zero
+-- floating point value.
+-- Otherwise, `0023 + num ADD 0017 0007` instruction in the
+-- IR below becomes `ADDOV` and the `CONV int.num` conversion is
+-- used by it.
local sum = 0.1
jit.opt.start('hotloop=1')
--- XXX: The test fails before the patch only
--- for `DUALNUM` mode. All of the IRs below are
--- produced by the corresponding LuaJIT build.
+-- XXX: The test fails before the patch only for `DUALNUM` mode.
+-- All of the IRs below are produced by the corresponding LuaJIT
+-- build.
--- When the trace is recorded, the IR
--- is the following before the patch:
+-- luacheck: push no max_comment_line_length
+-- When the trace is recorded, the IR is the following before the
+-- patch:
---- TRACE 1 IR
-- .... SNAP #0 [ ---- ---- ---- ---- ---- ---- ---- ---- ---- ]
-- 0001 u8 XLOAD [0x100dac521] V
@@ -104,6 +106,8 @@ jit.opt.start('hotloop=1')
---- TRACE 1 exit 0
---- TRACE 1 exit 2
--
+-- luacheck: pop
+--
-- Before the patch, the `0022 > int CONV 0017 int.num`
-- instruction is omitted due to DCE, which results in the
-- third side exit being taken, instead of the second,
===================================================================
Thoughts?
> -- XXX: These values were chosen to create type instability
> -- in the loop-carried dependency, so the checked `CONV int.num`
> -- instruction is emitted. See `loop_unrool` in `lj_opt_loop.c`.
> @@ -114,6 +115,7 @@ jit.opt.start('hotloop=1')
> --
> -- Note that DCE happens on the assembly part of the trace
> -- compilation. That is why `CONV` is present in both IRs.
> +-- luacheck: pop
>
> for _, val in ipairs(data) do
> if val == val then
<snipped>
--
Best regards,
Sergey Kaplun
More information about the Tarantool-patches
mailing list