8269146: Missing unreported constraints on pattern and other case label combination

8269301: Switch statement with a pattern, constant and default label elements crash javac

Reviewed-by: mcimadamore
This commit is contained in:
Jan Lahoda 2021-07-09 08:03:56 +00:00
parent 62ff55d383
commit 885f7b1141
8 changed files with 306 additions and 29 deletions
src/jdk.compiler/share/classes/com/sun/tools/javac/comp
test/langtools/tools/javac/patterns

@ -1684,7 +1684,6 @@ public class Attr extends JCTree.Visitor {
CaseTree.CaseKind caseKind = null;
boolean wasError = false;
MatchBindings prevBindings = null;
boolean prevCompletedNormally = false;
for (List<JCCase> l = cases; l.nonEmpty(); l = l.tail) {
JCCase c = l.head;
if (caseKind == null) {
@ -1702,9 +1701,9 @@ public class Attr extends JCTree.Visitor {
if (TreeInfo.isNull(expr)) {
preview.checkSourceLevel(expr.pos(), Feature.CASE_NULL);
if (hasNullPattern) {
log.error(c.pos(), Errors.DuplicateCaseLabel);
log.error(pat.pos(), Errors.DuplicateCaseLabel);
} else if (wasTotalPattern) {
log.error(c.pos(), Errors.PatternDominated);
log.error(pat.pos(), Errors.PatternDominated);
}
hasNullPattern = true;
attribExpr(expr, switchEnv, seltype);
@ -1714,7 +1713,7 @@ public class Attr extends JCTree.Visitor {
if (sym == null) {
log.error(expr.pos(), Errors.EnumLabelMustBeUnqualifiedEnum);
} else if (!labels.add(sym)) {
log.error(c.pos(), Errors.DuplicateCaseLabel);
log.error(pat.pos(), Errors.DuplicateCaseLabel);
} else {
checkCaseLabelDominated(pat.pos(), coveredTypes, sym.type);
}
@ -1758,16 +1757,10 @@ public class Attr extends JCTree.Visitor {
log.error(pat.pos(), Errors.DuplicateDefaultLabel);
} else if (hasTotalPattern) {
log.error(pat.pos(), Errors.TotalPatternAndDefault);
} else if (matchBindings.bindingsWhenTrue.nonEmpty()) {
//there was a pattern, and the execution flows into a default:
log.error(pat.pos(), Errors.FlowsThroughFromPattern);
}
hasDefault = true;
matchBindings = MatchBindingsComputer.EMPTY;
} else {
if (prevCompletedNormally) {
log.error(pat.pos(), Errors.FlowsThroughToPattern);
}
//binding pattern
attribExpr(pat, switchEnv);
var primary = TreeInfo.primaryPatternType((JCPattern) pat);
@ -1794,7 +1787,6 @@ public class Attr extends JCTree.Visitor {
}
}
currentBindings = matchBindingsComputer.switchCase(pat, currentBindings, matchBindings);
prevCompletedNormally = !TreeInfo.isNull(pat);
}
Env<AttrContext> caseEnv =
bindingEnv(switchEnv, c, currentBindings.bindingsWhenTrue);
@ -1805,12 +1797,13 @@ public class Attr extends JCTree.Visitor {
}
addVars(c.stats, switchEnv.info.scope);
boolean completesNormally = c.caseKind == CaseTree.CaseKind.STATEMENT ? flow.aliveAfter(caseEnv, c, make) : false;
prevBindings = completesNormally ? currentBindings : null;
prevCompletedNormally =
completesNormally &&
!(c.labels.size() == 1 &&
TreeInfo.isNull(c.labels.head) && c.stats.isEmpty());
c.completesNormally = flow.aliveAfter(caseEnv, c, make);
prevBindings = c.caseKind == CaseTree.CaseKind.STATEMENT && c.completesNormally ? currentBindings
: null;
}
if (patternSwitch) {
chk.checkSwitchCaseStructure(cases);
}
if (switchTree.hasTag(SWITCH)) {
((JCSwitch) switchTree).hasTotalPattern = hasDefault || hasTotalPattern;

@ -33,6 +33,7 @@ import javax.lang.model.element.ElementKind;
import javax.lang.model.element.NestingKind;
import javax.tools.JavaFileManager;
import com.sun.source.tree.CaseTree;
import com.sun.tools.javac.code.*;
import com.sun.tools.javac.code.Attribute.Compound;
import com.sun.tools.javac.code.Directive.ExportsDirective;
@ -4270,4 +4271,66 @@ public class Check {
}
}
/**
* Verify the case labels conform to the constraints. Checks constraints related
* combinations of patterns and other labels.
*
* @param cases the cases that should be checked.
*/
void checkSwitchCaseStructure(List<JCCase> cases) {
boolean wasConstant = false; // Seen a constant in the same case label
boolean wasDefault = false; // Seen a default in the same case label
boolean wasNullPattern = false; // Seen a null pattern in the same case label,
//or fall through from a null pattern
boolean wasPattern = false; // Seen a pattern in the same case label
//or fall through from a pattern
boolean wasTypePattern = false; // Seen a pattern in the same case label
//or fall through from a type pattern
boolean wasNonEmptyFallThrough = false;
for (List<JCCase> l = cases; l.nonEmpty(); l = l.tail) {
JCCase c = l.head;
for (JCCaseLabel pat : c.labels) {
if (pat.isExpression()) {
JCExpression expr = (JCExpression) pat;
if (TreeInfo.isNull(expr)) {
if (wasPattern && !wasTypePattern && !wasNonEmptyFallThrough) {
log.error(pat.pos(), Errors.FlowsThroughFromPattern);
}
wasNullPattern = true;
} else {
if (wasPattern && !wasNonEmptyFallThrough) {
log.error(pat.pos(), Errors.FlowsThroughFromPattern);
}
wasConstant = true;
}
} else if (pat.hasTag(DEFAULTCASELABEL)) {
if (wasPattern && !wasNonEmptyFallThrough) {
log.error(pat.pos(), Errors.FlowsThroughFromPattern);
}
wasDefault = true;
} else {
boolean isTypePattern = pat.hasTag(BINDINGPATTERN);
if (wasPattern || wasConstant || wasDefault ||
(wasNullPattern && (!isTypePattern || wasNonEmptyFallThrough))) {
log.error(pat.pos(), Errors.FlowsThroughToPattern);
}
wasPattern = true;
wasTypePattern = isTypePattern;
}
}
boolean completesNormally = c.caseKind == CaseTree.CaseKind.STATEMENT ? c.completesNormally
: false;
if (c.stats.nonEmpty()) {
wasConstant = false;
wasDefault = false;
wasNullPattern &= completesNormally;
wasPattern &= completesNormally;
wasTypePattern &= completesNormally;
}
wasNonEmptyFallThrough = c.stats.nonEmpty() && completesNormally;
}
}
}

@ -677,7 +677,6 @@ public class Flow {
handleConstantCaseLabel(constants, pat);
}
scanStats(c.stats);
c.completesNormally = alive != Liveness.DEAD;
if (alive != Liveness.DEAD && c.caseKind == JCCase.RULE) {
scanSyntheticBreak(make, tree);
alive = Liveness.DEAD;
@ -725,7 +724,6 @@ public class Flow {
Errors.SwitchExpressionCompletesNormally);
}
}
c.completesNormally = alive != Liveness.DEAD;
}
if (!tree.hasTotalPattern && !TreeInfo.isErrorEnumSwitch(tree.selector, tree.cases) &&
!isExhaustive(tree.selector.type, constants)) {

@ -129,9 +129,6 @@ public class MatchBindingsComputer extends TreeScanner {
public MatchBindings switchCase(JCTree tree, MatchBindings prevBindings, MatchBindings currentBindings) {
if (prevBindings == null)
return currentBindings;
if (!prevBindings.bindingsWhenTrue.isEmpty() && !currentBindings.bindingsWhenTrue.isEmpty()) {
log.error(tree.pos(), Errors.FlowsThroughToPattern);
}
if (prevBindings.nullPattern) {
return currentBindings;
}

@ -0,0 +1,160 @@
/*
* Copyright (c) 2021, 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.
*/
/*
* @test
* @bug 8269146
* @summary Check compilation outcomes for various combinations of case label element.
* @library /tools/lib /tools/javac/lib
* @modules
* jdk.compiler/com.sun.tools.javac.api
* jdk.compiler/com.sun.tools.javac.file
* jdk.compiler/com.sun.tools.javac.main
* jdk.compiler/com.sun.tools.javac.util
* @build toolbox.ToolBox toolbox.JavacTask
* @build combo.ComboTestHelper
* @compile CaseStructureTest.java
* @run main CaseStructureTest
*/
import combo.ComboInstance;
import combo.ComboParameter;
import combo.ComboTask;
import combo.ComboTestHelper;
import java.util.Arrays;
import java.util.stream.Collectors;
import toolbox.ToolBox;
public class CaseStructureTest extends ComboInstance<CaseStructureTest> {
private static final String JAVA_VERSION = System.getProperty("java.specification.version");
protected ToolBox tb;
CaseStructureTest() {
super();
tb = new ToolBox();
}
public static void main(String... args) throws Exception {
new ComboTestHelper<CaseStructureTest>()
.withDimension("AS_CASE_LABEL_ELEMENTS", (x, asCaseLabelElements) -> x.asCaseLabelElements = asCaseLabelElements, true, false)
.withArrayDimension("CASE_LABELS", (x, caseLabels, idx) -> x.caseLabels[idx] = caseLabels, DIMENSIONS, CaseLabel.values())
.withFilter(t -> Arrays.stream(t.caseLabels).anyMatch(l -> l != CaseLabel.NONE))
.withFailMode(ComboTestHelper.FailMode.FAIL_FAST)
.run(CaseStructureTest::new);
}
private static final int DIMENSIONS = 4;
private boolean asCaseLabelElements;
private CaseLabel[] caseLabels = new CaseLabel[DIMENSIONS];
private static final String MAIN_TEMPLATE =
"""
public class Test {
public static void doTest(Integer in) {
switch (in) {
case -1: break;
#{CASES}
#{DEFAULT}
}
}
}
""";
@Override
protected void doWork() throws Throwable {
String labelSeparator = asCaseLabelElements ? ", " : ": case ";
String labels = Arrays.stream(caseLabels).filter(l -> l != CaseLabel.NONE).map(l -> l.code).collect(Collectors.joining(labelSeparator, "case ", ": break;"));
boolean hasDefault = Arrays.stream(caseLabels).anyMatch(l -> l == CaseLabel.DEFAULT || l == CaseLabel.TYPE_PATTERN || l == CaseLabel.PARENTHESIZED_PATTERN);
ComboTask task = newCompilationTask()
.withSourceFromTemplate(MAIN_TEMPLATE.replace("#{CASES}", labels).replace("#{DEFAULT}", hasDefault ? "" : "default: break;"))
.withOption("--enable-preview")
.withOption("-source").withOption(JAVA_VERSION);
task.generate(result -> {
boolean shouldPass = true;
long patternCases = Arrays.stream(caseLabels).filter(l -> l == CaseLabel.TYPE_PATTERN || l == CaseLabel.GUARDED_PATTERN || l == CaseLabel.PARENTHESIZED_PATTERN).count();
long typePatternCases = Arrays.stream(caseLabels).filter(l -> l == CaseLabel.TYPE_PATTERN).count();
long constantCases = Arrays.stream(caseLabels).filter(l -> l == CaseLabel.CONSTANT).count();
long nullCases = Arrays.stream(caseLabels).filter(l -> l == CaseLabel.NULL).count();
long defaultCases = Arrays.stream(caseLabels).filter(l -> l == CaseLabel.DEFAULT).count();
if (constantCases > 1) {
shouldPass &= false;
}
if (constantCases > 0) {
shouldPass &= patternCases == 0;
}
if (defaultCases > 1) {
shouldPass &= false;
}
if (nullCases > 1) {
shouldPass &= false;
}
if (nullCases > 0 && patternCases > 0) {
shouldPass &= patternCases == typePatternCases;
}
if (patternCases > 1) {
shouldPass &= false;
}
if (patternCases > 0 && defaultCases > 0) {
shouldPass &= false;
}
if (!asCaseLabelElements) {
//as an edge case, `case <total-pattern>: case null:` is prohibited:
boolean seenPattern = false;
for (CaseLabel label : caseLabels) {
switch (label) {
case NULL: if (seenPattern) shouldPass = false; break;
case GUARDED_PATTERN, PARENTHESIZED_PATTERN, TYPE_PATTERN: seenPattern = true; break;
}
}
}
if (!(shouldPass ^ result.hasErrors())) {
throw new AssertionError("Unexpected result: shouldPass=" + shouldPass + ", actual: " + !result.hasErrors() + ", info: " + result.compilationInfo());
}
});
}
public enum CaseLabel implements ComboParameter {
NONE(""),
TYPE_PATTERN("Integer i"),
PARENTHESIZED_PATTERN("(Integer i)"),
GUARDED_PATTERN("Integer i && i > 0"),
CONSTANT("1"),
NULL("null"),
DEFAULT("default");
private final String code;
private CaseLabel(String code) {
this.code = code;
}
@Override
public String expand(String optParameter) {
throw new UnsupportedOperationException("Not supported.");
}
}
}

@ -1,6 +1,6 @@
/*
* @test /nodynamiccopyright/
* @bug 8262891
* @bug 8262891 8269146
* @summary Verify errors related to pattern switches.
* @compile/fail/ref=SwitchErrors.out --enable-preview -source ${jdk.version} -XDrawDiagnostics -XDshould-stop.at=FLOW SwitchErrors.java
*/
@ -185,6 +185,38 @@ public class SwitchErrors {
default -> null;
};
}
void test8269146a(Integer i) {
switch (i) {
//error - illegal combination of pattern and constant:
case Integer o && o != null, 1:
break;
default:
break;
}
}
void test8269146b(Integer i) {
switch (i) {
//error - illegal combination of null and pattern other than type pattern:
case null, Integer o && o != null:
break;
default:
break;
}
}
void test8269146c(Integer i) {
switch (i) {
//error - illegal combination of pattern and default:
case Integer o, default:
break;
}
}
void test8269301(Integer i) {
switch (i) {
//error - illegal combination of pattern, constant and default
case Integer o && o != null, 1, default:
break;
}
}
void exhaustiveAndNull(String s) {
switch (s) {
case null: break;

@ -6,15 +6,15 @@ SwitchErrors.java:28:18: compiler.err.prob.found.req: (compiler.misc.inconvertib
SwitchErrors.java:29:18: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: java.lang.String, int)
SwitchErrors.java:30:18: compiler.err.prob.found.req: (compiler.misc.inconvertible.types: int, java.lang.CharSequence)
SwitchErrors.java:36:20: compiler.err.total.pattern.and.default
SwitchErrors.java:42:13: compiler.err.pattern.dominated
SwitchErrors.java:42:18: compiler.err.pattern.dominated
SwitchErrors.java:42:24: compiler.err.total.pattern.and.default
SwitchErrors.java:48:18: compiler.err.total.pattern.and.default
SwitchErrors.java:54:18: compiler.err.duplicate.total.pattern
SwitchErrors.java:60:20: compiler.err.duplicate.default.label
SwitchErrors.java:66:20: compiler.err.duplicate.default.label
SwitchErrors.java:71:27: compiler.err.duplicate.default.label
SwitchErrors.java:77:13: compiler.err.duplicate.case.label
SwitchErrors.java:82:13: compiler.err.duplicate.case.label
SwitchErrors.java:77:18: compiler.err.duplicate.case.label
SwitchErrors.java:82:24: compiler.err.duplicate.case.label
SwitchErrors.java:87:28: compiler.err.flows.through.to.pattern
SwitchErrors.java:93:18: compiler.err.flows.through.to.pattern
SwitchErrors.java:100:18: compiler.err.flows.through.to.pattern
@ -27,10 +27,15 @@ SwitchErrors.java:137:28: compiler.err.flows.through.from.pattern
SwitchErrors.java:143:18: compiler.err.flows.through.from.pattern
SwitchErrors.java:148:27: compiler.err.flows.through.to.pattern
SwitchErrors.java:154:18: compiler.err.flows.through.to.pattern
SwitchErrors.java:160:13: compiler.err.pattern.dominated
SwitchErrors.java:160:18: compiler.err.pattern.dominated
SwitchErrors.java:172:18: compiler.err.pattern.expected
SwitchErrors.java:178:76: compiler.err.cant.resolve.location: kindname.variable, n, , , (compiler.misc.location: kindname.class, SwitchErrors, null)
SwitchErrors.java:184:71: compiler.err.cant.resolve.location: kindname.variable, n, , , (compiler.misc.location: kindname.class, SwitchErrors, null)
SwitchErrors.java:191:42: compiler.err.flows.through.from.pattern
SwitchErrors.java:200:24: compiler.err.flows.through.to.pattern
SwitchErrors.java:209:29: compiler.err.total.pattern.and.default
SwitchErrors.java:216:42: compiler.err.flows.through.from.pattern
SwitchErrors.java:216:45: compiler.err.flows.through.from.pattern
SwitchErrors.java:9:9: compiler.err.not.exhaustive.statement
SwitchErrors.java:15:9: compiler.err.not.exhaustive.statement
SwitchErrors.java:21:9: compiler.err.not.exhaustive.statement
@ -42,7 +47,7 @@ SwitchErrors.java:91:9: compiler.err.not.exhaustive.statement
SwitchErrors.java:97:9: compiler.err.not.exhaustive.statement
SwitchErrors.java:104:9: compiler.err.not.exhaustive.statement
SwitchErrors.java:164:9: compiler.err.not.exhaustive.statement
SwitchErrors.java:189:9: compiler.err.not.exhaustive.statement
SwitchErrors.java:221:9: compiler.err.not.exhaustive.statement
- compiler.note.preview.filename: SwitchErrors.java, DEFAULT
- compiler.note.preview.recompile
45 errors
50 errors

@ -67,6 +67,8 @@ public class Switches {
runStringWithConstant(this::testStringWithConstantExpression);
runFallThrough(this::testFallThroughStatement);
runFallThrough(this::testFallThroughExpression);
runFallThrough(this::testFallThrough2Statement);
runFallThrough(this::testFallThrough2Expression);
npeTest(this::npeTestStatement);
npeTest(this::npeTestExpression);
exhaustiveStatementSane("");
@ -397,6 +399,33 @@ public class Switches {
return r;
}
Integer testFallThrough2Statement(Integer i) {
int r = 0;
switch (i) {
case Integer o && o != null:
r = 1;
case -1: r = 1;
case null, default:
r = 2;
}
return r;
}
Integer testFallThrough2Expression(Integer i) {
int r = switch (i) {
case Integer o && o != null:
r = 1;
case -1: r = 1;
case null, default:
r = 2;
yield r;
};
return r;
}
void npeTestStatement(I i) {
switch (i) {
case A a -> {}