JDK-8212982: Rule cases in switch expression accepted even if complete normally (original) (raw)

Jan Lahoda jan.lahoda at oracle.com
Wed Nov 21 10:31:13 UTC 2018

Hi Maurizio,

Thanks for the comments. I've renamed NOT_ALIVE to DEAD, as suggested, and replace "alive.alive" with appropriate tests (usually either == Liveness.DEAD or != Liveness.DEAD). I was first afraid this would look too bad, but seems to be quite reasonable to me in the end.

Updated webrev is here: http://cr.openjdk.java.net/~jlahoda/8212982/webrev.01/

Any further feedback is welcome!

Thanks, Jan

On 20.11.2018 19:17, Maurizio Cimadamore wrote:

I like the tri-state fix; there is however something that leaves me perplexed. I see often idioms such as

if (alive.alive ... Now, it seems like the Liveliness enum has a dual nature; on the one hand it's a tri-state (alive, dead, recovery). But there are cases in which you want to project it down to a two-state. The use of an hidden boolean here makes the code less readable, because a reader might ask - how is that different from alive == Liveliness.ALIVE? I think we should try to think as to what is the condition that we want to capture with the alive.alive and then have a method on the enum which returns 'true' for certain constants and false for others. I think what the code is doing is checking whether the liveliness *is not* dead. So perhaps, conditions such as alive.alive might better be rewritten as alive != Liveliness.NOTALIVE (btw, probably better to call NOTALIVE just DEAD and avoid all the double negations). Maurizio

On 20/11/2018 17:14, Jan Lahoda wrote: Hi,

Bug: https://bugs.openjdk.java.net/browse/JDK-8212982 The issue here is that javac accepts switch expressions that complete normally without providing a value, which is not correct. A (simpler) fix is to enhance Flow with the necessary checks + enhancements to place the errors at a good place. Webrev: http://cr.openjdk.java.net/~jlahoda/8212982/webrev.00/ This patch has a problem in cases like: --- public class SE { private String t(int i) { return switch (i) { default: break ""; System.err.println(0); }; } } --- This produces: --- $ javac --enable-preview --source 12 SE.java SE.java:6: error: unreachable statement System.err.println(0); ^ SE.java:7: error: switch expression completes without providing a value }; ^ (switch expressions must either provide a value or throw for all possible input values) --- The second error is caused by the first one (Flow will reset "alive" to "true" when reporting the "unreachable statement" error as an error recovery). A patch that changes the "alive" field to be tri-state (ALIVE, NOTALIVE, RECOVERY) so that it can suppress the second error in case of this recovery is here: Webrev 2: http://cr.openjdk.java.net/~jlahoda/8212982/webrev.00b/ (The only differences between the patches are in the Flow.java and ExpressionSwitchUnreachable.out.) This is longer, but I think it provides better errors, so I'd prefer this patch, but I am also fine with the first one. Any feedback is welcome! Thanks, Jan

More information about the compiler-dev mailing list