Skip to content

Commit

Permalink
test: limit code and comment max length
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ligurio committed Feb 12, 2025
1 parent b98550d commit 9f6e14a
Show file tree
Hide file tree
Showing 36 changed files with 206 additions and 113 deletions.
3 changes: 3 additions & 0 deletions .luacheckrc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,9 @@ std = 'luajit'
-- This fork also introduces a new global for misc API namespace.
read_globals = { 'misc' }

max_code_line_length = 80
max_comment_line_length = 66

-- The `_TARANTOOL` global is often used for skip condition
-- checks in tests.
files['test/tarantool-tests/'] = {
Expand Down
4 changes: 4 additions & 0 deletions test/tarantool-tests/fix-argv-handling.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -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/[email protected]/
--
-- luacheck: pop

local utils = require('utils')
local execlib = require('execlib')
Expand Down
2 changes: 2 additions & 0 deletions test/tarantool-tests/fix-binary-number-parsing.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ local tap = require('tap')
-- Test file to demonstrate incorrect behaviour of binary number
-- parsing with fractional dot.
-- See also:
-- luacheck: push no max_comment_line_length
-- https://www.freelists.org/post/luajit/Fractional-binary-number-literals
-- luacheck: pop
local test = tap.test('fix-binary-number-parsing')
test:plan(2)

Expand Down
3 changes: 3 additions & 0 deletions test/tarantool-tests/gh-3196-incorrect-string-length.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,13 @@ test:plan(2)
--
-- gh-3196: incorrect string length if Lua hash returns 0
--
-- luacheck: push no max_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)
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ local tap = require('tap')
local test = tap.test("Tarantool 4773")
test:plan(3)

-- Test file to demonstrate LuaJIT tonumber routine fails on NUL char,
-- details:
-- Test file to demonstrate LuaJIT tonumber routine fails on NUL
-- char, details:
-- https://github.com/tarantool/tarantool/issues/4773

