Skip to content

refactor: make module namespace inclusion an explicit linking metadata field#10087

Merged
graphite-app[bot] merged 1 commit into
mainfrom
used-symbols-split-1-namespace-flag
Jul 2, 2026
Merged

refactor: make module namespace inclusion an explicit linking metadata field#10087
graphite-app[bot] merged 1 commit into
mainfrom
used-symbols-split-1-namespace-flag

Conversation

@hyfdev

@hyfdev hyfdev commented Jul 2, 2026

Copy link
Copy Markdown
Member

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

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.

@netlify

netlify Bot commented Jul 2, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs canceled.

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

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 refactors how “module namespace object is retained” is communicated across the generate/finalize pipeline by introducing an explicit LinkingMetadata::namespace_included flag and switching dedicated consumers off of used_symbol_refs membership checks. This reduces reliance on a mutable side channel after tree-shaking has settled, while keeping a mirror in used_symbol_refs for generic symbol-usage queries that may still traffic in namespace refs.

Changes:

  • Add LinkingMetadata::namespace_included as the post-finalization authority for whether a module’s namespace object is retained.
  • Compute and store that flag in finalized_module_namespace_ref_usage, and mirror the decision into used_symbol_refs (insert/remove).
  • Update downstream readers (finalizer JSON-inlining gate and cross-chunk JSON export-interface check) to consult namespace_included instead of used_symbol_refs.contains(namespace_ref).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File Description
crates/rolldown/src/types/linking_metadata.rs Introduces namespace_included as explicit linking metadata for namespace retention.
crates/rolldown/src/stages/generate_stage/compute_cross_chunk_links.rs Switches JSON export-interface filtering to consult meta.namespace_included.
crates/rolldown/src/stages/generate_stage/code_splitting.rs Computes meta.namespace_included during namespace finalization and mirrors into used_symbol_refs.
crates/rolldown/src/module_finalizers/finalizer_context.rs Switches the namespace-declaration / JSON-inlining gate to use linking_info.namespace_included.

Comment thread crates/rolldown/src/types/linking_metadata.rs Outdated
@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 used-symbols-split-1-namespace-flag (934fa29) with main (fdb678e)

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 used-symbols-split-1-namespace-flag branch from a9149d2 to 934fa29 Compare July 2, 2026 11:56
@graphite-app

graphite-app Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Merge activity

  • Jul 2, 1:00 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, 1:00 PM UTC: graphite-app added this pull request to the Graphite merge queue.
  • Jul 2, 1:07 PM UTC: Merged by the Graphite merge queue.

…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
graphite-app Bot force-pushed the used-symbols-split-1-namespace-flag branch from 934fa29 to cf51957 Compare July 2, 2026 13:01
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
graphite-app Bot merged commit cf51957 into main Jul 2, 2026
34 checks passed
@graphite-app
graphite-app Bot deleted the used-symbols-split-1-namespace-flag branch July 2, 2026 13:07
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 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 Bot pushed a commit that referenced this pull request Jul 3, 2026
…#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
@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.

3 participants