diff --git a/js/js.dart-ast/src/com/google/dart/compiler/backend/js/ast/JsNameRef.java b/js/js.dart-ast/src/com/google/dart/compiler/backend/js/ast/JsNameRef.java index 1a76477b268..26febe16140 100644 --- a/js/js.dart-ast/src/com/google/dart/compiler/backend/js/ast/JsNameRef.java +++ b/js/js.dart-ast/src/com/google/dart/compiler/backend/js/ast/JsNameRef.java @@ -102,7 +102,7 @@ public final class JsNameRef extends JsExpressionImpl implements HasName { public JsNameRef deepCopy() { JsExpression qualifierCopy = AstUtil.deepCopy(qualifier); - if (name != null) return new JsNameRef(name, qualifierCopy); + if (name != null) return new JsNameRef(name, qualifierCopy).withMetadataFrom(this); return new JsNameRef(ident, qualifierCopy).withMetadataFrom(this); } 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 c1cf52a2ecb..245123a99f8 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 @@ -39,6 +39,8 @@ var JsInvocation.typeCheck: TypeCheck? by MetadataProperty(default = null) var HasMetadata.synthetic: Boolean by MetadataProperty(default = false) +var HasMetadata.withoutSideEffects: Boolean by MetadataProperty(default = false) + enum class TypeCheck { TYPEOF, INSTANCEOF diff --git a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryVariableElimination.kt b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryVariableElimination.kt index 537b4bebf04..2b253b25d3f 100644 --- a/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryVariableElimination.kt +++ b/js/js.inliner/src/org/jetbrains/kotlin/js/inline/clean/TemporaryVariableElimination.kt @@ -17,7 +17,9 @@ package org.jetbrains.kotlin.js.inline.clean import com.google.dart.compiler.backend.js.ast.* +import com.google.dart.compiler.backend.js.ast.metadata.HasMetadata import com.google.dart.compiler.backend.js.ast.metadata.synthetic +import com.google.dart.compiler.backend.js.ast.metadata.withoutSideEffects import org.jetbrains.kotlin.js.inline.util.collectDefinedNames import org.jetbrains.kotlin.js.inline.util.collectFreeVariables import org.jetbrains.kotlin.js.translate.utils.JsAstUtils @@ -163,18 +165,24 @@ internal class TemporaryVariableElimination(private val root: JsStatement) { if (sideEffectOccurred) { lastProperlyUsedIndex = -1 } - if (index == nextExpectedIndex) { - ++nextExpectedIndex - } else { - lastProperlyUsedIndex = index - nextExpectedIndex = index + 1 + if (index == nextExpectedIndex) { + ++nextExpectedIndex + } + else { + lastProperlyUsedIndex = index + nextExpectedIndex = index + 1 + } } firstUsedIndex = Math.min(firstUsedIndex, index) } return } - super.visitNameRef(nameRef) + + if (nameRef.qualifier != null && !(nameRef is HasMetadata && nameRef.withoutSideEffects)) { + super.visitNameRef(nameRef) + sideEffectOccurred = true + } } override fun visitInvocation(invocation: JsInvocation) { @@ -204,10 +212,35 @@ internal class TemporaryVariableElimination(private val root: JsStatement) { } override fun visitBinaryExpression(x: JsBinaryOperation) { - super.visitBinaryExpression(x) if (x.operator == JsBinaryOperator.ASG) { + val left = x.arg1 + val right = x.arg2 + + if (left is JsNameRef) { + val qualifier = left.qualifier + if (qualifier != null) { + accept(qualifier) + } + } + else if (left is JsArrayAccess) { + accept(left.arrayExpression) + accept(left.indexExpression) + } + else { + accept(left) + } + + accept(right) sideEffectOccurred = true } + else { + super.visitBinaryExpression(x) + } + } + + override fun visitArrayAccess(x: JsArrayAccess) { + super.visitArrayAccess(x) + sideEffectOccurred = true } override fun visitArray(x: JsArrayLiteral) { @@ -368,7 +401,6 @@ internal class TemporaryVariableElimination(private val root: JsStatement) { firstUsedIndex = lastAssignedVars.size nextExpectedIndex = -1 sideEffectOccurred = false - accept(expression) if (lastProperlyUsedIndex >= 0 && nextExpectedIndex == lastAssignedVars.size) { diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/optimizer/TemporaryVariableEliminationTest.kt b/js/js.tests/test/org/jetbrains/kotlin/js/test/optimizer/TemporaryVariableEliminationTest.kt index a735bc935b8..9058d112efb 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/optimizer/TemporaryVariableEliminationTest.kt +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/optimizer/TemporaryVariableEliminationTest.kt @@ -32,4 +32,6 @@ class TemporaryVariableEliminationTest : BasicOptimizerTest("temporary-variable" @Test fun tryCatch() = box() @Test fun nonSideEffect() = box() + + @Test fun propertyAccess() = box() } \ No newline at end of file diff --git a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/InlineEvaluationOrderTestGenerated.java b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/InlineEvaluationOrderTestGenerated.java index e237786787f..2317682823d 100644 --- a/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/InlineEvaluationOrderTestGenerated.java +++ b/js/js.tests/test/org/jetbrains/kotlin/js/test/semantics/InlineEvaluationOrderTestGenerated.java @@ -245,6 +245,12 @@ public class InlineEvaluationOrderTestGenerated extends AbstractInlineEvaluation doTest(fileName); } + @TestMetadata("propertyAccessWithSideEffect.kt") + public void testPropertyAccessWithSideEffect() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/inlineEvaluationOrder/cases/propertyAccessWithSideEffect.kt"); + doTest(fileName); + } + @TestMetadata("ternaryConditional.kt") public void testTernaryConditional() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/inlineEvaluationOrder/cases/ternaryConditional.kt"); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/FunctionCallCases.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/FunctionCallCases.kt index 0ca4673bb4f..62d4003d3dc 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/FunctionCallCases.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/callTranslator/FunctionCallCases.kt @@ -17,6 +17,7 @@ package org.jetbrains.kotlin.js.translate.callTranslator import com.google.dart.compiler.backend.js.ast.* +import com.google.dart.compiler.backend.js.ast.metadata.withoutSideEffects import org.jetbrains.kotlin.descriptors.CallableDescriptor import org.jetbrains.kotlin.descriptors.ConstructorDescriptor import org.jetbrains.kotlin.descriptors.Visibilities @@ -80,7 +81,9 @@ object DefaultFunctionCallCase : FunctionCallCase() { val functionRef = context.aliasOrValue(callableDescriptor) { val qualifierForFunction = context.getQualifierForDescriptor(it) - JsNameRef(functionName, qualifierForFunction) + val result = JsNameRef(functionName, qualifierForFunction) + result.withoutSideEffects = true + result } return JsInvocation(functionRef, argumentsInfo.translateArguments) } 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 5522030c57c..b8c8e9481ef 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 @@ -18,6 +18,8 @@ package org.jetbrains.kotlin.js.translate.context; import com.google.common.collect.Maps; import com.google.dart.compiler.backend.js.ast.*; +import com.google.dart.compiler.backend.js.ast.metadata.HasMetadata; +import com.google.dart.compiler.backend.js.ast.metadata.MetadataProperties; import com.intellij.openapi.util.Factory; import com.intellij.util.containers.ContainerUtil; import org.jetbrains.annotations.NotNull; @@ -176,13 +178,15 @@ public final class StaticContext { return getQualifiedReference(((PackageFragmentDescriptor) descriptor).getFqName()); } - return new JsNameRef(getNameForDescriptor(descriptor), getQualifierForDescriptor(descriptor)); + JsNameRef result = new JsNameRef(getNameForDescriptor(descriptor), getQualifierForDescriptor(descriptor)); + applySideEffects(result, descriptor); + return result; } @NotNull public JsNameRef getQualifiedReference(@NotNull FqName packageFqName) { - return new JsNameRef(getNameForPackage(packageFqName), - packageFqName.isRoot() ? null : getQualifierForParentPackage(packageFqName.parent())); + JsName packageName = getNameForPackage(packageFqName); + return JsAstUtils.fqn(packageName, packageFqName.isRoot() ? null : getQualifierForParentPackage(packageFqName.parent())); } @NotNull @@ -211,7 +215,7 @@ public final class StaticContext { FqName fqName = packageFqName; while (true) { - JsNameRef ref = getNameForPackage(fqName).makeRef(); + JsNameRef ref = JsAstUtils.fqn(getNameForPackage(fqName), null); if (qualifier == null) { result = ref; @@ -565,7 +569,7 @@ public final class StaticContext { if (DescriptorUtils.isCompanionObject(container)) { result = Namer.getCompanionObjectAccessor(result); } - return result; + return applySideEffects(result, descriptor); } }; @@ -579,6 +583,18 @@ public final class StaticContext { } } + private static JsExpression applySideEffects(JsExpression expression, DeclarationDescriptor descriptor) { + if (expression instanceof HasMetadata) { + if (descriptor instanceof FunctionDescriptor || + descriptor instanceof PackageFragmentDescriptor || + descriptor instanceof ClassDescriptor + ) { + MetadataProperties.setSideEffects((HasMetadata) expression, false); + } + } + return expression; + } + private static class QualifierIsNullGenerator extends Generator { private QualifierIsNullGenerator() { diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java index 090fe9e23aa..8638d885f2c 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/context/TranslationContext.java @@ -17,6 +17,7 @@ package org.jetbrains.kotlin.js.translate.context; import com.google.dart.compiler.backend.js.ast.*; +import com.google.dart.compiler.backend.js.ast.metadata.MetadataProperties; import com.intellij.psi.PsiElement; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -227,7 +228,9 @@ public class TranslationContext { @NotNull public JsNameRef getQualifiedReference(@NotNull FqName packageFqName) { - return staticContext.getQualifiedReference(packageFqName); + JsNameRef result = staticContext.getQualifiedReference(packageFqName); + MetadataProperties.setWithoutSideEffects(result, true); + return result; } @Nullable diff --git a/js/js.translator/testData/inlineEvaluationOrder/cases/propertyAccessWithSideEffect.kt b/js/js.translator/testData/inlineEvaluationOrder/cases/propertyAccessWithSideEffect.kt new file mode 100644 index 00000000000..fe2df1291e2 --- /dev/null +++ b/js/js.translator/testData/inlineEvaluationOrder/cases/propertyAccessWithSideEffect.kt @@ -0,0 +1,39 @@ +package foo + +var g: Any? + get() { + log("g.get") + return null + } + set(v) { + log("g.set") + } + +public inline fun Array.boo() { + var a = g + for (element in this); +} + +public inline fun Iterable.boo(i: Any?) { + var a = i + for (element in this); +} + +fun test1(f: () -> Array) { + f().boo() +} + +fun test2(f: () -> Iterable) { + f().boo(g) +} + + +fun box(): String { + test1 { log("lambda1"); arrayOf() } + assertEquals("lambda1;g.get;", pullLog()) + + test2 { log("lambda2"); listOf() } + assertEquals("lambda2;g.get;", pullLog()) + + return "OK" +} \ No newline at end of file diff --git a/js/js.translator/testData/js-optimizer/temporary-variable/methodCall.optimized.js b/js/js.translator/testData/js-optimizer/temporary-variable/methodCall.optimized.js index bea2aa02aa0..b3163f78c69 100644 --- a/js/js.translator/testData/js-optimizer/temporary-variable/methodCall.optimized.js +++ b/js/js.translator/testData/js-optimizer/temporary-variable/methodCall.optimized.js @@ -52,6 +52,13 @@ function test5() { return $tmp1 + $tmp2 + $tmp3; } +function test6() { + init(); + + var $tmp = foo(1); + return foo(2) + $tmp; +} + function box() { var result = test1(); if (result != 3) return "fail1a: " + result; @@ -73,5 +80,9 @@ function box() { if (result != 6) return "fail5a: " + result; if (log != "{1}{2}{3}{4}") return "fail5b: " + log; + result = test6(); + if (result != 3) return "fail6a: " + result; + if (log != "{1}{2}") return "fail6b: " + result; + return "OK"; } \ No newline at end of file diff --git a/js/js.translator/testData/js-optimizer/temporary-variable/methodCall.original.js b/js/js.translator/testData/js-optimizer/temporary-variable/methodCall.original.js index 630d26f9fbe..1c0801f556a 100644 --- a/js/js.translator/testData/js-optimizer/temporary-variable/methodCall.original.js +++ b/js/js.translator/testData/js-optimizer/temporary-variable/methodCall.original.js @@ -59,6 +59,13 @@ function test5() { return $tmp1 + $tmp2 + $tmp3; } +function test6() { + init(); + + var $tmp = foo(1); + return foo(2) + $tmp; +} + function box() { var result = test1(); if (result != 3) return "fail1a: " + result; @@ -80,5 +87,9 @@ function box() { if (result != 6) return "fail5a: " + result; if (log != "{1}{2}{3}{4}") return "fail5b: " + log; + result = test6(); + if (result != 3) return "fail6a: " + result; + if (log != "{1}{2}") return "fail6b: " + result; + return "OK"; } \ No newline at end of file diff --git a/js/js.translator/testData/js-optimizer/temporary-variable/propertyAccess.optimized.js b/js/js.translator/testData/js-optimizer/temporary-variable/propertyAccess.optimized.js new file mode 100644 index 00000000000..715f4f39b17 --- /dev/null +++ b/js/js.translator/testData/js-optimizer/temporary-variable/propertyAccess.optimized.js @@ -0,0 +1,24 @@ +var log = ""; + +A = {}; +Object.defineProperty(A, "x", { + get: function() { + log += "A.x;"; + return 23; + } +}); + +function b() { + log += "b();"; + return 42; +} + +function box() { + var $tmp = A.x; + var result = b() + ";" + $tmp; + + if (result != "42;23") return "fail1: " + result; + if (log != "A.x;b();") return "fail2: " + log; + + return "OK"; +} \ No newline at end of file diff --git a/js/js.translator/testData/js-optimizer/temporary-variable/propertyAccess.original.js b/js/js.translator/testData/js-optimizer/temporary-variable/propertyAccess.original.js new file mode 100644 index 00000000000..715f4f39b17 --- /dev/null +++ b/js/js.translator/testData/js-optimizer/temporary-variable/propertyAccess.original.js @@ -0,0 +1,24 @@ +var log = ""; + +A = {}; +Object.defineProperty(A, "x", { + get: function() { + log += "A.x;"; + return 23; + } +}); + +function b() { + log += "b();"; + return 42; +} + +function box() { + var $tmp = A.x; + var result = b() + ";" + $tmp; + + if (result != "42;23") return "fail1: " + result; + if (log != "A.x;b();") return "fail2: " + log; + + return "OK"; +} \ No newline at end of file