From b9c27b93a56a4dbe012d4bd3cab11b5fd69b02b8 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Tue, 25 Jun 2019 05:44:32 -0700 Subject: [PATCH] Update all non-mutable slices into Rust to use `AllocCopy` This commit migrates all non-mutable slices incoming into Rust to use the standard `AllocCopy` binding instead of using a custom `Slice` binding defined by `wasm-bindgen`. This is done by freeing the memory from Rust rather than freeing the memory from JS. We can't do this for mutable slices yet but otherwise this should be working well! --- crates/cli-support/src/js/incoming.rs | 4 +- crates/cli-support/src/webidl/incoming.rs | 113 +++++++++++----------- src/convert/slices.rs | 13 +-- 3 files changed, 65 insertions(+), 65 deletions(-) diff --git a/crates/cli-support/src/js/incoming.rs b/crates/cli-support/src/js/incoming.rs index 2fe897bf..9791b7e2 100644 --- a/crates/cli-support/src/js/incoming.rs +++ b/crates/cli-support/src/js/incoming.rs @@ -153,7 +153,7 @@ impl<'a, 'b> Incoming<'a, 'b> { // Similar to `AllocCopy`, except that we deallocate in a finally // block. - NonstandardIncoming::Slice { kind, val, mutable } => { + NonstandardIncoming::MutableSlice { kind, val } => { let (expr, ty) = self.standard_typed(val)?; assert_eq!(ty, ast::WebidlScalarType::Any.into()); let func = self.cx.pass_to_wasm_function(*kind)?; @@ -162,7 +162,7 @@ impl<'a, 'b> Incoming<'a, 'b> { .prelude(&format!("const ptr{} = {}({});", i, func, expr)); self.js .prelude(&format!("const len{} = WASM_VECTOR_LEN;", i)); - self.finally_free_slice(&expr, i, *kind, *mutable)?; + self.finally_free_slice(&expr, i, *kind, true)?; self.js.typescript_required(kind.js_ty()); return Ok(vec![format!("ptr{}", i), format!("len{}", i)]); } diff --git a/crates/cli-support/src/webidl/incoming.rs b/crates/cli-support/src/webidl/incoming.rs index 7d99f9df..c9f7359f 100644 --- a/crates/cli-support/src/webidl/incoming.rs +++ b/crates/cli-support/src/webidl/incoming.rs @@ -47,15 +47,11 @@ pub enum NonstandardIncoming { expr: Box, }, - /// JS is passing a typed slice of data into Rust. Currently this is - /// implemented with a deallocation in the JS shim, hence a custom binding. - /// - /// TODO: we should move deallocation into Rust so we can use a vanilla and - /// standard webidl binding here. - Slice { + /// A mutable slice of values going from JS to Rust, and after Rust finishes + /// the JS slice is updated with the current value of the slice. + MutableSlice { kind: VectorKind, val: ast::IncomingBindingExpression, - mutable: bool, }, /// This is either a slice or `undefined` being passed into Rust. @@ -216,52 +212,11 @@ impl IncomingBuilder { Descriptor::Option(d) => self.process_option(d)?, Descriptor::String | Descriptor::Vector(_) => { - use wasm_webidl_bindings::ast::WebidlScalarType::*; - let kind = arg.vector_kind().ok_or_else(|| { format_err!("unsupported argument type for calling Rust function from JS {:?}", arg) })? ; self.wasm.extend(&[ValType::I32; 2]); - match kind { - VectorKind::I8 => self.alloc_copy(Int8Array), - VectorKind::U8 => self.alloc_copy(Uint8Array), - VectorKind::ClampedU8 => self.alloc_copy(Uint8ClampedArray), - VectorKind::I16 => self.alloc_copy(Int16Array), - VectorKind::U16 => self.alloc_copy(Uint16Array), - VectorKind::I32 => self.alloc_copy(Int32Array), - VectorKind::U32 => self.alloc_copy(Uint32Array), - VectorKind::F32 => self.alloc_copy(Float32Array), - VectorKind::F64 => self.alloc_copy(Float64Array), - VectorKind::String => { - let expr = ast::IncomingBindingExpressionAllocUtf8Str { - alloc_func_name: self.alloc_func_name(), - expr: Box::new(self.expr_get()), - }; - self.webidl.push(DomString); - self.bindings - .push(NonstandardIncoming::Standard(expr.into())); - } - VectorKind::I64 | VectorKind::U64 => { - let signed = match kind { - VectorKind::I64 => true, - _ => false, - }; - self.bindings.push(NonstandardIncoming::AllocCopyInt64 { - alloc_func_name: self.alloc_func_name(), - expr: Box::new(self.expr_get()), - signed, - }); - self.webidl.push(Any); - } - VectorKind::Anyref => { - self.bindings - .push(NonstandardIncoming::AllocCopyAnyrefArray { - alloc_func_name: self.alloc_func_name(), - expr: Box::new(self.expr_get()), - }); - self.webidl.push(Any); - } - } + self.alloc_copy_kind(kind) } // Can't be passed from JS to Rust yet @@ -309,12 +264,15 @@ impl IncomingBuilder { ) })?; self.wasm.extend(&[ValType::I32; 2]); - self.bindings.push(NonstandardIncoming::Slice { - kind, - val: self.expr_get(), - mutable, - }); - self.webidl.push(ast::WebidlScalarType::Any); + if mutable { + self.bindings.push(NonstandardIncoming::MutableSlice { + kind, + val: self.expr_get(), + }); + self.webidl.push(ast::WebidlScalarType::Any); + } else { + self.alloc_copy_kind(kind) + } } _ => bail!( "unsupported reference argument type for calling Rust function from JS: {:?}", @@ -445,6 +403,51 @@ impl IncomingBuilder { "__wbindgen_malloc".to_string() } + fn alloc_copy_kind(&mut self, kind: VectorKind) { + use wasm_webidl_bindings::ast::WebidlScalarType::*; + + match kind { + VectorKind::I8 => self.alloc_copy(Int8Array), + VectorKind::U8 => self.alloc_copy(Uint8Array), + VectorKind::ClampedU8 => self.alloc_copy(Uint8ClampedArray), + VectorKind::I16 => self.alloc_copy(Int16Array), + VectorKind::U16 => self.alloc_copy(Uint16Array), + VectorKind::I32 => self.alloc_copy(Int32Array), + VectorKind::U32 => self.alloc_copy(Uint32Array), + VectorKind::F32 => self.alloc_copy(Float32Array), + VectorKind::F64 => self.alloc_copy(Float64Array), + VectorKind::String => { + let expr = ast::IncomingBindingExpressionAllocUtf8Str { + alloc_func_name: self.alloc_func_name(), + expr: Box::new(self.expr_get()), + }; + self.webidl.push(DomString); + self.bindings + .push(NonstandardIncoming::Standard(expr.into())); + } + VectorKind::I64 | VectorKind::U64 => { + let signed = match kind { + VectorKind::I64 => true, + _ => false, + }; + self.bindings.push(NonstandardIncoming::AllocCopyInt64 { + alloc_func_name: self.alloc_func_name(), + expr: Box::new(self.expr_get()), + signed, + }); + self.webidl.push(Any); + } + VectorKind::Anyref => { + self.bindings + .push(NonstandardIncoming::AllocCopyAnyrefArray { + alloc_func_name: self.alloc_func_name(), + expr: Box::new(self.expr_get()), + }); + self.webidl.push(Any); + } + } + } + fn alloc_copy(&mut self, webidl: ast::WebidlScalarType) { let expr = ast::IncomingBindingExpressionAllocCopy { alloc_func_name: self.alloc_func_name(), diff --git a/src/convert/slices.rs b/src/convert/slices.rs index 05733352..fd1c718a 100644 --- a/src/convert/slices.rs +++ b/src/convert/slices.rs @@ -94,14 +94,11 @@ macro_rules! vectors { impl RefFromWasmAbi for [$t] { type Abi = WasmSlice; - type Anchor = &'static [$t]; + type Anchor = Box<[$t]>; #[inline] - unsafe fn ref_from_abi(js: WasmSlice) -> &'static [$t] { - slice::from_raw_parts( - <*const $t>::from_abi(js.ptr), - js.len as usize, - ) + unsafe fn ref_from_abi(js: WasmSlice) -> Box<[$t]> { + >::from_abi(js) } } @@ -195,11 +192,11 @@ impl<'a> OptionIntoWasmAbi for &'a str { impl RefFromWasmAbi for str { type Abi = <[u8] as RefFromWasmAbi>::Abi; - type Anchor = &'static str; + type Anchor = Box; #[inline] unsafe fn ref_from_abi(js: Self::Abi) -> Self::Anchor { - str::from_utf8_unchecked(<[u8]>::ref_from_abi(js)) + mem::transmute::, Box>(>::from_abi(js)) } }