Pin Data.Unit.unit to the shared foreign singleton (@inline unit never) - #14
Merged
Conversation
The FFI value unit = ({}) is a table constructor, not a value, so the
old `@inline unit always` pragma pasted a fresh {} allocation into every
use site (7 inlined {} in the linked Data.Array output alone, including
void = map(function() return {} end): one allocation per void step).
Flip the pragma to `@inline unit never` so unit stays one shared table
bound to the foreign module. This matches the foreign table-constructor
sharing the compiler performs by default since
purescript-lua/purescript-lua#175, and declares the sharing intent
explicitly against future heuristic changes.
Refs purescript-lua/purescript-lua#176
There was a problem hiding this comment.
Pull request overview
Pins Data.Unit.unit to a single shared foreign singleton by changing the inlining pragma from always to never, preventing repeated {} allocations should foreign inlining heuristics change in the future.
Changes:
- Updated
Data.Unit’s inlining pragma to-- @inline unit never. - Added a changelog fragment documenting the rationale and behavior.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Data/Unit.purs | Switches the unit inlining pragma to never to keep unit as a single binding. |
| changelog.d/20260712_165906_unisay.md | Documents the change and the allocation-sharing motivation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- changelog.d/20260712_165906_unisay.md:7 — keep `@inline unit never` code span on one line (#14 discussion_r3566905136)
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.
Data/Unit.pursopened with-- @inline unit always, on the assumption that unit is the cheapest possible thing to inline. That assumption is backwards: the FFI valueunit = ({})is a table constructor, so underalwaysthe compiler was free to paste a fresh{}allocation into every use site instead of sharing one table. This flips the pragma tonever, sounitstays a single binding onto the foreign module.Why
never, and why nowSince purescript-lua/purescript-lua#175 the compiler no longer inlines foreign-module expressions at all. A foreign table stays hoisted and every access folds into a field read off it, so the singleton is already shared by default. That makes this flip a declaration of intent rather than the fix itself: it pins
unitto one binding explicitly, so a later change to the inlining heuristics can't quietly start duplicating the constructor again. purescript#175 added@inline <name> neversupport for foreign exports precisely so a fork can state this.What changes in generated code
Linking the compiler's test corpus against this branch, holding everything else fixed:
Array Unitprinting class (Arrays of typeArray Unitare always empty purescript-lua#23), which still printsunit / unit / unit / 3.{}for unit, inside the hoisted foreign table (local Data_Unit_foreign = { unit = {} }), and the total{}count across the linked goldens does not change.alwaysfolded the accessor into each site,neverkeeps a singlelocal Data_Unit_unit = Data_Unit_foreign.unitand references it by name:local Data_Unit_foreign = { unit = {} } +local Data_Unit_unit = Data_Unit_foreign.unit ... -return { main = Effect_applicativeEffect.pure(Data_Unit_foreign.unit) } +return { main = Effect_applicativeEffect.pure(Data_Unit_unit) }Notes
.luaFFI changed, so luacheck is untouched, and./scripts/buildpasses.Fixes purescript-lua/purescript-lua#176