From 48a823c6857de336ddad9b647ec6b0ab0379df3f Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 5 May 2018 14:52:22 -0700 Subject: [PATCH] Remove slice logic of "commit to wasm" When adding support for mutable slices I was under the impression that if the wasm memory was reallocated while we were using it then we'd have to commit the changes from the original buffer back to the new buffer. What I didn't know, however, is that once the wasm memory is reallocated then all views into it are supposed to be defunkt. It looks like node 9 didn't have this implementation quite right and it appears fixed in node 10, causing the deleted test here to fail. While this commit does raise the question of whether this is the right approach to interact with slices in JS I think the answer is still "yes". The user can always initiate the copy if need be and that seems strictly better than copying 100% of the time. --- crates/cli-support/src/js/mod.rs | 54 ------------------ crates/cli-support/src/js/rust2js.rs | 3 - tests/all/slice.rs | 84 ---------------------------- 3 files changed, 141 deletions(-) diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs index 3390d8a0..f73065bb 100644 --- a/crates/cli-support/src/js/mod.rs +++ b/crates/cli-support/src/js/mod.rs @@ -1264,60 +1264,6 @@ impl<'a> Context<'a> { } } - fn expose_commit_slice_to_wasm(&mut self, ty: VectorKind) - -> Result<&'static str, Error> - { - let gen = |me: &mut Context, name: &'static str, size: usize, get: &str| { - me.global(&format!(" - function {name}(ptr, view) {{ - if (view.buffer !== wasm.memory.buffer) - {get}().set(view, ptr / {size}); - }} - ", - name = name, - size = size, - get = get, - )); - name - }; - match ty { - VectorKind::String => bail!("strings cannot be used with mutable slices"), - VectorKind::Anyref => bail!("js values cannot be used with mutable slices"), - VectorKind::I8 => { - self.expose_int8_memory(); - Ok(gen(self, "commitI8ToWasm", 1, "getInt8Memory")) - } - VectorKind::U8 => { - self.expose_uint8_memory(); - Ok(gen(self, "commitU8ToWasm", 1, "getUint8Memory")) - } - VectorKind::I16 => { - self.expose_int16_memory(); - Ok(gen(self, "commitI16ToWasm", 2, "getInt16Memory")) - } - VectorKind::U16 => { - self.expose_uint16_memory(); - Ok(gen(self, "commitU16ToWasm", 2, "getUint16Memory")) - } - VectorKind::I32 => { - self.expose_int32_memory(); - Ok(gen(self, "commitI32ToWasm", 4, "getInt32Memory")) - } - VectorKind::U32 => { - self.expose_uint32_memory(); - Ok(gen(self, "commitU32ToWasm", 4, "getUint32Memory")) - } - VectorKind::F32 => { - self.expose_f32_memory(); - Ok(gen(self, "commitF32ToWasm", 4, "getFloat32Memory")) - } - VectorKind::F64 => { - self.expose_f64_memory(); - Ok(gen(self, "commitF64ToWasm", 8, "getFloat64Memory")) - } - } - } - fn expose_get_global_argument(&mut self) -> Result<(), Error> { if !self.exposed_globals.insert("get_global_argument") { return Ok(()); diff --git a/crates/cli-support/src/js/rust2js.rs b/crates/cli-support/src/js/rust2js.rs index 874d2bf0..15296081 100644 --- a/crates/cli-support/src/js/rust2js.rs +++ b/crates/cli-support/src/js/rust2js.rs @@ -93,9 +93,6 @@ impl<'a, 'b> Rust2Js<'a, 'b> { wasm.__wbindgen_free({0}, {1} * {size});\ ", abi, abi2, size = ty.size())); self.cx.require_internal_export("__wbindgen_free")?; - } else if arg.is_mut_ref() { - let f = self.cx.expose_commit_slice_to_wasm(ty)?; - self.finally(&format!("{}({1}, v{1});", f, abi)); } self.js_arguments.push(format!("v{}", abi)); return Ok(()) diff --git a/tests/all/slice.rs b/tests/all/slice.rs index 13a59bb2..e5e3f13f 100644 --- a/tests/all/slice.rs +++ b/tests/all/slice.rs @@ -357,90 +357,6 @@ fn import_mut() { .test(); } -#[test] -fn import_mut_realloc_middle() { - project() - .file("src/lib.rs", r#" - #![feature(proc_macro, wasm_custom_section, wasm_import_module)] - - extern crate wasm_bindgen; - - use wasm_bindgen::prelude::*; - - macro_rules! doit { - ($(($rust:ident, $js:ident, $i:ident))*) => ( - $( - #[wasm_bindgen(module = "./test")] - extern { - fn $js(a: &mut [$i]); - } - - fn $rust() { - let mut buf = [ - 1 as $i, - 2 as $i, - 3 as $i, - ]; - $js(&mut buf); - assert_eq!(buf[0], 4 as $i); - assert_eq!(buf[1], 5 as $i); - assert_eq!(buf[2], 3 as $i); - } - )* - - #[wasm_bindgen] - pub fn run() { - $($rust();)* - } - - #[wasm_bindgen] - pub fn allocate() { - std::mem::forget(Vec::::with_capacity(128 * 1024)); - } - ) - } - - - doit! { - (rust_i8, js_i8, i8) - (rust_u8, js_u8, u8) - (rust_i16, js_i16, i16) - (rust_u16, js_u16, u16) - (rust_i32, js_i32, i32) - (rust_u32, js_u32, u32) - (rust_f32, js_f32, f32) - (rust_f64, js_f64, f64) - } - "#) - .file("test.ts", r#" - import * as assert from "assert"; - import * as wasm from "./out"; - - function foo(a: any) { - wasm.allocate(); - assert.strictEqual(a.length, 3); - assert.strictEqual(a[0], 1); - assert.strictEqual(a[1], 2); - a[0] = 4; - a[1] = 5; - } - - export const js_i8 = foo; - export const js_u8 = foo; - export const js_i16 = foo; - export const js_u16 = foo; - export const js_i32 = foo; - export const js_u32 = foo; - export const js_f32 = foo; - export const js_f64 = foo; - - export function test() { - wasm.run(); - } - "#) - .test(); -} - #[test] fn export_mut() { project()