[JS IR] Ignore unbound symbols in IC infrastructure

The IR linker is responsible for detecting unbound symbols, if it skips them for some reason, IC infrastructure must not fail with unbound symbols exceptions. Anyway, the IR validator verifies later if there are unbound symbols in reachable IR and generates the corresponding error. ^KT-56602 Fixed
2023-02-20 11:54:47 +01:00
parent ff22f456a0
commit 3c9d653595
16 changed files with 101 additions and 13 deletions
@@ -115,7 +115,15 @@ internal sealed class FileSignatureProvider(val irFile: IrFile) {
        }

        override fun getImplementedSymbols(): Map<IdSignature, IrSymbol> {
-            return collectImplementedSymbol(fileDeserializer.symbolDeserializer.deserializedSymbols)
+            // Sometimes linker may leave unbound symbols in IrSymbolDeserializer::deserializedSymbols map.
+            // Generally, all unbound symbols must be caught in KotlinIrLinker::checkNoUnboundSymbols,
+            // unfortunately it does not work properly in the current implementation.
+            // Also, reachable unbound symbols are caught by IrValidator, it works fine, but it works after this place.
+            // Filter unbound symbols here, because an error from IC infrastructure about the unbound symbols looks pretty wired
+            // and if the unbound symbol is really reachable from IR the error will be fired from IrValidator later.
+            // Otherwise, the unbound symbol is unreachable, and it cannot appear in IC dependency graph, so we can ignore them.
+            val deserializedSymbols = fileDeserializer.symbolDeserializer.deserializedSymbols.filter { it.value.isBound }
+            return collectImplementedSymbol(deserializedSymbols)
        }
    }

@@ -48,7 +48,8 @@ class ModuleInfo(val moduleName: String) {
        val dependencies: Collection<Dependency>,
        val modifications: List<Modification>,
        val expectedFileStats: Map<String, Set<String>>,
-        val expectedDTS: Set<String>
+        val expectedDTS: Set<String>,
+        val rebuildKlib: Boolean
    )

    val steps = mutableListOf<ModuleStep>()
@@ -67,6 +68,7 @@ private const val MODIFICATIONS = "modifications"
 private const val MODIFICATION_UPDATE = "U"
 private const val MODIFICATION_DELETE = "D"
 private const val EXPECTED_DTS_LIST = "expected dts"
+private const val REBUILD_KLIB = "rebuild klib"

 private val STEP_PATTERN = Pattern.compile("^\\s*STEP\\s+(\\d+)\\.*(\\d+)?\\s*:?$")

@@ -213,6 +215,7 @@ class ModuleInfoParser(infoFile: File) : InfoParser<ModuleInfo>(infoFile) {
        val friendDependencies = mutableSetOf<String>()
        val modifications = mutableListOf<ModuleInfo.Modification>()
        val expectedDTS = mutableSetOf<String>()
+        var rebuildKlib = true

        loop { line ->
            if (line.matches(STEP_PATTERN.toRegex()))
@@ -234,6 +237,9 @@ class ModuleInfoParser(infoFile: File) : InfoParser<ModuleInfo>(infoFile) {
                    FRIENDS -> getOpArgs().forEach { friendDependencies += it }
                    MODIFICATIONS -> modifications += parseModifications()
                    EXPECTED_DTS_LIST -> getOpArgs().forEach { expectedDTS += it }
+                    REBUILD_KLIB -> getOpArgs().singleOrNull()?.toBooleanStrictOrNull()?.let {
+                        rebuildKlib = it
+                    } ?: error(diagnosticMessage("$op expects true or false", line))
                    else -> error(diagnosticMessage("Unknown op $op", line))
                }
            }
@@ -255,7 +261,8 @@ class ModuleInfoParser(infoFile: File) : InfoParser<ModuleInfo>(infoFile) {
                dependencies = dependencies,
                modifications = modifications,
                expectedFileStats = expectedFileStats,
-                expectedDTS = expectedDTS
+                expectedDTS = expectedDTS,
+                rebuildKlib = rebuildKlib
            )
        }
    }
