feat(ops): support f32 slice parameters and f32/f64 buffer returns
#1208
Conversation
WalkthroughThis PR adds comprehensive support for 32-bit floating-point (f32) buffers across the op dispatch system. The changes introduce new op handlers for f32 buffer operations (slice and pointer variants), extend TypedArray mappings to support Float32Array and Float64Array conversions, add f32 to the fastcall dispatch type system, and implement V8Sliceable for f32. The modifications parallel existing f64 implementations while maintaining consistent patterns across runtime, code generation, and serialization layers. Sequence DiagramsequenceDiagram
participant JS as JavaScript
participant V8 as V8 Runtime
participant OpDispatch as Op Dispatcher
participant RustOp as Rust Op Handler
participant V8Conv as V8 Conversion
rect rgba(200, 220, 255, 0.3)
note over JS,V8Conv: f32 Buffer Fast Path
JS->>V8: Call op with Float32Array
V8->>OpDispatch: Route to fastcall dispatcher
OpDispatch->>OpDispatch: Map Float32Array to f32 type
OpDispatch->>RustOp: Invoke op_buffer_slice_f32
RustOp->>RustOp: Process &[f32] buffer
RustOp-->>OpDispatch: Return result
OpDispatch->>V8Conv: Convert via typedarray!(f32)
V8Conv-->>JS: Return Float32Array
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1208 +/- ##
==========================================
- Coverage 81.43% 80.26% -1.18%
==========================================
Files 97 105 +8
Lines 23877 27439 +3562
==========================================
+ Hits 19445 22023 +2578
- Misses 4432 5416 +984 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
serde_v8/magic/v8slice.rs (1)
47-57:V8Sliceable<f32>implementation looks correct; consider adding tests to cover itThis
f32impl is a straight copy of the existingu32/f64versions, usingv8::Float32Array::new(scope, buf, byte_offset, length), so the wiring itself looks good.Given Codecov flagged this file, it would be useful to extend the existing generic tests at the bottom to exercise
V8Slice<f32>(and optionallyf64) as well, for example:@@ #[test] pub fn test_split_off() { test_split_off_generic::<u8>(); test_split_off_generic::<u32>(); + test_split_off_generic::<f32>(); } @@ #[test] pub fn test_split_to() { test_split_to_generic::<u8>(); test_split_to_generic::<u32>(); + test_split_to_generic::<f32>(); } @@ #[test] pub fn test_truncate() { test_truncate_generic::<u8>(); test_truncate_generic::<u32>(); + test_truncate_generic::<f32>(); } @@ #[test] fn test_truncate_after_split() { test_truncate_after_split_generic::<u8>(); test_truncate_after_split_generic::<u32>(); + test_truncate_after_split_generic::<f32>(); }That will hit the new impl and reduce the reported coverage gap.
You can verify coverage improves by re‑running tests with coverage enabled and checking that these new
f32tests exerciseV8Sliceable for f32.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
core/runtime/ops.rs(6 hunks)core/runtime/ops_rust_to_v8.rs(1 hunks)ops/op2/dispatch_fast.rs(4 hunks)ops/op2/dispatch_shared.rs(2 hunks)serde_v8/magic/v8slice.rs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
ops/op2/dispatch_fast.rs (1)
core/core.d.ts (1)
Float32Array(912-912)
ops/op2/dispatch_shared.rs (2)
serde_v8/magic/v8slice.rs (13)
as_ref(372-374)as_mut(381-383)len(208-213)std(149-149)std(150-150)std(161-161)std(186-186)std(211-211)std(242-242)std(259-259)std(260-260)std(392-392)to_vec(222-224)serde_v8/magic/bytestring.rs (2)
as_ref(35-37)as_mut(41-43)
core/runtime/ops_rust_to_v8.rs (1)
core/core.d.ts (2)
Float32Array(912-912)Float64Array(918-918)
core/runtime/ops.rs (1)
serde_v8/magic/v8slice.rs (12)
len(208-213)std(149-149)std(150-150)std(161-161)std(186-186)std(211-211)std(242-242)std(259-259)std(260-260)std(392-392)ptr(166-166)ptr(191-191)
🔇 Additional comments (7)
core/runtime/ops_rust_to_v8.rs (1)
388-425: Float32Array/Float64Array typedarray mappings verifiedThe new
typedarray!(f32, Float32Array);andtypedarray!(f64, Float64Array);entries properly mapVec<f32>/Box<[f32]>andVec<f64>/Box<[f64]>into the Rust‑to‑V8 pipeline, consistent with existing integer typed arrays. Confirmed:op_create_buf_f32andop_create_buf_f64are referenced incore/runtime/ops.rs(lines 670-671), so the conversions have proper consumers. No issues.ops/op2/dispatch_shared.rs (2)
140-161: f32 support inbyte_slice_to_buffermatches existing integer/f64pathsIncluding
NumericArg::f32in theSlice/Ptr/Vec/BoxSlicearms keeps the byte‑slice fast path consistent with thev8slicepath and with existingu32/f64handling; nothing here changes the pointer/null semantics. Verification confirms call sites inops/op2/dispatch_fast.rs:880reach this path forf32buffers.
88-121: Extend v8slice buffer conversions to coverf32as wellThe added
NumericArg::f32cases mirror the existingu8/u32/f64handling for all buffer types:Slice(ref and mut),Ptr(ref and mut),Vec, andBoxSlice.V8Slice<f32>flows through the same conversions without changing behavior for other types.core/runtime/ops.rs (2)
615-616: f32 buffer ops and tests verified—code is correctConfirmed:
op_buffer_slice_f32andop_buffer_ptr_f32are properly defined (lines 1558–1567, 1572–1580) with identical signatures and logic to their f64 counterparts.- Test matrix includes the f32 entry at lines 1591–1595.
- All length assertions and copy semantics match the existing u32/f64 patterns.
No issues found. The implementation is solid.
2537-2551: All code changes verified and consistent.The macro at line 2537 correctly uses type-agnostic casting (
vec![1 as _, 2 as _, 3 as _, 4 as _]), float instantiations are present at lines 2550–2551, and thereturn_buffers()test includes calls for bothf32andf64at lines 2579–2580. The implementation properly extends the helper to floats without special-case logic.ops/op2/dispatch_fast.rs (2)
180-206: All Float32Array wiring is complete and consistentVerification confirms Float32Array is properly integrated across the fastcall type system:
- Enum and quoting: Lines 224 & 266 group Float32Array with other typed arrays, yielding
Local<Value>andCType::V8Value.as_info()- Buffer mapping: Line 886 routes f32 buffers from TypedArray source to Float32Array (mirroring f64→Float64Array)
- Signature mappings: Lines 75 & 103 in signature.rs connect NumericArg::f32 to V8Arg::Float32Array
No pattern matches are missing, and all changes maintain symmetry with existing typed array implementations.
841-895: Verified and correctThe f32 match arm is properly implemented at lines 879–886 in dispatch_fast.rs, correctly mapping all four buffer types (Slice, Ptr, Vec, BoxSlice) with
BufferSource::TypedArraytoV8FastCallType::Float32Array. The opsop_buffer_slice_f32andop_buffer_ptr_f32are correctly defined in core/runtime/ops.rs, registered, and included in generated signatures with Float32Array support. The implementation matches the u32 and f64 patterns exactly.
3 participants
Depends on denoland/rusty_v8#1858.