RFR: 8320220: Compilation of cyclic hierarchy causes infinite recursion

[Vicente Romero]

On Mon, 24 Feb 2025 16:46:35 GMT, Archie Cobbs <acobbs at openjdk.org> wrote:

>>> wouldn't a ClassSymbol method be better here like ClassSymbol::isSubClass?
>>> ...
>>> this method will invoke Class::isInstance again, probably better to just do (c -> (ClassSymbol)c)?
>> 
>> My goal was to replicate the existing logic, but do it for every class in `seenClasses` instead of just the current one. But I was not able to prove to myself that every symbol in `seenClasses` is in fact a `ClassSymbol`, although that may actually be true in practice. So that's why the code is being careful not to blind casting everything in `seenClasses` to `ClassSymbol`.
>> 
>> If you're confident that every symbol in `seenClasses` is always a `ClassSymbol` then we can simplify this... but if so, why wasn't `seenClasses` declared as a `Set<ClassSymbol>` instead of a `Set<Symbol>` in the first place? I'm wondering there is some weird (probably invalid) input that could cause that assumption to be violated.
>> 
>> Thanks for taking a look.
>
> Following up on previous comment...
> 
>>  I'm wondering there is some weird (probably invalid) input that could cause that assumption to be violated.
> 
> In fact, if we just blindly cast then there are 3 regression test failures. In particular, this input crashes the compiler:
> 
> public class Cyclic {
>     static class Outer {
>       class Inner {}
>     }
>     static class Test<X extends Outer>  {
>        class InnerTest extends X.Inner { InnerTest(Outer o) {o.super();} }
>     }
> }
> 
> with this error:
> 
> Finished building target 'interim-langtools' in configuration 'macosx-aarch64-server-release'
> An exception has occurred in the compiler (25-internal). Please file a bug against the Java compiler via the Java bug reporting page (https://bugreport.java.com) after checking the Bug Database (https://bugs.java.com) for duplicates. Include your program, the following diagnostic, and the parameters passed to the Java compiler in your report. Thank you.
> java.lang.ClassCastException: class com.sun.tools.javac.code.Symbol$TypeVariableSymbol cannot be cast to class com.sun.tools.javac.code.Symbol$ClassSymbol (com.sun.tools.javac.code.Symbol$TypeVariableSymbol and com.sun.tools.javac.code.Symbol$ClassSymbol are in module jdk.compiler.interim of loader 'app')
> 	at jdk.compiler.interim/com.sun.tools.javac.comp.Check$CycleChecker.checkSymbol(Check.java:2329)
> 	at jdk.compiler.interim/com.sun.tools.javac.comp.Check$CycleChecker.visitIdent(Check.java:2345)
> 	at jdk.compiler.interim/com.sun.tools.javac.tree.JCTree$JCIdent.accept(JCTree.java:2710)
> 	at jdk.compiler.interim/com.sun.tools.javac.tree.TreeScanner.scan(TreeScanner.java:50)
>         ...

yep I was thinking that tests will tell us if we could go or not with my proposal, nice that we have tests covering this, thanks for trying out

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23704#discussion_r1968057627