Work on review comments

crates/cli-support/src/js/rust2js.rs

@@ -548,19 +548,12 @@ impl<'a, 'b> Rust2Js<'a, 'b> {
                 unreachable!("the last argument of a variadic must be a slice");
             }
             let last_arg = self.js_arguments.len() - 1; // check implies >= 0
-            // The variadic part of the arguments could be passed as as a typed array in
-            // javascript, so we cannot just do `.concat(..)`, as this seems to convert the
-            // contents to strings (javascript is very strange).
-            write!(ret, "\
-                let args = [{}];\n\
-                {}.forEach(extra_arg => args.push(extra_arg));\n\
-                ",
-                self.js_arguments[..last_arg].join(", "),
-                self.js_arguments[last_arg]
-            ).unwrap();
             self.ret_expr.replace(
                 "JS",
-                &format!("{}.apply(this, args)", invoc),
+                &format!("{}({}, ...{})",
+                         invoc,
+                         self.js_arguments[..last_arg].join(", "),
+                         self.js_arguments[last_arg])
             )
         } else {
             self.ret_expr.replace(

crates/macro-support/src/parser.rs

@@ -376,7 +376,7 @@ impl<'a> ConvertToAst<()> for &'a mut syn::ItemStruct {
                 let getter = shared::struct_field_get(&ident, &name_str);
                 let setter = shared::struct_field_set(&ident, &name_str);
                 let opts = BindgenAttrs::find(&mut field.attrs)?;
-                assert_not_variadic(&opts)?;
+                assert_not_variadic(&opts, &field)?;
                 let comments = extract_doc_comments(&field.attrs);
                 fields.push(ast::StructField {
                     name: name.clone(),
@@ -562,7 +562,7 @@ impl ConvertToAst<BindgenAttrs> for syn::ForeignItemType {
     type Target = ast::ImportKind;
 
     fn convert(self, attrs: BindgenAttrs) -> Result<Self::Target, Diagnostic> {
-        assert_not_variadic(&attrs)?;
+        assert_not_variadic(&attrs, &self)?;
         let js_name = attrs
             .js_name()
             .map_or_else(|| self.ident.to_string(), |s| s.to_string());
@@ -586,7 +586,7 @@ impl ConvertToAst<BindgenAttrs> for syn::ForeignItemStatic {
         if self.mutability.is_some() {
             bail_span!(self.mutability, "cannot import mutable globals yet")
         }
-        assert_not_variadic(&opts)?;
+        assert_not_variadic(&opts, &self)?;
         let default_name = self.ident.to_string();
         let js_name = opts.js_name().unwrap_or(&default_name);
         let shim = format!(
@@ -624,7 +624,7 @@ impl ConvertToAst<BindgenAttrs> for syn::ItemFn {
         if self.unsafety.is_some() {
             bail_span!(self.unsafety, "can only #[wasm_bindgen] safe functions");
         }
-        assert_not_variadic(&attrs)?;
+        assert_not_variadic(&attrs, &self)?;
 
         let default_name = self.ident.to_string();
         let name = attrs.js_name().unwrap_or(&default_name);
@@ -1096,12 +1096,12 @@ fn assert_no_lifetimes(decl: &syn::FnDecl) -> Result<(), Diagnostic> {
 }
 
 /// This method always fails if the BindgenAttrs contain variadic
-fn assert_not_variadic(attrs: &BindgenAttrs) -> Result<(), Diagnostic> {
+fn assert_not_variadic(attrs: &BindgenAttrs, span: &dyn ToTokens) -> Result<(), Diagnostic> {
-    match attrs.variadic() {
+    if attrs.variadic() {
-        true => Err(Diagnostic::error("the `variadic` attribute can only be applied to imported \
+        bail_span!(span, "the `variadic` attribute can only be applied to imported \
-            (`extern`) functions")),
+            (`extern`) functions")
-        false => Ok(())
     }
+    Ok(())
 }
 
 /// Checks the function signature to ensure it finishes with a slice

tests/wasm/variadic.js

@@ -1,13 +1,44 @@
 const wasm = require('wasm-bindgen-test.js');
 const assert = require('assert');
 
-function variadic_sum() {
+function variadic_sum(...args) {
     let answer = 0;
-    for(var i=0; i<arguments.length; i++) {
+    for(var i=0; i<args.length; i++) {
-        answer += arguments[i];
+        answer += args[i];
     }
     return answer;
 }
 
-exports.variadic_sum = variadic_sum;
+exports.variadic_sum_u8 = variadic_sum;
+exports.variadic_sum_u16 = variadic_sum;
+exports.variadic_sum_u32 = variadic_sum;
+exports.variadic_sum_u64 = variadic_sum;
+exports.variadic_sum_i8 = variadic_sum;
+exports.variadic_sum_i16 = variadic_sum;
+exports.variadic_sum_i32 = variadic_sum;
+exports.variadic_sum_i64 = variadic_sum;
+exports.variadic_sum_f32 = variadic_sum;
+exports.variadic_sum_f64 = variadic_sum;
 
+function variadic_sum_opt(...args) {
+    let answer = 0;
+    for(var i=0; i<args.length; i++) {
+        if(args[i] != null) {
+            answer += args[i];
+        }
+    }
+    return answer;
+}
+
+exports.variadic_sum_opt = variadic_sum_opt;
+
+function variadic_concat(...args) {
+    let answer = "";
+    for(var i=0; i<args.length; i++) {
+        answer = `${answer}${args[i]}`;
+    }
+    return answer;
+}
+
+exports.variadic_concat_str = variadic_concat;
+exports.variadic_concat_string = variadic_concat;

tests/wasm/variadic.rs

@@ -4,13 +4,105 @@ use wasm_bindgen::prelude::*;
 #[wasm_bindgen(module = "tests/wasm/variadic.js")]
 extern {
     #[wasm_bindgen(variadic)]
-    fn variadic_sum(first: f64, second: f64, rest: &[f64]) -> f64;
+    fn variadic_sum_u8(first: u8, second: u8, rest: &[u8]) -> u8;
+    #[wasm_bindgen(variadic)]
+    fn variadic_sum_u16(first: u16, second: u16, rest: &[u16]) -> u16;
+    #[wasm_bindgen(variadic)]
+    fn variadic_sum_u32(first: u32, second: u32, rest: &[u32]) -> u32;
+    #[wasm_bindgen(variadic)]
+    fn variadic_sum_u64(first: u64, second: u64, rest: &[u64]) -> u64;
+    #[wasm_bindgen(variadic)]
+    fn variadic_sum_usize(first: usize, second: usize, rest: &[usize]) -> usize;
+    #[wasm_bindgen(variadic)]
+    fn variadic_sum_i8(first: i8, second: i8, rest: &[i8]) -> i8;
+    #[wasm_bindgen(variadic)]
+    fn variadic_sum_i16(first: i16, second: i16, rest: &[i16]) -> i16;
+    #[wasm_bindgen(variadic)]
+    fn variadic_sum_i32(first: i32, second: i32, rest: &[i32]) -> i32;
+    #[wasm_bindgen(variadic)]
+    fn variadic_sum_i64(first: i64, second: i64, rest: &[i64]) -> i64;
+    #[wasm_bindgen(variadic)]
+    fn variadic_sum_isize(first: isize, second: isize, rest: &[isize]) -> isize;
+    #[wasm_bindgen(variadic)]
+    fn variadic_sum_f32(first: f32, second: f32, rest: &[f32]) -> f32;
+    #[wasm_bindgen(variadic)]
+    fn variadic_sum_f64(first: f64, second: f64, rest: &[f64]) -> f64;
+    #[wasm_bindgen(variadic)]
+    fn variadic_sum_opt(first: Option<u32>, second: Option<u32>, rest: &[Option<u32>]) -> u32;
+    #[wasm_bindgen(variadic)]
+    fn variadic_concat_str(first: &str, second: &str, rest: &[&str]) -> String;
+    #[wasm_bindgen(variadic)]
+    fn variadic_concat_string(first: String, second: String, rest: Vec<String>) -> String;
+}
+
+// ints
+
+macro_rules! variadic_test_int {
+    ($fn_name:ident, $extern_name:ident) => {
+        #[wasm_bindgen_test]
+        fn $fn_name() {
+            assert_eq!($extern_name(1, 2, &[]), 3);
+            assert_eq!($extern_name(1, 2, &[3]), 6);
+            assert_eq!($extern_name(1, 2, &[3, 4]), 10);
+        }
+    }
+}
+
+// The <int>64 tests throw js `Cannot mix BigInt and other types, use explicit conversions`
+variadic_test_int!(variadic_simple_u8, variadic_sum_u8);
+variadic_test_int!(variadic_simple_u16, variadic_sum_u16);
+variadic_test_int!(variadic_simple_u32, variadic_sum_u32);
+//variadic_test_int!(variadic_simple_u64, variadic_sum_u64);
+variadic_test_int!(variadic_simple_usize, variadic_sum_usize);
+variadic_test_int!(variadic_simple_i8, variadic_sum_i8);
+variadic_test_int!(variadic_simple_i16, variadic_sum_i16);
+variadic_test_int!(variadic_simple_i32, variadic_sum_i32);
+//variadic_test_int!(variadic_simple_i64, variadic_sum_i64);
+variadic_test_int!(variadic_simple_isize, variadic_sum_isize);
+
+// floats
+
+macro_rules! variadic_test_float {
+    ($fn_name:ident, $extern_name:ident) => {
+        #[wasm_bindgen_test]
+        fn $fn_name() {
+            assert_eq!($extern_name(1., 2., &[]), 3.);
+            assert_eq!($extern_name(1., 2., &[3.]), 6.);
+            assert_eq!($extern_name(1., 2., &[3., 4.]), 10.);
+        }
+    }
+}
+
+variadic_test_float!(variadic_simple_f32, variadic_sum_f32);
+variadic_test_float!(variadic_simple_f64, variadic_sum_f64);
+
+// strings
+
+// `the trait `wasm_bindgen::convert::IntoWasmAbi` is not implemented for `&[&str]`
+#[wasm_bindgen_test]
+fn variadic_simple_str() {
+    assert_eq!(variadic_concat_str("a ", "test", &[]), "a test");
+    assert_eq!(variadic_concat_str("a", "nother ", &["test"]), "another test");
+    assert_eq!(variadic_concat_str("yet ", "a", &["nother ", "test"]), "yet another test");
 }
 
 #[wasm_bindgen_test]
-fn variadic_simple() {
+fn variadic_simple_string() {
-    assert_eq!(variadic_sum(1., 2., &[]), 3.);
+    assert_eq!(variadic_concat_string("a ".into(), "test".into(), vec![]), "a test");
-    assert_eq!(variadic_sum(1., 2., &[3.]), 6.);
+    assert_eq!(variadic_concat_string("a".into(), "nother ".into(), vec!["test".into()]),
-    assert_eq!(variadic_sum(1., 2., &[3., 4.]), 10.);
+               "another test");
+    assert_eq!(variadic_concat_string("yet ".into(),
+                                      "a".into(),
+                                      vec!["nother ".into(), "test".into()]),
+               "yet another test");
+}
+
+// options
+
+#[wasm_bindgen_test]
+fn variadic_simple_opt() {
+    assert_eq!(variadic_sum_opt(Some(1), None, &[]), 1);
+    assert_eq!(variadic_sum_opt(Some(1), None, &[Some(2)]), 3);
+    assert_eq!(variadic_sum_opt(Some(1), None, &[None, Some(2)]), 3);
 }