[JS IR IC] Interface default implementations affect IC hash

Adding or removing a method or property with a default implementation to an interface should invalidate all children: we must regenerate JS code for them ^KT-56237 Fixed
2023-04-25 14:53:34 +02:00
parent bad5c58952
commit 81b591ed21
45 changed files with 391 additions and 70 deletions
@@ -10,15 +10,14 @@ import org.jetbrains.kotlin.config.CompilerConfiguration
 import org.jetbrains.kotlin.config.CompilerConfigurationKey
 import org.jetbrains.kotlin.config.KotlinCompilerVersion
 import org.jetbrains.kotlin.config.languageVersionSettings
+import org.jetbrains.kotlin.descriptors.Modality
 import org.jetbrains.kotlin.ir.backend.js.transformers.irToJs.CrossModuleReferences
 import org.jetbrains.kotlin.ir.IrElement
-import org.jetbrains.kotlin.ir.declarations.IrAnnotationContainer
-import org.jetbrains.kotlin.ir.declarations.IrFunction
-import org.jetbrains.kotlin.ir.declarations.IrProperty
-import org.jetbrains.kotlin.ir.declarations.IrTypeParametersContainer
+import org.jetbrains.kotlin.ir.declarations.*
 import org.jetbrains.kotlin.ir.linkage.partial.PartialLinkageConfig
 import org.jetbrains.kotlin.ir.symbols.IrSymbol
 import org.jetbrains.kotlin.ir.util.DumpIrTreeVisitor
+import org.jetbrains.kotlin.ir.util.isInterface
 import org.jetbrains.kotlin.js.config.JSConfigurationKeys
 import org.jetbrains.kotlin.library.impl.buffer
 import org.jetbrains.kotlin.protobuf.CodedInputStream
@@ -74,6 +73,48 @@ private class HashCalculatorForIC {
        updateForEach(annotationContainer.annotations, ::update)
    }

+    fun updateSymbol(symbol: IrSymbol) {
+        update(symbol.toString())
+
+        (symbol.owner as? IrClass)?.takeIf { it.isInterface }?.let { irInterface ->
+            // Adding or removing a method or property with a default implementation to an interface
+            // should invalidate all children: we must regenerate JS code for them
+            val openDeclarationSymbols = buildList {
+                for (decl in irInterface.declarations) {
+                    if (decl is IrOverridableMember && decl.modality == Modality.OPEN) {
+                        add(decl.symbol)
+                    }
+                    if (decl is IrProperty) {
+                        decl.getter?.takeIf { it.modality == Modality.OPEN }?.symbol?.let(::add)
+                        decl.setter?.takeIf { it.modality == Modality.OPEN }?.symbol?.let(::add)
+                    }
+                }
+            }
+            updateForEach(openDeclarationSymbols, ::updateSymbol)
+        }
+
+        // symbol rendering prints very little information about type parameters
+        // TODO may be it make sense to update rendering?
+        (symbol.owner as? IrTypeParametersContainer)?.let { typeParameters ->
+            updateForEach(typeParameters.typeParameters) { typeParameter ->
+                update(typeParameter.symbol.toString())
+            }
+        }
+        (symbol.owner as? IrFunction)?.let { irFunction ->
+            updateForEach(irFunction.valueParameters) { functionParam ->
+                // symbol rendering doesn't print default params information
+                // it is important to understand if default params were added or removed
+                update(functionParam.defaultValue?.let { 1 } ?: 0)
+            }
+        }
+        (symbol.owner as? IrAnnotationContainer)?.let(::updateAnnotationContainer)
+        (symbol.owner as? IrProperty)?.let { irProperty ->
+            if (irProperty.isConst) {
+                irProperty.backingField?.initializer?.let(::update)
+            }
+        }
+    }
+
    inline fun <T> updateForEach(collection: Collection<T>, f: (T) -> Unit) {
        update(collection.size)
        collection.forEach { f(it) }
@@ -92,7 +133,7 @@ private class HashCalculatorForIC {
        }
    }

-    fun finalize(): ICHash {
+    fun finalizeAndGetHash(): ICHash {
        val hashBytes = md5Digest.digest()
        md5Digest.reset()
        return hashBytes.buffer.asLongBuffer().let { longBuffer ->
@@ -142,42 +183,22 @@ internal class ICHasher {
        }

        hashCalculator.update(config.languageVersionSettings.toString())
-        return hashCalculator.finalize()
+        return hashCalculator.finalizeAndGetHash()
    }

    fun calculateIrFunctionHash(function: IrFunction): ICHash {
        hashCalculator.update(function)
-        return hashCalculator.finalize()
+        return hashCalculator.finalizeAndGetHash()
    }

    fun calculateIrAnnotationContainerHash(container: IrAnnotationContainer): ICHash {
        hashCalculator.updateAnnotationContainer(container)
-        return hashCalculator.finalize()
+        return hashCalculator.finalizeAndGetHash()
    }

    fun calculateIrSymbolHash(symbol: IrSymbol): ICHash {
-        hashCalculator.update(symbol.toString())
-        // symbol rendering prints very little information about type parameters
-        // TODO may be it make sense to update rendering?
-        (symbol.owner as? IrTypeParametersContainer)?.let { typeParameters ->
-            hashCalculator.updateForEach(typeParameters.typeParameters) { typeParameter ->
-                hashCalculator.update(typeParameter.symbol.toString())
-            }
-        }
-        (symbol.owner as? IrFunction)?.let { irFunction ->
-            hashCalculator.updateForEach(irFunction.valueParameters) { functionParam ->
-                // symbol rendering doesn't print default params information
-                // it is important to understand if default params were added or removed
-                hashCalculator.update(functionParam.defaultValue?.let { 1 } ?: 0)
-            }
-        }
-        (symbol.owner as? IrAnnotationContainer)?.let(hashCalculator::updateAnnotationContainer)
-        (symbol.owner as? IrProperty)?.let { irProperty ->
-            if (irProperty.isConst) {
-                irProperty.backingField?.initializer?.let(hashCalculator::update)
-            }
-        }
-        return hashCalculator.finalize()
+        hashCalculator.updateSymbol(symbol)
+        return hashCalculator.finalizeAndGetHash()
    }
 }

@@ -213,4 +234,4 @@ internal fun CrossModuleReferences.crossModuleReferencesHashForIC() = HashCalcul
            update(import.moduleExporter.internalName.toString())
        }
    }
-}.finalize()
+}.finalizeAndGetHash()
@@ -319,6 +319,12 @@ public class JsFirInvalidationTestGenerated extends AbstractJsFirInvalidationTes
        runTest("js/js.translator/testData/incremental/invalidation/localInlineFunction/");
    }

+    @Test
+    @TestMetadata("localObjectsLeakThroughInterface")
+    public void testLocalObjectsLeakThroughInterface() throws Exception {
+        runTest("js/js.translator/testData/incremental/invalidation/localObjectsLeakThroughInterface/");
+    }
+
    @Test
    @TestMetadata("mainModuleInvalidation")
    public void testMainModuleInvalidation() throws Exception {
@@ -373,6 +379,12 @@ public class JsFirInvalidationTestGenerated extends AbstractJsFirInvalidationTes
        runTest("js/js.translator/testData/incremental/invalidation/privateInlineFunction1/");
    }

+    @Test
+    @TestMetadata("privateObjectsLeakThroughSealedInterface")
+    public void testPrivateObjectsLeakThroughSealedInterface() throws Exception {
+        runTest("js/js.translator/testData/incremental/invalidation/privateObjectsLeakThroughSealedInterface/");
+    }
+
    @Test
    @TestMetadata("removeFile")
    public void testRemoveFile() throws Exception {
@@ -319,6 +319,12 @@ public class JsIrES6InvalidationTestGenerated extends AbstractJsIrES6Invalidatio
        runTest("js/js.translator/testData/incremental/invalidation/localInlineFunction/");
    }

+    @Test
+    @TestMetadata("localObjectsLeakThroughInterface")
+    public void testLocalObjectsLeakThroughInterface() throws Exception {
+        runTest("js/js.translator/testData/incremental/invalidation/localObjectsLeakThroughInterface/");
+    }
+
    @Test
    @TestMetadata("mainModuleInvalidation")
    public void testMainModuleInvalidation() throws Exception {
@@ -373,6 +379,12 @@ public class JsIrES6InvalidationTestGenerated extends AbstractJsIrES6Invalidatio
        runTest("js/js.translator/testData/incremental/invalidation/privateInlineFunction1/");
    }

