wasmprinter: prints field names declared in the names section #1512
Conversation
There was a problem hiding this comment.
Thanks for this! Would you be up for implementing a bit more support so a test can be added for this? Ideally this PR would additionally have tests and/or examples showing the field names getting printed too, but I believe that the lack of parsing support is why they're not showing up. The implementation would go around here if you're interested.
If you're strapped for time though that's ok too. Test/etc can be added later in a future PR.
crates/wasmprinter/src/lib.rs
Outdated
| fn print_field_idx(&mut self, state: &State, ty: u32, idx: u32) -> Result<()> { | ||
| match state.core.field_names.index_to_name.get(&(ty, idx)) { | ||
| Some(name) => write!(self.result, "${}", name.identifier())?, | ||
| None if self.name_unnamed => write!(self.result, "$#local{idx}")?, |
There was a problem hiding this comment.
It's ok to not implement the --name-unnamed feature here if you'd prefer, but otherwise the #local here may want to change to #field and I think this'll need to update the field-printing-code as well to print this name if a name isn't otherwise listed.
There was a problem hiding this comment.
Good catch! I had this renamed to field at some point but must've gotten it changed back when shuffling things around.
I think this'll need to update the field-printing-code as well to print this name if a name isn't otherwise listed.
I think I did that in 4af11e1, but maybe check it matches what you had in mind
kritzcreek
force-pushed
the
printing-for-field-names
branch
from
5711508
to
929c51d
Compare
Hah! Lack of parsing support was my first guess, but when I checked out the parser it did parse the optional name. I didn't think to check if it ended up actually encoding it into the names section. I added an additional test case to explicitly check that referencing named struct fields via index get replaced with their names when printing the file back. PTAL |
Noticed these were missing from the output of
wasm-tools print. I'm surprised that none of the snapshots had to be updated. Is there something I'm missing?