diff --git a/CLAUDE.md b/CLAUDE.md index 5fac01a..c1e927c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -185,6 +185,19 @@ expression compiles would route them into the native branch and skip `to_unsigned()`. A consequence worth knowing: a Lua 5.1 install with LuaBitOp takes the LuaJIT path and reports `_compat.is_luajit = true`. +`_compat` probes `string.pack`/`string.unpack` the same way, and exports them as +`_compat.string_pack` / `_compat.string_unpack`, nil unless they honour the Lua 5.3 +contract. The byte helpers in bit32/bit64 take their fast path from those two names +and never from `string.pack` directly. Two unrelated APIs ship under those names: +Lua 5.3+ has `pack(fmt, ...)` with size-suffixed codes and `unpack(fmt, s, pos)` +returning value then position, while Control4's DriverWorks has lpack, with no size +suffixes and an argument-swapped `unpack(s, fmt, pos)` returning position then +value. Testing presence rather than contract is not a loud failure on a controller: +lpack reads the payload of a 5.3-shaped call as its format string, and that parser +stops at a NUL byte or when the data runs out instead of erroring, so `>I4` decodes +of 0, 42 and 65535 all returned 1 there with nothing raised. The probe therefore +checks returned values, across every format string the library uses. + ### Raw Operations (bit32 and bit64) Both modules expose `raw_*` variants of every operation (`raw_band`, `raw_bor`, @@ -214,6 +227,15 @@ make test-matrix # across Lua versions CI additionally covers **LuaJIT 2.0**, so a green local matrix is a weaker signal than a green CI one. +Every module runs twice per interpreter. The second pass installs +`test/lpack_shim.lua`, an lpack-shaped `string.pack`/`string.unpack` modelled on a +Control4 controller, before the module is required, and asserts that +`_compat.string_pack` came out nil before running the selftest. That assertion is +what makes the pass meaningful: without it the run would go green on an +interpreter that never had `string.pack` at all. On 5.3+ the shim also overwrites a +genuine `string.pack`, so the fallback is exercised on hosts that would otherwise +always take the fast path. + ## Benchmarking Each module includes a `benchmark()` function built on `bitn.utils.benchmark`, diff --git a/run_tests.sh b/run_tests.sh index 9a6a0d6..45cbfbe 100755 --- a/run_tests.sh +++ b/run_tests.sh @@ -115,6 +115,10 @@ run_test() { echo } +# Each module runs twice: once against whatever the interpreter provides, and once +# with string.pack/string.unpack replaced by the lpack-shaped pair a Control4 +# controller ships, which the byte helpers must detect and decline to call. The +# shim is installed before the require because the dialect is probed at load. run_selftest() { local module_name="$1" local module_key="$2" @@ -125,6 +129,18 @@ run_selftest() { os.exit(1) end " + run_test "$module_name [lpack string.pack]" "$module_key" " + require('test.lpack_shim').install() + local _compat = require('bitn._compat') + if _compat.string_pack ~= nil or _compat.string_unpack ~= nil then + print('FAIL: the dialect probe bound an lpack string.pack/string.unpack') + os.exit(1) + end + local result = require('$lua_module').selftest() + if not result then + os.exit(1) + end + " } run_selftest "16-bit operations" "bit16" "bitn.bit16" diff --git a/src/bitn/_compat.lua b/src/bitn/_compat.lua index bbc655a..9974521 100644 --- a/src/bitn/_compat.lua +++ b/src/bitn/_compat.lua @@ -29,6 +29,81 @@ _compat.to_unsigned = to_unsigned -- Constants local MASK32 = 0xFFFFFFFF +-------------------------------------------------------------------------------- +-- string.pack/string.unpack dialect detection +-------------------------------------------------------------------------------- + +-- Two unrelated APIs ship under these two names. Lua 5.3+ has pack(fmt, ...) with +-- size-suffixed codes and unpack(fmt, s, pos) returning value then position. +-- Control4's DriverWorks has lpack: pack(fmt, ...) with no size suffixes, and +-- unpack(s, fmt, pos) returning position then value. Presence says nothing about +-- which one is installed. +-- +-- Guessing wrong is not a loud failure. Measured on a controller, lpack reads a +-- 5.3-shaped call's payload as its format string, and that parser stops at a NUL +-- byte or when the data runs out rather than erroring, so `>I4` decodes of 0, 42 +-- and 65535 all return 1 with nothing raised. So test the contract by value, on +-- every format this module uses, and fall back unless all of them answer exactly. +local string_pack = rawget(string, "pack") +local string_unpack = rawget(string, "unpack") + +local function packs_like_lua53() + if not (string_pack and string_unpack) then + return false + end + + local ok, s = pcall(string_pack, ">I4", 0x01020304) + if not (ok and s == "\1\2\3\4") then + return false + end + ok, s = pcall(string_pack, "I4I4", 0x01020304, 0x05060708) + if not (ok and s == "\1\2\3\4\5\6\7\8") then + return false + end + ok, s = pcall(string_pack, "I4", "\1\2\3\4", 1) + if not (ok and high == 0x01020304 and pos == 5) then + return false + end + ok, high, pos = pcall(string_unpack, "I4I4", "\1\2\3\4\5\6\7\8", 1) + if not (ok and high == 0x01020304 and low == 0x05060708 and pos == 9) then + return false + end + ok, high, low, pos = pcall(string_unpack, "I4", "\0\1\2\3\4", 2) + return ok and high == 0x01020304 and pos == 6 +end + +if not packs_like_lua53() then + string_pack, string_unpack = nil, nil +end + +--- `string.pack` when it honours the Lua 5.3 contract, otherwise nil. +--- @type nil|fun(fmt: string, ...: integer): string +_compat.string_pack = string_pack + +--- `string.unpack` when it honours the Lua 5.3 contract, otherwise nil. +--- @type nil|fun(fmt: string, s: string, pos: integer): ... +_compat.string_unpack = string_unpack + -------------------------------------------------------------------------------- -- Implementation 1: Native operators (Lua 5.3+) -------------------------------------------------------------------------------- diff --git a/src/bitn/bit32.lua b/src/bitn/bit32.lua index ed7f89a..f8bbfac 100644 --- a/src/bitn/bit32.lua +++ b/src/bitn/bit32.lua @@ -223,8 +223,8 @@ end local string_char = string.char local string_byte = string.byte -local string_pack = rawget(string, "pack") -local string_unpack = rawget(string, "unpack") +local string_pack = _compat.string_pack +local string_unpack = _compat.string_unpack -- % is the whole operation on the pure Lua backend; skip the call. local fast_band = _compat.has_native_ops or _compat.has_bit_lib diff --git a/src/bitn/bit64.lua b/src/bitn/bit64.lua index cffc839..316f8c7 100644 --- a/src/bitn/bit64.lua +++ b/src/bitn/bit64.lua @@ -30,8 +30,8 @@ local bit32_rshift = bit32.rshift local impl_name = _compat.impl_name local math_floor = math.floor local string_char = string.char -local string_pack = rawget(string, "pack") -local string_unpack = rawget(string, "unpack") +local string_pack = _compat.string_pack +local string_unpack = _compat.string_unpack -- Private metatable for Int64 type identification local Int64Meta = { __name = "Int64" } diff --git a/test/lpack_shim.lua b/test/lpack_shim.lua new file mode 100644 index 0000000..35b8af6 --- /dev/null +++ b/test/lpack_shim.lua @@ -0,0 +1,117 @@ +--- @module "test.lpack_shim" +--- Installs an lpack-shaped `string.pack`/`string.unpack`, the pair Control4's +--- DriverWorks ships under those two names, so the suite can prove that bitn +--- rejects the dialect and takes its manual byte path instead of calling it. +--- +--- The shape and the two quiet behaviours below were measured on a controller: +--- * `pack(fmt, ...)` has no size-suffixed codes, so a digit is an unknown code +--- * `unpack(data, fmt, pos)` is argument-swapped, and returns position then value +--- * the format is read as a C string, so a NUL byte ends it +--- * a code with fewer bytes left than it needs stops the run without raising +--- +--- The last two are why a Lua 5.3 shaped call is not simply an error here: the +--- payload lands where the format is expected, so `>I4` decodes of 0, 42 and 65535 +--- all come back as the position 1, matching the controller. +--- +--- Only the integer codes are modelled. Sweeping the first payload byte over 0-255 +--- accepts 14 of them here against the controller's 22, and the two sets differ +--- both ways (`B` is accepted here and not there, `A a c d f n p z` there and not +--- here). Those bytes are reachable in principle: the swapped signature is what puts +--- the payload in the format slot, so a little-endian u32 whose low byte is one of +--- the eight is quiet on a controller and raises here, about 3% of values. +--- +--- It does not reach them today, and the reason is a short circuit rather than the +--- value comparisons. Counting calls at this boundary while `bitn._compat` loads, +--- under both lua5.1 and luajit, gives pack 1 and unpack 0: `packs_like_lua53()` +--- declines at its first `pack(">I4", ...)` on the `4` and returns, so the unpack +--- parser the divergence lives in is never entered. Widening `pack` here to accept +--- size suffixes would put the probe into that parser and make the divergence live. +local lpack_shim = {} + +local SIZE = { b = 1, B = 1, h = 2, H = 2, i = 4, I = 4, l = 8, L = 8 } + +--- @param fmt string +--- @return string codes Format up to the first NUL, which terminates it +local function codes(fmt) + local stop = string.find(fmt, "\0", 1, true) + return stop and string.sub(fmt, 1, stop - 1) or fmt +end + +--- @param fmt string Format string, lpack dialect +--- @param ... integer Values to pack +--- @return string bytes +function lpack_shim.pack(fmt, ...) + local args, taken, little, out = { ... }, 0, true, {} + local format = codes(fmt) + for i = 1, #format do + local c = string.sub(format, i, i) + if c == "<" or c == "=" then + little = true + elseif c == ">" then + little = false + elseif c ~= " " and c ~= "," then + local size = SIZE[c] + if not size then + error("lpack shim: unsupported code '" .. c .. "'") + end + taken = taken + 1 + local v = args[taken] % (2 ^ (size * 8)) + local bytes = {} + for b = 1, size do + bytes[b] = string.char(math.floor(v / 2 ^ ((b - 1) * 8)) % 256) + end + if not little then + local reversed = {} + for b = 1, size do + reversed[b] = bytes[size - b + 1] + end + bytes = reversed + end + out[#out + 1] = table.concat(bytes) + end + end + return table.concat(out) +end + +--- @param data string Binary data +--- @param fmt string Format string, lpack dialect +--- @param pos? integer Starting position (default: 1) +--- @return integer nextpos, integer ... Position first, then the values +function lpack_shim.unpack(data, fmt, pos) + pos = pos or 1 + local little, out = true, {} + local format = codes(fmt) + for i = 1, #format do + local c = string.sub(format, i, i) + if c == "<" or c == "=" then + little = true + elseif c == ">" then + little = false + elseif c ~= " " and c ~= "," then + local size = SIZE[c] + if not size then + error("lpack shim: unsupported code '" .. c .. "'") + end + if pos + size - 1 > #data then + break + end + local v = 0 + for b = 0, size - 1 do + local byte = string.byte(data, pos + b) + v = v + byte * 2 ^ (little and (b * 8) or ((size - 1 - b) * 8)) + end + out[#out + 1] = v + pos = pos + size + end + end + return pos, (unpack or table.unpack)(out) +end + +--- Replace `string.pack`/`string.unpack` with the lpack-shaped pair. Must run +--- before the module under test is required, since the dialect is probed at load. +function lpack_shim.install() + string.pack = lpack_shim.pack + string.unpack = lpack_shim.unpack +end + +return lpack_shim