Refactoring: Improve createExpectActualTypeParameterSubstitutor API

Review: https://jetbrains.team/p/kt/reviews/12279/files Motivation: make sure that cases like KT-62027 won't happen again Review: https://jetbrains.team/p/kt/reviews/12279/files Now it's responsibility of the `createExpectActualTypeParameterSubstitutor` calller to think about the case when parameters size isn't equal. You must not be able to create a substitutor if type parameters sizes are not equal Improvement in `createExpectActualTypeParameterSubstitutor` API also improves `AbstractExpectActualCompatibilityChecker.getCallablesCompatibility` API. Because suppose that you accidentally created a redundant wrapping substitutor => you need to handle the case of not equal type parameters size on the call site => you start thinking why you should do that on the call site? It must be a responsibility of `getCallablesCompatibility` => you realize that you created a redundant wrapping substitutor
2023-09-20 14:17:59 +02:00
parent 211102569c
commit 91a337074e
12 changed files with 48 additions and 62 deletions
@@ -19,6 +19,7 @@ import org.jetbrains.kotlin.fir.symbols.impl.FirCallableSymbol
 import org.jetbrains.kotlin.fir.types.areCompatibleExpectActualTypes
 import org.jetbrains.kotlin.fir.types.createExpectActualTypeParameterSubstitutor
 import org.jetbrains.kotlin.resolve.multiplatform.ExpectActualCompatibility
+import org.jetbrains.kotlin.utils.zipIfSizesAreEqual

 object FirActualCallableDeclarationChecker : FirCallableDeclarationChecker() {
    override fun check(declaration: FirCallableDeclaration, context: CheckerContext, reporter: DiagnosticReporter) {
@@ -34,12 +35,16 @@ object FirActualCallableDeclarationChecker : FirCallableDeclarationChecker() {
        val expectTypeParameters = expectFunctionSymbol.getContainingClassSymbol(expectFunctionSymbol.moduleData.session)
            ?.typeParameterSymbols.orEmpty()
        val actualClassTypeParameters = actualFunctionSymbol.getContainingClassSymbol(context.session)?.typeParameterSymbols.orEmpty()
-        val parentSubstitutor =
-            createExpectActualTypeParameterSubstitutor(expectTypeParameters, actualClassTypeParameters, context.session)
+
+        val parentSubstitutor = createExpectActualTypeParameterSubstitutor(
+            // It's responsibility of AbstractExpectActualCompatibilityChecker to report that
+            (expectTypeParameters zipIfSizesAreEqual actualClassTypeParameters) ?: return,
+            context.session
+        )

        val substitutor = createExpectActualTypeParameterSubstitutor(
-            expectFunctionSymbol.typeParameterSymbols,
-            actualFunctionSymbol.typeParameterSymbols,
+            // It's responsibility of AbstractExpectActualCompatibilityChecker to check that
+            (expectFunctionSymbol.typeParameterSymbols zipIfSizesAreEqual actualFunctionSymbol.typeParameterSymbols) ?: return,
            context.session,
            parentSubstitutor
        )
@@ -13,12 +13,11 @@ import org.jetbrains.kotlin.fir.symbols.impl.FirTypeParameterSymbol
 import org.jetbrains.kotlin.types.AbstractTypeChecker

 fun createExpectActualTypeParameterSubstitutor(
-    expectedTypeParameters: List<FirTypeParameterSymbol>,
-    actualTypeParameters: List<FirTypeParameterSymbol>,
+    expectActualTypeParameters: List<Pair<FirTypeParameterSymbol, FirTypeParameterSymbol>>,
    useSiteSession: FirSession,
    parentSubstitutor: ConeSubstitutor? = null
 ): ConeSubstitutor {
-    val substitution = expectedTypeParameters.zip(actualTypeParameters).associate { (expectedParameterSymbol, actualParameterSymbol) ->
+    val substitution = expectActualTypeParameters.associate { (expectedParameterSymbol, actualParameterSymbol) ->
        expectedParameterSymbol to actualParameterSymbol.toLookupTag().constructType(emptyArray(), isNullable = false)
    }
    val substitutor = ConeSubstitutorByMap(
@@ -36,8 +36,6 @@ import org.jetbrains.kotlin.types.model.KotlinTypeMarker
 import org.jetbrains.kotlin.types.model.SimpleTypeMarker
 import org.jetbrains.kotlin.types.model.TypeSubstitutorMarker
 import org.jetbrains.kotlin.types.model.TypeSystemContext
-import org.jetbrains.kotlin.utils.addToStdlib.UnsafeCastFunction
-import org.jetbrains.kotlin.utils.addToStdlib.castAll

 class FirExpectActualMatchingContextImpl private constructor(
    private val actualSession: FirSession,
@@ -147,15 +145,13 @@ class FirExpectActualMatchingContextImpl private constructor(
    override val PropertySymbolMarker.setter: FunctionSymbolMarker?
        get() = asSymbol().setterSymbol

-    @OptIn(UnsafeCastFunction::class)
    override fun createExpectActualTypeParameterSubstitutor(
-        expectTypeParameters: List<TypeParameterSymbolMarker>,
-        actualTypeParameters: List<TypeParameterSymbolMarker>,
+        expectActualTypeParameters: List<Pair<TypeParameterSymbolMarker, TypeParameterSymbolMarker>>,
        parentSubstitutor: TypeSubstitutorMarker?,
    ): TypeSubstitutorMarker {
+        @Suppress("UNCHECKED_CAST")
        return createExpectActualTypeParameterSubstitutor(
-            expectTypeParameters.castAll<FirTypeParameterSymbol>(),
-            actualTypeParameters.castAll<FirTypeParameterSymbol>(),
+            expectActualTypeParameters as List<Pair<FirTypeParameterSymbol, FirTypeParameterSymbol>>,
            actualSession,
            parentSubstitutor as ConeSubstitutor?
        )
@@ -19,6 +19,7 @@ import org.jetbrains.kotlin.fir.types.*
 import org.jetbrains.kotlin.mpp.CallableSymbolMarker
 import org.jetbrains.kotlin.resolve.calls.mpp.AbstractExpectActualCompatibilityChecker
 import org.jetbrains.kotlin.resolve.multiplatform.ExpectActualCompatibility
+import org.jetbrains.kotlin.utils.zipIfSizesAreEqual

 object FirExpectActualResolver {
    fun findExpectForActual(
@@ -46,14 +47,8 @@ object FirExpectActualResolver {
                            val expectTypeParameters = expectContainingClass?.typeParameterSymbols.orEmpty()
                            val actualTypeParameters = actualContainingClass?.typeParameterSymbols.orEmpty()

-                            parentSubstitutor = when (expectTypeParameters.size == actualTypeParameters.size) {
-                                true -> createExpectActualTypeParameterSubstitutor(
-                                    expectTypeParameters,
-                                    actualTypeParameters,
-                                    useSiteSession,
-                                )
-                                false -> null
-                            }
+                            parentSubstitutor = (expectTypeParameters zipIfSizesAreEqual actualTypeParameters)
+                                ?.let { createExpectActualTypeParameterSubstitutor(it, useSiteSession) }

                            when (actualSymbol) {
                                is FirConstructorSymbol -> expectContainingClass?.getConstructors(scopeSession)
@@ -37,8 +37,6 @@ import org.jetbrains.kotlin.types.Variance
 import org.jetbrains.kotlin.types.model.KotlinTypeMarker
 import org.jetbrains.kotlin.types.model.TypeSubstitutorMarker
 import org.jetbrains.kotlin.types.model.TypeSystemContext
-import org.jetbrains.kotlin.utils.addToStdlib.UnsafeCastFunction
-import org.jetbrains.kotlin.utils.addToStdlib.castAll
 import org.jetbrains.kotlin.utils.addToStdlib.shouldNotBeCalled

 internal abstract class IrExpectActualMatchingContext(
@@ -238,16 +236,14 @@ internal abstract class IrExpectActualMatchingContext(
    override val PropertySymbolMarker.setter: FunctionSymbolMarker?
        get() = asIr().setter?.symbol

-    @OptIn(UnsafeCastFunction::class)
    override fun createExpectActualTypeParameterSubstitutor(
-        expectTypeParameters: List<TypeParameterSymbolMarker>,
-        actualTypeParameters: List<TypeParameterSymbolMarker>,
+        expectActualTypeParameters: List<Pair<TypeParameterSymbolMarker, TypeParameterSymbolMarker>>,
        parentSubstitutor: TypeSubstitutorMarker?,
    ): TypeSubstitutorMarker {
-        val expectParameters = expectTypeParameters.castAll<IrTypeParameterSymbol>()
-        val actualParameters = actualTypeParameters.castAll<IrTypeParameterSymbol>()
-        val actualTypes = actualParameters.map { it.owner.defaultType }
-        val substitutor = IrTypeSubstitutor(expectParameters, actualTypes, allowEmptySubstitution = true)
+        val typeParametersToArguments = expectActualTypeParameters.associate { (expect, actual) ->
+            (expect as IrTypeParameterSymbol) to (actual as IrTypeParameterSymbol).owner.defaultType
+        }
+        val substitutor = IrTypeSubstitutor(typeParametersToArguments, allowEmptySubstitution = true)
        return when (parentSubstitutor) {
            null -> substitutor
            is AbstractIrTypeSubstitutor -> IrChainedSubstitutor(parentSubstitutor, substitutor)
@@ -20,6 +20,7 @@ import org.jetbrains.kotlin.ir.expressions.IrExpression
 import org.jetbrains.kotlin.ir.expressions.IrFunctionReference
 import org.jetbrains.kotlin.ir.expressions.IrStatementOrigin
 import org.jetbrains.kotlin.ir.expressions.impl.IrFunctionReferenceImpl
+import org.jetbrains.kotlin.ir.symbols.IrTypeParameterSymbol
 import org.jetbrains.kotlin.ir.types.IrTypeArgument
 import org.jetbrains.kotlin.ir.types.IrTypeSubstitutor
 import org.jetbrains.kotlin.ir.util.SYNTHETIC_OFFSET
@@ -50,11 +51,8 @@ class WrapInlineDeclarationsWithReifiedTypeParametersLowering(val context: Backe
                if (!owner.isInlineFunWithReifiedParameter()) {
                    return expression
                }
-                val substitutionMap = expression.typeSubstitutionMap.entries
-                val typeSubstitutor = IrTypeSubstitutor(
-                    substitutionMap.map { it.key },
-                    substitutionMap.map { it.value as IrTypeArgument },
-                )
+                @Suppress("UNCHECKED_CAST")
+                val typeSubstitutor = IrTypeSubstitutor(expression.typeSubstitutionMap as Map<IrTypeParameterSymbol, IrTypeArgument>)

                val function = irFactory.buildFun {
                    name = Name.identifier("${owner.name}${"$"}wrap")
@@ -65,12 +65,15 @@ abstract class BaseIrTypeSubstitutor : AbstractIrTypeSubstitutor() {
 }

 class IrTypeSubstitutor(
-    typeParameters: List<IrTypeParameterSymbol>,
-    typeArguments: List<IrTypeArgument>,
-    private val allowEmptySubstitution: Boolean = false,
+    private val substitution: Map<IrTypeParameterSymbol, IrTypeArgument>,
+    private val allowEmptySubstitution: Boolean = false
 ) : BaseIrTypeSubstitutor() {
-    init {
-        assert(typeParameters.size == typeArguments.size) {
+    constructor(
+        typeParameters: List<IrTypeParameterSymbol>,
+        typeArguments: List<IrTypeArgument>,
+        allowEmptySubstitution: Boolean = false,
+    ) : this(typeParameters.zip(typeArguments).toMap(), allowEmptySubstitution) {
+        check(typeParameters.size == typeArguments.size) {
            "Unexpected number of type arguments: ${typeArguments.size}\n" +
                    "Type parameters are:\n" +
                    typeParameters.joinToString(separator = "\n") { it.owner.render() } +
@@ -79,8 +82,6 @@ class IrTypeSubstitutor(
        }
    }

-    private val substitution = typeParameters.zip(typeArguments).toMap()
-
    override fun getSubstitutionArgument(typeParameter: IrTypeParameterSymbol): IrTypeArgument =
        substitution[typeParameter]
            ?: typeParameter.takeIf { allowEmptySubstitution }?.owner?.defaultType
@@ -595,17 +595,12 @@ interface IrTypeSystemContext : TypeSystemContext, TypeSystemCommonSuperTypesCon
    }

    override fun typeSubstitutorByTypeConstructor(map: Map<TypeConstructorMarker, KotlinTypeMarker>): TypeSubstitutorMarker {
-        val typeParameters = mutableListOf<IrTypeParameterSymbol>()
-        val typeArguments = mutableListOf<IrTypeArgument>()
-        for ((key, value) in map) {
-            typeParameters += key as IrTypeParameterSymbol
-            typeArguments += value as IrTypeArgument
-        }
-        return IrTypeSubstitutor(typeParameters, typeArguments)
+        @Suppress("UNCHECKED_CAST")
+        return IrTypeSubstitutor(map as Map<IrTypeParameterSymbol, IrTypeArgument>)
    }

    override fun createEmptySubstitutor(): TypeSubstitutorMarker {
-        return IrTypeSubstitutor(emptyList(), emptyList())
+        return IrTypeSubstitutor(emptyMap())
    }

    override fun TypeSubstitutorMarker.safeSubstitute(type: KotlinTypeMarker): KotlinTypeMarker {
@@ -208,8 +208,6 @@ class IrBodyDeserializer(
                return IrSimpleTypeBuilder().apply { classifier = klass.symbol }.buildSimpleType()
            }

-            val typeArguments = ArrayList<IrTypeArgument>(typeParameters.size)
-            val typeParameterSymbols = ArrayList<IrTypeParameterSymbol>(typeParameters.size)
            val rawType = with(IrSimpleTypeBuilder()) {
                arguments = typeParameters.memoryOptimizedMap {
                    classifier = it.symbol
@@ -219,15 +217,15 @@ class IrBodyDeserializer(
                buildSimpleType()
            }

+            val typeParametersToArguments = HashMap<IrTypeParameterSymbol, IrTypeArgument>(typeParameters.size)
            for (i in typeParameters.indices) {
                val typeParameter = typeParameters[i]
                val callTypeArgument = constructorCall.getTypeArgument(i) ?: error("No type argument for id $i")
                val typeArgument = makeTypeProjection(callTypeArgument, typeParameter.variance)
-                typeArguments.add(typeArgument)
-                typeParameterSymbols.add(typeParameter.symbol)
+                typeParametersToArguments[typeParameter.symbol] = typeArgument
            }

-            val substitutor = IrTypeSubstitutor(typeParameterSymbols, typeArguments)
+            val substitutor = IrTypeSubstitutor(typeParametersToArguments)
            return substitutor.substitute(rawType) as IrSimpleType
        }
    }
@@ -20,6 +20,7 @@ import org.jetbrains.kotlin.utils.SmartList
 import org.jetbrains.kotlin.utils.addToStdlib.enumMapOf
 import org.jetbrains.kotlin.utils.addToStdlib.enumSetOf
 import org.jetbrains.kotlin.utils.keysToMap
+import org.jetbrains.kotlin.utils.zipIfSizesAreEqual
 import java.util.*

 object AbstractExpectActualCompatibilityChecker {
@@ -124,8 +125,8 @@ object AbstractExpectActualCompatibilityChecker {
        }

        val substitutor = createExpectActualTypeParameterSubstitutor(
-            expectTypeParameterSymbols,
-            actualTypeParameterSymbols,
+            (expectTypeParameterSymbols zipIfSizesAreEqual actualTypeParameterSymbols)
+                ?: error("expect/actual type parameters sizes are checked above"),
            parentSubstitutor
        )

@@ -348,8 +349,8 @@ object AbstractExpectActualCompatibilityChecker {
        }

        val substitutor = createExpectActualTypeParameterSubstitutor(
-            expectedTypeParameters,
-            actualTypeParameters,
+            (expectedTypeParameters zipIfSizesAreEqual actualTypeParameters)
+                ?: error("expect/actual type parameters sizes are checked above"),
            parentSubstitutor
        )

@@ -103,8 +103,7 @@ interface ExpectActualMatchingContext<T : DeclarationSymbolMarker> : TypeSystemC
    val PropertySymbolMarker.setter: FunctionSymbolMarker?

    fun createExpectActualTypeParameterSubstitutor(
-        expectTypeParameters: List<TypeParameterSymbolMarker>,
-        actualTypeParameters: List<TypeParameterSymbolMarker>,
+        expectActualTypeParameters: List<Pair<TypeParameterSymbolMarker, TypeParameterSymbolMarker>>,
        parentSubstitutor: TypeSubstitutorMarker?
    ): TypeSubstitutorMarker

@@ -22,6 +22,9 @@ fun <K, V> Iterable<K>.keysToMap(value: (K) -> V): Map<K, V> {
    return associateBy({ it }, value)
 }

+infix fun <A, B> Collection<A>.zipIfSizesAreEqual(other: Collection<B>): List<Pair<A, B>>? =
+    if (size == other.size) zip(other) else null
+
 fun <K, V : Any> Iterable<K>.keysToMapExceptNulls(value: (K) -> V?): Map<K, V> {
    val map = LinkedHashMap<K, V>()
    for (k in this) {