json-from-wast: differences to wabt's wast2json

[ohorn]

I just saw the new json-from-wast command. It is great to see a wast2json-like tool as part of wasm-tools, especially since it supports many more proposed features than wabt.

However, I've noticed that json-from-wast outputs slightly different JSON than wabt. In particular:

• wabt writes the module names in their $-prefixed form, while json-from-wast drops the $ from the module names.
• wabt writes all numeric values in their unsigned representation (even if the corresponding type itself is signed like i32 and i64), while json-from-wast writes negative numbers as they are, with a minus sign.

Another difference is how the filenames for the associated modules are derived: wabt derives these names from the given output filename. IMHO, this has the advantage that both the JSON file and the associated module files share the same base name.

As noted in the commit, json-from-wast also doesn't write the expected return types for commands like assert_trap. Are there any plans to add this feature in the future?
I also experimented with my own wast2json implementation in the past. My approach was to parse and analyze the associated modules after encoding and store their exported function and global types so that they could be queried when needed.

[alexcrichton]

Hello and thanks for the report! Yeah I noticed these differences as well when writing it, and while they could be fixed it felt a bit odd given the implementation in this repository to go out of our way to represent that. Are these differences causing issues for consumption?

If you'd like I think it'd be reasonable to change the output in most of the cases here. I had my own reasons for not matching wabt exactly in some of these cases (mostly that it required "going out of the way" to exactly match as opposed to do what this repository does natively), but other cases (like the filename bits) I just didn't realized. I think it'd be totally fine to have a PR to update things, if you're willing.

also doesn't write the expected return types for commands like assert_trap. Are there any plans to add this feature in the future?

That could be added, we've in theory got all the tooling to support this. It would be a pretty big lift though because we'd have to (a) maintain a mapping of named modules to know which module is being referred to with assert_{trap,invoke} and (b) we'd have to parse and validate the module to determine the type signature. I was hoping that the expected bits wouldn't really be that necessary to avoid needing this work, but if you'd like to send a PR I think that's reasonable as well.

[ohorn]

If you'd like I think it'd be reasonable to change the output in most of the cases here. I had my own reasons for not matching wabt exactly in some of these cases (mostly that it required "going out of the way" to exactly match as opposed to do what this repository does natively), but other cases (like the filename bits) I just didn't realized. I think it'd be totally fine to have a PR to update things, if you're willing.

Ok, I see. How about adding a flag to switch between the current wasm-tools-native output and a wabt-compliant output? If that's ok, I will prepare a PR for it.

also doesn't write the expected return types for commands like assert_trap. Are there any plans to add this feature in the future?

That could be added, we've in theory got all the tooling to support this. It would be a pretty big lift though because we'd have to (a) maintain a mapping of named modules to know which module is being referred to with assert_{trap,invoke} and (b) we'd have to parse and validate the module to determine the type signature. I was hoping that the expected bits wouldn't really be that necessary to avoid needing this work, but if you'd like to send a PR I think that's reasonable as well.

Yes, I know this is not trivial. I already have some code to manage these mappings, at least for the core commands (the thread command adds its own complexity to this issue). I'll see if I can adapt that code and then open a PR.

[alexcrichton]

Heh I actually originally implemented a flag, but I decided against it since I don't think there's much benefit in having two modes. If you're interseted to change the output to match wabt exactly I think it'd be reasonable to just change the output and avoid a flag for it.

Out of curiosity, what's the use case for expected in situations like assert_trap?