Replace uint8 with varint7 in form field #662
Conversation
|
lgtm |
This needs to be variable-length.
lukewagner
force-pushed
the
varint7-type
branch
from
9f75077 to
7e8b1fd
Compare
|
Merging based on lgtm and since it doesn't actually change the binary format at this time. |
| @@ -126,7 +129,7 @@ The type section declares all function signatures that will be used in the modul | |||
| #### Type entry | |||
| | Field | Type | Description | | |||
| | ----- | ----- | ----- | | |||
| | form | `uint8` | `0x40`, indicating a function type | | |||
| | form | `varuint7` | `0x40`, indicating a function type | | |||
There was a problem hiding this comment.
Just a suggestion but could this field eventually use the same encoding as for the opcodes.
There was a problem hiding this comment.
True, that might make sense. My hope is that we can use varuint* as the go-to default for everything and that only b/c of the extreme prevalence of opcodes (assuming the opcode-with-immediates table) and the consequent win of doubling the number of 1-byte opcodes do we make an exception for opcodes.
|
Not quite true that it doesn't change the binary format. You allow more now. ;) FWIW, I agree with making all encodings of actual numbers var[u]ints, but I'm less convinced that it is necessary for non-numbers. Opcodes are just discrete constants, and the opcode space for types in particular is super-sparse. Not sure why we need to allow multi-byte encodings for them before we have to. |
* Merge pull request #648 from WebAssembly/current_memory Add current_memory operator * Reorder section size field (#639) * Prettify section names (#638) * Extensible encoding of function signatures (#640) * Prettify section names * Restructure encoding of function signatures * Revert "[Binary 11] Update the version number to 0xB." * Leave index space for growing the number of base types * Comments addressed * clarify how export/import names convert to JS strings (#569) (#573) * When embedded in the web, clarify how export/import names convert to JS strings (#569) * Fixes suggested by @jf * Address more feedback Added a link to http://monsur.hossa.in/2012/07/20/utf-8-in-javascript.html. Simplified the decoding algorithm thanks to Luke's feedback. * Access to proprietary APIs apart from HTML5 (#656) * comments * Merge pull request #641 from WebAssembly/postorder_opcodes Postorder opcodes * fix some text that seems to be in the wrong order (#670) * Clarify that br_table has a branch argument (#664) * Add explicit argument counts (#672) * Add explicit arities * Rename * Replace uint8 with varint7 in form field (#662) This needs to be variable-length.
There's still only one valid constant. I guess LEB128 allows leading zeroes, but other schemes in #601 do not. But I'd strongly disagree that variable-length schemes should be used only for numbers: it seems quite unlikely we've fully thought through all possible uses of this (or any other fixed-width-field) candidate. We're thinking today in terms of a small set of primitive and compound constructors, but it's hard to know there won't be, e.g., a user-defined set in the future, or some product-space we need to enumerate. At a very low cost, we avoid having to make this bet based on limited understanding. |
|
Not sure I follow. That's still no reason to extend the format now. Just allowing single bytes today is forward-compatible with generalising to LEB128 later, once that actually turns out to be needed. You can safely defer that generalisation until you know that you want to represent more than 128 cases, and that numbers are the best encoding. Restricting to single byte is conservative. |
|
It's more of a signal indicating that this won't always be a fixed-width field and the high bit of the byte is "reserved". |
This needs to be variable-length.