From 5b11ee251f82cbd4e960b61370231ae4f9ee3f59 Mon Sep 17 00:00:00 2001 From: Anton Bannykh Date: Wed, 28 Feb 2018 21:56:37 +0300 Subject: [PATCH] Simplify binary operation intrinsics Having to invoke `exist` so prevent a runtime exception is no better then trying to invoke a method on a null reference. It is actually worse, because Kotlin type system handles the latter. --- .../js/translate/intrinsic/Intrinsics.kt | 2 +- .../functions/FunctionIntrinsics.java | 1 - .../factories}/StringPlusCharFIF.kt | 19 +---- .../intrinsic/operation/AssignmentBOIF.kt | 2 +- .../intrinsic/operation/CompareToBOIF.kt | 8 +-- .../intrinsic/operation/EqualsBOIF.kt | 12 ++-- .../intrinsic/operation/LongCompareToBOIF.kt | 6 +- .../operation/binaryOperationIntrinsics.kt | 72 +++++++------------ .../operation/BinaryOperationTranslator.java | 4 +- 9 files changed, 47 insertions(+), 79 deletions(-) rename js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/{operation => functions/factories}/StringPlusCharFIF.kt (75%) diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/Intrinsics.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/Intrinsics.kt index d0be2c8e26b..fc7a85f8d3a 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/Intrinsics.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/Intrinsics.kt @@ -35,7 +35,7 @@ class Intrinsics { private val binaryOperationIntrinsics = BinaryOperationIntrinsics() private val objectIntrinsics = ObjectIntrinsics() - fun getBinaryOperationIntrinsic(expression: KtBinaryExpression, context: TranslationContext): BinaryOperationIntrinsic { + fun getBinaryOperationIntrinsic(expression: KtBinaryExpression, context: TranslationContext): BinaryOperationIntrinsic? { return binaryOperationIntrinsics.getIntrinsic(expression, context) } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/functions/FunctionIntrinsics.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/functions/FunctionIntrinsics.java index 6f3f36e6b0a..f0b81ae8a46 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/functions/FunctionIntrinsics.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/functions/FunctionIntrinsics.java @@ -23,7 +23,6 @@ import org.jetbrains.annotations.Nullable; import org.jetbrains.kotlin.descriptors.FunctionDescriptor; import org.jetbrains.kotlin.js.translate.intrinsic.functions.basic.FunctionIntrinsic; import org.jetbrains.kotlin.js.translate.intrinsic.functions.factories.*; -import org.jetbrains.kotlin.js.translate.intrinsic.operation.StringPlusCharFIF; import java.util.List; import java.util.Map; diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/StringPlusCharFIF.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/functions/factories/StringPlusCharFIF.kt similarity index 75% rename from js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/StringPlusCharFIF.kt rename to js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/functions/factories/StringPlusCharFIF.kt index 0ba2985cc64..61498e7653f 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/StringPlusCharFIF.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/functions/factories/StringPlusCharFIF.kt @@ -1,20 +1,9 @@ /* - * Copyright 2010-2016 JetBrains s.r.o. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Copyright 2000-2018 JetBrains s.r.o. 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.js.translate.intrinsic.operation +package org.jetbrains.kotlin.js.translate.intrinsic.functions.factories import org.jetbrains.kotlin.builtins.KotlinBuiltIns import org.jetbrains.kotlin.descriptors.FunctionDescriptor @@ -24,8 +13,6 @@ import org.jetbrains.kotlin.js.backend.ast.JsExpression import org.jetbrains.kotlin.js.translate.callTranslator.CallInfo import org.jetbrains.kotlin.js.translate.context.TranslationContext import org.jetbrains.kotlin.js.translate.intrinsic.functions.basic.FunctionIntrinsic -import org.jetbrains.kotlin.js.translate.intrinsic.functions.factories.FunctionIntrinsicFactory -import org.jetbrains.kotlin.js.translate.intrinsic.functions.factories.TopLevelFIF import org.jetbrains.kotlin.js.translate.utils.JsAstUtils import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameUnsafe import org.jetbrains.kotlin.types.TypeUtils diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/AssignmentBOIF.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/AssignmentBOIF.kt index 66293a8dbba..776fa0ff141 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/AssignmentBOIF.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/AssignmentBOIF.kt @@ -31,7 +31,7 @@ import org.jetbrains.kotlin.types.KotlinType object AssignmentBOIF : BinaryOperationIntrinsicFactory { - private object CharAssignmentIntrinsic : AbstractBinaryOperationIntrinsic() { + private object CharAssignmentIntrinsic : BinaryOperationIntrinsic { override fun apply(expression: KtBinaryExpression, left: JsExpression, right: JsExpression, context: TranslationContext): JsExpression { val operator = OperatorTable.getBinaryOperator(PsiUtils.getOperationToken(expression)) return JsBinaryOperation(operator, left, TranslationUtils.charToBoxedChar(context, right)) diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/CompareToBOIF.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/CompareToBOIF.kt index 32af369afe0..342a086ce32 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/CompareToBOIF.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/CompareToBOIF.kt @@ -32,28 +32,28 @@ import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.expressions.OperatorConventions object CompareToBOIF : BinaryOperationIntrinsicFactory { - private object CompareToIntrinsic : AbstractBinaryOperationIntrinsic() { + private object CompareToIntrinsic : BinaryOperationIntrinsic { override fun apply(expression: KtBinaryExpression, left: JsExpression, right: JsExpression, context: TranslationContext): JsExpression { val operator = OperatorTable.getBinaryOperator(getOperationToken(expression)) return JsBinaryOperation(operator, left, right) } } - private object CompareToCharIntrinsic : AbstractBinaryOperationIntrinsic() { + private object CompareToCharIntrinsic : BinaryOperationIntrinsic { override fun apply(expression: KtBinaryExpression, left: JsExpression, right: JsExpression, context: TranslationContext): JsExpression { val operator = OperatorTable.getBinaryOperator(getOperationToken(expression)) return JsBinaryOperation(operator, left, JsAstUtils.charToInt(right)) } } - private object CompareCharToPrimitiveIntrinsic : AbstractBinaryOperationIntrinsic() { + private object CompareCharToPrimitiveIntrinsic : BinaryOperationIntrinsic { override fun apply(expression: KtBinaryExpression, left: JsExpression, right: JsExpression, context: TranslationContext): JsExpression { val operator = OperatorTable.getBinaryOperator(getOperationToken(expression)) return JsBinaryOperation(operator, JsAstUtils.charToInt(left), right) } } - private object CompareToFunctionIntrinsic : AbstractBinaryOperationIntrinsic() { + private object CompareToFunctionIntrinsic : BinaryOperationIntrinsic{ override fun apply(expression: KtBinaryExpression, left: JsExpression, right: JsExpression, context: TranslationContext): JsExpression { val operator = OperatorTable.getBinaryOperator(getOperationToken(expression)) val compareTo = JsAstUtils.compareTo(left, right) diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/EqualsBOIF.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/EqualsBOIF.kt index d4562bd6bc0..33b9b1b25cf 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/EqualsBOIF.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/EqualsBOIF.kt @@ -40,7 +40,7 @@ import org.jetbrains.kotlin.types.typeUtil.makeNullable import java.util.* object EqualsBOIF : BinaryOperationIntrinsicFactory { - private object EqualsIntrinsic : AbstractBinaryOperationIntrinsic() { + private object EqualsIntrinsic : BinaryOperationIntrinsic { private val JS_NUMBER_PRIMITIVES = EnumSet.of(PrimitiveType.BYTE, PrimitiveType.SHORT, PrimitiveType.INT, PrimitiveType.DOUBLE, PrimitiveType.FLOAT) @@ -99,7 +99,7 @@ object EqualsBOIF : BinaryOperationIntrinsicFactory { } } - object EnumEqualsIntrinsic : AbstractBinaryOperationIntrinsic() { + object EnumEqualsIntrinsic : BinaryOperationIntrinsic { override fun apply(expression: KtBinaryExpression, left: JsExpression, right: JsExpression, context: TranslationContext): JsBinaryOperation { val operator = if (expression.isNegated()) JsBinaryOperator.REF_NEQ else JsBinaryOperator.REF_EQ return JsBinaryOperation(operator, left, right) @@ -112,11 +112,11 @@ object EqualsBOIF : BinaryOperationIntrinsicFactory { when { isEnumIntrinsicApplicable(descriptor, leftType, rightType) -> EnumEqualsIntrinsic - KotlinBuiltIns.isBuiltIn(descriptor) || - TopLevelFIF.EQUALS_IN_ANY.test(descriptor) -> EqualsIntrinsic + KotlinBuiltIns.isBuiltIn(descriptor) || + TopLevelFIF.EQUALS_IN_ANY.test(descriptor) -> EqualsIntrinsic - else -> null - } + else -> null + } private fun isEnumIntrinsicApplicable(descriptor: FunctionDescriptor, leftType: KotlinType?, rightType: KotlinType?): Boolean { return DescriptorUtils.isEnumClass(descriptor.containingDeclaration) && leftType != null && rightType != null && diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/LongCompareToBOIF.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/LongCompareToBOIF.kt index c20c9770c29..412e4818da0 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/LongCompareToBOIF.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/LongCompareToBOIF.kt @@ -42,21 +42,21 @@ object LongCompareToBOIF : BinaryOperationIntrinsicFactory { val LONG_COMPARE_TO_CHAR_PATTERN = pattern("Long.compareTo(Char)") val LONG_COMPARE_TO_LONG_PATTERN = pattern("Long.compareTo(Long)") - private object FLOATING_POINT_COMPARE_TO_LONG : AbstractBinaryOperationIntrinsic() { + private object FLOATING_POINT_COMPARE_TO_LONG : BinaryOperationIntrinsic { override fun apply(expression: KtBinaryExpression, left: JsExpression, right: JsExpression, context: TranslationContext): JsExpression { val operator = OperatorTable.getBinaryOperator(getOperationToken(expression)) return JsBinaryOperation(operator, left, invokeMethod(right, Namer.LONG_TO_NUMBER)) } } - private object LONG_COMPARE_TO_FLOATING_POINT : AbstractBinaryOperationIntrinsic() { + private object LONG_COMPARE_TO_FLOATING_POINT : BinaryOperationIntrinsic { override fun apply(expression: KtBinaryExpression, left: JsExpression, right: JsExpression, context: TranslationContext): JsExpression { val operator = OperatorTable.getBinaryOperator(getOperationToken(expression)) return JsBinaryOperation(operator, invokeMethod(left, Namer.LONG_TO_NUMBER), right) } } - private class CompareToBinaryIntrinsic(val toLeft: (JsExpression) -> JsExpression, val toRight: (JsExpression) -> JsExpression) : AbstractBinaryOperationIntrinsic() { + private class CompareToBinaryIntrinsic(val toLeft: (JsExpression) -> JsExpression, val toRight: (JsExpression) -> JsExpression) : BinaryOperationIntrinsic { override fun apply(expression: KtBinaryExpression, left: JsExpression, right: JsExpression, context: TranslationContext): JsExpression { val operator = OperatorTable.getBinaryOperator(getOperationToken(expression)) val compareInvocation = compareForObject(toLeft(left), toRight(right)) diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/binaryOperationIntrinsics.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/binaryOperationIntrinsics.kt index 01c35d493cf..8d08895341f 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/binaryOperationIntrinsics.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/intrinsic/operation/binaryOperationIntrinsics.kt @@ -26,46 +26,47 @@ import org.jetbrains.kotlin.js.translate.utils.getPrimitiveNumericComparisonInfo import org.jetbrains.kotlin.lexer.KtToken import org.jetbrains.kotlin.psi.KtBinaryExpression import org.jetbrains.kotlin.types.KotlinType +import org.jetbrains.kotlin.utils.addToStdlib.firstNotNullResult interface BinaryOperationIntrinsic { - fun apply(expression: KtBinaryExpression, left: JsExpression, right: JsExpression, context: TranslationContext): JsExpression - - fun exists(): Boolean } class BinaryOperationIntrinsics { - private val intrinsicCache = mutableMapOf() + private data class IntrinsicKey( + val token: KtToken, + val function: FunctionDescriptor, + val leftType: KotlinType?, + val rightType: KotlinType? + ) - private val factories = listOf(LongCompareToBOIF, EqualsBOIF, CompareToBOIF, AssignmentBOIF) + private val intrinsicCache = mutableMapOf() - fun getIntrinsic(expression: KtBinaryExpression, context: TranslationContext): BinaryOperationIntrinsic { - val token = getOperationToken(expression) - val descriptor = getCallableDescriptorForOperationExpression(context.bindingContext(), expression) - if (descriptor == null || descriptor !is FunctionDescriptor) { - return NO_INTRINSIC - } + fun getIntrinsic(expression: KtBinaryExpression, context: TranslationContext): BinaryOperationIntrinsic? { + val descriptor = getCallableDescriptorForOperationExpression(context.bindingContext(), expression) as? FunctionDescriptor ?: return null val (leftType, rightType) = binaryOperationTypes(expression, context) - val key = IntrinsicKey(token, descriptor, leftType, rightType) - return intrinsicCache.getOrPut(key) { computeIntrinsic(token, descriptor, leftType, rightType) } + val token = getOperationToken(expression) + + return computeAndCache(IntrinsicKey(token, descriptor, leftType, rightType)) } - private fun computeIntrinsic( - token: KtToken, descriptor: FunctionDescriptor, - leftType: KotlinType?, rightType: KotlinType? - ): BinaryOperationIntrinsic { - for (factory in factories) { - if (factory.getSupportTokens().contains(token)) { - val intrinsic = factory.getIntrinsic(descriptor, leftType, rightType) - if (intrinsic != null) { - return intrinsic - } - } + private val factories = listOf(LongCompareToBOIF, EqualsBOIF, CompareToBOIF, AssignmentBOIF) + + private fun computeAndCache(key: IntrinsicKey): BinaryOperationIntrinsic? { + if (key in intrinsicCache) return intrinsicCache[key] + + val result = factories.firstNotNullResult { factory -> + if (factory.getSupportTokens().contains(key.token)) { + factory.getIntrinsic(key.function, key.leftType, key.rightType) + } else null } - return NO_INTRINSIC + + intrinsicCache[key] = result + + return result } } @@ -78,28 +79,9 @@ fun binaryOperationTypes(expression: KtBinaryExpression, context: TranslationCon return expression.left?.let { context.getPrecisePrimitiveType(it) } to expression.right?.let { context.getPrecisePrimitiveType(it) } } -private data class IntrinsicKey( - val token: KtToken, val function: FunctionDescriptor, - val leftType: KotlinType?, val rightType: KotlinType? -) - interface BinaryOperationIntrinsicFactory { fun getSupportTokens(): Set fun getIntrinsic(descriptor: FunctionDescriptor, leftType: KotlinType?, rightType: KotlinType?): BinaryOperationIntrinsic? -} - -abstract class AbstractBinaryOperationIntrinsic : BinaryOperationIntrinsic { - - override abstract fun apply(expression: KtBinaryExpression, left: JsExpression, right: JsExpression, context: TranslationContext): JsExpression - - override fun exists(): Boolean = true -} - -object NO_INTRINSIC : AbstractBinaryOperationIntrinsic() { - override fun exists(): Boolean = false - - override fun apply(expression: KtBinaryExpression, left: JsExpression, right: JsExpression, context: TranslationContext): JsExpression = - throw UnsupportedOperationException("BinaryOperationIntrinsic#NO_INTRINSIC_#apply") -} +} \ No newline at end of file diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/operation/BinaryOperationTranslator.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/operation/BinaryOperationTranslator.java index 47ab7979221..1d7b95afe2f 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/operation/BinaryOperationTranslator.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/operation/BinaryOperationTranslator.java @@ -101,7 +101,7 @@ public final class BinaryOperationTranslator extends AbstractTranslator { @NotNull private JsExpression translate() { BinaryOperationIntrinsic intrinsic = getIntrinsicForExpression(); - if (intrinsic.exists()) { + if (intrinsic != null) { return applyIntrinsic(intrinsic); } if (operationToken == KtTokens.ELVIS) { @@ -158,7 +158,7 @@ public final class BinaryOperationTranslator extends AbstractTranslator { return result; } - @NotNull + @Nullable private BinaryOperationIntrinsic getIntrinsicForExpression() { return context().intrinsics().getBinaryOperationIntrinsic(expression, context()); }