wabt icon indicating copy to clipboard operation
wabt copied to clipboard

write invalid names using `@name` annotation

Open dzfrias opened this issue 2 years ago • 4 comments

Following up #2284, a few commits were reverted related to the @name annotation so that the PR would focus on a single addition. This PR makes use of the @name annotation when writing names containing invalid characters in the text format (see WebAssembly/spec#617).

Let me know if there's anything else I should add!

dzfrias avatar Sep 16 '23 05:09 dzfrias

Could you please also add a few more tests, e.g. a roundtrip test for the example given in https://github.com/WebAssembly/annotations/blob/main/proposals/annotations/Overview.md (under "Expressing names")?

Looking at this test, it seems I misinterpreted the use of @name. In the proposal example, parts of the WebAssembly module have two name: one for use in the WAT representation ($x, etc.), and one for use the binary ((@name "..."), etc.). My initial implementation just assumed that there would be one name for both. I'll work on implementing the intended version.

dzfrias avatar Sep 19 '23 16:09 dzfrias

Upon working on the implementation of the work in my above comment, it seems like there are many possible places one could use the @name annotation. Is there a well-defined collection of all these places? For example, would it work in module imports?

edit: Also, in the text format, do we want to accept names that only have the @name annotation, and no $name equivalent?

dzfrias avatar Sep 19 '23 21:09 dzfrias

You raise some good questions but I definitely don't have a view on what the behavior is supposed to be. I guess we should just match other implementations (if there are other implementations -- maybe this is too soon?). It's a little unfortunate that these annotation specifications aren't going to get tests. :-(

keithw avatar Sep 20 '23 17:09 keithw

I'll wait until WebAssembly/annotations#21 is resolved. If the $"name" syntax is approved, then most of these problems shouldn't be relevant anymore.

dzfrias avatar Sep 20 '23 20:09 dzfrias