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<Pr!, En<Pr>!>,
and a superType of R<T> with a superType constructor R.
In findCorrespondingSupertypes we get *two* similar supertypes here,
both have a string representation of:
R<Il<@EnhancedNullability En<Pr>!>!>.
If we create two forks because of it, we get NEW_INFERENCE_ERROR
with the following subtyping violation:
Il<@EnhancedNullability En<Pr>!>! <: En<Pr>!.
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<Il<@EnhancedNullability S>!>
- R<Il<(@EnhancedNullability S & Any, @EnhancedNullability S?)>!>
After substitution of S to En<Pr>! they become similar.
NB: if we change jspecify severity level from strict to warn,
then 'R<Il<(S & Any, S?)>!>' is the only remaining supertype,
and @EnhancedNullability annotation is no more in use.

The type hierarchy in this example looks like:
WithL<T, S> <: CanWithB<T, S>, R.F<S, Il<S>>
CanWithB<T, S> <: R.F<S, Il<S>>
R.F<S, Il<S>> <: R<Il<S>>

So R is reachable by two different ways, via CanWithB and directly via R.F.

#KT-59514 Fixed
This commit is contained in:
Mikhail Glukhikh
2023-09-22 16:25:38 +02:00
committed by Space Team
parent d4ec088afe
commit 87904fd766
4 changed files with 16 additions and 140 deletions
@@ -1,137 +0,0 @@
// JSPECIFY_STATE: strict
// WITH_STDLIB
// FIR_DUMP
// ISSUE: KT-59514
// FILE: lib/En.java
package lib;
public interface En<K extends Pr> extends Se<K> {}
// 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<T> {
interface MR<T> extends R<T> {}
interface F<S, T> extends R<T> {}
interface FMR<S, T> extends F<S, T>, MR<T> {}
}
// FILE: lib/Se.java
package lib;
public interface Se<R extends Ta> {
}
// 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<E> implements java.util.List<E> {
}
// 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 <R extends Ta, S extends Se<R>> Fr<R, S> from(Class<S> c) {
return null;
}
public interface WithL<T extends Ta, S extends Se<T>>
extends CanWithB<T, S>, R.F<S, Il<S>> {}
public interface CanWithL<T extends Ta, S extends Se<T>> {
WithL<T, S> withL(int l);
}
public interface Fr<T extends Ta, S extends Se<T>>
extends CanWithL<T, S>, R.F<S, Il<S>> {}
public interface CanWithB<T extends Ta, S extends Se<T>> extends R.F<S, Il<S>> {
WithB<T, S> withB(B b);
}
public interface WithB<T extends Ta, S extends Se<T>> extends R.F<S, Il<S>> {}
static public class MyWithL<R extends Ta, S extends Se<R>> implements WithL<R, S> {
public WithB<R, S> withB(B b) {
return null;
}
}
}
// FILE: lib/a/Eq.kt
package lib.a
import lib.Se
import lib.Ta
data class A<R : Ta, S : Se<out R>>(val a: Class<out S>) {
companion object {
@JvmStatic
fun <R : Ta, S : Se<R>> of(b: Class<out S>): A<R, S> = A(b)
}
}
class Eq<R : Ta, S : Se<R>>(val a: A<R, S>) : C.Fr<R, S> {
override fun withL(l: Int): C.WithL<R, S> {
return C.MyWithL<R, S>()
}
}
// 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 <T> f(r: R<T>): T
abstract fun <P : Pr, E : En<P>> g(p: P): R.FMR<E, E>
private fun isTableValid(c: Class<En<Pr>>, key: Pr, b: Boolean) {
<!NEW_INFERENCE_ERROR!>if (b) {
f(g(key))
} else {
f(C.from(c).withL(1))
}<!>
}
}
@@ -53,7 +53,7 @@ FILE: Repro.kt
private final fun isTableValid(c: R|java/lang/Class<lib/En<lib/Pr>>|, key: R|lib/Pr|, b: R|kotlin/Boolean|): R|kotlin/Unit| {
when () {
R|<local>/b| -> {
this@R|repro/Repro|.R|repro/Repro.f|<R|ft<lib/En<lib/Pr>, lib/En<lib/Pr>?>|>(this@R|repro/Repro|.R|repro/Repro.g|<R|lib/Pr|, R|ft<lib/En<lib/Pr>, lib/En<lib/Pr>?>|>(R|<local>/key|))
this@R|repro/Repro|.R|repro/Repro.f|<R|ft<it(lib/En<lib/Pr> & lib/Il<ft<@EnhancedNullability lib/En<lib/Pr>, @EnhancedNullability lib/En<lib/Pr>?>>), it(lib/En<lib/Pr>? & lib/Il<ft<@EnhancedNullability lib/En<lib/Pr>, @EnhancedNullability lib/En<lib/Pr>?>>?)>|>(this@R|repro/Repro|.R|repro/Repro.g|<R|lib/Pr|, R|ft<it(lib/En<lib/Pr> & lib/Il<ft<@EnhancedNullability lib/En<lib/Pr>, @EnhancedNullability lib/En<lib/Pr>?>>), it(lib/En<lib/Pr>? & lib/Il<ft<@EnhancedNullability lib/En<lib/Pr>, @EnhancedNullability lib/En<lib/Pr>?>>?)>|>(R|<local>/key|))
}
else -> {
this@R|repro/Repro|.R|repro/Repro.f|<R|ft<lib/Il<ft<@EnhancedNullability lib/En<lib/Pr>, @EnhancedNullability lib/En<lib/Pr>?>>, lib/Il<ft<@EnhancedNullability lib/En<lib/Pr>, @EnhancedNullability lib/En<lib/Pr>?>>?>|>(Q|lib/a/C|.R|lib/a/C.from*s|<R|lib/Pr!|, R|ft<lib/En<lib/Pr>, lib/En<lib/Pr>?>|>(R|<local>/c|).R|SubstitutionOverride<lib/a/C.Fr.withL: R|ft<lib/a/C.WithL<lib/Pr!, ft<lib/En<lib/Pr>, lib/En<lib/Pr>?>>, lib/a/C.WithL<lib/Pr!, ft<lib/En<lib/Pr>, lib/En<lib/Pr>?>>?>|>|(Int(1)))
@@ -1,3 +1,4 @@
// FIR_IDENTICAL
// JSPECIFY_STATE: strict
// WITH_STDLIB
// FIR_DUMP
@@ -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<NON COMPUTED YET>
// (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<Number> <: Array<String>
1 -> return state.isSubtypeForSameConstructor(supertypesWithSameConstructor.first().asArgumentList(), superType)