[Tarantool-patches] [PATCH luajit] test: limit code and comment max length by 80

[Tarantool-patches] [PATCH luajit] test: limit code and comment max length by 80

[Sergey Bronnikov via Tarantool-patches]

The patch sets a max length with 80 symbols and fixes tests
exceeding this length.
---
Branch: https://github.com/tarantool/luajit/tree/ligurio/gh-xxxx-set-max-length
Luacheck configuration file:
https://luacheck.readthedocs.io/en/stable/config.html

 .luacheckrc                                            |  5 +++++
 test/tarantool-tests/fix-argv-handling.test.lua        |  4 ++++
 .../gh-3196-incorrect-string-length.test.lua           |  3 +++
 test/tarantool-tests/gh-6163-min-max.test.lua          |  4 ++++
 .../lj-366-strtab-correct-size.test.lua                | 10 ++++++++--
 .../lj-494-table-chain-infinite-loop.test.lua          |  3 ++-
 test/tarantool-tests/lj-688-snap-ir-rename.test.lua    |  2 ++
 test/tarantool-tests/lj-819-fix-missing-uclo.test.lua  |  6 ++++--
 .../lj-918-fma-numerical-accuracy-jit.test.lua         |  4 ++++
 .../lj-918-fma-numerical-accuracy.test.lua             |  4 ++++
 test/tarantool-tests/or-144-gc64-asmref-l.test.lua     |  4 ++++
 11 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/.luacheckrc b/.luacheckrc
index f2573e42..8047677c 100644
--- a/.luacheckrc
+++ b/.luacheckrc
@@ -3,6 +3,11 @@ std = 'luajit'
 -- This fork also introduces a new global for misc API namespace.
 read_globals = { 'misc' }
 
