From 33d13474c5b7ab6bcb9aab79bf6415480b0193b8 Mon Sep 17 00:00:00 2001 From: Vsevolod Tolstopyatov Date: Mon, 20 Mar 2023 10:18:28 +0000 Subject: [PATCH] Make EnumEntries implementation eager * Addresses the recent design changes of the corresponding language feature * Also rework JS/Wasm enumEntries implementation with the function that accepts array instead of fabric function ^KT-57318 Co-authored-by: Artem Kobzar Merge-request: KT-MR-9204 Merged-by: Vsevolod Tolstopyatov --- .../ir/backend/js/JsIrBackendContext.kt | 2 +- .../kotlin/ir/backend/js/JsLoweringPhases.kt | 2 +- .../ir/backend/js/lower/EnumClassLowering.kt | 42 +++--------- .../kotlin/backend/wasm/WasmLoweringPhases.kt | 2 +- .../kotlin/backend/wasm/WasmSymbols.kt | 2 +- .../src/minimalEnumEntries.kt | 4 +- .../stdlib/src/kotlin/enums/EnumEntries.kt | 67 +++---------------- 7 files changed, 25 insertions(+), 96 deletions(-) diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsIrBackendContext.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsIrBackendContext.kt index b03a6062df0..a57792e9688 100644 --- a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsIrBackendContext.kt +++ b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsIrBackendContext.kt @@ -189,7 +189,7 @@ class JsIrBackendContext( override val enumEntries = getIrClass(ENUMS_PACKAGE_FQNAME.child(Name.identifier("EnumEntries"))) override val createEnumEntries = getFunctions(ENUMS_PACKAGE_FQNAME.child(Name.identifier("enumEntries"))) - .find { it.valueParameters.firstOrNull()?.type?.isFunctionType == true } + .find { it.valueParameters.firstOrNull()?.type?.isFunctionType == false } .let { symbolTable.referenceSimpleFunction(it!!) } override val ir = object : Ir(this) { diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsLoweringPhases.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsLoweringPhases.kt index 7829c5a9a78..8c3c53fd84a 100644 --- a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsLoweringPhases.kt +++ b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/JsLoweringPhases.kt @@ -365,7 +365,7 @@ private val enumEntryCreateGetInstancesFunsLoweringPhase = makeDeclarationTransf ) private val enumSyntheticFunsLoweringPhase = makeDeclarationTransformerPhase( - { EnumSyntheticFunctionsAndPropertiesLowering(it, supportRawFunctionReference = true) }, + ::EnumSyntheticFunctionsAndPropertiesLowering, name = "EnumSyntheticFunctionsAndPropertiesLowering", description = "Implement `valueOf, `values` and `entries`", prerequisite = setOf( diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/EnumClassLowering.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/EnumClassLowering.kt index 0bbefd4d393..3a299c34165 100644 --- a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/EnumClassLowering.kt +++ b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/EnumClassLowering.kt @@ -462,9 +462,7 @@ class EnumEntryCreateGetInstancesFunsLowering(val context: JsCommonBackendContex private const val ENTRIES_FIELD_NAME = "\$ENTRIES" class EnumSyntheticFunctionsAndPropertiesLowering( - val context: JsCommonBackendContext, - private val supportRawFunctionReference: Boolean = false, - private val syntheticFieldsShouldBeReinitialized: Boolean = false, + val context: JsCommonBackendContext ) : DeclarationTransformer { private val IrEnumEntry.getInstanceFun by context.mapping.enumEntryToGetInstanceFun private val IrClass.initEntryInstancesFun: IrSimpleFunction? by context.mapping.enumClassToInitEntryInstancesFun @@ -506,29 +504,27 @@ class EnumSyntheticFunctionsAndPropertiesLowering( private val throwISESymbol = context.ir.symbols.throwISE private fun createEnumEntriesBody(entriesGetter: IrFunction, enumClass: IrClass): IrBlockBody { - val entriesField = enumClass.addEnumEntriesField() + val entriesField = enumClass.buildEntriesField() + val valuesFunction = enumClass.searchForValuesFunction() + val createEnumEntriesFunction = context.createEnumEntries return context.createIrBuilder(entriesGetter.symbol).run { irBlockBody { - if (syntheticFieldsShouldBeReinitialized) { - +irIfThen( - irEqualsNull(irGetField(null, entriesField)), - irSetField(null, entriesField, entriesField.initializer!!.expression) - ) - } + +irIfThen( + irEqualsNull(irGetField(null, entriesField)), + irSetField(null, entriesField, irCall(createEnumEntriesFunction).apply { + putValueArgument(0, irCall(valuesFunction)) + }) + ) +irReturn(irGetField(null, entriesField)) } } } - private fun IrClass.addEnumEntriesField(): IrField { - return buildEntriesField(searchForValuesFunction()) - } - private fun IrClass.searchForValuesFunction(): IrFunction { return declarations.find { it is IrFunction && it.isStatic && it.returnType.isArray() } as IrFunction } - private fun IrClass.buildEntriesField(entriesHelper: IrFunction): IrField = with(context) { + private fun IrClass.buildEntriesField(): IrField = with(context) { addField { name = Name.identifier(ENTRIES_FIELD_NAME) type = enumEntries.defaultType @@ -536,25 +532,9 @@ class EnumSyntheticFunctionsAndPropertiesLowering( origin = IrDeclarationOrigin.FIELD_FOR_ENUM_ENTRIES isFinal = true isStatic = true - }.apply { - initializer = context.createIrBuilder(symbol).run { - irExprBody(irCall(createEnumEntries).apply { - val referenceType = context.irBuiltIns.functionN(0).typeWith(enumEntries.defaultType) - putValueArgument(0, referenceFor(entriesHelper, referenceType)) - }) - } } } - private fun IrBuilderWithScope.referenceFor(function: IrFunction, type: IrType): IrDeclarationReference { - return if (supportRawFunctionReference) { - irRawFunctionReference(type, function.symbol) - } else { - irFunctionReference(type, function.symbol) - } - } - - private fun createEnumValueOfBody(valueOfFun: IrFunction, irClass: IrClass): IrBlockBody { val nameParameter = valueOfFun.valueParameters[0] diff --git a/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/WasmLoweringPhases.kt b/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/WasmLoweringPhases.kt index c658edfda71..b698cedd68c 100644 --- a/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/WasmLoweringPhases.kt +++ b/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/WasmLoweringPhases.kt @@ -258,7 +258,7 @@ private val enumEntryCreateGetInstancesFunsLoweringPhase = makeWasmModulePhase( ) private val enumSyntheticFunsLoweringPhase = makeWasmModulePhase( - { EnumSyntheticFunctionsAndPropertiesLowering(it, syntheticFieldsShouldBeReinitialized = true) }, + ::EnumSyntheticFunctionsAndPropertiesLowering, name = "EnumSyntheticFunctionsAndPropertiesLowering", description = "Implement `valueOf`, `values` and `entries`", prerequisite = setOf( diff --git a/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/WasmSymbols.kt b/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/WasmSymbols.kt index c2371e9b14d..d2a295f3557 100644 --- a/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/WasmSymbols.kt +++ b/compiler/ir/backend.wasm/src/org/jetbrains/kotlin/backend/wasm/WasmSymbols.kt @@ -108,7 +108,7 @@ class WasmSymbols( val enumEntries = getIrClass(FqName.fromSegments(listOf("kotlin", "enums", "EnumEntries"))) val createEnumEntries = findFunctions(enumsInternalPackage.memberScope, Name.identifier("enumEntries")) - .find { it.valueParameters.firstOrNull()?.type?.isFunctionType == true } + .find { it.valueParameters.firstOrNull()?.type?.isFunctionType == false } .let { symbolTable.referenceSimpleFunction(it!!) } val enumValueOfIntrinsic = getInternalFunction("enumValueOfIntrinsic") diff --git a/libraries/stdlib/js-ir-minimal-for-test/src/minimalEnumEntries.kt b/libraries/stdlib/js-ir-minimal-for-test/src/minimalEnumEntries.kt index caec47d34cf..0abf41705ee 100644 --- a/libraries/stdlib/js-ir-minimal-for-test/src/minimalEnumEntries.kt +++ b/libraries/stdlib/js-ir-minimal-for-test/src/minimalEnumEntries.kt @@ -12,8 +12,8 @@ public sealed interface EnumEntries> @PublishedApi @ExperimentalStdlibApi @SinceKotlin("1.8") -internal fun > enumEntries(entriesProvider: () -> Array): EnumEntries = EnumEntriesList(entriesProvider) +internal fun > enumEntries(entries: Array): EnumEntries = EnumEntriesList(entries) @SinceKotlin("1.8") @ExperimentalStdlibApi -private class EnumEntriesList>(val entriesProvider: () -> Array) : EnumEntries +private class EnumEntriesList>(val entries: Array) : EnumEntries \ No newline at end of file diff --git a/libraries/stdlib/src/kotlin/enums/EnumEntries.kt b/libraries/stdlib/src/kotlin/enums/EnumEntries.kt index 32c23a1971f..3ca52f73635 100644 --- a/libraries/stdlib/src/kotlin/enums/EnumEntries.kt +++ b/libraries/stdlib/src/kotlin/enums/EnumEntries.kt @@ -23,78 +23,26 @@ public sealed interface EnumEntries> : List @PublishedApi @ExperimentalStdlibApi -@SinceKotlin("1.8") // Used by JVM compiler -internal fun > enumEntries(entriesProvider: () -> Array): EnumEntries = EnumEntriesList(entriesProvider) +@SinceKotlin("1.8") // Used by pre-1.9.0 JVM compiler for the feature in preview mode. Can be safely removed around 2.1 +internal fun > enumEntries(entriesProvider: () -> Array): EnumEntries = EnumEntriesList(entriesProvider()) @PublishedApi @ExperimentalStdlibApi -@SinceKotlin("1.8") // Used by Native/JS compilers and Java serialization -internal fun > enumEntries(entries: Array): EnumEntries = EnumEntriesList { entries }.also { - /* - * Here we are enforcing initialization of _entries property. - * It is required because of two reasons. - * 1. In old Native mm the object will be frozen after creation, so it must be immutable - * 2. Native doesn't support @Volatile for now, so this initialization is not generally safe, if - * done after object is published. - * - * This is very implementation-dependent hack, and it should be removed when/if both reasons above are gone. - */ - it.size -} +@SinceKotlin("1.8") +internal fun > enumEntries(entries: Array): EnumEntries = EnumEntriesList(entries) -/* - * For enum class E, this class is instantiated in the following manner (NB it's pseudocode that does not - * reflect code generation strategy precisely): - * ``` - * class E extends Enum { - * private static final E[] $VALUES - * private static final EnumEntries[] $ENTRIES - * - * static { - * $VALUES = $values(); - * val supplier = #invokedynamic ..args.. values; - * $ENTRIES = new EnumEntriesList(supplier); - * } - * - * public static EnumEntries getEntries() { - * return $ENTRIES; - * } - * - * private synthetic static E[] $values() { - * return new E[] { ... }; - * } - * } - * ``` - * - * This machinery is required as a workaround for a long-standing issue when people do reflectively change `$VALUES` of - * enums in order to workaround project-specific issues. - * We allow racy initialization (e.g. entriesProvider can be invoked multiple times), but the resulting array is safely - * published, preventing any read races after the initialization. - */ @SinceKotlin("1.8") @ExperimentalStdlibApi -private class EnumEntriesList>(private val entriesProvider: () -> Array) : EnumEntries, AbstractList(), Serializable { +private class EnumEntriesList>(private val entries: Array) : EnumEntries, AbstractList(), Serializable { // WA for JS IR bug: -// class type parameter MUST be different form E (AbstractList type parameter), +// class type parameter name MUST be different from 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 - get() { - var e = _entries - if (e != null) return e - e = entriesProvider() - _entries = e - return e - } - override val size: Int get() = entries.size override fun get(index: Int): T { - val entries = entries checkElementIndex(index, entries.size) return entries[index] } @@ -121,8 +69,9 @@ private class EnumEntriesList>(private val entriesProvider: () -> Ar override fun lastIndexOf(element: T): Int = indexOf(element) - @Suppress("unused") // Used for Java serialization + @Suppress("unused") private fun writeReplace(): Any { + // Used for Java serialization: EESP ensures that deserialized object **always** reflects the state of the enum on the target classpath return EnumEntriesSerializationProxy(entries) } }