From e13b6b2a90bf0da661bf6105ce37cc15b2ccdb11 Mon Sep 17 00:00:00 2001 From: Sergej Jaskiewicz Date: Tue, 30 Nov 2021 18:14:44 +0300 Subject: [PATCH] [JS IR] [runtime] Remove `valueOf` method from `Long` This method was used for coercing `Long` values to JavaScript `number`. However, it caused issues when `Long` values were concatenated to a string (see KT-8666, KT-26706). --- .../kotlin/ir/backend/js/JsLoweringPhases.kt | 7 -- .../js/lower/JsStringConcatenationLowering.kt | 89 ------------------- .../testData/codegen/box/intrinsics/kt8666.kt | 31 ++++++- .../testData/box/number/kt26706.kt | 56 +++++++++++- libraries/stdlib/js-ir/runtime/long.kt | 6 +- 5 files changed, 85 insertions(+), 104 deletions(-) delete mode 100644 compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/JsStringConcatenationLowering.kt 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 8860319ef43..075e856c8c0 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 @@ -161,12 +161,6 @@ private val expectDeclarationsRemovingPhase = makeDeclarationTransformerPhase( description = "Remove expect declaration from module fragment" ) -private val stringConcatenationLoweringPhase = makeJsModulePhase( - ::JsStringConcatenationLowering, - name = "JsStringConcatenationLowering", - description = "Call toString() for values of some types when concatenating strings" -).toModuleLowering() - private val lateinitNullableFieldsPhase = makeDeclarationTransformerPhase( ::NullableFieldsForLateinitCreationLowering, name = "LateinitNullableFields", @@ -832,7 +826,6 @@ val loweringList = listOf( copyInlineFunctionBodyLoweringPhase, removeInlineDeclarationsWithReifiedTypeParametersLoweringPhase, createScriptFunctionsPhase, - stringConcatenationLoweringPhase, callableReferenceLowering, singleAbstractMethodPhase, tailrecLoweringPhase, diff --git a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/JsStringConcatenationLowering.kt b/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/JsStringConcatenationLowering.kt deleted file mode 100644 index 06eaa7a0867..00000000000 --- a/compiler/ir/backend.js/src/org/jetbrains/kotlin/ir/backend/js/lower/JsStringConcatenationLowering.kt +++ /dev/null @@ -1,89 +0,0 @@ -/* - * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors. - * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. - */ - -package org.jetbrains.kotlin.ir.backend.js.lower - -import org.jetbrains.kotlin.backend.common.CommonBackendContext -import org.jetbrains.kotlin.backend.common.FileLoweringPass -import org.jetbrains.kotlin.ir.backend.js.ir.JsIrBuilder -import org.jetbrains.kotlin.ir.declarations.IrFile -import org.jetbrains.kotlin.ir.expressions.* -import org.jetbrains.kotlin.ir.symbols.IrFunctionSymbol -import org.jetbrains.kotlin.ir.types.* -import org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid -import org.jetbrains.kotlin.ir.visitors.transformChildrenVoid -import org.jetbrains.kotlin.utils.addToStdlib.safeAs - -class JsStringConcatenationLowering(val context: CommonBackendContext) : FileLoweringPass { - override fun lower(irFile: IrFile) { - irFile.transformChildrenVoid(JsStringConcatenationTransformer(context)) - } -} - -private class JsStringConcatenationTransformer(val context: CommonBackendContext) : IrElementTransformerVoid() { - - private val IrType.shouldExplicitlyConvertToString: Boolean - get() { - // If the type is Long or a supertype of Long, we want to call toString() on values of that type. - // See KT-39891 - if (this !is IrSimpleType) return false - return when (classifier.signature) { - IdSignatureValues.any, IdSignatureValues.comparable, IdSignatureValues.number, IdSignatureValues._long -> true - else -> false - } - } - - private fun IrExpression.explicitlyConvertedToString(): IrExpression { - assert(type.shouldExplicitlyConvertToString) - - return if (type.isNullable()) { - JsIrBuilder.buildCall(context.ir.symbols.extensionToString).apply { - extensionReceiver = this@explicitlyConvertedToString - } - } else { - JsIrBuilder.buildCall(context.ir.symbols.memberToString).apply { - dispatchReceiver = this@explicitlyConvertedToString - } - } - } - - private val IrFunctionSymbol.isStringPlus: Boolean - get() = context.ir.symbols.isStringPlus(this) - - override fun visitCall(expression: IrCall): IrExpression { - fun explicitlyConvertToStringIfNeeded(): IrExpression { - val lastArgIndex = expression.valueArgumentsCount - 1 - val plusArg = expression.getValueArgument(lastArgIndex) ?: return super.visitCall(expression) - if (!plusArg.type.shouldExplicitlyConvertToString) - return super.visitCall(expression) - - expression.putValueArgument(lastArgIndex, plusArg.explicitlyConvertedToString()) - return expression - } - - if (expression.valueArgumentsCount == 0) - return super.visitCall(expression) - - if (expression.symbol.isStringPlus) - return explicitlyConvertToStringIfNeeded() - - if (expression.dispatchReceiver.safeAs()?.symbol?.isStringPlus == true) - return explicitlyConvertToStringIfNeeded() - - return super.visitCall(expression) - } - - override fun visitStringConcatenation(expression: IrStringConcatenation): IrExpression { - expression - .transformChildrenVoid(object : IrElementTransformerVoid() { - override fun visitExpression(expression: IrExpression): IrExpression { - if (expression.type.shouldExplicitlyConvertToString) - return expression.explicitlyConvertedToString() - return expression - } - }) - return super.visitStringConcatenation(expression) - } -} diff --git a/compiler/testData/codegen/box/intrinsics/kt8666.kt b/compiler/testData/codegen/box/intrinsics/kt8666.kt index c2296d30835..53b09b3c1af 100644 --- a/compiler/testData/codegen/box/intrinsics/kt8666.kt +++ b/compiler/testData/codegen/box/intrinsics/kt8666.kt @@ -1,6 +1,17 @@ val MAX_LONG = "9223372036854775807" val PREFIX = "max = " +fun customToString(prefix: String, l: Long, concat: (String, Long) -> String) = concat(prefix, l) + +fun customToString(l: Long, concat: (Long) -> String) = concat(l) + +val stringPlus = String::plus +val stringNPlus = String?::plus +val emptyStringPlus = ""::plus +val emptyStringNPlus = ("" as String?)::plus +val prefixPlus = PREFIX::plus +val prefixNPlus = (PREFIX as String?)::plus + fun box(): String { if (MAX_LONG != "${Long.MAX_VALUE}") return "fail template" if (MAX_LONG != "" + Long.MAX_VALUE) return "fail \"\" +" @@ -8,19 +19,35 @@ fun box(): String { if (MAX_LONG != "".plus(Long.MAX_VALUE)) return "fail \"\".plus" if (MAX_LONG != ("" as String?).plus(Long.MAX_VALUE)) return "fail \"\"?.plus" if (MAX_LONG != (String::plus)("", Long.MAX_VALUE)) return "fail String::plus" + if (MAX_LONG != stringPlus("", Long.MAX_VALUE)) return "fail stringPlus(\"\", Long.MAX_VALUE)" + if (MAX_LONG != customToString("", Long.MAX_VALUE, String::plus)) return "fail customToString(\"\", Long.MAX_VALUE, String::plus)" if (MAX_LONG != (String?::plus)("", Long.MAX_VALUE)) return "fail String?::plus" + if (MAX_LONG != stringNPlus("", Long.MAX_VALUE)) return "fail stringNPlus(\"\", Long.MAX_VALUE)" + if (MAX_LONG != customToString("", Long.MAX_VALUE, String?::plus)) return "fail customToString(\"\", Long.MAX_VALUE, String?::plus)" if (MAX_LONG != (""::plus)(Long.MAX_VALUE)) return "fail \"\"::plus" + if (MAX_LONG != emptyStringPlus(Long.MAX_VALUE)) return "fail emptyStringPlus(Long.MAX_VALUE)" + if (MAX_LONG != customToString(Long.MAX_VALUE, ""::plus)) return "fail customToString(Long.MAX_VALUE, \"\"::plus)" if (MAX_LONG != (("" as String?)::plus)(Long.MAX_VALUE)) return "fail \"\"?::plus" + if (MAX_LONG != emptyStringNPlus(Long.MAX_VALUE)) return "fail emptyStringPlus(Long.MAX_VALUE)" + if (MAX_LONG != customToString(Long.MAX_VALUE, ("" as String?)::plus)) return "fail customToString(Long.MAX_VALUE, (\"\" as String?)::plus)" if (PREFIX + MAX_LONG != "max = ${Long.MAX_VALUE}") return "fail template with prefix" if (PREFIX + MAX_LONG != PREFIX + Long.MAX_VALUE) return "fail \"$PREFIX\" +" if (PREFIX + MAX_LONG != (PREFIX as String?) + Long.MAX_VALUE) return "fail \"$PREFIX\"? +" if (PREFIX + MAX_LONG != PREFIX.plus(Long.MAX_VALUE)) return "fail \"$PREFIX\".plus" if (PREFIX + MAX_LONG != (PREFIX as String?).plus(Long.MAX_VALUE)) return "fail \"$PREFIX\"?.plus" - if (PREFIX + MAX_LONG != (String::plus)(PREFIX, Long.MAX_VALUE)) return "fail String::plus($PREFIX, ...)" - if (PREFIX + MAX_LONG != (String?::plus)(PREFIX, Long.MAX_VALUE)) return "fail String?::plus($PREFIX, ...)" + if (PREFIX + MAX_LONG != (String::plus)(PREFIX, Long.MAX_VALUE)) return "fail String::plus(\"$PREFIX\", ...)" + if (PREFIX + MAX_LONG != stringPlus(PREFIX, Long.MAX_VALUE)) return "fail stringPlus(\"$PREFIX\", Long.MAX_VALUE)" + if (PREFIX + MAX_LONG != customToString(PREFIX, Long.MAX_VALUE, String::plus)) return "fail customToString(\"$PREFIX\", Long.MAX_VALUE, String::plus)" + if (PREFIX + MAX_LONG != (String?::plus)(PREFIX, Long.MAX_VALUE)) return "fail String?::plus(\"$PREFIX\", ...)" + if (PREFIX + MAX_LONG != stringNPlus(PREFIX, Long.MAX_VALUE)) return "fail stringNPlus(\"$PREFIX\", Long.MAX_VALUE)" + if (PREFIX + MAX_LONG != customToString(PREFIX, Long.MAX_VALUE, String?::plus)) return "fail customToString(\"$PREFIX\", Long.MAX_VALUE, String?::plus)" if (PREFIX + MAX_LONG != (PREFIX::plus)(Long.MAX_VALUE)) return "fail \"$PREFIX\"::plus" + if (PREFIX + MAX_LONG != prefixPlus(Long.MAX_VALUE)) return "fail prefixPlus(Long.MAX_VALUE)" + if (PREFIX + MAX_LONG != customToString(Long.MAX_VALUE, PREFIX::plus)) return "fail customToString(Long.MAX_VALUE, \"$PREFIX\"::plus)" if (PREFIX + MAX_LONG != ((PREFIX as String?)::plus)(Long.MAX_VALUE)) return "fail \"$PREFIX\"?::plus" + if (PREFIX + MAX_LONG != prefixNPlus(Long.MAX_VALUE)) return "fail prefixNPlus(Long.MAX_VALUE)" + if (PREFIX + MAX_LONG != customToString(Long.MAX_VALUE, (PREFIX as String?)::plus)) return "fail customToString(Long.MAX_VALUE, \"$PREFIX\"?::plus)" return "OK" } diff --git a/js/js.translator/testData/box/number/kt26706.kt b/js/js.translator/testData/box/number/kt26706.kt index 0e717baff93..bedf13ec2cd 100644 --- a/js/js.translator/testData/box/number/kt26706.kt +++ b/js/js.translator/testData/box/number/kt26706.kt @@ -2,6 +2,29 @@ // EXPECTED_REACHABLE_NODES: 1378 package foo +fun longToString(x: T) = " $x" +fun nullableLongToString1(x: T?) = " $x" +fun nullableLongToString2(x: T) = " $x" +fun nullableLongToString3(x: T?) = " $x" + +fun numberToString(x: T) = " $x" +fun nullableNumberToString1(x: T?) = " $x" +fun nullableNumberToString2(x: T) = " $x" +fun nullableNumberToString3(x: T?) = " $x" + +fun > comparableToString(x: T) = " $x" +fun > nullableComparableToString1(x: T?) = " $x" +fun ?> nullableComparableToString2(x: T) = " $x" +fun ?> nullableComparableToString3(x: T?) = " $x" + +fun anyToString(x: T) = " $x" +fun nullableAnyToString1(x: T?) = " $x" +fun nullableAnyToString2(x: T) = " $x" +fun nullableAnyToString3(x: T?) = " $x" + +fun arbitraryToString(x: T) = " $x" +fun nullableArbitraryToString(x: T?) = " $x" + fun box(): String { val x = "895065487315017728" if (" ${x.toLong()}" != " $x") return "FAIL 1" @@ -16,6 +39,37 @@ fun box(): String { if (" ${null as Number?}" != " null") return "FAIL 10" if (" ${x.toLong() as Any?}" != " $x") return "FAIL 11" if (" ${null as Any?}" != " null") return "FAIL 12" + if (longToString(x.toLong()) != " $x") return "FAIL 13" + if (nullableLongToString1(x.toLong()) != " $x") return "FAIL 14" + if (nullableLongToString1(null) != " null") return "FAIL 15" + if (nullableLongToString2(x.toLong()) != " $x") return "FAIL 16" + if (nullableLongToString2(null) != " null") return "FAIL 17" + if (nullableLongToString3(x.toLong()) != " $x") return "FAIL 18" + if (nullableLongToString3(null) != " null") return "FAIL 19" + if (numberToString(x.toLong()) != " $x") return "FAIL 20" + if (nullableNumberToString1(x.toLong()) != " $x") return "FAIL 21" + if (nullableNumberToString1(null) != " null") return "FAIL 22" + if (nullableNumberToString2(x.toLong()) != " $x") return "FAIL 23" + if (nullableNumberToString2(null) != " null") return "FAIL 24" + if (nullableNumberToString3(x.toLong()) != " $x") return "FAIL 25" + if (nullableNumberToString3(null) != " null") return "FAIL 26" + if (comparableToString(x.toLong()) != " $x") return "FAIL 27" + if (nullableComparableToString1(x.toLong()) != " $x") return "FAIL 28" + if (nullableComparableToString1(null) != " null") return "FAIL 29" + if (nullableComparableToString2(x.toLong()) != " $x") return "FAIL 30" + if (nullableComparableToString2(null) != " null") return "FAIL 31" + if (nullableComparableToString3(x.toLong()) != " $x") return "FAIL 32" + if (nullableComparableToString3(null) != " null") return "FAIL 33" + if (anyToString(x.toLong()) != " $x") return "FAIL 34" + if (nullableAnyToString1(x.toLong()) != " $x") return "FAIL 35" + if (nullableAnyToString1(null) != " null") return "FAIL 36" + if (nullableAnyToString2(x.toLong()) != " $x") return "FAIL 37" + if (nullableAnyToString2(null) != " null") return "FAIL 38" + if (nullableAnyToString3(x.toLong()) != " $x") return "FAIL 39" + if (nullableAnyToString3(null) != " null") return "FAIL 40" + if (arbitraryToString(x.toLong()) != " $x") return "FAIL 41" + if (nullableArbitraryToString(x.toLong()) != " $x") return "FAIL 42" + if (nullableArbitraryToString(null) != " null") return "FAIL 43" return "OK" -} \ No newline at end of file +} diff --git a/libraries/stdlib/js-ir/runtime/long.kt b/libraries/stdlib/js-ir/runtime/long.kt index e1738de723f..8e3cfe7bba1 100644 --- a/libraries/stdlib/js-ir/runtime/long.kt +++ b/libraries/stdlib/js-ir/runtime/long.kt @@ -279,13 +279,9 @@ public class Long internal constructor( public override fun toFloat(): Float = toDouble().toFloat() public override fun toDouble(): Double = toNumber() - // This method is used by `toString()` - @JsName("valueOf") - internal fun valueOf() = toDouble() - override fun equals(other: Any?): Boolean = other is Long && equalsLong(other) override fun hashCode(): Int = hashCode(this) override fun toString(): String = this.toStringImpl(radix = 10) -} \ No newline at end of file +}