Thank you Alex and Tagir. I have uploaded a new version of the spec at:

http://cr.openjdk.java.net/~gbierman/switch-expressions.html

This contains all the changes you suggested below. In addition, there is a 
small bug fix in 5.6.3 concerning widening 
(https://bugs.openjdk.java.net/browse/JDK-8213180). I have also taken the 
opportunity to reorder chapter 15 slightly, so switch expressions are now 
section 15.28 and constant expressions are now section 15.29 (the last section 
in the chapter). 

Comments welcome!
Gavin


> On 14 Jan 2019, at 21:40, Alex Buckley <[email protected]> wrote:
> 
> Hi Gavin,
> 
> Some points driven partly by the discussion with Tagir:
> 
> 1. In 14.11.1, SwitchLabeledBlock should not end with a `;` -- there is no 
> indication in JEP 325 that a semicolon is desired after `-> {...}` and javac 
> in JDK 12 does not accept one there. Also, SwitchLabeledThrowStatement should 
> not end with a `;` because ThrowStatement includes a `;`.
> 
> 2. In 14.11.1, "This block can either be empty, or take one of two forms:" is 
> wrong for switch expressions. The emptiness allowed by the grammar will be 
> banned semantically in 15.29.1, so 14.11.1 should avoid trouble by speaking 
> broadly of the forms in an educational tone: "A switch block can consist of 
> either: - _Switch labeled rules_, which use `->` to introduce either a 
> _switch labeled expression_, ..."    Also, "optionally followed by switch 
> labels." is wrong for switch expressions, so prefer: "- _Switch labeled 
> statement groups_, which use `:` to introduce block statements."
> 
> 3. In 15.29.1: (this is mainly driven by eyeballing against 14.11.2)
> 
> - Incorrect Markdown in section header.
> 
> - The error clause in the following bullet is redundant because the list 
> header already called for an error: "The switch block must be compatible with 
> the type of the selector expression, *****or a compile-time error 
> occurs*****."
> 
> - I would prefer to pull the choice of {default label, enum typed selector 
> expression} into a fourth bullet of the prior list, to align how 14.11.2's 
> list has a bullet concerning default label.
> 
> - The significant rule from 14.11.2 that "If the switch block consists of 
> switch labeled rules, then any switch labeled expression must be a statement 
> expression (14.8)." has no parallel in 15.29.1. Instead, for switch labeled 
> rules, 15.29.1 has a rule for switch labeled blocks. (1) We haven't seen 
> switch labeled blocks for ages, so a cross-ref to 14.11.1 is due. (2) A note 
> that switch exprs allow `-> ANY_EXPRESSION` while switch statements allow `-> 
> NOT_ANY_EXPRESSION` is due in both sections; grep ch.8 for "In this respect" 
> to see what I mean. (3) The semantic constraints on switch labeled 
> rules+statement groups in 15.29.1 should be easily contrastable with those in 
> 14.11.2 -- one approach is to pull the following constraints into 15.29.1's 
> "all conditions true, or error" list:
> 
> -----
> - If the switch block consists of switch labeled rules, then any switch 
> labeled block (14.11.1) MUST COMPLETE ABRUPTLY.
> - If the switch block consists of switch labeled statement groups, then the 
> last statement in the switch block MUST COMPLETE ABRUPTLY, and the switch 
> block MUST NOT HAVE ANY SWITCH LABELS AFTER THE LAST SWITCH LABELED STATEMENT 
> GROUP.
> -----
> 
> If you prefer to keep these semantic constraints standalone so that they have 
> negative polarity, then 14.11.2 should to the same for its 
> significant-but-easily-missed "must be a statement expression" constraint.
> 
> Alex
> 
> On 1/13/2019 2:53 AM, Tagir Valeev wrote:
>> Hello!
>> 
>>> I'm concerned about any claim of ambiguity in the grammar, though I'm
>>> not sure I'm following you correctly. I agree that your first fragment
>>> is parsed as two statements -- a switch statement and an empty statement
>>> -- but I don't know what you mean about "inside switch expression rule"
>>> for your second fragment. A switch expression is not an expression
>>> statement (JLS 14.8). In your second fragment, the leftmost default
>>> label is followed not by a block or a throw statement but by an
>>> expression (`switch (0) {...}`, a unary expression) and a semicolon.
>> 
>> Ah, ok, we moved away slightly from the spec draft [1]. I was not
>> aware, because I haven't wrote parser by myself. The draft says:
>> 
>> SwitchLabeledRule:
>>   SwitchLabeledExpression
>>   SwitchLabeledBlock
>>   SwitchLabeledThrowStatement
>> 
>> SwitchLabeledExpression:
>>   SwitchLabel -> Expression ;
>> SwitchLabeledBlock:
>>   SwitchLabel -> Block ;
>> SwitchLabeledThrowStatement:
>>   SwitchLabel -> ThrowStatement ;
>> 
>> (by the way I think that ; after block and throw should not be
>> present: current implementation does not require it after the block
>> and throw statement already includes a ; inside it).
>> 
>> Instead we implement it like:
>> 
>> SwitchLabeledRule:
>>   SwitchLabel -> SwitchLabeledRuleStatement
>> SwitchLabeledRuleStatement:
>>   ExpressionStatement
>>   Block
>>   ThrowStatement
>> 
>> So we assume that the right part of SwitchLabeledRule is always a
>> statement and reused ExpressionStatement to express Expression plus
>> semicolon, because syntactically it looks the same. Strictly following
>> a spec draft here looks even more ugly, because it requires more
>> object types in our code model and reduces the flexibility when we
>> need to perform code transformation. E.g. if we want to wrap
>> expression into block, currently we just need to replace an
>> ExpressionStatement with a Block not touching a SwitchLabel at all.
>> Had we mirrored the spec in our code model, we would need to replace
>> SwitchLabeledExpression with SwitchLabeledBlock which looks more
>> annoying.
>> 
>> With best regards,
>> Tagir Valeev
>> 
>> [1] http://cr.openjdk.java.net/~gbierman/switch-expressions.html#jep325-14.11
>> 

Reply via email to