Skip to content

Rename Zig-prefixed names in the JSC bindings to Bun equivalents#31822

Open
robobun wants to merge 22 commits into
mainfrom
farm/50d63c3b/rename-zigglobalobject
Open

Rename Zig-prefixed names in the JSC bindings to Bun equivalents#31822
robobun wants to merge 22 commits into
mainfrom
farm/50d63c3b/rename-zigglobalobject

Conversation

@robobun

@robobun robobun commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator

What

De-Zigs the JSC bindings layer, in two mechanical stages. The header carried // TODO: rename this to BunGlobalObject since April 2023 (aeb3bb9); the runtime port made the rest of the Zig-prefixed naming stale too.

Stage 1: the global object

  • Zig::GlobalObject and Zig::EvalGlobalObject move to namespace Bun
  • ZigGlobalObject.{h,cpp,lut.txt} become BunGlobalObject.{h,cpp,lut.txt}
  • extern "C" symbols: Zig__GlobalObject__* -> Bun__GlobalObject__*, ZigGlobalObject__* -> BunGlobalObject__*, Bun__ZigGlobalObject__uvLoop -> Bun__GlobalObject__uvLoop

Stage 2: the rest of namespace Zig and the Zig-prefixed binding files

  • namespace Zig is gone; all members (SourceProvider, JSFFIFunction, JSWrappingFunction, CallSite, ImportMetaObject, JSCStackTrace/JSCStackFrame, BunPlugin, string helpers) now live in namespace Bun
  • ZigSourceProvider.{h,cpp}, ZigException.cpp, ZigGeneratedCode.cpp, ZigLazyStaticFunctions{,-inlines}.h -> Bun*
  • C ABI exception types renamed on both C++ and Rust sides, including the Rust source files and modules: ZigException, ZigStackTrace, ZigStackFrame, ZigStackFramePosition, ZigStackFrameCode, ZigErrorType, ZigErrorCode -> Bun*
  • CommonStringsForZig -> CommonStringsForBun (and Bun__CommonStringsForZig__toJS -> Bun__CommonStringsForBun__toJS)
  • Zig_ErrorCodeParserError / Zig_ErrorCodeJSErrorObject -> Bun_*; zig__ModuleInfo* / zig__renderDiff -> bun__*
  • codegen templates (bundle-functions.ts, generate-classes.ts, cppbind.ts type maps), build scripts, $newCppFunction references, leaksan suppressions and docs updated to match

Relationship to #30747

#30747 is the full-codebase superset of this rename (same target names) but is currently conflicting and stacked on #30683. This PR lands the bindings-layer portion fresh against main. Still intentionally left for the bigger sweep: ZigString/ZigStringSlice and friends, ZIG_EXPORT/ZIG_DECL/ZIG_NONNULL, ZigGeneratedClasses (codegen artifact naming), and crate-internal zig_* names.

Non-mechanical bits

  • BunClientData.h forward-declared namespace Zig { class GlobalObject; } right before using namespace Zig;; both are now Bun
  • 20 headers forward-declared class GlobalObject; inside namespace Zig blocks; those decls moved to namespace Bun
  • unsafeEvalNoop was defined in both NodeVM.cpp and the formerly-Zig:: block of the global object cpp; the latter is now static to avoid a duplicate symbol after the namespace merge
  • the long-commented-out legacy destroy/visitChildrenImpl block in BunGlobalObject.cpp is deleted instead of renamed (the live implementations are earlier in the same file), and the stale bun__renderDiff home-path comment in phase_c_exports.rs now points at its actual definition in src/runtime/test_runner/diff_format.rs
  • JSWrappingFunction.cpp referenced Bun::NativeFunctionPtr for a type that then lived in namespace Zig; it only compiled because of unified-build include leakage, now consistent
  • napi.cpp needed one JSC::Exception qualification that became ambiguous once the using Exception = JSC::Exception alias moved into namespace Bun
  • the Module constructor in NodeModuleModule.cpp carried a TODO about static_casting the lexical global to the (renamed) class; it now uses defaultGlobalObject() so non-Bun globals (node:vm) fall back to the main global instead of reading through a bogus pointer

Verification

  • cargo check passes on the full workspace; full debug (ASAN) build passes
  • git grep "Zig::|namespace Zig|ZigGlobalObject|ZigException|ZigSourceProvider" comes back empty (modulo the intentionally-kept ZigString family and uncompiled .zig reference files)
  • smoke-tested the paths whose symbols or classes moved: streams.test.js, shadow.test.js, vm.test.ts, plugins.test.ts, worker_threads.test.ts, node-module-module.test.js, ffi.test.js, plus Error.prepareStackTrace CallSite formatting, uncaught-exception rendering, and bun build: all pass

No behavior change, so no new test; existing suites cover the renamed paths.

@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

This PR renames JSC-facing Zig-prefixed global object, exception, stack, source-provider, and related binding surfaces to Bun-prefixed names across generators, Rust, Zig, C++, headers, runtime modules, build scripts, and documentation.

Changes

Bun JSC runtime rename

