Conversation
|
As a simplification, this PR makes sense. However, we've gone back and forth on the label issue multiple times due to trying to serve different goals: either code density, performance, or implementation simplicity. While I'd be happy to make this change for simplification, I am not sure we should do that just yet without some data on the other goals. Specifically, implementing implicit blocks for if was quite a pain to get right, but was justified at the time that it would make sense for code size. It'd be a shame to remove them now for simplicity and then add them back later when we again want to optimize for code size. |
|
Changing if wasn't in my original proposal, but in his oposition to that proposal, @rossberg-chromium requested we change if too because he is "confident that is a super-rare case". |
AstSemantics.md
Outdated
| within the `block`'s body. | ||
| Branches may only reference labels defined by an outer *enclosing construct*, | ||
| which can be a `block` (with a label at the end), `loop` (with a label at the | ||
| beginning), or the function body (with a label at the end). This means that, |
There was a problem hiding this comment.
'or the function body (with a label at the end)' - this seems a significant new change too? Could this be deferred to discussion in a separate issue. One concern is that this might require an interpretation of the break arguments as the return values when the multi-value support has not been determined yet - this interpretation might be ok but there are different ways this could go.
|
I'm in favor of this PR too. |
|
I'm fine with this patch, too (although it's a bit sad to see the nice return=break unification go -- I'd be fine with leaving that alone, although my own consistency argument speaks against it :) ). If we do not make this change and keep all implicit labels, then I would at least propose to switch the order of the two loop labels. Then you can explain the design consistently by saying that all block-like expression sequences have an implicit label directly enclosing them. |
This removes loop's bottom label.
|
Updated this PR to just remove the bottom label of |
AstSemantics.md
Outdated
|
|
||
| * `block`: yields either the value of the last expression in the block or the result of an inner branch that targeted the label of the block | ||
| * `loop`: yields either the value of the last expression in the loop or the result of an inner branch that targeted the end label of the loop | ||
| * `loop`: yields either the value of the last expression in the loop |
lukewagner
changed the title
|
Merged with lgtms above. This PR was against the master branch and github doesn't allow this to be changed, so I've now pushed the change directly to binary_0xc, and am closing this PR. |
4 participants
This removes loop's top label and if/else's end label, simplifying them, according to @rossberg-chromium's suggestion in #652.
With this PR, the only constructs that introduce labels are
block(label at the end),loop(label at the beginning), and the function body itself (label at the end) (since the current semantics are that the function body acts like an implicit block).