From 82a3ee2a67eba4ebd0716471950f81fc32e5262c Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Wed, 9 Oct 2024 16:22:39 +0200 Subject: [PATCH 1/6] Replace all `Self` identifiers in method argument and return types --- crates/macro-support/Cargo.toml | 2 +- crates/macro-support/src/parser.rs | 28 ++++++++++++++-------------- tests/wasm/inner_self.rs | 28 ++++++++++++++++++++++++++++ tests/wasm/main.rs | 1 + 4 files changed, 44 insertions(+), 15 deletions(-) create mode 100644 tests/wasm/inner_self.rs diff --git a/crates/macro-support/Cargo.toml b/crates/macro-support/Cargo.toml index 26ae4a4c5a8..43bca5de987 100644 --- a/crates/macro-support/Cargo.toml +++ b/crates/macro-support/Cargo.toml @@ -21,6 +21,6 @@ strict-macro = [] [dependencies] proc-macro2 = "1.0" quote = '1.0' -syn = { version = '2.0', features = ['visit', 'full'] } +syn = { version = '2.0', features = ['visit', 'visit-mut', 'full'] } wasm-bindgen-backend = { path = "../backend", version = "=0.2.93" } wasm-bindgen-shared = { path = "../shared", version = "=0.2.93" } diff --git a/crates/macro-support/src/parser.rs b/crates/macro-support/src/parser.rs index b1bf4e1295f..8cc051db89a 100644 --- a/crates/macro-support/src/parser.rs +++ b/crates/macro-support/src/parser.rs @@ -11,6 +11,7 @@ use quote::ToTokens; use syn::ext::IdentExt; use syn::parse::{Parse, ParseStream, Result as SynResult}; use syn::spanned::Spanned; +use syn::visit_mut::VisitMut; use syn::{ItemFn, Lit, MacroDelimiter, ReturnType}; use crate::ClassMarker; @@ -884,6 +885,15 @@ pub(crate) fn is_js_keyword(keyword: &str, skip: Option<&[&str]>) -> bool { .any(|this| *this == keyword) } +struct SelfReplace(Ident); +impl VisitMut for SelfReplace { + fn visit_ident_mut(&mut self, i: &mut proc_macro2::Ident) { + if i == "Self" { + *i = self.0.clone(); + } + } +} + /// Construct a function (and gets the self type if appropriate) for our AST from a syn function. #[allow(clippy::too_many_arguments)] fn function_from_decl( @@ -911,24 +921,14 @@ fn function_from_decl( let syn::Signature { inputs, output, .. } = sig; - let replace_self = |t: syn::Type| { + let replace_self = |mut t: syn::Type| { let self_ty = match self_ty { Some(i) => i, None => return t, }; - let path = match get_ty(&t) { - syn::Type::Path(syn::TypePath { qself: None, path }) => path.clone(), - other => return other.clone(), - }; - let new_path = if path.segments.len() == 1 && path.segments[0].ident == "Self" { - self_ty.clone().into() - } else { - path - }; - syn::Type::Path(syn::TypePath { - qself: None, - path: new_path, - }) + let mut replace = SelfReplace(self_ty.clone()); + replace.visit_type_mut(&mut t); + t }; let replace_colliding_arg = |i: &mut syn::PatType| { diff --git a/tests/wasm/inner_self.rs b/tests/wasm/inner_self.rs new file mode 100644 index 00000000000..b1dd9d68cc4 --- /dev/null +++ b/tests/wasm/inner_self.rs @@ -0,0 +1,28 @@ +//! This tests that the `wasm_bindgen` macro produces code that compiles for these use cases. +//! `cargo test --target wasm32-unknown-unknown` will not run if any of these tests breaks. +use wasm_bindgen::prelude::*; + +#[wasm_bindgen] +pub struct A; + +#[wasm_bindgen] +pub struct SelfPortrait; + +#[wasm_bindgen] +impl A { + pub fn test_only_self() -> Self { + A + } + pub fn test_option_self() -> Option { + None + } + pub fn test_nested_self() -> Result, String> { + Ok(None) + } + pub fn test_self_slice() -> Box<[Self]> { + Box::new([]) + } + pub fn test_selfish() -> Result { + Ok(A) + } +} diff --git a/tests/wasm/main.rs b/tests/wasm/main.rs index fdef95f23d7..a496aa1a3d0 100644 --- a/tests/wasm/main.rs +++ b/tests/wasm/main.rs @@ -36,6 +36,7 @@ pub mod getters_and_setters; pub mod ignore; pub mod import_class; pub mod imports; +pub mod inner_self; pub mod intrinsics; pub mod js_keywords; pub mod js_objects; From d3c3e084f2f58a5fed212c48ee79e4783fd00ded Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Wed, 9 Oct 2024 16:42:23 +0200 Subject: [PATCH 2/6] Updated changelog Co-authored-by: Oliver T --- CHANGELOG.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f8fecd34af..26dd3094a7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,9 @@ * Added `unsupported` crate to `wasm_bindgen_test(unsupported = test)` as a way of running tests on non-Wasm targets as well. [#4150](https://github.com/rustwasm/wasm-bindgen/pull/4150) +* Added support for `Self` in complex type expressions in methods. + [#4155](https://github.com/rustwasm/wasm-bindgen/pull/4155) + ### Changed * Implicitly enable reference type and multivalue transformations if the module already makes use of the corresponding target features. From 847dfab06771fd149f97c1748c6de591524afd1c Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Wed, 9 Oct 2024 16:44:01 +0200 Subject: [PATCH 3/6] raw.js is my enemy --- crates/cli/tests/reference/raw.js | 48 +++++++++++++++---------------- 1 file changed, 24 insertions(+), 24 deletions(-) diff --git a/crates/cli/tests/reference/raw.js b/crates/cli/tests/reference/raw.js index 6c6e36065ee..02dcddad344 100644 --- a/crates/cli/tests/reference/raw.js +++ b/crates/cli/tests/reference/raw.js @@ -6,26 +6,6 @@ export function __wbg_set_wasm(val) { } -const lTextDecoder = typeof TextDecoder === 'undefined' ? (0, module.require)('util').TextDecoder : TextDecoder; - -let cachedTextDecoder = new lTextDecoder('utf-8', { ignoreBOM: true, fatal: true }); - -cachedTextDecoder.decode(); - -let cachedUint8ArrayMemory0 = null; - -function getUint8ArrayMemory0() { - if (cachedUint8ArrayMemory0 === null || cachedUint8ArrayMemory0.byteLength === 0) { - cachedUint8ArrayMemory0 = new Uint8Array(wasm.memory.buffer); - } - return cachedUint8ArrayMemory0; -} - -function getStringFromWasm0(ptr, len) { - ptr = ptr >>> 0; - return cachedTextDecoder.decode(getUint8ArrayMemory0().subarray(ptr, ptr + len)); -} - const heap = new Array(128).fill(undefined); heap.push(undefined, null, true, false); @@ -45,6 +25,26 @@ function takeObject(idx) { dropObject(idx); return ret; } + +const lTextDecoder = typeof TextDecoder === 'undefined' ? (0, module.require)('util').TextDecoder : TextDecoder; + +let cachedTextDecoder = new lTextDecoder('utf-8', { ignoreBOM: true, fatal: true }); + +cachedTextDecoder.decode(); + +let cachedUint8ArrayMemory0 = null; + +function getUint8ArrayMemory0() { + if (cachedUint8ArrayMemory0 === null || cachedUint8ArrayMemory0.byteLength === 0) { + cachedUint8ArrayMemory0 = new Uint8Array(wasm.memory.buffer); + } + return cachedUint8ArrayMemory0; +} + +function getStringFromWasm0(ptr, len) { + ptr = ptr >>> 0; + return cachedTextDecoder.decode(getUint8ArrayMemory0().subarray(ptr, ptr + len)); +} /** * @param {number} test * @returns {number} @@ -109,11 +109,11 @@ export function __wbg_test2_39fe629b9aa739cf() { return addHeapObject(ret); }; -export function __wbindgen_throw(arg0, arg1) { - throw new Error(getStringFromWasm0(arg0, arg1)); -}; - export function __wbindgen_object_drop_ref(arg0) { takeObject(arg0); }; +export function __wbindgen_throw(arg0, arg1) { + throw new Error(getStringFromWasm0(arg0, arg1)); +}; + From 09988b7ebab035bfa1d04248b46dee81ad10cd37 Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Wed, 9 Oct 2024 21:56:59 +0200 Subject: [PATCH 4/6] Add a comment for context and move some code around --- crates/macro-support/src/parser.rs | 31 +++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/crates/macro-support/src/parser.rs b/crates/macro-support/src/parser.rs index 8cc051db89a..d0ddadb0398 100644 --- a/crates/macro-support/src/parser.rs +++ b/crates/macro-support/src/parser.rs @@ -885,15 +885,6 @@ pub(crate) fn is_js_keyword(keyword: &str, skip: Option<&[&str]>) -> bool { .any(|this| *this == keyword) } -struct SelfReplace(Ident); -impl VisitMut for SelfReplace { - fn visit_ident_mut(&mut self, i: &mut proc_macro2::Ident) { - if i == "Self" { - *i = self.0.clone(); - } - } -} - /// Construct a function (and gets the self type if appropriate) for our AST from a syn function. #[allow(clippy::too_many_arguments)] fn function_from_decl( @@ -921,13 +912,23 @@ fn function_from_decl( let syn::Signature { inputs, output, .. } = sig; + // A helper function to replace `Self` in the function signature of methods + // The following comment explains why this is necessary: + // https://github.com/rustwasm/wasm-bindgen/issues/3105#issuecomment-1275160744 let replace_self = |mut t: syn::Type| { - let self_ty = match self_ty { - Some(i) => i, - None => return t, - }; - let mut replace = SelfReplace(self_ty.clone()); - replace.visit_type_mut(&mut t); + if let Some(self_ty) = self_ty { + struct SelfReplace(Ident); + impl VisitMut for SelfReplace { + fn visit_ident_mut(&mut self, i: &mut proc_macro2::Ident) { + if i == "Self" { + *i = self.0.clone(); + } + } + } + + let mut replace = SelfReplace(self_ty.clone()); + replace.visit_type_mut(&mut t); + } t }; From 5329a5822d414d7981a0a00e1240c6247d892b35 Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Thu, 10 Oct 2024 10:46:25 +0200 Subject: [PATCH 5/6] Updated changelog --- CHANGELOG.md | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f661e3183e..0995c1d2fed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,15 @@ # `wasm-bindgen` Change Log -------------------------------------------------------------------------------- +## Unreleased + +### Added + +* Added support for `Self` in complex type expressions in methods. + [#4155](https://github.com/rustwasm/wasm-bindgen/pull/4155) + +-------------------------------------------------------------------------------- + ## [0.2.94](https://github.com/rustwasm/wasm-bindgen/compare/0.2.93...0.2.94) Released 2024-10-09 @@ -37,9 +46,6 @@ Released 2024-10-09 * Added additional bindings for setters from WebIDL interface attributes with applicaple parameter types of just `JsValue`. [#4156](https://github.com/rustwasm/wasm-bindgen/pull/4156) -* Added support for `Self` in complex type expressions in methods. - [#4155](https://github.com/rustwasm/wasm-bindgen/pull/4155) - ### Changed * Implicitly enable reference type and multivalue transformations if the module already makes use of the corresponding target features. From 5c790cefe0185d87268c134b07b62bacdd908f0f Mon Sep 17 00:00:00 2001 From: RunDevelopment Date: Thu, 10 Oct 2024 12:31:05 +0200 Subject: [PATCH 6/6] More comments --- crates/macro-support/src/parser.rs | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/crates/macro-support/src/parser.rs b/crates/macro-support/src/parser.rs index d0ddadb0398..66f2923350d 100644 --- a/crates/macro-support/src/parser.rs +++ b/crates/macro-support/src/parser.rs @@ -912,11 +912,16 @@ fn function_from_decl( let syn::Signature { inputs, output, .. } = sig; - // A helper function to replace `Self` in the function signature of methods + // A helper function to replace `Self` in the function signature of methods. + // E.g. `fn get(&self) -> Option` to `fn get(&self) -> Option` // The following comment explains why this is necessary: // https://github.com/rustwasm/wasm-bindgen/issues/3105#issuecomment-1275160744 let replace_self = |mut t: syn::Type| { if let Some(self_ty) = self_ty { + // This uses a visitor to replace all occurrences of `Self` with + // the actual type identifier. The visitor guarantees that we find + // all occurrences of `Self`, even if deeply nested and even if + // future Rust versions add more places where `Self` can appear. struct SelfReplace(Ident); impl VisitMut for SelfReplace { fn visit_ident_mut(&mut self, i: &mut proc_macro2::Ident) { @@ -932,6 +937,8 @@ fn function_from_decl( t }; + // A helper function to replace argument names that are JS keywords. + // E.g. this will replace `fn foo(class: u32)` to `fn foo(_class: u32)` let replace_colliding_arg = |i: &mut syn::PatType| { if let syn::Pat::Ident(ref mut i) = *i.pat { let ident = i.ident.to_string(); @@ -946,14 +953,18 @@ fn function_from_decl( .into_iter() .filter_map(|arg| match arg { syn::FnArg::Typed(mut c) => { + // typical arguments like foo: u32 replace_colliding_arg(&mut c); c.ty = Box::new(replace_self(*c.ty)); Some(c) } syn::FnArg::Receiver(r) => { + // the self argument, so self, &self, &mut self, self: Box, etc. if !allow_self { panic!("arguments cannot be `self`") } + + // write down the way in which `self` is passed for later assert!(method_self.is_none()); if r.reference.is_none() { method_self = Some(ast::MethodSelf::ByValue); @@ -962,6 +973,7 @@ fn function_from_decl( } else { method_self = Some(ast::MethodSelf::RefShared); } + None } })