Skip to content

Add the GetFunctionTable JS API - #2554

Merged
tlively merged 6 commits into
WebAssembly:masterfrom
COFFEETALES:getftable_js_api
Feb 20, 2020
Merged

tlively merged 6 commits into
WebAssembly:masterfrom
COFFEETALES:getftable_js_api

Conversation

@COFFEETALES

Copy link
Copy Markdown
Contributor

Adds functionality to the JS and C API for getting the ftable segments.

@tlively tlively 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.

Nice work! I have a few comments around style and design consistency, but this looks good overall.

Comment thread src/js/binaryen.js-post.js Resolved
Comment thread src/js/binaryen.js-post.js Resolved
Comment thread src/js/binaryen.js-post.js Resolved
Comment thread src/binaryen-c.h Resolved
Comment thread src/binaryen-c.h Resolved
Comment thread src/binaryen-c.h Resolved
Comment thread test/binaryen.js/kitchen-sink.js Resolved
Comment thread test/binaryen.js/kitchen-sink.js Resolved
Comment thread test/binaryen.js/kitchen-sink.js Resolved
Comment thread test/example/c-api-kitchen-sink.c Resolved
@COFFEETALES
COFFEETALES requested a review from tlively January 16, 2020 14:23
Comment thread src/js/binaryen.js-post.js Resolved
Comment thread src/js/binaryen.js-post.js Resolved
@COFFEETALES
COFFEETALES requested a review from tlively February 18, 2020 19:25
Comment thread src/js/binaryen.js-post.js Outdated
'imported': Boolean(Module['_BinaryenIsFunctionTableImported'](module)),
'segments': (function() {
var arr = [];
for (var i = 0, numSegments = Module['_BinaryenGetNumFunctionTableSegments'](module) ; i !== numSegments ; ++i) {

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.

Can you remove the spaces before the semicolons in this loop and the loops in the C and JS tests as well? Otherwise LGTM! Nice job 👍

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. Thanks a lot 👍

@tlively
tlively merged commit 42e566c into WebAssembly:master Feb 20, 2020
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

Successfully merging this pull request may close these issues.

3 participants