+    @Test
+    @TestMetadata("privateObjectsLeakThroughSealedInterface")
+    public void testPrivateObjectsLeakThroughSealedInterface() throws Exception {
+        runTest("js/js.translator/testData/incremental/invalidation/privateObjectsLeakThroughSealedInterface/");
+    }
+
    @Test
    @TestMetadata("removeFile")
    public void testRemoveFile() throws Exception {
@@ -319,6 +319,12 @@ public class JsIrInvalidationTestGenerated extends AbstractJsIrInvalidationTest
        runTest("js/js.translator/testData/incremental/invalidation/localInlineFunction/");
    }

+    @Test
+    @TestMetadata("localObjectsLeakThroughInterface")
+    public void testLocalObjectsLeakThroughInterface() throws Exception {
+        runTest("js/js.translator/testData/incremental/invalidation/localObjectsLeakThroughInterface/");
+    }
+
    @Test
    @TestMetadata("mainModuleInvalidation")
    public void testMainModuleInvalidation() throws Exception {
@@ -373,6 +379,12 @@ public class JsIrInvalidationTestGenerated extends AbstractJsIrInvalidationTest
        runTest("js/js.translator/testData/incremental/invalidation/privateInlineFunction1/");
    }

+    @Test
+    @TestMetadata("privateObjectsLeakThroughSealedInterface")
+    public void testPrivateObjectsLeakThroughSealedInterface() throws Exception {
+        runTest("js/js.translator/testData/incremental/invalidation/privateObjectsLeakThroughSealedInterface/");
+    }
+
    @Test
    @TestMetadata("removeFile")
    public void testRemoveFile() throws Exception {
@@ -1,3 +1,3 @@
-interface Module2Interface {
+interface Module1Interface {
    public suspend fun testFunction2() = 2
 }
@@ -1,4 +1,4 @@
-interface Module2Interface {
+interface Module1Interface {
    public suspend fun testFunction2() = 2
    public fun testFunction22() = 22
 }
@@ -1,4 +1,4 @@
-interface Module2Interface {
+interface Module1Interface {
    public fun testFunction2() = 2
    public fun testFunction22() = 22
 }
@@ -1,4 +1,4 @@
-interface Module2Interface {
+interface Module1Interface {
    public fun testFunction2() = 2
    public fun testFunction22() = 22

@@ -1,4 +1,4 @@
-interface Module2Interface {
+interface Module1Interface {
    public fun testFunction2() = 20
    public fun testFunction22() = 220

@@ -1,2 +1,2 @@
-interface Module2Interface {
+interface Module1Interface {
 }
@@ -15,8 +15,8 @@ STEP 3:
    modifications:
        U : l1.3.kt -> l1.kt
    modified ir: l1.kt
-STEP 4:
    updated exports: l1.kt
+STEP 4:
 STEP 5:
    modifications:
        U : l1.5.kt -> l1.kt
@@ -30,5 +30,5 @@ STEP 7:
    modifications:
        U : l1.0.kt -> l1.kt
    modified ir: l1.kt
-STEP 8:
    updated exports: l1.kt
+STEP 8:
@@ -0,0 +1,3 @@
+class Module2Class: Module1Interface {
+    fun testFunction1() = 1
+}
@@ -1,3 +0,0 @@
-class Module1Class: Module2Interface {
-    fun testFunction1() = 1
-}
@@ -1,25 +1,28 @@
 STEP 0:
    dependencies: lib1
-    added file: l2.kt
+    added file: class2.kt, object2.kt
 STEP 1:
    dependencies: lib1
-    updated exports: l2.kt
+    updated exports: class2.kt, object2.kt
+    updated imports: class2.kt, object2.kt
 STEP 2:
    dependencies: lib1
-    updated imports: l2.kt
+    updated imports: class2.kt, object2.kt
 STEP 3:
    dependencies: lib1
+    updated imports: class2.kt, object2.kt
 STEP 4:
    dependencies: lib1
-    updated exports: l2.kt
+    updated exports: class2.kt, object2.kt
 STEP 5:
    dependencies: lib1
 STEP 6:
    dependencies: lib1
-    updated imports: l2.kt
-    updated exports: l2.kt
+    updated imports: class2.kt, object2.kt
+    updated exports: class2.kt, object2.kt
 STEP 7:
    dependencies: lib1
+    updated imports: class2.kt, object2.kt
 STEP 8:
    dependencies: lib1
-    updated exports: l2.kt
+    updated exports: class2.kt, object2.kt
@@ -0,0 +1,3 @@
+object Module2Object: Module1Interface {
+    fun testFunction1() = 1
+}
@@ -1,8 +1,9 @@
 fun box(stepId: Int): String {
-    val obj = Module1Class()
+    val obj = Module2Class()
    when (stepId) {
        0, 1, 2, 3, 4, 5, 6, 7, 8 -> {
-            if (obj.testFunction1() != 1) return "Fail 1"
+            if (obj.testFunction1() != 1) return "Fail 1 class"
+            if (Module2Object.testFunction1() != 1) return "Fail 1 object"
        }
        else -> return "Unknown"
    }
@@ -1,9 +1,11 @@
 fun box(stepId: Int): String {
-    val obj = Module1Class()
+    val obj = Module2Class()
    when (stepId) {
        0, 1, 2, 3, 4, 5, 6, 7, 8 -> {
-            if (obj.testFunction1() != 1) return "Fail 1"
-            if (obj.testFunction22() != 22) return "Fail 22"
+            if (obj.testFunction1() != 1) return "Fail 1 class"
+            if (obj.testFunction22() != 22) return "Fail 22 class"
+            if (Module2Object.testFunction1() != 1) return "Fail 1 object"
+            if (Module2Object.testFunction22() != 22) return "Fail 22 object"
        }
        else -> return "Unknown"
    }
@@ -1,15 +1,23 @@
 fun box(stepId: Int): String {
-    val obj = Module1Class()
+    val obj = Module2Class()
    when (stepId) {
        0, 1, 2, 3, 4, 6, 7, 8 -> {
-            if (obj.testFunction1() != 1) return "Fail 1"
-            if (obj.testFunction22() != 22) return "Fail 22"
-            if (obj.testField222 != 222) return "Fail 222"
+            if (obj.testFunction1() != 1) return "Fail 1 class"
+            if (obj.testFunction22() != 22) return "Fail 22 class"
+            if (obj.testField222 != 222) return "Fail 222 class"
+
+            if (Module2Object.testFunction1() != 1) return "Fail 1 object"
+            if (Module2Object.testFunction22() != 22) return "Fail 22 object"
+            if (Module2Object.testField222 != 222) return "Fail 222 object"
        }
        5 -> {
-            if (obj.testFunction1() != 1) return "Fail 1"
-            if (obj.testFunction22() != 220) return "Fail 220"
-            if (obj.testField222 != 2220) return "Fail 2220"
+            if (obj.testFunction1() != 1) return "Fail 1 class"
+            if (obj.testFunction22() != 220) return "Fail 220 class"
+            if (obj.testField222 != 2220) return "Fail 2220 class"
+
+            if (Module2Object.testFunction1() != 1) return "Fail 1 object"
+            if (Module2Object.testFunction22() != 220) return "Fail 220 object"
+            if (Module2Object.testField222 != 2220) return "Fail 2220 object"
        }
        else -> return "Unknown"
    }
@@ -1,13 +1,18 @@
-suspend fun testCrossReference(): Int {
-    val obj = Module1Class()
+suspend fun testCrossReferenceClass(): Int {
+    val obj = Module2Class()
    return obj.testFunction2()
 }

+suspend fun testCrossReferenceObject(): Int {
+    return Module2Object.testFunction2()
+}
+
 fun box(stepId: Int): String {
-    val obj = Module1Class()
+    val obj = Module2Class()
    when (stepId) {
        0, 1, 2, 3, 4, 5, 6, 7, 8 -> {
-            if (obj.testFunction1() != 1) return "Fail 1"
+            if (obj.testFunction1() != 1) return "Fail 1 class"
+            if (Module2Object.testFunction1() != 1) return "Fail 1 object"
        }
        else -> return "Unknown"
    }
@@ -8,10 +8,10 @@ STEP 2:
    dirty js: lib1, lib2
 STEP 3:
    libs: lib1, lib2, main
-    dirty js: lib1
+    dirty js: lib1, lib2
 STEP 4:
    libs: lib1, lib2, main
-    dirty js: lib1, lib2, main
+    dirty js: lib2, main
 STEP 5:
    libs: lib1, lib2, main
    dirty js: lib1
@@ -20,7 +20,7 @@ STEP 6:
    dirty js: lib1, lib2, main
 STEP 7:
    libs: lib1, lib2, main
-    dirty js: lib1
+    dirty js: lib1, lib2
 STEP 8:
    libs: lib1, lib2, main
-    dirty js: lib1, lib2, main
+    dirty js: lib2, main
@@ -19,8 +19,10 @@ STEP 4:
    modifications:
        U : Interface.4.kt -> Interface.kt
    modified ir: Interface.kt
+    updated exports: Interface.kt
+    updated imports: ClassB.kt, ClassA.kt
 STEP 5:
-    updated exports: Interface.kt, ClassB.kt
+    updated exports: ClassB.kt
 STEP 6:
    modifications:
        U : ClassB.6.kt -> ClassB.kt
@@ -0,0 +1,6 @@
+class ClassA {
+    fun leakObject(): Interface {
+        val obj = object : Interface {}
+        return obj
+    }
+}
@@ -0,0 +1,8 @@
+class ClassA {
+    fun leakObject(): Interface {
+        val obj = object : Interface {
+            override fun getNumber() = 1
+        }
+        return obj
+    }
+}
@@ -0,0 +1,5 @@
+interface Interface {
+    val extraNumber get() = 0
+
+    fun getNumber() = 0
+}
@@ -0,0 +1,7 @@
+interface Interface {
+    val extraNumber get() = 1
+
+    fun getNumber() = 0
+
+    fun getOtherNumber() = 1
+}
@@ -0,0 +1,9 @@
+interface Interface {
+    val extraNumber get() = 2
+
+    fun getNumber() = 0
+
+    fun getOtherNumber() = 1
+
+    val otherExtraNumber get() = 1
+}
@@ -0,0 +1,24 @@
+STEP 0:
+    modifications:
+        U : classA.0.kt -> classA.kt
+        U : interface.0.kt -> interface.kt
+    added file: classA.kt, interface.kt
+STEP 1:
+    modifications:
+        U : classA.1.kt -> classA.kt
+    modified ir: classA.kt
+STEP 2:
+    modifications:
+        U : interface.2.kt -> interface.kt
+    modified ir: interface.kt
+    updated exports: interface.kt
+    updated imports: classA.kt
+STEP 3:
+STEP 4:
+    modifications:
+        U : interface.4.kt -> interface.kt
+    modified ir: interface.kt
+    updated exports: interface.kt
+    updated imports: classA.kt
+STEP 5:
+    updated exports: interface.kt
@@ -0,0 +1,7 @@
+fun box(stepId: Int): String {
+    val x = test()
+    if (x != stepId) {
+        return "Fail $x != $stepId"
+    }
+    return "OK"
+}
@@ -0,0 +1,23 @@
+STEP 0:
+    dependencies: lib1
+    modifications:
+        U : test.0.kt -> test.kt
+    added file: m.kt, test.kt
+STEP 1:
+    dependencies: lib1
+STEP 2:
+    dependencies: lib1
+    updated imports: test.kt
+STEP 3:
+    dependencies: lib1
+    modifications:
+        U : test.3.kt -> test.kt
+    modified ir: test.kt
+STEP 4:
+    dependencies: lib1
+    updated imports: test.kt
+STEP 5:
+    dependencies: lib1
+    modifications:
+        U : test.5.kt -> test.kt
+    modified ir: test.kt
@@ -0,0 +1,5 @@
+fun test(): Int {
+    val a = ClassA()
+    val obj = a.leakObject()
+    return obj.getNumber() + obj.extraNumber
+}
@@ -0,0 +1,5 @@
+fun test(): Int {
+    val a = ClassA()
+    val obj = a.leakObject()
+    return obj.getNumber() + obj.extraNumber + obj.getOtherNumber()
+}
@@ -0,0 +1,5 @@
+fun test(): Int {
+    val a = ClassA()
+    val obj = a.leakObject()
+    return obj.getNumber() + obj.extraNumber + obj.getOtherNumber() + obj.otherExtraNumber
+}
@@ -0,0 +1,17 @@
+MODULES: lib1, main
+
+STEP 0:
+    libs: lib1, main
+    dirty js: lib1, main
+STEP 1:
+    libs: lib1, main
+    dirty js: lib1
+STEP 2:
+    libs: lib1, main
+    dirty js: lib1, main
+STEP 3:
+    libs: lib1, main
+    dirty js: main
+STEP 4..5:
+    libs: lib1, main
+    dirty js: lib1, main
@@ -0,0 +1,5 @@
+class ClassA {
+    val leakedObject: SealedInterface get() = PrivateObject
+}
+
+private object PrivateObject : SealedInterface
@@ -0,0 +1,7 @@
+class ClassA {
+    val leakedObject: SealedInterface get() = PrivateObject
+}
+
+private object PrivateObject : SealedInterface {
+    override fun getNumber() = 1
+}
@@ -0,0 +1,5 @@
+sealed interface SealedInterface {
+    val extraNumber get() = 0
+
+    fun getNumber() = 0
+}
@@ -0,0 +1,7 @@
+sealed interface SealedInterface {
+    val extraNumber get() = 1
+
+    fun getNumber() = 0
+
+    fun getOtherNumber() = 1
+}
@@ -0,0 +1,9 @@
+sealed interface SealedInterface {
+    val extraNumber get() = 2
+
+    fun getNumber() = 0
+
+    fun getOtherNumber() = 1
+
+    val otherExtraNumber get() = 1
+}
@@ -0,0 +1,24 @@
+STEP 0:
+    modifications:
+        U : classA.0.kt -> classA.kt
+        U : interface.0.kt -> interface.kt
+    added file: classA.kt, interface.kt
+STEP 1:
+    modifications:
+        U : classA.1.kt -> classA.kt
+    modified ir: classA.kt
+STEP 2:
+    modifications:
+        U : interface.2.kt -> interface.kt
+    modified ir: interface.kt
+    updated exports: interface.kt
+    updated imports: classA.kt
+STEP 3:
+STEP 4:
+    modifications:
+        U : interface.4.kt -> interface.kt
+    modified ir: interface.kt
+    updated exports: interface.kt
+    updated imports: classA.kt
+STEP 5:
+    updated exports: interface.kt
@@ -0,0 +1,7 @@
+fun box(stepId: Int): String {
+    val x = test()
+    if (x != stepId) {
+        return "Fail $x != $stepId"
+    }
+    return "OK"
+}
@@ -0,0 +1,23 @@
+STEP 0:
+    dependencies: lib1
+    modifications:
+        U : test.0.kt -> test.kt
+    added file: m.kt, test.kt
+STEP 1:
+    dependencies: lib1
+STEP 2:
+    dependencies: lib1
+    updated imports: test.kt
+STEP 3:
+    dependencies: lib1
+    modifications:
+        U : test.3.kt -> test.kt
+    modified ir: test.kt
+STEP 4:
+    dependencies: lib1
+    updated imports: test.kt
+STEP 5:
+    dependencies: lib1
+    modifications:
+        U : test.5.kt -> test.kt
+    modified ir: test.kt
@@ -0,0 +1,5 @@
+fun test(): Int {
+    val a = ClassA()
+    val obj = a.leakedObject
+    return obj.getNumber() + obj.extraNumber
+}
@@ -0,0 +1,5 @@
+fun test(): Int {
+    val a = ClassA()
+    val obj = a.leakedObject
+    return obj.getNumber() + obj.extraNumber + obj.getOtherNumber()
+}
@@ -0,0 +1,5 @@
+fun test(): Int {
+    val a = ClassA()
+    val obj = a.leakedObject
+    return obj.getNumber() + obj.extraNumber + obj.getOtherNumber() + obj.otherExtraNumber
+}
@@ -0,0 +1,17 @@
+MODULES: lib1, main
+
+STEP 0:
+    libs: lib1, main
+    dirty js: lib1, main
+STEP 1:
+    libs: lib1, main
+    dirty js: lib1
+STEP 2:
+    libs: lib1, main
+    dirty js: lib1, main
+STEP 3:
+    libs: lib1, main
+    dirty js: main
+STEP 4..5:
+    libs: lib1, main
+    dirty js: lib1, main