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

Maurizio Cimadamore maurizio.cimadamore at oracle.com
Tue Nov 20 18:17:19 UTC 2018

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.NOT_ALIVE

(btw, probably better to call NOT_ALIVE 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