feat: bring arrays to the canon and make the FFI Lua 5.1-safe#1
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR modernizes purescript-lua-arrays for the purescript-lua package set and updates the Lua FFI to be Lua 5.1-compatible (removing reliance on table.move, table.pack, and table.unpack). It also introduces Nix-based tooling/CI and adds a Lua regression test focused on the rewritten FFI paths.
Changes:
- Replace Lua 5.3-only FFI usages: implement an overlap-safe
moveinData/Array/ST.luaand switchData/Array.luacopies to{ unpack(...) }. - Add Lua regression tests and a
scripts/testrunner; harden build/test scripts and CI aroundnix develop. - Update package-set/flake inputs and remove legacy JS-era tooling files (npm/bower/eslint).
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/regression/array_st.lua | Adds Lua regression coverage for move-based operations and unpack-based array ops. |
| src/Data/Array/ST.lua | Replaces table.move with a local overlap-safe move and uses it in affected functions. |
| src/Data/Array.lua | Replaces table.pack(table.unpack(...)) with { unpack(...) } in several copy paths. |
| scripts/test | Adds a simple regression-test runner for the Lua checks. |
| scripts/build | Hardens build script (adds set -euo pipefail) and runs spago build + pslua emits. |
| packages.dhall | Switches package-set URL to the purescript-lua org. |
| package.json | Removes legacy npm/pulp tooling configuration. |
| flake.nix | Moves to purescript-overlay, pins toolchain, and adds Nix cache config. |
| flake.lock | Updates lockfile to match new flake inputs/pins. |
| CLAUDE.md | Adds a pointer to AGENTS.md. |
| bower.json | Removes legacy bower-era dependency manifest. |
| AGENTS.md | Documents Lua 5.1 constraints, commands, and release/tooling notes. |
| .gitignore | Simplifies ignore rules for the new workflow/tooling. |
| .github/workflows/ci.yml | Updates CI to use Nix, run build, regression tests (if present), and luacheck. |
| .eslintrc.json | Removes legacy ESLint config. |
Comments suppressed due to low confidence (1)
scripts/build:6
- The build script relies on
psluacreatingdist/implicitly. Creating the directory explicitly makes the build more robust acrosspsluaversions and avoids the failure mode called out in AGENTS.md.
set -euo pipefail
echo "Building..."
spago build
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Scaffolding. Move purescript-lua-arrays onto the package-set canon: an
overlay flake (purs 0.15.16, spago 0.21.0, Lua 5.1, pslua at the current
main), hardened CI, scripts/build, AGENTS.md + CLAUDE.md, a clean
.gitignore, and the purescript-lua org package-set URL. Drop the
.eslintrc, bower.json and package.json.
FFI fix. The foreign modules used Lua 5.2/5.3 builtins absent in 5.1:
src/Data/Array.lua called table.pack(table.unpack(...)) in unconsImpl,
_insertAt, _deleteAt, _updateAt and the sort, now { unpack(...) }; and
src/Data/Array/ST.lua used table.move in freeze/thaw, pushAll, unshiftAll,
splice and sortBy, now a hand-written overlap-safe move with the same
semantics as Lua 5.3 table.move (it copies backwards when the source and
destination overlap to the right).
Tests. scripts/test runs test/regression/array_st.lua, a lean guard over
exactly the rewritten paths: freeze independence, pushAll, splice in both
overlap directions, a stable sort, and the unpack-based uncons/insertAt/
deleteAt/updateAt. Verified locally with build, the regression run, and
luacheck --std lua51 all green.
d94a9a5 to
fb573d4
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings
purescript-lua-arrays(the last fork still on JavaScript-era tooling) onto the package-set canon and fixes the Lua 5.1 FFI that the canonluacheck --std lua51surfaced.Scaffolding. Overlay flake pinning purs 0.15.16 / spago 0.21.0 / Lua 5.1 and
psluaat the current main, hardened CI,scripts/build,AGENTS.md+CLAUDE.md, a clean.gitignore, and thepurescript-luaorg package-set URL. Drops.eslintrc,bower.json,package.json.FFI fix (Lua 5.1). The foreign modules used builtins that do not exist in Lua 5.1:
src/Data/Array.lua—table.pack(table.unpack(...))inunconsImpl,_insertAt,_deleteAt,_updateAtand the merge sort, replaced with{ unpack(...) }(the.nfield was never read).src/Data/Array/ST.lua—table.moveinfreeze/thaw,pushAll,unshiftAll,spliceandsortBy, replaced with a hand-writtenmovewhose semantics match Lua 5.3'stable.move, including copying backwards when source and destination overlap to the right.Tests.
scripts/testrunstest/regression/array_st.lua, a lean guard over exactly the rewritten paths:freezeindependence,pushAll,splicein both overlap directions (the shift-right case is what a naive forward copy gets wrong), a stable sort, and the unpack-baseduncons/insertAt/deleteAt/updateAt.Verified locally:
./scripts/build,./scripts/test(all FFI regression checks passed), andluacheck --std lua51are all green. This is asrc/change, so the fork needs a release before the set is re-tagged.