[Tarantool-patches] [PATCH luajit][v2] test: limit code and comment max length
Sergey Bronnikov
sergeyb at tarantool.org
Fri Feb 14 09:49:38 MSK 2025
Hi, Sergey!
Updated and force-pushed.
Sergey
On 13.02.2025 16:50, Sergey Kaplun via Tarantool-patches wrote:
> 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?
I've no objections, applied.
>
>> -- 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>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20250214/55b42f49/attachment.htm>
More information about the Tarantool-patches
mailing list