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

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

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