Generate for-in-array loops with proper semantics in 1.3+

In Kotlin 1.3+, assignment to the for-in-array loop range variable in
the loop body doesn't affect loop execution (as if it was a loop on an
array iterator, or some other container).

 #KT-21354 In Progress
 #KT-21321 In Progress
This commit is contained in:
Dmitry Petrov
2017-12-04 15:20:19 +03:00
parent 7a6f80606b
commit e4ecc13e13
9 changed files with 74 additions and 23 deletions
@@ -23,9 +23,12 @@ import org.jetbrains.kotlin.codegen.range.inExpression.InExpressionGenerator
import org.jetbrains.kotlin.psi.KtForExpression
import org.jetbrains.kotlin.psi.KtSimpleNameExpression
class ArrayRangeValue(private val canCacheArrayLength: Boolean) : RangeValue {
class ArrayRangeValue(
private val canCacheArrayLength: Boolean,
private val shouldAlwaysStoreArrayInNewVar: Boolean
) : RangeValue {
override fun createForLoopGenerator(codegen: ExpressionCodegen, forExpression: KtForExpression) =
ForInArrayLoopGenerator(codegen, forExpression, canCacheArrayLength)
ForInArrayLoopGenerator(codegen, forExpression, canCacheArrayLength, shouldAlwaysStoreArrayInNewVar)
override fun createInExpressionGenerator(codegen: ExpressionCodegen, operatorReference: KtSimpleNameExpression): InExpressionGenerator =
CallBasedInExpressionGenerator(codegen, operatorReference)
@@ -18,8 +18,10 @@ package org.jetbrains.kotlin.codegen.range
import org.jetbrains.kotlin.builtins.KotlinBuiltIns
import org.jetbrains.kotlin.codegen.*
import org.jetbrains.kotlin.config.LanguageFeature
import org.jetbrains.kotlin.descriptors.CallableDescriptor
import org.jetbrains.kotlin.descriptors.impl.LocalVariableDescriptor
import org.jetbrains.kotlin.descriptors.impl.SyntheticFieldDescriptor
import org.jetbrains.kotlin.name.Name
import org.jetbrains.kotlin.psi.*
import org.jetbrains.kotlin.resolve.BindingContext
@@ -46,8 +48,15 @@ fun ExpressionCodegen.createRangeValueForExpression(rangeExpression: KtExpressio
val builtIns = state.module.builtIns
return when {
asmRangeType.sort == Type.ARRAY ->
ArrayRangeValue(!isLocalVarReference(rangeExpression, bindingContext))
asmRangeType.sort == Type.ARRAY -> {
val properForInArraySemantics =
state.languageVersionSettings.supportsFeature(LanguageFeature.ProperForInArrayLoopRangeVariableAssignmentSemantic)
ArrayRangeValue(
properForInArraySemantics || !isLocalVarReference(rangeExpression, bindingContext),
properForInArraySemantics
)
}
isPrimitiveRange(rangeType) ->
PrimitiveRangeRangeValue()
isPrimitiveProgression(rangeType) ->
@@ -64,7 +73,10 @@ fun ExpressionCodegen.createRangeValueForExpression(rangeExpression: KtExpressio
fun isLocalVarReference(rangeExpression: KtExpression, bindingContext: BindingContext): Boolean {
if (rangeExpression !is KtSimpleNameExpression) return false
val resultingDescriptor = rangeExpression.getResolvedCall(bindingContext)?.resultingDescriptor ?: return false
return resultingDescriptor is LocalVariableDescriptor && resultingDescriptor.isVar
return resultingDescriptor is LocalVariableDescriptor &&
resultingDescriptor !is SyntheticFieldDescriptor &&
!resultingDescriptor.isDelegated &&
resultingDescriptor.isVar
}
private fun isSubtypeOfString(type: KotlinType, builtIns: KotlinBuiltIns) =
@@ -29,9 +29,8 @@ import org.jetbrains.org.objectweb.asm.Type
class ForInArrayLoopGenerator(
codegen: ExpressionCodegen,
forExpression: KtForExpression,
// We can cache array length if the corresponding range expression is not a local var.
// See https://youtrack.jetbrains.com/issue/KT-21354.
private val canCacheArrayLength: Boolean
private val canCacheArrayLength: Boolean,
private val shouldAlwaysStoreArrayInNewVar: Boolean
) : AbstractForLoopGenerator(codegen, forExpression) {
private var indexVar: Int = 0
private var arrayVar: Int = 0
@@ -46,7 +45,7 @@ class ForInArrayLoopGenerator(
val loopRange = forExpression.loopRange
val value = codegen.gen(loopRange)
val asmLoopRangeType = codegen.asmType(loopRangeType)
if (value is StackValue.Local && value.type == asmLoopRangeType) {
if (!shouldAlwaysStoreArrayInNewVar && value is StackValue.Local && value.type == asmLoopRangeType) {
arrayVar = value.index // no need to copy local variable into another variable
}
else {