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

FunctionEnvMut method for mutably borrowing both store and env #3592

Closed
rachel-bousfield opened this issue Feb 19, 2023 · 3 comments
Closed
Assignees
Labels
🎉 enhancement New feature!
Milestone

Comments

@rachel-bousfield
Copy link

Motivation

This feature would improve the ergonomics of FunctionEnvMut<T>.

In wasmer 3.x an &mut impl AsStoreMut, commonly a &mut StoreMut in practice, is needed to modify a Global.

If a FunctionEnvMut<T> carries a Global, then modification requires cloning it since FunctionEnv::as_store_mut() yields the needed StoreMut. This works fine if the FunctionEnvMut<T>'s underlying T is cheap to Clone, but for more complicated FunctionEnv's, one need discard references to produced by FunctionEnvMut::data_mut.

This is awkward, since it means designing host functions like

fn host_foo(mut env: WasmEnvMut) {
    let global = env.data().global.clone().unwrap();
    let mut store = env.as_store_mut();
    global.set(&mut store, 0);
    
    let rest_of_state = env.data_mut();
    // can't use `store` anymore due to `rest_of_state`
    drop(rest_of_state)
    
    let mut store = env.as_store_mut();
    // back to being able to modify globals
}

Proposed solution

We add a new method to FunctionEnvMut<T> that somehow returns both a StoreMut and the underlying &mut T. This would require deeper changes since these currently entail aliasing.

Alternatives

Don't require Global, Memory, and the like need a Store for access as was the case in wasmer 2.x.

We found the upgrade to wasmer 3.x to be quite difficult due to the above. Needing a Store required we re-architect our host functions, using complex types to achieve what used to be given for free.

@rachel-bousfield rachel-bousfield added the 🎉 enhancement New feature! label Feb 19, 2023
@ptitSeb ptitSeb self-assigned this Feb 21, 2023
@ptitSeb ptitSeb modified the milestones: v3.3, v3.x Feb 21, 2023
@svelterust
Copy link
Contributor

Would be really nice to have this.

@ptitSeb
Copy link
Contributor

ptitSeb commented Feb 28, 2023

The new function is now on master. Can this ticket be closed?

@Michael-F-Bryan
Copy link
Contributor

This should be fixed now. @knarkzel and @rachel-bousfield let us know if it's still an issue and we can reopen the ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎉 enhancement New feature!
Projects
None yet
Development

No branches or pull requests

4 participants