RFR: 8339296: Record deconstruction pattern in switch fails to compile

Jan Lahoda jlahoda at openjdk.org
Fri Sep 13 09:28:41 UTC 2024


Consider code like this:

    int nestedSwitchesInArgumentPosition(Object o1) {
        return id(switch (o1) {
            case R(var o2) -> switch (o2) {
                case R(String s) -> s;
                default -> "n";
            };
            default -> "";
        });
    }

    int id(String s) {
        return s.length();
    }

    int id(int i) {
        return i;
    }


Compiling this fails with a `StackOverflowError`, because:
 - there are speculative attribution happening for the switch expressions,
 - "completes normally" is computed during these speculative attribution, which have parts of the AST unfilled - specifically the nested `case R(String s)`
 - so, `Attr.PostAttrAnalyzer` fills in the missing types. In particular, the `String s` binding will get the `Symtab.unknownType`.
 - `Flow.makePatternDescription` will eventually ask `Types.isSubtype(..., unknownType)`. This is guaranteed to fail with the `StackOverflowError`, as:
 - `unknownType.isPartial()` returns `true`, so `Types.isSubtype(t, s)` (`s` is the `unknownType`) calls `Types.isSuperType(s, t)`, and `Types.isSuperType(s, t)` does not contain any special handling for the `unknownType`, so it will delegate to `Types.isSubtype(t, s)`, leading to an infinite recursion.

It may be possible to side-step the issue by not computing the completes normally property during speculative attribution, or move that computation outside of `Attr`. It may be good to do, for performance reasons.

But, that does not seem to solve the underlying issue with `unknownType`. Many methods in `Types` misbehave in weird ways when the `unknownType` is passed to them.

The proposal herein is to attempt to resolve that. In particular, the `UnknownType` is proposed to extend the `ErrorType`, dropping the (internal) `UNKNOWN` type tag. The uses of the `UNKNOWN` type tag appear to be equivalent to handling of the `ERROR` type tag anyway. And the special handling of the `unknownType` appear to usually use ` == syms.unknownType`:
https://github.com/openjdk/jdk/blob/0c36177fead8b64a4cee9da3c895e3799f8ba231/src/jdk.compiler/share/classes/com/sun/tools/javac/code/Types.java#L904

After this change, the `unknownType` should behave as an erroneous type, unless specifically requested otherwise.

The intent is that the public API behavior for the `unknownType` should remain the same.

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

Commit messages:
 - 8339296: Record deconstruction pattern in switch fails to compile

Changes: https://git.openjdk.org/jdk/pull/20990/files
  Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=20990&range=00
  Issue: https://bugs.openjdk.org/browse/JDK-8339296
  Stats: 174 lines in 6 files changed: 150 ins; 12 del; 12 mod
  Patch: https://git.openjdk.org/jdk/pull/20990.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/20990/head:pull/20990

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


More information about the compiler-dev mailing list