From b9189be7282f13c61ea703eb8bfda3ecd7a8a7b2 Mon Sep 17 00:00:00 2001 From: Artem Kobzar Date: Fri, 29 Jul 2022 12:41:17 +0000 Subject: [PATCH] [K/JS] fix(KT-46643): prevent removal of setter for overridden external field. --- .../jetbrains/kotlin/ir/backend/js/dce/Dce.kt | 3 +- .../js/dce/UsefulDeclarationProcessor.kt | 12 +++ .../transformers/irToJs/JsClassGenerator.kt | 10 +- .../kotlin/js/test/BoxJsTestGenerated.java | 6 ++ .../kotlin/js/test/ir/FirJsTestGenerated.java | 6 ++ .../js/test/ir/IrBoxJsTestGenerated.java | 6 ++ .../JsTranslatorWasmTestGenerated.java | 5 + .../box/native/externalValWithOverridenVar.js | 3 + .../box/native/externalValWithOverridenVar.kt | 91 +++++++++++++++++++ 9 files changed, 138 insertions(+), 4 deletions(-) create mode 100644 js/js.translator/testData/box/native/externalValWithOverridenVar.js create mode 100644 js/js.translator/testData/box/native/externalValWithOverridenVar.kt diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/dce/Dce.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/dce/Dce.kt index c57c571fc24..3a8ee77c33d 100644 --- a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/dce/Dce.kt +++ b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/dce/Dce.kt @@ -77,7 +77,8 @@ private fun IrDeclaration.addRootsTo( } } this is IrSimpleFunction -> { - if (correspondingPropertySymbol?.owner?.isExported(context) == true) { + val correspondingProperty = correspondingPropertySymbol?.owner ?: return + if (correspondingProperty.isExported(context)) { acceptVoid(nestedVisitor) } } diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/dce/UsefulDeclarationProcessor.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/dce/UsefulDeclarationProcessor.kt index c171b69ffd4..6a6f391ed1b 100644 --- a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/dce/UsefulDeclarationProcessor.kt +++ b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/dce/UsefulDeclarationProcessor.kt @@ -185,6 +185,10 @@ abstract class UsefulDeclarationProcessor( } } + if (declaration is IrSimpleFunction && declaration.isAccessorForOverriddenExternalField()) { + declaration.enqueue(declaration.correspondingPropertySymbol!!.owner, "overrides external property") + } + // A hack to enforce property lowering. // Until a getter is accessed it doesn't get moved to the declaration list. if (declaration is IrProperty) { @@ -197,6 +201,14 @@ abstract class UsefulDeclarationProcessor( } } + private fun IrSimpleFunction.isAccessorForOverriddenExternalField(): Boolean { + return correspondingPropertySymbol?.owner?.isExternalOrOverriddenExternal() ?: false + } + + private fun IrProperty.isExternalOrOverriddenExternal(): Boolean { + return isEffectivelyExternal() || overriddenSymbols.any { it.owner.isExternalOrOverriddenExternal() } + } + protected open fun handleAssociatedObjects(): Unit = Unit fun collectDeclarations(rootDeclarations: Iterable): Set { diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/transformers/irToJs/JsClassGenerator.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/transformers/irToJs/JsClassGenerator.kt index d68a16f6f68..6ff669df8ae 100644 --- a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/transformers/irToJs/JsClassGenerator.kt +++ b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/transformers/irToJs/JsClassGenerator.kt @@ -416,7 +416,7 @@ fun IrSimpleFunction?.shouldExportAccessor(context: JsIrBackendContext): Boolean if (parentAsClass.isExported(context)) return true - return overriddenStableProperty(context) + return isAccessorOfOverriddenStableProperty(context) } fun IrSimpleFunction.overriddenStableProperty(context: JsIrBackendContext): Boolean { @@ -429,10 +429,14 @@ fun IrSimpleFunction.overriddenStableProperty(context: JsIrBackendContext): Bool return overridesExternal() || property.getJsName() != null } -private fun IrSimpleFunction.overridesExternal(): Boolean { +fun IrSimpleFunction.isAccessorOfOverriddenStableProperty(context: JsIrBackendContext): Boolean { + return overriddenStableProperty(context) || correspondingPropertySymbol!!.owner.overridesExternal() +} + +private fun IrOverridableDeclaration<*>.overridesExternal(): Boolean { if (this.isEffectivelyExternal()) return true - return this.overriddenSymbols.any { it.owner.overridesExternal() } + return overriddenSymbols.any { (it.owner as IrOverridableDeclaration<*>).overridesExternal() } } private val IrClassifierSymbol.isInterface get() = (owner as? IrClass)?.isInterface == true diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/BoxJsTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/BoxJsTestGenerated.java index 7d9834761af..b8463701101 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/BoxJsTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/BoxJsTestGenerated.java @@ -7498,6 +7498,12 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest { runTest("js/js.translator/testData/box/native/externalNestedEnum.kt"); } + @Test + @TestMetadata("externalValWithOverridenVar.kt") + public void testExternalValWithOverridenVar() throws Exception { + runTest("js/js.translator/testData/box/native/externalValWithOverridenVar.kt"); + } + @Test @TestMetadata("inheritanceFromNativeClass.kt") public void testInheritanceFromNativeClass() throws Exception { diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/FirJsTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/FirJsTestGenerated.java index 6525ba1080e..8b80633a18a 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/FirJsTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/FirJsTestGenerated.java @@ -7970,6 +7970,12 @@ public class FirJsTestGenerated extends AbstractFirJsTest { runTest("js/js.translator/testData/box/native/externalNestedEnum.kt"); } + @Test + @TestMetadata("externalValWithOverridenVar.kt") + public void testExternalValWithOverridenVar() throws Exception { + runTest("js/js.translator/testData/box/native/externalValWithOverridenVar.kt"); + } + @Test @TestMetadata("inheritanceFromNativeClass.kt") public void testInheritanceFromNativeClass() throws Exception { diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsTestGenerated.java index 4e2cfa56be3..395c4304773 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/test/ir/IrBoxJsTestGenerated.java @@ -7970,6 +7970,12 @@ public class IrBoxJsTestGenerated extends AbstractIrBoxJsTest { runTest("js/js.translator/testData/box/native/externalNestedEnum.kt"); } + @Test + @TestMetadata("externalValWithOverridenVar.kt") + public void testExternalValWithOverridenVar() throws Exception { + runTest("js/js.translator/testData/box/native/externalValWithOverridenVar.kt"); + } + @Test @TestMetadata("inheritanceFromNativeClass.kt") public void testInheritanceFromNativeClass() throws Exception { diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/testOld/wasm/semantics/JsTranslatorWasmTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/testOld/wasm/semantics/JsTranslatorWasmTestGenerated.java index 65f977569ce..b6c6dfbf269 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/js/testOld/wasm/semantics/JsTranslatorWasmTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/js/testOld/wasm/semantics/JsTranslatorWasmTestGenerated.java @@ -125,6 +125,11 @@ public class JsTranslatorWasmTestGenerated extends AbstractJsTranslatorWasmTest runTest("js/js.translator/testData/box/native/classObject.kt"); } + @TestMetadata("externalValWithOverridenVar.kt") + public void testExternalValWithOverridenVar() throws Exception { + runTest("js/js.translator/testData/box/native/externalValWithOverridenVar.kt"); + } + @TestMetadata("kt2209.kt") public void testKt2209() throws Exception { runTest("js/js.translator/testData/box/native/kt2209.kt"); diff --git a/js/js.translator/testData/box/native/externalValWithOverridenVar.js b/js/js.translator/testData/box/native/externalValWithOverridenVar.js new file mode 100644 index 00000000000..8bb278b638a --- /dev/null +++ b/js/js.translator/testData/box/native/externalValWithOverridenVar.js @@ -0,0 +1,3 @@ +function Base() { + this.foo = "K" +} diff --git a/js/js.translator/testData/box/native/externalValWithOverridenVar.kt b/js/js.translator/testData/box/native/externalValWithOverridenVar.kt new file mode 100644 index 00000000000..7bf37e047ef --- /dev/null +++ b/js/js.translator/testData/box/native/externalValWithOverridenVar.kt @@ -0,0 +1,91 @@ +// KT-46643 +// IGNORE_BACKEND: WASM +// WITH_STDLIB + +import kotlin.reflect.KProperty + +external interface IBase { + val foo: String +} + +external abstract class Base : IBase + +open class A : Base() { + override var foo: String = "Error: A setter was not called." + set(k) { result = "O$k"} + + lateinit var result: String +} + +open class B : Base() { + override val foo: String = "OK" + + open val result: String get() = foo +} + +class C : B() { + override var foo: String = "Error: C setter was not called." + set(k) { result = "O$k"} + + override lateinit var result: String +} + +open class D : B() { + override val foo: String = "OK" +} + +open class E : D() { + override var foo: String = "Error: E setter was not called." + set(k) { result = "O$k"} + + override lateinit var result: String +} + +open class F: B() { + override var foo: String by CustomDelegator + + private object CustomDelegator { + operator fun getValue(thisRef: Any?, property: KProperty<*>): String { + return "Error: F setter was not called." + } + + operator fun setValue(thisRef: Any?, property: KProperty<*>, value: String) { + result = "O$value" + } + + lateinit var result: String + } + + override val result: String get() = CustomDelegator.result +} + +class G(val b: B): IBase by b { + val result: String get() = b.result +} + +fun box(): String { + val a = A() + if (a.result != "OK") return a.foo + + val b = B() + if (b.result != "OK") return b.foo + + val c = C() + if (c.result != "OK") return c.foo + + val d = D() + if (d.result != "OK") return d.foo + + val e = E() + if (e.result != "OK") return e.foo + + try { + val f = F() + return "Failed: it should not work for now, because of delegating objects initialization order" + } catch (e: Throwable) {} + + val g = G(e) + if (g.result != "OK") return g.foo + + return "OK" +} \ No newline at end of file