Clarify and regularize text format index parsing rules#655
Conversation
| aliastarget ::= export <idx> <name> | ||
| | core export <idx> <core:name> | ||
| | outer <idx> <idx> |
There was a problem hiding this comment.
This seems unfortunate to me because it's no longer clear which kind of index is required here. I'm not sure if this is how it works today, but could it possibly take e.g. <core:instanceidx> instead of just <idx>? This would allow either a bare index or (core instance $foo).
Alternatively you could maybe just define a little notation for what kind of bare index is required, just for reader comprehension.
There was a problem hiding this comment.
With the grammar rules as defined, a <core:instanceidx> would mean that you could use an inline alias inside an alias which seemed like maybe too inceptive. Ultimately, the grammar bottoms out at a <u32> or <id>, and validation rules say "it has to be an instance in the instance index space", so that seemed kinda good enough, given that the second <idx> also needs special validation rules?
There was a problem hiding this comment.
Fair enough; I just hope I won't be confused about what kind of index is required in any particular context.
This PR intends to resolve #648 by regularizing and more-clearly defining the text format rules for core- and component-level indices, identifiers and inline aliases. One nice thing with the changes in this PR is that there is now an explicit
core-prefix(...)in the grammar at all the places wherecoremight (when a plain$idis not used) get injected.The
test/syntax/indices.wasttries to test all the syntactic cases touched by this PR. The commented-out lines currently fail inwasm-toolsand the rest pass as-is. This helps identify what the concrete delta from the status quo is. I didn't add any negative tests since I expect we'll need a phase-out process before rejecting anything that is accepted now.PTAL @bvisness @alexcrichton