From 1350e3c4acf20cb9711f10b301b1b499190c8c58 Mon Sep 17 00:00:00 2001 From: Alexey Andreev Date: Fri, 8 Sep 2017 14:29:21 +0300 Subject: [PATCH] JS: improve optimization of for loop over range literals See KT-18329, partial fix for KT-8372 --- .../js/test/semantics/BoxJsTestGenerated.java | 6 + .../js/translate/expression/LoopTranslator.kt | 102 ++++++++++++---- .../box/expression/for/rangeOptimization.kt | 110 ++++++++++++++++++ 3 files changed, 197 insertions(+), 21 deletions(-) create mode 100644 js/js.translator/testData/box/expression/for/rangeOptimization.kt 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 9adecf0ce3f..142cce15ecb 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 @@ -2319,6 +2319,12 @@ public class BoxJsTestGenerated extends AbstractBoxJsTest { String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/expression/for/labeledForWithWhile.kt"); doTest(fileName); } + + @TestMetadata("rangeOptimization.kt") + public void testRangeOptimization() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("js/js.translator/testData/box/expression/for/rangeOptimization.kt"); + doTest(fileName); + } } @TestMetadata("js/js.translator/testData/box/expression/function") diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/LoopTranslator.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/LoopTranslator.kt index fb774d83cef..27b7e6b5963 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/LoopTranslator.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/expression/LoopTranslator.kt @@ -29,14 +29,12 @@ import org.jetbrains.kotlin.js.translate.utils.BindingUtils.* import org.jetbrains.kotlin.js.translate.utils.JsAstUtils import org.jetbrains.kotlin.js.translate.utils.JsAstUtils.* import org.jetbrains.kotlin.js.translate.utils.PsiUtils.getLoopRange -import org.jetbrains.kotlin.js.translate.utils.TranslationUtils -import org.jetbrains.kotlin.lexer.KtTokens -import org.jetbrains.kotlin.psi.KtBinaryExpression -import org.jetbrains.kotlin.psi.KtDestructuringDeclaration -import org.jetbrains.kotlin.psi.KtForExpression -import org.jetbrains.kotlin.psi.KtWhileExpressionBase +import org.jetbrains.kotlin.name.FqName +import org.jetbrains.kotlin.psi.* +import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall import org.jetbrains.kotlin.resolve.calls.model.ResolvedCall import org.jetbrains.kotlin.resolve.descriptorUtil.fqNameSafe +import org.jetbrains.kotlin.resolve.scopes.receivers.ExpressionReceiver fun createWhile(doWhile: Boolean, expression: KtWhileExpressionBase, context: TranslationContext): JsNode { val conditionExpression = expression.condition ?: @@ -78,18 +76,47 @@ fun createWhile(doWhile: Boolean, expression: KtWhileExpressionBase, context: Tr return result.source(expression) } +private val rangeToFunctionName = FqName("kotlin.Int.rangeTo") +private val untilFunctionName = FqName("kotlin.ranges.until") +private val downToFunctionName = FqName("kotlin.ranges.downTo") +private val stepFunctionName = FqName("kotlin.ranges.step") +private val intRangeName = FqName("kotlin.ranges.IntRange") +private val intProgressionName = FqName("kotlin.ranges.IntProgression") + fun translateForExpression(expression: KtForExpression, context: TranslationContext): JsStatement { - val loopRange = getLoopRange(expression) + val loopRange = KtPsiUtil.deparenthesize(getLoopRange(expression))!! val rangeType = getTypeForExpression(context.bindingContext(), loopRange) fun isForOverRange(): Boolean { //TODO: long range? val fqn = rangeType.constructor.declarationDescriptor?.fqNameSafe ?: return false - return fqn.asString() == "kotlin.ranges.IntRange" + return fqn == intRangeName } - fun isForOverRangeLiteral(): Boolean = - loopRange is KtBinaryExpression && loopRange.operationToken == KtTokens.RANGE && isForOverRange() + fun extractForOverRangeLiteral(): RangeLiteral? { + val fqn = rangeType.constructor.declarationDescriptor?.fqNameSafe + if (fqn != intRangeName && fqn != intProgressionName) return null + + var resolvedCall = loopRange.getResolvedCall(context.bindingContext()) ?: return null + var step: KtExpression? = null + if (resolvedCall.resultingDescriptor.fqNameSafe == stepFunctionName) { + step = resolvedCall.call.valueArguments[0].getArgumentExpression() ?: return null + resolvedCall = (resolvedCall.extensionReceiver as? ExpressionReceiver)?.expression?.getResolvedCall(context.bindingContext()) ?: + return null + } + + val first = ((resolvedCall.extensionReceiver ?: resolvedCall.dispatchReceiver) as? ExpressionReceiver)?.expression ?: return null + val second = resolvedCall.valueArgumentsByIndex?.firstOrNull()?.arguments?.firstOrNull()?.getArgumentExpression() ?: return null + + val type = when (resolvedCall.resultingDescriptor.fqNameSafe) { + rangeToFunctionName -> RangeType.RANGE_TO + untilFunctionName -> RangeType.UNTIL + downToFunctionName -> RangeType.DOWN_TO + else -> return null + } + + return RangeLiteral(type, first, second, step) + } fun isForOverArray(): Boolean { return KotlinBuiltIns.isArray(rangeType) || KotlinBuiltIns.isPrimitiveArray(rangeType) @@ -133,22 +160,45 @@ fun translateForExpression(expression: KtForExpression, context: TranslationCont } } - // TODO: implement reverse semantics - fun translateForOverLiteralRange(): JsStatement { - if (loopRange !is KtBinaryExpression) throw IllegalStateException("expected JetBinaryExpression, but ${loopRange.text}") - + fun translateForOverLiteralRange(literal: RangeLiteral): JsStatement { val startBlock = JsBlock() - val leftExpression = TranslationUtils.translateLeftExpression(context, loopRange, startBlock) + val leftExpression = Translation.translateAsExpression(literal.first, context, startBlock) val endBlock = JsBlock() - val rightExpression = TranslationUtils.translateRightExpression(context, loopRange, endBlock) - val rangeStart = context.cacheExpressionIfNeeded(leftExpression) + val rightExpression = Translation.translateAsExpression(literal.second, context, endBlock) + val stepBlock = JsBlock() + val stepExpression = literal.step?.let { Translation.translateAsExpression(it, context, stepBlock) } + context.addStatementsToCurrentBlockFrom(startBlock) + val rangeStart = context.cacheExpressionIfNeeded(leftExpression) context.addStatementsToCurrentBlockFrom(endBlock) val rangeEnd = context.defineTemporary(rightExpression) + context.addStatementsToCurrentBlockFrom(stepBlock) + val step = stepExpression?.let { context.defineTemporary(it) } val body = translateBody(null) - val conditionExpression = lessThanEq(parameterName.makeRef(), rangeEnd).source(expression) - val incrementExpression = JsPostfixOperation(JsUnaryOperator.INC, parameterName.makeRef()).source(expression) + val conditionExpression = when (literal.type) { + RangeType.RANGE_TO -> lessThanEq(parameterName.makeRef(), rangeEnd) + RangeType.UNTIL -> lessThan(parameterName.makeRef(), rangeEnd) + RangeType.DOWN_TO -> greaterThanEq(parameterName.makeRef(), rangeEnd) + }.source(expression) + + val incrementExpression = if (step == null) { + val incrementOperator = when (literal.type) { + RangeType.RANGE_TO, + RangeType.UNTIL -> JsUnaryOperator.INC + RangeType.DOWN_TO -> JsUnaryOperator.DEC + } + JsPostfixOperation(incrementOperator, parameterName.makeRef()).source(expression) + } + else { + val incrementOperator = when (literal.type) { + RangeType.RANGE_TO, + RangeType.UNTIL -> JsBinaryOperator.ASG_ADD + RangeType.DOWN_TO -> JsBinaryOperator.ASG_SUB + } + JsBinaryOperation(incrementOperator, parameterName.makeRef(), step).source(expression) + } + val initVars = newVar(parameterName, rangeStart).apply { source = expression } return JsFor(initVars, conditionExpression, incrementExpression, body) @@ -228,9 +278,11 @@ fun translateForExpression(expression: KtForExpression, context: TranslationCont return JsWhile(exitCondition, bodyStatements.singleOrNull() ?: JsBlock(bodyStatements)) } + val rangeLiteral = extractForOverRangeLiteral() + val result = when { - isForOverRangeLiteral() -> - translateForOverLiteralRange() + rangeLiteral != null -> + translateForOverLiteralRange(rangeLiteral) isForOverRange() -> translateForOverRange() @@ -244,3 +296,11 @@ fun translateForExpression(expression: KtForExpression, context: TranslationCont return result.apply { source = expression } } + +private enum class RangeType { + RANGE_TO, + UNTIL, + DOWN_TO +} + +private class RangeLiteral(val type: RangeType, val first: KtExpression, val second: KtExpression, var step: KtExpression?) diff --git a/js/js.translator/testData/box/expression/for/rangeOptimization.kt b/js/js.translator/testData/box/expression/for/rangeOptimization.kt new file mode 100644 index 00000000000..b99312aee0a --- /dev/null +++ b/js/js.translator/testData/box/expression/for/rangeOptimization.kt @@ -0,0 +1,110 @@ +// EXPECTED_REACHABLE_NODES: 1006 +// CHECK_CONTAINS_NO_CALLS: testRangeTo +// CHECK_CONTAINS_NO_CALLS: testRangeToFunction +// CHECK_CONTAINS_NO_CALLS: testUntil +// CHECK_CONTAINS_NO_CALLS: testDownTo +// CHECK_CONTAINS_NO_CALLS: testStep +// CHECK_CONTAINS_NO_CALLS: testEmptyRange +// CHECK_CONTAINS_NO_CALLS: testRangeToParams except=from;to + +fun testRangeTo(): String { + var result = "" + for (x in 1..3) { + result += x + } + return result +} + +fun testRangeToFunction(): String { + var result = "" + for (x in 1.rangeTo(3)) { + result += x + } + return result +} + +fun testUntil(): String { + var result = "" + for (x in 1 until 4) { + result += x + } + return result +} + +fun testDownTo(): String { + var result = "" + for (x in 3 downTo 1) { + result += x + } + return result +} + +fun testStep(): String { + var result = "" + for (x in 1..5 step 2) { + result += x + } + result += ";" + + for (x in 1 until 6 step 2) { + result += x + } + result += ";" + + for (x in 6.downTo(1).step(2)) { + result += x + } + + return result +} + +fun testEmptyRange(): String { + var result = "" + for (x in 3..1) { + result += x + } + return result +} + +fun testRangeToParams(from: () -> Int, to: () -> Int): String { + var result = "" + for (x in (from()..to())) { + result += x + } + return result +} + +fun box(): String { + var r = testRangeTo() + if (r != "123") return "fail: rangeTo: $r" + + r = testRangeToFunction() + if (r != "123") return "fail: rangeToFunction: $r" + + r = testUntil() + if (r != "123") return "fail: until: $r" + + r = testDownTo() + if (r != "321") return "fail: downTo: $r" + + r = testStep() + if (r != "135;135;642") return "fail: $r" + + r = testEmptyRange() + if (r != "") return "fail: emptyRange: $r" + + r = testRangeToParams({ 1 }, { 3 }) + if (r != "123") return "fail: rangeTo(1, 3): $r" + + var se = "" + r = testRangeToParams({ se += "Q"; 1 }, { se += "W"; 3 }) + if (r != "123") return "fail: rangeTo(1, 3) with side effects: $r" + if (se != "QW") return "fail: rangeTo(1, 3): side effects: $se" + + se = "" + r = testRangeToParams({ se += "Q"; 3 }, { se += "W"; 1 }) + if (r != "") return "fail: rangeTo(1, 3) with side effects: $r" + if (se != "QW") return "fail: rangeTo(1, 3): side effects: $se" + + return "OK" +} \ No newline at end of file