Skip to content

Commit 520eade

Browse files
kunyavskiySpace Team
authored andcommitted
[K/JVM] Fix incorrect optimization of local delegation
RCA: the bug was introduced in 1ecb034 during refactoring of using new reference nodes. In case where a local variable is delegated to a property with an untrivial bound expression, this expression is started to compute on each access of variable, instead of computing once on variable initialization. This commit fixes this behavior back to correct one by storing this expression to a local variable. Regression test is added to also cover local delegation case (tests for regular properties already existed). ^KT-84620 Fixed (cherry picked from commit d7464e2)
1 parent 7ef8bcc commit 520eade

4 files changed

Lines changed: 74 additions & 16 deletions

File tree

analysis/low-level-api-fir/tests-gen/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLBlackBoxTestGenerated.java

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

analysis/low-level-api-fir/tests-gen/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLReversedBlackBoxTestGenerated.java

Lines changed: 6 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/PropertyReferenceDelegationLowering.kt

Lines changed: 38 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import org.jetbrains.kotlin.ir.builders.IrBlockBuilder
1919
import org.jetbrains.kotlin.ir.builders.IrBuilder
2020
import org.jetbrains.kotlin.ir.builders.createTmpVariable
2121
import org.jetbrains.kotlin.ir.builders.declarations.buildField
22+
import org.jetbrains.kotlin.ir.builders.declarations.buildVariable
2223
import org.jetbrains.kotlin.ir.builders.irBlock
2324
import org.jetbrains.kotlin.ir.builders.irExprBody
2425
import org.jetbrains.kotlin.ir.builders.irGet
@@ -101,8 +102,7 @@ private class PropertyReferenceDelegationTransformer(val context: JvmBackendCont
101102
fun DeclarationIrBuilder.createGetterBody(
102103
getter: IrSimpleFunction,
103104
delegateReference: IrRichPropertyReference,
104-
receiver: IrExpression?,
105-
backingField: IrField?,
105+
receiverProvider: IrBuilder.() -> IrExpression?,
106106
): IrBody {
107107
val constInitializer = delegateReference.constInitializer
108108
return if (constInitializer != null) {
@@ -112,7 +112,7 @@ private class PropertyReferenceDelegationTransformer(val context: JvmBackendCont
112112
irExprBody(irBlock {
113113
+delegateReference.getterFunction.inline(
114114
getter,
115-
createAccessorArgumentsList(getter, delegateReference.getterFunction, isGetter = true, receiver, backingField)
115+
createAccessorArgumentsList(getter, delegateReference.getterFunction, isGetter = true, receiverProvider)
116116
)
117117
})
118118
}
@@ -121,14 +121,13 @@ private class PropertyReferenceDelegationTransformer(val context: JvmBackendCont
121121
fun DeclarationIrBuilder.createSetterBody(
122122
setter: IrSimpleFunction,
123123
delegateReference: IrRichPropertyReference,
124-
receiver: IrExpression?,
125-
backingField: IrField?,
124+
receiverProvider: IrBuilder.() -> IrExpression?,
126125
): IrBody {
127126
val delegateSetter = delegateReference.setterFunction ?: error("delegate was expected to have a setter")
128127
return irExprBody(irBlock {
129128
+delegateSetter.inline(
130129
setter,
131-
createAccessorArgumentsList(setter, delegateSetter, isGetter = false, receiver, backingField)
130+
createAccessorArgumentsList(setter, delegateSetter, isGetter = false, receiverProvider)
132131
)
133132
})
134133
}
@@ -140,10 +139,9 @@ private class PropertyReferenceDelegationTransformer(val context: JvmBackendCont
140139
accessor: IrSimpleFunction,
141140
delegateAccessor: IrSimpleFunction,
142141
isGetter: Boolean,
143-
remappedReceiver: IrExpression?,
144-
backingField: IrField?,
142+
receiverProvider: IrBuilder.() -> IrExpression?,
145143
): List<IrValueDeclaration> {
146-
val boundReceiverOrNull = createBoundReceiverExpr(accessor, backingField, remappedReceiver)
144+
val boundReceiverOrNull = receiverProvider()
147145
val setterParam = if (isGetter) null else accessor.parameters.lastOrNull() ?: error("setter must have at least one parameter")
148146
return buildList {
149147
if (boundReceiverOrNull != null) add(createTmpVariable(boundReceiverOrNull.deepCopyWithSymbols(accessor)))
@@ -184,12 +182,24 @@ private class PropertyReferenceDelegationTransformer(val context: JvmBackendCont
184182

185183
getter?.apply {
186184
body = with(context.createIrBuilder(symbol, startOffset, endOffset)) {
187-
createGetterBody(this@apply, delegate, remapReceiverIfNeeded(this@apply), backingField)
185+
createGetterBody(
186+
getter = this@apply,
187+
delegateReference = delegate,
188+
receiverProvider = {
189+
createBoundReceiverExpr(this@apply, backingField, remapReceiverIfNeeded(this@apply))
190+
}
191+
)
188192
}
189193
}
190194
setter?.apply {
191195
body = with(context.createIrBuilder(symbol, startOffset, endOffset)) {
192-
createSetterBody(this@apply, delegate, remapReceiverIfNeeded(this@apply), backingField)
196+
createSetterBody(
197+
setter = this@apply,
198+
delegateReference = delegate,
199+
receiverProvider = {
200+
createBoundReceiverExpr(this@apply, backingField, remapReceiverIfNeeded(this@apply))
201+
}
202+
)
193203
}
194204
}
195205

@@ -234,18 +244,30 @@ private class PropertyReferenceDelegationTransformer(val context: JvmBackendCont
234244
!declaration.getter.returnsResultOfStdlibCall ||
235245
declaration.setter?.returnsResultOfStdlibCall == false
236246
) return super.visitLocalDelegatedProperty(declaration)
237-
// Variables are cheap, so optimizing them out is not really necessary.
238-
val receiver = delegateInitializer.singleBoundValueOrNull?.transform(this@PropertyReferenceDelegationTransformer, null)
239-
// TODO: just like in `PropertyReferenceLowering`, probably better to inline the getter/setter rather than
247+
val receiver = delegateInitializer.singleBoundValueOrNull?.let { receiver ->
248+
with(delegate) {
249+
buildVariable(parent, startOffset, endOffset, origin, name, receiver.type)
250+
}.apply {
251+
initializer = receiver.transform(this@PropertyReferenceDelegationTransformer, null)
252+
}
253+
} // TODO: just like in `PropertyReferenceLowering`, probably better to inline the getter/setter rather than
240254
// generate them as local functions.
241255
val getter = declaration.getter.apply {
242256
with(context.createIrBuilder(symbol, startOffset, endOffset)) {
243-
body = createGetterBody(this@apply, delegateInitializer, receiver, backingField = null)
257+
body = createGetterBody(
258+
getter = this@apply,
259+
delegateReference = delegateInitializer,
260+
receiverProvider = { receiver?.let { irGet(it) } }
261+
)
244262
}
245263
}
246264
val setter = declaration.setter?.apply {
247265
with(context.createIrBuilder(symbol, startOffset, endOffset)) {
248-
body = createSetterBody(this@apply, delegateInitializer, receiver, backingField = null)
266+
body = createSetterBody(
267+
setter = this@apply,
268+
delegateReference = delegateInitializer,
269+
receiverProvider = { receiver?.let { irGet(it) } }
270+
)
249271
}
250272
}
251273
val statements = listOfNotNull(receiver, getter, setter)
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
// WITH_STDLIB
2+
3+
var created: Int = 0
4+
5+
class A(val x: Int) {
6+
init { created += 1 }
7+
}
8+
9+
class B(var x: Int) {
10+
init { created += 1000 }
11+
}
12+
13+
fun create() = A(1)
14+
15+
fun box(): String {
16+
val t1 by A(1)::x
17+
var t2 by B(1)::x
18+
if (t1 != 1) return "FAIL 1: $t1"
19+
if (t2 != 1) return "FAIL 2: $t2"
20+
t2 = 3
21+
if (t2 != 3) return "FAIL 3: $t2"
22+
if (created != 1001) return "FAIL 4: $created != 1001"
23+
return "OK"
24+
}

0 commit comments

Comments
 (0)