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

Module.deserialize - accept AsEngineRef #3433

Merged
merged 4 commits into from
Dec 23, 2022

Conversation

dsherret
Copy link
Contributor

I need this in order to use this code #3378 (comment) with Module::deserialize instead.

@syrusakbary
Copy link
Member

Thanks @dsherret, the CI error seems unrelated to your changes.
But could be possible to update also the wasmer-cache crate to use as well the AsEngineRef? Thanks!

@dsherret
Copy link
Contributor Author

@syrusakbary I looked into it just now. It would require updating the Cache trait, which would be a breaking change. Is that ok?

@syrusakbary
Copy link
Member

It would require updating the Cache trait, which would be a breaking change. Is that ok?

Yes, that should be ok @dsherret. Thanks for double checking though!

@dsherret
Copy link
Contributor Author

dsherret commented Dec 22, 2022

Should be good now I think.

@syrusakbary
Copy link
Member

Looks good. We have some issue in master that prevented tests to pass.
I updated the branch but tests should be passing soon!

@syrusakbary
Copy link
Member

Still failing but it's independent on this PR

@syrusakbary syrusakbary merged commit 1cf110f into wasmerio:master Dec 23, 2022
@theduke
Copy link
Contributor

theduke commented Dec 28, 2022

@syrusakbary

Note: this breaks existing code.

Concretely, a value with type &impl AsStoreRef is not acceptect anymore.

(this is on the wasix branch)

error[E0277]: the trait bound `impl AsStoreRef: AsEngineRef` is not satisfied
   --> lib/wasi/src/bin_factory/module_cache.rs:201:39
    |
201 |                 let module = unsafe { Module::deserialize(store, &module_bytes[..]).unwrap() };
    |                                       ^^^^^^^^^^^^^^^^^^^ the trait `AsEngineRef` is not implemented for `impl AsStoreRef`
    |
note: required by a bound in `wasmer::Module::deserialize`
   --> /home/theduke/dev/github.com/wasmerio/wasmer/lib/api/src/sys/module.rs:270:23
    |
270 |         engine: &impl AsEngineRef,
    |                       ^^^^^^^^^^^ required by this bound in `wasmer::Module::deserialize`
help: consider further restricting this bound
    |
163 |         store: &impl AsStoreRef + wasmer::AsEngineRef,
    |                                 +++++++++++++++++++++
    pub fn get_compiled_module(
        &self,
        store: &impl AsStoreRef,
        data_hash: &str,
        compiler: &str,
    ) -> Option<Module> {
    ...
     Module::deserialize(store, &module_bytes[..]).
    }

@dsherret dsherret deleted the as_engine_ref_deserialize branch June 2, 2023 20:52
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