Added conveniance function FunctionEnvMut::data_and_store_mut #3612
Conversation
|
|
||
| /// Borrows a new mutable reference of both the attached Store and host state | ||
| pub fn data_and_store_mut(&mut self) -> (&mut T, StoreMut) { | ||
| let data = self.func_env.as_mut(&mut self.store_mut) as *mut T; | ||
| // telling the borrow check to close his eyes here | ||
| // this is still relatively safe to do as func_env are | ||
| // stored in a specific vec of Store, separate from the other objects | ||
| // and not really directly accessible with the StoreMut | ||
| let data = unsafe { &mut *data }; | ||
| (data, self.store_mut.as_store_mut()) | ||
| } |
There was a problem hiding this comment.
Can we add it on the js version as well and add a #[universal_test] verifying the desired behavior? (that way, we test that it also works in js)
There was a problem hiding this comment.
It is on js (in the 2nd commit).
I'll find something for a test.
There was a problem hiding this comment.
Oh I missed the js impl. Let's just have an universal test
| // The Env is then accessed using `data()` or `data_mut()` method. | ||
| #[derive(Clone)] | ||
| struct Env { | ||
| counter: Arc<Mutex<i32>>, |
There was a problem hiding this comment.
Why this do need to be a mutex?
There was a problem hiding this comment.
But I'm not sure if I agree with it, we can discuss in a call
There was a problem hiding this comment.
It's based on the import_function_env example, it's really mostly copy/paste. But we can have a call about the examples for sure.
| /// Borrows a new mutable reference of both the attached Store and host state | ||
| pub fn data_and_store_mut(&mut self) -> (&mut T, StoreMut) { | ||
| let data = self.func_env.as_mut(&mut self.store_mut) as *mut T; | ||
| // telling the borrow check to close his eyes here |
There was a problem hiding this comment.
After reviewing the API, I don't see any other (public) way to get access to the FunctionEnv in StoreInner, so this should be safe from a public API standpoint.
Of course internally we could still violate this.
A thing I thought about yesterday : we could put the FunctionEnvs behind a RefCell to make this a bit more ssafer, but that would come with some overhead.
But otherwise I think this is a not ideal, but acceptable use of unsafe.
This allows to borrows mutable ref of both staore and data env from a FunctionEnvMut, making the writting of complex function s easier.
Should address #3592