From 56b93f6181438ace9764d280b5d906d75cfc5883 Mon Sep 17 00:00:00 2001 From: ruki Date: Wed, 15 Apr 2026 00:44:04 +0800 Subject: improve pairs for lua5.5 --- tests/modules/for_loop/test.lua | 124 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 124 insertions(+) create mode 100644 tests/modules/for_loop/test.lua (limited to 'tests/modules/for_loop/test.lua') diff --git a/tests/modules/for_loop/test.lua b/tests/modules/for_loop/test.lua new file mode 100644 index 000000000..926d7e700 --- /dev/null +++ b/tests/modules/for_loop/test.lua @@ -0,0 +1,124 @@ +-- Regression tests for Lua 5.4 for-loop customizations in xmake: +-- 1. RDKCONST is relaxed on the control variable (parser patch), so +-- reassigning `i` in a numeric-for or `k` in a generic-for compiles. +-- 2. Sandbox `pairs` snapshots keys so reassigning the first loop +-- variable cannot corrupt the iterator state — otherwise `next` +-- would later fail with "invalid key to 'next'". + +function test_numeric_for_reassign(t) + -- Just needs to compile & run without "attempt to assign to const". + local sum = 0 + for i = 1, 5 do + i = i * 10 + sum = sum + i + end + t:require(sum == 10 + 20 + 30 + 40 + 50) +end + +function test_generic_for_reassign_key(t) + local tbl = {foo_a = 1, foo_b = 2, foo_c = 3} + local seen = {} + local count = 0 + for k, v in pairs(tbl) do + k = k:gsub("_", "-") + seen[k] = v + count = count + 1 + end + t:require(count == 3) + t:require(seen["foo-a"] == 1) + t:require(seen["foo-b"] == 2) + t:require(seen["foo-c"] == 3) +end + +function test_generic_for_reassign_many_keys(t) + -- Stress the snapshot path with enough keys that a broken iterator + -- would deterministically trip `next` on the second iteration. + local tbl = {} + for i = 1, 64 do + tbl["key_" .. i] = i + end + local total = 0 + for name, value in pairs(tbl) do + name = name:gsub("_", "-") -- would corrupt `next`'s key arg + t:require(name:find("^key%-%d+$") ~= nil) + total = total + value + end + t:require(total == (1 + 64) * 64 / 2) +end + +function test_list_items_reassign(t) + import("core.base.list") + local l = list.new() + for i = 1, 5 do + l:push({name = "n" .. i}) + end + local names = {} + for item in l:items() do + item = nil -- would corrupt list:next on the next iteration + -- re-fetch to prove we don't rely on `item` + end + local count = 0 + for item in l:items() do + count = count + 1 + table.insert(names, item.name) + end + t:require(count == 5) + t:require(names[1] == "n1" and names[5] == "n5") +end + +function test_irpairs_reassign_index(t) + local arr = {"a", "b", "c", "d", "e"} + local collected = {} + for i, v in irpairs(arr) do + i = -1 -- would corrupt the index on the next iteration + table.insert(collected, v) + end + t:require(#collected == 5) + t:require(collected[1] == "e") + t:require(collected[5] == "a") +end + +function test_hashset_items_reassign(t) + import("core.base.hashset") + local set = hashset.from({"key_1", "key_2", "key_3", "key_4", "key_5"}) + local seen = {} + for item in set:items() do + item = item:gsub("_", "-") -- would corrupt `next`'s key arg + seen[item] = true + end + local count = 0 + for _ in pairs(seen) do count = count + 1 end + t:require(count == 5) + for i = 1, 5 do + t:require(seen["key-" .. i] == true) + end +end + +function test_ipairs_reassign_index(t) + -- With the stock Lua 5.4 `ipairs`, writing to the first loop variable + -- would silently shift the index on the next iteration (no error, + -- just wrong results). Sandbox `ipairs` hides the counter in an + -- upvalue so the body's write is harmless. + local list = {10, 20, 30, 40, 50} + local seen = {} + for i, v in ipairs(list) do + i = -1 -- would corrupt iteration if `i` were the control slot + table.insert(seen, v) + end + t:require(#seen == 5) + for idx = 1, 5 do + t:require(seen[idx] == list[idx]) + end +end + +function test_generic_for_reassign_value(t) + -- Writing to the second loop variable is always safe (it isn't the + -- iterator control), but exercise it anyway to pin the behavior. + local tbl = {a = 1, b = 2, c = 3} + local total = 0 + for _, v in pairs(tbl) do + v = v * 2 + total = total + v + end + t:require(total == 12) +end -- cgit v1.3.1 From 088cae88b4600bde070c73b9e218712195fe77ee Mon Sep 17 00:00:00 2001 From: ruki Date: Wed, 15 Apr 2026 00:45:16 +0800 Subject: improve test --- tests/modules/for_loop/test.lua | 50 ++++++++++++++++++++++++++++------------- 1 file changed, 35 insertions(+), 15 deletions(-) (limited to 'tests/modules/for_loop/test.lua') diff --git a/tests/modules/for_loop/test.lua b/tests/modules/for_loop/test.lua index 926d7e700..231337a29 100644 --- a/tests/modules/for_loop/test.lua +++ b/tests/modules/for_loop/test.lua @@ -1,9 +1,34 @@ --- Regression tests for Lua 5.4 for-loop customizations in xmake: --- 1. RDKCONST is relaxed on the control variable (parser patch), so --- reassigning `i` in a numeric-for or `k` in a generic-for compiles. --- 2. Sandbox `pairs` snapshots keys so reassigning the first loop --- variable cannot corrupt the iterator state — otherwise `next` --- would later fail with "invalid key to 'next'". +-- Regression tests for the Lua 5.4+ for-loop hazard: the parser merges +-- the generic-for control slot with the user's first loop variable and +-- marks it `RDKCONST`, so code like +-- +-- for k, v in pairs(t) do +-- k = k:gsub("_", "-") +-- ... +-- end +-- +-- normally fails to compile with "attempt to assign to const variable", +-- and if we only relax the const check the runtime silently corrupts the +-- iterator state (next iteration gets the modified key and `next` bails +-- with "invalid key to 'next'" — or worse, silently skips entries). +-- +-- xmake works around this in two coordinated places: +-- +-- 1. Compile-time: `core/src/lua/xmake.lua` (and xmake.sh) replace +-- `RDKCONST);` with `VDKREG);` in lparser.c so assignment to the +-- first loop variable is allowed through. +-- +-- 2. Runtime: the sandbox iterators (`pairs`, `ipairs`, `irpairs`) and +-- the base containers (`list:items/ritems`, `hashset:items/keys`) +-- are implemented as stateful closures keeping their cursor in an +-- upvalue, so user writes to the first loop variable cannot reach +-- the iterator's internal state. +-- +-- These tests pin both behaviors: that reassigning the first loop +-- variable (a) compiles and (b) produces correct, non-lossy iteration. + +import("core.base.list") +import("core.base.hashset") function test_numeric_for_reassign(t) -- Just needs to compile & run without "attempt to assign to const". @@ -31,8 +56,8 @@ function test_generic_for_reassign_key(t) end function test_generic_for_reassign_many_keys(t) - -- Stress the snapshot path with enough keys that a broken iterator - -- would deterministically trip `next` on the second iteration. + -- Stress the iterator with enough keys that a broken stateless + -- iterator would deterministically trip `next` on the second round. local tbl = {} for i = 1, 64 do tbl["key_" .. i] = i @@ -47,22 +72,18 @@ function test_generic_for_reassign_many_keys(t) end function test_list_items_reassign(t) - import("core.base.list") local l = list.new() for i = 1, 5 do l:push({name = "n" .. i}) end - local names = {} for item in l:items() do item = nil -- would corrupt list:next on the next iteration - -- re-fetch to prove we don't rely on `item` end - local count = 0 + local names = {} for item in l:items() do - count = count + 1 table.insert(names, item.name) end - t:require(count == 5) + t:require(#names == 5) t:require(names[1] == "n1" and names[5] == "n5") end @@ -79,7 +100,6 @@ function test_irpairs_reassign_index(t) end function test_hashset_items_reassign(t) - import("core.base.hashset") local set = hashset.from({"key_1", "key_2", "key_3", "key_4", "key_5"}) local seen = {} for item in set:items() do -- cgit v1.3.1 From 03133fa624c9f27554b0c393e9151213cc99996e Mon Sep 17 00:00:00 2001 From: ruki Date: Wed, 15 Apr 2026 00:46:49 +0800 Subject: fix review issue --- tests/modules/for_loop/test.lua | 35 +++++++++++++++++++++++++++++++++++ xmake/core/base/hashset.lua | 34 ++++++++++++++++++++-------------- 2 files changed, 55 insertions(+), 14 deletions(-) (limited to 'tests/modules/for_loop/test.lua') diff --git a/tests/modules/for_loop/test.lua b/tests/modules/for_loop/test.lua index 231337a29..42217ed40 100644 --- a/tests/modules/for_loop/test.lua +++ b/tests/modules/for_loop/test.lua @@ -114,6 +114,41 @@ function test_hashset_items_reassign(t) end end +function test_hashset_items_skip_nil_member(t) + -- A nil member lives in hashset under the `_NIL` sentinel; iteration + -- must skip it rather than terminate, so real entries after it are + -- still visited regardless of `next`'s order. + local set = hashset.new() + set:insert("a") + set:insert(nil) + set:insert("b") + set:insert("c") + local seen = {} + for item in set:items() do + seen[item] = true + end + local count = 0 + for _ in pairs(seen) do count = count + 1 end + t:require(count == 3) + t:require(seen.a and seen.b and seen.c) +end + +function test_hashset_orderitems_skip_nil_member(t) + -- use numeric members so the existing orderitems sort (which coerces + -- `_NIL` to math.inf) stays within one comparable type. + local set = hashset.new() + set:insert(1) + set:insert(nil) + set:insert(2) + set:insert(3) + local collected = {} + for item in set:orderitems() do + table.insert(collected, item) + end + t:require(#collected == 3) + t:require(collected[1] == 1 and collected[2] == 2 and collected[3] == 3) +end + function test_ipairs_reassign_index(t) -- With the stock Lua 5.4 `ipairs`, writing to the first loop variable -- would silently shift the index on the next iteration (no error, diff --git a/xmake/core/base/hashset.lua b/xmake/core/base/hashset.lua index ea184c6bf..fa0fd3b6c 100644 --- a/xmake/core/base/hashset.lua +++ b/xmake/core/base/hashset.lua @@ -127,13 +127,17 @@ function hashset:items() -- the first loop variable. In lua 5.4+ the for-in control slot is -- merged with the first user variable; threading the key through the -- loop would otherwise corrupt `next` on the following iteration. + -- + -- nil-as-a-member is stored under the `_NIL` sentinel. For-loop + -- semantics don't let us yield nil (it would end the loop), so we + -- skip the sentinel and continue to the next real key; the nil + -- member is omitted but entries after it are still visited. local data = self._DATA local k = nil return function () - k = next(data, k) - if k == nil or k == hashset._NIL then - return nil - end + repeat + k = next(data, k) + until k ~= hashset._NIL return k end end @@ -162,16 +166,18 @@ function hashset:orderitems() end return a < b end) - local i = 1 - return function (t, k) - k = orderkeys[i] - i = i + 1 - if k == hashset._NIL then - return nil - else - return k - end - end, self, nil + -- see hashset:items() for the `_NIL` handling rationale + local n = #orderkeys + local i = 0 + return function () + local k + repeat + i = i + 1 + if i > n then return nil end + k = orderkeys[i] + until k ~= hashset._NIL + return k + end end -- iterate keys (deprecated, please use items()) -- cgit v1.3.1 From dbed1aee901ec9cbb22b45ff22bb112cc0c26c0a Mon Sep 17 00:00:00 2001 From: ruki Date: Wed, 15 Apr 2026 00:56:47 +0800 Subject: fix test --- tests/modules/for_loop/test.lua | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) (limited to 'tests/modules/for_loop/test.lua') diff --git a/tests/modules/for_loop/test.lua b/tests/modules/for_loop/test.lua index 42217ed40..51d5f075a 100644 --- a/tests/modules/for_loop/test.lua +++ b/tests/modules/for_loop/test.lua @@ -31,13 +31,23 @@ import("core.base.list") import("core.base.hashset") function test_numeric_for_reassign(t) - -- Just needs to compile & run without "attempt to assign to const". - local sum = 0 + -- The point of this test is purely compile-time: without the + -- RDKCONST->VDKREG replace in core/src/lua/xmake.lua the write to + -- `i` would raise "attempt to assign to const variable" and this + -- file wouldn't even parse. + -- + -- We do NOT assert on the value of `i` during iteration. Lua 5.4 + -- merges the numeric-for control slot with the user's first loop + -- variable while 5.5 splits them, so the observable sequence of + -- `i` values after a reassignment differs between versions. The + -- iteration count lives in its own slot and stays stable, so that + -- is what we pin. + local n = 0 for i = 1, 5 do i = i * 10 - sum = sum + i + n = n + 1 end - t:require(sum == 10 + 20 + 30 + 40 + 50) + t:require(n == 5) end function test_generic_for_reassign_key(t) -- cgit v1.3.1