From 47bbd5e8c1b09638575218d96e37a680f48894c9 Mon Sep 17 00:00:00 2001 From: Alexander Korepanov Date: Tue, 13 Sep 2022 13:26:47 +0200 Subject: [PATCH] [JS IR] Add work around in EnumEntriesList for JS IR to avoid name clashes JS IR generates bridges with type checks for special class methods, however if parent and child type parameters are same, the JS signature for the generated brige will be clashed with the JS signature of original method. This patch changes type parameter name of EnumEntriesList to avoid the clash. ^KT-54011 Fixed --- .../org/jetbrains/kotlin/codegen/TestModel.kt | 8 ++++-- .../incremental/AbstractInvalidationTest.kt | 27 +++++++++++++++---- .../InvalidationTestGenerated.java | 5 ++++ .../invalidation/enum/lib1/l1.0.kt | 1 + .../invalidation/enum/lib1/l1.1.kt | 3 +++ .../invalidation/enum/lib1/module.info | 12 +++++++++ .../incremental/invalidation/enum/main/m.kt | 8 ++++++ .../invalidation/enum/main/module.info | 15 +++++++++++ .../enum/main/testEnumEntries.0.kt | 4 +++ .../enum/main/testEnumEntries.3.kt | 18 +++++++++++++ .../invalidation/enum/main/testEnumValues.kt | 9 +++++++ .../invalidation/enum/project.info | 16 +++++++++++ .../stdlib/src/kotlin/enums/EnumEntries.kt | 18 ++++++++----- 13 files changed, 130 insertions(+), 14 deletions(-) create mode 100644 js/js.translator/testData/incremental/invalidation/enum/lib1/l1.0.kt create mode 100644 js/js.translator/testData/incremental/invalidation/enum/lib1/l1.1.kt create mode 100644 js/js.translator/testData/incremental/invalidation/enum/lib1/module.info create mode 100644 js/js.translator/testData/incremental/invalidation/enum/main/m.kt create mode 100644 js/js.translator/testData/incremental/invalidation/enum/main/module.info create mode 100644 js/js.translator/testData/incremental/invalidation/enum/main/testEnumEntries.0.kt create mode 100644 js/js.translator/testData/incremental/invalidation/enum/main/testEnumEntries.3.kt create mode 100644 js/js.translator/testData/incremental/invalidation/enum/main/testEnumValues.kt create mode 100644 js/js.translator/testData/incremental/invalidation/enum/project.info diff --git a/compiler/tests-compiler-utils/tests/org/jetbrains/kotlin/codegen/TestModel.kt b/compiler/tests-compiler-utils/tests/org/jetbrains/kotlin/codegen/TestModel.kt index e1588692944..8def6f0bd8f 100644 --- a/compiler/tests-compiler-utils/tests/org/jetbrains/kotlin/codegen/TestModel.kt +++ b/compiler/tests-compiler-utils/tests/org/jetbrains/kotlin/codegen/TestModel.kt @@ -11,7 +11,7 @@ import java.util.regex.Pattern class ProjectInfo(val name: String, val modules: List, val steps: List, val muted: Boolean) { - class ProjectBuildStep(val id: Int, val order: List, val dirtyJS: List) + class ProjectBuildStep(val id: Int, val order: List, val dirtyJS: List, val language: List) } class ModuleInfo(val moduleName: String) { @@ -98,6 +98,7 @@ class ProjectInfoParser(infoFile: File) : InfoParser(infoFile) { private fun parseSteps(firstId: Int, lastId: Int): List { val order = mutableListOf() val dirtyJS = mutableListOf() + val language = mutableListOf() loop { line -> val splitIndex = line.indexOf(':') @@ -120,13 +121,16 @@ class ProjectInfoParser(infoFile: File) : InfoParser(infoFile) { "dirty js" -> { dirtyJS += splitted[1].splitAndTrim() } + "language" -> { + language += splitted[1].splitAndTrim() + } else -> println(diagnosticMessage("Unknown op $op", line)) } false } - return (firstId..lastId).map { ProjectInfo.ProjectBuildStep(it, order, dirtyJS) } + return (firstId..lastId).map { ProjectInfo.ProjectBuildStep(it, order, dirtyJS, language) } } override fun parse(entryName: String): ProjectInfo { diff --git a/js/js.tests/test/org/jetbrains/kotlin/incremental/AbstractInvalidationTest.kt b/js/js.tests/test/org/jetbrains/kotlin/incremental/AbstractInvalidationTest.kt index ecbc40271b2..76880fa30e7 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/incremental/AbstractInvalidationTest.kt +++ b/js/js.tests/test/org/jetbrains/kotlin/incremental/AbstractInvalidationTest.kt @@ -16,6 +16,8 @@ import org.jetbrains.kotlin.cli.jvm.compiler.KotlinCoreEnvironment import org.jetbrains.kotlin.codegen.* import org.jetbrains.kotlin.config.CommonConfigurationKeys import org.jetbrains.kotlin.config.CompilerConfiguration +import org.jetbrains.kotlin.config.LanguageFeature +import org.jetbrains.kotlin.config.languageVersionSettings import org.jetbrains.kotlin.ir.backend.js.* import org.jetbrains.kotlin.ir.backend.js.ic.* import org.jetbrains.kotlin.ir.backend.js.lower.serialization.ir.JsIrLinker @@ -31,6 +33,7 @@ import org.jetbrains.kotlin.js.testOld.V8IrJsTestChecker import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.psi.KtFile import org.jetbrains.kotlin.serialization.js.ModuleKind +import org.jetbrains.kotlin.test.builders.LanguageVersionSettingsBuilder import org.jetbrains.kotlin.test.KotlinTestWithEnvironment import org.jetbrains.kotlin.test.util.JUnit4Assertions import org.junit.ComparisonFailure @@ -93,11 +96,24 @@ abstract class AbstractInvalidationTest : KotlinTestWithEnvironment() { return File(File(buildDir, moduleName), "cache") } - private fun createConfiguration(moduleName: String): CompilerConfiguration { + private fun createConfiguration(moduleName: String, language: List): CompilerConfiguration { val copy = environment.configuration.copy() copy.put(CommonConfigurationKeys.MODULE_NAME, moduleName) copy.put(JSConfigurationKeys.MODULE_KIND, ModuleKind.PLAIN) copy.put(JSConfigurationKeys.PROPERTY_LAZY_INITIALIZATION, true) + + copy.languageVersionSettings = with(LanguageVersionSettingsBuilder()) { + language.forEach { + val switchLanguageFeature = when { + it.startsWith("+") -> this::enable + it.startsWith("-") -> this::disable + else -> error("Language feature should start with + or -") + } + val feature = LanguageFeature.fromString(it.substring(1)) ?: error("Unknown language feature $it") + switchLanguageFeature(feature) + } + build() + } return copy } @@ -115,7 +131,8 @@ abstract class AbstractInvalidationTest : KotlinTestWithEnvironment() { val expectedFileStats: Map> ) - private fun setupTestStep(projStepId: Int, module: String): TestStepInfo { + private fun setupTestStep(projStep: ProjectInfo.ProjectBuildStep, module: String): TestStepInfo { + val projStepId = projStep.id val moduleTestDir = File(testDir, module) val moduleSourceDir = File(sourceDir, module) val moduleInfo = moduleInfos[module] ?: error("No module info found for $module") @@ -127,7 +144,7 @@ abstract class AbstractInvalidationTest : KotlinTestWithEnvironment() { val dependencies = moduleStep.dependencies.mapTo(mutableListOf(File(STDLIB_KLIB))) { resolveModuleArtifact(it, buildDir) } val outputKlibFile = resolveModuleArtifact(module, buildDir) - val configuration = createConfiguration(module) + val configuration = createConfiguration(module, projStep.language) buildArtifact(configuration, module, moduleSourceDir, dependencies, outputKlibFile) val expectedFileStats = if (deletedFiles.isEmpty()) { @@ -232,9 +249,9 @@ abstract class AbstractInvalidationTest : KotlinTestWithEnvironment() { fun execute() { val stdlibCacheDir = resolveModuleCache(STDLIB_ALIAS, buildDir).canonicalPath for (projStep in projectInfo.steps) { - val testInfo = projStep.order.map { setupTestStep(projStep.id, it) } + val testInfo = projStep.order.map { setupTestStep(projStep, it) } - val configuration = createConfiguration(projStep.order.last()) + val configuration = createConfiguration(projStep.order.last(), projStep.language) val cacheUpdater = CacheUpdater( mainModule = testInfo.last().modulePath, allModules = testInfo.mapTo(mutableListOf(STDLIB_KLIB)) { it.modulePath }, diff --git a/js/js.tests/tests-gen/org/jetbrains/kotlin/incremental/InvalidationTestGenerated.java b/js/js.tests/tests-gen/org/jetbrains/kotlin/incremental/InvalidationTestGenerated.java index ffd542d9741..c5738c97704 100644 --- a/js/js.tests/tests-gen/org/jetbrains/kotlin/incremental/InvalidationTestGenerated.java +++ b/js/js.tests/tests-gen/org/jetbrains/kotlin/incremental/InvalidationTestGenerated.java @@ -75,6 +75,11 @@ public class InvalidationTestGenerated extends AbstractInvalidationTest { runTest("js/js.translator/testData/incremental/invalidation/eagerInitialization/"); } + @TestMetadata("enum") + public void testEnum() throws Exception { + runTest("js/js.translator/testData/incremental/invalidation/enum/"); + } + @TestMetadata("exportsThroughInlineFunction") public void testExportsThroughInlineFunction() throws Exception { runTest("js/js.translator/testData/incremental/invalidation/exportsThroughInlineFunction/"); diff --git a/js/js.translator/testData/incremental/invalidation/enum/lib1/l1.0.kt b/js/js.translator/testData/incremental/invalidation/enum/lib1/l1.0.kt new file mode 100644 index 00000000000..5627fe2df80 --- /dev/null +++ b/js/js.translator/testData/incremental/invalidation/enum/lib1/l1.0.kt @@ -0,0 +1 @@ +enum class TestEnum diff --git a/js/js.translator/testData/incremental/invalidation/enum/lib1/l1.1.kt b/js/js.translator/testData/incremental/invalidation/enum/lib1/l1.1.kt new file mode 100644 index 00000000000..2a6edf7ad92 --- /dev/null +++ b/js/js.translator/testData/incremental/invalidation/enum/lib1/l1.1.kt @@ -0,0 +1,3 @@ +enum class TestEnum { + A, B +} diff --git a/js/js.translator/testData/incremental/invalidation/enum/lib1/module.info b/js/js.translator/testData/incremental/invalidation/enum/lib1/module.info new file mode 100644 index 00000000000..576ac253fac --- /dev/null +++ b/js/js.translator/testData/incremental/invalidation/enum/lib1/module.info @@ -0,0 +1,12 @@ +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 +STEP 2: + modified config: l1.kt +STEP 3: + updated exports: l1.kt diff --git a/js/js.translator/testData/incremental/invalidation/enum/main/m.kt b/js/js.translator/testData/incremental/invalidation/enum/main/m.kt new file mode 100644 index 00000000000..6309be06f12 --- /dev/null +++ b/js/js.translator/testData/incremental/invalidation/enum/main/m.kt @@ -0,0 +1,8 @@ +@kotlin.ExperimentalStdlibApi +fun box(stepId: Int): String { + when { + !testEnumValues(stepId) -> return "Fail testEnumValues" + !testEnumEntries(stepId) -> return "Fail testEnumEntries" + } + return "OK" +} diff --git a/js/js.translator/testData/incremental/invalidation/enum/main/module.info b/js/js.translator/testData/incremental/invalidation/enum/main/module.info new file mode 100644 index 00000000000..5ddc8d4fbf2 --- /dev/null +++ b/js/js.translator/testData/incremental/invalidation/enum/main/module.info @@ -0,0 +1,15 @@ +STEP 0: + modifications: + U : testEnumEntries.0.kt -> testEnumEntries.kt + dependencies: lib1 + added file: m.kt, testEnumEntries.kt, testEnumValues.kt +STEP 1: + dependencies: lib1 +STEP 2: + dependencies: lib1 + modified config: m.kt, testEnumEntries.kt, testEnumValues.kt +STEP 3: + modifications: + U : testEnumEntries.3.kt -> testEnumEntries.kt + dependencies: lib1 + modified ir: testEnumEntries.kt diff --git a/js/js.translator/testData/incremental/invalidation/enum/main/testEnumEntries.0.kt b/js/js.translator/testData/incremental/invalidation/enum/main/testEnumEntries.0.kt new file mode 100644 index 00000000000..d2c34c1c2cf --- /dev/null +++ b/js/js.translator/testData/incremental/invalidation/enum/main/testEnumEntries.0.kt @@ -0,0 +1,4 @@ +@kotlin.ExperimentalStdlibApi +fun testEnumEntries(stepId: Int): Boolean { + return true +} diff --git a/js/js.translator/testData/incremental/invalidation/enum/main/testEnumEntries.3.kt b/js/js.translator/testData/incremental/invalidation/enum/main/testEnumEntries.3.kt new file mode 100644 index 00000000000..abb38b71104 --- /dev/null +++ b/js/js.translator/testData/incremental/invalidation/enum/main/testEnumEntries.3.kt @@ -0,0 +1,18 @@ +@kotlin.ExperimentalStdlibApi +fun testEnumEntries(stepId: Int): Boolean { + val entries = TestEnum.entries + + if (entries.contains((object {}) as Any?)) return false + + when (stepId) { + 0 -> if (entries.isNotEmpty()) return false + 1, 2, 3 -> { + if (entries.size != 2) return false + if (entries.indexOf(TestEnum.A) != 0) return false + if (entries.indexOf(TestEnum.B) != 1) return false + } + else -> return false + } + + return true +} diff --git a/js/js.translator/testData/incremental/invalidation/enum/main/testEnumValues.kt b/js/js.translator/testData/incremental/invalidation/enum/main/testEnumValues.kt new file mode 100644 index 00000000000..8eb1dd6e7da --- /dev/null +++ b/js/js.translator/testData/incremental/invalidation/enum/main/testEnumValues.kt @@ -0,0 +1,9 @@ +fun testEnumValues(stepId: Int): Boolean { + val values = enumValues().map { it.ordinal to it.name } + when (stepId) { + 0 -> if (values.isEmpty()) return true + 1, 2, 3 -> if (values == listOf(0 to "A", 1 to "B")) return true + else -> return false + } + return false +} diff --git a/js/js.translator/testData/incremental/invalidation/enum/project.info b/js/js.translator/testData/incremental/invalidation/enum/project.info new file mode 100644 index 00000000000..5aed1a56d19 --- /dev/null +++ b/js/js.translator/testData/incremental/invalidation/enum/project.info @@ -0,0 +1,16 @@ +MODULES: lib1, main + +STEP 0: + libs: lib1, main + dirty js: lib1, main +STEP 1: + libs: lib1, main + dirty js: lib1 +STEP 2: + language: +EnumEntries + libs: lib1, main + dirty js: main, lib1 +STEP 3: + language: +EnumEntries + libs: lib1, main + dirty js: main, lib1 diff --git a/libraries/stdlib/src/kotlin/enums/EnumEntries.kt b/libraries/stdlib/src/kotlin/enums/EnumEntries.kt index 9a627f6d480..836f5d0ba02 100644 --- a/libraries/stdlib/src/kotlin/enums/EnumEntries.kt +++ b/libraries/stdlib/src/kotlin/enums/EnumEntries.kt @@ -71,11 +71,15 @@ internal fun > enumEntries(entries: Array): EnumEntries = Enum */ @SinceKotlin("1.8") @ExperimentalStdlibApi -private class EnumEntriesList>(private val entriesProvider: () -> Array) : EnumEntries, AbstractList(), Serializable { +private class EnumEntriesList>(private val entriesProvider: () -> Array) : EnumEntries, AbstractList(), Serializable { +// WA for JS IR bug: +// class type parameter MUST be different form E (AbstractList type parameter), +// otherwise the bridge names for contains() and indexOf() will be clashed with the original method names, +// and produced JS code will not contain type checks and will not work correctly. @Volatile // Volatile is required for safe publication of the array. It doesn't incur any real-world penalties - private var _entries: Array? = null - private val entries: Array + private var _entries: Array? = null + private val entries: Array get() { var e = _entries if (e != null) return e @@ -87,7 +91,7 @@ private class EnumEntriesList>(private val entriesProvider: () -> Ar override val size: Int get() = entries.size - override fun get(index: Int): E { + override fun get(index: Int): T { val entries = entries checkElementIndex(index, entries.size) return entries[index] @@ -96,7 +100,7 @@ private class EnumEntriesList>(private val entriesProvider: () -> Ar // By definition, EnumEntries contains **all** enums in declaration order, // thus we are able to short-circuit the implementation here - override fun contains(element: E): Boolean { + override fun contains(element: T): Boolean { @Suppress("SENSELESS_COMPARISON") if (element === null) return false // WA for JS IR bug // Check identity due to UnsafeVariance @@ -104,7 +108,7 @@ private class EnumEntriesList>(private val entriesProvider: () -> Ar return target === element } - override fun indexOf(element: E): Int { + override fun indexOf(element: T): Int { @Suppress("SENSELESS_COMPARISON") if (element === null) return -1 // WA for JS IR bug // Check identity due to UnsafeVariance @@ -113,7 +117,7 @@ private class EnumEntriesList>(private val entriesProvider: () -> Ar return if (target === element) ordinal else -1 } - override fun lastIndexOf(element: E): Int = indexOf(element) + override fun lastIndexOf(element: T): Int = indexOf(element) @Suppress("unused") // Used for Java serialization private fun writeReplace(): Any {