-
Notifications
You must be signed in to change notification settings - Fork 695
Extensible encoding of function signatures #636
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -119,11 +119,15 @@ The type section declares all function signatures that will be used in the modul | |
| entries | `type_entry*` | repeated type entries as described below | | ||
|
||
#### Type entry | ||
| Field | Type | Description | | ||
| Field | Type/Value | Description | | ||
| ----- | ----- | ----- | | ||
| constructor | `0x05` | the function type constructor | | ||
| param_count | `varuint32` | the number of parameters to the function | | ||
| return_type | `value_type?` | the return type of the function, with `0` indicating no return type | | ||
| param_types | `value_type*` | the parameter types of the function | | ||
| return_count | `uint8` | the number of results from the function (0 or 1) | | ||
| return_type | `value_type?` | the result type of the function (if return_count is 1) | | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just as with sections, a string might be needed for each class of type declaration to avoid name clashes, and a size will be needed to skip unknown type declarations. Might it be prudent to just define the The 0 and 1 This is adding one byte per function that seems unnecessary and elsewhere some people have objected to this. The function signatures are ordered, in the same order as the function bodies are defined, and the count defines the number of functions which seems useful for loading a vector etc. The count now would be the number of type declarations. Is there any technical reason for not just using separate sections for future classes of type declarations? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On 5 April 2016 at 15:23, JSStats [email protected] wrote:
Unlike for unknown sections, which an implementation can ignore, there is
Note that uint8 is forward compatible with varuint32, and so is the The 0 and 1 return_count limitation might not even be a limit for some
This is adding one byte per function that seems unnecessary and elsewhere
No, this is one byte per different function type, typically a much The function signatures are ordered, in the same order as the function
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, forgot that the functions just have a signature index, and see the utility of them all sharing an index space, thanks. Just a thought, but might it be useful to fit the base types into this same index space, so that where base type indexes are currently accepted these could potentially be other types? If not then how would a local be declared to have a struct type in future etc? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, that's what the third bullet in the PR description is alluding to, and There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we might want to have a bit more index space for the primitive (and other nullary) type constructors. Or maybe that doesn't matter.... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I bet you to it on the retargetted PR. :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Understood that it's future compatible, but can we write the length to be a |
||
|
||
(Note: In the future, this section may contain other forms of type entries as well.) | ||
|
||
### Import section | ||
|
||
|
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.
Let me check my understanding of this interpretation of "constructor":
i32
and the other local types as having nullary constructors that construct the local type0x5
, the function constructor, constructs a function type given parameters/returns0x5
which told the decoder to decode a second index which was an index into this types section array. At a higher level, you're saying first which constructor and then providing the arguments to the constructor.0x6
, and the same scheme above would apply and the validation rules would require that the index following a local type0x5
/0x6
had to match on theconstructor
fieldIf that's all right, then that seems great. Maybe it would be good to update GC.md to include this plan of record.
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.
On Tue, Apr 5, 2016 at 5:49 PM, Luke Wagner [email protected]
wrote:
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 see, so if the was greater-than a fixed base, you'd subtract the base and use that as the index into the types section? (Or we could do the analogous thing with signed
varint32
and negatives.)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.
And now I remember the original question that motivated my post: given this single index space, then why does the
constructor
field need to start at any other value than0
? Function (and later struct, array) constructors are not in the same index space as the primitive types nor the elements of the types section.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.
On Tue, Apr 5, 2016 at 6:53 PM, Luke Wagner [email protected]
wrote: