RFR: JDK-8192963/JDK-8206986 JEP 325: Switch Expressions (Preview)

[Jan Lahoda]

Hi Vicente,

Thanks for the comments. An updated webrev is here:
http://cr.openjdk.java.net/~jlahoda/8192963/webrev.01/

Comments inline.

On 18.7.2018 19:59, Vicente Romero wrote:
> pretty nice work, some notes below:
>
> - SwitchExpressionTree.java is a new file so the copyright should say
> 2018 only plus you should remove some @author comments, probably taken
> from SwitchTree.

Done.

> - Attr, it is a bit weird seeing, the method below receiving a tree
> instead of an expression, consider renaming it to attribTree
>          public Type attribExpr(JCTree tree, Env<AttrContext> env,
> ResultInfo resultInfo)

Turned out this new overload is not needed anymore, I've remove it.

> - also in Attr, isn't: @Override public void visitMethodDef(JCMethodDecl
> tree) {} at the TreeScanner anonymous class created at
> visitSwitchExpression redundant?

Yes, that's not really necessary, removed.

> - at Lower, minor, inside method: convertCase, is it necessary to always
> create a block inside visitBreak?

Here, a single statement (value break) is replaced with two statements 
(an assignment and a break). If the original value break would be inside 
a block, then we could replace it with the statements inside the 
enclosing block. But we would still need to replace the value break with 
a block sometimes (if the value break would not be enclosed by a block), 
and using the block always shouldn't hurt much, so I opted for the 
current simpler state.

Jan

>
> overall great job!
> Vicente
>
> On 07/17/2018 02:00 PM, Jan Lahoda wrote:
>> Hi,
>>
>> As JEP 325 is in the proposed to target state, I thought it might be a
>> good idea to start a review process for the code.
>>
>> The code here strives to implement the current specification for the
>> Switch Expressions:
>> http://cr.openjdk.java.net/~gbierman/switch-expressions.html
>>
>> The current webrev is here:
>> http://cr.openjdk.java.net/~jlahoda/8192963/webrev.00/
>>
>> (includes a list of new errors.)
>>
>> JBS:
>> https://bugs.openjdk.java.net/browse/JDK-8192963
>> https://bugs.openjdk.java.net/browse/JDK-8206986
>>
>> CSRs:
>> https://bugs.openjdk.java.net/browse/JDK-8207241
>> https://bugs.openjdk.java.net/browse/JDK-8207406
>>
>> Any feedback is welcome!
>>
>> Thanks,
>>     Jan
>