Skip to content

binary encoding: remove the function name-present flag and require a name for imports and exports. #521

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

Closed
ghost opened this issue Jan 20, 2016 · 6 comments
Milestone

Comments

@ghost
Copy link

ghost commented Jan 20, 2016

The encoding (still being imported) has separate function decl flags for the function being an import and export, and a separate name-present flag. There is only one name for each function, and for imports and exports that name would be the external name not the internal label name. It seems that all imports and exports would require an external name to be specified, and that other functions have only internal labels. So the separate name-present flag appears to be redundant and a foot-gun see WebAssembly/binaryen#124 and perhaps could just be removed and a name would be include if and only-if the function is an import or export.

@weilianglin
Copy link

Do we need a lightweight, optional "debug symbol" section in the binary which associates names with each indexed entity? Stated in TextFormat.md#debug-symbol-integration

@ghost
Copy link
Author

ghost commented Jan 21, 2016

@weilianglin Yes, but it's not clear yet how it will be integrated. The existing encodings function name just does not appear appropriate, and will not work as-is for import and exports. We could just add another function decl flag for a separate label name, but there are also local variables and branch targets to name. Also there seems support for having the import and export names inlined along with the sections near the start of the file to allow early planning when streaming in the file, but the meta information could be at the end of the file to optimize runtime latency.

@kg
Copy link
Contributor

kg commented Jan 21, 2016

Do we need a lightweight, optional "debug symbol" section in the binary which associates names with each indexed entity? Stated in TextFormat.md#debug-symbol-integration

This was assumed in past design discussions, though I don't think we had decided whether we would formally spec it until later on. It improves the usability of view-source and debugging.

@lukewagner
Copy link
Member

Coincidentally, I was just commenting on this in #522.

@sunfishcode sunfishcode added this to the MVP milestone Feb 12, 2016
@ghost
Copy link
Author

ghost commented Feb 25, 2016

With imports moving to a separate section, that leaves only the exports that seems to need a name and perhaps it is even clearer that the name-present flag can be removed and a name only included when the function is an export.

@ghost
Copy link
Author

ghost commented Mar 12, 2016

Basically addressed now. Imports still need naming.

@ghost ghost closed this as completed Mar 12, 2016
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants