From 66c78be10f096970b4d131338ef2186d670f1709 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Fri, 9 Sep 2016 16:43:31 +0300 Subject: [PATCH] JS: don't optimize RTTI when possible, i.e. don't remove `is` and `as` when object is known at compile time to have type in RHS of expression. Substitute kotlin.Any with Object. Fix #KT-7665, #KT-7664 --- .../codegen/box/casts/lambdaToUnitCast.kt | 3 ++ .../js/ast/metadata/metadataProperties.kt | 1 - .../kotlin/js/test/rhino/RhinoUtils.java | 8 ++--- .../js/test/semantics/BoxJsTestGenerated.java | 12 +++---- .../semantics/JsCodegenBoxTestGenerated.java | 8 ++++- .../kotlin/js/translate/context/Namer.java | 5 --- .../js/translate/context/StaticContext.java | 3 ++ .../translate/expression/CatchTranslator.kt | 2 +- .../expression/PatternTranslator.java | 16 ++-------- .../translate/expression/WhenTranslator.java | 4 +-- .../js/translate/utils/expandIsCalls.kt | 5 --- .../box/expression/cast/noRuntimeTypeCheck.kt | 28 ---------------- .../testData/box/standardClasses/any.kt | 32 +++++++++++++++++++ js/js.translator/testData/kotlin_lib_ecma5.js | 18 +++++++---- 14 files changed, 71 insertions(+), 74 deletions(-) delete mode 100644 js/js.translator/testData/box/expression/cast/noRuntimeTypeCheck.kt create mode 100644 js/js.translator/testData/box/standardClasses/any.kt diff --git a/compiler/testData/codegen/box/casts/lambdaToUnitCast.kt b/compiler/testData/codegen/box/casts/lambdaToUnitCast.kt index 94ddfb05c00..42a86dcbb07 100644 --- a/compiler/testData/codegen/box/casts/lambdaToUnitCast.kt +++ b/compiler/testData/codegen/box/casts/lambdaToUnitCast.kt @@ -1,3 +1,6 @@ +// IGNORE_BACKEND: JS +// JS backend does not support Unit well. See KT-13932 + val foo: () -> Unit = {} fun box(): String { diff --git a/js/js.dart-ast/src/com/google/dart/compiler/backend/js/ast/metadata/metadataProperties.kt b/js/js.dart-ast/src/com/google/dart/compiler/backend/js/ast/metadata/metadataProperties.kt index f3ed20ab7da..e9f611b2091 100644 --- a/js/js.dart-ast/src/com/google/dart/compiler/backend/js/ast/metadata/metadataProperties.kt +++ b/js/js.dart-ast/src/com/google/dart/compiler/backend/js/ast/metadata/metadataProperties.kt @@ -111,7 +111,6 @@ enum class TypeCheck { TYPEOF, INSTANCEOF, OR_NULL, - IS_ANY, AND_PREDICATE } diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/rhino/RhinoUtils.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/rhino/RhinoUtils.java index c6626bbba0a..8d51c662057 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/rhino/RhinoUtils.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/rhino/RhinoUtils.java @@ -137,7 +137,7 @@ public final class RhinoUtils { } try { - ScriptableObject scope = getScope(ecmaVersion, context, jsLibraries); + Scriptable scope = getScope(ecmaVersion, context, jsLibraries); putGlobalVariablesIntoScope(scope, variables); context.evaluateString(scope, "kotlin.out = new kotlin.BufferedOutput();", "setup Kotlin.out", 0, null); @@ -158,10 +158,8 @@ public final class RhinoUtils { } @NotNull - private static ScriptableObject getScope(@NotNull EcmaVersion version, @NotNull Context context, @NotNull List jsLibraries) { - ScriptableObject scope = context.initStandardObjects(null, false); - scope.setParentScope(getParentScope(version, context, jsLibraries)); - return scope; + private static Scriptable getScope(@NotNull EcmaVersion version, @NotNull Context context, @NotNull List jsLibraries) { + return context.newObject(getParentScope(version, context, jsLibraries)); } @NotNull diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java index c90ca4a1803..a248d42c7fa 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/BoxJsTestGenerated.java @@ -1765,12 +1765,6 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest { doTest(fileName); } - @TestMetadata("noRuntimeTypeCheck.kt") - public void testNoRuntimeTypeCheck() throws Exception { - String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/expression/cast/noRuntimeTypeCheck.kt"); - doTest(fileName); - } - @TestMetadata("primitiveToClass.kt") public void testPrimitiveToClass() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/expression/cast/primitiveToClass.kt"); @@ -6974,6 +6968,12 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest { KotlinTestUtils.assertAllTestsPresentByMetadata(this.getClass(), new File("js/js.translator/testData/box/standardClasses"), Pattern.compile("^([^_](.+))\\.kt$"), TargetBackend.ANY, true); } + @TestMetadata("any.kt") + public void testAny() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/standardClasses/any.kt"); + doTest(fileName); + } + @TestMetadata("array.kt") public void testArray() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/standardClasses/array.kt"); diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java index 87c877b96ce..3cbb37e84f6 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/JsCodegenBoxTestGenerated.java @@ -2691,7 +2691,13 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest { @TestMetadata("lambdaToUnitCast.kt") public void testLambdaToUnitCast() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/casts/lambdaToUnitCast.kt"); - doTest(fileName); + try { + doTest(fileName); + } + catch (Throwable ignore) { + return; + } + throw new AssertionError("Looks like this test can be unmuted. Remove IGNORE_BACKEND directive for that."); } @TestMetadata("notIs.kt") diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/Namer.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/Namer.java index 9752bdde1d3..5eb5c854398 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/Namer.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/Namer.java @@ -318,11 +318,6 @@ public final class Namer { return invokeFunctionAndSetTypeCheckMetadata("andPredicate", Arrays.asList(a, b), TypeCheck.AND_PREDICATE); } - @NotNull - public JsExpression isAny() { - return invokeFunctionAndSetTypeCheckMetadata("isAny", Collections.emptyList(), TypeCheck.IS_ANY); - } - @NotNull public JsExpression isComparable() { return kotlin("isComparable"); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java index 88f2e2c3f93..a545931c61e 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/StaticContext.java @@ -279,6 +279,9 @@ public final class StaticContext { @NotNull public JsName getNameForDescriptor(@NotNull DeclarationDescriptor descriptor) { + if (descriptor instanceof ClassDescriptor && KotlinBuiltIns.isAny((ClassDescriptor) descriptor)) { + return rootScope.declareName("Object"); + } SuggestedName suggested = nameSuggestion.suggest(descriptor); if (suggested == null) { throw new IllegalArgumentException("Can't generate name for root declarations: " + descriptor); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/CatchTranslator.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/CatchTranslator.kt index fc679e270e3..91b625dda56 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/CatchTranslator.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/CatchTranslator.kt @@ -109,7 +109,7 @@ class CatchTranslator( // translateIsCheck won't ever return `null` if its second argument is `null` val typeCheck = with (patternTranslator(nextContext)) { - translateIsCheck(parameterRef, null, paramType) + translateIsCheck(parameterRef, paramType) }!! val elseBlock = translateCatches(nextContext, parameterDescriptor, parameterRef, catches) diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/PatternTranslator.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/PatternTranslator.java index 2866822474f..8acb5aa0ab3 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/PatternTranslator.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/PatternTranslator.java @@ -44,18 +44,14 @@ import org.jetbrains.kotlin.psi.KtIsExpression; import org.jetbrains.kotlin.psi.KtTypeReference; import org.jetbrains.kotlin.resolve.DescriptorUtils; import org.jetbrains.kotlin.resolve.descriptorUtil.DescriptorUtilsKt; -import org.jetbrains.kotlin.types.DynamicTypesKt; import org.jetbrains.kotlin.types.KotlinType; -import org.jetbrains.kotlin.types.typeUtil.TypeUtilsKt; import java.util.Collections; import static org.jetbrains.kotlin.builtins.FunctionTypesKt.isFunctionTypeOrSubtype; -import static org.jetbrains.kotlin.builtins.KotlinBuiltIns.isAnyOrNullableAny; import static org.jetbrains.kotlin.builtins.KotlinBuiltIns.isArray; import static org.jetbrains.kotlin.js.descriptorUtils.DescriptorUtilsKt.getNameIfStandardType; import static org.jetbrains.kotlin.js.translate.utils.BindingUtils.getTypeByReference; -import static org.jetbrains.kotlin.js.translate.utils.BindingUtils.getTypeForExpression; import static org.jetbrains.kotlin.js.translate.utils.JsAstUtils.equality; import static org.jetbrains.kotlin.js.translate.utils.JsAstUtils.not; import static org.jetbrains.kotlin.psi.KtPsiUtil.findChildByType; @@ -86,8 +82,7 @@ public final class PatternTranslator extends AbstractTranslator { KtTypeReference typeReference = expression.getRight(); assert typeReference != null: "Cast expression must have type reference"; - KotlinType sourceType = getTypeForExpression(bindingContext(), left); - JsExpression isCheck = translateIsCheck(temporary.assignmentExpression(), sourceType, typeReference); + JsExpression isCheck = translateIsCheck(temporary.assignmentExpression(), typeReference); if (isCheck == null) return expressionToCast; JsExpression onFail; @@ -108,8 +103,7 @@ public final class PatternTranslator extends AbstractTranslator { JsExpression left = Translation.translateAsExpression(expression.getLeftHandSide(), context()); KtTypeReference typeReference = expression.getTypeReference(); assert typeReference != null; - KotlinType sourceType = getTypeForExpression(bindingContext(), expression.getLeftHandSide()); - JsExpression result = translateIsCheck(left, sourceType, typeReference); + JsExpression result = translateIsCheck(left, typeReference); if (result == null) return JsLiteral.getBoolean(!expression.isNegated()); if (expression.isNegated()) { @@ -119,10 +113,8 @@ public final class PatternTranslator extends AbstractTranslator { } @Nullable - public JsExpression translateIsCheck(@NotNull JsExpression subject, @Nullable KotlinType sourceType, - @NotNull KtTypeReference targetTypeReference) { + public JsExpression translateIsCheck(@NotNull JsExpression subject, @NotNull KtTypeReference targetTypeReference) { KotlinType targetType = getTypeByReference(bindingContext(), targetTypeReference); - if (sourceType != null && !DynamicTypesKt.isDynamic(sourceType) && TypeUtilsKt.isSubtypeOf(sourceType, targetType)) return null; JsExpression checkFunReference = doGetIsTypeCheckCallable(targetType); if (checkFunReference == null) { @@ -191,8 +183,6 @@ public final class PatternTranslator extends AbstractTranslator { @Nullable private JsExpression getIsTypeCheckCallableForBuiltin(@NotNull KotlinType type) { - if (isAnyOrNullableAny(type)) return namer().isAny(); - if (isFunctionTypeOrSubtype(type) && !ReflectionTypes.isNumberedKPropertyOrKMutablePropertyType(type)) { return namer().isTypeOf(program().getStringLiteral("function")); } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/WhenTranslator.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/WhenTranslator.java index 22c1ced8ec7..b96c15c448c 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/WhenTranslator.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/WhenTranslator.java @@ -174,9 +174,7 @@ public final class WhenTranslator extends AbstractTranslator { KtExpression expressionToMatchNonTranslated = whenExpression.getSubjectExpression(); assert expressionToMatchNonTranslated != null : "expressionToMatch != null => expressionToMatchNonTranslated != null: " + PsiUtilsKt.getTextWithLocation(conditionIsPattern); - KotlinType expressionToMatchType = BindingUtils.getTypeForExpression(bindingContext(), expressionToMatchNonTranslated); - JsExpression result = Translation.patternTranslator(context).translateIsCheck(expressionToMatch, expressionToMatchType, - typeReference); + JsExpression result = Translation.patternTranslator(context).translateIsCheck(expressionToMatch, typeReference); return result != null ? result : JsLiteral.TRUE; } 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 3f75150aca2..f54bba1743f 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 @@ -71,11 +71,6 @@ private class TypeCheckRewritingVisitor(private val context: TranslationContext) private fun getReplacement(callee: JsInvocation, calleeArguments: List, argument: JsExpression): JsExpression? { val typeCheck = callee.typeCheck return when (typeCheck) { - TypeCheck.IS_ANY -> { - // `Kotlin.isAny()(argument)` -> `argument != null` - if (calleeArguments.isEmpty()) TranslationUtils.isNotNullCheck(argument) else null - } - TypeCheck.TYPEOF -> { // `Kotlin.isTypeOf(calleeArgument)(argument)` -> `typeOf argument === calleeArgument` if (calleeArguments.size == 1) typeOfIs(argument, calleeArguments[0] as JsStringLiteral) else null diff --git a/js/js.translator/testData/box/expression/cast/noRuntimeTypeCheck.kt b/js/js.translator/testData/box/expression/cast/noRuntimeTypeCheck.kt deleted file mode 100644 index d9b31bab488..00000000000 --- a/js/js.translator/testData/box/expression/cast/noRuntimeTypeCheck.kt +++ /dev/null @@ -1,28 +0,0 @@ -package foo - -// CHECK_NOT_CALLED_IN_SCOPE: scope=box function=isType - -open class A() - -class B() : A() - -fun box(): String { - assertTrue(B() is A) - assertTrue(B() is A?) - assertTrue(B() is B) - assertTrue(B() is B?) - assertTrue((B() as B?) is A?) - assertTrue((null as A?) is A?) - - assertNotEquals(null, B() as? A) - assertNotEquals(null, B() as? A?) - assertNotEquals(null, B() as? B) - assertNotEquals(null, B() as? B?) - - assertNotEquals(null, B() as A) - assertNotEquals(null, B() as A?) - assertNotEquals(null, B() as B) - assertNotEquals(null, B() as B?) - - return "OK" -} diff --git a/js/js.translator/testData/box/standardClasses/any.kt b/js/js.translator/testData/box/standardClasses/any.kt new file mode 100644 index 00000000000..2f6eb337d9c --- /dev/null +++ b/js/js.translator/testData/box/standardClasses/any.kt @@ -0,0 +1,32 @@ +// CHECK_CALLED_IN_SCOPE: function=isType scope=box +package foo + +class A : Any() + +fun Any?.asAny() = this + +fun box(): String { + val x = Any().asAny() + if (x !is Any) return "fail1" + if (x.asDynamic().constructor !== js("Object")) return "fail1a" + + if (A().asAny() !is Any) return "fail2" + + if (arrayOf(1, 2, 3).asAny() !is Any) return "fail3" + + if (createNakedObject() is Any) return "fail4" + + if (({ }).asAny() !is Any) return "fail5" + + if ((23).asAny() !is Any) return "fail6" + + if ((3.14).asAny() !is Any) return "fail7" + + if (false.asAny() !is Any) return "fail8" + + if ("bar".asAny() !is Any) return "fail9" + + return "OK" +} + +fun createNakedObject(): Any? = js("Object.create(null)") \ No newline at end of file diff --git a/js/js.translator/testData/kotlin_lib_ecma5.js b/js/js.translator/testData/kotlin_lib_ecma5.js index 392de3543d3..88da990f5b2 100644 --- a/js/js.translator/testData/kotlin_lib_ecma5.js +++ b/js/js.translator/testData/kotlin_lib_ecma5.js @@ -92,6 +92,18 @@ * @returns {Boolean} */ Kotlin.isType = function (object, klass) { + if (klass === Object) { + switch (typeof object) { + case "string": + case "number": + case "boolean": + case "function": + return true; + default: + return object instanceof Object; + } + } + if (object == null || klass == null || (typeof object !== 'object' && typeof object !== 'function')) { return false; } @@ -258,12 +270,6 @@ return object == null || fn(object); } }; - - Kotlin.isAny = function () { - return function (object) { - return object != null; - } - }; Kotlin.andPredicate = function (a, b) { return function (object) {