Fix conditional #[wasm_bindgen] in impls

Reported in rustwasm#1191 the fix requires us to get a bit creative I think. The
general gist is that a block like this:

    #[wasm_bindgen]
    impl Foo {
        pub fn foo() {}
    }

was previously expanded all in one go. Now, however, it's expanded into:

    impl Foo {
        #[__wasm_bindgen_class_marker(Foo = "Foo")]
        pub fn foo() {}
    }

    // goop generated by orginal #[wasm_bindgen]

This method of expansion takes advantage of rustc's recursive expansion
feature. It also allows us to expand `impl` blocks and allow inner items
to not be fully expanded yet, such as still having `#[cfg]` attributes
(like in the original bug report).

We use theinternal `__wasm_bindgen_class_marker` to indicate that we're
parsing an `ImplItemMethod` unconditionally, and then generation
proceeds as usual. The only final catch is that when we're expanding in
an `impl` block we have to generate tokens for the `Program`
(wasm-bindgen injected goop like the custom section) inside the body
of the function itself instead of next to it. Otherwise we'd get syntax
errors inside of impl blocks!

Closes rustwasm#1191

crates/macro-support/src/lib.rs

@@ -13,8 +13,11 @@ extern crate wasm_bindgen_shared as shared;
 
 use backend::{Diagnostic, TryToTokens};
 pub use parser::BindgenAttrs;
+use quote::ToTokens;
 use parser::MacroParse;
 use proc_macro2::TokenStream;
+use syn::parse::{Parse, ParseStream, Result as SynResult};
+use quote::TokenStreamExt;
 
 mod parser;
 
@@ -36,3 +39,68 @@ pub fn expand(attr: TokenStream, input: TokenStream) -> Result<TokenStream, Diag
 
     Ok(tokens)
 }
+
+/// Takes the parsed input from a `#[wasm_bindgen]` macro and returns the generated bindings
+pub fn expand_class_marker(attr: TokenStream, input: TokenStream) -> Result<TokenStream, Diagnostic> {
+    parser::reset_attrs_used();
+    let mut item = syn::parse2::<syn::ImplItemMethod>(input)?;
+    let opts: ClassMarker = syn::parse2(attr)?;
+
+    let mut program = backend::ast::Program::default();
+    item.macro_parse(&mut program, (&opts.class, &opts.js_class))?;
+    parser::assert_all_attrs_checked(); // same as above
+
+    // This is where things are slightly different, we are being expanded in the
+    // context of an impl so we can't inject arbitrary item-like tokens into the
+    // output stream. If we were to do that then it wouldn't parse!
+    //
+    // Instead what we want to do is to generate the tokens for `program` into
+    // the header of the function. This'll inject some no_mangle functions and
+    // statics and such, and they should all be valid in the context of the
+    // start of a function.
+    //
+    // We manually implement `ToTokens for ImplItemMethod` here, injecting our
+    // program's tokens before the actual method's inner body tokens.
+    let mut tokens = proc_macro2::TokenStream::new();
+    tokens.append_all(item.attrs.iter().filter(|attr| {
+        match attr.style {
+            syn::AttrStyle::Outer => true,
+            _ => false,
+        }
+    }));
+    item.vis.to_tokens(&mut tokens);
+    item.sig.to_tokens(&mut tokens);
+    let mut err = None;
+    item.block.brace_token.surround(&mut tokens, |tokens| {
+        if let Err(e) = program.try_to_tokens(tokens) {
+            err = Some(e);
+        }
+        tokens.append_all(item.attrs.iter().filter(|attr| {
+            match attr.style {
+                syn::AttrStyle::Inner(_) => true,
+                _ => false,
+            }
+        }));
+        tokens.append_all(&item.block.stmts);
+    });
+
+    if let Some(err) = err {
+        return Err(err)
+    }
+
+    Ok(tokens)
+}
+
+struct ClassMarker {
+    class: syn::Ident,
+    js_class: String,
+}
+
+impl Parse for ClassMarker {
+    fn parse(input: ParseStream) -> SynResult<Self> {
+        let class = input.parse::<syn::Ident>()?;
+        input.parse::<Token![=]>()?;
+        let js_class = input.parse::<syn::LitStr>()?.value();
+        Ok(ClassMarker { class, js_class })
+    }
+}

