refactor: seal used_symbol_refs by construction after its last writer#10091
Conversation
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. |
2965e4c to
4c189da
Compare
There was a problem hiding this comment.
Pull request overview
This PR enforces the documented used_symbol_refs “no mutation after inclusion finishes” contract at the type level by splitting it into a mutable builder phase and a sealed read-only phase, and then plumbing the sealed value through generation/rendering without storing it on LinkStageOutput.
Changes:
- Introduce
UsedSymbolRefsBuilder(mutable) +UsedSymbolRefs(sealed, read-only) withseal()consuming the builder. - Thread the builder through the link/generate boundary, seal it after
generate_chunks(after the chunk optimizer’s last write), and pass the sealed refs throughGenerateContext. - Remove the namespace-decision mirror writes into
used_symbol_refs, and update the affected downstream liveness decisions to consultmeta.namespace_includedexplicitly.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/rolldown/src/types/generator.rs | Adds used_symbol_refs: &UsedSymbolRefs to GenerateContext so renderers read the sealed set explicitly. |
| crates/rolldown/src/stages/link_stage/tree_shaking/include_statements.rs | Switches inclusion machinery to write into UsedSymbolRefsBuilder instead of the sealed type. |
| crates/rolldown/src/stages/link_stage/mod.rs | Removes used_symbol_refs from LinkStageOutput and returns the builder separately from link(). |
| crates/rolldown/src/stages/generate_stage/render_chunk_to_assets.rs | Plumbs sealed UsedSymbolRefs through chunk instantiation/rendering context construction. |
| crates/rolldown/src/stages/generate_stage/mod.rs | Accepts the builder in generate(), seals it after generate_chunks, and threads sealed refs into downstream passes. |
| crates/rolldown/src/stages/generate_stage/compute_cross_chunk_links.rs | Updates cross-chunk liveness filtering to use namespace_included for namespace refs and sealed refs otherwise. |
| crates/rolldown/src/stages/generate_stage/code_splitting.rs | Threads the builder into chunk generation/optimization and updates retained-export projection to consult namespace_included for namespace refs. |
| crates/rolldown/src/stages/generate_stage/chunk_optimizer.rs | Makes the optimizer’s inclusion re-run mutate via the shared UsedSymbolRefsBuilder. |
| crates/rolldown/src/ecmascript/format/esm.rs | Updates ESM formatting logic to read ctx.used_symbol_refs instead of link_output.used_symbol_refs. |
| crates/rolldown/src/bundle/bundle.rs | Updates bundling pipeline to receive the builder from link stage and pass it into generate stage. |
| crates/rolldown_common/src/types/used_symbol_refs.rs | Implements the builder/sealed split and removes post-seal mutation APIs (e.g. remove). |
| crates/rolldown_common/src/lib.rs | Re-exports UsedSymbolRefsBuilder alongside UsedSymbolRefs. |
Merging this PR will not alter performance
Comparing Footnotes
|
4f81d80 to
b60ec6b
Compare
286b070 to
111e607
Compare
b60ec6b to
fcc4aad
Compare
…a field (#10087) ### Description Part 1 of a 4-PR stack that gives "is this symbol used" a single authority per question, by splitting purpose-specific views out of `used_symbol_refs`. Whether a module's namespace object is retained was previously communicated to downstream passes by inserting/removing the module's `namespace_object_ref` in `used_symbol_refs` (#7002). That made the set a mutable side channel after tree-shaking had settled: from that point on the set and the statement-inclusion bits could disagree, and each reader picked one of them ad hoc. This PR records the decision on a dedicated field, `LinkingMetadata::namespace_included`, computed in `finalized_module_namespace_ref_usage` from `module_namespace_included_reason`, and switches the dedicated readers over: - the finalizer's namespace-declaration gate (`finalizer_context.rs`) - the JSON export-interface check in `compute_cross_chunk_links` The set insert/remove remains as a mirror derived from the flag, because generic symbol-usage queries can still receive namespace refs (e.g. a `resolved_export.symbol_ref` produced by `export * as ns from '...'`). No behavior change; snapshots are untouched. Stack: #10087 → #10088 → #10089 → #10090 → #10091
#10088) ### Description Part 2 of the `used_symbol_refs` decomposition stack (stacked on #10087). Symbols owned by external modules — an external module's `namespace_ref`, and the per-name facade symbols created by the external import binding merger in `bind_imports_and_exports` — have no statements, so their usage can only ever be tracked symbol-level. This PR gives them a dedicated `UsedExternalSymbols` set, written by `include_symbol` alongside `used_symbol_refs` whenever the inserted ref's owner is an external module, and switches the external-only consumers to it: - ESM import specifier rendering (`format/esm.rs`) - CJS/ESM external interop checks (`format/cjs.rs`, `format/utils/mod.rs`) - chunk-level deconflicting (`deconflict_chunk_symbols.rs`) The ESM named-import path keeps a `used_symbol_refs` fallback for canonical refs that stay importer-local: a named import that is itself re-exported skips the binding merger, so its canonical ref is not external-owned. (Part 3 initially tried to move that fallback to the export projection; it turned out to also cover eval-kept imports and platform-import cases that are no module's export, so it stays on the set — see the description there.) No behavior change; snapshots are untouched. Stack: #10087 → #10088 → #10089 → #10090 → #10091
…fs (#10089) ### Description Part 3 of the `used_symbol_refs` decomposition stack (stacked on #10088). Consumers that iterate a module's resolved exports were answering "is this export retained" by querying `used_symbol_refs` directly. This PR projects that answer into a dedicated `RetainedExportSymbols` set — for every module's `resolved_exports` entry whose usage survived, the export's `symbol_ref` as recorded plus its canonical form — computed right after the module-namespace decision so `export * as ns` exports reflect it. Switched consumers: - rendered-module export lists (`ecma_generator.rs`) - the namespace object property list in the finalizer - entry-chunk export naming and AllowExtension emitted-chunk exports (`compute_cross_chunk_links`) With both this and #10087 in place, the finalizer context's `used_symbol_refs` field had no readers left, so it is removed here. Note: the ESM external-import fallback from #10088 deliberately does NOT move to this projection. An instrumented attempt changed 20 snapshots: the non-external canonical refs it sees include imports kept alive by `eval` and platform-import cases, which are no module's export, so the export projection cannot answer them. No behavior change; snapshots are untouched. Stack: #10087 → #10088 → #10089 → #10090 → #10091
### Description Part 4 (last) of the `used_symbol_refs` decomposition stack (stacked on #10089). The original endgame was to delete `UsedSymbolRefs` entirely once every consumer had a purpose-specific home. An instrumented run over the whole test corpus rejected that: the cross-chunk filters (`compute_cross_chunk_links`) drop three populations of refs — - ~1.3k inlinable constants (absent from the set by design: their use sites hold the value) - ~0.7k namespace objects eliminated by the generate stage - **~1.3k over-collected depended refs that neither constants nor namespaces explain** — refs only the inclusion fixpoint's own record can rule dead The last population is not answerable by statement-inclusion bits (one bit per statement cannot distinguish which of a statement's bindings is referenced), nor by any of the new views. So the set stays — as the single record of fixpoint liveness. This PR writes that contract on the type: what membership means, the three sanctioned writers (the inclusion pass, the chunk optimizer's facade-elimination re-run, and the namespace-decision mirror), and pointers to the purpose-specific views (`LinkingMetadata::namespace_included`, `UsedExternalSymbols`, `RetainedExportSymbols`) that answer the common questions. The over-collected population also suggests `collect_depended_symbols` collects more than needed; that is left as a separate investigation. Docs/comments only; no behavior change. Stack: #10087 → #10088 → #10089 → #10090 → #10091
### Description Part 4 (last) of the `used_symbol_refs` decomposition stack (stacked on #10089). The original endgame was to delete `UsedSymbolRefs` entirely once every consumer had a purpose-specific home. An instrumented run over the whole test corpus rejected that: the cross-chunk filters (`compute_cross_chunk_links`) drop three populations of refs — - ~1.3k inlinable constants (absent from the set by design: their use sites hold the value) - ~0.7k namespace objects eliminated by the generate stage - **~1.3k over-collected depended refs that neither constants nor namespaces explain** — refs only the inclusion fixpoint's own record can rule dead The last population is not answerable by statement-inclusion bits (one bit per statement cannot distinguish which of a statement's bindings is referenced), nor by any of the new views. So the set stays — as the single record of fixpoint liveness. This PR writes that contract on the type: what membership means, the three sanctioned writers (the inclusion pass, the chunk optimizer's facade-elimination re-run, and the namespace-decision mirror), and pointers to the purpose-specific views (`LinkingMetadata::namespace_included`, `UsedExternalSymbols`, `RetainedExportSymbols`) that answer the common questions. The over-collected population also suggests `collect_depended_symbols` collects more than needed; that is left as a separate investigation. Docs/comments only; no behavior change. Stack: #10087 → #10088 → #10089 → #10090 → #10091
937fc9b to
6a679a7
Compare
fcc4aad to
2504208
Compare
6a679a7 to
92d5909
Compare
✅ Deploy Preview for rolldown-rs canceled.
|
Merge activity
|
92d5909 to
51dd2c6
Compare
…#10091) ### Description Part 5 (follow-up to the review discussion) of the `used_symbol_refs` decomposition stack (stacked on #10090). #10090 documented by convention that `used_symbol_refs` must not change after the inclusion machinery finishes. This PR enforces it at the type level instead: - The mutable phase is a separate `UsedSymbolRefsBuilder` (`insert`/`contains`), held by the link-stage fixpoint and threaded explicitly through `generate_chunks` into the chunk optimizer's facade-elimination re-run — the last writer. `remove()` no longer exists at all. - `seal()` consumes the builder and yields `UsedSymbolRefs`, which only exposes `contains()`. Downstream passes (retained-export projection, cross-chunk linking, chunk output exports, chunk rendering) receive the sealed value. Mutating after the seal point is a compile error, not a runtime check. - The set no longer lives on `LinkStageOutput`; the ESM generator reads it through a dedicated `GenerateContext` field. This also deletes the namespace-decision mirror left by #10087: `finalized_module_namespace_ref_usage` now only writes `meta.namespace_included`, and the three places whose answers previously depended on the mirrored set membership for namespace refs — the two cross-chunk liveness filters and the retained-export projection — consult the flag explicitly. `UsedExternalSymbols` and `RetainedExportSymbols` could get the same builder/sealed split as a follow-up. No behavior change; snapshots are untouched. Stack: #10087 → #10088 → #10089 → #10090 → #10091
51dd2c6 to
8a4cc39
Compare
## [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]>

Description
Part 5 (follow-up to the review discussion) of the
used_symbol_refsdecomposition stack (stacked on #10090).#10090 documented by convention that
used_symbol_refsmust not change after the inclusion machinery finishes. This PR enforces it at the type level instead:UsedSymbolRefsBuilder(insert/contains), held by the link-stage fixpoint and threaded explicitly throughgenerate_chunksinto the chunk optimizer's facade-elimination re-run — the last writer.remove()no longer exists at all.seal()consumes the builder and yieldsUsedSymbolRefs, which only exposescontains(). Downstream passes (retained-export projection, cross-chunk linking, chunk output exports, chunk rendering) receive the sealed value. Mutating after the seal point is a compile error, not a runtime check.LinkStageOutput; the ESM generator reads it through a dedicatedGenerateContextfield.This also deletes the namespace-decision mirror left by #10087:
finalized_module_namespace_ref_usagenow only writesmeta.namespace_included, and the three places whose answers previously depended on the mirrored set membership for namespace refs — the two cross-chunk liveness filters and the retained-export projection — consult the flag explicitly.UsedExternalSymbolsandRetainedExportSymbolscould get the same builder/sealed split as a follow-up.No behavior change; snapshots are untouched.
Stack: #10087 → #10088 → #10089 → #10090 → #10091