From 92457543bc9d33c80c830d17fbfd7983cf4bb274 Mon Sep 17 00:00:00 2001 From: Zalim Bashorov Date: Tue, 2 Jun 2015 20:07:48 +0300 Subject: [PATCH] JS backend: fix equality check for nullable types * Generate simple check when compare with `null` literal. * Use Kotlin.equals if type of receiver or argument is nullable. * Don't call equals method if any of receiver or argument is null (at runtime, not only literal). For more information see KT-4356. #KT-7530 fixed #KT-7916 fixed --- .../kotlin/js/test/semantics/EqualsTest.java | 12 +++++ .../operation/BinaryOperationTranslator.java | 32 ++++++++++++- .../cases/compareNullablesWithCustomEquals.kt | 47 +++++++++++++++++++ .../cases/compareToNullWithCustomEquals.kt | 16 +++++++ .../equals/cases/equalsBehaviorOnNull.kt | 23 +++++++++ js/js.translator/testData/kotlin_lib.js | 4 ++ 6 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 js/js.translator/testData/expression/equals/cases/compareNullablesWithCustomEquals.kt create mode 100644 js/js.translator/testData/expression/equals/cases/compareToNullWithCustomEquals.kt create mode 100644 js/js.translator/testData/expression/equals/cases/equalsBehaviorOnNull.kt diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/EqualsTest.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/EqualsTest.java index 740c2bd33c2..2b451f7eec1 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/EqualsTest.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/EqualsTest.java @@ -53,4 +53,16 @@ public final class EqualsTest extends AbstractExpressionTest { public void testCompareNullableListWithNull() throws Exception { checkFooBoxIsOk(); } + + public void testCompareToNullWithCustomEquals() throws Exception { + checkFooBoxIsOk(); + } + + public void testCompareNullablesWithCustomEquals() throws Exception { + checkFooBoxIsOk(); + } + + public void testEqualsBehaviorOnNull() throws Exception { + checkFooBoxIsOk(); + } } 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 4d738ffc95d..5ea45866e6f 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 @@ -26,6 +26,7 @@ import org.jetbrains.kotlin.js.translate.context.TemporaryVariable; import org.jetbrains.kotlin.js.translate.context.TranslationContext; import org.jetbrains.kotlin.js.translate.general.AbstractTranslator; import org.jetbrains.kotlin.js.translate.general.Translation; +import org.jetbrains.kotlin.js.translate.intrinsic.functions.factories.TopLevelFIF; import org.jetbrains.kotlin.js.translate.intrinsic.operation.BinaryOperationIntrinsic; import org.jetbrains.kotlin.js.translate.utils.JsAstUtils; import org.jetbrains.kotlin.js.translate.utils.TranslationUtils; @@ -35,8 +36,12 @@ import org.jetbrains.kotlin.psi.JetBinaryExpression; import org.jetbrains.kotlin.psi.JetExpression; import org.jetbrains.kotlin.resolve.bindingContextUtil.BindingContextUtilPackage; import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall; +import org.jetbrains.kotlin.types.JetType; +import org.jetbrains.kotlin.types.TypeUtils; import org.jetbrains.kotlin.types.expressions.OperatorConventions; +import java.util.Collections; + import static org.jetbrains.kotlin.js.translate.operation.AssignmentTranslator.isAssignmentOperator; import static org.jetbrains.kotlin.js.translate.operation.CompareToTranslator.isCompareToCall; import static org.jetbrains.kotlin.js.translate.utils.BindingUtils.getCallableDescriptorForOperationExpression; @@ -108,6 +113,9 @@ public final class BinaryOperationTranslator extends AbstractTranslator { if (isCompareToCall(operationToken, operationDescriptor)) { return CompareToTranslator.translate(expression, context()); } + if (isEquals()) { + return translateEquals(); + } assert operationDescriptor != null : "Overloadable operations must have not null descriptor"; return translateAsOverloadedBinaryOperation(); @@ -246,11 +254,33 @@ public final class BinaryOperationTranslator extends AbstractTranslator { return result; } + private boolean isEquals() { + return operationToken == JetTokens.EQEQ || operationToken == JetTokens.EXCLEQ; + } + + private JsExpression translateEquals() { + JsExpression left = Translation.translateAsExpression(leftJetExpression, context()); + JsExpression right = Translation.translateAsExpression(rightJetExpression, context()); + + if (left == JsLiteral.NULL || right == JsLiteral.NULL) { + JsBinaryOperator operator = operationToken == JetTokens.EXCLEQ ? JsBinaryOperator.NEQ : JsBinaryOperator.EQ; + return new JsBinaryOperation(operator, left, right); + } + + JetType leftType = context().bindingContext().getType(leftJetExpression); + JetType rightType = context().bindingContext().getType(rightJetExpression); + + if (leftType != null && TypeUtils.isNullableType(leftType) || rightType != null && TypeUtils.isNullableType(rightType)) { + return mayBeWrapWithNegation(TopLevelFIF.KOTLIN_EQUALS.apply(left, Collections.singletonList(right), context())); + } + + return translateAsOverloadedBinaryOperation(); + } @NotNull private JsExpression translateAsOverloadedBinaryOperation() { ResolvedCall resolvedCall = getFunctionResolvedCallWithAssert(expression, bindingContext()); - JsExpression result = CallTranslator.INSTANCE$.translate(context(), resolvedCall, getReceiver()); + JsExpression result = CallTranslator.translate(context(), resolvedCall, getReceiver()); return mayBeWrapWithNegation(result); } diff --git a/js/js.translator/testData/expression/equals/cases/compareNullablesWithCustomEquals.kt b/js/js.translator/testData/expression/equals/cases/compareNullablesWithCustomEquals.kt new file mode 100644 index 00000000000..8017b1913be --- /dev/null +++ b/js/js.translator/testData/expression/equals/cases/compareNullablesWithCustomEquals.kt @@ -0,0 +1,47 @@ +package foo + +class A { + override fun equals(other: Any?) = this === other +} + +fun box(): String { + val a: A? = null + val b: A? = null + val c: A? = A() + val d: A? = A() + val e: A = A() + + // compare nullable vals with null + testTrue { a == b } + testTrue { a == a } + testFalse { a != b } + testFalse { a != a } + + // compare null and non-null inside nullable vals + testFalse { a == c } + testTrue { a != c } + testFalse { c == a } + testTrue { c != a } + + // compare nullables vals with non-null + testFalse { c == d } + testTrue { c == c } + testTrue { c != d } + testFalse { d == c } + testTrue { d != c } + testFalse { d != d } + + // compare nullable val with null with non-nullable + testFalse { a == e } + testTrue { a != e } + testFalse { e == a } + testTrue { e != a } + + // compare nullable val with non-null with non-nullable + testFalse { c == e } + testTrue { c != e } + testFalse { e == c } + testTrue { e != c } + + return "OK" +} diff --git a/js/js.translator/testData/expression/equals/cases/compareToNullWithCustomEquals.kt b/js/js.translator/testData/expression/equals/cases/compareToNullWithCustomEquals.kt new file mode 100644 index 00000000000..1258615211c --- /dev/null +++ b/js/js.translator/testData/expression/equals/cases/compareToNullWithCustomEquals.kt @@ -0,0 +1,16 @@ +package foo + +class A { + override fun equals(other: Any?) = super.equals(other) +} + +fun box(): String { + val a: A? = null + + testTrue { a == null } + testFalse { a != null } + testTrue { null == a } + testFalse { null != a } + + return "OK" +} diff --git a/js/js.translator/testData/expression/equals/cases/equalsBehaviorOnNull.kt b/js/js.translator/testData/expression/equals/cases/equalsBehaviorOnNull.kt new file mode 100644 index 00000000000..a899ce0bf7c --- /dev/null +++ b/js/js.translator/testData/expression/equals/cases/equalsBehaviorOnNull.kt @@ -0,0 +1,23 @@ +package foo + +var log = "" + +class A { + override fun equals(o: Any?): Boolean { + log += "$o;" + return true + } +} + +fun box(): String { + val a = A() + + assertTrue(a.equals("aaa")) + assertEquals("aaa;", log) + assertFalse(a == null) + assertEquals("aaa;", log) + assertTrue(a.equals(null)) + assertEquals("aaa;null;", log) + + return "OK" +} diff --git a/js/js.translator/testData/kotlin_lib.js b/js/js.translator/testData/kotlin_lib.js index c0c4c6b545d..30c968e6a84 100644 --- a/js/js.translator/testData/kotlin_lib.js +++ b/js/js.translator/testData/kotlin_lib.js @@ -47,6 +47,10 @@ return obj2 == null; } + if (obj2 == null) { + return false; + } + if (Array.isArray(obj1)) { return Kotlin.arrayEquals(obj1, obj2); }