Skip to content

refactor: seal used_symbol_refs by construction after its last writer#10091

Merged
graphite-app[bot] merged 1 commit into
mainfrom
07-02-refactor_seal_used_symbol_refs_by_construction_after_its_last_writer
Jul 3, 2026
Merged

refactor: seal used_symbol_refs by construction after its last writer#10091
graphite-app[bot] merged 1 commit into
mainfrom
07-02-refactor_seal_used_symbol_refs_by_construction_after_its_last_writer

Conversation

@hyfdev

@hyfdev hyfdev commented Jul 2, 2026

Copy link
Copy Markdown
Member

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

hyfdev commented Jul 2, 2026

Copy link
Copy Markdown
Member Author

How to use the Graphite Merge Queue

Add 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.

@hyfdev
hyfdev force-pushed the 07-02-refactor_seal_used_symbol_refs_by_construction_after_its_last_writer branch from 2965e4c to 4c189da Compare July 2, 2026 11:41
@hyfdev
hyfdev marked this pull request as ready for review July 2, 2026 11:43
Copilot AI review requested due to automatic review settings July 2, 2026 11:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) with seal() 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 through GenerateContext.
  • Remove the namespace-decision mirror writes into used_symbol_refs, and update the affected downstream liveness decisions to consult meta.namespace_included explicitly.

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.

@codspeed-hq

codspeed-hq Bot commented Jul 2, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 7 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing 07-02-refactor_seal_used_symbol_refs_by_construction_after_its_last_writer (51dd2c6) with main (3325bf7)

Open in CodSpeed

Footnotes

  1. 10 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@hyfdev
hyfdev force-pushed the 07-02-docs_write_down_the_used_symbol_refs_contract branch from 4f81d80 to b60ec6b Compare July 2, 2026 11:56
@hyfdev
hyfdev force-pushed the 07-02-refactor_seal_used_symbol_refs_by_construction_after_its_last_writer branch 2 times, most recently from 286b070 to 111e607 Compare July 2, 2026 12:58
@hyfdev
hyfdev force-pushed the 07-02-docs_write_down_the_used_symbol_refs_contract branch from b60ec6b to fcc4aad Compare July 2, 2026 12:58
graphite-app Bot pushed a commit that referenced this pull request Jul 2, 2026
…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
graphite-app Bot pushed a commit that referenced this pull request Jul 2, 2026
#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
graphite-app Bot pushed a commit that referenced this pull request Jul 2, 2026
…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
@graphite-app
graphite-app Bot changed the base branch from 07-02-docs_write_down_the_used_symbol_refs_contract to graphite-base/10091 July 2, 2026 13:17
graphite-app Bot pushed a commit that referenced this pull request Jul 2, 2026
### 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
graphite-app Bot pushed a commit that referenced this pull request Jul 2, 2026
### 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
@graphite-app
graphite-app Bot force-pushed the 07-02-refactor_seal_used_symbol_refs_by_construction_after_its_last_writer branch from 937fc9b to 6a679a7 Compare July 2, 2026 15:28
@graphite-app
graphite-app Bot force-pushed the graphite-base/10091 branch from fcc4aad to 2504208 Compare July 2, 2026 15:28
@graphite-app
graphite-app Bot changed the base branch from graphite-base/10091 to main July 2, 2026 15:29
@graphite-app
graphite-app Bot force-pushed the 07-02-refactor_seal_used_symbol_refs_by_construction_after_its_last_writer branch from 6a679a7 to 92d5909 Compare July 2, 2026 15:29
@netlify

netlify Bot commented Jul 2, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

Name Link
🔨 Latest commit 8a4cc39
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/6a4746d1fc2ba00008597717

shulaoda commented Jul 2, 2026

Copy link
Copy Markdown
Member

Merge activity

  • Jul 2, 8:46 PM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jul 2, 8:46 PM UTC: shulaoda added this pull request to the Graphite merge queue.
  • Jul 2, 8:50 PM UTC: The Graphite merge queue couldn't merge this PR because it had merge conflicts.
  • Jul 3, 5:20 AM UTC: The merge label 'graphite: merge-when-ready' was detected. This PR will be added to the Graphite merge queue once it meets the requirements.
  • Jul 3, 5:21 AM UTC: shulaoda added this pull request to the Graphite merge queue.
  • Jul 3, 5:25 AM UTC: Merged by the Graphite merge queue.

@hyfdev
hyfdev force-pushed the 07-02-refactor_seal_used_symbol_refs_by_construction_after_its_last_writer branch from 92d5909 to 51dd2c6 Compare July 3, 2026 02:54
…#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
@graphite-app
graphite-app Bot force-pushed the 07-02-refactor_seal_used_symbol_refs_by_construction_after_its_last_writer branch from 51dd2c6 to 8a4cc39 Compare July 3, 2026 05:21
@graphite-app
graphite-app Bot merged commit 8a4cc39 into main Jul 3, 2026
34 checks passed
@graphite-app
graphite-app Bot deleted the 07-02-refactor_seal_used_symbol_refs_by_construction_after_its_last_writer branch July 3, 2026 05:25
@rolldown-guard rolldown-guard Bot mentioned this pull request Jul 8, 2026
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants