Skip to content

Wrap the tableswitch default target in a (default ...) node #191

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

Conversation

AndrewScheidecker
Copy link
Contributor

Wrap the tableswitch default target in a (default ...) node so that something like (default (case x)) stands out from its (case ...) siblings that define targets rather than references to targets.

Before:

(tableswitch (i32.const 1)
    (table (case $0))
    (case $default)
    (case $default ...)
    (case $0 ...)
)

After:

(tableswitch (i32.const 1)
    (table (case $0))
    (default (case $default))
    (case $default ...)
    (case $0 ...)
)

@kg
Copy link
Contributor

kg commented Nov 21, 2015

It would be much better if this were a part of the case node instead of a new imaginary (i.e. not actually a functional part of the AST or encoded into binary) s-expression node that adds extra nesting.

We have something like align=N for loads and stores, so how about (case default $label) vs the regular (case $label)? Alternately, you introduce a (default-case $label) opcode.

@AndrewScheidecker
Copy link
Contributor Author

I went with (default ...) to match the (table ...) node preceding it in the tableswitch syntax.

Alternately, you introduce a (default-case $label) opcode.

Then you'd also need default-br, since tableswitch supports a non-case label as the default, distinguished by (default (br ...)) (using this commit's syntax). Maybe finding a way to avoid that distinction would be best, though.

@rossberg
Copy link
Member

I agree the syntax of the default target can be slightly irritating, but is it really a problem serious enough to justify adding extra syntactic baggage?

If we think so, then I'd rather revive the earlier suggestion of simply including the default in the table, and define dispatch by clamping the index value. See #163 (comment) and subsequent discussion. At least that would make the syntax simpler, not more complicated, though I also understand Dan's reservations.

Another option would be to use a different keyword for the case in the table/default, but I don't have a good suggestion.

@sunfishcode
Copy link
Member

Another alternative is to remove the default from tableswitch altogether. A proposal to do that is now at WebAssembly/design#482 .

…omething like (default (case x)) stands out from its (case ...) siblings that define targets rather than references to targets.
@rossberg
Copy link
Member

Obsolete with br_table, closing.

@rossberg rossberg closed this Feb 25, 2016
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

Successfully merging this pull request may close these issues.

4 participants