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

[Jan Lahoda]

Hi Jon,

Thanks for the comments!

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

A delta webrev between this webrev and the webrev.00 is here:
http://cr.openjdk.java.net/~jlahoda/8192963/webrev.delta.00.01/

Does this look better?

Thanks!

Jan

On 28.7.2018 01:12, Jonathan Gibbons wrote:
> CaseTree.java:52: spelling, "lables"
>
> CaseTree.java:84:  "return" should be "returns".
>
> Lots of @Deprecated annotations without corresponding @deprecated
> javadoc tag.
> Should the @apiNote really be @deprecated.  I also think that using
> @Deprecated
> like this is "a bit smelly", and had a discussion with Stuart "Dr
> Deprecator" regarding
> my concerns. If nothing else, assuming the feature is a success, we are
> setting up
> a precedent of marking an API as deprecated-for-removal, and then in the
> next
> release, simply removing the annotation.
>
> Attr.java:1482: ugly use of @SuppressWarnings. I understand why it is
> there,
> but it exemplifies why @Deprecated on the declaration is a bad idea.
> It'll be too easy to not remove these annotations in JDK 13.
>
> Attr:1825: why XXX
>
> LambdaToMethod: unnecessary new import, CaseKind
> Flow: unnecessary new import, CaseKind
>
> JavacParser: order of imports
>
> messages: generally, I think the text of the messages could be improved
> (related but not new) we should somehow identify language keywords from
> English text (i.e. words like `break, `case`, `continue`, `return`)
> 204: saying the name is both a valid label and a valid expression hints
> at the cause but not the error (i.e. the name is ambiguous)
>
> JCTree: inconsistent layout of annotations; inconsistent use of
> @Deprecated and @Deprecated(forRemoval=true)
>
> (tests)
> Not reviewed in detail yet.
> New test descriptions should use "/*" for jtreg comment block; not
> "/**". These are not javadoc documentation comments.
> New test descriptions should have @summary
> Presumably, we will have to change the -old.out files if the preview
> feature becomes standard in JDK 13.
>
>
> Overall,
>
> The logic is nice, but the use of @Deprecated(forRemoval=true) and
> resulting @SuppressWarnings really suck.
> I don't think the current practice is serving us well.
>
> -- Jon
>
>
> On 07/17/2018 11:00 AM, 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
>