refactor: rename statement evaluation metadata#10078
Merged
Merged
Conversation
✅ Deploy Preview for rolldown-rs canceled.
|
hyfdev
changed the title
hyfdev
marked this pull request as ready for review
July 2, 2026 05:13
Member
Author
Merge activity
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors Rolldown’s statement side-effect metadata to better reflect that the scanner records multiple evaluation facts (not just “has side effects”), and it makes the tree-shaking predicate explicit via has_side_effect_for_tree_shaking() while keeping execution-order sensitivity checks separate.
Changes:
- Renamed
SideEffectDetail→StmtEvalFlagsandSideEffectDetector→StmtEvalAnalyzer, updating all affected call sites. - Replaced
StmtInfo.side_effectwithStmtInfo.eval_flags, and renamed theUnknownflag toUnknownSideEffect. - Updated tree-shaking and module-side-effects queries to use the new flag names and the explicit
has_side_effect_for_tree_shaking()helper where appropriate.
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 |
|---|---|
| crates/rolldown/tests/rolldown/function/experimental/strict_execution_order/pure_annotation_local_fn_reads_global/dep.js | Updates test commentary to use the new UnknownSideEffect name. |
| crates/rolldown/src/stages/link_stage/wrapping.rs | Updates generated wrapper statement metadata from side_effect to eval_flags. |
| crates/rolldown/src/stages/link_stage/tree_shaking/include_statements.rs | Switches tree-shaking checks to eval_flags, using UnknownSideEffect / has_side_effect_for_tree_shaking(). |
| crates/rolldown/src/stages/link_stage/reference_needed_symbols.rs | Replaces side_effect assignments with eval_flags for import/reexport statement info. |
| crates/rolldown/src/stages/link_stage/generate_lazy_export.rs | Updates CommonJS lazy-export path to mark eval_flags instead of side_effect. |
| crates/rolldown/src/stages/link_stage/cross_module_optimization.rs | Renames detector usage to StmtEvalAnalyzer and updates mutation plumbing to StmtEvalFlags. |
| crates/rolldown/src/stages/link_stage/create_exports_for_ecma_modules.rs | Updates synthesized StmtInfo initialization to use eval_flags. |
| crates/rolldown/src/module_loader/runtime_module_task.rs | Updates runtime module side-effect determination to query UnknownSideEffect on eval_flags. |
| crates/rolldown/src/ecmascript/ecma_module_view_factory.rs | Updates lazy CJS side-effect check to use eval_flags + UnknownSideEffect. |
| crates/rolldown/src/ast_scanner/stmt_eval_analyzer/mod.rs | Renames and reshapes the analyzer to produce StmtEvalFlags (including new helper naming in tests). |
| crates/rolldown/src/ast_scanner/mod.rs | Updates scanner post-processing to set UnknownSideEffect on eval_flags. |
| crates/rolldown/src/ast_scanner/impl_visit.rs | Uses StmtEvalAnalyzer and sets StmtInfo.eval_flags; updates execution-order sensitivity flag intersection. |
| crates/rolldown_common/src/types/stmt_info.rs | Replaces StmtInfo.side_effect with StmtInfo.eval_flags and updates debug struct accordingly. |
| crates/rolldown_common/src/types/stmt_eval_flags.rs | Adds the new StmtEvalFlags bitflags type and has_side_effect_for_tree_shaking() helper. |
| crates/rolldown_common/src/types/side_effect_detail.rs | Removes the old SideEffectDetail type. |
| crates/rolldown_common/src/types/mod.rs | Re-exports stmt_eval_flags instead of side_effect_detail. |
| crates/rolldown_common/src/lib.rs | Re-exports StmtEvalFlags from rolldown_common and drops SideEffectDetail. |
Merging this PR will not alter performance
Comparing 
Footnotes
|
IWANABETHATGUY
approved these changes
Jul 2, 2026
## Summary - Rename `SideEffectDetail` to `StmtEvalFlags` and `SideEffectDetector` to `StmtEvalAnalyzer`. - Rename `StmtInfo.side_effect` to `StmtInfo.eval_flags` and `Unknown` to `UnknownSideEffect`. - Make the tree-shaking predicate explicit as `has_side_effect_for_tree_shaking()` while keeping execution-order-sensitive checks separate. ## Why The old names suggested this data only answered "does this statement have side effects?". That is no longer accurate. The scanner now records several facts observed while evaluating a statement, and different consumers interpret those facts differently. For tree shaking, only `PureCjs` and `UnknownSideEffect` mean an otherwise unused statement must be kept. But `GlobalVarAccess` and `PureAnnotation` can be side-effect-free for tree-shaking purposes while still making the module execution-order-sensitive. In strict execution order mode, those flags prevent Rolldown from incorrectly skipping or eliding wrappers when the timing of evaluation is observable. This rename makes the split visible in the API: callers use `has_side_effect_for_tree_shaking()` when they are making a tree-shaking decision, and inspect specific `StmtEvalFlags` when they are making execution-order or metadata decisions. ## Validation - `just fix-rust` - `git diff --check` - `cargo test -p rolldown stmt_eval_analyzer` - `just lint-rust` - `just t-run crates/rolldown/tests/rolldown/function/experimental/strict_execution_order/pure_annotation_local_fn_reads_global/_config.json` - `cargo test -p rolldown --test integration strict_execution_order`
graphite-app
Bot
force-pushed
the
codex/stmt-eval-flags
branch
from
July 2, 2026 05:39
ebef016 to
a952b80
Compare
Merged
shulaoda
added a commit
that referenced
this pull request
Jul 8, 2026
## [1.1.5] - 2026-07-08 ### 🚀 Features - detect top-level import-binding reads as execution-order sensitive (#10180) by @hyf0 - sourcemap_filenames: add a sourcemapFileNames option (#9271) by @V1OL3TF0X - binding: record plugin hook result kind in tracing spans (#10154) by @IWANABETHATGUY - linking: skip side-effect-free modules in per-entry reachability (#10111) by @IWANABETHATGUY - improve error message for unresolved virtual imports (#10156) by @sapphi-red - add descriptive metadata to plugin API (#10106) by @sapphi-red - add `--configLoader=native` option (#10118) by @sapphi-red ### 🐛 Bug Fixes - improve invalid annotation warnings (#10185) by @hyf0 - keep deduplicated asset filenames stable once they can be observed (#10191) by @shulaoda - sourcemap_filenames: use public option name in pattern errors (#10188) by @IWANABETHATGUY - sourcemap_filenames: hash prepared sourcemap content (#10178) by @hyf0 - tree-shake unused circular declarators exported via export list (#10166) by @IWANABETHATGUY - dev: don't panic when an HMR rebuild hits an unresolved import (#10162) by @shulaoda - propagate errors from output.globals function (#9880) by @shulaoda - dev: revert cache mutations when a partial scan fails (#10110) by @shulaoda - dev: update importer relationships of cached modules in incremental build (#10107) by @shulaoda - hmr: fall back to full reload when a changed module is not registered as executed (#10132) by @shulaoda - chunk-optimizer: follow entry facade edges in runtime placement cycle check (#10101) by @hyf0 - dev: ignore watcher events after close (#10113) by @hyf0 - emit async wrapper for TLA modules under onDemandWrapping (#10086) by @IWANABETHATGUY - gate sideEffects:false modules' side effects on body demand (#10080) by @IWANABETHATGUY - rolldown_plugin_vite_resolve: return empty object for `browser: false` mapped modules (#10082) by @sapphi-red - reset the word-boundary state on newline in Hires::Boundary sourcemaps (#10025) by @shulaoda - trim an emptied chunk's outro/intro instead of skipping past it (#10029) by @shulaoda - test each edited chunk's own start against indent exclude ranges (#10026) by @shulaoda - preserve sourcemap mappings for indented lines when a CJS module shares the chunk (#10074) by @hyf0 ### 🚜 Refactor - separate tree-shaking side effects from execution order sensitivity (#10168) by @hyf0 - type construct_vite_preload_call to take an ObjectPattern (#10135) by @shulaoda - treeshake: single-source the own-export classification shared with the lazy-barrel loader (#10098) by @IWANABETHATGUY - dev: reuse Vite's bundledDev server (#10081) by @h-a-n-a - clippy: ban std HashMap/HashSet in favour of FxHashMap/FxHashSet (#10108) by @Boshen - treeshake: make body demand a second module bit instead of a stmt multimap (#10097) by @IWANABETHATGUY - seal used_symbol_refs by construction after its last writer (#10091) by @hyf0 - treeshake: replace inclusion mutual recursion with a worklist engine (#10096) by @IWANABETHATGUY - treeshake: split include_statements.rs into focused modules (#10095) by @IWANABETHATGUY - drop redundant is_user_defined filter on partitioned entries (#10050) by @shulaoda - project the retained export interface out of used_symbol_refs (#10089) by @hyf0 - track used external symbols separately from used_symbol_refs (#10088) by @hyf0 - make module namespace inclusion an explicit linking metadata field (#10087) by @hyf0 - rename statement evaluation metadata (#10078) by @hyf0 ### 📚 Documentation - virtual modules user-facing id convention (#10155) by @sapphi-red - cli: clarify disabling boolean/object flags like codeSplitting (#10153) by @IWANABETHATGUY - chore: remove Vite+ alpha banner (#10105) by @mdong1909 - write down the used_symbol_refs contract (#10090) by @hyf0 - dev/lazy: update design and implementation (#10079) by @h-a-n-a ### ⚡ Performance - ast_scanner: stop order-sensitivity checks once a module is flagged (#10190) by @IWANABETHATGUY - return impl ExactSizeIterator from slice-backed accessors (#10133) by @Boshen - binding: box dev and watcher napi futures (#10103) by @Boshen ### 🧪 Testing - move string_wizard replace unit tests to the JS magic-string suite (#10176) by @IWANABETHATGUY - dev: assert incremental scan state matches a fresh full build after each HMR step (#10115) by @shulaoda - dev: restore runtime assertions of delete_file_not_used_anymore (#10112) by @shulaoda - dev: fix flaky dev server tests in CI (#10152) by @h-a-n-a - add regression test for #10099 (lazyBarrel drops default-import binding but keeps its property reads) (#10109) by @IWANABETHATGUY ### ⚙️ Miscellaneous Tasks - deploy website to Void via GitHub OIDC (#10192) by @Boshen - deps: update oxc to 0.139.0 (#10161) by @shulaoda - deps: update test262 submodule for tests (#10160) by @rolldown-guard[bot] - rolldown_plugin_utils: remove dead asset-url and css scaffolding (#10131) by @shulaoda - deps: revert vite-plus to v0.2.1 (#10148) by @shulaoda - deps: update github actions (#10141) by @renovate[bot] - deps: update dependency rust to v1.96.1 (#10145) by @renovate[bot] - deps: update npm packages (#10142) by @renovate[bot] - deps: update rust crates (#10143) by @renovate[bot] - deps: update napi to v3.10.3 (#10121) by @renovate[bot] - rolldown_utils: remove unused time module (#10138) by @shulaoda - remove dead CopyModulePlugin::is_active method (#10129) by @shulaoda - remove dead LazyCompilationContext::is_lazy_module method (#10128) by @shulaoda - remove dead BuildDiagnostic::downcast_ref method (#10127) by @shulaoda - deps: update dependency vite-plus to v0.2.2 (#10084) by @renovate[bot] - deps: update rust crate oxc_sourcemap to v8.1.0 (#10122) by @renovate[bot] - deps: update crate-ci/typos action to v1.48.0 (#10124) by @renovate[bot] - enable more clippy restriction lints (#10114) by @Boshen - deps: update rust dependencies (#10100) by @Boshen - deps: update oxc resolver to v11.23.0 (#10083) by @renovate[bot] ###◀️ Revert - Revert "chore(deps): revert vite-plus to v0.2.1" (#10157) by @h-a-n-a - "fix(hmr): fall back to full reload when a changed module is not registered as executed (#10132)" (#10151) by @shulaoda ### ❤️ New Contributors * @V1OL3TF0X made their first contribution in [#9271](#9271) Co-authored-by: shulaoda <[email protected]>
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
SideEffectDetailtoStmtEvalFlagsandSideEffectDetectortoStmtEvalAnalyzer.StmtInfo.side_effecttoStmtInfo.eval_flagsandUnknowntoUnknownSideEffect.has_side_effect_for_tree_shaking()while keeping execution-order-sensitive checks separate.Why
The old names suggested this data only answered "does this statement have side effects?". That is no longer accurate. The scanner now records several facts observed while evaluating a statement, and different consumers interpret those facts differently.
For tree shaking, only
PureCjsandUnknownSideEffectmean an otherwise unused statement must be kept. ButGlobalVarAccessandPureAnnotationcan be side-effect-free for tree-shaking purposes while still making the module execution-order-sensitive. In strict execution order mode, those flags prevent Rolldown from incorrectly skipping or eliding wrappers when the timing of evaluation is observable.This rename makes the split visible in the API: callers use
has_side_effect_for_tree_shaking()when they are making a tree-shaking decision, and inspect specificStmtEvalFlagswhen they are making execution-order or metadata decisions.Validation
just fix-rustgit diff --checkcargo test -p rolldown stmt_eval_analyzerjust lint-rustjust t-run crates/rolldown/tests/rolldown/function/experimental/strict_execution_order/pure_annotation_local_fn_reads_global/_config.jsoncargo test -p rolldown --test integration strict_execution_order