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

Serialize Expressions #513

Open
yurydelendik opened this issue Jun 4, 2020 · 4 comments
Open

Serialize Expressions #513

yurydelendik opened this issue Jun 4, 2020 · 4 comments

Comments

@yurydelendik
Copy link
Contributor

During migration to 0.20 -> 0.21, I found it impossible to rounds trip Expression. There is Expression::raw() but I could not find "into_raw" or write capabilities.

See requirement at https://github.com/bytecodealliance/wasmtime/blob/7d88384c0fe087cd80a3d8ae1bbc80950b2b7d91/cranelift/codegen/src/isa/unwind/systemv.rs#L25

@philipc
Copy link
Collaborator

philipc commented Jun 4, 2020

See bytecodealliance/wasmtime#1466 (comment). Let me know if the changes I showed there are unsuitable.

@yurydelendik
Copy link
Contributor Author

They are. I just landed the similar patch today. Thanks.

@philipc
Copy link
Collaborator

philipc commented Jun 5, 2020

They are unsuitable?

write capabilities are already present. This is currently internal, but I don't think there is a need for users to write individual expressions? As I mentioned above the linked comment, references within the expression need to be handled, so they need to be written at the same time as writing the DIE they reference. I don't see a way to make this serializable. Can you be more specific about what you think needs to be done to resolve this issue?

@yurydelendik
Copy link
Contributor Author

Sorry. The changes are suitable.

As I mentioned above the linked comment, references within the expression need to be handled, so they need to be written at the same time as writing the DIE they reference. I don't see a way to make this serializable.

I'm just pointing out that there is Expression::raw but not operation to unwrap that back. It is probably not a big deal now since we are using the approach you described in the proposed patch.

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