+max_line_length = 80
+max_code_line_length = 80
+max_string_line_length = 80
+max_comment_line_length = 80
+
 -- The `_TARANTOOL` global is often used for skip condition
 -- checks in tests.
 files['test/tarantool-tests/'] = {
diff --git a/test/tarantool-tests/fix-argv-handling.test.lua b/test/tarantool-tests/fix-argv-handling.test.lua
index 84b626c3..5f147197 100644
--- a/test/tarantool-tests/fix-argv-handling.test.lua
+++ b/test/tarantool-tests/fix-argv-handling.test.lua
@@ -10,7 +10,11 @@ test:plan(1)
 -- to a single empty string if it is empty [1], so the issue is
 -- not reproducible on new kernels.
 --
+-- luacheck: push no max comment line length
+--
 -- [1]: https://lore.kernel.org/all/20220201000947.2453721-1-keescook@chromium.org/
+--
+-- luacheck: pop
 
 local utils = require('utils')
 local execlib = require('execlib')
diff --git a/test/tarantool-tests/gh-3196-incorrect-string-length.test.lua b/test/tarantool-tests/gh-3196-incorrect-string-length.test.lua
index b82029f6..79521608 100644
--- a/test/tarantool-tests/gh-3196-incorrect-string-length.test.lua
+++ b/test/tarantool-tests/gh-3196-incorrect-string-length.test.lua
@@ -6,10 +6,13 @@ test:plan(2)
 --
 -- gh-3196: incorrect string length if Lua hash returns 0
 --
+-- luacheck: push no max code line length
+--
 local h = "\x1F\x93\xE2\x1C\xCA\xDE\x28\x08\x26\x01\xED\x0A\x2F\xE4\x21\x02\x97\x77\xD9\x3E"
 test:is(h:len(), 20)
 
 h = "\x0F\x93\xE2\x1C\xCA\xDE\x28\x08\x26\x01\xED\x0A\x2F\xE4\x21\x02\x97\x77\xD9\x3E"
 test:is(h:len(), 20)
+-- luacheck: pop
 
 test:done(true)
diff --git a/test/tarantool-tests/gh-6163-min-max.test.lua b/test/tarantool-tests/gh-6163-min-max.test.lua
index e3efd1a4..ca6c2a7f 100644
--- a/test/tarantool-tests/gh-6163-min-max.test.lua
+++ b/test/tarantool-tests/gh-6163-min-max.test.lua
@@ -69,9 +69,13 @@ jit.opt.start('hotloop=1')
 -- pair is emitted, and the result is `NaN`, which is inconsistent with
 -- the result of the non-optimized mcode.
 --
+-- luacheck: push no max comment line length
+--
 -- [1]: https://developer.arm.com/documentation/dui0801/g/A64-Floating-point-Instructions/FCMP
 -- [2]: https://developer.arm.com/documentation/100069/0608/A64-Floating-point-Instructions/FCSEL
 -- [3]: https://developer.arm.com/documentation/dui0068/b/ARM-Instruction-Reference/Conditional-execution
+--
+-- luacheck: pop
 
 local result = {}
 for k = 1, 4 do
diff --git a/test/tarantool-tests/lj-366-strtab-correct-size.test.lua b/test/tarantool-tests/lj-366-strtab-correct-size.test.lua
index a20339e5..b46ace49 100644
--- a/test/tarantool-tests/lj-366-strtab-correct-size.test.lua
+++ b/test/tarantool-tests/lj-366-strtab-correct-size.test.lua
@@ -22,11 +22,15 @@ local utils = require('utils')
 --
 -- $ readelf --symbols xxx.obj
 --
+-- luacheck: push no max comment line length
+--
 -- Symbol table '.symtab' contains 2 entries:
 --    Num:    Value          Size Type    Bind   Vis      Ndx Name
 --      0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
 --      1: 0000000000000000    66 OBJECT  GLOBAL DEFAULT    4 luaJIT_BC_lango_team
 --
+-- luacheck: pop
+--
 -- and to display the information contained in the file's section
 -- headers, if it has any. For our purposes, we are interested in
 -- .symtab section, so other sections are snipped in the output:
@@ -143,7 +147,8 @@ end
 -- and symtab sections.
 local function read_elf(elf_content)
   local ELFobj_type = ffi.typeof(is64 and 'ELF64obj *' or 'ELF32obj *')
-  local ELFsectheader_type = ffi.typeof(is64 and 'ELF64sectheader *' or 'ELF32sectheader *')
+  local ELFsectheader_type = ffi.typeof(is64 and 'ELF64sectheader *' or
+                                        'ELF32sectheader *')
   local elf = ffi.cast(ELFobj_type, elf_content)
   local symtab_hdr, strtab_hdr
   -- Iterate by section headers.
@@ -178,7 +183,8 @@ assert(sym_cnt ~= 0, 'number of symbols is zero')
 test:is(strtab_size, EXPECTED_STRTAB_SIZE, 'check .strtab size')
 test:is(strtab_offset, EXPECTED_STRTAB_OFFSET, 'check .strtab offset')
 
-local strtab_str = string.sub(elf_content, strtab_offset, strtab_offset + strtab_size)
+local strtab_str = string.sub(elf_content, strtab_offset,
+                              strtab_offset + strtab_size)
 
 local strtab_p = ffi.cast('char *', strtab_str)
 local sym_is_found = false
diff --git a/test/tarantool-tests/lj-494-table-chain-infinite-loop.test.lua b/test/tarantool-tests/lj-494-table-chain-infinite-loop.test.lua
index 3dd17e7a..1c1d7128 100644
--- a/test/tarantool-tests/lj-494-table-chain-infinite-loop.test.lua
+++ b/test/tarantool-tests/lj-494-table-chain-infinite-loop.test.lua
@@ -172,6 +172,7 @@ collectgarbage()
 for c, v in pairs(victims) do
     str_hash(v)[0] = orig_hash[c]
 end
-test:ok(true, "table keys collisions are resolved properly (no assertions failed)")
+test:ok(true,
+        "table keys collisions are resolved properly (no assertions failed)")
 
 test:done(true)
diff --git a/test/tarantool-tests/lj-688-snap-ir-rename.test.lua b/test/tarantool-tests/lj-688-snap-ir-rename.test.lua
index 807e0811..5b458cd0 100644
--- a/test/tarantool-tests/lj-688-snap-ir-rename.test.lua
+++ b/test/tarantool-tests/lj-688-snap-ir-rename.test.lua
@@ -41,7 +41,9 @@ jit.opt.start('hotloop=1')
 -- platform-agnostic, there is no real necessity to test it
 -- against ARM/ARM64 separately.
 --
+-- luacheck: push no max comment line length
 -- See also https://drive.google.com/file/d/1iYkFx3F0DOtB9o9ykWfCEm-OdlJNCCL0/view?usp=share_link
+-- luacheck: pop
 
 
 local vals = {-0.1, 0.1, -0.1, 0.1}
diff --git a/test/tarantool-tests/lj-819-fix-missing-uclo.test.lua b/test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
index e2352c92..43a65eb8 100644
--- a/test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
+++ b/test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
@@ -100,7 +100,8 @@ local f = missing_uclo()
 local res = f()
 -- Without a patch we don't get here a function, because upvalue isn't closed
 -- as desirable.
-test:ok(type(res) == 'function', 'virtual machine consistency: type of returned value is correct')
+test:ok(type(res) == 'function',
+        'virtual machine consistency: type of returned value is correct')
 
 -- Make JIT compiler aggressive.
 jit.opt.start('hotloop=1')
@@ -110,6 +111,7 @@ f()
 f = missing_uclo()
 local _
 _, res = pcall(f)
-test:ok(type(res) == 'function', 'consistency on compilation: type of returned value is correct')
+test:ok(type(res) == 'function',
+        'consistency on compilation: type of returned value is correct')
 
 test:done(true)
diff --git a/test/tarantool-tests/lj-918-fma-numerical-accuracy-jit.test.lua b/test/tarantool-tests/lj-918-fma-numerical-accuracy-jit.test.lua
index 8b16d4c3..43c1a4a9 100644
--- a/test/tarantool-tests/lj-918-fma-numerical-accuracy-jit.test.lua
+++ b/test/tarantool-tests/lj-918-fma-numerical-accuracy-jit.test.lua
@@ -22,9 +22,13 @@ local _2pow52 = 2 ^ 52
 -- double rounding. The numbers from the original issue are good
 -- enough.
 --
+-- luacheck: push no max comment line length
+--
 -- [1]:https://developer.arm.com/documentation/dui0801/g/A64-Floating-point-Instructions/FMSUB
 -- [2]:https://en.wikipedia.org/wiki/Multiply%E2%80%93accumulate_operation
 --
+-- luacheck: pop
+--
 -- IEEE754 components to double:
 -- sign * (2 ^ (exp - 1023)) * (mantissa / _2pow52 + normal).
 local a = 1 * (2 ^ (1083 - 1023)) * (4080546448249347 / _2pow52 + 1)
diff --git a/test/tarantool-tests/lj-918-fma-numerical-accuracy.test.lua b/test/tarantool-tests/lj-918-fma-numerical-accuracy.test.lua
index 25b59707..2472f05a 100644
--- a/test/tarantool-tests/lj-918-fma-numerical-accuracy.test.lua
+++ b/test/tarantool-tests/lj-918-fma-numerical-accuracy.test.lua
@@ -20,9 +20,13 @@ local _2pow52 = 2 ^ 52
 -- double rounding. The numbers from the original issue are good
 -- enough.
 --
+-- luacheck: push no max comment line length
+--
 -- [1]:https://developer.arm.com/documentation/dui0801/g/A64-Floating-point-Instructions/FMSUB
 -- [2]:https://en.wikipedia.org/wiki/Multiply%E2%80%93accumulate_operation
 --
+-- luacheck: pop
+--
 -- IEEE754 components to double:
 -- sign * (2 ^ (exp - 1023)) * (mantissa / _2pow52 + normal).
 local a = 1 * (2 ^ (1083 - 1023)) * (4080546448249347 / _2pow52 + 1)
diff --git a/test/tarantool-tests/or-144-gc64-asmref-l.test.lua b/test/tarantool-tests/or-144-gc64-asmref-l.test.lua
index 18c6efb2..cbc13278 100644
--- a/test/tarantool-tests/or-144-gc64-asmref-l.test.lua
+++ b/test/tarantool-tests/or-144-gc64-asmref-l.test.lua
@@ -5,11 +5,15 @@ local test = tap.test('or-144-gc64-asmref-l'):skipcond({
 
 test:plan(1)
 
+-- luacheck: push no max comment line length
+--
 -- Test file to demonstrate LuaJIT `IR_LREF` assembling incorrect
 -- behaviour.
 -- See also:
 -- * https://github.com/openresty/lua-resty-core/issues/144.
 -- * https://www.freelists.org/post/luajit/Consistent-SEGV-on-x64-with-the-latest-LuaJIT-v21-GC64-mode.
+--
+-- luacheck: pop
 
 jit.opt.start('hotloop=1')
 
-- 
2.34.1

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

[Sergey Kaplun via Tarantool-patches]

Hi, Sergey!
Thanks for the patch!
It's a nice idea to automate this part of the review!

Please consider my questions below.

On 11.02.25, Sergey Bronnikov wrote:
> The patch sets a max length with 80 symbols and fixes tests
> exceeding this length.
> ---
> Branch: https://github.com/tarantool/luajit/tree/ligurio/gh-xxxx-set-max-length
> Luacheck configuration file:
> https://luacheck.readthedocs.io/en/stable/config.html
> 
>  .luacheckrc                                            |  5 +++++
>  test/tarantool-tests/fix-argv-handling.test.lua        |  4 ++++
>  .../gh-3196-incorrect-string-length.test.lua           |  3 +++
>  test/tarantool-tests/gh-6163-min-max.test.lua          |  4 ++++
>  .../lj-366-strtab-correct-size.test.lua                | 10 ++++++++--
>  .../lj-494-table-chain-infinite-loop.test.lua          |  3 ++-
>  test/tarantool-tests/lj-688-snap-ir-rename.test.lua    |  2 ++
>  test/tarantool-tests/lj-819-fix-missing-uclo.test.lua  |  6 ++++--
>  .../lj-918-fma-numerical-accuracy-jit.test.lua         |  4 ++++
>  .../lj-918-fma-numerical-accuracy.test.lua             |  4 ++++
>  test/tarantool-tests/or-144-gc64-asmref-l.test.lua     |  4 ++++
>  11 files changed, 44 insertions(+), 5 deletions(-)
> 
> diff --git a/.luacheckrc b/.luacheckrc
> index f2573e42..8047677c 100644
> --- a/.luacheckrc
> +++ b/.luacheckrc
> @@ -3,6 +3,11 @@ std = 'luajit'
>  -- This fork also introduces a new global for misc API namespace.
>  read_globals = { 'misc' }
>  
> +max_line_length = 80

I suppose this is excess since we have all other settings intact.

> +max_code_line_length = 80
> +max_string_line_length = 80

Do we need this particular check? Maybe it is covered by the
`max_code_line_length` and `max_line_length`. Can we avoid markers in
<gh-3196-incorrect-string-length.test.lua> without it?

> +max_comment_line_length = 80

Actually, the comment length for the Lua part is 66 symbols. Maybe we
may omit this check to avoid stubs in the comments?

> +
>  -- The `_TARANTOOL` global is often used for skip condition
>  -- checks in tests.
>  files['test/tarantool-tests/'] = {
> diff --git a/test/tarantool-tests/fix-argv-handling.test.lua b/test/tarantool-tests/fix-argv-handling.test.lua
> index 84b626c3..5f147197 100644
> --- a/test/tarantool-tests/fix-argv-handling.test.lua
> +++ b/test/tarantool-tests/fix-argv-handling.test.lua
> @@ -10,7 +10,11 @@ test:plan(1)
>  -- to a single empty string if it is empty [1], so the issue is
>  -- not reproducible on new kernels.
>  --
> +-- luacheck: push no max comment line length

Side note: Can it will be done without push/pop interface? Like for
globals we use:
| luacheck: no global

> +--
>  -- [1]: https://lore.kernel.org/all/20220201000947.2453721-1-keescook@chromium.org/
> +--
> +-- luacheck: pop
>  
>  local utils = require('utils')
>  local execlib = require('execlib')
> diff --git a/test/tarantool-tests/gh-3196-incorrect-string-length.test.lua b/test/tarantool-tests/gh-3196-incorrect-string-length.test.lua
> index b82029f6..79521608 100644
> --- a/test/tarantool-tests/gh-3196-incorrect-string-length.test.lua
> +++ b/test/tarantool-tests/gh-3196-incorrect-string-length.test.lua
> @@ -6,10 +6,13 @@ test:plan(2)
>  --
>  -- gh-3196: incorrect string length if Lua hash returns 0
>  --
> +-- luacheck: push no max code line length

Side note: Can it will be done without push/pop interface? Like for
globals we use:
| luacheck: no global

> +--
>  local h = "\x1F\x93\xE2\x1C\xCA\xDE\x28\x08\x26\x01\xED\x0A\x2F\xE4\x21\x02\x97\x77\xD9\x3E"
>  test:is(h:len(), 20)
>  
>  h = "\x0F\x93\xE2\x1C\xCA\xDE\x28\x08\x26\x01\xED\x0A\x2F\xE4\x21\x02\x97\x77\xD9\x3E"
>  test:is(h:len(), 20)
> +-- luacheck: pop
>  
>  test:done(true)
> diff --git a/test/tarantool-tests/gh-6163-min-max.test.lua b/test/tarantool-tests/gh-6163-min-max.test.lua
> index e3efd1a4..ca6c2a7f 100644
> --- a/test/tarantool-tests/gh-6163-min-max.test.lua
> +++ b/test/tarantool-tests/gh-6163-min-max.test.lua
> @@ -69,9 +69,13 @@ jit.opt.start('hotloop=1')
>  -- pair is emitted, and the result is `NaN`, which is inconsistent with
>  -- the result of the non-optimized mcode.
>  --
> +-- luacheck: push no max comment line length

Side note: Can it will be done without push/pop interface? Like for
globals we use:
| luacheck: no global

> +--
>  -- [1]: https://developer.arm.com/documentation/dui0801/g/A64-Floating-point-Instructions/FCMP
>  -- [2]: https://developer.arm.com/documentation/100069/0608/A64-Floating-point-Instructions/FCSEL
>  -- [3]: https://developer.arm.com/documentation/dui0068/b/ARM-Instruction-Reference/Conditional-execution
> +--
> +-- luacheck: pop
>  
>  local result = {}
>  for k = 1, 4 do
> diff --git a/test/tarantool-tests/lj-366-strtab-correct-size.test.lua b/test/tarantool-tests/lj-366-strtab-correct-size.test.lua
> index a20339e5..b46ace49 100644
> --- a/test/tarantool-tests/lj-366-strtab-correct-size.test.lua
> +++ b/test/tarantool-tests/lj-366-strtab-correct-size.test.lua
> @@ -22,11 +22,15 @@ local utils = require('utils')
>  --
>  -- $ readelf --symbols xxx.obj
>  --
> +-- luacheck: push no max comment line length
> +--
>  -- Symbol table '.symtab' contains 2 entries:
>  --    Num:    Value          Size Type    Bind   Vis      Ndx Name
>  --      0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
>  --      1: 0000000000000000    66 OBJECT  GLOBAL DEFAULT    4 luaJIT_BC_lango_team
>  --
> +-- luacheck: pop
> +--
>  -- and to display the information contained in the file's section
>  -- headers, if it has any. For our purposes, we are interested in
>  -- .symtab section, so other sections are snipped in the output:
> @@ -143,7 +147,8 @@ end
>  -- and symtab sections.
>  local function read_elf(elf_content)
>    local ELFobj_type = ffi.typeof(is64 and 'ELF64obj *' or 'ELF32obj *')
> -  local ELFsectheader_type = ffi.typeof(is64 and 'ELF64sectheader *' or 'ELF32sectheader *')
> +  local ELFsectheader_type = ffi.typeof(is64 and 'ELF64sectheader *' or
> +                                        'ELF32sectheader *')
>    local elf = ffi.cast(ELFobj_type, elf_content)
>    local symtab_hdr, strtab_hdr
>    -- Iterate by section headers.
> @@ -178,7 +183,8 @@ assert(sym_cnt ~= 0, 'number of symbols is zero')
>  test:is(strtab_size, EXPECTED_STRTAB_SIZE, 'check .strtab size')
>  test:is(strtab_offset, EXPECTED_STRTAB_OFFSET, 'check .strtab offset')
>  
> -local strtab_str = string.sub(elf_content, strtab_offset, strtab_offset + strtab_size)
> +local strtab_str = string.sub(elf_content, strtab_offset,
> +                              strtab_offset + strtab_size)
>  
>  local strtab_p = ffi.cast('char *', strtab_str)
>  local sym_is_found = false
> diff --git a/test/tarantool-tests/lj-494-table-chain-infinite-loop.test.lua b/test/tarantool-tests/lj-494-table-chain-infinite-loop.test.lua
> index 3dd17e7a..1c1d7128 100644
> --- a/test/tarantool-tests/lj-494-table-chain-infinite-loop.test.lua
> +++ b/test/tarantool-tests/lj-494-table-chain-infinite-loop.test.lua
> @@ -172,6 +172,7 @@ collectgarbage()
>  for c, v in pairs(victims) do
>      str_hash(v)[0] = orig_hash[c]
>  end
> -test:ok(true, "table keys collisions are resolved properly (no assertions failed)")
> +test:ok(true,
> +        "table keys collisions are resolved properly (no assertions failed)")
>  
>  test:done(true)
> diff --git a/test/tarantool-tests/lj-688-snap-ir-rename.test.lua b/test/tarantool-tests/lj-688-snap-ir-rename.test.lua
> index 807e0811..5b458cd0 100644
> --- a/test/tarantool-tests/lj-688-snap-ir-rename.test.lua
> +++ b/test/tarantool-tests/lj-688-snap-ir-rename.test.lua
> @@ -41,7 +41,9 @@ jit.opt.start('hotloop=1')
>  -- platform-agnostic, there is no real necessity to test it
>  -- against ARM/ARM64 separately.
>  --
> +-- luacheck: push no max comment line length

Side note: Can it will be done without push/pop interface? Like for
globals we use:
| luacheck: no global

>  -- See also https://drive.google.com/file/d/1iYkFx3F0DOtB9o9ykWfCEm-OdlJNCCL0/view?usp=share_link
> +-- luacheck: pop
>  
>  
>  local vals = {-0.1, 0.1, -0.1, 0.1}
> diff --git a/test/tarantool-tests/lj-819-fix-missing-uclo.test.lua b/test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
> index e2352c92..43a65eb8 100644
> --- a/test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
> +++ b/test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
> @@ -100,7 +100,8 @@ local f = missing_uclo()
>  local res = f()
>  -- Without a patch we don't get here a function, because upvalue isn't closed
>  -- as desirable.
> -test:ok(type(res) == 'function', 'virtual machine consistency: type of returned value is correct')
> +test:ok(type(res) == 'function',
> +        'virtual machine consistency: type of returned value is correct')
>  
>  -- Make JIT compiler aggressive.
>  jit.opt.start('hotloop=1')
> @@ -110,6 +111,7 @@ f()
>  f = missing_uclo()
>  local _
>  _, res = pcall(f)
> -test:ok(type(res) == 'function', 'consistency on compilation: type of returned value is correct')
> +test:ok(type(res) == 'function',
> +        'consistency on compilation: type of returned value is correct')
>  
>  test:done(true)
> diff --git a/test/tarantool-tests/lj-918-fma-numerical-accuracy-jit.test.lua b/test/tarantool-tests/lj-918-fma-numerical-accuracy-jit.test.lua
> index 8b16d4c3..43c1a4a9 100644
> --- a/test/tarantool-tests/lj-918-fma-numerical-accuracy-jit.test.lua
> +++ b/test/tarantool-tests/lj-918-fma-numerical-accuracy-jit.test.lua
> @@ -22,9 +22,13 @@ local _2pow52 = 2 ^ 52
>  -- double rounding. The numbers from the original issue are good
>  -- enough.
>  --
> +-- luacheck: push no max comment line length

Side note: Can it will be done without push/pop interface? Like for
globals we use:
| luacheck: no global

> +--
>  -- [1]:https://developer.arm.com/documentation/dui0801/g/A64-Floating-point-Instructions/FMSUB
>  -- [2]:https://en.wikipedia.org/wiki/Multiply%E2%80%93accumulate_operation
>  --
> +-- luacheck: pop
> +--
>  -- IEEE754 components to double:
>  -- sign * (2 ^ (exp - 1023)) * (mantissa / _2pow52 + normal).
>  local a = 1 * (2 ^ (1083 - 1023)) * (4080546448249347 / _2pow52 + 1)
> diff --git a/test/tarantool-tests/lj-918-fma-numerical-accuracy.test.lua b/test/tarantool-tests/lj-918-fma-numerical-accuracy.test.lua
> index 25b59707..2472f05a 100644
> --- a/test/tarantool-tests/lj-918-fma-numerical-accuracy.test.lua
> +++ b/test/tarantool-tests/lj-918-fma-numerical-accuracy.test.lua
> @@ -20,9 +20,13 @@ local _2pow52 = 2 ^ 52
>  -- double rounding. The numbers from the original issue are good
>  -- enough.
>  --
> +-- luacheck: push no max comment line length

Side note: Can it will be done without push/pop interface? Like for
globals we use:
| luacheck: no global

> +--
>  -- [1]:https://developer.arm.com/documentation/dui0801/g/A64-Floating-point-Instructions/FMSUB
>  -- [2]:https://en.wikipedia.org/wiki/Multiply%E2%80%93accumulate_operation
>  --
> +-- luacheck: pop
> +--
>  -- IEEE754 components to double:
>  -- sign * (2 ^ (exp - 1023)) * (mantissa / _2pow52 + normal).
>  local a = 1 * (2 ^ (1083 - 1023)) * (4080546448249347 / _2pow52 + 1)
> diff --git a/test/tarantool-tests/or-144-gc64-asmref-l.test.lua b/test/tarantool-tests/or-144-gc64-asmref-l.test.lua
> index 18c6efb2..cbc13278 100644
> --- a/test/tarantool-tests/or-144-gc64-asmref-l.test.lua
> +++ b/test/tarantool-tests/or-144-gc64-asmref-l.test.lua
> @@ -5,11 +5,15 @@ local test = tap.test('or-144-gc64-asmref-l'):skipcond({
>  
>  test:plan(1)
>  
> +-- luacheck: push no max comment line length
> +--
>  -- Test file to demonstrate LuaJIT `IR_LREF` assembling incorrect
>  -- behaviour.
>  -- See also:
>  -- * https://github.com/openresty/lua-resty-core/issues/144.
>  -- * https://www.freelists.org/post/luajit/Consistent-SEGV-on-x64-with-the-latest-LuaJIT-v21-GC64-mode.
> +--
> +-- luacheck: pop
>  
>  jit.opt.start('hotloop=1')
>  
> -- 
> 2.34.1
> 

-- 
Best regards,
Sergey Kaplun

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

[Sergey Bronnikov via Tarantool-patches]

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

Hi, Sergey

thanks for review! Please see my comments.

On 11.02.2025 14:51, Sergey Kaplun via Tarantool-patches wrote:
> Hi, Sergey!
> Thanks for the patch!
> It's a nice idea to automate this part of the review!
>
> Please consider my questions below.
>
> On 11.02.25, Sergey Bronnikov wrote:
>> The patch sets a max length with 80 symbols and fixes tests
>> exceeding this length.
>> ---
>> Branch:https://github.com/tarantool/luajit/tree/ligurio/gh-xxxx-set-max-length
>> Luacheck configuration file:
>> https://luacheck.readthedocs.io/en/stable/config.html
>>
>>   .luacheckrc                                            |  5 +++++
>>   test/tarantool-tests/fix-argv-handling.test.lua        |  4 ++++
>>   .../gh-3196-incorrect-string-length.test.lua           |  3 +++
>>   test/tarantool-tests/gh-6163-min-max.test.lua          |  4 ++++
>>   .../lj-366-strtab-correct-size.test.lua                | 10 ++++++++--
>>   .../lj-494-table-chain-infinite-loop.test.lua          |  3 ++-
>>   test/tarantool-tests/lj-688-snap-ir-rename.test.lua    |  2 ++
>>   test/tarantool-tests/lj-819-fix-missing-uclo.test.lua  |  6 ++++--
>>   .../lj-918-fma-numerical-accuracy-jit.test.lua         |  4 ++++
>>   .../lj-918-fma-numerical-accuracy.test.lua             |  4 ++++
>>   test/tarantool-tests/or-144-gc64-asmref-l.test.lua     |  4 ++++
>>   11 files changed, 44 insertions(+), 5 deletions(-)
>>
>> diff --git a/.luacheckrc b/.luacheckrc
>> index f2573e42..8047677c 100644
>> --- a/.luacheckrc
>> +++ b/.luacheckrc
>> @@ -3,6 +3,11 @@ std = 'luajit'
>>   -- This fork also introduces a new global for misc API namespace.
>>   read_globals = { 'misc' }
>>   
>> +max_line_length = 80
> I suppose this is excess since we have all other settings intact.
Removed.
>
>> +max_code_line_length = 80
>> +max_string_line_length = 80
> Do we need this particular check? Maybe it is covered by the
> `max_code_line_length` and `max_line_length`. Can we avoid markers in
> <gh-3196-incorrect-string-length.test.lua> without it?
>
>> +max_comment_line_length = 80
> Actually, the comment length for the Lua part is 66 symbols. Maybe we
> may omit this check to avoid stubs in the comments?

The main idea is automating this check and avoid comments like

"your comment is exceeded XXX symbols" on review. So I don't understand

how to omit this check.

BTW luacheck with "max_comment_line_length = 66" reports 89 warnings, it 
means

that manual (visual) checking of max length in comments is unreliable 
and we definitely

should automate it.

>
>> +
>>   -- The `_TARANTOOL` global is often used for skip condition
>>   -- checks in tests.
>>   files['test/tarantool-tests/'] = {
>> diff --git a/test/tarantool-tests/fix-argv-handling.test.lua b/test/tarantool-tests/fix-argv-handling.test.lua
>> index 84b626c3..5f147197 100644
>> --- a/test/tarantool-tests/fix-argv-handling.test.lua
>> +++ b/test/tarantool-tests/fix-argv-handling.test.lua
>> @@ -10,7 +10,11 @@ test:plan(1)
>>   -- to a single empty string if it is empty [1], so the issue is
>>   -- not reproducible on new kernels.
>>   --
>> +-- luacheck: push no max comment line length
> Side note: Can it will be done without push/pop interface? Like for
> globals we use:
> | luacheck: no global

AFAIR, no. Global suppression will cover the whole file, I don't think

it is a good idea. Approach with push/pop was already used in

commit 9ac79bd7cd12dc3dbfa0e502e29a99b3083676c1
  ("test: introduce test:done TAP helper") why we cannot continue using 
this approach for other files?

>
>> +--
>>   -- [1]:https://lore.kernel.org/all/20220201000947.2453721-1-keescook@chromium.org/
>> +--
>> +-- luacheck: pop
>>   
>>   local utils = require('utils')
>>   local execlib = require('execlib')
>> diff --git a/test/tarantool-tests/gh-3196-incorrect-string-length.test.lua b/test/tarantool-tests/gh-3196-incorrect-string-length.test.lua
>> index b82029f6..79521608 100644
>> --- a/test/tarantool-tests/gh-3196-incorrect-string-length.test.lua
>> +++ b/test/tarantool-tests/gh-3196-incorrect-string-length.test.lua
>> @@ -6,10 +6,13 @@ test:plan(2)
>>   --
>>   -- gh-3196: incorrect string length if Lua hash returns 0
>>   --
>> +-- luacheck: push no max code line length
> Side note: Can it will be done without push/pop interface? Like for
> globals we use:
> | luacheck: no global
See above.
>> +--
>>   local h = "\x1F\x93\xE2\x1C\xCA\xDE\x28\x08\x26\x01\xED\x0A\x2F\xE4\x21\x02\x97\x77\xD9\x3E"
>>   test:is(h:len(), 20)
>>   
>>   h = "\x0F\x93\xE2\x1C\xCA\xDE\x28\x08\x26\x01\xED\x0A\x2F\xE4\x21\x02\x97\x77\xD9\x3E"
>>   test:is(h:len(), 20)
>> +-- luacheck: pop
>>   
>>   test:done(true)
>> diff --git a/test/tarantool-tests/gh-6163-min-max.test.lua b/test/tarantool-tests/gh-6163-min-max.test.lua
>> index e3efd1a4..ca6c2a7f 100644
>> --- a/test/tarantool-tests/gh-6163-min-max.test.lua
>> +++ b/test/tarantool-tests/gh-6163-min-max.test.lua
>> @@ -69,9 +69,13 @@ jit.opt.start('hotloop=1')
>>   -- pair is emitted, and the result is `NaN`, which is inconsistent with
>>   -- the result of the non-optimized mcode.
>>   --
>> +-- luacheck: push no max comment line length
> Side note: Can it will be done without push/pop interface? Like for
> globals we use:
> | luacheck: no global
>
>> +--
>>   -- [1]:https://developer.arm.com/documentation/dui0801/g/A64-Floating-point-Instructions/FCMP
>>   -- [2]:https://developer.arm.com/documentation/100069/0608/A64-Floating-point-Instructions/FCSEL
>>   -- [3]:https://developer.arm.com/documentation/dui0068/b/ARM-Instruction-Reference/Conditional-execution
>> +--
>> +-- luacheck: pop
>>   
>>   local result = {}
>>   for k = 1, 4 do
>> diff --git a/test/tarantool-tests/lj-366-strtab-correct-size.test.lua b/test/tarantool-tests/lj-366-strtab-correct-size.test.lua
>> index a20339e5..b46ace49 100644
>> --- a/test/tarantool-tests/lj-366-strtab-correct-size.test.lua
>> +++ b/test/tarantool-tests/lj-366-strtab-correct-size.test.lua
>> @@ -22,11 +22,15 @@ local utils = require('utils')
>>   --
>>   -- $ readelf --symbols xxx.obj
>>   --
>> +-- luacheck: push no max comment line length
>> +--
>>   -- Symbol table '.symtab' contains 2 entries:
>>   --    Num:    Value          Size Type    Bind   Vis      Ndx Name
>>   --      0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT  UND
>>   --      1: 0000000000000000    66 OBJECT  GLOBAL DEFAULT    4 luaJIT_BC_lango_team
>>   --
>> +-- luacheck: pop
>> +--
>>   -- and to display the information contained in the file's section
>>   -- headers, if it has any. For our purposes, we are interested in
>>   -- .symtab section, so other sections are snipped in the output:
>> @@ -143,7 +147,8 @@ end
>>   -- and symtab sections.
>>   local function read_elf(elf_content)
>>     local ELFobj_type = ffi.typeof(is64 and 'ELF64obj *' or 'ELF32obj *')
>> -  local ELFsectheader_type = ffi.typeof(is64 and 'ELF64sectheader *' or 'ELF32sectheader *')
>> +  local ELFsectheader_type = ffi.typeof(is64 and 'ELF64sectheader *' or
>> +                                        'ELF32sectheader *')
>>     local elf = ffi.cast(ELFobj_type, elf_content)
>>     local symtab_hdr, strtab_hdr
>>     -- Iterate by section headers.
>> @@ -178,7 +183,8 @@ assert(sym_cnt ~= 0, 'number of symbols is zero')
>>   test:is(strtab_size, EXPECTED_STRTAB_SIZE, 'check .strtab size')
>>   test:is(strtab_offset, EXPECTED_STRTAB_OFFSET, 'check .strtab offset')
>>   
>> -local strtab_str = string.sub(elf_content, strtab_offset, strtab_offset + strtab_size)
>> +local strtab_str = string.sub(elf_content, strtab_offset,
>> +                              strtab_offset + strtab_size)
>>   
>>   local strtab_p = ffi.cast('char *', strtab_str)
>>   local sym_is_found = false
>> diff --git a/test/tarantool-tests/lj-494-table-chain-infinite-loop.test.lua b/test/tarantool-tests/lj-494-table-chain-infinite-loop.test.lua
>> index 3dd17e7a..1c1d7128 100644
>> --- a/test/tarantool-tests/lj-494-table-chain-infinite-loop.test.lua
>> +++ b/test/tarantool-tests/lj-494-table-chain-infinite-loop.test.lua
>> @@ -172,6 +172,7 @@ collectgarbage()
>>   for c, v in pairs(victims) do
>>       str_hash(v)[0] = orig_hash[c]
>>   end
>> -test:ok(true, "table keys collisions are resolved properly (no assertions failed)")
>> +test:ok(true,
>> +        "table keys collisions are resolved properly (no assertions failed)")
>>   
>>   test:done(true)
>> diff --git a/test/tarantool-tests/lj-688-snap-ir-rename.test.lua b/test/tarantool-tests/lj-688-snap-ir-rename.test.lua
>> index 807e0811..5b458cd0 100644
>> --- a/test/tarantool-tests/lj-688-snap-ir-rename.test.lua
>> +++ b/test/tarantool-tests/lj-688-snap-ir-rename.test.lua
>> @@ -41,7 +41,9 @@ jit.opt.start('hotloop=1')
>>   -- platform-agnostic, there is no real necessity to test it
>>   -- against ARM/ARM64 separately.
>>   --
>> +-- luacheck: push no max comment line length
> Side note: Can it will be done without push/pop interface? Like for
> globals we use:
> | luacheck: no global
>
>>   -- See alsohttps://drive.google.com/file/d/1iYkFx3F0DOtB9o9ykWfCEm-OdlJNCCL0/view?usp=share_link
>> +-- luacheck: pop
>>   
>>   
>>   local vals = {-0.1, 0.1, -0.1, 0.1}
>> diff --git a/test/tarantool-tests/lj-819-fix-missing-uclo.test.lua b/test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
>> index e2352c92..43a65eb8 100644
>> --- a/test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
>> +++ b/test/tarantool-tests/lj-819-fix-missing-uclo.test.lua
>> @@ -100,7 +100,8 @@ local f = missing_uclo()
>>   local res = f()
>>   -- Without a patch we don't get here a function, because upvalue isn't closed
>>   -- as desirable.
>> -test:ok(type(res) == 'function', 'virtual machine consistency: type of returned value is correct')
>> +test:ok(type(res) == 'function',
>> +        'virtual machine consistency: type of returned value is correct')
>>   
>>   -- Make JIT compiler aggressive.
>>   jit.opt.start('hotloop=1')
>> @@ -110,6 +111,7 @@ f()
>>   f = missing_uclo()
>>   local _
>>   _, res = pcall(f)
>> -test:ok(type(res) == 'function', 'consistency on compilation: type of returned value is correct')
>> +test:ok(type(res) == 'function',
>> +        'consistency on compilation: type of returned value is correct')
>>   
>>   test:done(true)
>> diff --git a/test/tarantool-tests/lj-918-fma-numerical-accuracy-jit.test.lua b/test/tarantool-tests/lj-918-fma-numerical-accuracy-jit.test.lua
>> index 8b16d4c3..43c1a4a9 100644
>> --- a/test/tarantool-tests/lj-918-fma-numerical-accuracy-jit.test.lua
>> +++ b/test/tarantool-tests/lj-918-fma-numerical-accuracy-jit.test.lua
>> @@ -22,9 +22,13 @@ local _2pow52 = 2 ^ 52
>>   -- double rounding. The numbers from the original issue are good
>>   -- enough.
>>   --
>> +-- luacheck: push no max comment line length
> Side note: Can it will be done without push/pop interface? Like for
> globals we use:
> | luacheck: no global
>
>> +--
>>   -- [1]:https://developer.arm.com/documentation/dui0801/g/A64-Floating-point-Instructions/FMSUB
>>   -- [2]:https://en.wikipedia.org/wiki/Multiply%E2%80%93accumulate_operation
>>   --
>> +-- luacheck: pop
>> +--
>>   -- IEEE754 components to double:
>>   -- sign * (2 ^ (exp - 1023)) * (mantissa / _2pow52 + normal).
>>   local a = 1 * (2 ^ (1083 - 1023)) * (4080546448249347 / _2pow52 + 1)
>> diff --git a/test/tarantool-tests/lj-918-fma-numerical-accuracy.test.lua b/test/tarantool-tests/lj-918-fma-numerical-accuracy.test.lua
>> index 25b59707..2472f05a 100644
>> --- a/test/tarantool-tests/lj-918-fma-numerical-accuracy.test.lua
>> +++ b/test/tarantool-tests/lj-918-fma-numerical-accuracy.test.lua
>> @@ -20,9 +20,13 @@ local _2pow52 = 2 ^ 52
>>   -- double rounding. The numbers from the original issue are good
>>   -- enough.
>>   --
>> +-- luacheck: push no max comment line length
> Side note: Can it will be done without push/pop interface? Like for
> globals we use:
> | luacheck: no global
>
>> +--
>>   -- [1]:https://developer.arm.com/documentation/dui0801/g/A64-Floating-point-Instructions/FMSUB
>>   -- [2]:https://en.wikipedia.org/wiki/Multiply%E2%80%93accumulate_operation
>>   --
>> +-- luacheck: pop
>> +--
>>   -- IEEE754 components to double:
>>   -- sign * (2 ^ (exp - 1023)) * (mantissa / _2pow52 + normal).
>>   local a = 1 * (2 ^ (1083 - 1023)) * (4080546448249347 / _2pow52 + 1)
>> diff --git a/test/tarantool-tests/or-144-gc64-asmref-l.test.lua b/test/tarantool-tests/or-144-gc64-asmref-l.test.lua
>> index 18c6efb2..cbc13278 100644
>> --- a/test/tarantool-tests/or-144-gc64-asmref-l.test.lua
>> +++ b/test/tarantool-tests/or-144-gc64-asmref-l.test.lua
>> @@ -5,11 +5,15 @@ local test = tap.test('or-144-gc64-asmref-l'):skipcond({
>>   
>>   test:plan(1)
>>   
>> +-- luacheck: push no max comment line length
>> +--
>>   -- Test file to demonstrate LuaJIT `IR_LREF` assembling incorrect
>>   -- behaviour.
>>   -- See also:
>>   -- *https://github.com/openresty/lua-resty-core/issues/144.
>>   -- *https://www.freelists.org/post/luajit/Consistent-SEGV-on-x64-with-the-latest-LuaJIT-v21-GC64-mode.
>> +--
>> +-- luacheck: pop
>>   
>>   jit.opt.start('hotloop=1')
>>   
>> -- 
>> 2.34.1
>>

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

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

[Sergey Kaplun via Tarantool-patches]

Hi, Sergey!
Thanks for the clarification.
See my answers below.

On 12.02.25, Sergey Bronnikov wrote:
> Hi, Sergey
> 
> thanks for review! Please see my comments.
> 
> On 11.02.2025 14:51, Sergey Kaplun via Tarantool-patches wrote:
> > Hi, Sergey!
> > Thanks for the patch!
> > It's a nice idea to automate this part of the review!
> >
> > Please consider my questions below.
> >
> > On 11.02.25, Sergey Bronnikov wrote:
> >> The patch sets a max length with 80 symbols and fixes tests
> >> exceeding this length.
> >> ---
> >> Branch:https://github.com/tarantool/luajit/tree/ligurio/gh-xxxx-set-max-length
> >> Luacheck configuration file:
> >> https://luacheck.readthedocs.io/en/stable/config.html
> >>
> >>   .luacheckrc                                            |  5 +++++
> >>   test/tarantool-tests/fix-argv-handling.test.lua        |  4 ++++
> >>   .../gh-3196-incorrect-string-length.test.lua           |  3 +++
> >>   test/tarantool-tests/gh-6163-min-max.test.lua          |  4 ++++
> >>   .../lj-366-strtab-correct-size.test.lua                | 10 ++++++++--
> >>   .../lj-494-table-chain-infinite-loop.test.lua          |  3 ++-
> >>   test/tarantool-tests/lj-688-snap-ir-rename.test.lua    |  2 ++
> >>   test/tarantool-tests/lj-819-fix-missing-uclo.test.lua  |  6 ++++--
> >>   .../lj-918-fma-numerical-accuracy-jit.test.lua         |  4 ++++
> >>   .../lj-918-fma-numerical-accuracy.test.lua             |  4 ++++
> >>   test/tarantool-tests/or-144-gc64-asmref-l.test.lua     |  4 ++++
> >>   11 files changed, 44 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/.luacheckrc b/.luacheckrc
> >> index f2573e42..8047677c 100644
> >> --- a/.luacheckrc
> >> +++ b/.luacheckrc
> >> @@ -3,6 +3,11 @@ std = 'luajit'
> >>   -- This fork also introduces a new global for misc API namespace.
> >>   read_globals = { 'misc' }
> >>   
> >> +max_line_length = 80
> > I suppose this is excess since we have all other settings intact.
> Removed.
> >
> >> +max_code_line_length = 80
> >> +max_string_line_length = 80
> > Do we need this particular check? Maybe it is covered by the
> > `max_code_line_length` and `max_line_length`. Can we avoid markers in
> > <gh-3196-incorrect-string-length.test.lua> without it?
> >
> >> +max_comment_line_length = 80
> > Actually, the comment length for the Lua part is 66 symbols. Maybe we
> > may omit this check to avoid stubs in the comments?
> 
> The main idea is automating this check and avoid comments like
> "your comment is exceeded XXX symbols" on review. So I don't understand
> how to omit this check.

Sorry, I misled you with bad wording.
Originally, according to the commit message. I think that we would
change the limit to 80 symbols for all code (and check comments manually
or don't check them at all). But since we want to do it fully
automated,
we should use the following 2 lines:
| max_code_line_length = 80
| max_comment_line_length = 66

Or we still should check manually the comment line length on the review.

We don't need `max_line_length`, since we distinguish lines with/without
comments.
And we don't need string lines, I suppose. At least I don't fully
understand why we need them.

> 
> BTW luacheck with "max_comment_line_length = 66" reports 89 warnings, it 
> means

Yep, totally 118 warnings for the code we want to check.

> 
> that manual (visual) checking of max length in comments is unreliable 
> and we definitely
> should automate it.

Totally agree.

> 
> >
> >> +
> >>   -- The `_TARANTOOL` global is often used for skip condition
> >>   -- checks in tests.
> >>   files['test/tarantool-tests/'] = {
> >> diff --git a/test/tarantool-tests/fix-argv-handling.test.lua b/test/tarantool-tests/fix-argv-handling.test.lua
> >> index 84b626c3..5f147197 100644
> >> --- a/test/tarantool-tests/fix-argv-handling.test.lua
> >> +++ b/test/tarantool-tests/fix-argv-handling.test.lua
> >> @@ -10,7 +10,11 @@ test:plan(1)
> >>   -- to a single empty string if it is empty [1], so the issue is
> >>   -- not reproducible on new kernels.
> >>   --
> >> +-- luacheck: push no max comment line length
> > Side note: Can it will be done without push/pop interface? Like for
> > globals we use:
> > | luacheck: no global
> 
> AFAIR, no. Global suppression will cover the whole file, I don't think
> 
> it is a good idea. Approach with push/pop was already used in
> 
> commit 9ac79bd7cd12dc3dbfa0e502e29a99b3083676c1
>   ("test: introduce test:done TAP helper") why we cannot continue using 
> this approach for other files?

I'm not against it. I just prefer the inline suppressions.
Actually we _may_ use
| -- luacheck: no max_comment_line_length
But this is a global (the whole file below this line) suppression.
I thought that this suppresses warnings until the next line of code, like
ignore does. BTW,
| -- luacheck: ignore
does exactly what I want, but unfortunately it may not be clear what
exactly we ignore :).

But this is a matter of taste, so feel free to choose any option here.

> 
> >
> >> +--
> >>   -- [1]:https://lore.kernel.org/all/20220201000947.2453721-1-keescook@chromium.org/
> >> +--
> >> +-- luacheck: pop
> >>   

<snipped>

> >> -- 
> >> 2.34.1
> >>

-- 
Best regards,
Sergey Kaplun

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

[Sergey Bronnikov via Tarantool-patches]

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

Hi, Sergey!

On 12.02.2025 10:41, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the clarification.
> See my answers below.
>
> On 12.02.25, Sergey Bronnikov wrote:
>> Hi, Sergey
>>
>> thanks for review! Please see my comments.
>>
>> On 11.02.2025 14:51, Sergey Kaplun via Tarantool-patches wrote:
>>> Hi, Sergey!
>>> Thanks for the patch!
>>> It's a nice idea to automate this part of the review!
>>>
>>> Please consider my questions below.
>>>
>>> On 11.02.25, Sergey Bronnikov wrote:
>>>> The patch sets a max length with 80 symbols and fixes tests
>>>> exceeding this length.
>>>> ---
>>>> Branch:https://github.com/tarantool/luajit/tree/ligurio/gh-xxxx-set-max-length
>>>> Luacheck configuration file:
>>>> https://luacheck.readthedocs.io/en/stable/config.html
>>>>
>>>>    .luacheckrc                                            |  5 +++++
>>>>    test/tarantool-tests/fix-argv-handling.test.lua        |  4 ++++
>>>>    .../gh-3196-incorrect-string-length.test.lua           |  3 +++
>>>>    test/tarantool-tests/gh-6163-min-max.test.lua          |  4 ++++
>>>>    .../lj-366-strtab-correct-size.test.lua                | 10 ++++++++--
>>>>    .../lj-494-table-chain-infinite-loop.test.lua          |  3 ++-
>>>>    test/tarantool-tests/lj-688-snap-ir-rename.test.lua    |  2 ++
>>>>    test/tarantool-tests/lj-819-fix-missing-uclo.test.lua  |  6 ++++--
>>>>    .../lj-918-fma-numerical-accuracy-jit.test.lua         |  4 ++++
>>>>    .../lj-918-fma-numerical-accuracy.test.lua             |  4 ++++
>>>>    test/tarantool-tests/or-144-gc64-asmref-l.test.lua     |  4 ++++
>>>>    11 files changed, 44 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/.luacheckrc b/.luacheckrc
>>>> index f2573e42..8047677c 100644
>>>> --- a/.luacheckrc
>>>> +++ b/.luacheckrc
>>>> @@ -3,6 +3,11 @@ std = 'luajit'
>>>>    -- This fork also introduces a new global for misc API namespace.
>>>>    read_globals = { 'misc' }
>>>>    
>>>> +max_line_length = 80
>>> I suppose this is excess since we have all other settings intact.
>> Removed.
>>>> +max_code_line_length = 80
>>>> +max_string_line_length = 80
>>> Do we need this particular check? Maybe it is covered by the
>>> `max_code_line_length` and `max_line_length`. Can we avoid markers in
>>> <gh-3196-incorrect-string-length.test.lua> without it?
>>>
>>>> +max_comment_line_length = 80
>>> Actually, the comment length for the Lua part is 66 symbols. Maybe we
>>> may omit this check to avoid stubs in the comments?
>> The main idea is automating this check and avoid comments like
>> "your comment is exceeded XXX symbols" on review. So I don't understand
>> how to omit this check.
> Sorry, I misled you with bad wording.
> Originally, according to the commit message. I think that we would
> change the limit to 80 symbols for all code (and check comments manually
> or don't check them at all). But since we want to do it fully
> automated,
> we should use the following 2 lines:
> | max_code_line_length = 80
> | max_comment_line_length = 66
Well, did it in updated patch.
>
> Or we still should check manually the comment line length on the review.
>
> We don't need `max_line_length`, since we distinguish lines with/without
> comments.
> And we don't need string lines, I suppose. At least I don't fully
> understand why we need them.
both removed from config.
>
>> BTW luacheck with "max_comment_line_length = 66" reports 89 warnings, it
>> means
> Yep, totally 118 warnings for the code we want to check.

All warnings were fixed and patch force-pushed to the branch.

See v2 version in ML.

>
>> that manual (visual) checking of max length in comments is unreliable
>> and we definitely
>> should automate it.
> Totally agree.
>
>>>> +
>>>>    -- The `_TARANTOOL` global is often used for skip condition
>>>>    -- checks in tests.
>>>>    files['test/tarantool-tests/'] = {
>>>> diff --git a/test/tarantool-tests/fix-argv-handling.test.lua b/test/tarantool-tests/fix-argv-handling.test.lua
>>>> index 84b626c3..5f147197 100644
>>>> --- a/test/tarantool-tests/fix-argv-handling.test.lua
>>>> +++ b/test/tarantool-tests/fix-argv-handling.test.lua
>>>> @@ -10,7 +10,11 @@ test:plan(1)
>>>>    -- to a single empty string if it is empty [1], so the issue is
>>>>    -- not reproducible on new kernels.
>>>>    --
>>>> +-- luacheck: push no max comment line length
>>> Side note: Can it will be done without push/pop interface? Like for
>>> globals we use:
>>> | luacheck: no global
>> AFAIR, no. Global suppression will cover the whole file, I don't think
>>
>> it is a good idea. Approach with push/pop was already used in
>>
>> commit 9ac79bd7cd12dc3dbfa0e502e29a99b3083676c1
>>    ("test: introduce test:done TAP helper") why we cannot continue using
>> this approach for other files?
> I'm not against it. I just prefer the inline suppressions.
> Actually we _may_ use
> | -- luacheck: no max_comment_line_length
> But this is a global (the whole file below this line) suppression.
> I thought that this suppresses warnings until the next line of code, like
> ignore does. BTW,
> | -- luacheck: ignore
> does exactly what I want, but unfortunately it may not be clear what
> exactly we ignore :).
>
> But this is a matter of taste, so feel free to choose any option here.
>
Discussed it in a private conversation and decided a variant with push/pop

and a warning name with underscores instead whitespaces.

>>>> +--
>>>>    -- [1]:https://lore.kernel.org/all/20220201000947.2453721-1-keescook@chromium.org/
>>>> +--
>>>> +-- luacheck: pop
>>>>    
> <snipped>
>
>>>> -- 
>>>> 2.34.1
>>>>

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