diff --git a/crates/web-sys/tests/wasm/element.js b/crates/web-sys/tests/wasm/element.js index ed51fbdbe71..3f34bc5f64e 100644 --- a/crates/web-sys/tests/wasm/element.js +++ b/crates/web-sys/tests/wasm/element.js @@ -151,6 +151,11 @@ export function new_title() { return document.createElement("title"); } +export function new_webgl_rendering_context() { + const canvas = document.createElement('canvas'); + return canvas.getContext('webgl'); +} + export function new_xpath_result() { let xmlDoc = new DOMParser().parseFromString("tomato", "application/xml"); let xpathResult = xmlDoc.evaluate("/root//value", xmlDoc, null, XPathResult.ANY_TYPE, null); diff --git a/crates/web-sys/tests/wasm/main.rs b/crates/web-sys/tests/wasm/main.rs index 8f2fd5ffd49..e3d3b48af0e 100644 --- a/crates/web-sys/tests/wasm/main.rs +++ b/crates/web-sys/tests/wasm/main.rs @@ -56,6 +56,7 @@ pub mod style_element; pub mod table_element; pub mod title_element; pub mod xpath_result; +pub mod whitelisted_immutable_slices; #[wasm_bindgen_test] fn deref_works() { diff --git a/crates/web-sys/tests/wasm/whitelisted_immutable_slices.rs b/crates/web-sys/tests/wasm/whitelisted_immutable_slices.rs new file mode 100644 index 00000000000..07804f652af --- /dev/null +++ b/crates/web-sys/tests/wasm/whitelisted_immutable_slices.rs @@ -0,0 +1,57 @@ +//! When generating our web_sys APIs we default to setting slice references that +//! get passed to JS as mutable in case they get mutated in JS. +//! +//! In certain cases we know for sure that the slice will not get mutated - for +//! example when working with the WebGlRenderingContext APIs. +//! +//! These tests ensure that whitelisted methods do indeed accept mutable slices. +//! Especially important since this whitelist is stringly typed and currently +//! maintained by hand. +//! +//! @see https://github.com/rustwasm/wasm-bindgen/issues/1005 + +use wasm_bindgen::prelude::*; +use wasm_bindgen_test::*; +use web_sys::WebGlRenderingContext; + +#[wasm_bindgen(module = "./tests/wasm/element.js")] +extern "C" { + fn new_webgl_rendering_context() -> WebGlRenderingContext; + // TODO: Add a function to create another type to test here. + // These functions come from element.js +} + +// TODO: Uncomment WebGlRenderingContext test. Every now and then we can check if this works +// in the latest geckodriver. +// +// Currently commented out because WebGl isn't working in geckodriver. +// +// It currently works in chromedriver so if you need to run this in the meantime you can +// uncomment this block and run it in using chromedriver. +// +// CHROMEDRIVER=chromedriver cargo test --manifest-path crates/web-sys/Cargo.toml --target wasm32-unknown-unknown --all-features test_webgl_rendering_context_immutable_slices + +// Ensure that our whitelisted WebGlRenderingContext methods work +//#[wasm_bindgen_test] +//fn test_webgl_rendering_context_immutable_slices() { +// let gl = new_webgl_rendering_context(); +// +// gl.vertex_attrib1fv_with_f32_array(0, &[1.]); +// gl.vertex_attrib2fv_with_f32_array(0, &[1.]); +// gl.vertex_attrib3fv_with_f32_array(0, &[1.]); +// gl.vertex_attrib4fv_with_f32_array(0, &[1.]); +// +// gl.uniform1fv_with_f32_array(None, &[1.]); +// gl.uniform2fv_with_f32_array(None, &[1.]); +// gl.uniform3fv_with_f32_array(None, &[1.]); +// gl.uniform4fv_with_f32_array(None, &[1.]); +// +// gl.uniform_matrix2fv_with_f32_array(None, false, &[1.]); +// gl.uniform_matrix3fv_with_f32_array(None, false, &[1.]); +// gl.uniform_matrix4fv_with_f32_array(None, false, &[1.]); +//} + +// TODO: +//#[wasm_bindgen_test] +//fn test_another_types_immutable_slices_here() { +//} diff --git a/crates/webidl-tests/lib.rs b/crates/webidl-tests/lib.rs index 44425d9c15f..ef346239a83 100644 --- a/crates/webidl-tests/lib.rs +++ b/crates/webidl-tests/lib.rs @@ -1 +1 @@ -// intentionally left blank +// Intentionally left blank in order for Cargo to work diff --git a/crates/webidl/src/first_pass.rs b/crates/webidl/src/first_pass.rs index 79b5a2da75c..02ab48e07a1 100644 --- a/crates/webidl/src/first_pass.rs +++ b/crates/webidl/src/first_pass.rs @@ -37,6 +37,7 @@ pub(crate) struct FirstPassRecord<'src> { pub(crate) dictionaries: BTreeMap<&'src str, DictionaryData<'src>>, pub(crate) callbacks: BTreeSet<&'src str>, pub(crate) callback_interfaces: BTreeMap<&'src str, CallbackInterfaceData<'src>>, + pub(crate) immutable_f32_whitelist: BTreeSet<&'static str> } /// We need to collect interface data during the first pass, to be used later. @@ -83,6 +84,9 @@ pub(crate) struct CallbackInterfaceData<'src> { #[derive(PartialEq, Eq, PartialOrd, Ord, Debug, Clone, Copy)] pub(crate) enum OperationId<'src> { Constructor(IgnoreTraits<&'src str>), + /// The name of a function in crates/web-sys/webidls/enabled/*.webidl + /// + /// ex: Operation(Some("vertexAttrib1fv")) Operation(Option<&'src str>), IndexingGetter, IndexingSetter, diff --git a/crates/webidl/src/idl_type.rs b/crates/webidl/src/idl_type.rs index d2dd3e8b234..ca9fcb58223 100644 --- a/crates/webidl/src/idl_type.rs +++ b/crates/webidl/src/idl_type.rs @@ -41,7 +41,10 @@ pub(crate) enum IdlType<'a> { Uint16Array, Int32Array, Uint32Array, - Float32Array, + Float32Array { + /// Whether or not the generated web-sys function should use an immutable slice + immutable: bool + }, Float64Array, ArrayBufferView, BufferSource, @@ -324,6 +327,15 @@ impl<'a> ToIdlType<'a> for Identifier<'a> { } } +// We default to Float32Array's being mutable, but in certain cases where we're certain that +// slices won't get mutated on the JS side (such as the WebGL APIs) we might, later in the flow, +// instead use the immutable version. +impl<'a> ToIdlType<'a> for term::Float32Array { + fn to_idl_type(&self, _record: &FirstPassRecord<'a>) -> IdlType<'a> { + IdlType::Float32Array {immutable: false} + } +} + macro_rules! terms_to_idl_type { ($($t:tt => $r:tt)*) => ($( impl<'a> ToIdlType<'a> for term::$t { @@ -331,7 +343,7 @@ macro_rules! terms_to_idl_type { IdlType::$r } } - )*) + )*); } terms_to_idl_type! { @@ -358,7 +370,6 @@ terms_to_idl_type! { Uint16Array => Uint16Array Uint32Array => Uint32Array Uint8ClampedArray => Uint8ClampedArray - Float32Array => Float32Array Float64Array => Float64Array ArrayBufferView => ArrayBufferView BufferSource => BufferSource @@ -396,7 +407,7 @@ impl<'a> IdlType<'a> { IdlType::Uint16Array => dst.push_str("u16_array"), IdlType::Int32Array => dst.push_str("i32_array"), IdlType::Uint32Array => dst.push_str("u32_array"), - IdlType::Float32Array => dst.push_str("f32_array"), + IdlType::Float32Array { .. } => dst.push_str("f32_array"), IdlType::Float64Array => dst.push_str("f64_array"), IdlType::ArrayBufferView => dst.push_str("array_buffer_view"), IdlType::BufferSource => dst.push_str("buffer_source"), @@ -501,16 +512,16 @@ impl<'a> IdlType<'a> { IdlType::ArrayBuffer => js_sys("ArrayBuffer"), IdlType::DataView => None, - IdlType::Int8Array => Some(array("i8", pos)), - IdlType::Uint8Array => Some(array("u8", pos)), - IdlType::Uint8ArrayMut => Some(array("u8", pos)), - IdlType::Uint8ClampedArray => Some(clamped(array("u8", pos))), - IdlType::Int16Array => Some(array("i16", pos)), - IdlType::Uint16Array => Some(array("u16", pos)), - IdlType::Int32Array => Some(array("i32", pos)), - IdlType::Uint32Array => Some(array("u32", pos)), - IdlType::Float32Array => Some(array("f32", pos)), - IdlType::Float64Array => Some(array("f64", pos)), + IdlType::Int8Array => Some(array("i8", pos, false)), + IdlType::Uint8Array => Some(array("u8", pos, false)), + IdlType::Uint8ArrayMut => Some(array("u8", pos, false)), + IdlType::Uint8ClampedArray => Some(clamped(array("u8", pos, false))), + IdlType::Int16Array => Some(array("i16", pos, false)), + IdlType::Uint16Array => Some(array("u16", pos, false)), + IdlType::Int32Array => Some(array("i32", pos, false)), + IdlType::Uint32Array => Some(array("u32", pos, false)), + IdlType::Float32Array {immutable} => Some(array("f32", pos, *immutable)), + IdlType::Float64Array => Some(array("f64", pos, false)), IdlType::ArrayBufferView | IdlType::BufferSource => js_sys("Object"), IdlType::Interface(name) diff --git a/crates/webidl/src/lib.rs b/crates/webidl/src/lib.rs index 2e11b1282ef..637070c0ab4 100644 --- a/crates/webidl/src/lib.rs +++ b/crates/webidl/src/lib.rs @@ -82,6 +82,8 @@ fn parse(webidl_source: &str, allowed_types: Option<&[&str]>) -> Result let mut first_pass_record: FirstPassRecord = Default::default(); first_pass_record.builtin_idents = builtin_idents(); + first_pass_record.immutable_f32_whitelist = immutable_f32_whitelist(); + definitions.first_pass(&mut first_pass_record, ())?; let mut program = Default::default(); let mut submodules = Vec::new(); @@ -179,6 +181,26 @@ fn builtin_idents() -> BTreeSet { ) } +fn immutable_f32_whitelist() -> BTreeSet<&'static str> { + BTreeSet::from_iter( + vec![ + // WebGlRenderingContext + "uniform1fv", + "uniform2fv", + "uniform3fv", + "uniform4fv", + "uniformMatrix2fv", + "uniformMatrix3fv", + "uniformMatrix4fv", + "vertexAttrib1fv", + "vertexAttrib2fv", + "vertexAttrib3fv", + "vertexAttrib4fv", + // TODO: Add another type's functions here. Leave a comment header with the type name + ] + ) +} + /// Run codegen on the AST to generate rust code. fn compile_ast(mut ast: Program) -> String { // Iteratively prune all entries from the AST which reference undefined diff --git a/crates/webidl/src/util.rs b/crates/webidl/src/util.rs index 412e45d7c66..f0e896247ef 100644 --- a/crates/webidl/src/util.rs +++ b/crates/webidl/src/util.rs @@ -67,13 +67,13 @@ pub fn mdn_doc(class: &str, method: Option<&str>) -> String { format!("[MDN Documentation]({})", link).into() } -// Array type is borrowed for arguments (`&[T]`) and owned for return value (`Vec`). -pub(crate) fn array(base_ty: &str, pos: TypePosition) -> syn::Type { +// Array type is borrowed for arguments (`&mut [T]` or `&[T]`) and owned for return value (`Vec`). +pub(crate) fn array(base_ty: &str, pos: TypePosition, immutable: bool) -> syn::Type { match pos { TypePosition::Argument => { shared_ref( slice_ty(ident_ty(raw_ident(base_ty))), - /*mutable =*/ true, + /*mutable =*/ !immutable, ) } TypePosition::Return => vec_ty(ident_ty(raw_ident(base_ty))), @@ -433,7 +433,10 @@ impl<'src> FirstPassRecord<'src> { ); signatures.push((signature, idl_args.clone())); } - idl_args.push(arg.ty.to_idl_type(self)); + + let mut idl_type = arg.ty.to_idl_type(self); + let idl_type = self.maybe_adjust(idl_type, id); + idl_args.push(idl_type); } signatures.push((signature, idl_args)); } @@ -627,6 +630,34 @@ impl<'src> FirstPassRecord<'src> { } return ret; } + + + /// When generating our web_sys APIs we default to setting slice references that + /// get passed to JS as mutable in case they get mutated in JS. + /// + /// In certain cases we know for sure that the slice will not get mutated - for + /// example when working with the WebGlRenderingContext APIs. + /// + /// Here we implement a whitelist for those cases. This whitelist is currently + /// maintained by hand. + /// + /// When adding to this whitelist add tests to crates/web-sys/tests/wasm/whitelisted_immutable_slices.rs + fn maybe_adjust<'a>(&self, mut idl_type: IdlType<'a>, id: &'a OperationId) -> IdlType<'a> { + let op = match id { + OperationId::Operation(Some(op)) => op, + _ => return idl_type + }; + + if self.immutable_f32_whitelist.contains(op) { + flag_slices_immutable(&mut idl_type) + } + + // TODO: Add other whitelisted slices here, such as F64 or u8.. + + idl_type + } + + } /// Search for an attribute by name in some webidl object's attributes. @@ -707,3 +738,22 @@ pub fn public() -> syn::Visibility { pub_token: Default::default(), }) } + +fn flag_slices_immutable(ty: &mut IdlType) { + match ty { + IdlType::Float32Array { immutable } => *immutable = true, + IdlType::Nullable(item) => flag_slices_immutable(item), + IdlType::FrozenArray(item) => flag_slices_immutable(item), + IdlType::Sequence(item) => flag_slices_immutable(item), + IdlType::Promise(item) => flag_slices_immutable(item), + IdlType::Record(item1, item2) => { + flag_slices_immutable(item1); + flag_slices_immutable(item2); + }, + IdlType::Union(list) => { + for item in list { flag_slices_immutable(item); } + } + // catch-all for everything else like Object + _ => {} + } +}