fix: make @inline never actually prevent inlining (#131)#132
Merged
Conversation
The @inline <name> never pragma parsed and was stored but never acted as a veto: both inlining sites decided with `isInlinableExpr expr || <used once>`, and isInlinableExpr maps Just Never to the same result as Nothing, so a never-annotated reference, small literal, or single-use binding was still inlined. Add an inlineForbidden check (getAnn expr == Just Never) that short-circuits the decision at both sites (optimizeModule's top-level binding inliner and inlineLocalBindings' let inliner), overriding the heuristic and the single-use rule. Update Note [Inline annotations and inlining heuristics] accordingly. Add an IR.Optimizer test: a never-annotated, used-once top-level binding is kept rather than inlined (red before, green after). @inline always is unaffected and no golden uses @inline never, so generated output is unchanged. Closes #131.
There was a problem hiding this comment.
Pull request overview
This PR fixes the IR optimizer so @inline <name> never is no longer a no-op: it now actively vetoes inlining in both the top-level binding inliner and the local let binding inliner, aligning behavior with the documented intent and closing #131.
Changes:
- Add an
inlineForbiddenguard (getAnn expr == Just Never) to short-circuit inlining decisions at both inlining sites. - Add a regression test ensuring a
Never-annotated, single-use top-level binding is retained byoptimizedUberModule. - Update the inline-annotation documentation note and add a changelog fragment for the user-facing fix.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
lib/Language/PureScript/Backend/IR/Optimizer.hs |
Implements inlineForbidden and applies it to both inlining decision points. |
test/Language/PureScript/Backend/IR/Optimizer/Spec.hs |
Adds a regression test for @inline never at the top-level binding inliner. |
lib/Language/PureScript/Backend/IR/Inliner.hs |
Updates documentation note to reflect never as a true inlining veto. |
changelog.d/20260625_210853_unisay_inline_never_veto.md |
Documents the fix as a user-facing change. |
…iew) Copilot noted the first cut read getAnn on the post-optimization expression, so a rewrite that drops the annotation off a binding's root (e.g. constant folding) could still let `@inline never` be ignored. Collect the never-annotated top-level binding names once from the pristine UberModule (neverInlineNames) and thread that set into optimizeModule, which refuses to inline any binding in it. The veto is now keyed by name, so it is stable across rewrites and the idempotent fixpoint. Revert the speculative let-binding-inliner guard: @inline pragmas name top-level bindings, so the annotation never reaches the local Let inliner. Strengthen the regression test to a constant-foldable binding (foo = 1 == 1) whose root annotation is dropped by folding, proving the name-based veto holds.
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.
Summary
@inline <name> neverparsed and was stored, but it never affected the optimizer, so the annotation was a no-op. Both inlining sites decided withisInlinableExpr expr || <used once>, andisInlinableExprmapsJust Neverto the same result asNothing. Anever-annotated binding that was a reference, a small literal, or used exactly once was therefore still inlined through the second disjunct.The fix collects the
never-annotated binding names once from the pristine module (neverInlineNames) and threads that set intooptimizeModule, the top-level binding inliner, which refuses to inline any name in it. The veto is keyed by name rather than by re-reading the annotation, so it survives rewrites that drop the annotation off a binding's root (for example constant folding) and the idempotent optimization fixpoint.@inlinepragmas only attach to top-level bindings, so that inliner is the only site the annotation reaches; the localletinliner is left unchanged.@inline alwaysis unaffected. No golden uses@inline never, so generated output is unchanged for every existing test.Closes #131.
How it was verified
A new
IR.Optimizertest builds anUberModulewith a top-level bindingfoo = (1 == 1)annotatednever, exported and used once. Constant folding rewrites the root totrue, dropping the annotation, so the test specifically exercises the name-based veto: it asserts the optimizer keepsfooas a binding rather than inlining it away. Red before the fix (foo was inlined and dropped), green after. I also updatedNote [Inline annotations and inlining heuristics], which previously documented the broken behavior.Checklist
changelog.d/fragment for any user-facing change (scriv createin the dev shell), or this change ships nothing releasable (CI, docs, or an
internal refactor).
nix develop),fourmolu -i lib/ exe/ test/andhlint lib/ exe/ test/are clean.cabal test allpasses; structural goldens werere-accepted on purpose if codegen moved (
PSLUA_GOLDEN_ACCEPT=1), andeval/golden.txtstill holds.