From 4d7906fe44235fc37342f6b2cc988cb1d90f6860 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Fri, 18 Mar 2016 14:11:30 +0300 Subject: [PATCH] Prevent temporary variable from substituting to branches of ternary conditional expression, since it can sometimes violate evaluation order --- .../clean/TemporaryVariableElimination.kt | 20 +++++++++++++++++++ .../InlineEvaluationOrderTestGenerated.java | 6 ++++++ .../cases/ternaryConditional.kt | 20 +++++++++++++++++++ 3 files changed, 46 insertions(+) create mode 100644 js/js.translator/testData/inlineEvaluationOrder/cases/ternaryConditional.kt 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 cedb951a5eb..521bbdfeed3 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 @@ -64,6 +64,16 @@ internal class TemporaryVariableElimination(private val root: JsStatement) { return super.visit(x, ctx) } + override fun visit(x: JsIf, ctx: JsContext<*>): Boolean { + accept(x.ifExpression) + lastAssignedVars.clear() + + accept(x.thenStatement) + x.elseStatement?.let { accept(it) } + + return false + } + override fun visit(x: JsInvocation, ctx: JsContext<*>): Boolean { x.arguments.asReversed().forEach { accept(it) } accept(x.qualifier) @@ -129,6 +139,16 @@ internal class TemporaryVariableElimination(private val root: JsStatement) { x.collectFreeVariables().forEach { useVariable(it); useVariable(it) } return super.visit(x, ctx) } + + override fun visit(x: JsConditional, ctx: JsContext<*>): Boolean { + accept(x.testExpression) + lastAssignedVars.clear() + + accept(x.thenExpression) + accept(x.elseExpression) + + return false + } }.accept(root) } 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 a60a139498c..e237786787f 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("ternaryConditional.kt") + public void testTernaryConditional() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/inlineEvaluationOrder/cases/ternaryConditional.kt"); + doTest(fileName); + } + @TestMetadata("while.kt") public void testWhile() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/inlineEvaluationOrder/cases/while.kt"); diff --git a/js/js.translator/testData/inlineEvaluationOrder/cases/ternaryConditional.kt b/js/js.translator/testData/inlineEvaluationOrder/cases/ternaryConditional.kt new file mode 100644 index 00000000000..6e61fadb01a --- /dev/null +++ b/js/js.translator/testData/inlineEvaluationOrder/cases/ternaryConditional.kt @@ -0,0 +1,20 @@ +package foo + +// CHECK_NOT_CALLED: bar_vux9f0$ + +inline fun bar(n: Int, x: Int) = if (n <= 5) x else 10 + +fun test(n: Int): Int = bar(n, fizz(n)) + +fun box(): String { + var result = test(4) + if (result != 4) return "fail1: $result" + + result = test(8) + if (result != 10) return "fail2: $result" + + var log = pullLog() + if (log != "fizz(4);fizz(8);") return "fail_log: $log" + + return "OK" +} \ No newline at end of file