From 8bab6c3076d41ab2520172ab49f36c5d259b26ea Mon Sep 17 00:00:00 2001 From: "sebastian.sellmair" Date: Mon, 12 Jul 2021 16:21:43 +0200 Subject: [PATCH] [Commonizer] Commonize 'val' and 'var' properties If a setter is not present on all platforms a fallback private setter shall be emitted. ^KT-47502 Verification Pending ^KT-47691 Verification Pending --- .../commonizer/core/AssociativeCommonizer.kt | 26 ++++ .../commonizer/core/PropertyCommonizer.kt | 11 +- .../core/PropertySetterCommonizer.kt | 27 +++- .../setters/commonized/common/package_root.kt | 10 +- .../core/PropertySetterCommonizerTest.kt | 137 ++++++------------ 5 files changed, 111 insertions(+), 100 deletions(-) diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/AssociativeCommonizer.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/AssociativeCommonizer.kt index e7e42747f68..ab42fbca591 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/AssociativeCommonizer.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/AssociativeCommonizer.kt @@ -34,4 +34,30 @@ open class AssociativeCommonizerAdapter( _result = commonizer.commonize(currentResult, next) return _result != null } +} + +fun AssociativeCommonizer.asNullableCommonizer(): Commonizer = NullableAssociativeCommonizerAdapter(this) + +open class NullableAssociativeCommonizerAdapter( + private val commonizer: AssociativeCommonizer +) : Commonizer { + + private var isInitialized = false + private var _result: T? = null + + override val result: T? + get() = _result + + override fun commonizeWith(next: T?): Boolean { + val currentResult = _result + + if (!isInitialized) { + _result = next + isInitialized = true + return true + } + + _result = commonizer.commonize(currentResult, next) + return true + } } \ No newline at end of file diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/PropertyCommonizer.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/PropertyCommonizer.kt index 38e7fc273d3..c429169dbb4 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/PropertyCommonizer.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/PropertyCommonizer.kt @@ -10,14 +10,19 @@ import org.jetbrains.kotlin.commonizer.cir.CirProperty import org.jetbrains.kotlin.commonizer.cir.CirPropertyGetter import org.jetbrains.kotlin.commonizer.core.PropertyCommonizer.ConstCommonizationState.* import org.jetbrains.kotlin.commonizer.mergedtree.CirKnownClassifiers +import org.jetbrains.kotlin.descriptors.Modality class PropertyCommonizer(classifiers: CirKnownClassifiers) : AbstractFunctionOrPropertyCommonizer(classifiers) { - private val setter = PropertySetterCommonizer() + private val setter = PropertySetterCommonizer.asNullableCommonizer() private var isExternal = true private lateinit var constCommonizationState: ConstCommonizationState override fun commonizationResult(): CirProperty { - val setter = setter.result + val modality = modality.result + + val setter = setter.result?.takeIf { setter -> + setter !== PropertySetterCommonizer.privateFallbackSetter || modality == Modality.FINAL + } val constCommonizationState = constCommonizationState val constCompileTimeInitializer = (constCommonizationState as? ConstSameValue)?.compileTimeInitializer @@ -27,7 +32,7 @@ class PropertyCommonizer(classifiers: CirKnownClassifiers) : AbstractFunctionOrP name = name, typeParameters = typeParameters.result, visibility = visibility.result, - modality = modality.result, + modality = modality, containingClass = null, // does not matter isExternal = isExternal, extensionReceiver = extensionReceiver.result, diff --git a/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/PropertySetterCommonizer.kt b/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/PropertySetterCommonizer.kt index 6bedaf2274f..26388b4f615 100644 --- a/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/PropertySetterCommonizer.kt +++ b/native/commonizer/src/org/jetbrains/kotlin/commonizer/core/PropertySetterCommonizer.kt @@ -5,12 +5,25 @@ package org.jetbrains.kotlin.commonizer.core -import org.jetbrains.kotlin.commonizer.cir.CirHasVisibility import org.jetbrains.kotlin.commonizer.cir.CirPropertySetter -import org.jetbrains.kotlin.descriptors.Visibility +import org.jetbrains.kotlin.descriptors.Visibilities -class PropertySetterCommonizer : AbstractNullableCommonizer( - wrappedCommonizerFactory = { VisibilityCommonizer.equalizing() }, - extractor = { it }, - builder = CirPropertySetter::createDefaultNoAnnotations -) +object PropertySetterCommonizer : AssociativeCommonizer { + override fun commonize(first: CirPropertySetter?, second: CirPropertySetter?): CirPropertySetter? { + if (first == null && second == null) return null + if (first != null && second == null) return privateFallbackSetter + if (first == null && second != null) return privateFallbackSetter + + if (first != null && second != null) { + if (Visibilities.compare(first.visibility, second.visibility) == 0) { + return CirPropertySetter.createDefaultNoAnnotations(first.visibility) + } + + return privateFallbackSetter + } + + return null + } + + val privateFallbackSetter = CirPropertySetter.createDefaultNoAnnotations(Visibilities.Private) +} \ No newline at end of file diff --git a/native/commonizer/testData/propertyCommonization/setters/commonized/common/package_root.kt b/native/commonizer/testData/propertyCommonization/setters/commonized/common/package_root.kt index fd350302eb7..6ad84801dec 100644 --- a/native/commonizer/testData/propertyCommonization/setters/commonized/common/package_root.kt +++ b/native/commonizer/testData/propertyCommonization/setters/commonized/common/package_root.kt @@ -9,8 +9,16 @@ expect var setterWithDelegation1: Int expect var setterWithDelegation2: Int expect var defaultSetteCustomVisibility1: Int +expect var defaultSetteCustomVisibility2: Int + private set expect var defaultSetteCustomVisibility3 = 42 internal set +expect var defaultSetteCustomVisibility4: Int + private set +expect var defaultSetteCustomVisibility5: Int + private set expect val propertyWithoutSetter: Int -expect var propertyWithSetter: Int +expect var propertyMaybeSetter: Int + private set +expect var propertyWithSetter: Int \ No newline at end of file diff --git a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/core/PropertySetterCommonizerTest.kt b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/core/PropertySetterCommonizerTest.kt index 356dbcf877c..3185074ca2c 100644 --- a/native/commonizer/tests/org/jetbrains/kotlin/commonizer/core/PropertySetterCommonizerTest.kt +++ b/native/commonizer/tests/org/jetbrains/kotlin/commonizer/core/PropertySetterCommonizerTest.kt @@ -6,101 +6,60 @@ package org.jetbrains.kotlin.commonizer.core import org.jetbrains.kotlin.commonizer.cir.CirPropertySetter -import org.jetbrains.kotlin.descriptors.Visibilities.Internal -import org.jetbrains.kotlin.descriptors.Visibilities.Local -import org.jetbrains.kotlin.descriptors.Visibilities.Private -import org.jetbrains.kotlin.descriptors.Visibilities.Protected -import org.jetbrains.kotlin.descriptors.Visibilities.Public -import org.jetbrains.kotlin.descriptors.Visibility +import org.jetbrains.kotlin.descriptors.Visibilities import org.junit.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull +import kotlin.test.assertSame -class PropertySetterCommonizerTest : AbstractCommonizerTest() { +class PropertySetterCommonizerTest { @Test - fun missingOnly() = super.doTestSuccess( - expected = null, - null, null, null - ) - - @Test(expected = IllegalCommonizerStateException::class) - fun missingAndPublic() = doTestFailure( - null, null, null, Public - ) - - @Test(expected = IllegalCommonizerStateException::class) - fun publicAndMissing() = doTestFailure( - Public, Public, Public, null - ) - - @Test(expected = IllegalCommonizerStateException::class) - fun protectedAndMissing() = doTestFailure( - Protected, Protected, null - ) - - @Test(expected = IllegalCommonizerStateException::class) - fun missingAndInternal() = doTestFailure( - null, null, Internal - ) - - @Test - fun publicOnly() = doTestSuccess( - expected = Public, - Public, Public, Public - ) - - @Test - fun protectedOnly() = doTestSuccess( - expected = Protected, - Protected, Protected, Protected - ) - - @Test - fun internalOnly() = doTestSuccess( - expected = Internal, - Internal, Internal, Internal - ) - - @Test(expected = IllegalCommonizerStateException::class) - fun privateOnly() = doTestFailure( - Private - ) - - @Test(expected = IllegalCommonizerStateException::class) - fun publicAndProtected() = doTestFailure( - Public, Public, Protected - ) - - @Test(expected = IllegalCommonizerStateException::class) - fun publicAndInternal() = doTestFailure( - Public, Public, Internal - ) - - @Test(expected = IllegalCommonizerStateException::class) - fun protectedAndInternal() = doTestFailure( - Protected, Protected, Internal - ) - - @Test(expected = IllegalCommonizerStateException::class) - fun publicAndPrivate() = doTestFailure( - Public, Public, Private - ) - - @Test(expected = IllegalCommonizerStateException::class) - fun somethingUnexpected() = doTestFailure( - Public, Local - ) - - private fun doTestSuccess(expected: Visibility?, vararg variants: Visibility?) = - super.doTestSuccess( - expected = expected?.let { CirPropertySetter.createDefaultNoAnnotations(expected) }, - *variants.map { it?.let(CirPropertySetter::createDefaultNoAnnotations) }.toTypedArray() + fun `missing only`() { + assertNull( + PropertySetterCommonizer.commonize(null, null), + "Expected no property setting being commonized from nulls" ) + } - private fun doTestFailure(vararg variants: Visibility?) = - super.doTestFailure( - *variants.map { it?.let(CirPropertySetter::createDefaultNoAnnotations) }.toTypedArray(), - shouldFailOnFirstVariant = false + @Test + fun `missing and public`() { + assertSame( + PropertySetterCommonizer.privateFallbackSetter, + PropertySetterCommonizer.commonize(null, CirPropertySetter.createDefaultNoAnnotations(Visibilities.Public)) ) + } - override fun createCommonizer() = PropertySetterCommonizer() + @Test + fun `public public`() { + assertEquals( + CirPropertySetter.createDefaultNoAnnotations(Visibilities.Public), + PropertySetterCommonizer.commonize( + CirPropertySetter.createDefaultNoAnnotations(Visibilities.Public), + CirPropertySetter.createDefaultNoAnnotations(Visibilities.Public) + ) + ) + } + + @Test + fun `internal internal`() { + assertEquals( + CirPropertySetter.createDefaultNoAnnotations(Visibilities.Internal), + PropertySetterCommonizer.commonize( + CirPropertySetter.createDefaultNoAnnotations(Visibilities.Internal), + CirPropertySetter.createDefaultNoAnnotations(Visibilities.Internal) + ) + ) + } + + @Test + fun `internal public`() { + assertEquals( + PropertySetterCommonizer.privateFallbackSetter, + PropertySetterCommonizer.commonize( + CirPropertySetter.createDefaultNoAnnotations(Visibilities.Internal), + CirPropertySetter.createDefaultNoAnnotations(Visibilities.Public) + ) + ) + } }