Skip to content

Conversation

@alexcrichton
Copy link
Contributor

This commit starts wasm-bindgen down the path of supporting closures. We
discussed this at the recent Rust All-Hands but I ended up needing to pretty
significantly scale back the ambitions of what closures are supported. This
commit is just the initial support and provides only a small amount of support
but will hopefully provide a good basis for future implementations.

Specifically this commit adds support for passing &Fn(...) to an imported
function
, but nothing elese. The &Fn type can have any lifetime and the JS
object is invalidated as soon as the import returns. The arguments and return
value of Fn must currently implement the WasmAbi trait, aka they can't
require any conversions like strings/types/etc.

I'd like to soon expand this to &mut FnMut as well as 'static closures that
can be passed around for a long time in JS, but for now I'm putting that off
until later. I'm not currently sure how to implement richer argument types, but
hopefully that can be figured out at some point!

Copy link
Contributor

@fitzgen fitzgen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great!

Only nitpick is that it would be great to have somewhere where the overall strategy, and how the pieces fit together, is documented.

👍

}

#[derive(Copy, Clone)]
pub enum TypeKind {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OwnershipKind? When I saw TypeKind I was expecting something like scalar vs struct vs enum vs function...

@alexcrichton
Copy link
Contributor Author

Already this PR is simply the bare-bones of supporting closures, it only supports stack closures being passed to imported functions (aka Rust closures passed to JS) whose arguments are only numbers. There's three other features I'd ideally like to support long-term

The last two are unfortunately stalled as I can't figure out a way to do them right now, but the first one I'm hoping to attach to this PR before landing

@alexcrichton
Copy link
Contributor Author

Ok! I've pushed up some more ambitious changes which support long-lived 'static closures as well. There's associated documentation/examples as well.

@fitzgen mind giving this another look? I'm specifically curious for your take on src/closure.rs, which took me way too long to figure out and I'm still not particularly happy with unfortunately.

This commit starts wasm-bindgen down the path of supporting closures. We
discussed this at the recent Rust All-Hands but I ended up needing to pretty
significantly scale back the ambitions of what closures are supported. This
commit is just the initial support and provides only a small amount of support
but will hopefully provide a good basis for future implementations.

Specifically this commit adds support for passing `&Fn(...)` to an *imported
function*, but nothing elese. The `&Fn` type can have any lifetime and the JS
object is invalidated as soon as the import returns. The arguments and return
value of `Fn` must currently implement the `WasmAbi` trait, aka they can't
require any conversions like strings/types/etc.

I'd like to soon expand this to `&mut FnMut` as well as `'static` closures that
can be passed around for a long time in JS, but for now I'm putting that off
until later. I'm not currently sure how to implement richer argument types, but
hopefully that can be figured out at some point!
Docs coming soon!
As soon as we've removed unneeded exports immediately run a gc pass to ensure
that we don't bind functions in JS that don't actually end up getting needed.
@alexcrichton alexcrichton merged commit ee93122 into master Apr 9, 2018
@alexcrichton alexcrichton deleted the closures branch April 9, 2018 21:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants