Skip to content
This repository has been archived by the owner. It is now read-only.
Permalink
Browse files
8269146: Missing unreported constraints on pattern and other case lab…
…el combination

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

Reviewed-by: mcimadamore
  • Loading branch information
Jan Lahoda committed Jul 9, 2021
1 parent 62ff55d commit 885f7b1141d1d8e6b560ebaf0c2d4878be0ea8ba
@@ -1684,7 +1684,6 @@ private void handleSwitch(JCTree switchTree,
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 @@ private void handleSwitch(JCTree switchTree,
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 @@ private void handleSwitch(JCTree switchTree,
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 @@ private void handleSwitch(JCTree switchTree,
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 @@ private void handleSwitch(JCTree switchTree,
}
}
currentBindings = matchBindingsComputer.switchCase(pat, currentBindings, matchBindings);
prevCompletedNormally = !TreeInfo.isNull(pat);
}
Env<AttrContext> caseEnv =
bindingEnv(switchEnv, c, currentBindings.bindingsWhenTrue);
@@ -1805,12 +1797,13 @@ private void handleSwitch(JCTree switchTree,
}
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.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 @@ void checkModuleRequires(final DiagnosticPosition pos, final RequiresDirective r
}
}

/**
* 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 void visitSwitch(JCSwitch tree) {
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 void visitSwitchExpression(JCSwitchExpression tree) {
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 MatchBindings andOperation(DiagnosticPosition pos, MatchBindings lhsBindi
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 @@ Object guardWithMatchingExpression(Object o1, Object o2) {
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;
Loading

1 comment on commit 885f7b1

@openjdk-notifier

This comment has been minimized.

Copy link

@openjdk-notifier openjdk-notifier bot commented on 885f7b1 Jul 9, 2021

Please sign in to comment.