<!DOCTYPE html>
<html data-lt-installed="true">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body style="padding-bottom: 1px;">
<p>Hi, Sergey</p>
<p>thanks for review! Please see my comments.<br>
</p>
<div class="moz-cite-prefix">On 11.02.2025 14:51, Sergey Kaplun via
Tarantool-patches wrote:<br>
</div>
<blockquote type="cite" cite="mid:Z6s5pnii2imXHzfg@root">
<pre class="moz-quote-pre" wrap="">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:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">The patch sets a max length with 80 symbols and fixes tests
exceeding this length.
---
Branch: <a class="moz-txt-link-freetext" href="https://github.com/tarantool/luajit/tree/ligurio/gh-xxxx-set-max-length">https://github.com/tarantool/luajit/tree/ligurio/gh-xxxx-set-max-length</a>
Luacheck configuration file:
<a class="moz-txt-link-freetext" href="https://luacheck.readthedocs.io/en/stable/config.html">https://luacheck.readthedocs.io/en/stable/config.html</a>
.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
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
I suppose this is excess since we have all other settings intact.</pre>
</blockquote>
Removed.<br>
<blockquote type="cite" cite="mid:Z6s5pnii2imXHzfg@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+max_code_line_length = 80
+max_string_line_length = 80
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
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?
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+max_comment_line_length = 80
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Actually, the comment length for the Lua part is 66 symbols. Maybe we
may omit this check to avoid stubs in the comments?</pre>
</blockquote>
<p>The main idea is automating this check and avoid comments like</p>
<p>"your comment is exceeded XXX symbols" on review. So I don't
understand</p>
<p>how to omit this check.</p>
<p>BTW luacheck with "max_comment_line_length = 66" reports 89
warnings, it means</p>
<p>that manual (visual) checking of max length in comments is
unreliable and we definitely</p>
<p>should automate it.<br>
</p>
<blockquote type="cite" cite="mid:Z6s5pnii2imXHzfg@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+
-- 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
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Side note: Can it will be done without push/pop interface? Like for
globals we use:
| luacheck: no global</pre>
</blockquote>
<p>AFAIR, no. Global suppression will cover the whole file, I don't
think</p>
<p>it is a good idea. Approach with push/pop was already used in</p>
<p>commit 9ac79bd7cd12dc3dbfa0e502e29a99b3083676c1<br>
("test: introduce test:done TAP helper") why we cannot continue
using this approach for other files?<br>
</p>
<blockquote type="cite" cite="mid:Z6s5pnii2imXHzfg@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+--
-- [1]: <a class="moz-txt-link-freetext" href="https://lore.kernel.org/all/20220201000947.2453721-1-keescook@chromium.org/">https://lore.kernel.org/all/20220201000947.2453721-1-keescook@chromium.org/</a>
+--
+-- 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
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Side note: Can it will be done without push/pop interface? Like for
globals we use:
| luacheck: no global
</pre>
</blockquote>
See above.<br>
<blockquote type="cite" cite="mid:Z6s5pnii2imXHzfg@root">
<pre class="moz-quote-pre" wrap="">
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+--
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
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Side note: Can it will be done without push/pop interface? Like for
globals we use:
| luacheck: no global
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+--
-- [1]: <a class="moz-txt-link-freetext" href="https://developer.arm.com/documentation/dui0801/g/A64-Floating-point-Instructions/FCMP">https://developer.arm.com/documentation/dui0801/g/A64-Floating-point-Instructions/FCMP</a>
-- [2]: <a class="moz-txt-link-freetext" href="https://developer.arm.com/documentation/100069/0608/A64-Floating-point-Instructions/FCSEL">https://developer.arm.com/documentation/100069/0608/A64-Floating-point-Instructions/FCSEL</a>
-- [3]: <a class="moz-txt-link-freetext" href="https://developer.arm.com/documentation/dui0068/b/ARM-Instruction-Reference/Conditional-execution">https://developer.arm.com/documentation/dui0068/b/ARM-Instruction-Reference/Conditional-execution</a>
+--
+-- 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
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Side note: Can it will be done without push/pop interface? Like for
globals we use:
| luacheck: no global
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap=""> -- See also <a class="moz-txt-link-freetext" href="https://drive.google.com/file/d/1iYkFx3F0DOtB9o9ykWfCEm-OdlJNCCL0/view?usp=share_link">https://drive.google.com/file/d/1iYkFx3F0DOtB9o9ykWfCEm-OdlJNCCL0/view?usp=share_link</a>
+-- 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
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Side note: Can it will be done without push/pop interface? Like for
globals we use:
| luacheck: no global
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+--
-- [1]:<a class="moz-txt-link-freetext" href="https://developer.arm.com/documentation/dui0801/g/A64-Floating-point-Instructions/FMSUB">https://developer.arm.com/documentation/dui0801/g/A64-Floating-point-Instructions/FMSUB</a>
-- [2]:<a class="moz-txt-link-freetext" href="https://en.wikipedia.org/wiki/Multiply%E2%80%93accumulate_operation">https://en.wikipedia.org/wiki/Multiply%E2%80%93accumulate_operation</a>
--
+-- 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
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Side note: Can it will be done without push/pop interface? Like for
globals we use:
| luacheck: no global
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+--
-- [1]:<a class="moz-txt-link-freetext" href="https://developer.arm.com/documentation/dui0801/g/A64-Floating-point-Instructions/FMSUB">https://developer.arm.com/documentation/dui0801/g/A64-Floating-point-Instructions/FMSUB</a>
-- [2]:<a class="moz-txt-link-freetext" href="https://en.wikipedia.org/wiki/Multiply%E2%80%93accumulate_operation">https://en.wikipedia.org/wiki/Multiply%E2%80%93accumulate_operation</a>
--
+-- 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:
-- * <a class="moz-txt-link-freetext" href="https://github.com/openresty/lua-resty-core/issues/144">https://github.com/openresty/lua-resty-core/issues/144</a>.
-- * <a class="moz-txt-link-freetext" href="https://www.freelists.org/post/luajit/Consistent-SEGV-on-x64-with-the-latest-LuaJIT-v21-GC64-mode">https://www.freelists.org/post/luajit/Consistent-SEGV-on-x64-with-the-latest-LuaJIT-v21-GC64-mode</a>.
+--
+-- luacheck: pop
jit.opt.start('hotloop=1')
--
2.34.1
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
</pre>
</blockquote>
</body>
<lt-container></lt-container>
</html>