crates/macro-support/src/parser.rs

@@ -796,7 +796,7 @@ impl<'a> MacroParse<(Option<BindgenAttrs>, &'a mut TokenStream)> for syn::Item {
 }
 
 impl<'a> MacroParse<BindgenAttrs> for &'a mut syn::ItemImpl {
-    fn macro_parse(self, program: &mut ast::Program, opts: BindgenAttrs) -> Result<(), Diagnostic> {
+    fn macro_parse(self, _program: &mut ast::Program, opts: BindgenAttrs) -> Result<(), Diagnostic> {
         if self.defaultness.is_some() {
             bail_span!(
                 self.defaultness,
@@ -830,7 +830,7 @@ impl<'a> MacroParse<BindgenAttrs> for &'a mut syn::ItemImpl {
         };
         let mut errors = Vec::new();
         for item in self.items.iter_mut() {
-            if let Err(e) = (&name, item).macro_parse(program, &opts) {
+            if let Err(e) = prepare_for_impl_recursion(item, &name, &opts) {
                 errors.push(e);
             }
         }
@@ -840,77 +840,106 @@ impl<'a> MacroParse<BindgenAttrs> for &'a mut syn::ItemImpl {
     }
 }
 
-impl<'a, 'b> MacroParse<&'a BindgenAttrs> for (&'a Ident, &'b mut syn::ImplItem) {
+// Prepare for recursion into an `impl` block. Here we want to attach an
+// internal attribute, `__wasm_bindgen_class_marker`, with any metadata we need
+// to pass from the impl to the impl item. Recursive macro expansion will then
+// expand the `__wasm_bindgen_class_marker` attribute.
+//
+// Note that we currently do this because inner items may have things like cfgs
+// on them, so we want to expand the impl first, let the insides get cfg'd, and
+// then go for the rest.
+fn prepare_for_impl_recursion(
+    item: &mut syn::ImplItem,
+    class: &Ident,
+    impl_opts: &BindgenAttrs
+) -> Result<(), Diagnostic> {
+    let method = match item {
+        syn::ImplItem::Method(m) => m,
+        syn::ImplItem::Const(_) => {
+            bail_span!(
+                &*item,
+                "const definitions aren't supported with #[wasm_bindgen]"
+            );
+        }
+        syn::ImplItem::Type(_) => bail_span!(
+            &*item,
+            "type definitions in impls aren't supported with #[wasm_bindgen]"
+        ),
+        syn::ImplItem::Existential(_) => bail_span!(
+            &*item,
+            "existentials in impls aren't supported with #[wasm_bindgen]"
+        ),
+        syn::ImplItem::Macro(_) => {
+            // In theory we want to allow this, but we have no way of expanding
+            // the macro and then placing our magical attributes on the expanded
+            // functions. As a result, just disallow it for now to hopefully
+            // ward off buggy results from this macro.
+            bail_span!(&*item, "macros in impls aren't supported");
+        }
+        syn::ImplItem::Verbatim(_) => panic!("unparsed impl item?"),
+    };
+
+    let js_class = impl_opts
+        .js_class()
+        .map(|s| s.0.to_string())
+        .unwrap_or(class.to_string());
+
+    method.attrs.insert(0, syn::Attribute {
+        pound_token: Default::default(),
+        style: syn::AttrStyle::Outer,
+        bracket_token: Default::default(),
+        path: syn::Ident::new("__wasm_bindgen_class_marker", Span::call_site()).into(),
+        tts: quote::quote! { (#class = #js_class) }.into(),
+    });
+
+    Ok(())
+}
+
+impl<'a, 'b> MacroParse<(&'a Ident, &'a str)> for &'b mut syn::ImplItemMethod {
     fn macro_parse(
         self,
         program: &mut ast::Program,
-        impl_opts: &'a BindgenAttrs,
+        (class, js_class): (&'a Ident, &'a str),
     ) -> Result<(), Diagnostic> {
-        let (class, item) = self;
-        let method = match item {
-            syn::ImplItem::Method(ref mut m) => m,
-            syn::ImplItem::Const(_) => {
-                bail_span!(
-                    &*item,
-                    "const definitions aren't supported with #[wasm_bindgen]"
-                );
-            }
-            syn::ImplItem::Type(_) => bail_span!(
-                &*item,
-                "type definitions in impls aren't supported with #[wasm_bindgen]"
-            ),
-            syn::ImplItem::Existential(_) => bail_span!(
-                &*item,
-                "existentials in impls aren't supported with #[wasm_bindgen]"
-            ),
-            syn::ImplItem::Macro(_) => {
-                bail_span!(&*item, "macros in impls aren't supported");
-            }
-            syn::ImplItem::Verbatim(_) => panic!("unparsed impl item?"),
-        };
-        match method.vis {
+        match self.vis {
             syn::Visibility::Public(_) => {}
             _ => return Ok(()),
         }
-        if method.defaultness.is_some() {
+        if self.defaultness.is_some() {
             panic!("default methods are not supported");
         }
-        if method.sig.constness.is_some() {
+        if self.sig.constness.is_some() {
             bail_span!(
-                method.sig.constness,
+                self.sig.constness,
                 "can only #[wasm_bindgen] non-const functions",
             );
         }
-        if method.sig.unsafety.is_some() {
-            bail_span!(method.sig.unsafety, "can only bindgen safe functions",);
+        if self.sig.unsafety.is_some() {
+            bail_span!(self.sig.unsafety, "can only bindgen safe functions",);
         }
 
-        let opts = BindgenAttrs::find(&mut method.attrs)?;
-        let comments = extract_doc_comments(&method.attrs);
+        let opts = BindgenAttrs::find(&mut self.attrs)?;
+        let comments = extract_doc_comments(&self.attrs);
         let is_constructor = opts.constructor().is_some();
         let (function, method_self) = function_from_decl(
-            &method.sig.ident,
+            &self.sig.ident,
             &opts,
-            Box::new(method.sig.decl.clone()),
-            method.attrs.clone(),
-            method.vis.clone(),
+            Box::new(self.sig.decl.clone()),
+            self.attrs.clone(),
+            self.vis.clone(),
             true,
             Some(class),
         )?;
-        let js_class = impl_opts
-            .js_class()
-            .map(|s| s.0.to_string())
-            .unwrap_or(class.to_string());
 
         program.exports.push(ast::Export {
             rust_class: Some(class.clone()),
-            js_class: Some(js_class),
+            js_class: Some(js_class.to_string()),
             method_self,
             is_constructor,
             function,
             comments,
             start: false,
-            rust_name: method.sig.ident.clone(),
+            rust_name: self.sig.ident.clone(),
         });
         opts.check_used()?;
         Ok(())

crates/macro/src/lib.rs

@@ -19,3 +19,16 @@ pub fn wasm_bindgen(attr: TokenStream, input: TokenStream) -> TokenStream {
         Err(diagnostic) => (quote! { #diagnostic }).into(),
     }
 }
+
+#[proc_macro_attribute]
+pub fn __wasm_bindgen_class_marker(attr: TokenStream, input: TokenStream) -> TokenStream {
+    match macro_support::expand_class_marker(attr.into(), input.into()) {
+        Ok(tokens) => {
+            if cfg!(feature = "xxx_debug_only_print_generated_code") {
+                println!("{}", tokens);
+            }
+            tokens.into()
+        }
+        Err(diagnostic) => (quote! { #diagnostic }).into(),
+    }
+}

crates/macro/ui-tests/invalid-methods.rs

@@ -36,8 +36,14 @@ impl A {
     x!();
 
     // pub default fn foo() {} // TODO: compiler's pretty printer totally broken
+}
 
-
+#[wasm_bindgen]
+impl A {
     pub const fn foo() {}
+}
+
+#[wasm_bindgen]
+impl A {
     pub unsafe fn foo() {}
 }

crates/macro/ui-tests/invalid-methods.stderr

@@ -47,15 +47,15 @@ error: macros in impls aren't supported
    |     ^^^^^
 
 error: can only #[wasm_bindgen] non-const functions
-  --> $DIR/invalid-methods.rs:41:9
+  --> $DIR/invalid-methods.rs:43:9
    |
-41 |     pub const fn foo() {}
+43 |     pub const fn foo() {}
    |         ^^^^^
 
 error: can only bindgen safe functions
-  --> $DIR/invalid-methods.rs:42:9
+  --> $DIR/invalid-methods.rs:48:9
    |
-42 |     pub unsafe fn foo() {}
+48 |     pub unsafe fn foo() {}
    |         ^^^^^^
 
 error: aborting due to 10 previous errors

crates/macro/ui-tests/unused-attributes.rs

@@ -4,6 +4,8 @@ extern crate wasm_bindgen;
 
 use wasm_bindgen::prelude::*;
 
+struct A;
+
 #[wasm_bindgen]
 impl A {
     #[wasm_bindgen(method)]

crates/macro/ui-tests/unused-attributes.stderr

@@ -1,13 +1,13 @@
 error: unused #[wasm_bindgen] attribute
- --> $DIR/unused-attributes.rs:9:20
-  |
-9 |     #[wasm_bindgen(method)]
-  |                    ^^^^^^
+  --> $DIR/unused-attributes.rs:11:20
+   |
+11 |     #[wasm_bindgen(method)]
+   |                    ^^^^^^
 
 error: unused #[wasm_bindgen] attribute
-  --> $DIR/unused-attributes.rs:10:20
+  --> $DIR/unused-attributes.rs:12:20
    |
-10 |     #[wasm_bindgen(method)]
+12 |     #[wasm_bindgen(method)]
    |                    ^^^^^^
 
 error: aborting due to 2 previous errors

src/lib.rs

@@ -37,6 +37,8 @@ macro_rules! if_std {
 /// ```
 pub mod prelude {
     pub use wasm_bindgen_macro::wasm_bindgen;
+    #[doc(hidden)]
+    pub use wasm_bindgen_macro::__wasm_bindgen_class_marker;
     pub use JsValue;
 
     if_std! {

tests/wasm/classes.js

@@ -143,3 +143,8 @@ exports.js_renamed_export = () => {
     x.foo();
     x.bar(x);
 };
+
+exports.js_conditional_bindings = () => {
+    const x = new wasm.ConditionalBindings();
+    x.free();
+};

tests/wasm/classes.rs

@@ -22,6 +22,7 @@ extern "C" {
     fn js_js_rename();
     fn js_access_fields();
     fn js_renamed_export();
+    fn js_conditional_bindings();
 }
 
 #[wasm_bindgen_test]
@@ -402,3 +403,19 @@ impl RenamedExport {
 fn renamed_export() {
     js_renamed_export();
 }
+
+#[cfg_attr(target_arch = "wasm32", wasm_bindgen)]
+pub struct ConditionalBindings {
+}
+
+#[cfg_attr(target_arch = "wasm32", wasm_bindgen)]
+impl ConditionalBindings {
+    #[cfg_attr(target_arch = "wasm32", wasm_bindgen(constructor))]
+    pub fn new() -> ConditionalBindings {
+        ConditionalBindings {}
+    }
+}
+#[wasm_bindgen_test]
+fn conditional_bindings() {
+    js_conditional_bindings();
+}