test: lock regressions for #28 and #30#47
Merged
Conversation
Reproduces the Data.Lens._Newtype miscompilation from issue #28 without profunctor-lenses: a binding abstracted over a Profunctor dictionary, used more than once so the used-once inliner cannot mask a lost dictionary lambda. The bug was fixed by the eta reduction guard in 833cec8 and the subsequent removal of eta reduction entirely (#45); this eval golden locks it in.
Reproduces the nested maybe/map chain from issue #30 that used to crash the currying optimizer with 'Impossible subexpressions: IfThenElse'. The crash site was removed wholesale with the currying optimizer in 965d1fe; lib/ no longer contains the error call at all. Two modules cover both link modes: Golden.MaybeChain.Test with main and an eval golden (LinkAsApplication), and Golden.MaybeChainModule.Test without main (LinkAsModule, the mode the crash was reported in).
There was a problem hiding this comment.
Pull request overview
Adds eval-backed golden regression tests to permanently lock in fixes for previously reported backend bugs (#28 and #30), providing executable evidence so the issues can be closed confidently.
Changes:
- Add a new golden test (
Golden.ProfunctorDictLens.Test) that reproduces the_Newtypemiscompile shape and validates runtime behavior via eval golden output. - Add two golden tests (
Golden.MaybeChain.TestandGolden.MaybeChainModule.Test) to cover the nestedmaybe/mapchain that previously triggered the optimizer crash, in both application and module link modes. - Update the PureScript test project dependencies to include
maybe,newtype, andprofunctordirectly.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/ps/spago.dhall | Adds direct PS dependencies needed by the new golden repro modules. |
| test/ps/golden/Golden/ProfunctorDictLens/Test.purs | New repro module for #28 (Profunctor dictionary lambda preservation), includes main for eval golden. |
| test/ps/output/Golden.ProfunctorDictLens.Test/corefn.json | Golden CoreFn snapshot for the new #28 repro module. |
| test/ps/output/Golden.ProfunctorDictLens.Test/golden.ir | Golden IR snapshot for the new #28 repro module. |
| test/ps/output/Golden.ProfunctorDictLens.Test/golden.lua | Golden Lua snapshot for the new #28 repro module (expects dict lambda to be preserved). |
| test/ps/output/Golden.ProfunctorDictLens.Test/eval/golden.txt | Expected runtime output for #28 repro module. |
| test/ps/output/Golden.ProfunctorDictLens.Test/eval/.gitignore | Ignores eval actual.txt artifacts for #28 repro module. |
| test/ps/golden/Golden/MaybeChain/Test.purs | New application-link-mode repro for #30 with main for eval golden. |
| test/ps/output/Golden.MaybeChain.Test/corefn.json | Golden CoreFn snapshot for the new #30 application-mode repro. |
| test/ps/output/Golden.MaybeChain.Test/golden.ir | Golden IR snapshot for the new #30 application-mode repro. |
| test/ps/output/Golden.MaybeChain.Test/golden.lua | Golden Lua snapshot for the new #30 application-mode repro. |
| test/ps/output/Golden.MaybeChain.Test/eval/golden.txt | Expected runtime output for #30 application-mode repro. |
| test/ps/output/Golden.MaybeChain.Test/eval/.gitignore | Ignores eval actual.txt artifacts for #30 application-mode repro. |
| test/ps/golden/Golden/MaybeChainModule/Test.purs | New module-link-mode repro for #30 (no main, compiled as AsModule). |
| test/ps/output/Golden.MaybeChainModule.Test/corefn.json | Golden CoreFn snapshot for the new #30 module-mode repro. |
| test/ps/output/Golden.MaybeChainModule.Test/golden.ir | Golden IR snapshot for the new #30 module-mode repro. |
| test/ps/output/Golden.MaybeChainModule.Test/golden.lua | Golden Lua snapshot for the new #30 module-mode repro. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Both bugs turned out to be already fixed on main, but neither issue got closed because the reporters never confirmed. This PR adds eval-backed regression tests so they can be closed with evidence rather than on the assumption that some commit took care of them.
#28 (
Data.Lens._Newtypemiscompiled). Root cause was the unguarded eta reduction dropping a lambda whose dictionary parameter was still in use. It was fixed twice: 833cec8 added a free-variable guard, and #45 later removed eta reduction entirely (seeNote [Eta reduction is unsound]). The newGolden.ProfunctorDictLens.Testreproduces the_Newtypeshape without pulling in profunctor-lenses:_Wrapped = dimap unwrap Wrappedis abstracted over the Profunctor dictionary and used more than once, so the used-once inliner cannot hide a lost dictionary lambda. The generated Lua keeps the binding asfunction(dictProfunctor) ... end, and the eval golden checks the runtime output.#30 (
pslua: Impossible subexpressions: IfThenElse). The crash lived in the currying optimizer, which 965d1fe removed wholesale;grep -rn Impossible lib/comes back empty today.Golden.MaybeChain.Testreproduces the nested maybe/map chain from the report with an eval golden (application link mode), andGolden.MaybeChainModule.Testcovers module link mode, which is where the crash was originally reported.test/ps/spago.dhallgainsmaybe,newtypeandprofunctoras direct dependencies; all three were already present in the package set transitively.Closes #28
Closes #30