Don't relocate temporary variable which receives property access, unless it's provable that this property don't have side effects

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);
     }

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

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
+                            if (index == nextExpectedIndex) {
-                            nextExpectedIndex = index + 1
+                                ++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) {

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()
 }

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");

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)
     }

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),
+        JsName packageName = getNameForPackage(packageFqName);
-                             packageFqName.isRoot() ? null : getQualifierForParentPackage(packageFqName.parent()));
+        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<Boolean> {
 
         private QualifierIsNullGenerator() {

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

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<String>.boo() {
+    var a = g
+    for (element in this);
+}
+
+public inline fun Iterable<String>.boo(i: Any?) {
+    var a = i
+    for (element in this);
+}
+
+fun test1(f: () -> Array<String>) {
+    f().boo()
+}
+
+fun test2(f: () -> Iterable<String>) {
+    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"
+}

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";
 }

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";
 }

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";
+}

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";
+}