Skip to content

Commit

Permalink
Merge pull request #1208 from alexcrichton/expand-recursive
Browse files Browse the repository at this point in the history
Fix conditional #[wasm_bindgen] in impls
  • Loading branch information
alexcrichton authored Jan 28, 2019
2 parents c56dff8 + c35d6f4 commit 3e52802
Show file tree
Hide file tree
Showing 10 changed files with 197 additions and 55 deletions.
68 changes: 68 additions & 0 deletions crates/macro-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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 })
}
}
117 changes: 73 additions & 44 deletions crates/macro-support/src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
}
Expand All @@ -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(())
Expand Down
13 changes: 13 additions & 0 deletions crates/macro/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
}
}
8 changes: 7 additions & 1 deletion crates/macro/ui-tests/invalid-methods.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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() {}
}
8 changes: 4 additions & 4 deletions crates/macro/ui-tests/invalid-methods.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions crates/macro/ui-tests/unused-attributes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ extern crate wasm_bindgen;

use wasm_bindgen::prelude::*;

struct A;

#[wasm_bindgen]
impl A {
#[wasm_bindgen(method)]
Expand Down
12 changes: 6 additions & 6 deletions crates/macro/ui-tests/unused-attributes.stderr
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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! {
Expand Down
5 changes: 5 additions & 0 deletions tests/wasm/classes.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,3 +143,8 @@ exports.js_renamed_export = () => {
x.foo();
x.bar(x);
};

exports.js_conditional_bindings = () => {
const x = new wasm.ConditionalBindings();
x.free();
};
17 changes: 17 additions & 0 deletions tests/wasm/classes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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();
}

0 comments on commit 3e52802

Please sign in to comment.