Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consistency in canonical definition indices #90

Closed
pl-semiotics opened this issue Aug 15, 2022 · 2 comments
Closed

Consistency in canonical definition indices #90

pl-semiotics opened this issue Aug 15, 2022 · 2 comments

Comments

@pl-semiotics
Copy link
Collaborator

pl-semiotics commented Aug 15, 2022

Prior to #42, the Canonical ABI syntax used core-prefix(core:<core:sort>idx) for several core:sorts as part of the canon option syntax. This was slightly strange, since the core-prefix meta-rule is only defined for rules which parse conses, whereas the various core:...idx rules in question parsed atoms, and additionally made things quite verbose, so #42 replaced them with core:memidx and core:funcidx, which makes a great deal of sense.

However, the explainer treats this as allowing either an actual identifier/number (as from core:memidx/core:funcidx) or an alias of syntax $id "name"+. This does not match how the inline export alias syntax is defined elsewhere in the explainer, which states:

For export aliases, the inline sugar has the form (<sort> <instanceidx> <name>+) and can be used in place of a sortidx or any sort-specific index (such as a typeidx or funcidx).

This gives rise to syntax such as

sortidx     ::= (<sort> <u32>)
              | (<sort> <instanceidx> <name>+)
core:memidx ::= <u32>
              | (<sort> <instanceidx> <name>+)  # Where <sort> must in fact be "core memory"

By that rule, the current grammar accepts canonopts such as (memory 0) (meaning the first entry in the core memory index space of the current component), (memory $M) (the same, or an outer alias, via scoped identifier resolution), and (memory (core memory $coremod "name")), which uses an inline export alias. (Whether this inline export alias would consult the core or component index space, or have syntax for both, is also an open question; see #89.)

I think this is a reasonable option, but it doesn't match the syntax in most of the explainer examples, which should perhaps be updated. Another option would be to modify the way that we write export alias sugar everywhere to specify that when an export alias replaces a sort-specific index it (a) does not name the sort and (b) does not appear inside parentheses (in order to reach the precise syntax currently in the explainer); i.e. to specify something more like

For export aliases, the inline sugar has the form <instanceidx> <name>+ and can be used in place of any index into an index space.

Giving rise to syntax such as

sortidx     ::= (<sort> <u32>)
              | (<sort> <instanceidx> <name>+)
core:memidx ::= <u32>
              | <instanceidx> <name>+

However, this may cause issues with grammatical ambiguity.

@pl-semiotics pl-semiotics changed the title Consistency in canonical abi indices Consistency in canonical definition indices Aug 15, 2022
@lukewagner
Copy link
Member

That's a good idea, and it would help remove some redundancy in the canonopt case. Scanning through the examples in the explainer just now and trying it out, one problem I'm seeing is that, when the <...idx> isn't the only operand of the s-expression, the inline sequence <instanceidx> <name>+ does end up sortof looking visually (and maybe grammatically) ambiguous so that it's hard to tell when the <...idx> stops and the next operand beings (take, e.g., the <funcidx> operand in the middle of canon lower).

So I thinking the fix is just to fix the examples to follow the existing rules. I'll file a PR for that in a sec...

@lukewagner
Copy link
Member

I fixed the examples in #94, but happy to continue discussing your syntax idea here, or close this issue otherwise.

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

No branches or pull requests

2 participants