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

Revise the tableswitch wording. #444

Merged
merged 3 commits into from
Nov 6, 2015
Merged

Revise the tableswitch wording. #444

merged 3 commits into from
Nov 6, 2015

Conversation

sunfishcode
Copy link
Member

Say "target" to describe tableswitch's table elements, to avoid the
ambiguity of whether a case can be branched to from other constructs,
and the conflict with the previous paragraph describing limitations on
the use of labels.

Say "target" to describe `tableswitch`'s table elements, to avoid the
ambiguity of whether a `case` can be branched to from other constructs,
and the conflict with the previous paragraph describing limitations on
the use of labels.
be either labels or `case` nodes.

A `case` node consists of a statement, and may be referenced in the parent
`tableswitch`'s array. Control falls through into the next `case` (or the end
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"By default control falls through..."

no parens on (or the end...)

@sunfishcode
Copy link
Member Author

Clarified that tableswitch can only target its immediate child cases, and mention that fallthrough only happens if the code doesn't exit via some other means first. And dropped parens.

@rossberg
Copy link
Member

rossberg commented Nov 4, 2015

This does not say how you actually target a case, and thereby still glosses over the fact that the jump table actually needs to differentiate the two kinds of target. In all your examples so far, you have somehow assumed that the distinction is inferred from the use of symbolic labels. But the AST does not have symbolic labels, it just has relative numeric references, which do not enable such inference.

So, an explicit jump table would need to have explicit tags differentiating the target types. In other words, the AST structure would actually need to be something like

(tableswitch (<expr>)
  (table (brcase <i>) (br <n>) (brcase <j>) ...) (brcase <k>)
  (case <expr>)
  (case <expr>)
  ...
)

@sunfishcode
Copy link
Member Author

The design docs don't currently specify the encoding of label references in the binary format, or the syntax of label names in the s-expression language. Should they?

Since they currently don't, it's currently consistent they also don't specify the encoding of tableswitch targets. (And there are fairly simple ways we can encode these.)

It's similarly consistent that they don't specify the s-expression syntax for tableswitch targets. I expect we'll represent a tableswitch target as either an integer following whatever the binary encoding does (as we already do for labels and other things), or for human friendliness a name that shares the namespace of labels. Or, if it's too much of a burden for s-expression parsers to think about different kinds of things sharing a namespace, perhaps we can pick a distinct sigil for cases.

@rossberg
Copy link
Member

rossberg commented Nov 4, 2015

I agree that the AST design doc doesn't need to (in fact, should not) describe encodings. However, the purpose of an AST is to make all structure explicit in a suitable form. And under this structural factorisation of tableswitch, having different kinds of references is a significant part of that structure, because it is semantically relevant.

S-expressions are largely irrelevant in this context. They should merely be a direct reflection of the AST.

@sunfishcode
Copy link
Member Author

It sounds like you're saying we shouldn't use sum types to describe things in WebAssembly except for the sum type of the main tree. What is the advantage of such a restriction?

@rossberg
Copy link
Member

rossberg commented Nov 4, 2015

Not sure I understand. More like the opposite: any part of the syntax that naturally maps to sum types is an essential part of the AST, and as such, would belong into AstSemantics. Or do you only consider operator nodes the AST? (For everything but switch you can almost get away with that, but that's rather incidental.)

@sunfishcode
Copy link
Member Author

This PR is now stalled.

@titzer
Copy link

titzer commented Nov 4, 2015

Is the issue simply that the table of the tableswitch can reference either
labels (i.e. from enclosing constructs) or cases?

IMO making a distinct AST category for cases as per this PR avoids
polluting the concept of labels with something that only applies to
switches.

On Wed, Nov 4, 2015 at 9:52 AM, Dan Gohman [email protected] wrote:

This PR is now stalled.


Reply to this email directly or view it on GitHub
#444 (comment).

@rossberg
Copy link
Member

rossberg commented Nov 6, 2015

FWIW, I'm fine with landing this and address the loose ends later.

sunfishcode added a commit that referenced this pull request Nov 6, 2015
@sunfishcode sunfishcode merged commit 86f942e into master Nov 6, 2015
@sunfishcode sunfishcode deleted the clarify-tableswitch branch November 6, 2015 23:14
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.

3 participants