stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8289196: Pattern domination not working properly for record patterns

Browse Source 
Reviewed-by: vromero
This commit is contained in:
Jan Lahoda 2022-07-07 07:54:18 +00:00
parent 8f24d25168
commit 8dd94a2c14
5 changed files with 176 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Attr.java
View File

@ -1694,9 +1694,7 @@ public class Attr extends JCTree.Visitor {
// Attribute all cases and
// check that there are no duplicate case labels or default clauses.
Set<Object> labels = new HashSet<>(); // The set of case labels.
List<Type> coveredTypesForPatterns = List.nil();
List<Type> coveredTypesForConstants = List.nil();
Set<Object> constants = new HashSet<>(); // The set of case constants.
boolean hasDefault = false; // Is there a default label?
boolean hasUnconditionalPattern = false; // Is there a unconditional pattern?
boolean lastPatternErroneous = false; // Has the last pattern erroneous type?
@ -1732,10 +1730,8 @@ public class Attr extends JCTree.Visitor {
Symbol sym = enumConstant(expr, seltype);
if (sym == null) {
log.error(expr.pos(), Errors.EnumLabelMustBeUnqualifiedEnum);
} else if (!labels.add(sym)) {
} else if (!constants.add(sym)) {
log.error(label.pos(), Errors.DuplicateCaseLabel);
} else {
checkCaseLabelDominated(label.pos(), coveredTypesForConstants, sym.type);
}
} else if (errorEnumSwitch) {
//error recovery: the selector is erroneous, and all the case labels
@ -1765,10 +1761,8 @@ public class Attr extends JCTree.Visitor {
}
} else if (!stringSwitch && !types.isAssignable(seltype, syms.intType)) {
log.error(label.pos(), Errors.ConstantLabelNotCompatible(pattype, seltype));
} else if (!labels.add(pattype.constValue())) {
} else if (!constants.add(pattype.constValue())) {
log.error(c.pos(), Errors.DuplicateCaseLabel);
} else {
checkCaseLabelDominated(label.pos(), coveredTypesForConstants, types.boxedTypeOrType(pattype));
}
}
}
@ -1820,13 +1814,6 @@ public class Attr extends JCTree.Visitor {
hasUnconditionalPattern = true;
}
lastPatternErroneous = patternType.isErroneous();
checkCaseLabelDominated(pat.pos(), coveredTypesForPatterns, patternType);
if (!patternType.isErroneous()) {
coveredTypesForConstants = coveredTypesForConstants.prepend(patternType);
if (unguarded) {
coveredTypesForPatterns = coveredTypesForPatterns.prepend(patternType);
}
}
} else {
Assert.error();
}
@ -1850,6 +1837,7 @@ public class Attr extends JCTree.Visitor {
}
if (patternSwitch) {
chk.checkSwitchCaseStructure(cases);
chk.checkSwitchCaseLabelDominated(cases);
}
if (switchTree.hasTag(SWITCH)) {
((JCSwitch) switchTree).hasUnconditionalPattern =
@ -1875,14 +1863,6 @@ public class Attr extends JCTree.Visitor {
switchScope.enter(((JCVariableDecl) stat).sym);
}
}
private void checkCaseLabelDominated(DiagnosticPosition pos,
List<Type> coveredTypes, Type patternType) {
for (Type existing : coveredTypes) {
if (types.isSubtype(patternType, existing)) {
log.error(pos, Errors.PatternDominated);
}
}
}
// where
/** Return the selected enumeration constant symbol, or null. */
private Symbol enumConstant(JCTree tree, Type enumType) {

87
src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Check.java
View File

@ -4373,6 +4373,93 @@ public class Check {
}
}
void checkSwitchCaseLabelDominated(List<JCCase> cases) {
List<JCCaseLabel> caseLabels = List.nil();
for (List<JCCase> l = cases; l.nonEmpty(); l = l.tail) {
JCCase c = l.head;
for (JCCaseLabel label : c.labels) {
if (label.hasTag(DEFAULTCASELABEL) || TreeInfo.isNullCaseLabel(label)) {
continue;
}
Type currentType = labelType(label);
for (JCCaseLabel testCaseLabel : caseLabels) {
Type testType = labelType(testCaseLabel);
if (types.isSubtype(currentType, testType) &&
!currentType.hasTag(ERROR) && !testType.hasTag(ERROR)) {
//the current label is potentially dominated by the existing (test) label, check:
boolean dominated = false;
if (label instanceof JCConstantCaseLabel) {
dominated |= !(testCaseLabel instanceof JCConstantCaseLabel);
} else if (label instanceof JCPatternCaseLabel patternCL &&
testCaseLabel instanceof JCPatternCaseLabel testPatternCaseLabel &&
TreeInfo.unguardedCaseLabel(testCaseLabel)) {
dominated = patternDominated(testPatternCaseLabel.pat,
patternCL.pat);
}
if (dominated) {
log.error(label.pos(), Errors.PatternDominated);
}
}
}
caseLabels = caseLabels.prepend(label);
}
}
}
//where:
private Type labelType(JCCaseLabel label) {
return types.erasure(switch (label.getTag()) {
case PATTERNCASELABEL -> ((JCPatternCaseLabel) label).pat.type;
case CONSTANTCASELABEL -> types.boxedTypeOrType(((JCConstantCaseLabel) label).expr.type);
default -> throw Assert.error("Unexpected tree kind: " + label.getTag());
});
}
private boolean patternDominated(JCPattern existingPattern, JCPattern currentPattern) {
Type existingPatternType = types.erasure(existingPattern.type);
Type currentPatternType = types.erasure(currentPattern.type);
if (existingPatternType.isPrimitive() ^ currentPatternType.isPrimitive()) {
return false;
}
if (existingPatternType.isPrimitive()) {
return types.isSameType(existingPatternType, currentPatternType);
} else {
if (!types.isSubtype(currentPatternType, existingPatternType)) {
return false;
}
}
while (existingPattern instanceof JCParenthesizedPattern parenthesized) {
existingPattern = parenthesized.pattern;
}
while (currentPattern instanceof JCParenthesizedPattern parenthesized) {
currentPattern = parenthesized.pattern;
}
if (currentPattern instanceof JCBindingPattern) {
return existingPattern instanceof JCBindingPattern;
} else if (currentPattern instanceof JCRecordPattern currentRecordPattern) {
if (existingPattern instanceof JCBindingPattern) {
return true;
} else if (existingPattern instanceof JCRecordPattern existingRecordPattern) {
List<JCPattern> existingNested = existingRecordPattern.nested;
List<JCPattern> currentNested = currentRecordPattern.nested;
if (existingNested.size() != currentNested.size()) {
return false;
}
while (existingNested.nonEmpty()) {
if (!patternDominated(existingNested.head, currentNested.head)) {
return false;
}
existingNested = existingNested.tail;
currentNested = currentNested.tail;
}
return true;
} else {
Assert.error("Unknown pattern: " + existingPattern.getTag());
}
} else {
Assert.error("Unknown pattern: " + currentPattern.getTag());
}
return false;
}
/** check if a type is a subtype of Externalizable, if that is available. */
boolean isExternalizable(Type t) {
try {

75
test/langtools/tools/javac/patterns/Domination.java
View File

@ -136,4 +136,79 @@ public class Domination {
}
}
int testRecordPatternsDominated1() {
record R(int a) {}
Object o = null;
switch (o) {
case R r: return 1;
case R(int a): return -1;
}
}
int testRecordPatternsDominated2() {
record R(int a) {}
Object o = null;
switch (o) {
case R(int a): return 1;
case R(int a): return -1;
}
}
int testRecordPatternsDominated3() {
record R(int a) {}
Object o = null;
switch (o) {
case R r when guard(): return 1;
case R(int a): return -1;
}
}
int testRecordPatternsDominated4() {
record R(int a) {}
Object o = null;
switch (o) {
case R(int a) when guard(): return 1;
case R(int a): return -1;
}
}
boolean guard() {
return false;
}
int testRecordPatternsDominated5() {
record R(int a) {}
Object o = null;
switch (o) {
case ((R r)): return 1;
case ((R(int a))): return -1;
}
}
int testRecordPatternsDominated6() {
record R(int a) {}
Object o = null;
switch (o) {
case ((R(int a))): return 1;
case ((R(int a))): return -1;
}
}
int testRecordPatternsDominated7() {
record R(int a) {}
Object o = null;
switch (o) {
case R r when true: return 1;
case R(int a): return -1;
}
}
int testRecordPatternsDominated8() {
record R(int a) {}
Object o = null;
switch (o) {
case R(int a) when true: return 1;
case R(int a): return -1;
}
}
}

