diff --git a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ExhaustivenessComputer.java b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ExhaustivenessComputer.java index 3b66ac010cf..036e86dff5e 100644 --- a/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ExhaustivenessComputer.java +++ b/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/ExhaustivenessComputer.java @@ -39,6 +39,7 @@ import com.sun.tools.javac.tree.JCTree.*; import com.sun.tools.javac.code.Kinds.Kind; import com.sun.tools.javac.code.Type.TypeVar; +import java.util.ArrayList; import java.util.Arrays; import java.util.Iterator; import java.util.function.Predicate; @@ -60,6 +61,7 @@ public class ExhaustivenessComputer { private final Types types; private final Check chk; private final Infer infer; + private final Map, Boolean> isSubtypeCache = new HashMap<>(); public static ExhaustivenessComputer instance(Context context) { ExhaustivenessComputer instance = context.get(exhaustivenessKey); @@ -120,6 +122,7 @@ public class ExhaustivenessComputer { } } Set patterns = patternSet; + Set> seenFallback = new HashSet<>(); boolean useHashes = true; try { boolean repeat = true; @@ -141,7 +144,7 @@ public class ExhaustivenessComputer { //but hashing in reduceNestedPatterns will not allow that //disable the use of hashing, and use subtyping in //reduceNestedPatterns to handle situations like this: - repeat = useHashes; + repeat = useHashes && seenFallback.add(updatedPatterns); useHashes = false; } else { //if a reduction happened, make sure hashing in reduceNestedPatterns @@ -154,6 +157,8 @@ public class ExhaustivenessComputer { } catch (CompletionFailure cf) { chk.completionError(selector.pos(), cf); return true; //error recovery + } finally { + isSubtypeCache.clear(); } } @@ -211,10 +216,9 @@ public class ExhaustivenessComputer { //if a binding pattern for clazz already exists, no need to analyze it again: !existingBindings.contains(clazz)) { //do not reduce to types unrelated to the selector type: - Type clazzErasure = types.erasure(clazz.type); + Type clazzType = clazz.type; if (components(selectorType).stream() - .map(types::erasure) - .noneMatch(c -> types.isSubtype(clazzErasure, c))) { + .noneMatch(c -> isSubtypeErasure(clazzType, c))) { continue; } @@ -238,18 +242,18 @@ public class ExhaustivenessComputer { //cover using bpOther: //all types that are permitted subtypes of bpOther's type: - pendingPermitted.removeIf(pending -> types.isSubtype(types.erasure(pending.type), - types.erasure(bpOther.type))); + pendingPermitted.removeIf(pending -> isSubtypeErasure(pending.type, + bpOther.type)); if (bpOther.type.tsym.isAbstract()) { //all types that are in a diamond hierarchy with bpOther's type //i.e. there's a common subtype of the given type and bpOther's type: Predicate check = pending -> permitted.stream() - .filter(perm -> types.isSubtype(types.erasure(perm.type), - types.erasure(bpOther.type))) - .filter(perm -> types.isSubtype(types.erasure(perm.type), - types.erasure(pending.type))) + .filter(perm -> isSubtypeErasure(perm.type, + bpOther.type)) + .filter(perm -> isSubtypeErasure(perm.type, + pending.type)) .findAny() .isPresent(); @@ -374,30 +378,15 @@ public class ExhaustivenessComputer { join.append(rpOne); - NEXT_PATTERN: for (int nextCandidate = 0; - nextCandidate < candidatesArr.length; - nextCandidate++) { + for (int nextCandidate = 0; nextCandidate < candidatesArr.length; nextCandidate++) { if (firstCandidate == nextCandidate) { continue; } RecordPattern rpOther = candidatesArr[nextCandidate]; - if (rpOne.recordType.tsym == rpOther.recordType.tsym) { - for (int i = 0; i < rpOne.nested.length; i++) { - if (i != mismatchingCandidate) { - if (!rpOne.nested[i].equals(rpOther.nested[i])) { - if (useHashes || - //when not using hashes, - //check if rpOne.nested[i] is - //a subtype of rpOther.nested[i]: - !(rpOne.nested[i] instanceof BindingPattern bpOne) || - !(rpOther.nested[i] instanceof BindingPattern bpOther) || - !types.isSubtype(types.erasure(bpOne.type), types.erasure(bpOther.type))) { - continue NEXT_PATTERN; - } - } - } - } + + if (rpOne.recordType.tsym == rpOther.recordType.tsym && + nestedComponentsEquivalent(rpOne, rpOther, mismatchingCandidate, useHashes)) { join.append(rpOther); } } @@ -418,9 +407,11 @@ public class ExhaustivenessComputer { PatternDescription[] newNested = Arrays.copyOf(rpOne.nested, rpOne.nested.length); newNested[mismatchingCandidateFin] = nested; - current.add(new RecordPattern(rpOne.recordType(), + RecordPattern nue = new RecordPattern(rpOne.recordType(), rpOne.fullComponentTypes(), - newNested)); + newNested, + new HashSet<>(join)); + current.add(nue); } } } @@ -437,6 +428,70 @@ public class ExhaustivenessComputer { return patterns; } + /* Returns true if all nested components of existing and candidate are + * equivalent (if useHashes == true), or "substitutable" (if useHashes == false). + * A candidate pattern is "substitutable" if it is a binding pattern, and: + * - it's type is a supertype of the existing pattern's type + * - it was produced by a reduction from a record pattern that is equivalent to + * the existing pattern + */ + private boolean nestedComponentsEquivalent(RecordPattern existing, + RecordPattern candidate, + int mismatchingCandidate, + boolean useHashes) { + NEXT_NESTED: + for (int i = 0; i < existing.nested.length; i++) { + if (i != mismatchingCandidate) { + if (!existing.nested[i].equals(candidate.nested[i])) { + if (useHashes) { + return false; + } + //when not using hashes, + //check if rpOne.nested[i] is + //a subtype of rpOther.nested[i]: + if (!(candidate.nested[i] instanceof BindingPattern nestedCandidate)) { + return false; + } + if (existing.nested[i] instanceof BindingPattern nestedExisting) { + if (!isSubtypeErasure(nestedExisting.type, nestedCandidate.type)) { + return false; + } + } else if (existing.nested[i] instanceof RecordPattern nestedExisting) { + java.util.List pendingReplacedPatterns = + new ArrayList<>(nestedCandidate.sourcePatterns()); + + while (!pendingReplacedPatterns.isEmpty()) { + PatternDescription currentReplaced = pendingReplacedPatterns.removeLast(); + + if (nestedExisting.equals(currentReplaced)) { + //candidate.nested[i] is substitutable for existing.nested[i] + //continue with the next nested pattern: + continue NEXT_NESTED; + } + + pendingReplacedPatterns.addAll(currentReplaced.sourcePatterns()); + } + + return false; + } else { + return false; + } + } + } + } + + return true; + } + + /*The same as types.isSubtype(types.erasure(t), types.erasure(s)), but cached. + */ + private boolean isSubtypeErasure(Type t, Type s) { + Pair key = Pair.of(t, s); + + return isSubtypeCache.computeIfAbsent(key, _ -> + types.isSubtype(types.erasure(t), types.erasure(s))); + } + /* In the set of patterns, find those for which, given: * $record($nested1, $nested2, ...) * all the $nestedX pattern cover the given record component, @@ -480,9 +535,11 @@ public class ExhaustivenessComputer { covered &= checkCovered(componentType[i], List.of(newNested)); } if (covered) { - return new BindingPattern(rpOne.recordType); + PatternDescription pd = new BindingPattern(rpOne.recordType, Set.of(pattern)); + return pd; } else if (reducedNestedPatterns != null) { - return new RecordPattern(rpOne.recordType, rpOne.fullComponentTypes(), reducedNestedPatterns); + PatternDescription pd = new RecordPattern(rpOne.recordType, rpOne.fullComponentTypes(), reducedNestedPatterns, Set.of(pattern)); + return pd; } } return pattern; @@ -517,7 +574,9 @@ public class ExhaustivenessComputer { return false; } - sealed interface PatternDescription { } + sealed interface PatternDescription { + public Set sourcePatterns(); + } public PatternDescription makePatternDescription(Type selectorType, JCPattern pattern) { if (pattern instanceof JCBindingPattern binding) { Type type = !selectorType.isPrimitive() && types.isSubtype(selectorType, binding.type) @@ -552,7 +611,12 @@ public class ExhaustivenessComputer { throw Assert.error(); } } - record BindingPattern(Type type) implements PatternDescription { + record BindingPattern(Type type, Set sourcePatterns) implements PatternDescription { + + public BindingPattern(Type type) { + this(type, Set.of()); + } + @Override public int hashCode() { return type.tsym.hashCode(); @@ -567,10 +631,14 @@ public class ExhaustivenessComputer { return type.tsym + " _"; } } - record RecordPattern(Type recordType, int _hashCode, Type[] fullComponentTypes, PatternDescription... nested) implements PatternDescription { + record RecordPattern(Type recordType, int _hashCode, Type[] fullComponentTypes, PatternDescription[] nested, Set sourcePatterns) implements PatternDescription { public RecordPattern(Type recordType, Type[] fullComponentTypes, PatternDescription[] nested) { - this(recordType, hashCode(-1, recordType, nested), fullComponentTypes, nested); + this(recordType, fullComponentTypes, nested, Set.of()); + } + + public RecordPattern(Type recordType, Type[] fullComponentTypes, PatternDescription[] nested, Set sourcePatterns) { + this(recordType, hashCode(-1, recordType, nested), fullComponentTypes, nested, sourcePatterns); } @Override diff --git a/test/langtools/tools/javac/patterns/Exhaustiveness.java b/test/langtools/tools/javac/patterns/Exhaustiveness.java index f328765b579..84e67855b3b 100644 --- a/test/langtools/tools/javac/patterns/Exhaustiveness.java +++ b/test/langtools/tools/javac/patterns/Exhaustiveness.java @@ -23,7 +23,7 @@ /** * @test - * @bug 8262891 8268871 8274363 8281100 8294670 8311038 8311815 8325215 8333169 8327368 8366968 + * @bug 8262891 8268871 8274363 8281100 8294670 8311038 8311815 8325215 8333169 8327368 8366968 8364991 * @summary Check exhaustiveness of switches over sealed types. * @library /tools/lib * @modules jdk.compiler/com.sun.tools.javac.api @@ -40,6 +40,7 @@ import java.nio.file.Paths; import java.util.ArrayList; import java.util.HashSet; import java.util.Iterator; +import java.util.LinkedHashSet; import java.util.List; import java.util.Set; import java.util.stream.Collectors; @@ -1543,7 +1544,7 @@ public class Exhaustiveness extends TestRunner { private static final int NESTING_CONSTANT = 4; Set createDeeplyNestedVariants() { - Set variants = new HashSet<>(); + Set variants = new LinkedHashSet<>(); variants.add("C _"); variants.add("R(I _, I _, I _)"); for (int n = 0; n < NESTING_CONSTANT; n++) { @@ -1636,10 +1637,11 @@ public class Exhaustiveness extends TestRunner { @Test public void testDeeplyNestedNotExhaustive(Path base) throws Exception { List variants = createDeeplyNestedVariants().stream().collect(Collectors.toCollection(ArrayList::new)); - variants.remove((int) (Math.random() * variants.size())); + int removed = (int) (Math.random() * variants.size()); + variants.remove(removed); String code = testCodeForVariants(variants); - System.err.println("analyzing:"); + System.err.println("analyzing (removed: " + removed + "):"); System.err.println(code); doTest(base, new String[0], @@ -2311,6 +2313,180 @@ public class Exhaustiveness extends TestRunner { "1 error"); } + @Test //JDK-8364991 + public void testDifferentReductionPaths(Path base) throws Exception { + doTest(base, + new String[0], + """ + package test; + public class Test { + private int test(Root r) { + return switch (r) { + case Root(R1 _, _, _) -> 0; + case Root(R2 _, R1 _, _) -> 0; + case Root(R2 _, R2 _, R1 _) -> 0; + case Root(R2 _, R2(R1 _, R1 _), R2(R1 _, R1 _)) -> 0; + case Root(R2 _, R2(R1 _, R1 _), R2(R1 _, R2 _)) -> 0; + case Root(R2 _, R2(R1 _, R1 _), R2(R2 _, R1 _)) -> 0; + case Root(R2 _, R2(R1 _, R1 _), R2(R2 _, R2 _)) -> 0; + case Root(R2 _, R2(R1 _, R2 _), R2(R1 _, R1 _)) -> 0; + case Root(R2 _, R2(R1 _, R2 _), R2(R1 _, R2 _)) -> 0; + case Root(R2 _, R2(R1 _, R2 _), R2(R2 _, R1 _)) -> 0; + case Root(R2 _, R2(R1 _, R2 _), R2(R2 _, R2 _)) -> 0; + case Root(R2 _, R2(R2 _, R1 _), R2(R1 _, R1 _)) -> 0; + case Root(R2 _, R2(R2 _, R1 _), R2(R1 _, R2 _)) -> 0; + case Root(R2 _, R2(R2 _, R1 _), R2(R2 _, R1 _)) -> 0; + case Root(R2 _, R2(R2 _, R1 _), R2(R2 _, R2 _)) -> 0; + case Root(R2 _, R2(R2 _, R2 _), R2(R1 _, R1 _)) -> 0; + case Root(R2 _, R2(R2 _, R2 _), R2(R1 _, R2 _)) -> 0; + case Root(R2 _, R2(R2 _, R2 _), R2(R2 _, R1 _)) -> 0; + case Root(R2 _, R2(R2 _, R2 _), R2 _) -> 0; //functionally equivalent to: Root(R2 _, R2(R2 _, R2 _), R2(R2 _, R2 _)) + }; + } + sealed interface Base {} + record R1() implements Base {} + record R2(Base b1, Base b2) implements Base {} + record Root(Base b1, Base b2, Base b3) {} + } + """); + doTest(base, + new String[0], + """ + package test; + public class Test { + private int test(Root r) { + return switch (r) { + case Root(R1 _, _, _) -> 0; + case Root(R2 _, R1 _, _) -> 0; + case Root(R2 _, R2 _, R1 _) -> 0; + case Root(R2 _, R2(R1 _, R1 _), R2(R1 _, R1 _)) -> 0; + case Root(R2 _, R2(R1 _, R1 _), R2(R1 _, R2 _)) -> 0; + case Root(R2 _, R2(R1 _, R1 _), R2(R2 _, R1 _)) -> 0; + case Root(R2 _, R2(R1 _, R1 _), R2(R2 _, R2 _)) -> 0; + case Root(R2 _, R2(R1 _, R2 _), R2(R1 _, R1 _)) -> 0; + case Root(R2 _, R2(R1 _, R2 _), R2(R1 _, R2 _)) -> 0; + case Root(R2 _, R2(R1 _, R2 _), R2(R2 _, R1 _)) -> 0; + case Root(R2 _, R2(R1 _, R2 _), R2(R2 _, R2 _)) -> 0; + case Root(R2 _, R2(R2 _, R1 _), R2(R1 _, R1 _)) -> 0; + case Root(R2 _, R2(R2 _, R1 _), R2(R1 _, R2 _)) -> 0; + case Root(R2 _, R2(R2 _, R1 _), R2(R2 _, R1 _)) -> 0; + case Root(R2 _, R2(R2 _, R1 _), R2(R2 _, R2 _)) -> 0; + case Root(R2 _, R2(R2 _, R2 _), R2(R1 _, R1 _)) -> 0; + case Root(R2 _, R2(R2 _, R2 _), R2(R1 _, R2 _)) -> 0; + case Root(R2 _, R2(R2 _, R2 _), R2(R2 _, R1 _)) -> 0; + case Root(R2 _, R2(R2 _, R2 _), R2 _) -> 0; //functionally equivalent to: Root(R2 _, R2(R2 _, R2 _), R2(R2 _, R2 _)) + }; + } + sealed interface Base {} + record R1() implements Base {} + record R2(Base b1, Base b2) implements Base {} + record Root(T b1, T b2, T b3) {} + } + """); + } + + @Test //JDK-8364991 + public void testDifferentReductionPathsSimplified(Path base) throws Exception { + doTest(base, + new String[0], + """ + package test; + public class Test { + private int test1(Root r) { + return switch (r) { + case Root(R2(R1 _), R2(R1 _)) -> 0; + case Root(R2(R1 _), R2(R2 _)) -> 0; + case Root(R2(R2 _), R2(R1 _)) -> 0; + case Root(R2(R2 _), R2 _) -> 0; + }; + } + private int test2(Root r) { + return switch (r) { + case Root(R2(R1 _), R2(R1 _)) -> 0; + case Root(R2(R2 _), R2(R1 _)) -> 0; + case Root(R2(R1 _), R2(R2 _)) -> 0; + case Root(R2 _, R2(R2 _)) -> 0; + }; + } + sealed interface Base {} + record R1() implements Base {} + record R2(Base b1) implements Base {} + record Root(R2 b2, R2 b3) {} + } + """); + doTest(base, + new String[0], + """ + package test; + public class Test { + private int test1(Root> r) { + return switch (r) { + case Root(R2(R1 _), R2(R1 _)) -> 0; + case Root(R2(R1 _), R2(R2 _)) -> 0; + case Root(R2(R2 _), R2(R1 _)) -> 0; + case Root(R2(R2 _), R2 _) -> 0; + }; + } + private int test2(Root> r) { + return switch (r) { + case Root(R2(R1 _), R2(R1 _)) -> 0; + case Root(R2(R2 _), R2(R1 _)) -> 0; + case Root(R2(R1 _), R2(R2 _)) -> 0; + case Root(R2 _, R2(R2 _)) -> 0; + }; + } + sealed interface Base {} + record R1() implements Base {} + record R2(T b1) implements Base {} + record Root(T b2, T b3) {} + } + """); + } + + @Test //JDK-8364991 + public void testBindingPatternDoesNotStandInPlaceOfRecordPatterns(Path base) throws Exception { + doTest(base, + new String[0], + """ + package test; + public class Test { + private int test(Root r) { + return switch (r) { + case Root(R2 _, R2(R1 _)) -> 0; + case Root(R2(R1 _), R2(R2 _)) -> 0; + case Root(R2(R2 _), R2 _) -> 0; + }; + } + sealed interface Base {} + record R1() implements Base {} + record R2(Base b) implements Base {} + record Root(R2 b2, R2 b3) {} + } + """, + "Test.java:4:16: compiler.err.not.exhaustive", + "1 error"); + doTest(base, + new String[0], + """ + package test; + public class Test { + private int test(Root> r) { + return switch (r) { + case Root(R2 _, R2(R1 _)) -> 0; + case Root(R2(R1 _), R2(R2 _)) -> 0; + case Root(R2(R2 _), R2 _) -> 0; + }; + } + sealed interface Base {} + record R1() implements Base {} + record R2(T b) implements Base {} + record Root(T b2, T b3) {} + } + """, + "Test.java:4:16: compiler.err.not.exhaustive", + "1 error"); + } + private void doTest(Path base, String[] libraryCode, String testCode, String... expectedErrors) throws IOException { doTest(base, libraryCode, testCode, false, expectedErrors); }