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

Vicente Romero vicente.romero at oracle.com
Thu Jul 19 20:04:07 UTC 2018


I don't have any further comments. It would be interesting to do a code 
coverage test,

Thanks,
Vicente

On 07/19/2018 03:39 PM, Jan Lahoda wrote:
> 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
>>



More information about the compiler-dev mailing list