@@ -152,7 +152,7 @@ abstract class AbstractInvalidationTest(
            val expectedDTS: String?
        )

-        private fun setupTestStep(projStep: ProjectInfo.ProjectBuildStep, module: String, buildKlib: Boolean): TestStepInfo {
+        private fun setupTestStep(projStep: ProjectInfo.ProjectBuildStep, module: String): TestStepInfo {
            val projStepId = projStep.id
            val moduleTestDir = File(testDir, module)
            val moduleSourceDir = File(sourceDir, module)
@@ -172,7 +172,7 @@ abstract class AbstractInvalidationTest(
            val outputKlibFile = resolveModuleArtifact(module, buildDir)

            val friends = mutableListOf<File>()
-            if (buildKlib) {
+            if (moduleStep.rebuildKlib) {
                val dependencies = mutableListOf(File(STDLIB_KLIB))
                for (dep in moduleStep.dependencies) {
                    val klibFile = resolveModuleArtifact(dep.moduleName, buildDir)
@@ -313,7 +313,7 @@ abstract class AbstractInvalidationTest(

        fun execute() {
            for (projStep in projectInfo.steps) {
-                val testInfo = projStep.order.map { setupTestStep(projStep, it, true) }
+                val testInfo = projStep.order.map { setupTestStep(projStep, it) }

                val mainModuleInfo = testInfo.last()
                testInfo.find { it != mainModuleInfo && it.friends.isNotEmpty() }?.let {
@@ -341,7 +341,7 @@ abstract class AbstractInvalidationTest(
                    }
                )

-                val removedModulesInfo = (projectInfo.modules - projStep.order.toSet()).map { setupTestStep(projStep, it, false) }
+                val removedModulesInfo = (projectInfo.modules - projStep.order.toSet()).map { setupTestStep(projStep, it) }

                val icCaches = cacheUpdater.actualizeCaches()
                verifyCacheUpdateStats(projStep.id, cacheUpdater.getDirtyFileLastStats(), testInfo + removedModulesInfo)
@@ -40,6 +40,11 @@ public class JsIrES6InvalidationTestGenerated extends AbstractJsIrES6Invalidatio
        KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("js/js.translator/testData/incremental/invalidation"), Pattern.compile("^([^_](.+))$"), null, TargetBackend.JS_IR_ES6, false);
    }

+    @TestMetadata("breakKlibBinaryCompatibilityWithVariance")
+    public void testBreakKlibBinaryCompatibilityWithVariance() throws Exception {
+        runTest("js/js.translator/testData/incremental/invalidation/breakKlibBinaryCompatibilityWithVariance/");
+    }
+
    @TestMetadata("circleExportsUpdate")
    public void testCircleExportsUpdate() throws Exception {
        runTest("js/js.translator/testData/incremental/invalidation/circleExportsUpdate/");
@@ -40,6 +40,11 @@ public class JsIrInvalidationTestGenerated extends AbstractJsIrInvalidationTest
        KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("js/js.translator/testData/incremental/invalidation"), Pattern.compile("^([^_](.+))$"), null, TargetBackend.JS_IR, false);
    }

+    @TestMetadata("breakKlibBinaryCompatibilityWithVariance")
+    public void testBreakKlibBinaryCompatibilityWithVariance() throws Exception {
+        runTest("js/js.translator/testData/incremental/invalidation/breakKlibBinaryCompatibilityWithVariance/");
+    }
+
    @TestMetadata("circleExportsUpdate")
    public void testCircleExportsUpdate() throws Exception {
        runTest("js/js.translator/testData/incremental/invalidation/circleExportsUpdate/");
@@ -1,4 +1,5 @@
 STEP 0:
+    rebuild klib: false
 STEP 1:
    dependencies: lib1
    modifications:
@@ -10,6 +11,8 @@ STEP 2:
        U : l2.2.kt -> l2.kt
    modified ir: l2.kt
 STEP 3:
+    rebuild klib: false
    modifications:
        D : l2.kt
 STEP 4:
+    rebuild klib: false
@@ -0,0 +1,5 @@
+interface GenericInterface<T>
+
+interface TestInterface {
+    fun <T> test(f: () -> GenericInterface<out T>) {}
+}
@@ -0,0 +1,5 @@
+interface GenericInterface<out T>
+
+interface TestInterface {
+    fun <T> test(f: () -> GenericInterface<T>) {}
+}
@@ -0,0 +1,9 @@
+STEP 0:
+    modifications:
+        U : l1.0.kt -> l1.kt
+    added file: l1.kt
+STEP 1:
+    modifications:
+        U : l1.1.kt -> l1.kt
+    modified ir: l1.kt
+    updated exports: l1.kt
@@ -0,0 +1,5 @@
+class TestClass : TestInterface {
+    override fun <T> test(f: () -> GenericInterface<out T>) {
+        f()
+    }
+}
@@ -0,0 +1,8 @@
+STEP 0:
+    dependencies: lib1
+    modifications:
+        U : l2.0.kt -> l2.kt
+    added file: l2.kt
+STEP 1:
+    rebuild klib: false
+    updated imports: l2.kt
@@ -0,0 +1,16 @@
+fun box(stepId: Int): String {
+    var x = -1
+    TestClass().test {
+        object : GenericInterface<Int> {
+            init {
+                x = stepId
+            }
+        }
+    }
+
+    when (stepId) {
+        in 0..1 -> if (x != stepId) return "Fail, got $x"
+        else -> return "Unknown"
+    }
+    return "OK"
+}
@@ -0,0 +1,6 @@
+STEP 0:
+    dependencies: lib1, lib2
+    added file: m.kt
+STEP 1:
+    dependencies: lib1, lib2
+    updated imports: m.kt
@@ -0,0 +1,8 @@
+MODULES: lib1, lib2, main
+
+STEP 0:
+    libs: lib1, lib2, main
+    dirty js: lib1, lib2, main
+STEP 1:
+    libs: lib1, lib2, main
+    dirty js: lib1, lib2, main
@@ -1,4 +1,5 @@
 STEP 0..7:
+    rebuild klib: false
 STEP 8:
    dependencies: lib1
    modifications:
@@ -43,6 +43,7 @@ class Kotlin2JsIrBeIncrementalCompilationIT : KGPBaseTest() {
            }

            val srcFile = projectPath.resolve("app/src/main/kotlin/App.kt").toFile()
+            val guardFile = projectPath.resolve("app/build/klib/cache/cache.guard").toFile()
            val badCode = srcFile.readText()

            var successfulBuildCacheFiles = emptyMap<String, Int>()
@@ -56,15 +57,10 @@ class Kotlin2JsIrBeIncrementalCompilationIT : KGPBaseTest() {

            srcFile.writeText(badCode)

-            val guardFile = projectPath.resolve("app/build/klib/cache/cache.guard").toFile()
            for (i in 0..1) {
                buildAndFail("nodeDevelopmentRun") {
                    assertTasksFailed(":app:compileDevelopmentExecutableKotlinJs")
-                    val failedBuildCacheFiles = readCacheFiles()
-                    val expectedFiles = successfulBuildCacheFiles.toMutableMap()
-                    expectedFiles[guardFile.absolutePath] = ByteArray(0).contentHashCode()
-                    assertEquals(expectedFiles, failedBuildCacheFiles, "The cache files should not be modified")
-                    guardFile.delete()
+                    assertTrue("guard file after compilation error expected") { guardFile.exists() }
                }
            }

@@ -75,6 +71,7 @@ class Kotlin2JsIrBeIncrementalCompilationIT : KGPBaseTest() {
                val successfulRebuildCacheFiles = readCacheFiles()
                assertEquals(successfulBuildCacheFiles.size, successfulRebuildCacheFiles.size, "The number of files must be the same")
                assertNotEquals(successfulBuildCacheFiles, successfulRebuildCacheFiles, "The cache files should be modified")
+                assertFalse("guard file after successful compilation must be removed") { guardFile.exists() }
            }
        }
    }