8
test/langtools/tools/javac/patterns/Domination.out
View File

@ -10,6 +10,12 @@ Domination.java:102:18: compiler.err.pattern.dominated
Domination.java:113:18: compiler.err.pattern.dominated
Domination.java:124:18: compiler.err.pattern.dominated
Domination.java:135:18: compiler.err.pattern.dominated
Domination.java:144:18: compiler.err.pattern.dominated
Domination.java:153:18: compiler.err.pattern.dominated
Domination.java:184:18: compiler.err.pattern.dominated
Domination.java:193:18: compiler.err.pattern.dominated
Domination.java:202:18: compiler.err.pattern.dominated
Domination.java:211:18: compiler.err.pattern.dominated
- compiler.note.preview.filename: Domination.java, DEFAULT
- compiler.note.preview.recompile
12 errors
18 errors

6
test/langtools/tools/javac/patterns/SwitchErrors.out
View File

@ -32,11 +32,11 @@ SwitchErrors.java:172:18: compiler.err.pattern.expected
SwitchErrors.java:178:78: compiler.err.cant.resolve.location: kindname.variable, n, , , (compiler.misc.location: kindname.class, SwitchErrors, null)
SwitchErrors.java:184:73: compiler.err.cant.resolve.location: kindname.variable, n, , , (compiler.misc.location: kindname.class, SwitchErrors, null)
SwitchErrors.java:191:21: compiler.err.flows.through.to.pattern
SwitchErrors.java:200:44: compiler.err.pattern.dominated
SwitchErrors.java:200:44: compiler.err.flows.through.from.pattern
SwitchErrors.java:218:29: compiler.err.unconditional.pattern.and.default
SwitchErrors.java:225:21: compiler.err.flows.through.to.pattern
SwitchErrors.java:225:47: compiler.err.flows.through.from.pattern
SwitchErrors.java:232:44: compiler.err.pattern.dominated
SwitchErrors.java:232:44: compiler.err.flows.through.from.pattern
SwitchErrors.java:232:47: compiler.err.flows.through.from.pattern
SwitchErrors.java:244:18: compiler.err.duplicate.unconditional.pattern
SwitchErrors.java:249:18: compiler.err.prob.found.req: (compiler.misc.not.applicable.types: int, java.lang.Integer)
@ -55,4 +55,4 @@ SwitchErrors.java:164:9: compiler.err.not.exhaustive.statement
SwitchErrors.java:237:9: compiler.err.not.exhaustive.statement
- compiler.note.preview.filename: SwitchErrors.java, DEFAULT
- compiler.note.preview.recompile
55 errors
55 errors