8212982: Rule cases in switch expression accepted even if complete normally
Summary: Ensure an error is reported if switch expression does not correctly produce a value.
Reviewed-by: mcimadamore
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java Wed Nov 21 15:05:21 2018 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java Wed Nov 21 15:22:57 2018 +0100
@@ -357,9 +357,9 @@
}
/** Resolve all jumps of this statement. */
- private boolean resolveJump(JCTree tree,
- ListBuffer<P> oldPendingExits,
- JumpKind jk) {
+ private Liveness resolveJump(JCTree tree,
+ ListBuffer<P> oldPendingExits,
+ JumpKind jk) {
boolean resolved = false;
List<P> exits = pendingExits.toList();
pendingExits = oldPendingExits;
@@ -373,16 +373,16 @@
pendingExits.append(exit);
}
}
- return resolved;
+ return Liveness.from(resolved);
}
/** Resolve all continues of this statement. */
- boolean resolveContinues(JCTree tree) {
+ Liveness resolveContinues(JCTree tree) {
return resolveJump(tree, new ListBuffer<P>(), JumpKind.CONTINUE);
}
/** Resolve all breaks of this statement. */
- boolean resolveBreaks(JCTree tree, ListBuffer<P> oldPendingExits) {
+ Liveness resolveBreaks(JCTree tree, ListBuffer<P> oldPendingExits) {
return resolveJump(tree, oldPendingExits, JumpKind.BREAK);
}
@@ -417,11 +417,11 @@
/** A flag that indicates whether the last statement could
* complete normally.
*/
- private boolean alive;
+ private Liveness alive;
@Override
void markDead() {
- alive = false;
+ alive = Liveness.DEAD;
}
/*************************************************************************
@@ -432,7 +432,7 @@
*/
void scanDef(JCTree tree) {
scanStat(tree);
- if (tree != null && tree.hasTag(JCTree.Tag.BLOCK) && !alive) {
+ if (tree != null && tree.hasTag(JCTree.Tag.BLOCK) && alive == Liveness.DEAD) {
log.error(tree.pos(),
Errors.InitializerMustBeAbleToCompleteNormally);
}
@@ -441,9 +441,9 @@
/** Analyze a statement. Check that statement is reachable.
*/
void scanStat(JCTree tree) {
- if (!alive && tree != null) {
+ if (alive == Liveness.DEAD && tree != null) {
log.error(tree.pos(), Errors.UnreachableStmt);
- if (!tree.hasTag(SKIP)) alive = true;
+ if (!tree.hasTag(SKIP)) alive = Liveness.RECOVERY;
}
scan(tree);
}
@@ -460,7 +460,7 @@
public void visitClassDef(JCClassDecl tree) {
if (tree.sym == null) return;
- boolean alivePrev = alive;
+ Liveness alivePrev = alive;
ListBuffer<PendingExit> pendingExitsPrev = pendingExits;
Lint lintPrev = lint;
@@ -506,10 +506,10 @@
Assert.check(pendingExits.isEmpty());
try {
- alive = true;
+ alive = Liveness.ALIVE;
scanStat(tree.body);
- if (alive && !tree.sym.type.getReturnType().hasTag(VOID))
+ if (alive == Liveness.ALIVE && !tree.sym.type.getReturnType().hasTag(VOID))
log.error(TreeInfo.diagEndPos(tree.body), Errors.MissingRetStmt);
List<PendingExit> exits = pendingExits.toList();
@@ -544,21 +544,21 @@
ListBuffer<PendingExit> prevPendingExits = pendingExits;
pendingExits = new ListBuffer<>();
scanStat(tree.body);
- alive |= resolveContinues(tree);
+ alive = alive.or(resolveContinues(tree));
scan(tree.cond);
- alive = alive && !tree.cond.type.isTrue();
- alive |= resolveBreaks(tree, prevPendingExits);
+ alive = alive.and(!tree.cond.type.isTrue());
+ alive = alive.or(resolveBreaks(tree, prevPendingExits));
}
public void visitWhileLoop(JCWhileLoop tree) {
ListBuffer<PendingExit> prevPendingExits = pendingExits;
pendingExits = new ListBuffer<>();
scan(tree.cond);
- alive = !tree.cond.type.isFalse();
+ alive = Liveness.from(!tree.cond.type.isFalse());
scanStat(tree.body);
- alive |= resolveContinues(tree);
- alive = resolveBreaks(tree, prevPendingExits) ||
- !tree.cond.type.isTrue();
+ alive = alive.or(resolveContinues(tree));
+ alive = resolveBreaks(tree, prevPendingExits).or(
+ !tree.cond.type.isTrue());
}
public void visitForLoop(JCForLoop tree) {
@@ -567,15 +567,15 @@
pendingExits = new ListBuffer<>();
if (tree.cond != null) {
scan(tree.cond);
- alive = !tree.cond.type.isFalse();
+ alive = Liveness.from(!tree.cond.type.isFalse());
} else {
- alive = true;
+ alive = Liveness.ALIVE;
}
scanStat(tree.body);
- alive |= resolveContinues(tree);
+ alive = alive.or(resolveContinues(tree));
scan(tree.step);
- alive = resolveBreaks(tree, prevPendingExits) ||
- tree.cond != null && !tree.cond.type.isTrue();
+ alive = resolveBreaks(tree, prevPendingExits).or(
+ tree.cond != null && !tree.cond.type.isTrue());
}
public void visitForeachLoop(JCEnhancedForLoop tree) {
@@ -584,16 +584,16 @@
scan(tree.expr);
pendingExits = new ListBuffer<>();
scanStat(tree.body);
- alive |= resolveContinues(tree);
+ alive = alive.or(resolveContinues(tree));
resolveBreaks(tree, prevPendingExits);
- alive = true;
+ alive = Liveness.ALIVE;
}
public void visitLabelled(JCLabeledStatement tree) {
ListBuffer<PendingExit> prevPendingExits = pendingExits;
pendingExits = new ListBuffer<>();
scanStat(tree.body);
- alive |= resolveBreaks(tree, prevPendingExits);
+ alive = alive.or(resolveBreaks(tree, prevPendingExits));
}
public void visitSwitch(JCSwitch tree) {
@@ -602,7 +602,7 @@
scan(tree.selector);
boolean hasDefault = false;
for (List<JCCase> l = tree.cases; l.nonEmpty(); l = l.tail) {
- alive = true;
+ alive = Liveness.ALIVE;
JCCase c = l.head;
if (c.pats.isEmpty())
hasDefault = true;
@@ -612,13 +612,13 @@
}
}
scanStats(c.stats);
- c.completesNormally = alive;
- if (alive && c.caseKind == JCCase.RULE) {
+ c.completesNormally = alive != Liveness.DEAD;
+ if (alive != Liveness.DEAD && c.caseKind == JCCase.RULE) {
scanSyntheticBreak(make, tree);
- alive = false;
+ alive = Liveness.DEAD;
}
// Warn about fall-through if lint switch fallthrough enabled.
- if (alive &&
+ if (alive == Liveness.ALIVE &&
lint.isEnabled(Lint.LintCategory.FALLTHROUGH) &&
c.stats.nonEmpty() && l.tail.nonEmpty())
log.warning(Lint.LintCategory.FALLTHROUGH,
@@ -626,9 +626,9 @@
Warnings.PossibleFallThroughIntoCase);
}
if (!hasDefault) {
- alive = true;
+ alive = Liveness.ALIVE;
}
- alive |= resolveBreaks(tree, prevPendingExits);
+ alive = alive.or(resolveBreaks(tree, prevPendingExits));
}
@Override
@@ -644,9 +644,9 @@
}
}
boolean hasDefault = false;
- boolean prevAlive = alive;
+ Liveness prevAlive = alive;
for (List<JCCase> l = tree.cases; l.nonEmpty(); l = l.tail) {
- alive = true;
+ alive = Liveness.ALIVE;
JCCase c = l.head;
if (c.pats.isEmpty())
hasDefault = true;
@@ -662,13 +662,22 @@
}
}
scanStats(c.stats);
- c.completesNormally = alive;
+ if (alive == Liveness.ALIVE) {
+ if (c.caseKind == JCCase.RULE) {
+ log.error(TreeInfo.diagEndPos(c.body),
+ Errors.RuleCompletesNormally);
+ } else if (l.tail.isEmpty()) {
+ log.error(TreeInfo.diagEndPos(tree),
+ Errors.SwitchExpressionCompletesNormally);
+ }
+ }
+ c.completesNormally = alive != Liveness.DEAD;
}
if ((constants == null || !constants.isEmpty()) && !hasDefault) {
log.error(tree, Errors.NotExhaustive);
}
alive = prevAlive;
- alive |= resolveBreaks(tree, prevPendingExits);
+ alive = alive.or(resolveBreaks(tree, prevPendingExits));
}
public void visitTry(JCTry tree) {
@@ -686,22 +695,22 @@
}
scanStat(tree.body);
- boolean aliveEnd = alive;
+ Liveness aliveEnd = alive;
for (List<JCCatch> l = tree.catchers; l.nonEmpty(); l = l.tail) {
- alive = true;
+ alive = Liveness.ALIVE;
JCVariableDecl param = l.head.param;
scan(param);
scanStat(l.head.body);
- aliveEnd |= alive;
+ aliveEnd = aliveEnd.or(alive);
}
if (tree.finalizer != null) {
ListBuffer<PendingExit> exits = pendingExits;
pendingExits = prevPendingExits;
- alive = true;
+ alive = Liveness.ALIVE;
scanStat(tree.finalizer);
- tree.finallyCanCompleteNormally = alive;
- if (!alive) {
+ tree.finallyCanCompleteNormally = alive != Liveness.DEAD;
+ if (alive == Liveness.DEAD) {
if (lint.isEnabled(Lint.LintCategory.FINALLY)) {
log.warning(Lint.LintCategory.FINALLY,
TreeInfo.diagEndPos(tree.finalizer),
@@ -726,12 +735,12 @@
scan(tree.cond);
scanStat(tree.thenpart);
if (tree.elsepart != null) {
- boolean aliveAfterThen = alive;
- alive = true;
+ Liveness aliveAfterThen = alive;
+ alive = Liveness.ALIVE;
scanStat(tree.elsepart);
- alive = alive | aliveAfterThen;
+ alive = alive.or(aliveAfterThen);
} else {
- alive = true;
+ alive = Liveness.ALIVE;
}
}
@@ -776,12 +785,12 @@
}
ListBuffer<PendingExit> prevPending = pendingExits;
- boolean prevAlive = alive;
+ Liveness prevAlive = alive;
try {
pendingExits = new ListBuffer<>();
- alive = true;
+ alive = Liveness.ALIVE;
scanStat(tree.body);
- tree.canCompleteNormally = alive;
+ tree.canCompleteNormally = alive != Liveness.DEAD;
}
finally {
pendingExits = prevPending;
@@ -807,7 +816,7 @@
attrEnv = env;
Flow.this.make = make;
pendingExits = new ListBuffer<>();
- alive = true;
+ alive = Liveness.ALIVE;
scan(tree);
} finally {
pendingExits = null;
@@ -2776,4 +2785,58 @@
}
}
}
+
+ enum Liveness {
+ ALIVE {
+ @Override
+ public Liveness or(Liveness other) {
+ return this;
+ }
+ @Override
+ public Liveness and(Liveness other) {
+ return other;
+ }
+ },
+ DEAD {
+ @Override
+ public Liveness or(Liveness other) {
+ return other;
+ }
+ @Override
+ public Liveness and(Liveness other) {
+ return this;
+ }
+ },
+ RECOVERY {
+ @Override
+ public Liveness or(Liveness other) {
+ if (other == ALIVE) {
+ return ALIVE;
+ } else {
+ return this;
+ }
+ }
+ @Override
+ public Liveness and(Liveness other) {
+ if (other == DEAD) {
+ return DEAD;
+ } else {
+ return this;
+ }
+ }
+ };
+
+ public abstract Liveness or(Liveness other);
+ public abstract Liveness and(Liveness other);
+ public Liveness or(boolean value) {
+ return or(from(value));
+ }
+ public Liveness and(boolean value) {
+ return and(from(value));
+ }
+ public static Liveness from(boolean value) {
+ return value ? ALIVE : DEAD;
+ }
+ }
+
}
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java Wed Nov 21 15:05:21 2018 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/parser/JavacParser.java Wed Nov 21 15:22:57 2018 +0100
@@ -1392,6 +1392,7 @@
case RBRACE: case EOF:
JCSwitchExpression e = to(F.at(switchPos).SwitchExpression(selector,
cases.toList()));
+ e.endpos = token.pos;
accept(RBRACE);
return e;
default:
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties Wed Nov 21 15:05:21 2018 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/resources/compiler.properties Wed Nov 21 15:22:57 2018 +0100
@@ -200,6 +200,14 @@
compiler.err.return.outside.switch.expression=\
return outside of enclosing switch expression
+compiler.err.rule.completes.normally=\
+ switch rule completes without providing a value\n\
+ (switch rules in switch expressions must either provide a value or throw)
+
+compiler.err.switch.expression.completes.normally=\
+ switch expression completes without providing a value\n\
+ (switch expressions must either provide a value or throw for all possible input values)
+
# 0: name
compiler.err.break.ambiguous.target=\
ambiguous reference to ''{0}''\n\
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java Wed Nov 21 15:05:21 2018 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/JCTree.java Wed Nov 21 15:22:57 2018 +0100
@@ -1305,6 +1305,8 @@
public static class JCSwitchExpression extends JCPolyExpression implements SwitchExpressionTree {
public JCExpression selector;
public List<JCCase> cases;
+ /** Position of closing brace, optional. */
+ public int endpos = Position.NOPOS;
protected JCSwitchExpression(JCExpression selector, List<JCCase> cases) {
this.selector = selector;
this.cases = cases;
--- a/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java Wed Nov 21 15:05:21 2018 +0100
+++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/tree/TreeInfo.java Wed Nov 21 15:22:57 2018 +0100
@@ -383,6 +383,9 @@
JCTry t = (JCTry) tree;
return endPos((t.finalizer != null) ? t.finalizer
: (t.catchers.nonEmpty() ? t.catchers.last().body : t.body));
+ } else if (tree.hasTag(SWITCH_EXPRESSION) &&
+ ((JCSwitchExpression) tree).endpos != Position.NOPOS) {
+ return ((JCSwitchExpression) tree).endpos;
} else
return tree.pos;
}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/diags/examples/RuleCompletesNormally.java Wed Nov 21 15:22:57 2018 +0100
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+// key: compiler.err.rule.completes.normally
+// key: compiler.note.preview.filename
+// key: compiler.note.preview.recompile
+// options: --enable-preview -source 12
+
+class RuleCompletesNormally {
+ public String convert(int i) {
+ return switch (i) {
+ default -> {}
+ };
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/diags/examples/SwitchExpressionCompletesNormally.java Wed Nov 21 15:22:57 2018 +0100
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
+ *
+ * This code is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+// key: compiler.err.switch.expression.completes.normally
+// key: compiler.note.preview.filename
+// key: compiler.note.preview.recompile
+// options: --enable-preview -source 12
+
+class SwitchExpressionCompletesNormally {
+ public String convert(int i) {
+ return switch (i) {
+ default:
+ };
+ }
+}
--- a/test/langtools/tools/javac/expswitch/ExpSwitchNestingTest.java Wed Nov 21 15:05:21 2018 +0100
+++ b/test/langtools/tools/javac/expswitch/ExpSwitchNestingTest.java Wed Nov 21 15:22:57 2018 +0100
@@ -46,7 +46,7 @@
private static final String ESWITCH_S = "String res_string = switch (x) { case 0 -> { # } default -> \"default\"; };";
private static final String INT_FN_ESWITCH = "java.util.function.IntSupplier r = switch (x) { case 0 -> { # } default -> null; };";
private static final String INT_ESWITCH_DEFAULT = "int res = switch (x) { default -> { # } };";
- private static final String IF = "if (cond) { # }";
+ private static final String IF = "if (cond) { # } else throw new RuntimeException();";
private static final String BLOCK = "{ # }";
private static final String BREAK_Z = "break 0;";
private static final String BREAK_S = "break \"hello world\";";
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/switchexpr/ExpressionSwitchFlow.java Wed Nov 21 15:22:57 2018 +0100
@@ -0,0 +1,66 @@
+/*
+ * @test /nodynamiccopyright/
+ * @bug 8212982
+ * @summary Verify a compile-time error is produced if switch expression does not provide a value
+ * @compile/fail/ref=ExpressionSwitchFlow.out --enable-preview -source 12 -XDrawDiagnostics ExpressionSwitchFlow.java
+ */
+
+public class ExpressionSwitchFlow {
+ private String test1(int i) {
+ return switch (i) {
+ case 0 -> {}
+ default -> { break "other"; }
+ };
+ }
+ private String test2(int i) {
+ return switch (i) {
+ case 0 -> {
+ }
+ default -> "other";
+ };
+ }
+ private String test3(int i) {
+ return switch (i) {
+ case 0 -> {}
+ default -> throw new IllegalStateException();
+ };
+ }
+ private String test4(int i) {
+ return switch (i) {
+ case 0 -> { break "other"; }
+ default -> {}
+ };
+ }
+ private String test5(int i) {
+ return switch (i) {
+ case 0 -> "other";
+ default -> {}
+ };
+ }
+ private String test6(int i) {
+ return switch (i) {
+ case 0 -> throw new IllegalStateException();
+ default -> {}
+ };
+ }
+ private String test7(int i) {
+ return switch (i) {
+ case 0: throw new IllegalStateException();
+ default:
+ };
+ }
+ private String test8(int i) {
+ return switch (i) {
+ case 0: i++;
+ default: {
+ }
+ };
+ }
+ private String test9(int i) {
+ return switch (i) {
+ case 0:
+ default:
+ System.err.println();
+ };
+ }
+}
--- /dev/null Thu Jan 01 00:00:00 1970 +0000
+++ b/test/langtools/tools/javac/switchexpr/ExpressionSwitchFlow.out Wed Nov 21 15:22:57 2018 +0100
@@ -0,0 +1,12 @@
+ExpressionSwitchFlow.java:11:24: compiler.err.rule.completes.normally
+ExpressionSwitchFlow.java:18:13: compiler.err.rule.completes.normally
+ExpressionSwitchFlow.java:24:24: compiler.err.rule.completes.normally
+ExpressionSwitchFlow.java:31:25: compiler.err.rule.completes.normally
+ExpressionSwitchFlow.java:37:25: compiler.err.rule.completes.normally
+ExpressionSwitchFlow.java:43:25: compiler.err.rule.completes.normally
+ExpressionSwitchFlow.java:50:9: compiler.err.switch.expression.completes.normally
+ExpressionSwitchFlow.java:57:9: compiler.err.switch.expression.completes.normally
+ExpressionSwitchFlow.java:64:9: compiler.err.switch.expression.completes.normally
+- compiler.note.preview.filename: ExpressionSwitchFlow.java
+- compiler.note.preview.recompile
+9 errors