Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add a wat2wasm subcommand #232

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Add a wat2wasm subcommand #232

wants to merge 1 commit into from

Conversation

epatrizio
Copy link
Collaborator

No description provided.

@zapashcanon zapashcanon mentioned this pull request Apr 24, 2024
Open
@epatrizio epatrizio changed the title wat2wasm command line Add wat2wasm subcommand Apr 24, 2024
@epatrizio epatrizio changed the title Add wat2wasm subcommand Add a wat2wasm subcommand Apr 24, 2024
@epatrizio epatrizio force-pushed the wat2wasm branch 7 times, most recently from 19a8d98 to 54c8be7 Compare April 26, 2024 16:00
@epatrizio epatrizio force-pushed the wat2wasm branch 2 times, most recently from f8102ca to de8e588 Compare May 10, 2024 15:18
@epatrizio epatrizio force-pushed the wat2wasm branch 6 times, most recently from cf9c448 to f1907ee Compare May 17, 2024 15:52
@epatrizio epatrizio force-pushed the wat2wasm branch 2 times, most recently from 65a5596 to 99b27f9 Compare May 22, 2024 16:02
@epatrizio epatrizio force-pushed the wat2wasm branch 5 times, most recently from 940a9b3 to ccab5b5 Compare May 29, 2024 10:23
@epatrizio epatrizio force-pushed the wat2wasm branch 2 times, most recently from e35de07 to 50c8743 Compare May 31, 2024 14:55
zapashcanon
zapashcanon reviewed Jun 3, 2024
View reviewed changes
src/binary_to_text/binary_encoder.ml Outdated
| Table_fill idx ->
write_fc buf 17;
write_indice buf idx
| _ -> assert false (* TODO *)
Copy link
Member

Choose a reason for hiding this comment

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

Could you list all the cases here ? So that we don't forget to update it when adding new stuff.

@zapashcanon
Copy link
Member

zapashcanon commented Jun 3, 2024 •
edited

Thanks a lot, we're almost there! :)

Could you rebase properly ? Also, it would be nice to update README.md, example/README.md and create example/wat2wasm/* as it is already done for other commands.

Also I believe src/binary_to_text/binary_encoder.ml should be src/ast/binary_printer.ml or src/ast/binary_encoder.ml if you prefer; but it has nothing to do with the binary to text conversion IMO.

Also, I would like to get at least one or two tests in test/wat2wasm (and maybe we can start to write a fuzzer that generate a module, then goes wat2wasm -> wasm2wat -> wat2wasm, and check the result or something like this, but this can be done later!). :)

@epatrizio epatrizio force-pushed the wat2wasm branch 7 times, most recently from 59cd77c to 5e5139f Compare June 5, 2024 13:26
zapashcanon
zapashcanon reviewed Jun 5, 2024
View reviewed changes
src/dune Outdated Show resolved Hide resolved
@epatrizio epatrizio force-pushed the wat2wasm branch 2 times, most recently from 2dacee4 to 2d6dfdf Compare June 5, 2024 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@zapashcanon zapashcanon zapashcanon left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@epatrizio @zapashcanon