Layer / File(s) Summary
Runtime and binding rename
.claude/skills/implementing-jsc-classes-cpp/SKILL.md, scripts/build/*, src/jsc/..., src/jsc/bindings/..., src/codegen/..., src/jsc/modules/..., src/bundler/..., src/bundler_jsc/..., src/ast_jsc/lib.rs, CLAUDE.md, .gitattributes
Generated code, FFI symbols, runtime wrappers, JSC bindings, headers, native modules, build wiring, and docs are retargeted from Zig-named global object and exception surfaces to Bun-named equivalents.

Suggested reviewers

  • Jarred-Sumner
  • alii

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

@github-actions github-actions Bot added the claude label Jun 4, 2026
@robobun

robobun commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 9:48 PM PT - Jun 18th, 2026

@robobun, your commit ba9836d has 3 failures in Build #63440 (All Failures):


🧪   To try this PR locally:

bunx bun-pr 31822

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

bun-31822 --bun

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

This PR may be a duplicate of:

  1. Rename Zig-prefixed identifiers to Bun/Rust equivalents #30747 - Superset rename of all Zig-prefixed identifiers to Bun/Rust equivalents, which includes the same ZigGlobalObject → BunGlobalObject rename performed here

🤖 Generated with Claude Code

@robobun

robobun commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

Not a coincidence: #30747 is the superset rename (all Zig-prefixed identifiers) but it is currently conflicting and stacked on #30683. This PR is the same ZigGlobalObject -> BunGlobalObject naming, rebuilt fresh against main, and is being extended to cover the remaining Zig-prefixed files in src/jsc/bindings (SourceProvider, Exception, GeneratedCode, LazyStaticFunctions) plus the namespace Zig -> Bun merge, using the same target names as #30747. The ZigString -> UnsafeStringView and ZIG_EXPORT -> RUST_EXPORT sweeps remain in #30747's scope.

@robobun robobun changed the title Rename ZigGlobalObject to BunGlobalObject Rename Zig-prefixed names in the JSC bindings to Bun equivalents Jun 4, 2026
@robobun

robobun commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

Extended per request: namespace Zig is now fully merged into namespace Bun, and the remaining Zig-prefixed binding files (SourceProvider, Exception, GeneratedCode, LazyStaticFunctions) plus the C ABI exception types are renamed on both the C++ and Rust sides. ZigString and the ZIG_EXPORT macro family remain with #30747.

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

I didn't find any issues with the rename itself, but this is a 282-file change spanning the C++/Rust/Zig FFI boundary with a few hand-fixed non-mechanical bits (and CI is currently red on the musl build-cpp jobs), so it's worth a human pass before merge.

Extended reasoning...

Overview

This PR mechanically renames Zig::GlobalObjectBun::GlobalObject (and ZigGlobalObject.{h,cpp,lut.txt}BunGlobalObject.*, plus the Zig__GlobalObject__* / ZigGlobalObject__* extern "C" symbols) across 282 files. It touches the core JSC global object class, the codegen templates that emit references to it, the Rust/Zig FFI declarations, build scripts, leaksan suppressions, and docs. The diff is overwhelmingly mechanical find-and-replace, and the author confirmed git grep for the old names is now empty.

Security risks

None identified. This is a pure rename of an internal C++ class and its linker symbols; no auth, crypto, permission, or input-handling logic is changed. The one behavior change (NodeModuleModule's Module constructor now uses defaultGlobalObject() instead of static-casting the lexical global) is a defensive fix for node:vm contexts and is called out in the description.

Level of scrutiny

High, for two reasons. First, the global object is the most central type in Bun's JSC bindings — every extern "C" symbol rename must match on all three sides (C++, Rust, Zig) or the link fails or, worse, silently mismatches. Second, the PR explicitly mixes in several non-mechanical hand-fixes: the empty namespace Zig {} shim in BunClientData.h to keep using namespace Zig valid, making unsafeEvalNoop static to avoid an ODR collision with NodeVM.cpp, the Zig::NativeFunctionPtr qualification fix in JSWrappingFunction.cpp, and the defaultGlobalObject() change in NodeModuleModule. These are individually small but each one is the kind of thing that only shows up on specific build configurations.

Other factors

CI on commit 387b866 shows the musl build-cpp jobs failing (x64-musl, aarch64-musl, x64-musl-baseline); the latest commit ac64c4e may or may not have re-run yet. The author also noted in the thread that this PR is still being extended to cover more Zig-prefixed files, and it overlaps with the larger #30747. Given the scale, the cross-language symbol renames, the red CI, and the in-flight scope, this should get a human sign-off rather than a bot approval.

@robobun robobun force-pushed the farm/50d63c3b/rename-zigglobalobject branch from ac64c4e to d846f74 Compare June 4, 2026 22:26
Comment thread CLAUDE.md
Comment thread src/jsc/BunException.rs Outdated
@robobun

robobun commented Jun 4, 2026

Copy link
Copy Markdown
Collaborator Author

CI status: every lane is green except :windows: x64-baseline verify-baseline, which is failing on main too (builds 60403 and 60365 fail identically): the Intel SDE download in that step returns a 0-byte sde.tar.xz and 7-Zip exits 2 before any test runs. Not related to this diff.

Summary for review: namespace Zig is fully merged into namespace Bun, the Zig-prefixed binding files and the C ABI exception types are renamed on both the C++ and Rust sides, and review feedback (stale CLAUDE.md mention, Zig*.zig reference-file paths in comments) is addressed. ZigString/ZigStringSlice, ZIG_EXPORT/ZIG_DECL, ZigGeneratedClasses and crate-internal zig_* names are intentionally left for #30747. Ready for a human pass.

Comment thread src/jsc/VirtualMachine.rs Outdated
Comment thread src/jsc/lib.rs Outdated
Comment thread src/jsc/VirtualMachine.rs Outdated
Comment thread src/jsc/bindings/JSBufferList.cpp Outdated
Comment thread src/jsc/bindings/ModuleLoader.cpp Outdated
@robobun

robobun commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator Author

CI update: the test-bun lanes on build 60500 all fail on a single file, test/cli/install/bunx.test.ts ("should handle package that requires node 24"). This is external: @angular/cli@22.0.0 was just published with engines.node = "^22.22.3 || ^24.15.0 || >=26.0.0", and Bun reports Node v24.3.0, which satisfies none of those ranges, so the Angular CLI exits 3. The failure reproduces identically with the pre-rename release binary (USE_SYSTEM_BUN=1), so it is unrelated to this diff and will hit every branch including main on its next build. Fixing it likely means bumping REPORTED_NODEJS_VERSION or pinning the test to @angular/cli@21, either of which is a separate change.

The windows x64-baseline verify-baseline failure remains the known 0-byte Intel SDE download, also broken on main (builds 60403, 60365). Everything else is green.

@robobun robobun force-pushed the farm/50d63c3b/rename-zigglobalobject branch from 27a6dec to c727d0d Compare June 5, 2026 01:21
Comment thread src/jsc/bindings/BunException.cpp Outdated

@coderabbitai coderabbitai 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.

Actionable comments posted: 8

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
src/jsc/ErrorCode.rs (1)

1503-1516: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Don't export placeholder parser/JSError sentinels under the new Bun_* ABI names.

The updated comment says C++ must not link against these Rust statics until they are derived from the same source as ErrorCode::from(), but Lines 1513 and 1516 still export exactly those Bun_* symbols. That means the rename carries forward a real behavior bug: C++ will compare against 0xFFFE/0xFFFD, while ErrorCode::from() never produces those values, so parser-error / JS-error-object detection silently stops matching. Either derive these exports from the same anyerror source as from(), or keep the C++ side on the existing authoritative source until that port lands.

🤖 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/ErrorCode.rs` around lines 1503 - 1516, The two Rust statics
Bun_ErrorCodeParserError and Bun_ErrorCodeJSErrorObject are being exported under
the new Bun_* ABI names but do not match ErrorCode::from() values, so stop
exporting these placeholder sentinels: remove or rename the #[unsafe(no_mangle)]
exports for Bun_ErrorCodeParserError and Bun_ErrorCodeJSErrorObject (i.e., make
them private/internal rust statics or drop the no_mangle so C++ cannot link
against them), or alternatively change their initialization to derive the same
anyerror-backed u16 that ErrorCode::from() uses (e.g., obtain the value from the
same bun_core::Error anyerror interning path) and only then retain the Bun_* ABI
export; reference symbols: Bun_ErrorCodeParserError, Bun_ErrorCodeJSErrorObject,
ErrorCode::PARSER_ERROR, ErrorCode::JS_ERROR_OBJECT, and ErrorCode::from().
src/jsc/bindings/ImportMetaObject.cpp (1)

295-306: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Guard nullable dynamicDowncast result before first dereference.

Line 295 uses dynamicDowncast, but Line 305 dereferences globalObject before the null-check at Line 317. That can crash on non-Bun globals.

🔧 Suggested fix
-    if (globalObject->onLoadPlugins.hasVirtualModules()) {
+    if (globalObject && globalObject->onLoadPlugins.hasVirtualModules()) {
         if (moduleName.isString()) {
             auto moduleString = moduleName.toWTFString(globalObject);
             if (auto resolvedString = globalObject->onLoadPlugins.resolveVirtualModule(moduleString, from.toWTFString(globalObject))) {
                 if (moduleString == resolvedString.value())
                     return JSC::JSValue::encode(moduleName);
                 return JSC::JSValue::encode(jsString(vm, resolvedString.value()));
             }
         }
     }
🤖 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/bindings/ImportMetaObject.cpp` around lines 295 - 306, The result of
dynamicDowncast<Bun::GlobalObject> stored in globalObject may be null and is
dereferenced later; add an immediate null check after the dynamicDowncast
(before any use of globalObject, e.g., before accessing
globalObject->onLoadPlugins) and handle the null case by returning early (using
the same error/exception flow as other early returns, e.g., via
RETURN_IF_EXCEPTION or an empty return) or throwing an appropriate JS exception
so we never dereference a null pointer in ImportMetaObject.cpp; update the logic
around the existing RETURN_IF_EXCEPTION and subsequent uses of
moduleName/from/isESM/isRequireDotResolve/userPathList to assume globalObject is
non-null after the guard.
src/jsc/bindings/BunGlobalObject.cpp (1)

3298-3327: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Delete the commented-out legacy GlobalObject implementation instead of renaming it.

This block is dead code, and keeping the renamed Zig-era implementation here makes the file harder to audit during the rest of this migration. Please remove it rather than updating identifiers inside the comments.

As per coding guidelines, "Delete dead code in the same PR that makes it dead" and "Comments carry only durable non-obvious content."

🤖 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/bindings/BunGlobalObject.cpp` around lines 3298 - 3327, Remove the
entire commented-out legacy GlobalObject implementation block (the commented
definitions for GlobalObject::destroy, GlobalObject::visitChildrenImpl, and the
trailing DEFINE_VISIT_CHILDREN(Bun::GlobalObject);) instead of editing
identifiers in comments; simply delete that dead-code comment region so only the
active Zig-era implementation remains, and run a quick grep for
GlobalObject::destroy, visitChildrenImpl, and DEFINE_VISIT_CHILDREN to ensure no
needed code was accidentally removed.
src/jsc/bindings/NodeFSStatFSBinding.cpp (1)

364-377: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Use defaultGlobalObject() for the subclass realm.

getFunctionRealm() can return a non-Bun global here, so the static_cast<Bun::GlobalObject*> on Line 372 can read m_JSStatFS*ClassStructure from the wrong object layout. This is the same cross-realm bug class the PR already fixed elsewhere with defaultGlobalObject().

Suggested fix
-        auto* functionGlobalObject = static_cast<Bun::GlobalObject*>(
-            // ShadowRealm functions belong to a different global object.
-            getFunctionRealm(lexicalGlobalObject, newTarget));
+        auto* functionGlobalObject = defaultGlobalObject(
+            // ShadowRealm functions belong to a different global object.
+            getFunctionRealm(lexicalGlobalObject, newTarget));
🤖 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/bindings/NodeFSStatFSBinding.cpp` around lines 364 - 377, The code
currently static_casts the result of getFunctionRealm(lexicalGlobalObject,
newTarget) to Bun::GlobalObject*, which can yield a non-Bun layout; instead call
defaultGlobalObject on the realm returned by getFunctionRealm so you get a
Bun::GlobalObject* in the subclass path. Replace the static_cast line building
functionGlobalObject with something like: auto* functionGlobalObject =
defaultGlobalObject(getFunctionRealm(lexicalGlobalObject, newTarget)), then pass
that functionGlobalObject into getStatFSStructure<isBigInt> and
createSubclassStructure calls (symbols: getFunctionRealm, defaultGlobalObject,
functionGlobalObject, getStatFSStructure<isBigInt>, createSubclassStructure,
lexicalGlobalObject, newTarget).
🤖 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/codegen/generate-classes.ts`:
- Around line 659-663: The generated call() methods currently reinterpret_cast
the lexicalGlobalObject to Bun::GlobalObject and dereference Bun-only fields;
replace that with using Bun::defaultGlobalObject(lexicalGlobalObject) to obtain
a safe Bun::GlobalObject pointer (same approach as construct()), then get the VM
from that object and use DECLARE_THROW_SCOPE(vm); update the call()
implementations (and the other similar block around the other occurrence) to
mirror the construct() path by calling defaultGlobalObject(...) instead of
reinterpret_cast and proceed as before.
- Around line 773-775: The C++ thunks like ${typeName}__getConstructor must not
narrow their ABI parameter to Bun::GlobalObject*; revert the generated function
signatures to accept the generic JSC::JSGlobalObject* (or JSGlobalObject*) that
callers (Zig/Rust) expect so we don't downcast foreign globals; update the
signature for ${typeName}__getConstructor and apply the same pattern to
__createWithValues, __createWithInitialValues, and
__createWithValuesAndInitialValues so each uses the generic JSGlobalObject* and
then obtain the Bun-specific constructor via the
className(typeName)Constructor() accessor inside the function body.

In `@src/jsc/bindings/BunException.cpp`:
- Around line 2-5: Update the stale header that says “Zig exceptions” to reflect
the rename (e.g., “Bun exceptions” or “BunException”) in the top comment of
BunException.cpp and any other occurrences in this file; locate the header block
that begins with "BunException handling and error processing utilities" and
replace references to "Zig exceptions" with the correct term, and also sweep the
file for any other stray mentions of "Zig" (including function comments around
the exception conversion utilities) and update them to the new name.

In `@src/jsc/bindings/BunObject.cpp`:
- Line 623: Rename the misleading local variable zigGlobalObject to
bunGlobalObject wherever it's declared from
uncheckedDowncast<Bun::GlobalObject>(globalObject); specifically update the
declaration at the shown occurrence and the same pattern at the other
occurrences (lines referenced in the review) so any usage referring to
zigGlobalObject in functions that work with Bun::GlobalObject now use
bunGlobalObject for consistency with the type; ensure you update all variable
references and keep the uncheckedDowncast<Bun::GlobalObject>(globalObject)
expression unchanged.

In `@src/jsc/bindings/BunProcess.cpp`:
- Around line 392-395: Several public process host functions are directly
downcasting lexicalGlobalObject/globalObject_ to Bun::GlobalObject (e.g.,
Process_functionDlopen, Process_setUncaughtExceptionCaptureCallback,
Process_functionHRTime, Process_functionHRTimeBigInt, Process_emitWarning,
Process_functionGetReport, Process_functionBinding,
Process_functionLoadBuiltinModule); change those downcasts to normalize the
global object using defaultGlobalObject(...) (or add an inherits guard) like the
other process entry points do, then use the returned GlobalObject* (or bail
out/throw if null) before calling DECLARE_THROW_SCOPE/getVM or accessing
napiModuleRegisterCallCount to ensure safe, consistent global-object handling
across these functions.

In `@src/jsc/bindings/JSBufferList.cpp`:
- Line 467: The return contains an unnecessary static_cast of globalObject to
Bun::GlobalObject*; simply return globalObject->JSBufferList() instead. Update
the function in JSBufferList.cpp to remove static_cast<Bun::GlobalObject*> and
return globalObject->JSBufferList(), keeping the original return type unchanged
and relying on the existing globalObject parameter type.

In `@src/jsc/bindings/SQLClient.cpp`:
- Around line 138-139: Rename the local variable zigGlobal to reflect its actual
type Bun::GlobalObject* (e.g., bunGlobal or globalObjectPtr) wherever it's
declared and used; specifically update the declaration that currently reads
"Bun::GlobalObject* zigGlobal =
uncheckedDowncast<Bun::GlobalObject>(globalObject);" and all subsequent uses
such as the call to JSBufferSubclassStructure() and the other occurrences
mentioned (around the later block at the same file). Ensure you update every
reference (including the uses at the later 178-179 region) so grep/identifiers
are consistent with the new Bun::GlobalObject* type.

In `@src/jsc/VirtualMachine.rs`:
- Around line 3775-3777: Update the stale symbol name in the comment that
mentions the old constructor: replace `BunGlobalObject__create` with
`Bun__GlobalObject__create` in the comment block near the global-init logic (the
comment that explains routing through `init` then swapping the global), and
sweep the same PR for any other comments/JSDoc that still reference the old
`BunGlobalObject__create` symbol so all documentation matches the current
constructor name (`Bun__GlobalObject__create`).

---

Outside diff comments:
In `@src/jsc/bindings/BunGlobalObject.cpp`:
- Around line 3298-3327: Remove the entire commented-out legacy GlobalObject
implementation block (the commented definitions for GlobalObject::destroy,
GlobalObject::visitChildrenImpl, and the trailing
DEFINE_VISIT_CHILDREN(Bun::GlobalObject);) instead of editing identifiers in
comments; simply delete that dead-code comment region so only the active Zig-era
implementation remains, and run a quick grep for GlobalObject::destroy,
visitChildrenImpl, and DEFINE_VISIT_CHILDREN to ensure no needed code was
accidentally removed.

In `@src/jsc/bindings/ImportMetaObject.cpp`:
- Around line 295-306: The result of dynamicDowncast<Bun::GlobalObject> stored
in globalObject may be null and is dereferenced later; add an immediate null
check after the dynamicDowncast (before any use of globalObject, e.g., before
accessing globalObject->onLoadPlugins) and handle the null case by returning
early (using the same error/exception flow as other early returns, e.g., via
RETURN_IF_EXCEPTION or an empty return) or throwing an appropriate JS exception
so we never dereference a null pointer in ImportMetaObject.cpp; update the logic
around the existing RETURN_IF_EXCEPTION and subsequent uses of
moduleName/from/isESM/isRequireDotResolve/userPathList to assume globalObject is
non-null after the guard.

In `@src/jsc/bindings/NodeFSStatFSBinding.cpp`:
- Around line 364-377: The code currently static_casts the result of
getFunctionRealm(lexicalGlobalObject, newTarget) to Bun::GlobalObject*, which
can yield a non-Bun layout; instead call defaultGlobalObject on the realm
returned by getFunctionRealm so you get a Bun::GlobalObject* in the subclass
path. Replace the static_cast line building functionGlobalObject with something
like: auto* functionGlobalObject =
defaultGlobalObject(getFunctionRealm(lexicalGlobalObject, newTarget)), then pass
that functionGlobalObject into getStatFSStructure<isBigInt> and
createSubclassStructure calls (symbols: getFunctionRealm, defaultGlobalObject,
functionGlobalObject, getStatFSStructure<isBigInt>, createSubclassStructure,
lexicalGlobalObject, newTarget).

In `@src/jsc/ErrorCode.rs`:
- Around line 1503-1516: The two Rust statics Bun_ErrorCodeParserError and
Bun_ErrorCodeJSErrorObject are being exported under the new Bun_* ABI names but
do not match ErrorCode::from() values, so stop exporting these placeholder
sentinels: remove or rename the #[unsafe(no_mangle)] exports for
Bun_ErrorCodeParserError and Bun_ErrorCodeJSErrorObject (i.e., make them
private/internal rust statics or drop the no_mangle so C++ cannot link against
them), or alternatively change their initialization to derive the same
anyerror-backed u16 that ErrorCode::from() uses (e.g., obtain the value from the
same bun_core::Error anyerror interning path) and only then retain the Bun_* ABI
export; reference symbols: Bun_ErrorCodeParserError, Bun_ErrorCodeJSErrorObject,
ErrorCode::PARSER_ERROR, ErrorCode::JS_ERROR_OBJECT, and ErrorCode::from().
🪄 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: 2d5dd5c3-4e23-4ccc-920a-5a5d7b104d14

📥 Commits

Reviewing files that changed from the base of the PR and between ef89527 and b1cccec.

📒 Files selected for processing (300)
  • .claude/skills/implementing-jsc-classes-cpp/SKILL.md
  • .gitattributes
  • CLAUDE.md
  • scripts/build/codegen.ts
  • scripts/build/unified.ts
  • src/ast_jsc/lib.rs
  • src/bun_bin/phase_c_exports.rs
  • src/bun_core/string/mod.rs
  • src/bundler/analyze_transpiled_module.rs
  • src/bundler_jsc/analyze_jsc.rs
  • src/codegen/bundle-functions.ts
  • src/codegen/cppbind.ts
  • src/codegen/generate-classes.ts
  • src/codegen/generate-host-exports.ts
  • src/codegen/generate-js2native.ts
  • src/codegen/generate-jssink.ts
  • src/codegen/shared-types.ts
  • src/event_loop/README.md
  • src/http_jsc/websocket_client.rs
  • src/js/builtins/shell.ts
  • src/js/node/worker_threads.ts
  • src/jsc/BunErrorType.rs
  • src/jsc/BunException.rs
  • src/jsc/BunStackFrame.rs
  • src/jsc/BunStackFrameCode.rs
  • src/jsc/BunStackFramePosition.rs
  • src/jsc/BunStackTrace.rs
  • src/jsc/CommonStrings.rs
  • src/jsc/ConsoleObject.rs
  • src/jsc/DOMFormData.rs
  • src/jsc/Debugger.rs
  • src/jsc/ErrorCode.rs
  • src/jsc/Errorable.rs
  • src/jsc/Exception.rs
  • src/jsc/FetchHeaders.rs
  • src/jsc/JSGlobalObject.rs
  • src/jsc/JSGlobalObject.zig
  • src/jsc/JSValue.rs
  • src/jsc/ModuleLoader.rs
  • src/jsc/ResolvedSource.rs
  • src/jsc/RuntimeTranspilerStore.rs
  • src/jsc/SavedSourceMap.rs
  • src/jsc/VM.rs
  • src/jsc/VirtualMachine.rs
  • src/jsc/VirtualMachine.zig
  • src/jsc/array_buffer.rs
  • src/jsc/bindings/AsymmetricKeyValue.cpp
  • src/jsc/bindings/AsyncContextFrame.cpp
  • src/jsc/bindings/BakeAdditionsToGlobalObject.cpp
  • src/jsc/bindings/BunAnalyzeTranspiledModule.cpp
  • src/jsc/bindings/BunAnalyzeTranspiledModule.h
  • src/jsc/bindings/BunCPUProfiler.cpp
  • src/jsc/bindings/BunClientData.cpp
  • src/jsc/bindings/BunClientData.h
  • src/jsc/bindings/BunCommonStrings.cpp
  • src/jsc/bindings/BunDebugger.cpp
  • src/jsc/bindings/BunException.cpp
  • src/jsc/bindings/BunGeneratedCode.cpp
  • src/jsc/bindings/BunGlobalObject.cpp
  • src/jsc/bindings/BunGlobalObject.h
  • src/jsc/bindings/BunGlobalObject.lut.txt
  • src/jsc/bindings/BunGlobalScope.cpp
  • src/jsc/bindings/BunHttp2CommonStrings.cpp
  • src/jsc/bindings/BunLazyStaticFunctions-inlines.h
  • src/jsc/bindings/BunLazyStaticFunctions.h
  • src/jsc/bindings/BunMarkdownMeta.cpp
  • src/jsc/bindings/BunMarkdownMeta.h
  • src/jsc/bindings/BunMarkdownTagStrings.cpp
  • src/jsc/bindings/BunObject.cpp
  • src/jsc/bindings/BunPlugin.cpp
  • src/jsc/bindings/BunPlugin.h
  • src/jsc/bindings/BunProcess.cpp
  • src/jsc/bindings/BunProcess.h
  • src/jsc/bindings/BunProcessReportObjectWindows.cpp
  • src/jsc/bindings/BunSecureContextCache.cpp
  • src/jsc/bindings/BunSecureContextCache.h
  • src/jsc/bindings/BunSourceProvider.cpp
  • src/jsc/bindings/BunSourceProvider.h
  • src/jsc/bindings/BunString.cpp
  • src/jsc/bindings/BundlerMetafile.cpp
  • src/jsc/bindings/CallSite.cpp
  • src/jsc/bindings/CallSite.h
  • src/jsc/bindings/CallSitePrototype.cpp
  • src/jsc/bindings/CallSitePrototype.h
  • src/jsc/bindings/CodeCoverage.cpp
  • src/jsc/bindings/ConsoleObject.h
  • src/jsc/bindings/DOMWrapperWorld.cpp
  • src/jsc/bindings/DOMWrapperWorld.h
  • src/jsc/bindings/ErrorCode.cpp
  • src/jsc/bindings/ErrorCode.h
  • src/jsc/bindings/ErrorStackFrame.cpp
  • src/jsc/bindings/ErrorStackFrame.h
  • src/jsc/bindings/ErrorStackTrace.cpp
  • src/jsc/bindings/ErrorStackTrace.h
  • src/jsc/bindings/EventLoopTaskNoContext.h
  • src/jsc/bindings/ExposeNodeModuleGlobals.cpp
  • src/jsc/bindings/FormatStackTraceForJS.cpp
  • src/jsc/bindings/FormatStackTraceForJS.h
  • src/jsc/bindings/FuzzilliREPRL.cpp
  • src/jsc/bindings/HTMLEntryPoint.cpp
  • src/jsc/bindings/IPC.cpp
  • src/jsc/bindings/ImportMetaObject.cpp
  • src/jsc/bindings/ImportMetaObject.h
  • src/jsc/bindings/InspectorBunFrontendDevServerAgent.cpp
  • src/jsc/bindings/InspectorHTTPServerAgent.cpp
  • src/jsc/bindings/InspectorLifecycleAgent.cpp
  • src/jsc/bindings/InspectorLifecycleAgent.h
  • src/jsc/bindings/InspectorTestReporterAgent.cpp
  • src/jsc/bindings/InternalForTesting.cpp
  • src/jsc/bindings/InternalForTesting.h
  • src/jsc/bindings/InternalModuleRegistry.cpp
  • src/jsc/bindings/IsolatedModuleCache.cpp
  • src/jsc/bindings/IsolatedModuleCache.h
  • src/jsc/bindings/JS2Native.cpp
  • src/jsc/bindings/JSBakeResponse.cpp
  • src/jsc/bindings/JSBakeResponse.h
  • src/jsc/bindings/JSBuffer.cpp
  • src/jsc/bindings/JSBufferList.cpp
  • src/jsc/bindings/JSBufferList.h
  • src/jsc/bindings/JSBunRequest.cpp
  • src/jsc/bindings/JSBunRequest.h
  • src/jsc/bindings/JSBundlerPlugin.cpp
  • src/jsc/bindings/JSBundlerPlugin.h
  • src/jsc/bindings/JSCTestingHelpers.cpp
  • src/jsc/bindings/JSCTestingHelpers.h
  • src/jsc/bindings/JSCommonJSExtensions.cpp
  • src/jsc/bindings/JSCommonJSModule.cpp
  • src/jsc/bindings/JSCommonJSModule.h
  • src/jsc/bindings/JSDOMExceptionHandling.cpp
  • src/jsc/bindings/JSDOMFile.cpp
  • src/jsc/bindings/JSDOMGlobalObject.cpp
  • src/jsc/bindings/JSDOMGlobalObject.h
  • src/jsc/bindings/JSDOMWrapper.h
  • src/jsc/bindings/JSDOMWrapperCache.cpp
  • src/jsc/bindings/JSEnvironmentVariableMap.cpp
  • src/jsc/bindings/JSEnvironmentVariableMap.h
  • src/jsc/bindings/JSFFIFunction.cpp
  • src/jsc/bindings/JSFFIFunction.h
  • src/jsc/bindings/JSMockFunction.cpp
  • src/jsc/bindings/JSMockFunction.h
  • src/jsc/bindings/JSNodePerformanceHooksHistogram.cpp
  • src/jsc/bindings/JSNodePerformanceHooksHistogramConstructor.cpp
  • src/jsc/bindings/JSNodePerformanceHooksHistogramPrototype.h
  • src/jsc/bindings/JSPropertyIterator.cpp
  • src/jsc/bindings/JSReactElement.cpp
  • src/jsc/bindings/JSReactElement.h
  • src/jsc/bindings/JSS3File.cpp
  • src/jsc/bindings/JSS3File.h
  • src/jsc/bindings/JSSecrets.cpp
  • src/jsc/bindings/JSSocketAddressDTO.cpp
  • src/jsc/bindings/JSSocketAddressDTO.h
  • src/jsc/bindings/JSStringDecoder.cpp
  • src/jsc/bindings/JSWrappingFunction.cpp
  • src/jsc/bindings/JSWrappingFunction.h
  • src/jsc/bindings/JSX509Certificate.cpp
  • src/jsc/bindings/JSX509Certificate.h
  • src/jsc/bindings/JSX509CertificateConstructor.cpp
  • src/jsc/bindings/JSX509CertificateConstructor.h
  • src/jsc/bindings/JSX509CertificatePrototype.cpp
  • src/jsc/bindings/ModuleLoader.cpp
  • src/jsc/bindings/ModuleLoader.h
  • src/jsc/bindings/NapiClass.cpp
  • src/jsc/bindings/NapiRef.cpp
  • src/jsc/bindings/NapiWeakValue.cpp
  • src/jsc/bindings/NativePromiseContext.cpp
  • src/jsc/bindings/NodeAsyncHooks.cpp
  • src/jsc/bindings/NodeAsyncHooks.h
  • src/jsc/bindings/NodeDirent.cpp
  • src/jsc/bindings/NodeFSStatBinding.cpp
  • src/jsc/bindings/NodeFSStatFSBinding.cpp
  • src/jsc/bindings/NodeFetch.cpp
  • src/jsc/bindings/NodeFetch.h
  • src/jsc/bindings/NodeHTTP.cpp
  • src/jsc/bindings/NodeHTTP.h
  • src/jsc/bindings/NodeTLS.cpp
  • src/jsc/bindings/NodeTLS.h
  • src/jsc/bindings/NodeTimerObject.cpp
  • src/jsc/bindings/NodeURL.cpp
  • src/jsc/bindings/NodeURL.h
  • src/jsc/bindings/NodeVM.cpp
  • src/jsc/bindings/NodeVM.h
  • src/jsc/bindings/NodeValidator.cpp
  • src/jsc/bindings/NodeValidator.h
  • src/jsc/bindings/Path.cpp
  • src/jsc/bindings/Path.h
  • src/jsc/bindings/ProcessBindingFs.cpp
  • src/jsc/bindings/ProcessBindingHTTPParser.cpp
  • src/jsc/bindings/ProcessBindingTTYWrap.cpp
  • src/jsc/bindings/ProcessBindingTTYWrap.h
  • src/jsc/bindings/ProcessBindingUV.cpp
  • src/jsc/bindings/SQLClient.cpp
  • src/jsc/bindings/ScriptExecutionContext.cpp
  • src/jsc/bindings/ServerRouteList.cpp
  • src/jsc/bindings/ServerRouteList.h
  • src/jsc/bindings/ShellBindings.cpp
  • src/jsc/bindings/StrongRef.cpp
  • src/jsc/bindings/URLSearchParams.cpp
  • src/jsc/bindings/Undici.cpp
  • src/jsc/bindings/Undici.h
  • src/jsc/bindings/UtilInspect.cpp
  • src/jsc/bindings/bindings.cpp
  • src/jsc/bindings/headers-cpp.h
  • src/jsc/bindings/headers-handwritten.h
  • src/jsc/bindings/headers.h
  • src/jsc/bindings/helpers.h
  • src/jsc/bindings/napi.cpp
  • src/jsc/bindings/napi.h
  • src/jsc/bindings/napi_external.h
  • src/jsc/bindings/napi_handle_scope.cpp
  • src/jsc/bindings/napi_handle_scope.h
  • src/jsc/bindings/napi_type_tag.cpp
  • src/jsc/bindings/node/JSNodeHTTPServerSocket.cpp
  • src/jsc/bindings/node/JSNodeHTTPServerSocket.h
  • src/jsc/bindings/node/JSNodeHTTPServerSocketPrototype.cpp
  • src/jsc/bindings/node/crypto/CryptoUtil.cpp
  • src/jsc/bindings/node/crypto/CryptoUtil.h
  • src/jsc/bindings/node/crypto/JSCipher.cpp
  • src/jsc/bindings/node/crypto/JSDiffieHellman.cpp
  • src/jsc/bindings/node/crypto/JSDiffieHellmanGroup.cpp
  • src/jsc/bindings/node/crypto/JSDiffieHellmanGroupConstructor.cpp
  • src/jsc/bindings/node/crypto/JSECDH.cpp
  • src/jsc/bindings/node/crypto/JSKeyObject.cpp
  • src/jsc/bindings/node/crypto/JSKeyObjectConstructor.cpp
  • src/jsc/bindings/node/crypto/JSPrivateKeyObject.cpp
  • src/jsc/bindings/node/crypto/JSPublicKeyObject.cpp
  • src/jsc/bindings/node/crypto/JSSecretKeyObject.cpp
  • src/jsc/bindings/node/crypto/JSSign.cpp
  • src/jsc/bindings/node/crypto/JSVerify.cpp
  • src/jsc/bindings/node/crypto/KeyObject.cpp
  • src/jsc/bindings/node/crypto/node_crypto_binding.cpp
  • src/jsc/bindings/node/crypto/node_crypto_binding.h
  • src/jsc/bindings/node/http/JSConnectionsListConstructor.cpp
  • src/jsc/bindings/node/http/JSHTTPParserConstructor.cpp
  • src/jsc/bindings/node/http/JSHTTPParserPrototype.cpp
  • src/jsc/bindings/node/http/NodeHTTPParser.cpp
  • src/jsc/bindings/objects.h
  • src/jsc/bindings/root-pch.h
  • src/jsc/bindings/sqlite/JSSQLStatement.cpp
  • src/jsc/bindings/sqlite/JSSQLStatement.h
  • src/jsc/bindings/v8/V8Array.cpp
  • src/jsc/bindings/v8/V8Context.h
  • src/jsc/bindings/v8/V8Function.cpp
  • src/jsc/bindings/v8/V8Isolate.cpp
  • src/jsc/bindings/v8/V8Isolate.h
  • src/jsc/bindings/v8/V8Object.cpp
  • src/jsc/bindings/v8/shim/FunctionTemplate.cpp
  • src/jsc/bindings/v8/shim/GlobalInternals.cpp
  • src/jsc/bindings/v8/shim/GlobalInternals.h
  • src/jsc/bindings/v8/v8.h
  • src/jsc/bindings/webcore/AbortController.h
  • src/jsc/bindings/webcore/AbortSignal.h
  • src/jsc/bindings/webcore/DOMIsoSubspaces.h
  • src/jsc/bindings/webcore/JSCallbackData.cpp
  • src/jsc/bindings/webcore/JSDOMConstructorBase.h
  • src/jsc/bindings/webcore/JSDOMConvertBase.h
  • src/jsc/bindings/webcore/JSDOMGlobalObjectInlines.h
  • src/jsc/bindings/webcore/JSErrorEvent.cpp
  • src/jsc/bindings/webcore/JSEventEmitter.cpp
  • src/jsc/bindings/webcore/JSEventEmitterCustom.cpp
  • src/jsc/bindings/webcore/JSEventListener.cpp
  • src/jsc/bindings/webcore/JSEventTargetCustom.cpp
  • src/jsc/bindings/webcore/JSMIMEBindings.cpp
  • src/jsc/bindings/webcore/JSMIMEBindings.h
  • src/jsc/bindings/webcore/JSMIMEParams.cpp
  • src/jsc/bindings/webcore/JSMIMEParams.h
  • src/jsc/bindings/webcore/JSMIMEType.cpp
  • src/jsc/bindings/webcore/JSPerformance.cpp
  • src/jsc/bindings/webcore/JSPerformanceObserverCallback.cpp
  • src/jsc/bindings/webcore/JSReadableStream.cpp
  • src/jsc/bindings/webcore/JSWebSocket.cpp
  • src/jsc/bindings/webcore/JSWebSocket.h
  • src/jsc/bindings/webcore/MessagePort.cpp
  • src/jsc/bindings/webcore/PerformanceMark.cpp
  • src/jsc/bindings/webcore/PerformanceObserver.cpp
  • src/jsc/bindings/webcore/ReadableStream.cpp
  • src/jsc/bindings/webcore/SerializedScriptValue.cpp
  • src/jsc/bindings/webcore/WebSocket.cpp
  • src/jsc/bindings/webcore/Worker.cpp
  • src/jsc/bindings/webcore/Worker.h
  • src/jsc/bindings/xxhash3.cpp
  • src/jsc/bindings/xxhash3_testing.cpp
  • src/jsc/headergen/sizegen.cpp
  • src/jsc/host_fn.rs
  • src/jsc/jsc.zig
  • src/jsc/lib.rs
  • src/jsc/modules/AbortControllerModuleModule.h
  • src/jsc/modules/BunAppModule.h
  • src/jsc/modules/BunJSCModule.h
  • src/jsc/modules/BunObjectModule.h
  • src/jsc/modules/BunTestModule.h
  • src/jsc/modules/NodeBufferModule.h
  • src/jsc/modules/NodeConstantsModule.h
  • src/jsc/modules/NodeModuleModule.cpp
  • src/jsc/modules/NodeModuleModule.h
  • src/jsc/modules/NodeProcessModule.h
  • src/jsc/modules/NodeStringDecoderModule.h
  • src/jsc/modules/NodeTTYModule.cpp
  • src/jsc/modules/NodeTTYModule.h
  • src/jsc/modules/NodeUtilTypesModule.cpp
  • src/jsc/modules/NodeUtilTypesModule.h

Comment thread src/codegen/generate-classes.ts
Comment thread src/codegen/generate-classes.ts
Comment thread src/jsc/bindings/BunException.cpp
Comment thread src/jsc/bindings/BunObject.cpp Outdated
Comment thread src/jsc/bindings/BunProcess.cpp
Comment thread src/jsc/bindings/JSBufferList.cpp Outdated
Comment thread src/jsc/bindings/SQLClient.cpp Outdated
Comment thread src/jsc/VirtualMachine.rs
@robobun robobun force-pushed the farm/50d63c3b/rename-zigglobalobject branch from fd6338b to 50d99c9 Compare June 5, 2026 03:53
@robobun

robobun commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator Author

Final CI status for build 60623 (50d99c9): every build lane passed on all platforms, and the only failure is windows x64-baseline verify-baseline. That lane fails identically on main: build 60590 at 8553428 (the exact commit this branch is rebased on) has the same single failure out of 30 statuses. The step's Intel SDE download returns a 0-byte sde.tar.xz and 7-Zip exits 2 before any test runs, so it cannot be related to this rename diff.

A retrigger was already spent on the same infra failure earlier, so I will not push another. The diff is green and review threads are resolved; this is ready for a maintainer.

Comment thread src/jsc/bindings/BakeAdditionsToGlobalObject.cpp Outdated
@robobun

robobun commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator Author

Same story on build 60643 (af18345, the bare zig local rename): every build lane passed and the only failure is windows x64-baseline verify-baseline, the known Intel SDE infra issue (0-byte sde.tar.xz download, 7-Zip exit 2) that fails identically on main's build 60590 at 8553428. Not caused by this diff and a retrigger was already spent on it earlier.

All review threads are resolved and the diff is green; ready for a maintainer.

@robobun

robobun commented Jun 5, 2026

Copy link
Copy Markdown
Collaborator Author

Final tally for build 60643 (af18345) now that the test lanes finished. Every failure is pre-existing or infra, none touch this rename:

  • test/cli/install/bunx.test.ts on 12 lanes: the external @angular/cli@22.0.0 engines issue documented above (engines.node ^22.22.3 || ^24.15.0 || >=26.0.0 vs Bun's reported v24.3.0). Verified earlier to reproduce identically with the pre-rename release binary (USE_SYSTEM_BUN=1); it will hit every branch including main on its next build.
  • test/js/web/streams/streams-leak.test.ts on exactly 1 of ~20 lanes (Debian 13 x64-baseline): an RSS-threshold assertion (250 MB / 1.5x baseline); passed on every other lane including ASAN, and passed on all lanes in this PR's previous full run (build 60500). Runner memory-pressure flake.
  • windows x64-baseline verify-baseline: the known Intel SDE 0-byte download infra failure, identical on main (build 60590 at 8553428).
  • 3 darwin test lanes Expired: Buildkite agent availability, no tests ran.

All build lanes green on every platform, review threads resolved, retrigger already spent. The diff itself is green; ready for a maintainer.

robobun and others added 6 commits June 10, 2026 00:07
The class dates to when Bun's runtime was written in Zig and the "Zig"
prefix meant "the global object that bridges to Zig code". The runtime
has since been ported, the prefix is misleading, and the header carried
a TODO asking for this rename since 2023.

- Zig::GlobalObject and Zig::EvalGlobalObject move to namespace Bun
- ZigGlobalObject.{h,cpp,lut.txt} become BunGlobalObject.{h,cpp,lut.txt}
- extern "C" symbols Zig__GlobalObject__* become Bun__GlobalObject__*,
  ZigGlobalObject__* become BunGlobalObject__*, and
  Bun__ZigGlobalObject__uvLoop becomes Bun__GlobalObject__uvLoop
- codegen templates, build scripts, leak suppressions and docs updated

No behavior change.
The constructor static_cast the lexical global to Bun::GlobalObject,
which is wrong when invoked from a non-Bun global such as a node:vm
context. Use defaultGlobalObject() so those callers fall back to the
main global's CommonJSModule structure instead of reading through a
bogus pointer.
…xed binding files

Continues the rename started with ZigGlobalObject:

- namespace Zig is gone; all members (SourceProvider, JSFFIFunction,
  CallSite, ImportMetaObject, JSCStackTrace, string helpers, ...) now
  live in namespace Bun
- ZigSourceProvider.{h,cpp}, ZigException.cpp, ZigGeneratedCode.cpp and
  ZigLazyStaticFunctions{,-inlines}.h are renamed to Bun*
- the C ABI exception types (ZigException, ZigStackTrace, ZigStackFrame,
  ZigStackFramePosition, ZigStackFrameCode, ZigErrorType, ZigErrorCode)
  are renamed to Bun* on both the C++ and Rust sides, along with their
  Rust source files and modules
- CommonStringsForZig -> CommonStringsForBun, Zig_ErrorCode* ->
  Bun_ErrorCode*, zig__ModuleInfo* and zig__renderDiff -> bun__*

ZigString/ZigStringSlice, ZIG_EXPORT/ZIG_DECL and the crate-internal
zig_* names are left for the larger sweep in #30747.
CLAUDE.md gained another ZigGlobalObject mention upstream between the
two rename passes; update it. Also restore the "ported from" and inline
comment references in the renamed Rust exception files to point at the
Zig*.zig reference files, which keep their original names until they
are deleted.
Restore the two quoted zig_exception enum spellings in VirtualMachine.rs
doc comments, correct the closing-brace comments on WebCore namespace
blocks in JSBufferList.cpp and JSStringDecoder.cpp, and drop the
self-referential using namespace Bun directives in ModuleLoader.cpp and
KeyObject.cpp that the namespace merge made no-ops.
@robobun

robobun commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

CI status for build 61595 (93d2cf8): the only error-level failure is test/cli/install/bunx.test.ts "should handle package that requires node 24", on debian 13 x64-asan, windows 2019 x64, and windows 2019 x64-baseline (the build is still running, so more test lanes will likely hit the same test).

That failure is environmental, not from this PR. The test runs bun x --bun @angular/cli@latest --help against the live registry, and Angular's latest release now requires a newer Node than Bun reports. Reproduced with released bun 1.4.0, no changes from this branch:

$ bun x --bun @angular/cli@latest --help
Node.js version v24.3.0 detected.
The Angular CLI requires a minimum Node.js version of v22.22.3 or v24.15.0 or v26.0.0.
(exit code 3)

It will fail on any branch until the test stops tracking @latest; #31820 is already open to pin the @angular/cli version.

The two warning-level annotations (test/js/node/worker_threads/15787.test.ts LSan ConcurrentTask leak on x64-asan, test/cli/install/bun-install.test.ts on windows x64) both passed on retry and are classified flaky by the pipeline; neither touches the rename.

No failure in this build involves the renamed symbols or files. The diff itself remains green: all build lanes on every platform compile, and earlier runs (60623, 60643) showed the same picture with only pre-existing or infra failures. This should be ready for a maintainer once #31820 lands or the bunx failure is waived.

@robobun

robobun commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

Update: main was merged into this branch (cdad656), which picks up #32042 skipping bunx.test.ts in CI, so the environmental failure from build 61595 no longer applies. I re-checked the merged tree: git grep for the renamed symbols (Zig::, namespace Zig, ZigGlobalObject, ZigException, ZigSourceProvider, ZigStackTrace, CommonStringsForZig, Zig__GlobalObject) still comes back empty outside the uncompiled .zig reference files, so nothing from main reintroduced the old names. CI is running fresh on build 61625.

@robobun

robobun commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

CI status for build 61625 (cdad656, the branch with main merged in): zero test failures so far. The build shows as failing only because 12 test shards (debian 13 aarch64, ubuntu 25.04 aarch64/x64, debian 13 x64) expired in the agent queue before being picked up, and expired jobs count against the build. They have not been retried; my Buildkite token is read-only so I cannot retry them from here.

Current job tally: 99 passed, ~175 still queued or running, 12 expired, 0 failed. The only annotations are three flaky tests that passed on retry (fetch-tls-abortsignal-timeout on windows, test/package.json on asan, shell rm on alpine), none related to the rename.

If the remaining lanes pass, the only thing standing between this and green is someone with write access clicking retry on the 12 expired shards at https://buildkite.com/bun/bun/builds/61625.

@robobun

robobun commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

Final picture for build 61625 (cdad656): 270 shards passed, 0 failures related to this PR. The red consists of:

  • 13 expired shards (debian/ubuntu x64 and aarch64) that timed out in the agent queue and never ran.
  • 1 failed darwin 26 aarch64 shard, which is a sick agent rather than a test problem: worker_heap_snapshot_gc.test.ts resolved its in-tree fixture for 9 attempts and then got "Module not found" for the same path, and seconds later the test runner itself crashed with uv_os_get_passwd returned ENOENT from os.userInfo(). That test plus fixture landed in worker: fix cross-thread HandleSet race in getHeapSnapshot #30185 a month ago and passed on this branch in builds 60623, 60643, and 61595.

The 11 flaky-test annotations all passed on retry and none touch the rename.

Everything red is retry-safe. My Buildkite token is read-only, so this needs someone with write access to hit retry on the expired shards and the darwin job at https://buildkite.com/bun/bun/builds/61625, or a rebuild.

@robobun

robobun commented Jun 10, 2026

Copy link
Copy Markdown
Collaborator Author

Build 61625 has now fully finished: 271 passed, 2 failed, 13 expired. The one failure that resolved since my last comment is autobahn.test.ts on darwin 14 aarch64, and it is also infra: the bun-autobahn:local docker image is linux/amd64 but the agent's docker host is linux/arm64 ("The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8)"), so the container dies with exec /opt/pypy/bin/wstest: exec format error before any WebSocket test runs.

Complete failure inventory, none touching this PR:

  • darwin 26 aarch64: sick agent (in-tree fixture became unreadable mid-test, then the runner crashed with uv_os_get_passwd ENOENT)
  • darwin 14 aarch64: autobahn docker image arch mismatch, test never executed
  • 13 expired shards: agent queue backlog, never ran

All of it is retry-safe; needs someone with Buildkite write access to retry those jobs or rebuild https://buildkite.com/bun/bun/builds/61625.

@robobun

robobun commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator Author

Merged current main into the branch again (46452b0) to clear the conflicts. One real conflict, in src/jsc/SavedSourceMap.rs: #32077 refactored the open-coded value-release logic into a release_value helper with a new type-erased AnySourceProvider representation, deleting the code where this PR had only renamed a comment. Resolved by taking main's side wholesale; this PR no longer touches that file.

Verification after the merge: the semantic-conflict sweep (git grep for Zig::, namespace Zig, ZigGlobalObject, ZigException, ZigSourceProvider, ZigStackTrace, CommonStringsForZig, Zig__GlobalObject, ZigErrorType) is clean outside the uncompiled .zig reference files, a full debug build passes, and vm.test.ts, internal-sourcemap.test.ts (exercises the conflicted SavedSourceMap path), and worker_heap_snapshot_gc.test.ts all pass locally.

Main also picked up #32072 (auto-retry jobs killed by agent loss) and #32033 (serialized docker service startup), which should address the infra failures that plagued builds 61595 and 61625.

Comment thread src/jsc/modules/NodeModuleModule.cpp Outdated
Comment thread src/jsc/modules/NodeModuleModule.h Outdated

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

5737a4b addresses my last nit on NodeModuleModule.h — nothing further from the bot pass. Deferring to a human given this is a 300-file rename across the C++/Rust FFI boundary in core JSC bindings, with a handful of non-mechanical bits (the defaultGlobalObject() swap in NodeModuleModule.cpp, the static unsafeEvalNoop, the JSC::Exception qualification) that warrant maintainer sign-off.

Extended reasoning...

Overview

This PR mechanically renames Zig-prefixed identifiers in the JSC bindings layer to Bun-prefixed equivalents: Zig::GlobalObjectBun::GlobalObject, namespace Zig merged into namespace Bun, ZigGlobalObject.{h,cpp,lut.txt}BunGlobalObject.*, ZigSourceProvider/ZigException/ZigStackTrace/etc. → Bun*, plus the matching extern "C" symbol renames (Zig__GlobalObject__*Bun__GlobalObject__*, zig__ModuleInfo*bun__*, etc.) coordinated across C++, Rust, Zig reference files, codegen templates (generate-classes.ts, bundle-functions.ts, cppbind.ts, generate-jssink.ts), build scripts, and docs. 300 files changed. It is the bindings-layer subset of the larger #30747 rename, landed fresh against main.

The PR description enumerates a small set of non-mechanical changes: making unsafeEvalNoop static to avoid a duplicate symbol after the namespace merge, qualifying one JSC::Exception in napi.cpp that became ambiguous, swapping a TODO'd static_cast<Zig::GlobalObject*> for defaultGlobalObject() in the Module constructor in NodeModuleModule.cpp, deleting a long-dead commented-out block in BunGlobalObject.cpp, and moving ~20 forward-decls from namespace Zig to namespace Bun blocks. A follow-up sweep (50d99c9, af18345) also renamed zigGlobal/zigGlobalObject/zig local variables tree-wide.

Security risks

None identified. This is a pure identifier/namespace rename with no changes to authentication, authorization, input handling, or data exposure. The one behavioral change (NodeModuleModule.cpp using defaultGlobalObject() instead of an unchecked static_cast) is a hardening fix for the node:vm foreign-realm case, not a new attack surface.

Level of scrutiny

High. While the change is overwhelmingly mechanical and the compiler/linker enforce most of the correctness (mismatched extern "C" symbols would fail to link; type mismatches would fail to compile), this touches the core runtime: BunGlobalObject.cpp (the global object itself), ModuleLoader.cpp, napi.cpp, bindings.cpp, the codegen templates that produce all generated JS classes, and the C ABI exception/stack-trace types shared between C++ and Rust. The extern "C" symbol renames span the FFI boundary in both directions and must agree exactly across C++, Rust, and the codegen output. The PR also represents a project-level staging decision (landing the bindings subset of #30747 separately) that a maintainer should ratify.

Other factors

  • The bug-hunting system found no issues on this run. Across ~10 prior bot review rounds on this PR, every finding was a documentation/cosmetic nit (stale .zig reference-file paths in comments, redundant namespace Bun blocks, mis-named local variables) — none were functional bugs. All have been addressed; the most recent (NodeModuleModule.h adjacent-namespace block) is fixed in commit 5737a4b, which I verified against the current file contents.
  • CodeRabbit raised three out-of-scope suggestions (using defaultGlobalObject() in more places, narrowing generated ABI signatures) which the author correctly declined as pre-existing behavior unrelated to the rename; CodeRabbit retracted those.
  • CI has been green on all build lanes across multiple runs; remaining test failures were documented as external (@angular/cli engines bump, Intel SDE infra, agent-loss expirations) and unrelated to this diff.
  • The author smoke-tested the renamed code paths (streams, vm, plugins, worker_threads, ffi, Error.prepareStackTrace, bun build) per the PR description.
  • No human reviewer has weighed in yet; the author has flagged "ready for a maintainer" several times.

Given the scale, the criticality of the touched files, and the project-level nature of the staging decision relative to #30747, this warrants a human maintainer's approval rather than bot sign-off.

@robobun

robobun commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator Author

CI status for build 61978 (5737a4b): 285 of 286 shards passed. The single failure is the darwin 14 aarch64 test shard, which timed out due to agent environment, not this diff:

  • test-docker-build-alpine/debian, sql.test.ts, sql-postgres-datetime-roundtrip, valkey complex-operations: the agent's Docker daemon is broken (docker-buildx plugin: permission denied), so every container-backed test on that machine fails or hangs.
  • test/cli/init/init.test.ts: the agent has Claude Code installed, so bun init emits a CLAUDE.md (per Template.isClaudeCodeInstalled() in src/cli/init_command.rs) and the inline snapshot gains an extra entry. Machine-specific, unrelated to this rename; the test passes on every other shard.

All build lanes are green, binary size is unchanged, and the bot review pass is complete with no findings. The diff is ready for maintainer review.

@robobun

robobun commented Jun 15, 2026

Copy link
Copy Markdown
Collaborator Author

Final CI status for build 62341 (7638aa0, after merging current main): 222 passed, 63 failed, 1 timed out. Every failure is environmental and reproduces on other unrelated PRs that ran in the same window; none are caused by this diff.

api.github.com outage (accounts for the vast majority of the 63 red shards):

  • package.json setup step: failed to download bun-tracestrings@github:oven-sh/bun.report#912ca63: HTTP 5xx on dozens of shards, so no tests ran on them at all.
  • test/cli/install/bun-install.test.ts, bun-add.test.ts, bun-create.test.ts, bun-install-lifecycle-scripts.test.ts, bun-install-registry.test.ts, GHSA-pfwx-36v6-832x.test.ts: all fail with GET https://api.github.com/repos/.../tarball/... - 504 or error.NPMIsDown.
  • The same bun.report 5xx and the same install-test 504s appear on concurrent unrelated builds 62342, 62343, 62344, 62345, 62346, 62350, 62351.

darwin agent flakes (each on 1 or 2 darwin aarch64 shards only):

  • test/cli/init/init.test.ts: snapshot gained an unexpected CLAUDE.md because this particular macOS agent has claude on PATH, which flips is_claude_code_installed() in init_command.rs. Also failing on unrelated build 62350.
  • test/js/bun/io/fetch/fetch-abort-slow-connect.test.ts: 50 ms AbortSignal.timeout race returned a generic Error instead of TimeoutError. Also failing on build 62350.
  • test/js/valkey/reliability/connection-failures.test.ts: 2 ms connection-timeout race returned Connection closed instead of the timeout message. Also failing on build 62350.
  • test/js/web/fetch/fetch-leak.test.ts: RSS threshold check off by 1.3 MB (received 54, expected ≥ 55.3125).

The binary-size annotation reports +0.0 KB on every target, so the compiled output is byte-identical to main, which is expected for a rename-only refactor. This branch does not touch src/install, src/runtime/cli/init_command.rs, src/runtime/webcore/fetch*, src/http, or any valkey code.

The previous full run on this branch (build 61978) was 285/286 green. This diff is ready for review/merge; the red here is the api.github.com blip plus the usual darwin flakes, and just needs a re-run.

…zigglobalobject

# Conflicts:
#	src/jsc/bindings/JSFFIFunction.cpp
#	src/jsc/bindings/v8/shim/FunctionTemplate.cpp
@robobun

robobun commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator Author

Merged current main into the branch again (0cf88d3). Two real conflicts, both from the recent upgrades:

  • src/jsc/bindings/JSFFIFunction.cpp: Upgrade WebKit to 51cc3feb7298 #31796 (WebKit upgrade) removed the finishCreation(VM&, NativeExecutable*, unsigned, const String&) overload on main while this branch had renamed its Zig::GlobalObject* parameter. Kept main's removal and the Bun::GlobalObject* signature on create().
  • src/jsc/bindings/v8/shim/FunctionTemplate.cpp: Upgrade reported Node.js version to 26.3.0 #31991 (Node 26.3.0) rewrote the ImplicitArgs struct into the new ApiCallbackExitFrame slot-vector layout. Took main's implementation and re-applied the one Zig::GlobalObjectBun::GlobalObject rename in the context comment.

Also caught a fresh Zig::GlobalObject mention in a new comment that #32407 added to src/jsc/bindings/BunClientData.h (auto-merged without conflict) and renamed it to match.

Debug build links and bun bd test passes locally for the FFI suite and this PR's own regression tests.

@robobun

robobun commented Jun 16, 2026

Copy link
Copy Markdown
Collaborator Author

Build 62890 (0cf88d3) did not run: the Buildkite orchestrator returned Image not found: linux-*-v37 for every build agent, so 1 pipeline step passed, 33 were canceled, and 252 never left waiting. No code was compiled or tested. The same agent-image error took down main build 62878 and every other PR build in the 62885-62890 range.

The diff built and passed tests locally after the merge (see previous comment). This just needs CI to be re-kicked once the build images are available again.

@robobun

robobun commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

Build 63071 (21ca0a3, after the latest main merge) finished: 285 shards passed, 1 failed.

The single failure is test/js/web/fetch/fetch-leak.test.ts > should not leak using readable stream on the darwin 14 aarch64 shard, where the RSS threshold check in fetch-leak-test-fixture-6.js came in at 55.73 MB against a 57.89 MB floor (~2 MB over). This same test is in the flaky annotation on main build 63036, and it was also the darwin flake on this branch's previous run (62341). This diff does not touch fetch, streams, or any allocation path.

Binary-size annotation reports +0.0 KB on every target against main #63069, which is the expected outcome for a rename-only refactor.

The diff is ready for review/merge.

…zigglobalobject

# Conflicts:
#	src/jsc/bindings/BunProcess.cpp
#	src/jsc/modules/NodeModuleModule.cpp
#	src/jsc/modules/NodeModuleModule.h
@robobun

robobun commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

Merged current main into the branch again (a7d58a4). Three conflicts, all from #31826 (the node:stream v26.3.0 sync):

Debug build links and bun bd test passes locally for this PR's regression tests plus a process.env / node:module smoke check.

Comment thread src/jsc/bindings/BunClientData.h
@robobun

robobun commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator Author

Build 63229 (f28363b, the test-comment fix on top of the a7d58a4 merge) finished: 260 shards passed, 1 failed, 25 canceled.

Canceled (25 Windows shards): fleet-wide agent provisioning outage. The robobun annotation reports Failed to create agent for {11 aarch64, 2019 x64, 2019 x64-baseline} - test-bun for every Windows test shard; the same 25-cancel pattern appears on concurrent unrelated builds 63224 (main), 63226, 63227, and 63228.

Failed (1): test/js/node/test/parallel/test-fs-watch-abort-signal.js with SIGTRAP on the single darwin 14 aarch64 shard. This branch's only AbortSignal-adjacent change is renaming the #include "ZigGlobalObject.h" line in src/jsc/bindings/webcore/AbortSignal.h to the new header path; the header content is identical. fs.watch itself was most recently touched on main by #32457 and #31848, both of which are already in this branch via the merge. The binary-size annotation reports +0.0 KB on darwin-aarch64 against main #63224, so the compiled output on the failing platform is byte-identical to main.

The diff remains ready for review; the red here is the Windows agent outage plus a single-shard darwin fs.watch trap that this rename cannot have introduced.

…zigglobalobject

# Conflicts:
#	src/jsc/bindings/NodeDirent.cpp
@robobun

robobun commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator Author

Merged current main into the branch again (ba9836d). One real conflict plus one new-file include to retarget:

git grep for Zig::, ZigGlobalObject, namespace Zig, and zigGlobalObject across src/ scripts/ packages/ test/ is now empty (excluding .zig reference files and the unrelated CryptoHasherZig Rust type name). Debug build links and this PR's regression tests plus an fs.Dirent constructor smoke check all pass locally.

@robobun

robobun commented Jun 19, 2026

Copy link
Copy Markdown
Collaborator Author

Build 63440 (ba9836d, after the latest main merge) finished: 284 shards passed, 2 failed. Binary-size annotation reports +0.0 KB on every target against main #63422.

The two failed shards:

  • test/bake/deinitialization.test.ts on debian 13 x64-asan: ASAN heap-use-after-free on thread T5 during the dev-server teardown stress. The same test is in the [flaky] annotation of unrelated concurrent build 63428 (farm/4b07618d/internal-http2-util). This branch's bake diffs are pure identifier renames (zig/zigGlobalbunGlobal, Zig::GlobalObjectBun::GlobalObject, ZigGlobalObject.hBunGlobalObject.h) with no control-flow change.
  • darwin 26 aarch64: two unrelated infra timeouts on one shard.
    • test/integration/next-pages/test/dev-server.test.ts: puppeteer's install.mjs threw Failed to set up chrome-headless-shell v139.0.7258.66 during the download step; the test never reached Bun code.
    • test/js/bun/s3/s3.test.ts: R2 > ... > should be able to upload large files in one go using Bun.write timed out at 15 s against the live R2 endpoint.

None of these touch the rename surface. The diff is ready for review/merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant