From 87904fd7661eb532c3263a2d5de50bb7baea3747 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Fri, 22 Sep 2023 16:25:38 +0200 Subject: [PATCH] AbstractTypeChecker: don't create forks for identical supertypes The main reason of this commit is the fact that it makes no sense to create type checker forks in case of two or more types which equal each other. Also, this commit fixes the test taken from KT-59514 (see interconnectedGenerics.kt test changed in this commit). In this particular case we have a subType of C.WithL!>, and a superType of R with a superType constructor R. In findCorrespondingSupertypes we get *two* similar supertypes here, both have a string representation of: R!>!>. If we create two forks because of it, we get NEW_INFERENCE_ERROR with the following subtyping violation: Il<@EnhancedNullability En!>! <: En!. NEW_INFERENCE_ERROR happens because we make a redundant fork in that case, but the forks should still work despite it (see KT-62333). These two types appear due to the content of FirCorrespondingSupertypeCache. For a key C.WithL and a superType R it stores the following pair of supertypes: - R!> - R!> After substitution of S to En! they become similar. NB: if we change jspecify severity level from strict to warn, then 'R!>' is the only remaining supertype, and @EnhancedNullability annotation is no more in use. The type hierarchy in this example looks like: WithL <: CanWithB, R.F> CanWithB <: R.F> R.F> <: R> So R is reachable by two different ways, via CanWithB and directly via R.F. #KT-59514 Fixed --- .../strictMode/interconnectedGenerics.fir.kt | 137 ------------------ .../strictMode/interconnectedGenerics.fir.txt | 2 +- .../strictMode/interconnectedGenerics.kt | 1 + .../kotlin/types/AbstractTypeChecker.kt | 16 +- 4 files changed, 16 insertions(+), 140 deletions(-) delete mode 100644 compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jspecify/strictMode/interconnectedGenerics.fir.kt diff --git a/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jspecify/strictMode/interconnectedGenerics.fir.kt b/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jspecify/strictMode/interconnectedGenerics.fir.kt deleted file mode 100644 index 56f37620841..00000000000 --- a/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jspecify/strictMode/interconnectedGenerics.fir.kt +++ /dev/null @@ -1,137 +0,0 @@ -// JSPECIFY_STATE: strict -// WITH_STDLIB -// FIR_DUMP -// ISSUE: KT-59514 - -// FILE: lib/En.java - -package lib; - -public interface En extends Se {} - -// FILE: lib/Ke.java - -package lib; - -public interface Ke extends Ta {} - -// FILE: lib/Pr.java - -package lib; - -public interface Pr extends Ke {} - -// FILE: lib/R.java - -package lib; - -public interface R { - interface MR extends R {} - interface F extends R {} - interface FMR extends F, MR {} -} - -// FILE: lib/Se.java - -package lib; - -public interface Se { -} - -// FILE: lib/Ta.java - -package lib; - -public interface Ta { -} - -// FILE: lib/Il.java - -package lib; - -import org.jspecify.nullness.NullMarked; - -@NullMarked -public abstract class Il implements java.util.List { -} - -// FILE: lib/a/C.java - -package lib.a; - -import lib.Ta; -import lib.Se; -import lib.R; -import lib.Il; - -class B {} - -public final class C { - public static > Fr from(Class c) { - return null; - } - - public interface WithL> - extends CanWithB, R.F> {} - - public interface CanWithL> { - WithL withL(int l); - } - - public interface Fr> - extends CanWithL, R.F> {} - - public interface CanWithB> extends R.F> { - WithB withB(B b); - } - - public interface WithB> extends R.F> {} - - static public class MyWithL> implements WithL { - public WithB withB(B b) { - return null; - } - } -} - -// FILE: lib/a/Eq.kt - -package lib.a - -import lib.Se -import lib.Ta - -data class A>(val a: Class) { - companion object { - @JvmStatic - fun > of(b: Class): A = A(b) - } -} - -class Eq>(val a: A) : C.Fr { - override fun withL(l: Int): C.WithL { - return C.MyWithL() - } -} - -// FILE: repro/Repro.kt - -package repro - -import lib.R -import lib.a.C -import lib.Ke -import lib.Pr -import lib.En - -abstract class Repro { - abstract fun f(r: R): T - abstract fun

