Skip to content
This repository was archived by the owner on Nov 3, 2021. It is now read-only.

Adjust binary format of element segments in the spec - #46

Merged
rossberg merged 2 commits into
WebAssembly:masterfrom
gahaas:elem-section
May 27, 2019
Merged

rossberg merged 2 commits into
WebAssembly:masterfrom
gahaas:elem-section

Conversation

@gahaas

@gahaas gahaas commented May 27, 2019 •

Copy link
Copy Markdown
Contributor

This PR fixes issue #36.

In #18 we decided that the binary encoding of element segments in this proposal should be the same as for the bulk memory proposal. This PR does this change in the specification document.

As far as I can tell, the spec interpreter already has the correct behavior.

@rossberg rossberg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, can you add a reference to #36, which this fixes?

Comment thread document/core/binary/modules.rst Outdated
x{:}\Btableidx~~e{:}\Bexpr~~y^\ast{:}\Bvec(\Bfuncidx)
&\Rightarrow& \{ \ETABLE~x, \EOFFSET~e, \EINIT~y^\ast \} \\
\hex{00}~~e{:}\Bexpr~~y^\ast{:}\Bvec(\Bfuncidx)
&\Rightarrow& \{ \ETABLE~0, \EOFFSET~e, \EINIT~((\REFFUNC~y)~\END)^\ast \} \\ &&|&

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the ((\REFFUNC~y)~\END)^\ast on the RHS needs to be y^\ast for now, otherwise it won't match with the current AST definition. Same below.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@rossberg
rossberg merged commit 86c7e35 into WebAssembly:master May 27, 2019
@gahaas
gahaas deleted the elem-section branch May 28, 2019 08:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants