[Commonizer] Implement TypeNullabilityCommonizer and ReturnTypeCommonizer

^KT-48567 Verification Pending
2021-09-04 14:51:57 +02:00
parent b13f3599cf
commit 9e34382db5
10 changed files with 102 additions and 56 deletions
@@ -5,9 +5,11 @@

 package org.jetbrains.kotlin.commonizer.core

-import org.jetbrains.kotlin.commonizer.cir.*
+import org.jetbrains.kotlin.commonizer.cir.CirFunctionOrProperty
+import org.jetbrains.kotlin.commonizer.cir.CirName
+import org.jetbrains.kotlin.commonizer.cir.CirType
+import org.jetbrains.kotlin.commonizer.core.TypeCommonizer.Options.Companion.default
 import org.jetbrains.kotlin.commonizer.mergedtree.CirKnownClassifiers
-import org.jetbrains.kotlin.commonizer.utils.safeCastValues
 import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor
 import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor.Kind.DELEGATION
 import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor.Kind.SYNTHESIZED
@@ -45,19 +47,7 @@ private class ReturnTypeCommonizer(
    override fun invoke(values: List<CirFunctionOrProperty>): CirType? {
        if (values.isEmpty()) return null
        val isTopLevel = values.all { it.containingClass == null }
-        val returnTypes = if (isTopLevel) makeNullableIfNecessary(values.map { it.returnType }) else values.map { it.returnType }
-        return TypeCommonizer(classifiers).asCommonizer().commonize(returnTypes)
-    }
-
-    private fun makeNullableIfNecessary(types: List<CirType>): List<CirType> {
-        val simpleTypes = types.safeCastValues<CirType, CirSimpleType>() ?: return types
-
-        if (
-            simpleTypes.all { type -> type.isMarkedNullable } ||
-            simpleTypes.none { type -> type.isMarkedNullable }
-        ) return types
-
-        return simpleTypes.map { type -> type.makeNullable() }
+        return TypeCommonizer(classifiers, default.withCovariantNullabilityCommonizationEnabled(isTopLevel))
+            .asCommonizer().commonize(values.map { it.returnType })
    }
 }
-
@@ -17,8 +17,8 @@ internal class ClassOrTypeAliasTypeCommonizer(

    override fun commonize(first: CirClassOrTypeAliasType, second: CirClassOrTypeAliasType): CirClassOrTypeAliasType? {
        if (first is CirClassType && second is CirClassType) {
-            return ClassTypeCommonizer(classifiers).commonize(listOf(first, second))
-                ?: if (options.allowOptimisticNumberTypeCommonization) OptimisticNumbersTypeCommonizer.commonize(first, second) else null
+            return ClassTypeCommonizer(classifiers, options).commonize(listOf(first, second))
+                ?: if (options.enableOptimisticNumberTypeCommonization) OptimisticNumbersTypeCommonizer.commonize(first, second) else null
        }

        if (first is CirTypeAliasType && second is CirTypeAliasType) {
@@ -26,9 +26,10 @@ internal class ClassOrTypeAliasTypeCommonizer(
            In case regular type-alias-type commonization fails, we try to expand all type-aliases and
            try our luck with commonizing those class types
             */
-            return TypeAliasTypeCommonizer(classifiers).commonize(listOf(first, second))
-                ?: ClassOrTypeAliasTypeCommonizer(classifiers, options.withAllowOptimisticNumberTypeCommonization())
-                    .commonize(first.expandedType(), second.expandedType())
+            return TypeAliasTypeCommonizer(classifiers, options).commonize(listOf(first, second))
+                ?: ClassOrTypeAliasTypeCommonizer(
+                    classifiers, options.withOptimisticNumberTypeCommonizationEnabled()
+                ).commonize(first.expandedType(), second.expandedType())
        }

        val classType = when {
@@ -47,7 +48,7 @@ internal class ClassOrTypeAliasTypeCommonizer(
        TypeAliasCommonizer will be able to figure out if the typealias will be represented as expect class in common.
        If so, re-use this class type, otherwise: try to expand the typeAlias
         */
-        val typeAliasClassType = TypeAliasTypeCommonizer(classifiers).commonize(listOf(typeAliasType))?.expandedType()
+        val typeAliasClassType = TypeAliasTypeCommonizer(classifiers, options).commonize(listOf(typeAliasType))?.expandedType()
            ?: typeAliasType.expandedType()

        return commonize(classType, typeAliasClassType)
@@ -11,13 +11,16 @@ import org.jetbrains.kotlin.commonizer.mergedtree.CirKnownClassifiers
 import org.jetbrains.kotlin.commonizer.utils.isUnderKotlinNativeSyntheticPackages
 import org.jetbrains.kotlin.descriptors.Visibility

-internal class ClassTypeCommonizer(private val classifiers: CirKnownClassifiers) :
+internal class ClassTypeCommonizer(
+    private val classifiers: CirKnownClassifiers,
+    options: TypeCommonizer.Options = TypeCommonizer.Options.default
+) :
    AbstractStandardCommonizer<CirClassType, CirClassType>() {
    private lateinit var classId: CirEntityId
    private val outerType = OuterClassTypeCommonizer(classifiers)
    private lateinit var anyVisibility: Visibility
    private val arguments = TypeArgumentListCommonizer(classifiers)
-    private var isMarkedNullable = false
+    private val isMarkedNullable = TypeNullabilityCommonizer(options).asCommonizer()

    override fun commonizationResult() = CirClassType.createInterned(
        classId = classId,
@@ -28,17 +31,16 @@ internal class ClassTypeCommonizer(private val classifiers: CirKnownClassifiers)
        // to reach better interning rate.
        visibility = anyVisibility,
        arguments = arguments.result,
-        isMarkedNullable = isMarkedNullable
+        isMarkedNullable = isMarkedNullable.result
    )

    override fun initialize(first: CirClassType) {
        classId = first.classifierId
        anyVisibility = first.visibility
-        isMarkedNullable = first.isMarkedNullable
    }

    override fun doCommonizeWith(next: CirClassType) =
-        isMarkedNullable == next.isMarkedNullable
+        isMarkedNullable.commonizeWith(next.isMarkedNullable)
                && classId == next.classifierId
                && outerType.commonizeWith(next.outerType)
                && isClassifierAvailableInCommon(classifiers, classId)
@@ -0,0 +1,23 @@
+/*
+ * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.commonizer.core
+
+interface NullableContextualSingleInvocationCommonizer<T : Any, R : Any> {
+    operator fun invoke(values: List<T>): R?
+}
+
+fun <Context : Any, R : Any> NullableContextualSingleInvocationCommonizer<Context, R>.asCommonizer(): Commonizer<Context, R?> =
+    object : Commonizer<Context, R?> {
+        private val collectedValues = mutableListOf<Context>()
+
+        override val result: R?
+            get() = this@asCommonizer.invoke(collectedValues)
+
+        override fun commonizeWith(next: Context): Boolean {
+            collectedValues.add(next)
+            return true
+        }
+    }
@@ -22,18 +22,3 @@ fun <T : Any> NullableSingleInvocationCommonizer<T>.asCommonizer(): Commonizer<T
 }


-interface NullableContextualSingleInvocationCommonizer<T : Any, R : Any> {
-    operator fun invoke(values: List<T>): R?
-}
-
-fun <T : Any, R: Any> NullableContextualSingleInvocationCommonizer<T, R>.asCommonizer(): Commonizer<T, R?> = object : Commonizer<T, R?> {
-    private val collectedValues = mutableListOf<T>()
-
-    override val result: R?
-        get() = this@asCommonizer.invoke(collectedValues)
-
-    override fun commonizeWith(next: T): Boolean {
-        collectedValues.add(next)
-        return true
-    }
-}
@@ -22,7 +22,7 @@ class TypeAliasCommonizer(

        val typeParameters = TypeParameterListCommonizer(classifiers).commonize(values.map { it.typeParameters }) ?: return null

-        val underlyingType = TypeCommonizer(classifiers, TypeCommonizer.Options.default.withAllowOptimisticNumberTypeCommonization())
+        val underlyingType = TypeCommonizer(classifiers, TypeCommonizer.Options.default.withOptimisticNumberTypeCommonizationEnabled())
            .asCommonizer().commonize(values.map { it.underlyingType }) as? CirClassOrTypeAliasType ?: return null

        val visibility = VisibilityCommonizer.lowering().commonize(values) ?: return null
@@ -10,13 +10,16 @@ import org.jetbrains.kotlin.commonizer.core.CommonizedTypeAliasAnswer.Companion.
 import org.jetbrains.kotlin.commonizer.core.CommonizedTypeAliasAnswer.Companion.SUCCESS_FROM_DEPENDENCY_LIBRARY
 import org.jetbrains.kotlin.commonizer.mergedtree.CirKnownClassifiers

-internal class TypeAliasTypeCommonizer(private val classifiers: CirKnownClassifiers) :
+internal class TypeAliasTypeCommonizer(
+    private val classifiers: CirKnownClassifiers,
+    options: TypeCommonizer.Options
+) :
    AbstractStandardCommonizer<CirTypeAliasType, CirClassOrTypeAliasType>() {

    private lateinit var typeAliasId: CirEntityId
    private val arguments = TypeArgumentListCommonizer(classifiers)
    private val underlyingTypeArguments = TypeArgumentListCommonizer(classifiers)
-    private var isMarkedNullable = false
+    private val isMarkedNullable = TypeNullabilityCommonizer(options).asCommonizer()
    private var commonizedTypeBuilder: CommonizedTypeAliasTypeBuilder? = null // null means not selected yet

    override fun commonizationResult() =
@@ -24,17 +27,16 @@ internal class TypeAliasTypeCommonizer(private val classifiers: CirKnownClassifi
            typeAliasId = typeAliasId,
            arguments = arguments.result,
            underlyingTypeArguments = underlyingTypeArguments.result,
-            isMarkedNullable = isMarkedNullable
+            isMarkedNullable = isMarkedNullable.result
        )

    override fun initialize(first: CirTypeAliasType) {
        typeAliasId = first.classifierId
-        isMarkedNullable = first.expandedType().isMarkedNullable
    }

    override fun doCommonizeWith(next: CirTypeAliasType): Boolean {
-        if (isMarkedNullable != next.expandedType().isMarkedNullable || typeAliasId != next.classifierId)
-            return false
+        if (typeAliasId != next.classifierId) return false
+        if (!isMarkedNullable.commonizeWith(next.expandedType().isMarkedNullable)) return false

        if (commonizedTypeBuilder == null) {
            val answer = commonizeTypeAlias(typeAliasId, classifiers)
@@ -30,12 +30,18 @@ class TypeCommonizer(
    }

    data class Options(
-        val allowOptimisticNumberTypeCommonization: Boolean = false
+        val enableOptimisticNumberTypeCommonization: Boolean = false,
+        val enableCovariantNullabilityCommonization: Boolean = false
    ) {

-        fun withAllowOptimisticNumberTypeCommonization(): Options {
-            return if (allowOptimisticNumberTypeCommonization) this
-            else copy(allowOptimisticNumberTypeCommonization = true)
+        fun withOptimisticNumberTypeCommonizationEnabled(enabled: Boolean = true): Options {
+            return if (enableOptimisticNumberTypeCommonization == enabled) this
+            else copy(enableOptimisticNumberTypeCommonization = enabled)
+        }
+
+        fun withCovariantNullabilityCommonizationEnabled(enabled: Boolean = true): Options {
+            return if (enableCovariantNullabilityCommonization == enabled) this
+            else copy(enableCovariantNullabilityCommonization = enabled)
        }

        companion object {
@@ -0,0 +1,28 @@
+/*
+ * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors.
+ * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file.
+ */
+
+package org.jetbrains.kotlin.commonizer.core
+
+private typealias IsMarkedNullable = Boolean
+
+internal interface TypeNullabilityCommonizer : AssociativeCommonizer<IsMarkedNullable>
+
+internal fun TypeNullabilityCommonizer(options: TypeCommonizer.Options): TypeNullabilityCommonizer {
+    return if (options.enableCovariantNullabilityCommonization) CovariantTypeNullabilityCommonizer
+    else EqualTypeNullabilityCommonizer
+}
+
+private object CovariantTypeNullabilityCommonizer : TypeNullabilityCommonizer {
+    override fun commonize(first: IsMarkedNullable, second: IsMarkedNullable): IsMarkedNullable {
+        return first || second
+    }
+}
+
+private object EqualTypeNullabilityCommonizer : TypeNullabilityCommonizer {
+    override fun commonize(first: IsMarkedNullable, second: IsMarkedNullable): IsMarkedNullable? {
+        if (first != second) return null
+        return first
+    }
+}
@@ -92,7 +92,7 @@ class ReturnTypeNullabilityCommonizationTest : AbstractInlineSourcesCommonizatio
        result.assertCommonized(
            "(a, b)", """
                expect class X
-                expect fun x(): X
+                expect fun x(): X?
            """.trimIndent()
        )
    }
@@ -124,7 +124,9 @@ class ReturnTypeNullabilityCommonizationTest : AbstractInlineSourcesCommonizatio
                typealias Z = Any
                expect class Y
                expect class X
-                fun x(): X
+                
+                // Still cary nullability mark here to be extra safe
+                expect fun x(): X?
            """.trimIndent()
        )
    }
@@ -156,7 +158,8 @@ class ReturnTypeNullabilityCommonizationTest : AbstractInlineSourcesCommonizatio
                typealias Z = Any
                expect class Y
                expect class X
-                expect val x: X
+                // Still cary nullability mark here to be more safe!
+                expect val x: X?
            """.trimIndent()
        )
    }
@@ -176,6 +179,12 @@ class ReturnTypeNullabilityCommonizationTest : AbstractInlineSourcesCommonizatio
        result.assertCommonized("(a, b, c, d)", "expect val x: Any?")
    }

+
+    /*
+    We expect *no* covariant nullability commonization on any member function/property because this might mess
+    with overrides of super classes/interfaces
+     */
+
    fun `test member - property - hierarchically`() {
        val result = commonize {
            outputTarget("(a, b)", "(c, d)", "(a, b, c, d)")