Re: [Tarantool-patches] [PATCH luajit][v2] test: limit code and comment max length

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

[-- Attachment #1: Type: text/plain, Size: 6339 bytes --]

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>
>

[-- Attachment #2: Type: text/html, Size: 7259 bytes --]