From dce0971c525df84e7bc49eca10690a8b32e253ca Mon Sep 17 00:00:00 2001 From: Alexey Tsvetkov Date: Wed, 10 Jun 2015 21:39:56 +0300 Subject: [PATCH] JS/RTTI: fixed double testing for null for reified --- .../test/semantics/ReifiedTestGenerated.java | 6 ++ .../js/test/utils/DirectiveTestUtils.java | 3 + .../js/translate/utils/expandIsCalls.kt | 63 ++++++++++++++----- .../testData/reified/cases/isTNullable.kt | 21 +++++++ 4 files changed, 79 insertions(+), 14 deletions(-) create mode 100644 js/js.translator/testData/reified/cases/isTNullable.kt diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/ReifiedTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/ReifiedTestGenerated.java index 13f243c6441..09a38bde12c 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/ReifiedTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/ReifiedTestGenerated.java @@ -95,6 +95,12 @@ public class ReifiedTestGenerated extends AbstractReifiedTest { doTest(fileName); } + @TestMetadata("isTNullable.kt") + public void testIsTNullable() throws Exception { + String fileName = JetTestUtils.navigationMetadata("js/js.translator/testData/reified/cases/isTNullable.kt"); + doTest(fileName); + } + @TestMetadata("lambda.kt") public void testLambda() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/reified/cases/lambda.kt"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/utils/DirectiveTestUtils.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/utils/DirectiveTestUtils.java index 0c8fd84f5e8..189273c5485 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/utils/DirectiveTestUtils.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/utils/DirectiveTestUtils.java @@ -146,6 +146,8 @@ public class DirectiveTestUtils { private static final DirectiveHandler COUNT_BREAKS = new CountNodesDirective("CHECK_BREAKS_COUNT", JsBreak.class); + private static final DirectiveHandler COUNT_NULLS = new CountNodesDirective("CHECK_NULLS_COUNT", JsNullLiteral.class); + private static final DirectiveHandler HAS_INLINE_METADATA = new DirectiveHandler("CHECK_HAS_INLINE_METADATA") { @Override void processEntry(@NotNull JsNode ast, @NotNull ArgumentsHelper arguments) throws Exception { @@ -175,6 +177,7 @@ public class DirectiveTestUtils { COUNT_LABELS, COUNT_VARS, COUNT_BREAKS, + COUNT_NULLS, HAS_INLINE_METADATA, HAS_NO_INLINE_METADATA ); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/expandIsCalls.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/expandIsCalls.kt index ef59ed36c83..f8205c51244 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/expandIsCalls.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/expandIsCalls.kt @@ -19,6 +19,7 @@ package org.jetbrains.kotlin.js.translate.utils import com.google.dart.compiler.backend.js.ast.* import com.google.dart.compiler.backend.js.ast.metadata.TypeCheck import com.google.dart.compiler.backend.js.ast.metadata.typeCheck +import org.jetbrains.kotlin.js.inline.util.IdentitySet import org.jetbrains.kotlin.js.translate.context.TranslationContext import org.jetbrains.kotlin.js.translate.utils.JsAstUtils.* import java.util.* @@ -30,14 +31,22 @@ fun expandIsCalls(node: JsNode, context: TranslationContext) { private class TypeCheckRewritingVisitor(private val context: TranslationContext) : JsVisitorWithContextImpl() { private val scopes = Stack() + private val localVars = Stack>() override fun visit(x: JsFunction, ctx: JsContext<*>): Boolean { scopes.push(x.scope) + localVars.push(IdentitySet()) + return super.visit(x, ctx) + } + + override fun visit(x: JsVars.JsVar, ctx: JsContext<*>): Boolean { + localVars.peek().add(x.name) return super.visit(x, ctx) } override fun endVisit(x: JsFunction, ctx: JsContext<*>) { scopes.pop() + localVars.pop() super.endVisit(x, ctx) } @@ -60,28 +69,54 @@ private class TypeCheckRewritingVisitor(private val context: TranslationContext) } private fun getReplacement(callee: JsInvocation, calleeArgument: JsExpression, argument: JsExpression): JsExpression? { - return when (callee.typeCheck) { - // Kotlin.isTypeOf(calleeArgument)(argument) -> typeOf argument === calleeArgument - TypeCheck.TYPEOF -> - typeOfIs(argument, calleeArgument as JsStringLiteral) + // Kotlin.isTypeOf(calleeArgument)(argument) -> typeOf argument === calleeArgument + if (callee.typeCheck == TypeCheck.TYPEOF) { + return typeOfIs(argument, calleeArgument as JsStringLiteral) + } - // Kotlin.isInstanceOf(calleeArgument)(argument) -> argument instanceof calleeArgument - TypeCheck.INSTANCEOF -> - context.namer().isInstanceOf(argument, calleeArgument) + // Kotlin.isInstanceOf(calleeArgument)(argument) -> argument instanceof calleeArgument + if (callee.typeCheck == TypeCheck.INSTANCEOF) { + return context.namer().isInstanceOf(argument, calleeArgument) + } - // Kotlin.orNull(calleeArgument)(argument) -> (tmp = argument) == null || calleeArgument(tmp) - TypeCheck.OR_NULL -> { + // Kotlin.orNull(calleeArgument)(argument) -> (tmp = argument) == null || calleeArgument(tmp) + if (callee.typeCheck == TypeCheck.OR_NULL) { + if (calleeArgument is JsInvocation && calleeArgument.typeCheck == TypeCheck.OR_NULL) { + return JsInvocation(calleeArgument, argument) + } + + var nullCheckTarget = argument + var nextCheckTarget = argument + + if (argument.isAssignmentToLocalVar) { + // Kotlin.orNull(Kotlin.isInstance(SomeType))(localVar=someExpr) -> (localVar=someExpr) != null || Kotlin.isInstance(SomeType)(localVar) + val localVar = (argument as JsBinaryOperation).getArg1() + nextCheckTarget = localVar + } + else if (!argument.isLocalVar) { val currentScope = scopes.peek() val tmp = currentScope.declareTemporary() val statementContext = lastStatementLevelContext statementContext.addPrevious(newVar(tmp, null)) - val assignment = assignment(tmp.makeRef(), argument) - val tmpIsNull = TranslationUtils.isNullCheck(assignment) - or(tmpIsNull, JsInvocation(calleeArgument, tmp.makeRef())) + nullCheckTarget = assignment(tmp.makeRef(), argument) + nextCheckTarget = tmp.makeRef() } - else -> - null + val isNull = TranslationUtils.isNullCheck(nullCheckTarget) + return or(isNull, JsInvocation(calleeArgument, nextCheckTarget)) } + + return null } + + private val JsExpression.isLocalVar: Boolean + get() { + if (localVars.empty() || this !is JsNameRef) return false + + val name = this.getName() + return name != null && localVars.peek().contains(name) + } + + private val JsExpression.isAssignmentToLocalVar: Boolean + get() = this is JsBinaryOperation && getOperator() == JsBinaryOperator.ASG } diff --git a/js/js.translator/testData/reified/cases/isTNullable.kt b/js/js.translator/testData/reified/cases/isTNullable.kt new file mode 100644 index 00000000000..fe9f5e8e4f2 --- /dev/null +++ b/js/js.translator/testData/reified/cases/isTNullable.kt @@ -0,0 +1,21 @@ +package foo + +// CHECK_NOT_CALLED: isTypeOfOrNull +// CHECK_NULLS_COUNT: function=box count=8 + +inline +fun Any?.isTypeOfOrNull() = this is T? + +class A +class B + +fun box(): String { + assertEquals(true, null.isTypeOfOrNull(), "null.isTypeOfOrNull()") + assertEquals(true, null.isTypeOfOrNull(), "null.isTypeOfOrNull()") + assertEquals(true, A().isTypeOfOrNull(), "A().isTypeOfOrNull()") + assertEquals(true, A().isTypeOfOrNull(), "A().isTypeOfOrNull()") + assertEquals(false, A().isTypeOfOrNull(), "A().isTypeOfOrNull()") + assertEquals(false, A().isTypeOfOrNull(), "A().isTypeOfOrNull()") + + return "OK" +} \ No newline at end of file