Fixed methods with self: &Self consuming the object (#4178)

CHANGELOG.md

@@ -1,6 +1,15 @@
 # `wasm-bindgen` Change Log
 --------------------------------------------------------------------------------
 
+## Unreleased
+
+### Fixed
+
+* Fixed methods with `self: &Self` consuming the object
+  [#4178](https://github.com/rustwasm/wasm-bindgen/pull/4178)
+
+--------------------------------------------------------------------------------
+
 ## [0.2.95](https://github.com/rustwasm/wasm-bindgen/compare/0.2.94...0.2.95)
 
 Released 2024-10-10

crates/cli/tests/reference/self-type.d.ts

@@ -0,0 +1,12 @@
+/* tslint:disable */
+/* eslint-disable */
+export class Test {
+  free(): void;
+  constructor();
+  consume_self(): void;
+  ref_self(): void;
+  ref_mut_self(): void;
+  self_Self(): void;
+  self_ref_Self(): void;
+  self_ref_mut_Self(): void;
+}

crates/cli/tests/reference/self-type.js

@@ -0,0 +1,75 @@
+let wasm;
+export function __wbg_set_wasm(val) {
+    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 TestFinalization = (typeof FinalizationRegistry === 'undefined')
+    ? { register: () => {}, unregister: () => {} }
+    : new FinalizationRegistry(ptr => wasm.__wbg_test_free(ptr >>> 0, 1));
+
+export class Test {
+
+    __destroy_into_raw() {
+        const ptr = this.__wbg_ptr;
+        this.__wbg_ptr = 0;
+        TestFinalization.unregister(this);
+        return ptr;
+    }
+
+    free() {
+        const ptr = this.__destroy_into_raw();
+        wasm.__wbg_test_free(ptr, 0);
+    }
+    constructor() {
+        const ret = wasm.test_new();
+        this.__wbg_ptr = ret >>> 0;
+        TestFinalization.register(this, this.__wbg_ptr, this);
+        return this;
+    }
+    consume_self() {
+        const ptr = this.__destroy_into_raw();
+        wasm.test_consume_self(ptr);
+    }
+    ref_self() {
+        wasm.test_ref_self(this.__wbg_ptr);
+    }
+    ref_mut_self() {
+        wasm.test_ref_mut_self(this.__wbg_ptr);
+    }
+    self_Self() {
+        const ptr = this.__destroy_into_raw();
+        wasm.test_self_Self(ptr);
+    }
+    self_ref_Self() {
+        wasm.test_self_ref_Self(this.__wbg_ptr);
+    }
+    self_ref_mut_Self() {
+        wasm.test_self_ref_mut_Self(this.__wbg_ptr);
+    }
+}
+
+export function __wbindgen_throw(arg0, arg1) {
+    throw new Error(getStringFromWasm0(arg0, arg1));
+};
+

crates/cli/tests/reference/self-type.rs

@@ -0,0 +1,20 @@
+use wasm_bindgen::prelude::*;
+
+#[wasm_bindgen]
+pub struct Test;
+
+#[wasm_bindgen]
+impl Test {
+    #[wasm_bindgen(constructor)]
+    pub fn new() -> Test {
+        Test
+    }
+
+    pub fn consume_self(self) {}
+    pub fn ref_self(&self) {}
+    pub fn ref_mut_self(&mut self) {}
+
+    pub fn self_Self(self: Self) {}
+    pub fn self_ref_Self(self: &Self) {}
+    pub fn self_ref_mut_Self(self: &mut Self) {}
+}

crates/cli/tests/reference/self-type.wat

@@ -0,0 +1,25 @@
+(module $reference_test.wasm
+  (type (;0;) (func (result i32)))
+  (type (;1;) (func (param i32)))
+  (type (;2;) (func (param i32 i32)))
+  (func $test_ref_self (;0;) (type 1) (param i32))
+  (func $test_ref_mut_self (;1;) (type 1) (param i32))
+  (func $test_self_ref_Self (;2;) (type 1) (param i32))
+  (func $test_self_ref_mut_Self (;3;) (type 1) (param i32))
+  (func $__wbg_test_free (;4;) (type 2) (param i32 i32))
+  (func $test_new (;5;) (type 0) (result i32))
+  (func $test_consume_self (;6;) (type 1) (param i32))
+  (func $test_self_Self (;7;) (type 1) (param i32))
+  (memory (;0;) 17)
+  (export "memory" (memory 0))
+  (export "__wbg_test_free" (func $__wbg_test_free))
+  (export "test_new" (func $test_new))
+  (export "test_consume_self" (func $test_consume_self))
+  (export "test_ref_self" (func $test_ref_self))
+  (export "test_ref_mut_self" (func $test_ref_mut_self))
+  (export "test_self_Self" (func $test_self_Self))
+  (export "test_self_ref_Self" (func $test_self_ref_Self))
+  (export "test_self_ref_mut_Self" (func $test_self_ref_mut_Self))
+  (@custom "target_features" (after code) "\02+\0fmutable-globals+\08sign-ext")
+)
+

crates/macro-support/src/parser.rs

@@ -886,6 +886,26 @@ pub(crate) fn is_js_keyword(keyword: &str, skip: Option<&[&str]>) -> bool {
         .any(|this| *this == keyword)
 }
 
+/// Returns whether `self` is passed by reference or by value.
+fn get_self_method(r: syn::Receiver) -> ast::MethodSelf {
+    // The tricky part here is that `r` can have many forms. E.g. `self`,
+    // `&self`, `&mut self`, `self: Self`, `self: &Self`, `self: &mut Self`,
+    // `self: Box<Self>`, `self: Rc<Self>`, etc.
+    // Luckily, syn always populates the `ty` field with the type of `self`, so
+    // e.g. `&self` gets the type `&Self`. So we only have check whether the
+    // type is a reference or not.
+    match &*r.ty {
+        syn::Type::Reference(ty) => {
+            if ty.mutability.is_some() {
+                ast::MethodSelf::RefMutable
+            } else {
+                ast::MethodSelf::RefShared
+            }
+        }
+        _ => ast::MethodSelf::ByValue,
+    }
+}
+
 /// 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(
@@ -965,15 +985,10 @@ fn function_from_decl(
                     panic!("arguments cannot be `self`")
                 }
 
-                // write down the way in which `self` is passed for later
+                // We need to know *how* `self` is passed to the method (by
+                // value or by reference) to generate the correct JS shim.
                 assert!(method_self.is_none());
-                if r.reference.is_none() {
-                    method_self = Some(ast::MethodSelf::ByValue);
-                } else if r.mutability.is_some() {
-                    method_self = Some(ast::MethodSelf::RefMutable);
-                } else {
-                    method_self = Some(ast::MethodSelf::RefShared);
-                }
+                method_self = Some(get_self_method(r));
 
                 None
             }