local t = {
Expand All @@ -13,8 +13,9 @@ local t = {
tail = 'imun',
}

-- Since VM, Lua/C API and compiler use a single routine for conversion numeric
-- string to a number, test cases are reduced to the following:
-- Since VM, Lua/C API and compiler use a single routine for
-- conversion numeric string to a number, test cases are reduced
-- to the following:
test:is(tonumber(t.zero), 0)
test:is(tonumber(t.zero .. t.tail), nil)
test:is(tonumber(t.zero .. t.null .. t.tail), nil)
Expand Down
68 changes: 42 additions & 26 deletions test/tarantool-tests/gh-6163-min-max.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -45,33 +45,37 @@ jit.opt.start('hotloop=1')
-- copy-pasted to preserve optimization semantics.

-- Without the `(a o b) o a ==> a o b` fold optimization for
-- `math.min()/math.max()` the following mcode is emitted on aarch64
-- for the `math.min(math.min(x, nan), x)` expression:
-- `math.min()/math.max()` the following mcode is emitted on
-- aarch64 for the `math.min(math.min(x, nan), x)` expression:
--
-- | fcmp d2, d3 ; fcmp 1.0, nan
-- | fcsel d1, d2, d3, cc ; d1 == nan after this instruction
-- | ...
-- | fcmp d1, d2 ; fcmp nan, 1.0
-- | fcsel d0, d1, d2, cc ; d0 == 1.0 after this instruction
--
-- According to the `fcmp` docs[1], if either of the operands is NaN,
-- then the operands are unordered. It results in the following state
-- of the flags register: N=0, Z=0, C=1, V=1
-- According to the `fcmp` docs[1], if either of the operands is
-- NaN, then the operands are unordered. It results in the
-- following state of the flags register: N=0, Z=0, C=1, V=1
--
-- According to the `fcsel` docs[2], if the condition is met, then
-- the first register value is taken, otherwise -- the second.
-- In our case, the condition is cc, which means that the `C` flag
-- should be clear[3], which is false. Then, the second value is taken,
-- which is `NaN` for the first `fcmp`-`fcsel` pair, and `1.0` for
-- the second.
-- should be clear[3], which is false. Then, the second value is
-- taken, which is `NaN` for the first `fcmp`-`fcsel` pair, and
-- `1.0` for the second.
--
-- If that fold optimization is applied, then only the first `fcmp`-`fcsel`
-- pair is emitted, and the result is `NaN`, which is inconsistent with
-- the result of the non-optimized mcode.
-- If that fold optimization is applied, then only the first
-- `fcmp`-`fcsel` 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
Expand All @@ -87,19 +91,20 @@ end
-- expected: 1 1 1 1
test:samevalues(result, 'math.max: reassoc_dup')

-- If one gets the expression like `math.min(x, math.min(x, nan))`,
-- and the `comm_dup` optimization is applied, it results in the
-- same situation as explained above. With the `comm_dup_minmax`
-- there is no swap, hence, everything is consistent again:
-- If one gets the expression like
-- `math.min(x, math.min(x, nan))`, and the `comm_dup`
-- optimization is applied, it results in the same situation as
-- explained above. With the `comm_dup_minmax` there is no swap,
-- hence, everything is consistent again:
--
-- | fcmp d2, d3 ; fcmp 1.0, nan
-- | fcsel d1, d3, d2, pl ; d1 == nan after this instruction
-- | ...
-- | fcmp d2, d1 ; fcmp 1.0, nan
-- | fcsel d0, d1, d2, pl ; d0 == nan after this instruction
-- `pl` (aka `CC_PL`) condition means that N flag is 0 [2], that
-- is true when we are comparing something with NaN. So, the value of the
-- first source register is taken
-- is true when we are comparing something with NaN. So, the value
-- of the first source register is taken

result = {}
for k = 1, 4 do
Expand Down Expand Up @@ -193,8 +198,10 @@ end
-- expected: nan nan nan nan
test:samevalues(result, 'max-min-case2: reassoc_minmax_right')

-- XXX: If we look into the disassembled code of `lj_vm_foldarith()`
-- we can see the following:
-- XXX: If we look into the disassembled code of
-- `lj_vm_foldarith()` we can see the following:
--
-- luacheck: push no max_comment_line_length
--
-- | /* In our test x == 7.1, y == nan */
-- | case IR_MIN - IR_ADD: return x > y ? y : x; break;
Expand All @@ -208,10 +215,12 @@ test:samevalues(result, 'max-min-case2: reassoc_minmax_right')
-- | <lj_vm_foldarith+358>: mov rax,QWORD PTR [rsp+0x18] ; else return 7.1
-- | <lj_vm_foldarith+363>: jmp <lj_vm_foldarith+398> ;
--
-- According to `comisd` documentation [4] in case when one of the operands
-- is NaN, the result is unordered and ZF,PF,CF := 111. This means that `jbe`
-- condition is true (CF=1 or ZF=1)[5], so we return 7.1 (the first
-- operand) for case `IR_MIN`.
-- luacheck: pop
--
-- According to `comisd` documentation [4] in case when one of the
-- operands is NaN, the result is unordered and ZF,PF,CF := 111.
-- This means that `jbe` condition is true (CF=1 or ZF=1)[5], so
-- we return 7.1 (the first operand) for case `IR_MIN`.
--
-- However, in `lj_ff_math_min()` in the VM we see the following:
-- |7:
Expand All @@ -221,7 +230,11 @@ test:samevalues(result, 'max-min-case2: reassoc_minmax_right')
-- either a NaN or a valid floating-point value, is
-- written to the result.
--
-- So the patch changes the `lj_vm_foldairth()` assembly in the following way:
-- So the patch changes the `lj_vm_foldairth()` assembly in the
-- following way:
--
-- luacheck: push no max_comment_line_length
--
-- | ; case IR_MIN
-- | <lj_vm_foldarith+337>: movsd xmm0,QWORD PTR [rsp+0x10] ; xmm0 <- nan
-- | <lj_vm_foldarith+343>: comisd xmm0,QWORD PTR [rsp+0x18] ; comisd nan, 7.1
Expand All @@ -231,10 +244,13 @@ test:samevalues(result, 'max-min-case2: reassoc_minmax_right')
-- | <lj_vm_foldarith+358>: mov rax,QWORD PTR [rsp+0x10] ; else return nan
-- | <lj_vm_foldarith+363>: jmp <lj_vm_foldarith+398> ;
--
-- luacheck: pop
--
-- So now we always return the second operand.
--
-- XXX: The two tests below use the `0/0` constant instead of `nan`
-- variable is dictated by the `fold_kfold_numarith` semantics.
-- XXX: The two tests below use the `0/0` constant instead of
-- `nan` variable is dictated by the `fold_kfold_numarith`
-- semantics.
result = {}
for k = 1, 4 do
result[k] = min(min(7.1, 0/0), 1.1)
Expand Down
3 changes: 2 additions & 1 deletion test/tarantool-tests/gh-7745-oom-on-trace.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,8 @@ local test = tap.test('OOM on trace'):skipcond({

test:plan(1)

-- NB: When GC64 is enabled, fails with TABOV, otherwise -- with OOM.
-- NB: When GC64 is enabled, fails with TABOV, otherwise -- with
-- OOM.
local function memory_payload()
local t = {} -- luacheck: no unused
for i = 1, 1e10 do
Expand Down
6 changes: 3 additions & 3 deletions test/tarantool-tests/lj-1004-oom-error-frame.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ local function eatchunks(size)
end
end

-- The chunk size below is empirical. It is big enough, so the test
-- is not too long, yet small enough for the OOM frame issue to have
-- enough iterations in the second loop to trigger.
-- The chunk size below is empirical. It is big enough, so the
-- test is not too long, yet small enough for the OOM frame issue
-- to have enough iterations in the second loop to trigger.
pcall(eatchunks, 512 * 1024 * 1024)

local anchor = {}
Expand Down
2 changes: 2 additions & 0 deletions test/tarantool-tests/lj-1116-redzones-checks.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ jit.opt.start('hotloop=1')
-- This makes this reproducer unstable (regarding the register
-- allocator changes). So, lets use this as a regression test.
--
-- luacheck: push no max_comment_line_length
-- [1]: https://wiki.osdev.org/X86-64_Instruction_Encoding#REX_prefix
-- luacheck: pop

_G.a = 0
_G.b = 0
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ test:plan(1)
-- The number value for the test has the same precision
-- (`prec` = 5) and amount of digits (`hilen` = 5) for the decimal
-- representation. Hence, with `ndhi` == 0, the `ndlo` part
-- becomes 64 (the size of the `nd` stack buffer), and the overflow
-- occurs.
-- becomes 64 (the size of the `nd` stack buffer), and the
-- overflow occurs.
-- See details in the <src/lj_strfmt_num.c>:`lj_strfmt_wfnum()`.
test:is(string.format('%7g', 0x1.144399609d407p+401), '5.5733e+120',
'correct format %7g result')
Expand Down
17 changes: 12 additions & 5 deletions test/tarantool-tests/lj-366-strtab-correct-size.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,17 @@ local utils = require('utils')
-- Command below exports bytecode as an object file in ELF format:
-- $ luajit -b -n 'lango_team' -e 'print()' xxx.obj
-- $ file xxx.obj
-- xxx.obj: ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV), not stripped
-- xxx.obj: ELF 64-bit LSB relocatable, x86-64, version 1 (SYSV),
-- not stripped
--
-- With read_elf(1) it is possible to display entries in symbol
-- table section of the file, if it has one. Object file contains
-- a single symbol with name 'luaJIT_BC_lango_team':
--
-- $ 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
Expand All @@ -45,6 +48,8 @@ local utils = require('utils')
-- Reference numbers for strtab offset and size could be obtained
-- with readelf(1). Note that number system of these numbers is
-- hexadecimal.
--
-- luacheck: pop

-- Symbol name prefix for LuaJIT bytecode defined in bcsave.lua.
local LJBC_PREFIX = 'luaJIT_BC_'
Expand Down Expand Up @@ -139,11 +144,12 @@ local function create_obj_file(name)
return elf_filename
end

-- Parses a buffer in an ELF format and returns an offset and a size of strtab
-- and symtab sections.
-- Parses a buffer in an ELF format and returns an offset and a
-- size of strtab 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.
Expand Down Expand Up @@ -178,7 +184,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
Expand Down
28 changes: 16 additions & 12 deletions test/tarantool-tests/lj-416-xor-before-jcc.test.lua
Original file line number Diff line number Diff line change
Expand Up @@ -6,20 +6,22 @@ local test = tap.test('lj-416-xor-before-jcc'):skipcond({
test:plan(1)

-- To reproduce this issue, we need:
-- 0) IR for either "internal" (e.g. type check, hmask check) or "external"
-- (e.g. branch or loop condition) guard begins to be emitted to
-- mcode.
-- 1) JCC to side exit is emitted to the trace mcode at the beginning.
-- 0) IR for either "internal" (e.g. type check, hmask check) or
-- "external" (e.g. branch or loop condition) guard begins to
-- be emitted to mcode.
-- 1) JCC to side exit is emitted to the trace mcode at the
-- beginning.
-- 2) Condition (i.e. comparison) is going to be emitted.
-- 3) Fuse optimization takes its place, that ought to allocate a register
-- for the load base.
-- 3) Fuse optimization takes its place, that ought to allocate a
-- register for the load base.
-- 4) There are no free registers at this point.
-- 5) The one storing the constant zero is chosen to be sacrificed and
-- reallocated (consider the allocation cost in ra_alloc for constant
-- materialization).
-- 6) Before (or in the sense of trace execution, after) register is
-- being used in the aforementioned comparison, register (r14 in our
-- case) is reset by XOR emitted right after (before) jump instruction.
-- 5) The one storing the constant zero is chosen to be sacrificed
-- and reallocated (consider the allocation cost in ra_alloc
-- for constant materialization).
-- 6) Before (or in the sense of trace execution, after) register
-- is being used in the aforementioned comparison, register
-- (r14 in our case) is reset by XOR emitted right after
-- (before) jump instruction.
-- 7) The comparison with fused load within is emitted.
--
-- This leads to assembly code like the following:
Expand Down Expand Up @@ -69,6 +71,7 @@ local function testf()
end
end

-- luacheck: push no max_comment_line_length
-- The code below is recorded with the following IRs:
-- .... SNAP #1 [ lj-416-xor-before-jcc.test.lua:44|---- ]
-- 0012 > num UGT 0009 +42
Expand All @@ -80,6 +83,7 @@ local function testf()
--
-- As a result, this branch is taken due to the emitted `xor`
-- instruction until the issue is not resolved.
-- luacheck: pop
if value <= MAGIC then
return true
end
Expand Down
Loading

0 comments on commit 9f6e14a

Please sign in to comment.