Skip to content

refactor(link_stage): remove unsafe in determine_module_exports_kind#9253

Merged
graphite-app[bot] merged 1 commit into
mainfrom
04-27-fix_avoid_unsafe_in_determine_module_exports_kind
Apr 29, 2026
Merged

refactor(link_stage): remove unsafe in determine_module_exports_kind#9253
graphite-app[bot] merged 1 commit into
mainfrom
04-27-fix_avoid_unsafe_in_determine_module_exports_kind

Conversation

@IWANABETHATGUY

@IWANABETHATGUY IWANABETHATGUY commented Apr 28, 2026

Copy link
Copy Markdown
Member

Replaces the three unsafe { addr_of!(*importee).cast_mut() } blocks in determine_module_exports_kind with index-based iteration that releases the module_table borrow before each as_normal_mut() mutation. Reads and writes still interleave in the same order as the original closure walk, so behavior is preserved bit-for-bit — no fixture snapshots are touched.

IWANABETHATGUY commented Apr 28, 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.

@IWANABETHATGUY IWANABETHATGUY changed the title u refactor(link_stage): remove unsafe in determine_module_exports_kind Apr 28, 2026
@IWANABETHATGUY
IWANABETHATGUY force-pushed the 04-28-docs_determine_module_exports_kind branch from 1165f21 to a6ebf91 Compare April 29, 2026 12:23
@IWANABETHATGUY
IWANABETHATGUY force-pushed the 04-27-fix_avoid_unsafe_in_determine_module_exports_kind branch from 4cf1bb0 to 1cc9a48 Compare April 29, 2026 12:23
@graphite-app
graphite-app Bot changed the base branch from 04-28-docs_determine_module_exports_kind to graphite-base/9253 April 29, 2026 13:01
@graphite-app
graphite-app Bot force-pushed the 04-27-fix_avoid_unsafe_in_determine_module_exports_kind branch from 1cc9a48 to 1f8cd40 Compare April 29, 2026 13:02
@graphite-app
graphite-app Bot force-pushed the graphite-base/9253 branch from a6ebf91 to c5e62cd Compare April 29, 2026 13:02
@graphite-app
graphite-app Bot changed the base branch from graphite-base/9253 to main April 29, 2026 13:03
@graphite-app
graphite-app Bot force-pushed the 04-27-fix_avoid_unsafe_in_determine_module_exports_kind branch from 1f8cd40 to f3496ae Compare April 29, 2026 13:03
@netlify

netlify Bot commented Apr 29, 2026

Copy link
Copy Markdown

Deploy Preview for rolldown-rs ready!

Name Link
🔨 Latest commit 92c9230
🔍 Latest deploy log https://app.netlify.com/projects/rolldown-rs/deploys/69f22a2ec589c60008dadee0
😎 Deploy Preview https://deploy-preview-9253--rolldown-rs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@IWANABETHATGUY
IWANABETHATGUY marked this pull request as ready for review April 29, 2026 15:52
@h-a-n-a
h-a-n-a requested a review from Copilot April 29, 2026 15:54
@graphite-app

graphite-app Bot commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Merge activity

…9253)

Replaces the three `unsafe { addr_of!(*importee).cast_mut() }` blocks in `determine_module_exports_kind` with index-based iteration that releases the `module_table` borrow before each `as_normal_mut()` mutation. Reads and writes still interleave in the same order as the original closure walk, so behavior is preserved bit-for-bit — no fixture snapshots are touched.
@graphite-app
graphite-app Bot force-pushed the 04-27-fix_avoid_unsafe_in_determine_module_exports_kind branch from 1c19c64 to 92c9230 Compare April 29, 2026 15:56

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

Refactors the link stage’s determine_module_exports_kind pass to eliminate unsafe pointer-based mutation by restructuring iteration so module_table borrows don’t overlap with as_normal_mut() mutations. This keeps the same promotion/wrapping semantics while making the code borrow-checker-friendly.

Changes:

  • Removed unsafe { addr_of!(...).cast_mut() } mutations of exports_kind.
  • Reworked the walk over importers/import records to use index-based iteration and short immutable borrows before each mutation.
  • Updated imports to use ImportRecordIdx for index-based access.

Comment on lines +17 to +23
let importer_indices: Vec<_> = self
.module_table
.modules
.iter_enumerated()
.filter_map(|(idx, m)| matches!(m, Module::Normal(_)).then_some(idx))
.collect();

Comment on lines +25 to +36
let n_records = match &self.module_table[importer_idx] {
Module::Normal(m) => m.import_records.len(),
Module::External(_) => continue,
};

