-
Notifications
You must be signed in to change notification settings - Fork 313
Fix exact import in custom descriptors #2391
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
Fix exact import in custom descriptors #2391
Conversation
alexcrichton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a bit wary to match the spec exactly here in AST-representation where I would predict that idiomatically in Rust it'd be nicer to have Thing::{Func,FuncExact}(u32) as opposed to Thing::Func(ThingType { id: u32, exact: bool }). Did you try that already though and find that it was unworkable? If not, mind trying that out?
As for components/wasm-smith/wasm-encoder, where possible it's nice to keep things working but if major surgery or refactorings are required feel free to return an error and/or panic in those situations. I can take a look at any particular location in review and see if anything can't be semi-easily-handled or if it needs to do something else.
Yep, I can try that. |
bf1c05f to
aff3d8e
Compare
|
Nice that looks like it worked out well to me at least, with green CI should be good to go 👍 |
be0f680 to
1033764
Compare
1033764 to
9665eb0
Compare
alexcrichton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Judging by WebAssembly/testsuite#146 it looks like some instruction names may be getting an update too? (fine to land after this of course)
Yes, this is my next item. The newest update to the proposal will rollback some code around |
Allows exact function imports such as:
The patch adds FuncExact to external kind and uses heap type reference (instead of index) in the entity function references field. I tried to match changes to the overview explanations in the Exact Function Imports sections.
I need help with: