From 27aeda2732e5f7d580369460a7bb8cb287743544 Mon Sep 17 00:00:00 2001 From: Yura Lazarev Date: Thu, 25 Jun 2026 21:09:24 +0200 Subject: [PATCH 1/3] fix: make @inline never actually prevent inlining (#131) The @inline never pragma parsed and was stored but never acted as a veto: both inlining sites decided with `isInlinableExpr expr || `, 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. --- ...0260625_210853_unisay_inline_never_veto.md | 8 +++++++ lib/Language/PureScript/Backend/IR/Inliner.hs | 10 ++++---- .../PureScript/Backend/IR/Optimizer.hs | 13 ++++++++-- .../PureScript/Backend/IR/Optimizer/Spec.hs | 24 +++++++++++++++++++ 4 files changed, 48 insertions(+), 7 deletions(-) create mode 100644 changelog.d/20260625_210853_unisay_inline_never_veto.md diff --git a/changelog.d/20260625_210853_unisay_inline_never_veto.md b/changelog.d/20260625_210853_unisay_inline_never_veto.md new file mode 100644 index 00000000..a7309fd3 --- /dev/null +++ b/changelog.d/20260625_210853_unisay_inline_never_veto.md @@ -0,0 +1,8 @@ +### Fixed + +- `@inline never` now actually prevents inlining. The annotation was + parsed and stored but never consulted as a veto: both inlining sites decided + with `isInlinableExpr expr || `, so a `never`-annotated binding + that was a reference, a small literal, or used once was still inlined. A new + `inlineForbidden` check now vetoes inlining of any `Just Never` binding at + both sites, overriding the heuristic and the single-use rule (#131). diff --git a/lib/Language/PureScript/Backend/IR/Inliner.hs b/lib/Language/PureScript/Backend/IR/Inliner.hs index 88d86bb9..0b88d54b 100644 --- a/lib/Language/PureScript/Backend/IR/Inliner.hs +++ b/lib/Language/PureScript/Backend/IR/Inliner.hs @@ -19,11 +19,11 @@ travels to the optimizer's inlining decision through several stages: moves each into the annotated binding's 'Ann' as the binding is translated (see Note [Inliner annotations must all be consumed]). 3. From there the 'Annotation' rides along as the expression's @ann@. - 4. 'Language.PureScript.Backend.IR.Optimizer.isInlinableExpr' reads it back - with 'getAnn': @Just Always@ forces inlining. @Just Never@ and @Nothing@ - are treated alike -- both leave the heuristic (a ref, a small literal, or - a single-use binding) to decide. So @never@ currently only withholds the - forced case; it does not veto heuristic inlining. + 4. The optimizer reads it back with 'getAnn': @Just Always@ (via + 'Language.PureScript.Backend.IR.Optimizer.isInlinableExpr') forces + inlining, and @Just Never@ (via 'inlineForbidden') vetoes it outright, + overriding the ref / small-literal / single-use heuristic. @Nothing@ + leaves that heuristic to decide. The linker also synthesises @Just Always@ directly: each foreign name is bound to an 'ObjectProp' marked 'Inline.Always' so the wrapper around the FFI object diff --git a/lib/Language/PureScript/Backend/IR/Optimizer.hs b/lib/Language/PureScript/Backend/IR/Optimizer.hs index 092df8f3..11d5cbf8 100644 --- a/lib/Language/PureScript/Backend/IR/Optimizer.hs +++ b/lib/Language/PureScript/Backend/IR/Optimizer.hs @@ -249,7 +249,8 @@ optimizeModule UberModule {..} = withBinding binding (bindings, exports) = case binding of Standalone (qname, optimizedExpression → expr) → - if isInlinableExpr expr || isUsedOnce qname + if not (inlineForbidden expr) + && (isInlinableExpr expr || isUsedOnce qname) then ( substituteInBindings qname expr bindings , substituteInExports qname expr exports @@ -427,10 +428,18 @@ inlineLocalBinding grouping body = case grouping of RecursiveGroup _grp → body -- Not going to inline recursive bindings Standalone (_ann, Local → name, inlinee) → - if isInlinableExpr inlinee || countFreeRef name body == 1 + if not (inlineForbidden inlinee) + && (isInlinableExpr inlinee || countFreeRef name body == 1) then substitute name 0 inlinee body else body +{- | @inline never@ vetoes inlining outright, overriding both the +'isInlinableExpr' heuristic and the single-use rule. See +Note [Inline annotations and inlining heuristics]. +-} +inlineForbidden ∷ Exp → Bool +inlineForbidden expr = getAnn expr == Just Never + -- See Note [Inline annotations and inlining heuristics] isInlinableExpr ∷ Exp → Bool isInlinableExpr expr = diff --git a/test/Language/PureScript/Backend/IR/Optimizer/Spec.hs b/test/Language/PureScript/Backend/IR/Optimizer/Spec.hs index 4e50e2b4..1af645dd 100644 --- a/test/Language/PureScript/Backend/IR/Optimizer/Spec.hs +++ b/test/Language/PureScript/Backend/IR/Optimizer/Spec.hs @@ -5,6 +5,7 @@ import Data.Map qualified as Map import Hedgehog (PropertyT, annotateShow, forAll, (===)) import Hedgehog.Gen qualified as Gen import Language.PureScript.Backend.IR.Gen qualified as Gen +import Language.PureScript.Backend.IR.Inliner (Annotation (Never)) import Language.PureScript.Backend.IR.Linker (LinkMode (..)) import Language.PureScript.Backend.IR.Linker qualified as Linker import Language.PureScript.Backend.IR.Names @@ -165,6 +166,29 @@ spec = describe "IR Optimizer" do annotateShow original optimizedExpression original === original + describe "respects @inline never (issue #131)" do + test "keeps a never-annotated top-level binding instead of inlining it" do + let mainModule = moduleNameFromString "Main" + -- foo = 42, annotated `@inline never`: a literal used once, which the + -- optimizer would otherwise inline and drop. + fooExp = LiteralInt (Just Never) 42 + original = + Linker.UberModule + { uberModuleForeigns = [] + , uberModuleBindings = + [Standalone (QName mainModule (Name "foo"), fooExp)] + , uberModuleExports = + [(Name "main", refImported mainModule (Name "foo") 0)] + } + optimized = optimizedUberModule original + fooKept = + [ qn + | Standalone (qn, _) ← Linker.uberModuleBindings optimized + , qn == QName mainModule (Name "foo") + ] + annotateShow optimized + fooKept === [QName mainModule (Name "foo")] + describe "inliner unlocks more optimizations" do test "constant folding after inlining" do name ← forAll Gen.name From 44e24efe80ff882331bfddfb4750f2e96958a0a5 Mon Sep 17 00:00:00 2001 From: Yura Lazarev Date: Fri, 26 Jun 2026 08:28:39 +0200 Subject: [PATCH 2/3] fix: veto @inline never by name so it survives optimization (#132 review) 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. --- ...0260625_210853_unisay_inline_never_veto.md | 8 +-- lib/Language/PureScript/Backend/IR/Inliner.hs | 9 ++-- .../PureScript/Backend/IR/Optimizer.hs | 51 ++++++++++++------- .../PureScript/Backend/IR/Optimizer/Spec.hs | 7 +-- 4 files changed, 46 insertions(+), 29 deletions(-) diff --git a/changelog.d/20260625_210853_unisay_inline_never_veto.md b/changelog.d/20260625_210853_unisay_inline_never_veto.md index a7309fd3..2f06e5ee 100644 --- a/changelog.d/20260625_210853_unisay_inline_never_veto.md +++ b/changelog.d/20260625_210853_unisay_inline_never_veto.md @@ -3,6 +3,8 @@ - `@inline never` now actually prevents inlining. The annotation was parsed and stored but never consulted as a veto: both inlining sites decided with `isInlinableExpr expr || `, so a `never`-annotated binding - that was a reference, a small literal, or used once was still inlined. A new - `inlineForbidden` check now vetoes inlining of any `Just Never` binding at - both sites, overriding the heuristic and the single-use rule (#131). + that was a reference, a small literal, or used once was still inlined. The + optimizer now collects the `never`-annotated binding names once up front, so + the veto survives later rewrites that drop the annotation, and refuses to + inline those bindings regardless of the heuristic or the single-use rule + (#131). diff --git a/lib/Language/PureScript/Backend/IR/Inliner.hs b/lib/Language/PureScript/Backend/IR/Inliner.hs index 0b88d54b..53e37732 100644 --- a/lib/Language/PureScript/Backend/IR/Inliner.hs +++ b/lib/Language/PureScript/Backend/IR/Inliner.hs @@ -19,11 +19,12 @@ travels to the optimizer's inlining decision through several stages: moves each into the annotated binding's 'Ann' as the binding is translated (see Note [Inliner annotations must all be consumed]). 3. From there the 'Annotation' rides along as the expression's @ann@. - 4. The optimizer reads it back with 'getAnn': @Just Always@ (via + 4. The optimizer reads it back: @Just Always@ (via 'Language.PureScript.Backend.IR.Optimizer.isInlinableExpr') forces - inlining, and @Just Never@ (via 'inlineForbidden') vetoes it outright, - overriding the ref / small-literal / single-use heuristic. @Nothing@ - leaves that heuristic to decide. + inlining. For @Just Never@, 'optimizedUberModule' collects the annotated + binding names once up front (so the veto survives later rewrites that drop + the annotation off a binding's root) and refuses to inline them. @Nothing@ + leaves the ref / small-literal / single-use heuristic to decide. The linker also synthesises @Just Always@ directly: each foreign name is bound to an 'ObjectProp' marked 'Inline.Always' so the wrapper around the FFI object diff --git a/lib/Language/PureScript/Backend/IR/Optimizer.hs b/lib/Language/PureScript/Backend/IR/Optimizer.hs index 11d5cbf8..7a50e90b 100644 --- a/lib/Language/PureScript/Backend/IR/Optimizer.hs +++ b/lib/Language/PureScript/Backend/IR/Optimizer.hs @@ -38,27 +38,33 @@ import Language.PureScript.Backend.IR.Types ) optimizedUberModule ∷ UberModule → UberModule -optimizedUberModule = - idempotently (eliminateDeadCode . optimizeModule) +optimizedUberModule uber = + uber + & idempotently (eliminateDeadCode . optimizeModule neverNames) -- by merging foreign bindings into the main bindings, we can -- unblock even more optimizations, e.g. inline foreign bindings. - >>> mergeForeignsIntoBindings - >>> idempotently (eliminateDeadCode . optimizeModule) + & mergeForeignsIntoBindings + & idempotently (eliminateDeadCode . optimizeModule neverNames) -- Must run last among the index-sensitive passes: -- see Note [Locals are uniquely named after renameShadowedNames] - >>> renameShadowedNames + & renameShadowedNames -- Magic-do is the final lowering (issue #46): it relies on the unique -- naming established above and preserves it, and must run after dead-code -- elimination so the statements it introduces for `discard` are not -- dropped as dead. See Language.PureScript.Backend.IR.MagicDo. - >>> magicDo + & magicDo -- Flatten the remaining deeply-nested expression trees (issues #104, #108): -- continuation/bind chains of any monad (lambda-lifted into $kont helpers) -- and applicative/flipped-bind application spines (A-normalised into $tmp -- locals). Runs after magicDo (which consumes Effect/ST chains, leaving only -- non-Effect/ST ones) and likewise consumes and preserves the unique naming. -- See Language.PureScript.Backend.IR.FlattenDeepBinds. - >>> flattenDeepBinds + & flattenDeepBinds + where + -- Collect @inline never bindings once from the pristine module: later + -- rewrites may strip the annotation off a binding's root, so the veto keys + -- off the name (see Note [Inline annotations and inlining heuristics]). + neverNames = neverInlineNames uber mergeForeignsIntoBindings ∷ UberModule → UberModule mergeForeignsIntoBindings uberModule@UberModule {..} = @@ -230,8 +236,22 @@ idempotently = fix $ \i f a → -- tr ∷ Show x ⇒ String → x → y → y -- tr l x y = trace ("\n\n" <> l <> "\n" <> (toString . pShow) x <> "\n") y -optimizeModule ∷ UberModule → UberModule -optimizeModule UberModule {..} = +{- | The top-level bindings annotated @inline never@, collected once from the +pristine module. Later rewrites can drop the annotation off a binding's root +expression (e.g. constant folding replaces it with a fresh node), so the veto +must key off the name rather than re-reading the annotation after optimization. +See Note [Inline annotations and inlining heuristics]. +-} +neverInlineNames ∷ UberModule → Set QName +neverInlineNames UberModule {uberModuleBindings} = + Set.fromList + [ qname + | Standalone (qname, expr) ← uberModuleBindings + , getAnn expr == Just Never + ] + +optimizeModule ∷ Set QName → UberModule → UberModule +optimizeModule neverNames UberModule {..} = UberModule { uberModuleForeigns , uberModuleBindings = uberModuleBindings' @@ -249,7 +269,8 @@ optimizeModule UberModule {..} = withBinding binding (bindings, exports) = case binding of Standalone (qname, optimizedExpression → expr) → - if not (inlineForbidden expr) + -- See Note [Inline annotations and inlining heuristics] + if qname `Set.notMember` neverNames && (isInlinableExpr expr || isUsedOnce qname) then ( substituteInBindings qname expr bindings @@ -428,18 +449,10 @@ inlineLocalBinding grouping body = case grouping of RecursiveGroup _grp → body -- Not going to inline recursive bindings Standalone (_ann, Local → name, inlinee) → - if not (inlineForbidden inlinee) - && (isInlinableExpr inlinee || countFreeRef name body == 1) + if isInlinableExpr inlinee || countFreeRef name body == 1 then substitute name 0 inlinee body else body -{- | @inline never@ vetoes inlining outright, overriding both the -'isInlinableExpr' heuristic and the single-use rule. See -Note [Inline annotations and inlining heuristics]. --} -inlineForbidden ∷ Exp → Bool -inlineForbidden expr = getAnn expr == Just Never - -- See Note [Inline annotations and inlining heuristics] isInlinableExpr ∷ Exp → Bool isInlinableExpr expr = diff --git a/test/Language/PureScript/Backend/IR/Optimizer/Spec.hs b/test/Language/PureScript/Backend/IR/Optimizer/Spec.hs index 1af645dd..9ffb10c2 100644 --- a/test/Language/PureScript/Backend/IR/Optimizer/Spec.hs +++ b/test/Language/PureScript/Backend/IR/Optimizer/Spec.hs @@ -169,9 +169,10 @@ spec = describe "IR Optimizer" do describe "respects @inline never (issue #131)" do test "keeps a never-annotated top-level binding instead of inlining it" do let mainModule = moduleNameFromString "Main" - -- foo = 42, annotated `@inline never`: a literal used once, which the - -- optimizer would otherwise inline and drop. - fooExp = LiteralInt (Just Never) 42 + -- foo = (1 == 1) with `@inline never`. Constant folding rewrites the + -- root to `true` (dropping the annotation) and foo is used once, so + -- without a name-based veto it would be inlined away. + fooExp = Eq (Just Never) (literalInt 1) (literalInt 1) original = Linker.UberModule { uberModuleForeigns = [] From 30e32ff451bc8d4d0dcf6bdac8cc2824f130b53d Mon Sep 17 00:00:00 2001 From: Yura Lazarev Date: Fri, 26 Jun 2026 08:36:19 +0200 Subject: [PATCH 3/3] docs: clarify pragmas govern non-foreign bindings (#132 review) --- lib/Language/PureScript/Backend/IR/Inliner.hs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/Language/PureScript/Backend/IR/Inliner.hs b/lib/Language/PureScript/Backend/IR/Inliner.hs index 53e37732..04d10b94 100644 --- a/lib/Language/PureScript/Backend/IR/Inliner.hs +++ b/lib/Language/PureScript/Backend/IR/Inliner.hs @@ -26,9 +26,11 @@ travels to the optimizer's inlining decision through several stages: the annotation off a binding's root) and refuses to inline them. @Nothing@ leaves the ref / small-literal / single-use heuristic to decide. -The linker also synthesises @Just Always@ directly: each foreign name is bound -to an 'ObjectProp' marked 'Inline.Always' so the wrapper around the FFI object -is always inlined away (see +Pragmas reach this map only for non-foreign top-level bindings (the ones +'useAnnotation' drains as it translates them). The linker synthesises +@Just Always@ separately and independently of any pragma: each foreign name is +bound to an 'ObjectProp' marked 'Inline.Always' so the wrapper around the FFI +object is always inlined away (see Note [Foreign bindings structure emitted by the Linker]). -} data Annotation = Always | Never