Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

zig: add JSValue assertion to cppFn #15445

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 13 additions & 20 deletions src/bun.js/bindings/bindings.zig
Original file line number Diff line number Diff line change
Expand Up @@ -6703,6 +6703,15 @@ pub const EncodedJSValue = extern union {
asPtr: ?*anyopaque,
asDouble: f64,
};

// this is called a lot, only triggers in the combo of safe release mode and CI/canary
pub inline fn assertJSValue(globalThis: *JSGlobalObject, value: JSValue) JSValue {
if (bun.Environment.allow_assert and bun.Environment.is_canary) {
bun.assert((value == .zero) == globalThis.hasException());
}
return value;
}

pub const JSHostFunctionType = fn (*JSGlobalObject, *CallFrame) callconv(JSC.conv) JSValue;
pub const JSHostFunctionTypeWithCCallConvForAssertions = fn (*JSGlobalObject, *CallFrame) callconv(.C) JSValue;
pub const JSHostFunctionPtr = *const JSHostFunctionType;
Expand All @@ -6714,35 +6723,19 @@ pub fn toJSHostFunction(comptime Function: JSHostZigFunction) JSC.JSHostFunction
globalThis: *JSC.JSGlobalObject,
callframe: *JSC.CallFrame,
) callconv(JSC.conv) JSC.JSValue {
if (bun.Environment.allow_assert and bun.Environment.is_canary) {
const value = Function(globalThis, callframe) catch |err| switch (err) {
error.JSError => .zero,
error.OutOfMemory => globalThis.throwOutOfMemoryValue(),
};
bun.assert((value == .zero) == globalThis.hasException());
return value;
}
return @call(.always_inline, Function, .{ globalThis, callframe }) catch |err| switch (err) {
return assertJSValue(globalThis, @call(.always_inline, Function, .{ globalThis, callframe }) catch |err| switch (err) {
error.JSError => .zero,
error.OutOfMemory => globalThis.throwOutOfMemoryValue(),
};
});
}
}.function;
}

pub fn toJSHostValue(globalThis: *JSGlobalObject, value: error{ OutOfMemory, JSError }!JSValue) JSValue {
if (bun.Environment.allow_assert and bun.Environment.is_canary) {
const normal = value catch |err| switch (err) {
error.JSError => .zero,
error.OutOfMemory => globalThis.throwOutOfMemoryValue(),
};
bun.assert((normal == .zero) == globalThis.hasException());
return normal;
}
return value catch |err| switch (err) {
return assertJSValue(globalThis, value catch |err| switch (err) {
error.JSError => .zero,
error.OutOfMemory => globalThis.throwOutOfMemoryValue(),
};
});
}

const ParsedHostFunctionErrorSet = struct {
Expand Down
30 changes: 14 additions & 16 deletions src/bun.js/bindings/shimmer.zig
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ const std = @import("std");
const bun = @import("root").bun;
const StaticExport = @import("./static_export.zig");
const Sizes = @import("./sizes.zig");
pub const is_bindgen: bool = false;
const headers = @import("./headers.zig");

fn isNullableType(comptime Type: type) bool {
Expand Down Expand Up @@ -193,22 +192,21 @@ pub fn Shimmer(comptime _namespace: []const u8, comptime _name: []const u8, comp
}) {
log(comptime name ++ "__" ++ typeName, .{});
@setEvalBranchQuota(99999);
if (comptime is_bindgen) {
unreachable;
} else {
{
const Fn = comptime @field(headers, symbolName(typeName));
if (@typeInfo(@TypeOf(Fn)).Fn.params.len > 0)
return matchNullable(
comptime @typeInfo(@TypeOf(@field(Parent, typeName))).Fn.return_type.?,
comptime @typeInfo(@TypeOf(Fn)).Fn.return_type.?,
@call(.auto, Fn, args),
);

return matchNullable(
comptime @typeInfo(@TypeOf(@field(Parent, typeName))).Fn.return_type.?,
comptime @typeInfo(@TypeOf(Fn)).Fn.return_type.?,
Fn(),
);
const ExpectedReturnType = @typeInfo(@TypeOf(@field(Parent, typeName))).Fn.return_type.?;
const ExternReturnType = @typeInfo(@TypeOf(Fn)).Fn.return_type.?;
if (ExpectedReturnType == bun.JSC.JSValue) {
comptime bun.assert(ExpectedReturnType == ExternReturnType);
// if return type is JSValue and function accepts a js global object, add JSValue exception assertion check
// otherwise return. jsBoolean etc are the only prominent examples
if (comptime std.mem.indexOfScalar(type, bun.meta.FieldTypes(@TypeOf(args)), *bun.JSC.JSGlobalObject)) |idx| {
return bun.JSC.assertJSValue(args[idx], @call(.auto, Fn, args));
}
}
// return type may be bool, *JSObject, etc.
// TODO: replace this whole shimmer mechanism with much better bindings
return matchNullable(ExpectedReturnType, ExternReturnType, @call(.auto, Fn, args));
}
}
};
Expand Down
7 changes: 7 additions & 0 deletions src/meta.zig
Original file line number Diff line number Diff line change
Expand Up @@ -322,3 +322,10 @@ pub fn looksLikeListContainerType(comptime T: type) ?struct { list: ListContaine

return null;
}

pub fn FieldTypes(comptime T: type) []const type {
const fields = std.meta.fields(T);
var t: [fields.len]type = undefined;
for (fields, 0..) |f, i| t[i] = f.type;
return &t;
}