Uh oh!
There was an error while loading. Please reload this page.
fix: restore the STFn calling convention in the Data.Array.ST FFI - #3
Merged
Conversation
The PureScript side has declared thawImpl/freezeImpl/toAssocArrayImpl (STFn1) since the upstream STFn migration, but the Lua module still exported only the pre-migration names thaw/freeze/toAssocArray in the curried+thunk shape, leaving the declared foreigns unbound: thaw, freeze, withArray and toAssocArray crashed with a nil call. peekImpl (STFn4) returned an inner thunk instead of its result, so peek produced a function instead of the Maybe. Rename the exports to the *Impl names, perform the effects directly on the n-ary call, and pin the STFn convention in the regression guard. Refs purescript-lua/purescript-lua#186.
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Problem
The
.pursside has declaredthawImpl/freezeImpl/toAssocArrayImpl(STFn1) since the upstream STFn migration, butData/Array/ST.luastill exported only the pre-migration namesthaw/freeze/toAssocArray, in the old curried+thunk shape. The declared foreigns were unbound, sothaw,freeze,withArrayandtoAssocArraycrashed with "attempt to call … a nil value" in the released v7.4.1. In the same migration,peekImpl(declaredSTFn4) kept an inner thunk, sopeekreturned a function instead of theMaybe.Found by the FFI audit in purescript-lua/purescript-lua#186. It stayed invisible because the regression guard called the stale
ST.freeze(xs)()export directly, and no compiler golden exercises these functions.Change
Data/Array/ST.lua: exportthawImpl/freezeImpl(the existingcopyImpl, now performing the copy directly) andtoAssocArrayImpl; unwrappeekImplso it returns its result on the 4-ary call; drop the stalethaw/freeze/toAssocArrayentries.test/regression/array_st.lua: the guard now drives all four entries through the STFn convention (n-ary call performs the effect, result returned directly,*Implnames). It was red against the old FFI (nil call onfreezeImpl) and is green after the reshape.Verification
scripts/testgreen (regression suites for ST and NonEmpty). Reproduction against the released code, exactly as a compiled program calls it:runSTFn1(ST.thawImpl)({1,2,3})()crashed before ("attempt to call upvalue 'fn' (a nil value)") and returns the copied table after;ST.peekImpl(just, nothing, 1, {10,20,30})returned a function before and returnsJust 20after.Suggested release: patch (v7.4.2) plus a package-set bump.
Refs purescript-lua/purescript-lua#186.