-
Notifications
You must be signed in to change notification settings - Fork 824
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 #3197 #3221
Fix #3197 #3221
Conversation
@syrusakbary wasmtime has a different architecture in this section, which is why they can just return |
We need to address this based on today's latest review (check the length, not just the descriptor minimum, on link time) |
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.
It looks great, although we forgot to update the is_memory_compatible
with the same fix as we did in the table
@syrusakbary yeah I wasn't sure if I should do that |
As long as that's done we should be good to merge |
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.
LGTM 👍
I've got a couple queries, but they're more so I can understand the decision (e.g. u32
vs usize
) or to make troubleshooting test_table_grow_3197()
easier if it ever fails down the track.
Co-authored-by: Michael Bryan <[email protected]>
@syrusakbary I have no idea why the Linux CI doesn't run, otherwise tests should pass now. |
Fixes #3197