Skip to content
This repository has been archived by the owner on Nov 26, 2024. It is now read-only.

Initial implementation of wasmbinary to wat #207

Open
wants to merge 10 commits into
base: stylus
Choose a base branch
from
Open

Conversation

joshuacolvin0
Copy link
Member

@joshuacolvin0 joshuacolvin0 commented Mar 10, 2024

Initial implementation. The wasm instructions are currently not formatted in wat format, so everything is provided, but result is not valid wat.

Resolves STY-22

@cla-bot cla-bot bot added the s label Mar 10, 2024
@joshuacolvin0 joshuacolvin0 marked this pull request as ready for review March 11, 2024 19:31
for (i, type_idx) in self.functions.iter().enumerate() {
let i1 = i as u32;

let export_str = match self.maybe_func_name(i1 + 1) {
Copy link
Member Author

Choose a reason for hiding this comment

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

Concerned that incorrect index is being used here

Copy link
Contributor

Choose a reason for hiding this comment

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

Fixed :)

@joshuacolvin0 joshuacolvin0 changed the title Initial implementation of wasmbinary to wat STY-22: Initial implementation of wasmbinary to wat Mar 11, 2024
@rachel-bousfield rachel-bousfield changed the title STY-22: Initial implementation of wasmbinary to wat Initial implementation of wasmbinary to wat Mar 12, 2024
Copy link
Contributor

@rachel-bousfield rachel-bousfield left a comment

Choose a reason for hiding this comment

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

Made some simplifications & fixes. There's just two things to note

  • The reason function names are sometimes wrong is that we're not considering imports. If a WASM has no imports, then the names are correct. However if there's n imports, the names will be shifted n away. Fixing this requires changing func_name, maybe_func_name, etc to distinguish imports from local functions.
  • Some of the changes affect Module printing, which may be worth checking again and fixing where visually affected

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants