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

Separate desugaring from parser & add label sugar #117

Merged
merged 5 commits into from
Oct 9, 2015
Merged

Conversation

rossberg
Copy link
Member

@rossberg rossberg commented Oct 8, 2015

This patch supersedes #99.

Defines sugared forms in a new module spec/sugar.

Introduces sugar for defining labels directly as part of a block, loop, or switch. A loop allows up to two labels, one to exit, the other to continue the loop.

Redefines return as sugar for break, slightly simplifying the spec, and making the core language compositional (substitution is now possible for function bodies, up to variable renaming).

@lukewagner
Copy link
Member

Using functions that generate the AST seems like a nice compromise to having a whole duplicate AST; I like it.

So we'd also talked about having 1 constructor per op name (in AstSemantics.md). Do you have any plans to (and if not, would you be opposed to me) adding 1 Sugar function for every single op? What's nice is the naming convention should allow us to use identical identifiers. That actually sounds really nice: we could have each op name in AstSemantics.md be a link to the Sugar.ml definition from which people could work their way through the semantics.

@lukewagner
Copy link
Member

Anyhow, lgtm as is; Sugar-All-The-Ops could be a followup.

@rossberg
Copy link
Member Author

rossberg commented Oct 9, 2015

Actually, I already had implemented exactly that, but then undid it because I was on the fence deciding whether this was overkill or not. But since you prefer it, I redid it. Please take another look.

@lukewagner
Copy link
Member

I had actually been thinking of going even further and, e.g., not just having 1 load, but having all of i32.load, i32.load8_s, etc. It does mean more lines of code in parser.mly and sugar.ml, but it's trivial enumeration and I think having a literal 1:1 correspondence is valuable. Using nested modules, it seems like we could even literally have Sugar.i32.load. Also, I was thinking maybe Op instead of Sugar so we could say "here's our list of ops" and not have to explain the sugar distinction.

@rossberg
Copy link
Member Author

rossberg commented Oct 9, 2015

Oh. Are you serious? That would really blow up everything big time (like 20x) and create a major nuisance for adding future opcodes. It would also over-emphasize concrete lexical syntax, which I don't think belongs here. I'd rather not go there.

@lukewagner
Copy link
Member

I think this depends on how you interpret the names in AstSemantics.md. What you're saying makes sense if you think of, e.g., i32.add as the name of the Binary AST node plus the I32.Add constant operand. But that's something we've been moving away from (design/#374), viewing i32.add as simply an AST node (no constant operands, just 2 child subexprs). This is the discrepancy I was hoping to close. Yes, it'll be 100+ lines in parser.mly, sugar.ml(i), but each of those lines is going to be trivial and so I don't see it actually increasing the cognitive load. It seems we need the full list somewhere in spec/ and this seems like pretty much the minimal, meaningful list.

@rossberg
Copy link
Member Author

rossberg commented Oct 9, 2015

You would also need to expand out all the lexer. That sounds super ugly. You'd basically be throwing out and forgetting all structure early on, just to reconstruct it later. With all the code duplication that implies in the middle. From an implementation stand point I don't think that makes a lot of sense.

I have been viewing the xyzop parameters of various constructors in ast as part of the opcode. Basically, they are a means for providing some amount of internal structure to opcodes (admittedly, the distinction from regular arguments would be clearer if ML allowed curried constructors).

You say the opcode names should appear in spec/ somewhere. I see what you mean, but I'm not sure it is crucial to be that literal, as long as the rendering is obvious from looking at ast + sugar. The literal opcode names are a specific textual representation for the sake of exposition. As such, they are concrete syntax, not abstract syntax.

@rossberg
Copy link
Member Author

rossberg commented Oct 9, 2015

PS: Independent of such considerations, do you think the current PR is good to go for now? :)

@lukewagner
Copy link
Member

Ok, I see what you're saying and I don't really have a strong opinion here. I also realized that, when we define the binary encoding in ml-proto, that will include the module opcode table and that will establish the literal correspondence in spec/.

PS: Independent of such considerations, do you think the current PR is good to go for now? :)

Doubly yes, then.

rossberg added a commit that referenced this pull request Oct 9, 2015
Separate desugaring from parser & add label sugar
@rossberg rossberg merged commit 283bbac into master Oct 9, 2015
@rossberg rossberg deleted the label-sugar branch October 9, 2015 18:00
Connicpu pushed a commit to Connicpu/wasm-spec that referenced this pull request May 11, 2020
…ebAssembly#117)

Attempting to use atomic access operators on non-shared linear memory is a validation error.
rossberg pushed a commit that referenced this pull request Feb 11, 2021
dhil pushed a commit to dhil/webassembly-spec that referenced this pull request Mar 2, 2023
rossberg pushed a commit that referenced this pull request Sep 4, 2024
rossberg pushed a commit that referenced this pull request Sep 4, 2024
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.

2 participants