refactor(treeshake): scan pure-annotated IIFE bodies for global reads#9432
Closed
Dunqing wants to merge 1 commit into
Closed
Conversation
`SideEffectDetector::detect_side_effect_of_call_expr` treats pure-annotated calls as fully side-effect-free and never recurses into the callee's body, so a pure-annotated IIFE like `/* @__PURE__ */ (() => globalValue)()` loses the fact that its body reads an unresolved global. Reading a top-level global IS an execution-order dependence — the global may be assigned by a sibling module — even when the call itself is side-effect-free per the annotation. The two concepts are orthogonal but the detector currently conflates them: the wrap trigger in `impl_visit.rs:92-98` uses `PureAnnotation` as a "be cautious" proxy because `GlobalVarAccess` isn't being produced for these cases. Add an `iife_body()` helper (handles arrow and function-expression callees, peeled through `Expression::get_inner_expression`) and, when the call is pure-annotated, scan each body statement through the existing detector, masking the result to `GlobalVarAccess` only. `Unknown` is deliberately not propagated — the annotation already certified no side effect at the call level; this scan exists strictly to harvest the order-sensitivity signal. This is purely additive: the wrap trigger still intersects with `PureAnnotation`, so cases that previously wrapped continue to wrap. The new `GlobalVarAccess` signal sits alongside `PureAnnotation` for now. Phase 1 of replacing `PureAnnotation`-as-wrap-trigger with `GlobalVarAccess`. A follow-up will retire `PureAnnotation` from the trigger (`impl_visit.rs:92-98`) — modules that were over-wrapping (pure call to local empty function with no globals) stop wrapping; modules that need wrapping (pure IIFE reading globals) keep wrapping via the `GlobalVarAccess` path that this commit makes possible. Parameter defaults that evaluate globals (`/* @__PURE__ */ ((x = globalValue) => 1)()`) are not yet covered — noted as a follow-up in the code comment.
Contributor
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
How to use the Graphite Merge QueueAdd the label graphite: merge-when-ready to this PR to add it to the merge queue. You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
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.

Stacked on #9431.
Motivation
SideEffectDetector::detect_side_effect_of_call_exprtreats pure-annotated calls as fully side-effect-free and never recurses into the callee's body. So a pure-annotated IIFE like/* @__PURE__ */ (() => globalValue)()loses the fact that its body reads an unresolved global.That matters because the two concepts are orthogonal:
/* @__PURE__ */ (() => 1)()/* @__PURE__ */ (() => globalValue)()globalValuemay be set by sibling module)Reading a top-level unresolved global IS an execution-order dependence even when the call is side-effect-free per the annotation. The current wrap trigger in
impl_visit.rs:92-98papers over this by usingPureAnnotationas a "be cautious" proxy, which over-marks (row 1 wraps too) while under-modeling (the actual signal —GlobalVarAccessfrom the body — is missing).Changes
iife_body()free function: returns theFunctionBodyifcallee.get_inner_expression()isArrowFunctionExpressionorFunctionExpression. Transparent wrappers ((callee),callee as T, etc.) are peeled byget_inner_expression.detect_side_effect_of_call_expr, when the call is pure-annotated andiife_body()returnsSome, walk each body statement through the existing detector and OR in only theGlobalVarAccessbit.Unknownis deliberately masked off — the annotation already certified no side effect at the call level; this scan exists strictly to harvest the order-sensitivity signal.Why no snapshot churn
Purely additive. The wrap trigger still intersects with
PureAnnotation, so cases that previously wrapped continue to wrap. The newGlobalVarAccesssignal sits alongsidePureAnnotationfor now.Phase 1 of a multi-step reconcile
This PR is the first of three steps to resolve a design tension in
SideEffectDetail::PureAnnotation:GlobalVarAccessfor pure-annotated IIFE bodies. Additive. No behavior change.PureAnnotationfrom the wrap trigger inimpl_visit.rs:92-98. Reconciles with the sibling branchfix/remove-pure-annotation-execution-order-sensitive. Modules that were over-wrapping (pure call to local empty function with no globals) stop wrapping; modules that need wrapping (pure IIFE reading globals) keep wrapping via theGlobalVarAccesspath this PR adds. Will have snapshot churn — that's the point.SideEffectDetail::PureAnnotationflag entirely if no consumer remains.Not addressed
Parameter defaults that evaluate globals at call time —
/* @__PURE__ */ ((x = globalValue) => 1)(). Thex = globalValueruns but isn't insidebody.statements. Noted as a follow-up in the code comment. Probably a small extension toiife_body()-adjacent code; not blocking the Phase 1/2 reconcile.Tests
test_pure_annotation_propagates_through_compound_expr— existing test, expectations updated. Cases that readglobalValuenow expectPureAnnotation | GlobalVarAccess(the new combined semantic). Cases with bodies that don't read globals ((() => 2)(),(() => true)(),(() => 'k')()) still expect justPureAnnotation.test_pure_iife_body_global_propagates_global_var_access— new test, direct coverage for the body scan: arrow with expression body, arrow with block + explicit return, function expression IIFE, pure IIFE with no globals (negative case), member expression rooted at an unresolved global.