diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java index 04280096f55..ce739a5da04 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java @@ -850,8 +850,8 @@ public class Flow { } } try { - Pair> coveredResult = isCovered(selector.type, patternSet); - if (coveredResult.fst) { + CoverageResult coveredResult = isCovered(selector.type, patternSet); + if (coveredResult.covered()) { return true; } if (missingExhaustivenessTimeout == 0) { @@ -859,7 +859,7 @@ public class Flow { } PatternDescription defaultPattern = new BindingPattern(selector.type); - Set missingPatterns = expandMissingPatternDescriptions(selector.type, selector.type, defaultPattern, coveredResult.snd, Set.of(defaultPattern)); + Set missingPatterns = expandMissingPatternDescriptions(selector.type, selector.type, defaultPattern, coveredResult.incompletePatterns(), Set.of(defaultPattern)); for (PatternDescription missing : missingPatterns) { pendingNotExhaustiveDetails.add(missing.toString()); @@ -896,7 +896,7 @@ public class Flow { Set replaced = replace(inMissingPatterns, toExpand, reducedPermittedPatterns); - if (isCovered(selectorType, joinSets(basePatterns, replaced)).fst) { + if (isCovered(selectorType, joinSets(basePatterns, replaced)).covered()) { it.remove(); reduced = true; } @@ -957,16 +957,16 @@ public class Flow { reducedAdded.remove(current); - if (isCovered(selectorType, joinSets(basePatterns, replace(inMissingPatterns, bp, reducedAdded))).fst) { + if (isCovered(selectorType, joinSets(basePatterns, replace(inMissingPatterns, bp, reducedAdded))).covered()) { it.remove(); } } - Pair> combinatorial = isCovered(targetType, combinatorialPatterns); + CoverageResult coverageResult = isCovered(targetType, combinatorialPatterns); - if (!combinatorial.fst) { + if (!coverageResult.covered()) { //nothing better can be done(?) - combinatorialPatterns = combinatorial.snd; + combinatorialPatterns = coverageResult.incompletePatterns(); } //combine sealed subtypes into the supertype, if all is covered. @@ -981,7 +981,7 @@ public class Flow { reducedAdded.remove(current); - if (isCovered(selectorType, joinSets(basePatterns, replace(inMissingPatterns, bp, reducedAdded))).fst) { + if (isCovered(selectorType, joinSets(basePatterns, replace(inMissingPatterns, bp, reducedAdded))).covered()) { it.remove(); } } @@ -1104,16 +1104,14 @@ public class Flow { //false otherwise //TODO: there may be a better name for this method: private boolean isMoreImportant(PatternDescription pd1, PatternDescription pd2, Set basePatterns, Set missingPatterns) { - if (!(pd1 instanceof RecordPattern rp1)) { - throw new AssertionError(); - } - if (!(pd2 instanceof RecordPattern rp2)) { - throw new AssertionError(); - } - for (int c = 0; c < rp1.nested.length; c++) { - BindingPattern bp1 = (BindingPattern) rp1.nested[c]; + if (pd1 instanceof RecordPattern rp1 && pd2 instanceof RecordPattern rp2) { + for (int c = 0; c < rp1.nested.length; c++) { + if (isMoreImportant((BindingPattern) rp1.nested[c], (BindingPattern) rp2.nested[c], basePatterns, missingPatterns)) { + return true; + } + } + } else if (pd1 instanceof BindingPattern bp1 && pd2 instanceof BindingPattern bp2) { Type t1 = bp1.type(); - BindingPattern bp2 = (BindingPattern) rp2.nested[c]; Type t2 = bp2.type(); boolean t1IsImportantRecord = (t1.tsym.flags_field & RECORD) != 0 && hasMatchingRecordPattern(basePatterns, missingPatterns, bp1); boolean t2IsImportantRecord = (t2.tsym.flags_field & RECORD) != 0 && hasMatchingRecordPattern(basePatterns, missingPatterns, bp2); @@ -1160,7 +1158,7 @@ public class Flow { } } - private Pair> isCovered(Type selectorType, Set patterns) { + private CoverageResult isCovered(Type selectorType, Set patterns) { boolean backtrack = true; //XXX: this should be inverted, right? boolean repeat = true; Set updatedPatterns; @@ -1171,7 +1169,7 @@ public class Flow { updatedPatterns = removeCoveredRecordPatterns(updatedPatterns); repeat = !updatedPatterns.equals(patterns); if (checkCovered(selectorType, patterns)) { - return Pair.of(true, null); + return new CoverageResult(true, null); } if (!repeat) { //there may be situation like: @@ -1191,11 +1189,13 @@ public class Flow { patterns = updatedPatterns; } while (repeat); if (checkCovered(selectorType, patterns)) { - return Pair.of(true, null); + return new CoverageResult(true, null); } - return Pair.of(false, updatedPatterns); + return new CoverageResult(false, updatedPatterns); } + private record CoverageResult(boolean covered, Set incompletePatterns) {} + private boolean checkCovered(Type seltype, Iterable patterns) { for (Type seltypeComponent : components(seltype)) { for (PatternDescription pd : patterns) { diff --git a/test/langtools/tools/javac/patterns/ExhaustivenessConvenientErrors.java b/test/langtools/tools/javac/patterns/ExhaustivenessConvenientErrors.java index 1b34e109b3a..96e3f6f39a3 100644 --- a/test/langtools/tools/javac/patterns/ExhaustivenessConvenientErrors.java +++ b/test/langtools/tools/javac/patterns/ExhaustivenessConvenientErrors.java @@ -310,7 +310,7 @@ public class ExhaustivenessConvenientErrors extends TestRunner { "test.Test.Triple(test.Test.A _, test.Test.C(test.Test.Nested _, test.Test.NestedBaseC _), test.Test.C(test.Test.Nested _, test.Test.NestedBaseC _))"); } -// @Test TODO: this still produced sub-optimal results: + @Test public void testComplex4(Path base) throws Exception { doTest(base, new String[0], @@ -347,7 +347,50 @@ public class ExhaustivenessConvenientErrors extends TestRunner { record Root(Base b1, Base b2, Base b3) {} } """, - "test.Test.Root(test.Test.R2 _, test.Test.R2(test.Test.R2 _, test.Test.R2 _), test.Test.R2(test.Test.R2 _, test.Test.R2 _))"); + "test.Test.Root(test.Test.R2 _, test.Test.R2(test.Test.Base _, test.Test.R2 _), test.Test.R2(test.Test.R2 _, test.Test.Base _))"); + //ideally, the result would be as follow, but it is difficult to split Base on two distinct places: +// "test.Test.Root(test.Test.R2 _, test.Test.R2(test.Test.R1 _, test.Test.R2 _), test.Test.R2(test.Test.R2 _, test.Test.R1 _))", +// "test.Test.Root(test.Test.R2 _, test.Test.R2(test.Test.R2 _, test.Test.R2 _), test.Test.R2(test.Test.R2 _, test.Test.R2 _))"); + } + + @Test + public void testComplex5(Path base) throws Exception { + doTest(base, + new String[0], + """ + package test; + public class Test { + private int test(Triple p) { + return switch (p) { + case Triple(B _, _, _) -> 0; + case Triple(_, A _, _) -> 0; + case Triple(_, _, A _) -> 0; +// case Triple(A p, C(Nested _, NestedBaseA _), _) -> 0; + case Triple(A p, C(Nested _, NestedBaseB _), C(Nested _, NestedBaseA _)) -> 0; + case Triple(A p, C(Nested _, NestedBaseB _), C(Nested _, NestedBaseB _)) -> 0; + case Triple(A p, C(Nested _, NestedBaseB _), C(Nested _, NestedBaseC _)) -> 0; + case Triple(A p, C(Nested _, NestedBaseC _), C(Nested _, NestedBaseA _)) -> 0; + case Triple(A p, C(Nested _, NestedBaseC _), C(Nested _, NestedBaseB _)) -> 0; +// case Path(A p, C(Nested _, NestedBaseC _), C(Nested _, NestedBaseC _)) -> 0; + }; + } + record Triple(Base c1, Base c2, Base c3) {} + sealed interface Base permits A, B {} + record A(boolean key) implements Base { + } + sealed interface B extends Base {} + record C(Nested n, NestedBase b) implements B {} + record Nested() {} + sealed interface NestedBase {} + record NestedBaseA() implements NestedBase {} + record NestedBaseB() implements NestedBase {} + record NestedBaseC() implements NestedBase {} + } + """, + "test.Test.Triple(test.Test.A _, test.Test.C(test.Test.Nested _, test.Test.NestedBaseA _), test.Test.C _)", + //the following could be: + //test.Test.Triple(test.Test.A _, test.Test.C(test.Test.Nested _, test.Test.NestedBaseC _), test.Test.C(test.Test.Nested _, test.Test.NestedBaseC _)) + "test.Test.Triple(test.Test.A _, test.Test.C(test.Test.Nested _, test.Test.NestedBaseC _), test.Test.C _)"); } @Test