Fix unnecessary reinterpret_casts from JSGlobalObject to Zig::GlobalObject
#23387
Conversation
(For internal tracking: fixes STAB-1384)
|
Updated 7:03 PM PT - Oct 8th, 2025
❌ @taylordotfish, your commit 7310d38 has 3 failures in
🧪 To try this PR locally: bunx bun-pr 23387That installs a local version of the PR into your bun-23387 --bun |
WalkthroughReplaced multiple reinterpret_cast conversions to static_cast when converting to Zig::GlobalObject* across bindings, webcore, node, sqlite, and module headers. No control flow, error handling, or public API changes. Adjustments are limited to cast semantics within existing functions and construction paths. Changes
Suggested reviewers
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/bun.js/bindings/JSBakeResponse.cpp (1)
239-247: Consider defaultGlobalObject/jsCast for cross‑realm safetystatic_cast assumes lexicalGlobalObject is always a Zig::GlobalObject. For consistency with construct() and ShadowRealm handling, prefer defaultGlobalObject(lexicalGlobalObject) (or jsCast with an assert).
Apply:
- Zig::GlobalObject* globalObject = static_cast<Zig::GlobalObject*>(lexicalGlobalObject); + Zig::GlobalObject* globalObject = defaultGlobalObject(lexicalGlobalObject);Verify no call sites can pass a non‑Zig JSGlobalObject into call().
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (26)
src/bun.js/bindings/BakeAdditionsToGlobalObject.cpp(3 hunks)src/bun.js/bindings/BunDebugger.cpp(7 hunks)src/bun.js/bindings/BunPlugin.cpp(1 hunks)src/bun.js/bindings/BunProcess.cpp(5 hunks)src/bun.js/bindings/JSBakeResponse.cpp(3 hunks)src/bun.js/bindings/JSBufferList.cpp(3 hunks)src/bun.js/bindings/JSDOMFile.cpp(1 hunks)src/bun.js/bindings/JSMockFunction.cpp(1 hunks)src/bun.js/bindings/JSStringDecoder.cpp(1 hunks)src/bun.js/bindings/ModuleLoader.cpp(2 hunks)src/bun.js/bindings/NapiClass.cpp(1 hunks)src/bun.js/bindings/NodeFSStatBinding.cpp(1 hunks)src/bun.js/bindings/NodeFSStatFSBinding.cpp(1 hunks)src/bun.js/bindings/ScriptExecutionContext.cpp(1 hunks)src/bun.js/bindings/ZigGlobalObject.cpp(1 hunks)src/bun.js/bindings/bindings.cpp(7 hunks)src/bun.js/bindings/node/crypto/node_crypto_binding.cpp(1 hunks)src/bun.js/bindings/sqlite/JSSQLStatement.cpp(2 hunks)src/bun.js/bindings/webcore/JSEventEmitter.cpp(1 hunks)src/bun.js/bindings/webcore/JSEventEmitterCustom.cpp(1 hunks)src/bun.js/bindings/webcore/JSPerformance.cpp(1 hunks)src/bun.js/bindings/webcore/WebSocket.cpp(1 hunks)src/bun.js/modules/AbortControllerModuleModule.h(1 hunks)src/bun.js/modules/BunAppModule.h(1 hunks)src/bun.js/modules/NodeBufferModule.h(1 hunks)src/bun.js/modules/_NativeModule.h(1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{cpp,h}
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.{cpp,h}: When exposing a JS class with public Constructor and Prototype, define three C++ types: class Foo : public JSC::DestructibleObject (if it has C++ fields), class FooPrototype : public JSC::JSNonFinalObject, and class FooConstructor : public JSC::InternalFunction
If the class has C++ data members, inherit from JSC::DestructibleObject and provide proper destruction; if it has no C++ fields (only JS properties), avoid a class and use JSC::constructEmptyObject(vm, structure) with putDirectOffset
Prefer placing the subspaceFor implementation in the .cpp file rather than the header when possible
Files:
src/bun.js/bindings/NodeFSStatBinding.cppsrc/bun.js/bindings/ScriptExecutionContext.cppsrc/bun.js/bindings/webcore/WebSocket.cppsrc/bun.js/bindings/webcore/JSPerformance.cppsrc/bun.js/bindings/BunProcess.cppsrc/bun.js/bindings/JSBufferList.cppsrc/bun.js/bindings/BunDebugger.cppsrc/bun.js/bindings/BakeAdditionsToGlobalObject.cppsrc/bun.js/bindings/node/crypto/node_crypto_binding.cppsrc/bun.js/modules/_NativeModule.hsrc/bun.js/bindings/JSStringDecoder.cppsrc/bun.js/bindings/NodeFSStatFSBinding.cppsrc/bun.js/bindings/JSMockFunction.cppsrc/bun.js/modules/BunAppModule.hsrc/bun.js/bindings/sqlite/JSSQLStatement.cppsrc/bun.js/bindings/NapiClass.cppsrc/bun.js/bindings/BunPlugin.cppsrc/bun.js/bindings/webcore/JSEventEmitter.cppsrc/bun.js/bindings/ModuleLoader.cppsrc/bun.js/modules/AbortControllerModuleModule.hsrc/bun.js/modules/NodeBufferModule.hsrc/bun.js/bindings/JSBakeResponse.cppsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/webcore/JSEventEmitterCustom.cppsrc/bun.js/bindings/JSDOMFile.cpp
**/*.cpp
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
**/*.cpp: Include "root.h" at the top of C++ binding files to satisfy lints
Define prototype properties using a const HashTableValue array and declare accessors/functions with JSC_DECLARE_* macros
Prototype classes should subclass JSC::JSNonFinalObject, provide create/createStructure, DECLARE_INFO, finishCreation that reifies static properties, and set mayBePrototype on the Structure
Custom getters should use JSC_DEFINE_CUSTOM_GETTER, jsDynamicCast to validate this, and throwThisTypeError on mismatch
Custom setters should use JSC_DEFINE_CUSTOM_SETTER, validate this via jsDynamicCast, and store via WriteBarrier/set semantics
Prototype functions should use JSC_DEFINE_HOST_FUNCTION, validate this with jsDynamicCast, and return encoded JSValue
Constructors should subclass JSC::InternalFunction, return internalFunctionSpace in subspaceFor, set the prototype property as non-configurable/non-writable, and provide create/createStructure
Provide a setup function that builds the Prototype, Constructor, and Structure, and assigns them to the LazyClassStructure initializer
Use the cached Structure via globalObject->m_.get(globalObject) when constructing instances
Expose constructors to Zig via an extern "C" function that returns the constructor from the LazyClassStructure
Provide an extern "C" Bun____toJS function that creates an instance using the cached Structure and returns an EncodedJSValue
Files:
src/bun.js/bindings/NodeFSStatBinding.cppsrc/bun.js/bindings/ScriptExecutionContext.cppsrc/bun.js/bindings/webcore/WebSocket.cppsrc/bun.js/bindings/webcore/JSPerformance.cppsrc/bun.js/bindings/BunProcess.cppsrc/bun.js/bindings/JSBufferList.cppsrc/bun.js/bindings/BunDebugger.cppsrc/bun.js/bindings/BakeAdditionsToGlobalObject.cppsrc/bun.js/bindings/node/crypto/node_crypto_binding.cppsrc/bun.js/bindings/JSStringDecoder.cppsrc/bun.js/bindings/NodeFSStatFSBinding.cppsrc/bun.js/bindings/JSMockFunction.cppsrc/bun.js/bindings/sqlite/JSSQLStatement.cppsrc/bun.js/bindings/NapiClass.cppsrc/bun.js/bindings/BunPlugin.cppsrc/bun.js/bindings/webcore/JSEventEmitter.cppsrc/bun.js/bindings/ModuleLoader.cppsrc/bun.js/bindings/JSBakeResponse.cppsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/webcore/JSEventEmitterCustom.cppsrc/bun.js/bindings/JSDOMFile.cpp
src/bun.js/bindings/**/*.cpp
📄 CodeRabbit inference engine (CLAUDE.md)
src/bun.js/bindings/**/*.cpp: When a JS class has a public constructor, implement Foo, FooPrototype, and FooConstructor (JSDestructibleObject, JSNonFinalObject, InternalFunction)
Define class properties using HashTableValue arrays in C++ bindings
Add iso subspaces for classes with C++ fields
Cache structures in ZigGlobalObject for C++-backed classes
Files:
src/bun.js/bindings/NodeFSStatBinding.cppsrc/bun.js/bindings/ScriptExecutionContext.cppsrc/bun.js/bindings/webcore/WebSocket.cppsrc/bun.js/bindings/webcore/JSPerformance.cppsrc/bun.js/bindings/BunProcess.cppsrc/bun.js/bindings/JSBufferList.cppsrc/bun.js/bindings/BunDebugger.cppsrc/bun.js/bindings/BakeAdditionsToGlobalObject.cppsrc/bun.js/bindings/node/crypto/node_crypto_binding.cppsrc/bun.js/bindings/JSStringDecoder.cppsrc/bun.js/bindings/NodeFSStatFSBinding.cppsrc/bun.js/bindings/JSMockFunction.cppsrc/bun.js/bindings/sqlite/JSSQLStatement.cppsrc/bun.js/bindings/NapiClass.cppsrc/bun.js/bindings/BunPlugin.cppsrc/bun.js/bindings/webcore/JSEventEmitter.cppsrc/bun.js/bindings/ModuleLoader.cppsrc/bun.js/bindings/JSBakeResponse.cppsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/webcore/JSEventEmitterCustom.cppsrc/bun.js/bindings/JSDOMFile.cpp
src/bun.js/bindings/ZigGlobalObject.cpp
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
Initialize the LazyClassStructure in GlobalObject::finishCreation and visit it in GlobalObject::visitChildrenImpl
Files:
src/bun.js/bindings/ZigGlobalObject.cpp
src/bun.js/bindings/ZigGlobalObject.{h,cpp}
📄 CodeRabbit inference engine (.cursor/rules/javascriptcore-class.mdc)
If only a class (no public constructor/prototype), use JSC::LazyProperty<JSGlobalObject, Structure>, initialize it in finishCreation, and visit it in visitChildren
Files:
src/bun.js/bindings/ZigGlobalObject.cpp
🧠 Learnings (21)
📓 Common learnings
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Cache structures in ZigGlobalObject for C++-backed classes
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Use parameter name globalObject (not ctx) and accept (*JSC.JSGlobalObject, *JSC.CallFrame) in binding methods/constructors
Applied to files:
src/bun.js/bindings/NodeFSStatBinding.cppsrc/bun.js/bindings/ScriptExecutionContext.cppsrc/bun.js/bindings/webcore/JSPerformance.cppsrc/bun.js/bindings/BunProcess.cppsrc/bun.js/bindings/JSBufferList.cppsrc/bun.js/bindings/BunDebugger.cppsrc/bun.js/bindings/BakeAdditionsToGlobalObject.cppsrc/bun.js/modules/_NativeModule.hsrc/bun.js/bindings/NodeFSStatFSBinding.cppsrc/bun.js/bindings/JSMockFunction.cppsrc/bun.js/modules/BunAppModule.hsrc/bun.js/bindings/NapiClass.cppsrc/bun.js/bindings/BunPlugin.cppsrc/bun.js/bindings/webcore/JSEventEmitter.cppsrc/bun.js/bindings/ModuleLoader.cppsrc/bun.js/modules/AbortControllerModuleModule.hsrc/bun.js/bindings/JSBakeResponse.cppsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/webcore/JSEventEmitterCustom.cppsrc/bun.js/bindings/JSDOMFile.cpp
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : Cache structures in ZigGlobalObject for C++-backed classes
Applied to files:
src/bun.js/bindings/NodeFSStatBinding.cppsrc/bun.js/bindings/ScriptExecutionContext.cppsrc/bun.js/bindings/webcore/WebSocket.cppsrc/bun.js/bindings/webcore/JSPerformance.cppsrc/bun.js/bindings/BunProcess.cppsrc/bun.js/bindings/JSBufferList.cppsrc/bun.js/bindings/BakeAdditionsToGlobalObject.cppsrc/bun.js/modules/_NativeModule.hsrc/bun.js/bindings/JSMockFunction.cppsrc/bun.js/modules/BunAppModule.hsrc/bun.js/bindings/NapiClass.cppsrc/bun.js/bindings/BunPlugin.cppsrc/bun.js/bindings/ModuleLoader.cppsrc/bun.js/modules/NodeBufferModule.hsrc/bun.js/bindings/JSBakeResponse.cppsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/webcore/JSEventEmitterCustom.cppsrc/bun.js/bindings/JSDOMFile.cpp
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/**/*.{h,cpp} : Use localToJSValue() when converting V8 handles to JSC values before performing JSC operations
Applied to files:
src/bun.js/bindings/webcore/JSPerformance.cppsrc/bun.js/bindings/JSBakeResponse.cppsrc/bun.js/bindings/bindings.cpp
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Access JS call data via CallFrame (argument(i), argumentCount(), thisValue()) and throw errors with globalObject.throw(...)
Applied to files:
src/bun.js/bindings/BunProcess.cppsrc/bun.js/bindings/BunPlugin.cppsrc/bun.js/bindings/JSBakeResponse.cpp
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/bindings/generated_classes_list.zig : Update src/bun.js/bindings/generated_classes_list.zig to include new classes
Applied to files:
src/bun.js/bindings/JSBufferList.cppsrc/bun.js/modules/_NativeModule.hsrc/bun.js/modules/BunAppModule.hsrc/bun.js/bindings/NapiClass.cppsrc/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : In Zig binding structs, expose generated bindings via pub const js = JSC.Codegen.JS<ClassName> and re-export toJS/fromJS/fromJSDirect
Applied to files:
src/bun.js/bindings/JSBufferList.cppsrc/bun.js/modules/_NativeModule.hsrc/bun.js/modules/BunAppModule.hsrc/bun.js/bindings/NapiClass.cppsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/JSDOMFile.cpp
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to **/*.zig : Wrap the Bun__<Type>__toJS extern in a Zig method that takes a JSGlobalObject and returns JSC.JSValue
Applied to files:
src/bun.js/bindings/JSBufferList.cppsrc/bun.js/bindings/BakeAdditionsToGlobalObject.cppsrc/bun.js/modules/_NativeModule.hsrc/bun.js/modules/BunAppModule.hsrc/bun.js/bindings/BunPlugin.cppsrc/bun.js/bindings/JSBakeResponse.cppsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/JSDOMFile.cpp
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to src/bun.js/bindings/ZigGlobalObject.{h,cpp} : If only a class (no public constructor/prototype), use JSC::LazyProperty<JSGlobalObject, Structure>, initialize it in finishCreation, and visit it in visitChildren
Applied to files:
src/bun.js/bindings/BakeAdditionsToGlobalObject.cppsrc/bun.js/modules/_NativeModule.hsrc/bun.js/bindings/JSMockFunction.cppsrc/bun.js/modules/BunAppModule.hsrc/bun.js/bindings/sqlite/JSSQLStatement.cppsrc/bun.js/bindings/NapiClass.cppsrc/bun.js/bindings/BunPlugin.cppsrc/bun.js/bindings/webcore/JSEventEmitter.cppsrc/bun.js/bindings/ModuleLoader.cppsrc/bun.js/bindings/JSBakeResponse.cppsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/ZigGlobalObject.cppsrc/bun.js/bindings/webcore/JSEventEmitterCustom.cppsrc/bun.js/bindings/JSDOMFile.cpp
📚 Learning: 2025-08-30T00:11:57.076Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/registering-bun-modules.mdc:0-0
Timestamp: 2025-08-30T00:11:57.076Z
Learning: Applies to src/{**/js_*.zig,bun.js/api/**/*.zig} : Use bun.JSError!JSValue for proper error propagation in JS-exposed Zig functions
Applied to files:
src/bun.js/bindings/BakeAdditionsToGlobalObject.cpp
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to src/bun.js/bindings/ZigGlobalObject.h : When there is a class, prototype, and constructor, add a JSC::LazyClassStructure field for the class to ZigGlobalObject.h
Applied to files:
src/bun.js/bindings/BakeAdditionsToGlobalObject.cppsrc/bun.js/modules/BunAppModule.hsrc/bun.js/bindings/sqlite/JSSQLStatement.cppsrc/bun.js/bindings/NapiClass.cppsrc/bun.js/bindings/ModuleLoader.cppsrc/bun.js/bindings/JSBakeResponse.cppsrc/bun.js/bindings/bindings.cppsrc/bun.js/bindings/JSDOMFile.cpp
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Constructors and prototype methods should return bun.JSError!JSC.JSValue to integrate Zig error handling with JS exceptions
Applied to files:
src/bun.js/bindings/BakeAdditionsToGlobalObject.cpp
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to **/*.cpp : Provide an extern "C" Bun__<Type>__toJS function that creates an instance using the cached Structure and returns an EncodedJSValue
Applied to files:
src/bun.js/bindings/BakeAdditionsToGlobalObject.cppsrc/bun.js/bindings/bindings.cpp
📚 Learning: 2025-08-30T00:13:36.815Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/zig-javascriptcore-classes.mdc:0-0
Timestamp: 2025-08-30T00:13:36.815Z
Learning: Applies to src/bun.js/**/*.zig : Implement getters as get<PropertyName>(this, globalObject) returning JSC.JSValue and matching the .classes.ts interface
Applied to files:
src/bun.js/bindings/BakeAdditionsToGlobalObject.cppsrc/bun.js/modules/NodeBufferModule.h
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/src/napi/napi.zig : Add new V8 API method mangled symbols to the V8API struct in src/napi/napi.zig for both GCC/Clang and MSVC
Applied to files:
src/bun.js/modules/_NativeModule.hsrc/bun.js/bindings/NapiClass.cpp
📚 Learning: 2025-09-08T00:41:12.052Z
Learnt from: CR
PR: oven-sh/bun#0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-09-08T00:41:12.052Z
Learning: Applies to src/bun.js/bindings/v8/V8*.h : Ensure all public V8 API functions are marked with BUN_EXPORT for symbol visibility
Applied to files:
src/bun.js/modules/BunAppModule.hsrc/bun.js/bindings/BunPlugin.cpp
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to src/bun.js/bindings/ZigGlobalObject.cpp : Initialize the LazyClassStructure in GlobalObject::finishCreation and visit it in GlobalObject::visitChildrenImpl
Applied to files:
src/bun.js/bindings/sqlite/JSSQLStatement.cppsrc/bun.js/bindings/NapiClass.cppsrc/bun.js/bindings/BunPlugin.cppsrc/bun.js/bindings/ModuleLoader.cppsrc/bun.js/bindings/JSBakeResponse.cppsrc/bun.js/bindings/ZigGlobalObject.cpp
📚 Learning: 2025-10-01T21:59:54.571Z
Learnt from: taylordotfish
PR: oven-sh/bun#23169
File: src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h:47-74
Timestamp: 2025-10-01T21:59:54.571Z
Learning: In the new bindings generator (bindgenv2) for `src/bun.js/bindings/webcore/JSDOMConvertEnumeration.h`, the context-aware enumeration conversion overloads intentionally use stricter validation (requiring `value.isString()` without ToString coercion), diverging from Web IDL semantics. This is a design decision documented in comments.
Applied to files:
src/bun.js/bindings/webcore/JSEventEmitter.cppsrc/bun.js/bindings/webcore/JSEventEmitterCustom.cppsrc/bun.js/bindings/JSDOMFile.cpp
📚 Learning: 2025-09-19T19:55:22.427Z
Learnt from: markovejnovic
PR: oven-sh/bun#22568
File: src/valkey/js_valkey_functions.zig:853-854
Timestamp: 2025-09-19T19:55:22.427Z
Learning: In Bun's JSPromise API, `resolvedPromiseValue(globalThis: *JSGlobalObject, value: JSValue) JSValue` returns a JSValue directly (not a JSPromise that needs `.toJS()` called on it). This is different from `resolvedPromise()` which returns a *JSPromise that would need `.toJS()`.
Applied to files:
src/bun.js/bindings/bindings.cpp
📚 Learning: 2025-08-30T00:11:00.890Z
Learnt from: CR
PR: oven-sh/bun#0
File: .cursor/rules/javascriptcore-class.mdc:0-0
Timestamp: 2025-08-30T00:11:00.890Z
Learning: Applies to **/*.cpp : Prototype functions should use JSC_DEFINE_HOST_FUNCTION, validate this with jsDynamicCast, and return encoded JSValue
Applied to files:
src/bun.js/bindings/bindings.cpp
📚 Learning: 2025-10-04T09:51:30.294Z
Learnt from: CR
PR: oven-sh/bun#0
File: CLAUDE.md:0-0
Timestamp: 2025-10-04T09:51:30.294Z
Learning: Applies to src/bun.js/bindings/**/*.cpp : When a JS class has a public constructor, implement Foo, FooPrototype, and FooConstructor (JSDestructibleObject, JSNonFinalObject, InternalFunction)
Applied to files:
src/bun.js/bindings/JSDOMFile.cpp
🧬 Code graph analysis (21)
src/bun.js/bindings/webcore/WebSocket.cpp (1)
src/bun.js/bindings/BunDebugger.cpp (5)
globalObject(48-51)globalObject(48-48)globalObject(195-198)globalObject(208-261)globalObject(208-208)
src/bun.js/bindings/webcore/JSPerformance.cpp (1)
src/bun.js/bindings/BunProcess.cpp (1)
Bun__readOriginTimerStart(708-708)
src/bun.js/bindings/BunProcess.cpp (2)
src/bun.js/bindings/BunDebugger.cpp (5)
globalObject(48-51)globalObject(48-48)globalObject(195-198)globalObject(208-261)globalObject(208-208)src/bun.js/bindings/ZigGlobalObject.h (1)
globalObject(127-127)
src/bun.js/bindings/JSBufferList.cpp (1)
src/bun.js/bindings/BunDebugger.cpp (5)
globalObject(48-51)globalObject(48-48)globalObject(195-198)globalObject(208-261)globalObject(208-208)
src/bun.js/bindings/BunDebugger.cpp (1)
src/bun.js/bindings/ScriptExecutionContext.cpp (3)
Bun__eventLoop__incrementRefConcurrently(117-117)globalObject(93-96)globalObject(93-93)
src/bun.js/bindings/BakeAdditionsToGlobalObject.cpp (2)
src/bun.js/bindings/BunDebugger.cpp (5)
globalObject(48-51)globalObject(48-48)globalObject(195-198)globalObject(208-261)globalObject(208-208)src/bun.js/bindings/ZigGlobalObject.h (1)
globalObject(127-127)
src/bun.js/bindings/node/crypto/node_crypto_binding.cpp (2)
src/bun.js/bindings/ModuleLoader.cpp (4)
create(166-170)create(166-166)create(199-205)create(199-199)src/bun.js/bindings/ZigGlobalObject.cpp (8)
create(889-894)create(889-889)create(896-901)create(896-896)create(903-908)create(903-903)create(910-915)create(910-910)
src/bun.js/bindings/JSStringDecoder.cpp (1)
src/bun.js/bindings/BunDebugger.cpp (5)
globalObject(48-51)globalObject(48-48)globalObject(195-198)globalObject(208-261)globalObject(208-208)
src/bun.js/bindings/JSMockFunction.cpp (3)
src/bun.js/bindings/ModuleLoader.cpp (4)
create(166-170)create(166-166)create(199-205)create(199-199)src/bun.js/bindings/ZigGlobalObject.cpp (8)
create(889-894)create(889-889)create(896-901)create(896-896)create(903-908)create(903-903)create(910-915)create(910-910)src/bun.js/bindings/ZigGlobalObject.h (1)
globalObject(127-127)
src/bun.js/modules/BunAppModule.h (3)
src/bun.js/modules/AbortControllerModuleModule.h (1)
Zig(9-52)src/bun.js/bindings/ZigGlobalObject.cpp (3)
GlobalObject(1305-1318)GlobalObject(1320-1333)GlobalObject(1335-1341)src/bun.js/bindings/ZigGlobalObject.h (4)
GlobalObject(787-790)GlobalObject(793-800)GlobalObject(801-804)globalObject(127-127)
src/bun.js/bindings/sqlite/JSSQLStatement.cpp (2)
src/bun.js/bindings/JSMockFunction.cpp (8)
create(664-781)create(664-664)create(1296-1300)create(1296-1296)create(1330-1335)create(1330-1330)globalObject(154-160)globalObject(154-154)src/bun.js/bindings/ZigGlobalObject.cpp (8)
create(889-894)create(889-889)create(896-901)create(896-896)create(903-908)create(903-903)create(910-915)create(910-910)
src/bun.js/bindings/NapiClass.cpp (2)
src/bun.js/bindings/BunDebugger.cpp (5)
globalObject(48-51)globalObject(48-48)globalObject(195-198)globalObject(208-261)globalObject(208-208)src/bun.js/bindings/ScriptExecutionContext.cpp (2)
globalObject(93-96)globalObject(93-93)
src/bun.js/bindings/BunPlugin.cpp (1)
src/bun.js/bindings/BunDebugger.cpp (5)
globalObject(48-51)globalObject(48-48)globalObject(195-198)globalObject(208-261)globalObject(208-208)
src/bun.js/bindings/webcore/JSEventEmitter.cpp (1)
src/bun.js/bindings/JSMockFunction.cpp (11)
create(664-781)create(664-664)create(1296-1300)create(1296-1296)create(1330-1335)create(1330-1330)object(842-842)object(857-857)object(871-871)object(886-886)object(902-902)
src/bun.js/bindings/ModuleLoader.cpp (2)
src/bun.js/bindings/ZigGlobalObject.cpp (8)
create(889-894)create(889-889)create(896-901)create(896-896)create(903-908)create(903-903)create(910-915)create(910-910)src/bun.js/bindings/ZigGlobalObject.h (1)
globalObject(127-127)
src/bun.js/modules/AbortControllerModuleModule.h (3)
src/bun.js/modules/BunAppModule.h (1)
Zig(7-23)src/bun.js/modules/NodeBufferModule.h (1)
Zig(13-218)src/bun.js/bindings/ZigGlobalObject.cpp (3)
GlobalObject(1305-1318)GlobalObject(1320-1333)GlobalObject(1335-1341)
src/bun.js/modules/NodeBufferModule.h (4)
src/bun.js/modules/AbortControllerModuleModule.h (1)
Zig(9-52)src/bun.js/modules/BunAppModule.h (1)
Zig(7-23)src/bun.js/bindings/helpers.h (5)
Zig(15-17)JSValue(336-348)JSValue(350-356)JSValue(358-364)JSValue(366-372)src/bun.js/bindings/ZigGlobalObject.cpp (3)
GlobalObject(1305-1318)GlobalObject(1320-1333)GlobalObject(1335-1341)
src/bun.js/bindings/JSBakeResponse.cpp (2)
src/bun.js/bindings/BunDebugger.cpp (5)
globalObject(48-51)globalObject(48-48)globalObject(195-198)globalObject(208-261)globalObject(208-208)src/bun.js/bindings/ZigGlobalObject.h (1)
globalObject(127-127)
src/bun.js/bindings/bindings.cpp (3)
src/bun.js/bindings/ZigGlobalObject.h (1)
globalObject(127-127)src/bun.js/bindings/ZigGlobalObject.cpp (8)
create(889-894)create(889-889)create(896-901)create(896-896)create(903-908)create(903-903)create(910-915)create(910-910)src/bun.js/bindings/AsyncContextFrame.cpp (4)
create(15-22)create(15-15)create(24-34)create(24-24)
src/bun.js/bindings/ZigGlobalObject.cpp (2)
src/bun.js/bindings/BunDebugger.cpp (4)
globalObject(48-51)globalObject(48-48)globalObject(195-198)globalObject(208-261)src/bun.js/bindings/ZigGlobalObject.h (1)
globalObject(127-127)
src/bun.js/bindings/webcore/JSEventEmitterCustom.cpp (1)
src/bun.js/bindings/BunDebugger.cpp (5)
globalObject(48-51)globalObject(48-48)globalObject(195-198)globalObject(208-261)globalObject(208-208)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Format
🔇 Additional comments (26)
src/bun.js/bindings/node/crypto/node_crypto_binding.cpp (1)
200-200: Type-safety improvement:static_castis the correct choice.The change from
reinterpret_casttostatic_castimproves type safety by leveraging compile-time inheritance checking betweenJSC::JSGlobalObjectandZig::GlobalObject. This is the proper cast operator for downcasting within a class hierarchy.src/bun.js/bindings/sqlite/JSSQLStatement.cpp (1)
1601-1601: Type-safety improvements:static_castis the correct choice for polymorphic downcasts.Both changes replace
reinterpret_castwithstatic_castwhen convertingJSGlobalObject*toZig::GlobalObject*. This leverages compile-time type checking to ensure the inheritance relationship is valid, making the code safer without changing behavior.Also applies to: 1862-1862
src/bun.js/bindings/bindings.cpp (6)
588-588: static_cast change looks goodSwitching to
static_castkeeps the intent while giving us compile-time checking that we’re really treating aZig::GlobalObjectlike a derivedJSGlobalObject. No functional change, but lower risk than the previousreinterpret_cast. Nice.
1818-1819: Safe castSame note here—the new
static_castasserts the expected inheritance at compile time without altering behaviour. 👍
1835-1837: Consistent safer castGood to see the safer cast applied consistently across all helper paths. No issues spotted.
2043-2044: Safer castThis path also now benefits from the safer downcast; everything else stays unchanged.
6191-6191: Maintains invariants
queueMicrotaskalready requires the Bun global; usingstatic_castenforces that assumption cleanly.
6504-6504: Uniform cast updateConsistent application of the safer cast in the plug-in helpers—looks good.
src/bun.js/bindings/BunPlugin.cpp (1)
943-943: Safer downcast
jsFunctionBunPluginClearis only called with Bun’s global, so switching tostatic_castkeeps behaviour identical while enforcing the inheritance expectation. Looks good.src/bun.js/bindings/NapiClass.cpp (1)
110-110: static_cast from JSGlobalObject to Zig::GlobalObject is appropriate hereMatches project-wide change; no behavior change.
src/bun.js/bindings/JSBakeResponse.cpp (2)
46-47: Cast tightening LGTMstatic_castZig::GlobalObject* from JSGlobalObject is consistent and safe in this context.
303-310: Cast tightening LGTMUsing static_cast on init.global is fine; equivalent to patterns elsewhere.
src/bun.js/modules/NodeBufferModule.h (2)
140-144: Getter cast update LGTMstatic_castZig::GlobalObject* is appropriate here.
146-156: Setter cast update LGTMConsistent with getter and broader refactor.
src/bun.js/bindings/NodeFSStatBinding.cpp (1)
823-831: Cast tightening LGTMstatic_cast from getFunctionRealm(...) to Zig::GlobalObject* matches usage elsewhere and maintains behavior.
src/bun.js/bindings/BunProcess.cpp (5)
372-377: Cast tightening LGTMstatic_cast to Zig::GlobalObject* in Process_functionDlopen maintains semantics.
776-783: Cast tightening LGTMProcess_setUncaughtExceptionCaptureCallback: safe replacement.
814-821: Cast tightening LGTMProcess_functionHRTime: safe; no behavior change.
866-870: Cast tightening LGTMProcess_functionHRTimeBigInt: safe; no behavior change.
2107-2110: No functional impactThis is a commented line; nothing to do.
src/bun.js/bindings/JSMockFunction.cpp (1)
1332-1332: LGTM! Type-safe cast improvement.Replacing
reinterpret_castwithstatic_castis correct here sinceZig::GlobalObjectinherits fromJSC::JSGlobalObject. Thestatic_castprovides compile-time type checking and is the appropriate cast for downcasting within an inheritance hierarchy.src/bun.js/bindings/BakeAdditionsToGlobalObject.cpp (1)
49-49: LGTM! Consistent type-safe cast improvements.All replacements of
reinterpret_castwithstatic_castare correct sinceZig::GlobalObjectinherits fromJSC::JSGlobalObject. These changes improve type safety across the file without altering behavior.Also applies to: 56-56, 62-62, 68-68, 75-75, 87-87, 127-127
src/bun.js/modules/AbortControllerModuleModule.h (1)
17-17: LGTM! Type-safe cast improvement.Replacing
reinterpret_castwithstatic_castis correct for this downcast fromJSGlobalObject*toZig::GlobalObject*.src/bun.js/bindings/webcore/WebSocket.cpp (1)
1189-1189: LGTM! Type-safe cast improvement.The
static_castreplacement is correct for downcasting toZig::GlobalObject*when accessingJSBufferSubclassStructure().src/bun.js/bindings/ModuleLoader.cpp (1)
201-201: LGTM! Type-safe cast improvements.Both replacements of
reinterpret_castwithstatic_castare correct for downcasting toZig::GlobalObject*. The changes improve type safety without altering behavior.Also applies to: 1156-1156
src/bun.js/bindings/BunDebugger.cpp (1)
102-102: LGTM! Consistent type-safe cast improvements.All replacements of
reinterpret_castwithstatic_castare correct sinceZig::GlobalObjectinherits fromJSC::JSGlobalObject. These changes improve type safety across debugger connection handling, inspector thread operations, and message dispatching without altering any behavior.Also applies to: 132-132, 190-190, 210-210, 336-336, 352-352, 368-368
| Structure* structure = JSEventEmitter::createStructure(vm, lexicalGlobalObject, jsValue); | ||
| JSEventEmitter* instance | ||
| = JSEventEmitter::create(structure, reinterpret_cast<Zig::GlobalObject*>(lexicalGlobalObject), object.copyRef()); | ||
| = JSEventEmitter::create(structure, static_cast<Zig::GlobalObject*>(lexicalGlobalObject), object.copyRef()); | ||
| RETURN_IF_EXCEPTION(throwScope, {}); | ||
| RELEASE_AND_RETURN(throwScope, JSValue::encode(instance)); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Prefer jsCast for asserted downcast (optional)
static_cast works given runtime guarantees. Using jsCastZig::GlobalObject*(lexicalGlobalObject) adds a debug assertion without runtime cost.
- = JSEventEmitter::create(structure, static_cast<Zig::GlobalObject*>(lexicalGlobalObject), object.copyRef());
+ = JSEventEmitter::create(structure, jsCast<Zig::GlobalObject*>(lexicalGlobalObject), object.copyRef());📝 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.
| Structure* structure = JSEventEmitter::createStructure(vm, lexicalGlobalObject, jsValue); | |
| JSEventEmitter* instance | |
| = JSEventEmitter::create(structure, reinterpret_cast<Zig::GlobalObject*>(lexicalGlobalObject), object.copyRef()); | |
| = JSEventEmitter::create(structure, static_cast<Zig::GlobalObject*>(lexicalGlobalObject), object.copyRef()); | |
| RETURN_IF_EXCEPTION(throwScope, {}); | |
| RELEASE_AND_RETURN(throwScope, JSValue::encode(instance)); | |
| Structure* structure = JSEventEmitter::createStructure(vm, lexicalGlobalObject, jsValue); | |
| JSEventEmitter* instance | |
| = JSEventEmitter::create(structure, jsCast<Zig::GlobalObject*>(lexicalGlobalObject), object.copyRef()); | |
| RETURN_IF_EXCEPTION(throwScope, {}); | |
| RELEASE_AND_RETURN(throwScope, JSValue::encode(instance)); |
🤖 Prompt for AI Agents
In src/bun.js/bindings/webcore/JSEventEmitter.cpp around lines 152 to 156, the
code uses static_cast<Zig::GlobalObject*>(lexicalGlobalObject) for an asserted
downcast; replace this with jsCast<Zig::GlobalObject*>(lexicalGlobalObject) to
get a debug-time assertion without runtime cost. Update the create call to pass
jsCast<Zig::GlobalObject*>(lexicalGlobalObject) instead of static_cast, ensuring
any required header for jsCast is included if not already present.
(For internal tracking: fixes STAB-1384)