for rec_pos in 0..n_records {
let (kind, importee_idx) = {
let Module::Normal(m) = &self.module_table[importer_idx] else { continue };
let rec = &m.import_records[ImportRecordIdx::from_usize(rec_pos)];
let Some(importee_idx) = rec.resolved_module else { continue };
(rec.kind, importee_idx)
};
@graphite-app
graphite-app Bot merged commit 92c9230 into main Apr 29, 2026
39 checks passed
@graphite-app
graphite-app Bot deleted the 04-27-fix_avoid_unsafe_in_determine_module_exports_kind branch April 29, 2026 16:01
@codspeed-hq

codspeed-hq Bot commented Apr 29, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 4 untouched benchmarks
⏩ 10 skipped benchmarks1


Comparing 04-27-fix_avoid_unsafe_in_determine_module_exports_kind (92c9230) with main (f98ff45)2

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.

  2. No successful run was found on main (92c9230) during the generation of this report, so f98ff45 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@rolldown-guard rolldown-guard Bot mentioned this pull request May 6, 2026
@rolldown-guard rolldown-guard Bot mentioned this pull request May 7, 2026
@shulaoda shulaoda mentioned this pull request May 7, 2026
shulaoda added a commit that referenced this pull request May 7, 2026
## [1.0.0] - 2026-05-07

### 🐛 Bug Fixes

- dev/lazy: lazily compiled modules should be watched (#9301) by @h-a-n-a
- implement dynamic dominator merge logic (#9270) by @TheAlexLichter
- dev: apply __toCommonJS interop when CJS requires ESM in HMR finalizer (#9261) by @h-a-n-a

### 🚜 Refactor

- ecma_ast: tighten allocator access to enforce Sync invariant (#9278) by @IWANABETHATGUY
- scan_stage: remove stmt_infos field from EcmaView (#9276) by @IWANABETHATGUY
- link_stage: detach stmt_infos from EcmaView (#9274) by @IWANABETHATGUY
- link_stage: detach depended_runtime_helper from EcmaView to remove unsafe (#9265) by @IWANABETHATGUY
- link_stage: remove unsafe in determine_module_exports_kind (#9253) by @IWANABETHATGUY

### 📚 Documentation

- getting-started: remove RC warning for 1.0.0 release (#9310) by @shulaoda
- getting-started: update version references for 1.0.0 release (#9309) by @shulaoda
- add Vite+ tab to getting-started snippets (#9285) by @shulaoda
- lazy-barrel: clarify own-exports behavior for import-then-export records (#9298) by @shulaoda
- restructure top navigation around Learn vs Reference (#9284) by @shulaoda
- builtin-plugins: add bundle analyzer plugin docs (#9292) by @shulaoda
- design doc for reference_needed_symbols (#9264) by @IWANABETHATGUY

### ⚡ Performance

- devtools: write logs on a background thread (#9219) by @IWANABETHATGUY

### ⚙️ Miscellaneous Tasks

- mark esbuild/ts/parameter_props_use_define_for_class_fields_true as passed (#9308) by @sapphi-red
- deps: upgrade oxc to 0.129.0 (#9297) by @shulaoda
- deps: update rollup submodule for tests to v4.60.3 (#9294) by @sapphi-red
- deps: update test262 submodule for tests (#9295) by @sapphi-red
- ai: add rolldown REPL decode skill (#9245) by @Dunqing
pull Bot pushed a commit to olrtg/rolldown that referenced this pull request May 7, 2026
## [1.0.0] - 2026-05-07

### 🐛 Bug Fixes

- dev/lazy: lazily compiled modules should be watched (rolldown#9301) by @h-a-n-a
- implement dynamic dominator merge logic (rolldown#9270) by @TheAlexLichter
- dev: apply __toCommonJS interop when CJS requires ESM in HMR finalizer (rolldown#9261) by @h-a-n-a

### 🚜 Refactor

- ecma_ast: tighten allocator access to enforce Sync invariant (rolldown#9278) by @IWANABETHATGUY
- scan_stage: remove stmt_infos field from EcmaView (rolldown#9276) by @IWANABETHATGUY
- link_stage: detach stmt_infos from EcmaView (rolldown#9274) by @IWANABETHATGUY
- link_stage: detach depended_runtime_helper from EcmaView to remove unsafe (rolldown#9265) by @IWANABETHATGUY
- link_stage: remove unsafe in determine_module_exports_kind (rolldown#9253) by @IWANABETHATGUY

### 📚 Documentation

- getting-started: remove RC warning for 1.0.0 release (rolldown#9310) by @shulaoda
- getting-started: update version references for 1.0.0 release (rolldown#9309) by @shulaoda
- add Vite+ tab to getting-started snippets (rolldown#9285) by @shulaoda
- lazy-barrel: clarify own-exports behavior for import-then-export records (rolldown#9298) by @shulaoda
- restructure top navigation around Learn vs Reference (rolldown#9284) by @shulaoda
- builtin-plugins: add bundle analyzer plugin docs (rolldown#9292) by @shulaoda
- design doc for reference_needed_symbols (rolldown#9264) by @IWANABETHATGUY

### ⚡ Performance

- devtools: write logs on a background thread (rolldown#9219) by @IWANABETHATGUY

### ⚙️ Miscellaneous Tasks

- mark esbuild/ts/parameter_props_use_define_for_class_fields_true as passed (rolldown#9308) by @sapphi-red
- deps: upgrade oxc to 0.129.0 (rolldown#9297) by @shulaoda
- deps: update rollup submodule for tests to v4.60.3 (rolldown#9294) by @sapphi-red
- deps: update test262 submodule for tests (rolldown#9295) by @sapphi-red
- ai: add rolldown REPL decode skill (rolldown#9245) by @Dunqing

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