From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id ED4EEF38E02; Tue, 11 Feb 2025 14:51:49 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org ED4EEF38E02 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1739274710; bh=B8MPNpJbDkk/Aaf/a5BIuFIaSPmfzpdg7GjdRe8YEc8=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=TjR1QAYLHgSjkZIPXN8V02sUtN+mrFZk46yXOZngayoN2p1L1uBNp9HUoPvgjd4p9 zyg1/gbTpGTjyLirBk3RDZntq0DVNP+VZ/7CZRDToUOJlRxovuBr4tqOykZCwbC3Gw 3Wjuq3O0rmdwc3Tp/PEla1BLVU6zZwXykQRgBl/w= Received: from send129.i.mail.ru (send129.i.mail.ru [89.221.237.224]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 31135592F02 for ; Tue, 11 Feb 2025 14:51:49 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 31135592F02 Received: by exim-smtp-79fd7578cb-t54pg with esmtpa (envelope-from ) id 1thonr-000000002gp-3kld; Tue, 11 Feb 2025 14:51:48 +0300 Date: Tue, 11 Feb 2025 14:51:02 +0300 To: Sergey Bronnikov Cc: tarantool-patches@dev.tarantool.org Message-ID: References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9D183B3A831A946F4643300CEA5956C05446444A2DECCD393182A05F53808504041504CF0598BFEC03DE06ABAFEAF67056BA9262E3687469195737BE0519356F5CAD80AB92358092F X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7E99F7FC786761FD8C2099A533E45F2D0395957E7521B51C2CFCAF695D4D8E9FCEA1F7E6F0F101C6778DA827A17800CE74378043A27BE1642EA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38BC08E230531AC9C90534A4EE47A6038F47F827E9EC294FB88B1984ADAE8563527A471835C12D1D9774AD6D5ED66289B5278DA827A17800CE7820CF4CC0E318EFB9FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C3776A0366D588B3C3117882F4460429728AD0CFFFB425014E868A13BD56FB6657E2021AF6380DFAD1A18204E546F3947CB11811A4A51E3B096D1867E19FE1407959CC434672EE6371089D37D7C0E48F6C8AA50765F7900637BC468E7E89D8C5D6EFF80C71ABB335746BA297DBC24807EABDAD6C7F3747799A X-C1DE0DAB: 0D63561A33F958A5940ECD8C183AE7EB5002B1117B3ED696E2697F0D8C002A150E58516B1639A14B823CB91A9FED034534781492E4B8EEAD8D8BB953E4894305BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF990F344419A3D8328CEEFDB96F31BF866256661BEDF24D4148132BB4AC28BAF646C06E59B4E81405AE28C187D4EE0176FEB80BF72E5DCBC00E3B7DD4C6E28541C0A95A55D23D5EB9111DC66A97D0BFE2913E6812662D5F2A5EAB5682573093F7837F15F2B5E4A70B33F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj4p5EDZXZmhFjF+kBgt0aEw== X-DA7885C5: 166AFFCFA180E28AF255D290C0D534F979F3E3B1A9E1DFEC12915E53B2F69F416FF3FD03AF672B945B1A4C17EAA7BC4BEF2421ABFA55128DAF83EF9164C44C7E X-Mailru-Sender: 689FA8AB762F739381B31377CF4CA219CC162EEB70950C4C271E66C09CE804E7D0E8F50E8F32BF35E49D44BB4BD9522A059A1ED8796F048DB274557F927329BE89D5A3BC2B10C37545BD1C3CC395C826B4A721A3011E896F X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] test: limit code and comment max length by 80 X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "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 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