chore: update rusty_v8 to 0.56.0 #16814
Conversation
ops/lib.rs
Outdated
| let data = b.data(); | ||
| assert!(data.is_some()); | ||
| let store = data.unwrap().cast::<u8>().as_ptr(); |
There was a problem hiding this comment.
Changes in this file are related to denoland/rusty_v8#1131
There was a problem hiding this comment.
This is related to #16161. IIUC ArrayBuffer::data will return None for all empty buffers, in which case most APIs would expect &[] or &mut [].
For FFI APIs, however, this might break things, and it might need the #[op] macro to also support Option<&[u8]>.
There was a problem hiding this comment.
Thanks for the heads up, I'm gonna merge the PR if it goes green and let Divy handle that in a follow up.
There was a problem hiding this comment.
Welp, it seems I hit that problem already in test_ffi/tests/test.js in
const emptyBuffer = new BigUint64Array(0);
console.log(Boolean(dylib.symbols.is_null_ptr(Deno.UnsafePointer.of(emptyBuffer))));
It prints false while it should print true
There was a problem hiding this comment.
@andreubotella so you're saying that all buffers should be mapped to Option<& [u8]>? In which case empty buffers would be None?
There was a problem hiding this comment.
Not really. Only the ptr_of op is probably interested in the real pointer behind the slice instead of being interested in the slice itself.
There's just no good way to (currently at least) explain this to serde_v8 since while a plain pointer would be accurate to what the op needs, it doesn't really explain the JS param type.
There was a problem hiding this comment.
My idea was to that op parameters could be either &[u8] or Option<&[u8]>, so that only ops that care about the pointer possibly being null have to deal with this.
There was a problem hiding this comment.
Thanks for comments. it's a bit unclear what to do here, I'll wait for Divy's opinion
There was a problem hiding this comment.
Option<&[u8]> interferes with undefined and null values in serde_v8. I opened a PR adding support for raw ptr in ops macro: #16826
See #16814 (comment). Allows nullable buffers in low-level ops like FFI: ```rust fn op_ffi_ptr_of<FP>( state: &mut OpState, buf: *const u8, out: &mut [u32], ) where FP: FfiPermissions + 'static { // .. } ```
Yeah, clippy screamed and me and I fixed that. |
See #16814 (comment). Allows nullable buffers in low-level ops like FFI: ```rust fn op_ffi_ptr_of<FP>( state: &mut OpState, buf: *const u8, out: &mut [u32], ) where FP: FfiPermissions + 'static { // .. } ```
Co-authored-by: Divy Srivastava <[email protected]>
No description provided.