Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closures should accept & for arguments #1399

Open
Pauan opened this issue Mar 26, 2019 · 3 comments
Open

Closures should accept & for arguments #1399

Pauan opened this issue Mar 26, 2019 · 3 comments
Labels
closures Adding more support for closures on the boundary help wanted We could use some help fixing this issue! more-types Adding support for more Rust types to cross the boundary

Comments

@Pauan
Copy link
Contributor

Pauan commented Mar 26, 2019

Motivation

If a Closure accepts & by argument it can use the faster stack-allocation rather than the slower heap-allocation.

Basically, this should be possible:

let f = move |e: &Foo| { ... };
let x = Closure::wrap(Box::new(f) as Box<FnMut(&Foo)>);

Additional Context

rustwasm/gloo#30 (comment)

@Pauan Pauan added the enhancement New feature or request label Mar 26, 2019
@alexcrichton alexcrichton added closures Adding more support for closures on the boundary help wanted We could use some help fixing this issue! more-types Adding support for more Rust types to cross the boundary and removed enhancement New feature or request labels Mar 26, 2019
@alexcrichton
Copy link
Contributor

Out of curiosity, since I think that a lot of code duplication would be required here is there perhaps one or two signatures which would hit 90% of use cases? For example would FnMut(&T) be enough to cover basically everything?

@Pauan
Copy link
Contributor Author

Pauan commented Apr 1, 2019

@alexcrichton It would certainly cover our particular use case (of event listeners). Though we would need FnOnce(&T) as well. So that's 2 signatures.

alexcrichton added a commit to alexcrichton/wasm-bindgen that referenced this issue Apr 1, 2019
This is work towards rustwasm#1399, although it's just for one-argument closures
where the first argument is a reference. No other closures with
references in argument position are supported yet
alexcrichton added a commit to alexcrichton/wasm-bindgen that referenced this issue Apr 1, 2019
This is work towards rustwasm#1399, although it's just for one-argument closures
where the first argument is a reference. No other closures with
references in argument position are supported yet
@alexcrichton
Copy link
Contributor

Ok I've posted an initial idea of this in #1417

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closures Adding more support for closures on the boundary help wanted We could use some help fixing this issue! more-types Adding support for more Rust types to cross the boundary
Projects
None yet
Development

No branches or pull requests

2 participants