> g(p: P): R.FMR - private fun isTableValid(c: Class>, key: Pr, b: Boolean) { - if (b) { - f(g(key)) - } else { - f(C.from(c).withL(1)) - } - } -} diff --git a/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jspecify/strictMode/interconnectedGenerics.fir.txt b/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jspecify/strictMode/interconnectedGenerics.fir.txt index f6d0e836d57..fca21f65196 100644 --- a/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jspecify/strictMode/interconnectedGenerics.fir.txt +++ b/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jspecify/strictMode/interconnectedGenerics.fir.txt @@ -53,7 +53,7 @@ FILE: Repro.kt private final fun isTableValid(c: R|java/lang/Class>|, key: R|lib/Pr|, b: R|kotlin/Boolean|): R|kotlin/Unit| { when () { R|/b| -> { - this@R|repro/Repro|.R|repro/Repro.f|, lib/En?>|>(this@R|repro/Repro|.R|repro/Repro.g|, lib/En?>|>(R|/key|)) + this@R|repro/Repro|.R|repro/Repro.f| & lib/Il, @EnhancedNullability lib/En?>>), it(lib/En? & lib/Il, @EnhancedNullability lib/En?>>?)>|>(this@R|repro/Repro|.R|repro/Repro.g| & lib/Il, @EnhancedNullability lib/En?>>), it(lib/En? & lib/Il, @EnhancedNullability lib/En?>>?)>|>(R|/key|)) } else -> { this@R|repro/Repro|.R|repro/Repro.f|, @EnhancedNullability lib/En?>>, lib/Il, @EnhancedNullability lib/En?>>?>|>(Q|lib/a/C|.R|lib/a/C.from*s|, lib/En?>|>(R|/c|).R|SubstitutionOverride, lib/En?>>, lib/a/C.WithL, lib/En?>>?>|>|(Int(1))) diff --git a/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jspecify/strictMode/interconnectedGenerics.kt b/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jspecify/strictMode/interconnectedGenerics.kt index 74b0a0da061..aabdc446e74 100644 --- a/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jspecify/strictMode/interconnectedGenerics.kt +++ b/compiler/testData/diagnostics/foreignAnnotationsTests/java8Tests/jspecify/strictMode/interconnectedGenerics.kt @@ -1,3 +1,4 @@ +// FIR_IDENTICAL // JSPECIFY_STATE: strict // WITH_STDLIB // FIR_DUMP diff --git a/core/compiler.common/src/org/jetbrains/kotlin/types/AbstractTypeChecker.kt b/core/compiler.common/src/org/jetbrains/kotlin/types/AbstractTypeChecker.kt index 8fc64288794..9f1bde00f4f 100644 --- a/core/compiler.common/src/org/jetbrains/kotlin/types/AbstractTypeChecker.kt +++ b/core/compiler.common/src/org/jetbrains/kotlin/types/AbstractTypeChecker.kt @@ -374,8 +374,20 @@ object AbstractTypeChecker { if (areEqualTypeConstructors(subType.typeConstructor(), superConstructor) && superConstructor.parametersCount() == 0) return true if (superType.typeConstructor().isAnyConstructor()) return true - val supertypesWithSameConstructor = findCorrespondingSupertypes(state, subType, superConstructor) - .map { state.prepareType(it).asSimpleType() ?: it } + val supertypesWithSameConstructor = with(findCorrespondingSupertypes(state, subType, superConstructor)) { + // Note: in K1, we can have partially computed types here, like SomeType + // (see e.g. interClassesRecursion.kt from diagnostic tests) + // In this case we don't want to affect lazy computation in normal case (size <= 1), that's why we don't create a set + // (adding to a hash set requires hash-code calculation for each set element) + + if (size > 1 && (state.typeSystemContext as? TypeSystemInferenceExtensionContext)?.isK2 == true) { + // Here we want to filter out equivalent types to avoid unnecessary forking + mapTo(mutableSetOf()) { state.prepareType(it).asSimpleType() ?: it } + } else { + // TODO: drop this branch together with K1 code + map { state.prepareType(it).asSimpleType() ?: it } + } + } when (supertypesWithSameConstructor.size) { 0 -> return hasNothingSupertype(state, subType) // todo Nothing & Array <: Array 1 -> return state.isSubtypeForSameConstructor(supertypesWithSameConstructor.first().asArgumentList(), superType)