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

BREAKING(ffi/unstable): always return u64 as bigint #23981

Merged
merged 1 commit into from
May 28, 2024
Merged
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
6 changes: 3 additions & 3 deletions cli/tsc/dts/lib.deno.unstable.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ declare namespace Deno {
: T extends NativeU32Enum<infer U> ? U
: T extends NativeI32Enum<infer U> ? U
: number
: T extends NativeBigIntType ? number | bigint
: T extends NativeBigIntType ? bigint
: T extends NativeBooleanType ? boolean
: T extends NativePointerType
? T extends NativeTypedPointer<infer U> ? U | null : PointerValue
Expand Down Expand Up @@ -292,7 +292,7 @@ declare namespace Deno {
: T extends NativeU32Enum<infer U> ? U
: T extends NativeI32Enum<infer U> ? U
: number
: T extends NativeBigIntType ? number | bigint
: T extends NativeBigIntType ? bigint
: T extends NativeBooleanType ? boolean
: T extends NativePointerType
? T extends NativeTypedPointer<infer U> ? U | null : PointerValue
Expand All @@ -319,7 +319,7 @@ declare namespace Deno {
: T extends NativeU32Enum<infer U> ? U
: T extends NativeI32Enum<infer U> ? U
: number
: T extends NativeBigIntType ? number | bigint
: T extends NativeBigIntType ? bigint
: T extends NativeBooleanType ? boolean
: T extends NativePointerType
? T extends NativeTypedPointer<infer U> ? U | null : PointerValue
Expand Down
17 changes: 1 addition & 16 deletions ext/ffi/00_ffi.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ const {
ObjectDefineProperty,
ObjectHasOwn,
ObjectPrototypeIsPrototypeOf,
Number,
NumberIsSafeInteger,
TypedArrayPrototypeGetBuffer,
TypedArrayPrototypeGetByteLength,
Expand Down Expand Up @@ -348,10 +347,6 @@ function isReturnedAsBigInt(type) {
type === "usize" || type === "isize";
}

function isI64(type) {
return type === "i64" || type === "isize";
}

function isStruct(type) {
return typeof type === "object" && type !== null &&
typeof type.struct === "object";
Expand Down Expand Up @@ -562,7 +557,6 @@ class DynamicLibrary {
const call = this.symbols[symbol];
const parameters = symbols[symbol].parameters;
const vi = new Int32Array(2);
const vui = new Uint32Array(TypedArrayPrototypeGetBuffer(vi));
const b = new BigInt64Array(TypedArrayPrototypeGetBuffer(vi));

const params = ArrayPrototypeJoin(
Expand All @@ -572,22 +566,13 @@ class DynamicLibrary {
// Make sure V8 has no excuse to not optimize this function.
this.symbols[symbol] = new Function(
"vi",
"vui",
"b",
"call",
"NumberIsSafeInteger",
"Number",
`return function (${params}) {
call(${params}${parameters.length > 0 ? ", " : ""}vi);
${
isI64(resultType)
? `const n1 = Number(b[0])`
: `const n1 = vui[0] + 2 ** 32 * vui[1]` // Faster path for u64
};
if (NumberIsSafeInteger(n1)) return n1;
return b[0];
}`,
)(vi, vui, b, call, NumberIsSafeInteger, Number);
)(vi, b, call);
} else if (isStructResult && !isNonBlocking) {
const call = this.symbols[symbol];
const parameters = symbols[symbol].parameters;
Expand Down
15 changes: 2 additions & 13 deletions ext/ffi/callback.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ use crate::check_unstable;
use crate::symbol::NativeType;
use crate::FfiPermissions;
use crate::ForeignFunction;
use crate::MAX_SAFE_INTEGER;
use crate::MIN_SAFE_INTEGER;
use deno_core::error::AnyError;
use deno_core::op2;
use deno_core::v8;
Expand Down Expand Up @@ -243,20 +241,11 @@ unsafe fn do_ffi_callback(
}
NativeType::I64 | NativeType::ISize => {
let result = *((*val) as *const i64);
if result > MAX_SAFE_INTEGER as i64 || result < MIN_SAFE_INTEGER as i64
{
v8::BigInt::new_from_i64(scope, result).into()
} else {
v8::Number::new(scope, result as f64).into()
}
v8::BigInt::new_from_i64(scope, result).into()
}
NativeType::U64 | NativeType::USize => {
let result = *((*val) as *const u64);
if result > MAX_SAFE_INTEGER as u64 {
v8::BigInt::new_from_u64(scope, result).into()
} else {
v8::Number::new(scope, result as f64).into()
}
v8::BigInt::new_from_u64(scope, result).into()
}
NativeType::Pointer | NativeType::Buffer | NativeType::Function => {
let result = *((*val) as *const *mut c_void);
Expand Down
43 changes: 4 additions & 39 deletions ext/ffi/ir.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.

use crate::symbol::NativeType;
use crate::MAX_SAFE_INTEGER;
use crate::MIN_SAFE_INTEGER;
use deno_core::error::type_error;
use deno_core::error::AnyError;
use deno_core::v8;
Expand Down Expand Up @@ -100,46 +98,13 @@ impl NativeValue {
v8::Integer::new_from_unsigned(scope, self.u32_value).into()
}
NativeType::I32 => v8::Integer::new(scope, self.i32_value).into(),
NativeType::U64 => {
let value = self.u64_value;
let local_value: v8::Local<v8::Value> =
if value > MAX_SAFE_INTEGER as u64 {
v8::BigInt::new_from_u64(scope, value).into()
} else {
v8::Number::new(scope, value as f64).into()
};
local_value
}
NativeType::I64 => {
let value = self.i64_value;
let local_value: v8::Local<v8::Value> =
if value > MAX_SAFE_INTEGER as i64 || value < MIN_SAFE_INTEGER as i64
{
v8::BigInt::new_from_i64(scope, self.i64_value).into()
} else {
v8::Number::new(scope, value as f64).into()
};
local_value
}
NativeType::U64 => v8::BigInt::new_from_u64(scope, self.u64_value).into(),
NativeType::I64 => v8::BigInt::new_from_i64(scope, self.i64_value).into(),
NativeType::USize => {
let value = self.usize_value;
let local_value: v8::Local<v8::Value> =
if value > MAX_SAFE_INTEGER as usize {
v8::BigInt::new_from_u64(scope, value as u64).into()
} else {
v8::Number::new(scope, value as f64).into()
};
local_value
v8::BigInt::new_from_u64(scope, self.usize_value as u64).into()
}
NativeType::ISize => {
let value = self.isize_value;
let local_value: v8::Local<v8::Value> =
if !(MIN_SAFE_INTEGER..=MAX_SAFE_INTEGER).contains(&value) {
v8::BigInt::new_from_i64(scope, self.isize_value as i64).into()
} else {
v8::Number::new(scope, value as f64).into()
};
local_value
v8::BigInt::new_from_i64(scope, self.isize_value as i64).into()
}
NativeType::F32 => v8::Number::new(scope, self.f32_value as f64).into(),
NativeType::F64 => v8::Number::new(scope, self.f64_value).into(),
Expand Down
3 changes: 0 additions & 3 deletions ext/ffi/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ const _: () = {
assert!(size_of::<*const ()>() == 8);
};

pub(crate) const MAX_SAFE_INTEGER: isize = 9007199254740991;
pub(crate) const MIN_SAFE_INTEGER: isize = -9007199254740991;

pub const UNSTABLE_FEATURE_NAME: &str = "ffi";

fn check_unstable(state: &OpState, api_name: &str) {
Expand Down
32 changes: 7 additions & 25 deletions ext/ffi/static.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

use crate::dlfcn::DynamicLibraryResource;
use crate::symbol::NativeType;
use crate::MAX_SAFE_INTEGER;
use crate::MIN_SAFE_INTEGER;
use deno_core::error::type_error;
use deno_core::error::AnyError;
use deno_core::op2;
Expand Down Expand Up @@ -90,45 +88,29 @@ pub fn op_ffi_get_static<'scope>(
NativeType::U64 => {
// SAFETY: ptr is user provided
let result = unsafe { ptr::read_unaligned(data_ptr as *const u64) };
let integer: v8::Local<v8::Value> = if result > MAX_SAFE_INTEGER as u64 {
v8::BigInt::new_from_u64(scope, result).into()
} else {
v8::Number::new(scope, result as f64).into()
};
let integer: v8::Local<v8::Value> =
v8::BigInt::new_from_u64(scope, result).into();
integer
}
NativeType::I64 => {
// SAFETY: ptr is user provided
let result = unsafe { ptr::read_unaligned(data_ptr as *const i64) };
let integer: v8::Local<v8::Value> = if result > MAX_SAFE_INTEGER as i64
|| result < MIN_SAFE_INTEGER as i64
{
v8::BigInt::new_from_i64(scope, result).into()
} else {
v8::Number::new(scope, result as f64).into()
};
let integer: v8::Local<v8::Value> =
v8::BigInt::new_from_i64(scope, result).into();
integer
}
NativeType::USize => {
// SAFETY: ptr is user provided
let result = unsafe { ptr::read_unaligned(data_ptr as *const usize) };
let integer: v8::Local<v8::Value> = if result > MAX_SAFE_INTEGER as usize
{
v8::BigInt::new_from_u64(scope, result as u64).into()
} else {
v8::Number::new(scope, result as f64).into()
};
let integer: v8::Local<v8::Value> =
v8::BigInt::new_from_u64(scope, result as u64).into();
integer
}
NativeType::ISize => {
// SAFETY: ptr is user provided
let result = unsafe { ptr::read_unaligned(data_ptr as *const isize) };
let integer: v8::Local<v8::Value> =
if !(MIN_SAFE_INTEGER..=MAX_SAFE_INTEGER).contains(&result) {
v8::BigInt::new_from_i64(scope, result as i64).into()
} else {
v8::Number::new(scope, result as f64).into()
};
v8::BigInt::new_from_i64(scope, result as i64).into();
integer
}
NativeType::F32 => {
Expand Down
4 changes: 2 additions & 2 deletions tests/ffi/tests/ffi_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ let r_1: number | bigint = result;
const result2 = remote.symbols.method17();
// @ts-expect-error: Invalid argument
result2.then((_0: string) => {});
result2.then((_1: number | bigint) => {});
result2.then((_1: bigint) => {});

const result3 = remote.symbols.method18();
// @ts-expect-error: Invalid argument
Expand Down Expand Up @@ -430,7 +430,7 @@ type __Tests__ = [
symbols: {
foo: (
...args: (number | Deno.PointerValue | null)[]
) => number | bigint;
) => bigint;
};
close(): void;
},
Expand Down
10 changes: 5 additions & 5 deletions tests/ffi/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ fn basic() {
5\n\
5\n\
579\n\
8589934590\n\
-8589934590\n\
8589934590\n\
-8589934590\n\
8589934590n\n\
-8589934590n\n\
8589934590n\n\
-8589934590n\n\
9007199254740992n\n\
9007199254740992n\n\
-9007199254740992n\n\
Expand Down Expand Up @@ -110,7 +110,7 @@ fn basic() {
Before\n\
After\n\
logCallback\n\
1 -1 2 -2 3 -3 4 -4 0.5 -0.5 1 2 3 4 5 6 7 8\n\
1 -1 2 -2 3 -3 4n -4n 0.5 -0.5 1 2 3 4 5 6 7 8\n\
u8: 8\n\
buf: [1, 2, 3, 4, 5, 6, 7, 8]\n\
logCallback\n\
Expand Down
2 changes: 1 addition & 1 deletion tests/ffi/tests/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -637,7 +637,7 @@ await dylib.symbols.call_fn_ptr_return_u8_thread_safe(returnU8Callback.pointer);

// Test statics
assertEquals(dylib.symbols.static_u32, 42);
assertEquals(dylib.symbols.static_i64, -1242464576485);
assertEquals(dylib.symbols.static_i64, -1242464576485n);
assert(
typeof dylib.symbols.static_ptr === "object"
);
Expand Down