Skip to content

Collapse ThreadPool::init dead generic and fix cpuid cfg gate#30886

Merged
Jarred-Sumner merged 1 commit into
mainfrom
claude/dead-code-cleanup
May 17, 2026
Merged

Collapse ThreadPool::init dead generic and fix cpuid cfg gate#30886
Jarred-Sumner merged 1 commit into
mainfrom
claude/dead-code-cleanup

Conversation

@Jarred-Sumner

Copy link
Copy Markdown
Collaborator

ThreadPool::init dead generic. bundler::ThreadPool::init (and its helper init_with_pool) were generic over V2 because, during the phased Zig→Rust port, bundle_v2.rs carried two BundleV2 definitions — the canonical one plus a bv2_impl draft module — and both needed to call init. The PORT NOTE on the function explicitly said it should collapse to &BundleV2<'_> once bv2_impl was dropped. That module is gone (grep -rn "struct BundleV2" src/bundler/ finds exactly one definition, bundle_v2.rs:75), so this collapses the generic, names the concrete type, and drops the now-stale PORT NOTE. The body was already storing a type-erased raw pointer, so the monomorphised code is identical.

cpuid cfg-gate mismatch. perf/hw_timer.rs defines struct CpuidResult and fn cpuid() under #[cfg(target_arch = "x86_64")], but their only callers live inside a #[cfg(not(any(target_os = "macos", target_os = "freebsd")))] block (macOS/FreeBSD read the boot-time TSC frequency from sysctl instead of probing CPUID). On x86_64-apple-darwin and x86_64-unknown-freebsd the helpers were therefore compiled with no callers, producing never constructed / never used warnings. The cfg gates now mirror the callers' conditions.

Verified with cargo check --workspace --keep-going (clean), bun run rust:check-all (10/10 targets), and cargo fmt -p bun_bundler -p bun_perf --check. Confirmed the dead-code warnings on x86_64-apple-darwin are present without this change and gone with it. File set is disjoint from #30879.

@robobun

robobun commented May 16, 2026

Copy link
Copy Markdown
Collaborator
Updated 5:18 AM PT - May 16th, 2026

@Jarred-Sumner, your commit 7c46dda has 1 failures in Build #55204 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 30886

That installs a local version of the PR into your bun-30886 executable, so you can run:

bun-30886 --bun

@coderabbitai

coderabbitai Bot commented May 16, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3d378d77-55f2-40e3-800d-ba3c3861a97a

📥 Commits

Reviewing files that changed from the base of the PR and between e750984 and 7c46dda.

📒 Files selected for processing (2)
  • src/bundler/ThreadPool.rs
  • src/perf/hw_timer.rs

Walkthrough

This PR specializes two generic or platform-agnostic APIs: ThreadPool's initialization functions are changed to accept BundleV2 directly instead of a generic type parameter, with explicit pointer casting for the backreference; and CpuidResult's conditional compilation is narrowed to exclude macOS and FreeBSD platforms.

Changes

Type refinements and platform targeting

Layer / File(s) Summary
ThreadPool specialization to BundleV2
src/bundler/ThreadPool.rs
ThreadPool::init and ThreadPool::init_with_pool are specialized to accept &BundleV2<'_> directly instead of generic V2 parameter. The v2 backreference assignment is updated with explicit casting from &BundleV2<'_> to &'static BundleV2 using std::ptr::from_ref(v2).cast::<BundleV2<'static>>().
CPUID platform restrictions
src/perf/hw_timer.rs
CpuidResult conditional compilation guard is narrowed from x86_64 to x86_64 and not(any(target_os = "macos", target_os = "freebsd")), excluding those operating systems from defining the type.
🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes both main changes: collapsing a dead generic in ThreadPool::init and fixing the cpuid cfg gate in hw_timer.rs.
Description check ✅ Passed The description covers the required template sections with clear explanations of changes and verification methods, though not explicitly labeled per template structure.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Comment @coderabbitai help to get the list of available commands and usage tips.

