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

Mutable methods in wit resources #178

Closed
tqwewe opened this issue Mar 21, 2022 · 4 comments
Closed

Mutable methods in wit resources #178

tqwewe opened this issue Mar 21, 2022 · 4 comments

Comments

@tqwewe
Copy link

tqwewe commented Mar 21, 2022

I'm looking into using resources with the following .wit file:

resource aggregate {
  static new: function(id: string) -> aggregate
  apply-events: function(event: list<list<u8>>)
}

Which generates:

pub trait Aggregate {
    fn new(id: String,) -> wit_bindgen_rust::Handle<super::Aggregate>;
    fn apply_events(&self,event: Vec<Vec<u8>>,);
}

But using the online demo to see the generated rust code for export, the apply-events method takes &self. But what if I want to mutate self with &mut self, is this possible?

@alexcrichton
Copy link
Member

This is currently due to the assumption that components allow for being recursively entered. This means that if Rust handed out &mut T then it would be memory unsafe to recursively call the component. This means that there's no way to use &mut self and you need to use interior mutability such as Cell<T> or RefCell<T>.

I believe the most recent iteration of the component model disallows recursively entering a component, though, so a future iteration of wit-bindgen for the updated component model will likely define all methods with &mut self since you'll have exclusive access at all times.

@tqwewe
Copy link
Author

tqwewe commented Mar 21, 2022

Oh right! Thanks for all the info.
For now I'll use Cell or RefCell, and with that update I guess I can easily modify my code to use &mut self instead.

I truly appreciate the hard work you guys are doing. Feel free to close this issue if you think it's not needed, or it can be kept open as a tracker until &mut self.

@alexcrichton
Copy link
Member

Ok I'll leave this open so when we see this again I can confirm that this change was made or not.

@alexcrichton
Copy link
Member

Ok I'm going to close this in favor of #201 since this is part of the new canonical ABI updates.

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

No branches or pull requests

2 participants