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

Multi value #103

Merged
merged 6 commits into from Jun 24, 2019
Merged

Multi value #103

merged 6 commits into from Jun 24, 2019

Conversation

@fitzgen
Copy link
Member

fitzgen commented Jun 5, 2019

Builds on top of #102

@fitzgen

This comment has been minimized.

Copy link
Member Author

fitzgen commented Jun 5, 2019

(Note: this just enables function calls with multiple return values, it doesn't attempt to handle multi-value blocks yet, which I figure can come in follow up PRs).

@fitzgen

This comment has been minimized.

Copy link
Member Author

fitzgen commented Jun 5, 2019

Added support for multi-value blocks, which was slightly more involved since it had to mess with control frames / BlockState in the validator a bit.

@fitzgen fitzgen force-pushed the fitzgen:multi-value branch from bfa825e to c870ca1 Jun 5, 2019
@fitzgen fitzgen force-pushed the fitzgen:multi-value branch from c870ca1 to 3956c60 Jun 5, 2019
@yurydelendik

This comment has been minimized.

Copy link
Collaborator

yurydelendik commented Jun 10, 2019

It looks good. The only question have is where I can find a spec for the encoding used in the patch? There is https://webassembly.github.io/multi-value/core/binary/types.html#function-types, but it does not match the logic at the read_type_or_func_type.

@fitzgen

This comment has been minimized.

Copy link
Member Author

fitzgen commented Jun 24, 2019

See https://webassembly.github.io/multi-value/core/binary/instructions.html#binary-blocktype and the paragraph just above it:

Block types are encoded in special compressed form, by either the byte 0x40 indicating the empty type, as a single value type, or as a type index encoded as a positive signed integer.

Copy link
Collaborator

yurydelendik left a comment

Looks good with the minor changes

src/binary_reader.rs Outdated Show resolved Hide resolved
src/binary_reader.rs Outdated Show resolved Hide resolved
@fitzgen

This comment has been minimized.

Copy link
Member Author

fitzgen commented Jun 24, 2019

Pushed a new commit with feedback addressed.

@yurydelendik yurydelendik merged commit 443126a into bytecodealliance:master Jun 24, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@yurydelendik

This comment has been minimized.

Copy link
Collaborator

yurydelendik commented Jun 24, 2019

Thank you for the patch.

@fitzgen fitzgen deleted the fitzgen:multi-value branch Jun 24, 2019
@fitzgen

This comment has been minimized.

Copy link
Member Author

fitzgen commented Jun 24, 2019

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.