@claude claude Bot 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.

LGTM — mechanical de-genericization and cfg-gate tightening, both compile-time-only with no runtime behavior change.

Extended reasoning...

Overview

Two unrelated cleanup changes:

  • src/bundler/ThreadPool.rs: collapses ThreadPool::init<V2> and init_with_pool<V2> from generic to concrete &BundleV2<'_>, drops the now-stale PORT NOTE that explicitly said to do this once the duplicate BundleV2 definition was gone, and rewrites the pointer cast to name the concrete target type.
  • src/perf/hw_timer.rs: narrows the #[cfg] gate on CpuidResult / cpuid() from target_arch = "x86_64" to additionally exclude macOS/FreeBSD, matching the gate on their only call site inside read_frequency().

Verification

  • Confirmed only one struct BundleV2 definition exists (bundle_v2.rs:75). The bv2_impl module still exists but only as a method-body container, not a second struct definition — which is what the PORT NOTE actually depended on.
  • The sole external call site (bundle_v2.rs:2934) passes &mut *this where this: &mut BundleV2, so de-genericization type-checks with no caller changes.
  • The from_ref::<V2>(v2).cast()from_ref(v2).cast::<BundleV2<'static>>() rewrite is semantically identical: same source pointer, same target type (the field is *const BundleV2<'static>), just the turbofish moved from inference-at-source to explicit-at-cast.
  • In hw_timer.rs, the only references to cpuid / CpuidResult are inside the #[cfg(not(any(target_os = "macos", target_os = "freebsd")))] arm of read_frequency(), so the new gate exactly mirrors callers and only removes dead code on x86_64 Darwin/FreeBSD.

Security risks

None. No new logic, no new unsafe, no I/O or auth surface. Both changes are type/cfg-level and produce identical or strictly-subset codegen.

Level of scrutiny

Low. This is the kind of stale-port-scaffolding cleanup the codebase's own PORT NOTE asked for, plus a dead-code-warning fix. PR description reports clean cargo check --workspace and rust:check-all across 10 targets.

Other factors

No bugs from the bug-hunting system, no outstanding reviewer comments, no prior reviews.

@Jarred-Sumner Jarred-Sumner merged commit c7a7579 into main May 17, 2026
78 of 79 checks passed
@Jarred-Sumner Jarred-Sumner deleted the claude/dead-code-cleanup branch May 17, 2026 21:28
Jarred-Sumner added a commit that referenced this pull request May 17, 2026
…onsistent docs

- bundler/ThreadPool.rs: take main's post-#30886 version and drop the stranded
  empty-backtick parenthetical at the MimallocArena PORT NOTE
- runtime/ffi/host_fns.rs: drop stranded empty-backtick parenthetical
- css/rules/mod.rs: align vendor_prefix_to_css doc with the rewrite at :347-351
  (canonical impl is in css_parser.rs, not lib.rs)
- js_parser/parser.rs: delete dead ConvertESMExportsForHmr / ImportScanner stub
  modules (zero callers; real types live in lower/ and scan/)
- runtime/api/html_rewriter.rs, runtime/node/node_zlib_binding.rs: fix //; and
  //. stranded by the R-2 tag strip
Jarred-Sumner added a commit that referenced this pull request May 17, 2026
…onsistent docs

- bundler/ThreadPool.rs: take main's post-#30886 version and drop the stranded
  empty-backtick parenthetical at the MimallocArena PORT NOTE
- runtime/ffi/host_fns.rs: drop stranded empty-backtick parenthetical
- css/rules/mod.rs: align vendor_prefix_to_css doc with the rewrite at :347-351
  (canonical impl is in css_parser.rs, not lib.rs)
- js_parser/parser.rs: delete dead ConvertESMExportsForHmr / ImportScanner stub
  modules (zero callers; real types live in lower/ and scan/)
- runtime/api/html_rewriter.rs, runtime/node/node_zlib_binding.rs: fix //; and
  //. stranded by the R-2 tag strip
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.

2 participants