resolver: split the port's module wrapper into files; type the extern-Rust pointers#30880
Conversation
The Zig->Rust port wrapped the entire resolver implementation (Resolver,
Result, MatchResult, options, allocators, ...) in a single 7,664-line
pub mod __phase_a_body { ... } inside src/resolver/lib.rs. There is no
name for a module that wraps a crate's whole body that isn't redundant,
which is the tell that the wrapper shouldn't exist. Split it into
sibling files following the crate's existing convention
(data_url.rs / dir_info.rs / package_json.rs):
- options.rs (357 lines): BundleOptions, Packages, ExternalModules,
Framework, ExtOrder, ...
- result.rs (578 lines): Result, MatchResult, PathPair, DebugLogs,
PendingResolution, LoadResult, ...
- resolver.rs (6,640 lines): Resolver struct + impl, threadlocal
Bufs scratch, the local shim modules
- allocators.rs (6 lines): re-exports referenced cross-file
- standalone_module_graph.rs (30 lines): the StandaloneModuleGraph trait
lib.rs shrinks 10,273 -> 2,615 lines. Public API surface is byte-identical
(bun_resolver::Resolver, ::Result, ::options, ... all unchanged).
Also un-erase the extern "Rust" link-time pointers where the declaring
crate already names the type. The port applied 'type-erase across the
crate boundary' uniformly to every #[no_mangle] upward call, but
extern "Rust" carries full Rust types and most declaring crates already
depend on the crate that defines the parameter types. Where visible,
use the typed pointer with the Zig pointer shape (NonNull<T> for *T,
Option<NonNull<T>> for ?*T):
- __bun_resolver_init_package_manager: log *mut Log -> NonNull<Log>,
install *const () -> Option<NonNull<BunInstall>>,
env *mut c_void -> NonNull<Loader<'static>>
- BundleOptions.install: *const () -> Option<NonNull<BunInstall>>
- Resolver.log: *mut Log -> NonNull<Log>
- __bun_jsc_enable_hot_module_reloading_for_bundler:
*mut () -> NonNull<BundleV2<'static>>
The implementation-side cast::<T>() calls -- the tell that the erasure
was unnecessary -- are removed.
Sites where the type is genuinely not visible (bun_event_loop ->
bun_jsc::VirtualMachine, bun_js_parser -> bun_bundler::Transpiler) are
left as-is: those are the real layering boundary the pattern is for.
|
Updated 1:07 AM PT - May 16th, 2026
❌ @Jarred-Sumner, your commit 93be703 has 1 failures in
🧪 To try this PR locally: bunx bun-pr 30880That installs a local version of the PR into your bun-30880 --bun |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (5)
WalkthroughThis PR modernizes pointer handling across the resolver and bundler stack by introducing typed ChangesResolver options, results, and pointer standardization
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/jsc/hot_reloader.rs (1)
1284-1298:⚠️ Potential issue | 🟠 Major | ⚡ Quick winFix the reconstructed path bounds here.
This copy starts
changed_nameat the same index where Line 1282 wroteSEP, so the separator is overwritten, andpath_slicethen includes one extra byte past the copied filename. The resulting path/hash is malformed on this branch.Suggested fix
- _on_file_update_path_buf - [file_path_without_trailing_slash.len() - ..file_path_without_trailing_slash.len() - + changed_name.len()] - .copy_from_slice(changed_name); - let path_slice = &_on_file_update_path_buf[0 - ..file_path_without_trailing_slash.len() - + changed_name.len() - + 1]; + let dst_start = + file_path_without_trailing_slash.len() + 1; + _on_file_update_path_buf + [dst_start..dst_start + changed_name.len()] + .copy_from_slice(changed_name); + let path_slice = + &_on_file_update_path_buf[..dst_start + changed_name.len()];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/jsc/hot_reloader.rs` around lines 1284 - 1298, The reconstructed path overwrites the separator and includes one extra byte; fix by copying changed_name starting after the separator and slicing to include exactly separator + filename: use _on_file_update_path_buf[file_path_without_trailing_slash.len() + 1 .. file_path_without_trailing_slash.len() + 1 + changed_name.len()].copy_from_slice(changed_name) and set path_slice = &_on_file_update_path_buf[0 .. file_path_without_trailing_slash.len() + 1 + changed_name.len()] so the SEP (written earlier) is preserved and no extra byte is included; update the ranges around _on_file_update_path_buf, file_path_without_trailing_slash, and changed_name accordingly.src/bundler/transpiler.rs (1)
166-173:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRe-thread
options.loginset_log.
set_log()updatesself.log,linker.log, andresolver.log, but leavesself.options.logpointing at the previous logger. Line 2848 already consultsself.options.log().level, so a later log swap can split diagnostics and log-level checks across two differentLoginstances.wire_after_move()already maintains this invariant;set_log()should too.Suggested fix
pub fn set_log(&mut self, log: *mut bun_ast::Log) { self.log = log; + self.options.log = log; self.linker.log = log; // SAFETY: caller (`ThreadPool::Worker::create`) passes the per-worker // arena-allocated `Log`, which outlives this `Transpiler<'a>`. Zig // aliased the same `*Log` into `resolver.log`. self.resolver.log = core::ptr::NonNull::new(log).expect("set_log: log is non-null"); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/bundler/transpiler.rs` around lines 166 - 173, set_log currently updates self.log, self.linker.log, and self.resolver.log but fails to update the logger stored in self.options, which leaves self.options.log() pointing at the old Log and can split diagnostics and level checks; update set_log to also re-thread self.options.log to the new log pointer (mirror what wire_after_move does) so that self.options.log(), self.log, self.linker.log, and self.resolver.log all reference the same Log instance after calling set_log.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/bundler/bundle_v2.rs`:
- Around line 1477-1493: The watcher is installed too early in BundleV2::init
such that enable_hot_module_reloading_for_bundler (and the NonNull bv2 pointer)
can outlive the Box<BundleV2> when generate_from_cli returns Err before
mem::forget(this); fix by moving the watcher installation to after the bundle is
guaranteed retained (i.e., after the point where generate_from_cli would call
mem::forget or otherwise leak/retain the Box), or alternatively ensure every
watch-mode error exit explicitly retains/leaks the Box<BundleV2> (call
mem::forget or convert to a leaked pointer) before returning so the bv2 passed
to enable_hot_module_reloading_for_bundler is truly 'static; update
BundleV2::init and generate_from_cli call-sites accordingly.
In `@src/jsc/AsyncModule.rs`:
- Around line 1266-1280: The temporary log replacement misses swapping
transpiler.options.log, so calls that go through self.options.log() still use
the old VM log; update the unsafe block to also set
(*jsc_vm).transpiler.options.log = log_ptr (alongside transpiler.linker.log,
transpiler.log, transpiler.resolver.log, package_manager().log) and in the
scopeguard restore closure restore (*jsc_vm).transpiler.options.log =
old_log_ptr (alongside the other restores) so the options log is correctly
overridden and restored for the duration of the scope.
In `@src/resolver/result.rs`:
- Around line 447-450: The decrease_indent method unconditionally subtracts 1
from self.indent.list.len(), which will underflow and panic if the list is
empty; update decrease_indent (in src/resolver/result.rs) to check whether
self.indent.list is empty before removing an element (e.g., use an if
!self.indent.list.is_empty() guard and then truncate or call pop) so the method
becomes a no-op on empty indent instead of panicking.
- Around line 527-529: The initializer is using the package sentinel for a
dependency field—change the root_dependency_id assignment to use
Install::INVALID_DEPENDENCY_ID instead of Install::INVALID_PACKAGE_ID; update
the code where root_dependency_id (a DependencyID) is set so it references
Install::INVALID_DEPENDENCY_ID to keep the correct sentinel type and semantic
clarity for the DependencyID field.
---
Outside diff comments:
In `@src/bundler/transpiler.rs`:
- Around line 166-173: set_log currently updates self.log, self.linker.log, and
self.resolver.log but fails to update the logger stored in self.options, which
leaves self.options.log() pointing at the old Log and can split diagnostics and
level checks; update set_log to also re-thread self.options.log to the new log
pointer (mirror what wire_after_move does) so that self.options.log(), self.log,
self.linker.log, and self.resolver.log all reference the same Log instance after
calling set_log.
In `@src/jsc/hot_reloader.rs`:
- Around line 1284-1298: The reconstructed path overwrites the separator and
includes one extra byte; fix by copying changed_name starting after the
separator and slicing to include exactly separator + filename: use
_on_file_update_path_buf[file_path_without_trailing_slash.len() + 1 ..
file_path_without_trailing_slash.len() + 1 +
changed_name.len()].copy_from_slice(changed_name) and set path_slice =
&_on_file_update_path_buf[0 .. file_path_without_trailing_slash.len() + 1 +
changed_name.len()] so the SEP (written earlier) is preserved and no extra byte
is included; update the ranges around _on_file_update_path_buf,
file_path_without_trailing_slash, and changed_name accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5479db7c-401c-4a11-878c-b645dbb0f9b2
📒 Files selected for processing (22)
src/bundler/bundle_v2.rssrc/bundler/options.rssrc/bundler/transpiler.rssrc/install/PackageManager.rssrc/install/PackageManager/PackageManagerOptions.rssrc/install/auto_installer.rssrc/jsc/AsyncModule.rssrc/jsc/VirtualMachine.rssrc/jsc/hot_reloader.rssrc/resolver/allocators.rssrc/resolver/dir_info.rssrc/resolver/lib.rssrc/resolver/options.rssrc/resolver/resolver.rssrc/resolver/result.rssrc/resolver/standalone_module_graph.rssrc/router/lib.rssrc/runtime/api/filesystem_router.rssrc/runtime/bake/production.rssrc/runtime/cli/repl_command.rssrc/runtime/cli/run_command.rssrc/runtime/jsc_hooks.rs
| /// `'static` matches the impl side: the only caller (`bun build | ||
| /// --watch`) leaks the CLI arena, so the pointee is process-lifetime. | ||
| fn __bun_jsc_enable_hot_module_reloading_for_bundler( | ||
| bv2: core::ptr::NonNull<super::BundleV2<'static>>, | ||
| ); | ||
| } | ||
|
|
||
| /// `Watcher.enableHotModuleReloading(this, null)` for `bun build --watch`. | ||
| #[inline] | ||
| pub fn enable_hot_module_reloading_for_bundler(bv2: *mut super::BundleV2<'_>) { | ||
| // SAFETY: link-time-resolved Rust-ABI fn in `bun_jsc::hot_reloader`. | ||
| // Not `safe fn`: the callee re-types the erased `*mut ()` as | ||
| // `*mut BundleV2<'static>` and dereferences it, so `bv2` must point to | ||
| // a live `BundleV2` whose backing allocation outlives the watcher | ||
| // (sole caller is `BundleV2::init` with the leaked CLI arena). | ||
| unsafe { __bun_jsc_enable_hot_module_reloading_for_bundler(bv2.cast()) } | ||
| // Not `safe fn`: the callee dereferences `bv2`, so it must point to a | ||
| // live `BundleV2` whose backing allocation outlives the watcher (sole | ||
| // caller is `BundleV2::init` with the leaked CLI arena — `'static`). | ||
| let bv2 = core::ptr::NonNull::new(bv2.cast::<super::BundleV2<'static>>()) | ||
| .expect("BundleV2 watcher: bv2 is non-null"); | ||
| unsafe { __bun_jsc_enable_hot_module_reloading_for_bundler(bv2) } |
There was a problem hiding this comment.
The new 'static HMR contract is not upheld on watch-mode error exits.
Line 1477 says this pointer is process-lifetime because the CLI bundle is leaked, but BundleV2::init() installs the watcher before generate_from_cli() reaches its final mem::forget(this). In watch mode, any earlier return Err(...) drops the Box<BundleV2> while the reloader still owns the pointer, so the next file event can dereference freed memory. Please move watcher installation to the point where the bundle is definitely retained, or retain/leak the bundle on every watch-mode exit once HMR is enabled.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/bundler/bundle_v2.rs` around lines 1477 - 1493, The watcher is installed
too early in BundleV2::init such that enable_hot_module_reloading_for_bundler
(and the NonNull bv2 pointer) can outlive the Box<BundleV2> when
generate_from_cli returns Err before mem::forget(this); fix by moving the
watcher installation to after the bundle is guaranteed retained (i.e., after the
point where generate_from_cli would call mem::forget or otherwise leak/retain
the Box), or alternatively ensure every watch-mode error exit explicitly
retains/leaks the Box<BundleV2> (call mem::forget or convert to a leaked
pointer) before returning so the bv2 passed to
enable_hot_module_reloading_for_bundler is truly 'static; update BundleV2::init
and generate_from_cli call-sites accordingly.
| unsafe { | ||
| (*jsc_vm).transpiler.linker.log = log_ptr; | ||
| (*jsc_vm).transpiler.log = log_ptr; | ||
| (*jsc_vm).transpiler.resolver.log = log_ptr; | ||
| (*jsc_vm).transpiler.resolver.log = log_nn; | ||
| (*jsc_vm).package_manager().log = log_ptr; | ||
| } | ||
| let _restore = scopeguard::guard((jsc_vm, old_log), |(jsc_vm, old_log)| { | ||
| // SAFETY: same per-thread VM; restoring the original `*mut Log` | ||
| // values stored above. | ||
| // SAFETY: same per-thread VM; restoring the original log pointers | ||
| // stored above. | ||
| unsafe { | ||
| let old_log_ptr = old_log.map(|p| p.as_ptr()).unwrap_or(core::ptr::null_mut()); | ||
| let old_log_ptr = old_log.as_ptr(); | ||
| (*jsc_vm).transpiler.linker.log = old_log_ptr; | ||
| (*jsc_vm).transpiler.log = old_log_ptr; | ||
| (*jsc_vm).transpiler.resolver.log = old_log_ptr; | ||
| (*jsc_vm).transpiler.resolver.log = old_log; | ||
| (*jsc_vm).package_manager().log = old_log_ptr; |
There was a problem hiding this comment.
Swap transpiler.options.log with the other temporary log aliases.
This scope overrides transpiler.log, linker.log, resolver.log, and package_manager.log, but leaves transpiler.options.log on the VM log. Anything on the link() / print_with_source_map() path that calls self.options.log() will still write to the stale buffer, so async-module diagnostics can bypass the local log.
Suggested fix
unsafe {
+ (*jsc_vm).transpiler.options.log = log_ptr;
(*jsc_vm).transpiler.linker.log = log_ptr;
(*jsc_vm).transpiler.log = log_ptr;
(*jsc_vm).transpiler.resolver.log = log_nn;
(*jsc_vm).package_manager().log = log_ptr;
}
@@
unsafe {
let old_log_ptr = old_log.as_ptr();
+ (*jsc_vm).transpiler.options.log = old_log_ptr;
(*jsc_vm).transpiler.linker.log = old_log_ptr;
(*jsc_vm).transpiler.log = old_log_ptr;
(*jsc_vm).transpiler.resolver.log = old_log;
(*jsc_vm).package_manager().log = old_log_ptr;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/jsc/AsyncModule.rs` around lines 1266 - 1280, The temporary log
replacement misses swapping transpiler.options.log, so calls that go through
self.options.log() still use the old VM log; update the unsafe block to also set
(*jsc_vm).transpiler.options.log = log_ptr (alongside transpiler.linker.log,
transpiler.log, transpiler.resolver.log, package_manager().log) and in the
scopeguard restore closure restore (*jsc_vm).transpiler.options.log =
old_log_ptr (alongside the other restores) so the options log is correctly
overridden and restored for the duration of the scope.
| pub fn decrease_indent(&mut self) { | ||
| let new_len = self.indent.list.len() - 1; | ||
| self.indent.list.truncate(new_len); | ||
| } |
There was a problem hiding this comment.
Guard against empty-indent underflow in decrease_indent.
Line 448 subtracts 1 unconditionally; calling this on an empty indent buffer panics.
Proposed fix
pub fn decrease_indent(&mut self) {
- let new_len = self.indent.list.len() - 1;
- self.indent.list.truncate(new_len);
+ if let Some(new_len) = self.indent.list.len().checked_sub(1) {
+ self.indent.list.truncate(new_len);
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pub fn decrease_indent(&mut self) { | |
| let new_len = self.indent.list.len() - 1; | |
| self.indent.list.truncate(new_len); | |
| } | |
| pub fn decrease_indent(&mut self) { | |
| if let Some(new_len) = self.indent.list.len().checked_sub(1) { | |
| self.indent.list.truncate(new_len); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/resolver/result.rs` around lines 447 - 450, The decrease_indent method
unconditionally subtracts 1 from self.indent.list.len(), which will underflow
and panic if the list is empty; update decrease_indent (in
src/resolver/result.rs) to check whether self.indent.list is empty before
removing an element (e.g., use an if !self.indent.list.is_empty() guard and then
truncate or call pop) so the method becomes a no-op on empty indent instead of
panicking.
| resolution_id: Install::INVALID_PACKAGE_ID, | ||
| root_dependency_id: Install::INVALID_PACKAGE_ID, | ||
| import_record_id: u32::MAX, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify the canonical invalid constants/types in resolver hooks.
rg -n --type rust "type\s+PackageID|type\s+DependencyID|INVALID_[A-Z_]*PACKAGE|INVALID_[A-Z_]*DEPENDENCY"
# Verify all root_dependency_id initializations/usages.
rg -n --type rust "root_dependency_id\s*[:=]"Repository: oven-sh/bun
Length of output: 8877
Use Install::INVALID_DEPENDENCY_ID for the root_dependency_id field.
Line 528 initializes a DependencyID field with Install::INVALID_PACKAGE_ID. Although both sentinels currently have the same value (u32::MAX), they represent different types. Using the correct sentinel constant ensures semantic clarity and prevents bugs if the invalid constants ever diverge.
Proposed fix
- root_dependency_id: Install::INVALID_PACKAGE_ID,
+ root_dependency_id: Install::INVALID_DEPENDENCY_ID,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| resolution_id: Install::INVALID_PACKAGE_ID, | |
| root_dependency_id: Install::INVALID_PACKAGE_ID, | |
| import_record_id: u32::MAX, | |
| resolution_id: Install::INVALID_PACKAGE_ID, | |
| root_dependency_id: Install::INVALID_DEPENDENCY_ID, | |
| import_record_id: u32::MAX, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/resolver/result.rs` around lines 527 - 529, The initializer is using the
package sentinel for a dependency field—change the root_dependency_id assignment
to use Install::INVALID_DEPENDENCY_ID instead of Install::INVALID_PACKAGE_ID;
update the code where root_dependency_id (a DependencyID) is set so it
references Install::INVALID_DEPENDENCY_ID to keep the correct sentinel type and
semantic clarity for the DependencyID field.
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
I didn't find any bugs, but this is a large refactor (~8k lines moved out of resolver/lib.rs into new files) that also retypes several extern "Rust" link-time signatures and adds new .expect() panic paths on vm.log — worth a human pass to confirm the move is byte-faithful and both sides of each #[no_mangle] boundary agree.
Extended reasoning...
Overview
Splits the 7,664-line __phase_a_body wrapper module in src/resolver/lib.rs into five sibling files (options.rs, result.rs, resolver.rs, standalone_module_graph.rs, allocators.rs), and retypes several cross-crate extern "Rust" pointers from erased *mut () / *mut c_void / *mut Log to NonNull<T> / Option<NonNull<T>>. Ripples through 22 files across bundler, install, jsc, resolver, router, and runtime. Also scrubs a large number of "Phase A/B" port-artifact comments.
Security risks
None identified — no auth, crypto, or untrusted-input parsing is touched. The extern "Rust" signature changes are an ABI concern (both declaration and definition must agree exactly or it's UB), but that's a correctness risk rather than a security one, and cargo check --workspace / bun bd linking would catch a mismatch.
Level of scrutiny
High. The resolver is on the hot path for every import/require in Bun, and this PR moves ~6,640 lines into a new resolver.rs that the diff tool can't render. The claim is "public API surface byte-identical," but verifying that requires comparing the moved bodies, not just the re-export list. The NonNull<Log> retyping of Resolver.log introduces .expect("vm.log set in init") at several restore sites (AsyncModule.rs, jsc_hooks.rs::resolve_hook) where the old code tolerated None via .unwrap_or(null_mut()) — the PR asserts these are infallible, which is probably true, but it's a behavioral tightening worth a second pair of eyes.
Other factors
The bug-hunting system found nothing, and the verification section lists cargo check, cargo fmt, all-triple rust:check-all, and a successful bun bd link+run. That's good coverage for the mechanical parts. Still, the combination of (a) a multi-thousand-line code move that can't be diffed inline, (b) link-time ABI retyping across four #[no_mangle] boundaries, and (c) touching nearly every major subsystem puts this well outside "simple/mechanical" territory.
| @@ -0,0 +1,6 @@ | |||
| //! ── bun_alloc::allocators re-export ────────────────────────────────────── | |||
There was a problem hiding this comment.
Delete this slop file. Do not re-export.
…-Rust pointers (oven-sh#30880) ## What ### Module split The Zig→Rust port wrapped the entire resolver implementation in a single 7,664-line `pub mod __phase_a_body { ... }` inside `src/resolver/lib.rs` (lines 2609–10,273) — a port artifact ("this is the mechanically-translated block"). There's no name for a module that wraps a crate's whole body that isn't redundant, which is the tell the wrapper shouldn't exist. Split it into sibling files following the crate's existing convention (`data_url.rs` / `dir_info.rs` / `package_json.rs`): | File | Lines | Holds | |---|---|---| | `options.rs` | 357 | `BundleOptions`, `Packages`, `ExternalModules`, `Framework`, `ExtOrder`, … | | `result.rs` | 578 | `Result`, `MatchResult`, `PathPair`, `DebugLogs`, `PendingResolution`, `LoadResult`, … | | `resolver.rs` | 6,640 | `Resolver` struct + impl, threadlocal `Bufs`, local shim modules | | `standalone_module_graph.rs` | 30 | the `StandaloneModuleGraph` trait | | `allocators.rs` | 6 | re-exports referenced cross-file from `dir_info.rs` | `lib.rs` shrinks 10,273 → 2,615 lines. Public API surface is byte-identical — `bun_resolver::Resolver`, `::Result`, `::options`, … all resolve as before. ### Typed extern-Rust pointers Un-erase the `extern "Rust"` link-time pointers where the declaring crate already names the type. The port applied "type-erase across the crate boundary" uniformly to every `#[no_mangle]` upward call, but `extern "Rust"` carries full Rust types — both crates can name the parameters. Where visible, use the typed pointer with the Zig pointer shape (`NonNull<T>` for `*T`, `Option<NonNull<T>>` for `?*T`): - `__bun_resolver_init_package_manager`: `log: *mut Log` → `NonNull<Log>`, `install: *const ()` → `Option<NonNull<BunInstall>>`, `env: *mut c_void` → `NonNull<Loader<'static>>` - `BundleOptions.install`: `*const ()` → `Option<NonNull<BunInstall>>` - `Resolver.log`: `*mut Log` → `NonNull<Log>` - `__bun_jsc_enable_hot_module_reloading_for_bundler`: `*mut ()` → `NonNull<BundleV2<'static>>` The implementation-side `cast::<T>()` calls — the tell that the erasure was unnecessary — are removed. Sites where the type is genuinely not visible (`bun_event_loop` → `bun_jsc::VirtualMachine`, `bun_js_parser` → `bun_bundler::Transpiler`) are left as-is — that's the real layering boundary the pattern exists for. ## Verification - `cargo check --workspace` clean - `cargo fmt --check` clean - `bun run rust:check-all` (all 6 target triples) - `bun bd` links and runs - `grep -rn resolver_body src/` returns nothing
2 participants
What
Module split
The Zig→Rust port wrapped the entire resolver implementation in a single 7,664-line
pub mod __phase_a_body { ... }insidesrc/resolver/lib.rs(lines 2609–10,273) — a port artifact ("this is the mechanically-translated block"). There's no name for a module that wraps a crate's whole body that isn't redundant, which is the tell the wrapper shouldn't exist. Split it into sibling files following the crate's existing convention (data_url.rs/dir_info.rs/package_json.rs):options.rsBundleOptions,Packages,ExternalModules,Framework,ExtOrder, …result.rsResult,MatchResult,PathPair,DebugLogs,PendingResolution,LoadResult, …resolver.rsResolverstruct + impl, threadlocalBufs, local shim modulesstandalone_module_graph.rsStandaloneModuleGraphtraitallocators.rsdir_info.rslib.rsshrinks 10,273 → 2,615 lines. Public API surface is byte-identical —bun_resolver::Resolver,::Result,::options, … all resolve as before.Typed extern-Rust pointers
Un-erase the
extern "Rust"link-time pointers where the declaring crate already names the type. The port applied "type-erase across the crate boundary" uniformly to every#[no_mangle]upward call, butextern "Rust"carries full Rust types — both crates can name the parameters. Where visible, use the typed pointer with the Zig pointer shape (NonNull<T>for*T,Option<NonNull<T>>for?*T):__bun_resolver_init_package_manager:log: *mut Log→NonNull<Log>,install: *const ()→Option<NonNull<BunInstall>>,env: *mut c_void→NonNull<Loader<'static>>BundleOptions.install:*const ()→Option<NonNull<BunInstall>>Resolver.log:*mut Log→NonNull<Log>__bun_jsc_enable_hot_module_reloading_for_bundler:*mut ()→NonNull<BundleV2<'static>>The implementation-side
cast::<T>()calls — the tell that the erasure was unnecessary — are removed.Sites where the type is genuinely not visible (
bun_event_loop→bun_jsc::VirtualMachine,bun_js_parser→bun_bundler::Transpiler) are left as-is — that's the real layering boundary the pattern exists for.Verification
cargo check --workspacecleancargo fmt --checkcleanbun run rust:check-all(all 6 target triples)bun bdlinks and runsgrep -rn resolver_body src/returns nothing