Skip to content

Commit 6d9da7c

Browse files
lahodajlahodaj
committed
8311815: Incorrect exhaustivity computation
Reviewed-by: vromero Backport-of: a441216
1 parent 48760d7 commit 6d9da7c

File tree

2 files changed

+78
-47
lines changed

2 files changed

+78
-47
lines changed

‎src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java‎

Lines changed: 48 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -773,15 +773,16 @@ private boolean exhausts(JCExpression selector, List<JCCase> cases) {
773773
patternSet.add(new BindingPattern(e.getKey().type));
774774
}
775775
}
776-
List<PatternDescription> patterns = List.from(patternSet);
776+
Set<PatternDescription> patterns = patternSet;
777777
try {
778778
boolean repeat = true;
779779
while (repeat) {
780-
List<PatternDescription> updatedPatterns;
780+
Set<PatternDescription> updatedPatterns;
781781
updatedPatterns = reduceBindingPatterns(selector.type, patterns);
782782
updatedPatterns = reduceNestedPatterns(updatedPatterns);
783783
updatedPatterns = reduceRecordPatterns(updatedPatterns);
784-
repeat = updatedPatterns != patterns;
784+
updatedPatterns = removeCoveredRecordPatterns(updatedPatterns);
785+
repeat = !updatedPatterns.equals(patterns);
785786
patterns = updatedPatterns;
786787
if (checkCovered(selector.type, patterns)) {
787788
return true;
@@ -794,7 +795,7 @@ private boolean exhausts(JCExpression selector, List<JCCase> cases) {
794795
}
795796
}
796797

797-
private boolean checkCovered(Type seltype, List<PatternDescription> patterns) {
798+
private boolean checkCovered(Type seltype, Iterable<PatternDescription> patterns) {
798799
for (Type seltypeComponent : components(seltype)) {
799800
for (PatternDescription pd : patterns) {
800801
if (pd instanceof BindingPattern bp &&
@@ -830,15 +831,14 @@ private List<Type> components(Type seltype) {
830831
* is found, it is removed, and replaced with a binding pattern
831832
* for the sealed supertype.
832833
*/
833-
private List<PatternDescription> reduceBindingPatterns(Type selectorType, List<PatternDescription> patterns) {
834+
private Set<PatternDescription> reduceBindingPatterns(Type selectorType, Set<PatternDescription> patterns) {
834835
Set<Symbol> existingBindings = patterns.stream()
835836
.filter(pd -> pd instanceof BindingPattern)
836837
.map(pd -> ((BindingPattern) pd).type.tsym)
837838
.collect(Collectors.toSet());
838839

839840
for (PatternDescription pdOne : patterns) {
840841
if (pdOne instanceof BindingPattern bpOne) {
841-
Set<PatternDescription> toRemove = new HashSet<>();
842842
Set<PatternDescription> toAdd = new HashSet<>();
843843

844844
for (Type sup : types.directSupertypes(bpOne.type)) {
@@ -871,7 +871,6 @@ private List<PatternDescription> reduceBindingPatterns(Type selectorType, List<P
871871

872872
for (PatternDescription pdOther : patterns) {
873873
if (pdOther instanceof BindingPattern bpOther) {
874-
boolean reduces = false;
875874
Set<Symbol> currentPermittedSubTypes =
876875
allPermittedSubTypes((ClassSymbol) bpOther.type.tsym, s -> true);
877876

@@ -888,33 +887,21 @@ private List<PatternDescription> reduceBindingPatterns(Type selectorType, List<P
888887
if (types.isSubtype(types.erasure(perm.type),
889888
types.erasure(bpOther.type))) {
890889
it.remove();
891-
reduces = true;
892-
}
893-
}
894-
895-
if (reduces) {
896-
if (!types.isSubtype(types.erasure(clazz.type), types.erasure(bpOther.type))) {
897-
bindings.append(pdOther);
898890
}
899891
}
900892
}
901893
}
902894

903895
if (permitted.isEmpty()) {
904-
toRemove.addAll(bindings);
905896
toAdd.add(new BindingPattern(clazz.type));
906897
}
907898
}
908899
}
909900

910-
if (!toAdd.isEmpty() || !toRemove.isEmpty()) {
911-
for (PatternDescription pd : toRemove) {
912-
patterns = List.filter(patterns, pd);
913-
}
914-
for (PatternDescription pd : toAdd) {
915-
patterns = patterns.prepend(pd);
916-
}
917-
return patterns;
901+
if (!toAdd.isEmpty()) {
902+
Set<PatternDescription> newPatterns = new HashSet<>(patterns);
903+
newPatterns.addAll(toAdd);
904+
return newPatterns;
918905
}
919906
}
920907
}
@@ -958,7 +945,7 @@ private Set<Symbol> allPermittedSubTypes(ClassSymbol root, Predicate<ClassSymbol
958945
* of patterns is replaced with a new set of patterns of the form:
959946
* $record($prefix$, $resultOfReduction, $suffix$)
960947
*/
961-
private List<PatternDescription> reduceNestedPatterns(List<PatternDescription> patterns) {
948+
private Set<PatternDescription> reduceNestedPatterns(Set<PatternDescription> patterns) {
962949
/* implementation note:
963950
* finding a sub-set of patterns that only differ in a single
964951
* column is time-consuming task, so this method speeds it up by:
@@ -977,13 +964,14 @@ private List<PatternDescription> reduceNestedPatterns(List<PatternDescription> p
977964

978965
for (var e : groupByRecordClass.entrySet()) {
979966
int nestedPatternsCount = e.getKey().getRecordComponents().size();
967+
Set<RecordPattern> current = new HashSet<>(e.getValue());
980968

981969
for (int mismatchingCandidate = 0;
982970
mismatchingCandidate < nestedPatternsCount;
983971
mismatchingCandidate++) {
984972
int mismatchingCandidateFin = mismatchingCandidate;
985973
var groupByHashes =
986-
e.getValue()
974+
current
987975
.stream()
988976
//error recovery, ignore patterns with incorrect number of nested patterns:
989977
.filter(pd -> pd.nested.length == nestedPatternsCount)
@@ -1018,37 +1006,35 @@ private List<PatternDescription> reduceNestedPatterns(List<PatternDescription> p
10181006
}
10191007
}
10201008

1021-
var nestedPatterns = join.stream().map(rp -> rp.nested[mismatchingCandidateFin]).collect(List.collector());
1009+
var nestedPatterns = join.stream().map(rp -> rp.nested[mismatchingCandidateFin]).collect(Collectors.toSet());
10221010
var updatedPatterns = reduceNestedPatterns(nestedPatterns);
10231011

10241012
updatedPatterns = reduceRecordPatterns(updatedPatterns);
1013+
updatedPatterns = removeCoveredRecordPatterns(updatedPatterns);
10251014
updatedPatterns = reduceBindingPatterns(rpOne.fullComponentTypes()[mismatchingCandidateFin], updatedPatterns);
10261015

1027-
if (nestedPatterns != updatedPatterns) {
1028-
ListBuffer<PatternDescription> result = new ListBuffer<>();
1029-
Set<PatternDescription> toRemove = Collections.newSetFromMap(new IdentityHashMap<>());
1030-
1031-
toRemove.addAll(join);
1032-
1033-
for (PatternDescription p : patterns) {
1034-
if (!toRemove.contains(p)) {
1035-
result.append(p);
1036-
}
1037-
}
1016+
if (!nestedPatterns.equals(updatedPatterns)) {
1017+
current.removeAll(join);
10381018

10391019
for (PatternDescription nested : updatedPatterns) {
10401020
PatternDescription[] newNested =
10411021
Arrays.copyOf(rpOne.nested, rpOne.nested.length);
10421022
newNested[mismatchingCandidateFin] = nested;
1043-
result.append(new RecordPattern(rpOne.recordType(),
1023+
current.add(new RecordPattern(rpOne.recordType(),
10441024
rpOne.fullComponentTypes(),
10451025
newNested));
10461026
}
1047-
return result.toList();
10481027
}
10491028
}
10501029
}
10511030
}
1031+
1032+
if (!current.equals(new HashSet<>(e.getValue()))) {
1033+
Set<PatternDescription> result = new HashSet<>(patterns);
1034+
result.removeAll(e.getValue());
1035+
result.addAll(current);
1036+
return result;
1037+
}
10521038
}
10531039
return patterns;
10541040
}
@@ -1058,22 +1044,22 @@ private List<PatternDescription> reduceNestedPatterns(List<PatternDescription> p
10581044
* all the $nestedX pattern cover the given record component,
10591045
* and replace those with a simple binding pattern over $record.
10601046
*/
1061-
private List<PatternDescription> reduceRecordPatterns(List<PatternDescription> patterns) {
1062-
var newPatterns = new ListBuffer<PatternDescription>();
1047+
private Set<PatternDescription> reduceRecordPatterns(Set<PatternDescription> patterns) {
1048+
var newPatterns = new HashSet<PatternDescription>();
10631049
boolean modified = false;
10641050
for (PatternDescription pd : patterns) {
10651051
if (pd instanceof RecordPattern rpOne) {
10661052
PatternDescription reducedPattern = reduceRecordPattern(rpOne);
10671053
if (reducedPattern != rpOne) {
1068-
newPatterns.append(reducedPattern);
1054+
newPatterns.add(reducedPattern);
10691055
modified = true;
10701056
continue;
10711057
}
10721058
}
1073-
newPatterns.append(pd);
1059+
newPatterns.add(pd);
10741060
}
1075-
return modified ? newPatterns.toList() : patterns;
1076-
}
1061+
return modified ? newPatterns : patterns;
1062+
}
10771063

10781064
private PatternDescription reduceRecordPattern(PatternDescription pattern) {
10791065
if (pattern instanceof RecordPattern rpOne) {
@@ -1105,6 +1091,23 @@ private PatternDescription reduceRecordPattern(PatternDescription pattern) {
11051091
return pattern;
11061092
}
11071093

1094+
private Set<PatternDescription> removeCoveredRecordPatterns(Set<PatternDescription> patterns) {
1095+
Set<Symbol> existingBindings = patterns.stream()
1096+
.filter(pd -> pd instanceof BindingPattern)
1097+
.map(pd -> ((BindingPattern) pd).type.tsym)
1098+
.collect(Collectors.toSet());
1099+
Set<PatternDescription> result = new HashSet<>(patterns);
1100+
1101+
for (Iterator<PatternDescription> it = result.iterator(); it.hasNext();) {
1102+
PatternDescription pd = it.next();
1103+
if (pd instanceof RecordPattern rp && existingBindings.contains(rp.recordType.tsym)) {
1104+
it.remove();
1105+
}
1106+
}
1107+
1108+
return result;
1109+
}
1110+
11081111
public void visitTry(JCTry tree) {
11091112
ListBuffer<PendingExit> prevPendingExits = pendingExits;
11101113
pendingExits = new ListBuffer<>();

‎test/langtools/tools/javac/patterns/Exhaustiveness.java‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323

2424
/**
2525
* @test
26-
* @bug 8262891 8268871 8274363 8281100 8294670 8311038
26+
* @bug 8262891 8268871 8274363 8281100 8294670 8311038 8311815
2727
* @summary Check exhaustiveness of switches over sealed types.
2828
* @library /tools/lib
2929
* @modules jdk.compiler/com.sun.tools.javac.api
@@ -1540,7 +1540,7 @@ int test(Object o) {
15401540
"1 error");
15411541
}
15421542

1543-
private static final int NESTING_CONSTANT = 5;
1543+
private static final int NESTING_CONSTANT = 4;
15441544

15451545
Set<String> createDeeplyNestedVariants() {
15461546
Set<String> variants = new HashSet<>();
@@ -1968,6 +1968,34 @@ record Rec(Object o) {}
19681968
""");
19691969
}
19701970

1971+
@Test //JDK-8311815:
1972+
public void testAmbiguousRecordUsage(Path base) throws Exception {
1973+
doTest(base,
1974+
new String[0],
1975+
"""
1976+
package test;
1977+
public class Test {
1978+
record Pair(I i1, I i2) {}
1979+
sealed interface I {}
1980+
record C() implements I {}
1981+
record D() implements I {}
1982+
1983+
void exhaustinvenessWithInterface(Pair pairI) {
1984+
switch (pairI) {
1985+
case Pair(D fst, C snd) -> {
1986+
}
1987+
case Pair(C fst, C snd) -> {
1988+
}
1989+
case Pair(C fst, I snd) -> {
1990+
}
1991+
case Pair(D fst, D snd) -> {
1992+
}
1993+
}
1994+
}
1995+
}
1996+
""");
1997+
}
1998+
19711999
private void doTest(Path base, String[] libraryCode, String testCode, String... expectedErrors) throws IOException {
19722000
doTest(base, libraryCode, testCode, false, expectedErrors);
19732001
}

0 commit comments

Comments
 (0)