Integrated: 8311038: Incorrect exhaustivity computation

[Jan Lahoda]

On Thu, 29 Jun 2023 13:24:05 GMT, Jan Lahoda <jlahoda at openjdk.org> wrote:

> Consider this code:
> 
> public class Test {
> 
>     record Rec(Object t) {}
> 
>     private void test2() {
>         Rec r = new Rec("test");
>         int res = switch (r) {
>             case Rec(String x): {
>                 yield x.length();
>             }
>             case Rec(Object x): {
>                 yield -3;
>             }
>         };
>     }
> 
> }
> 
> 
> It is obviously exhaustive (there's `Rec(Object x)`, which is unconditional on `Rec`), but javac fails to compile this code:
> 
> /tmp/Test.java:7: error: the switch expression does not cover all possible input values
>         int res = switch (r) {
>                   ^
> 1 error
> 
> 
> The cause seems to be that when binding patterns for the (nested) `String` and `Object` are reduced, they are reduced to `String` and `ConstantDesc`, which is no longer exhaustive for `Object`.
> 
> This is a result of this code:
> https://github.com/openjdk/jdk/blob/07734f6dde2b29574b6ef98eeb9e007d8801a3ea/src/jdk.compiler/share/classes/com/sun/tools/javac/comp/Flow.java#L886
> which is intended to help with patterns that don't stand directly in the selector's hierarchy, like:
> https://github.com/openjdk/jdk/blob/07734f6dde2b29574b6ef98eeb9e007d8801a3ea/test/langtools/tools/javac/patterns/Exhaustiveness.java#L1494
> where `I3` is only related to the selector (`I`) via a permitted subtype.
> 
> Part of the problem is that `Object` is removed from the pattern set - adding `ConstantDesc` is not a problem. The proposal in this patch is to keep supertypes of the type that is being added in the pattern set. This will lead to pattern set `ConstantDesc`, `Object` and `String`, which should be OK for correctness, although generally enhancing the pattern set might slow down the processing.
> 
> There are two complicating factors, which make the observed bug surprising and difficult to diagnose:
>  - if (in this example) `ConstantDesc` has not been completed yet, `.isSealed()` will return `false` for it, and the code will compile. This may happen when `x.length()` is removed/replaced in the example. This patch proposes to complete the Symbols before checking for sealedness.
>  - it appears to be difficult to reproduce with a synthetic testcase, due to variations in ordering in maps. Hence this patch is using `String` in the testcase, as opposed to a test-only class.

This pull request has now been integrated.

Changeset: bbb7ce51
Author:    Jan Lahoda <jlahoda at openjdk.org>
URL:       https://git.openjdk.org/jdk/commit/bbb7ce5137cd3e8365552b42610e19b7ebe43ba1
Stats:     28 lines in 2 files changed: 26 ins; 0 del; 2 mod

8311038: Incorrect exhaustivity computation

Reviewed-by: vromero

-------------

PR: https://git.openjdk.org/jdk/pull/14711