From ff653241724f66595f7ea9ab7b897aa568123fd7 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Tue, 9 May 2023 15:12:23 +0200 Subject: [PATCH] JVM: use a safer version of fileParent for Java declarations Java declarations do not have a file parent, their outer element is IrExternalPackageFragment. The test did not fail in K1 only by accident, because there's an extra IrTypeOperatorCall on the property reference's dispatch receiver, which automatically makes it non-inlinable. In K2, it's IrGetField, so we were trying to compute if its dispatch receiver is inlinable and for that we tried to get its fileParent, which resulted in exception if it comes from Java. Also refactor PropertyReferenceDelegationTransformer.canInline a bit. #KT-57955 Fixed --- ...LightTreeBlackBoxCodegenTestGenerated.java | 6 ++++ .../FirPsiBlackBoxCodegenTestGenerated.java | 6 ++++ .../PropertyReferenceDelegationLowering.kt | 36 +++++++++++++------ .../kotlin/backend/jvm/ir/JvmIrUtils.kt | 13 ++++--- .../delegateToAnother/kt57955.kt | 16 +++++++++ .../codegen/BlackBoxCodegenTestGenerated.java | 6 ++++ .../IrBlackBoxCodegenTestGenerated.java | 6 ++++ ...kBoxCodegenWithIrInlinerTestGenerated.java | 6 ++++ .../LightAnalysisModeTestGenerated.java | 5 +++ 9 files changed, 84 insertions(+), 16 deletions(-) create mode 100644 compiler/testData/codegen/box/delegatedProperty/delegateToAnother/kt57955.kt diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirLightTreeBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirLightTreeBlackBoxCodegenTestGenerated.java index 313971e3eaf..b4249e61f89 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirLightTreeBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirLightTreeBlackBoxCodegenTestGenerated.java @@ -15142,6 +15142,12 @@ public class FirLightTreeBlackBoxCodegenTestGenerated extends AbstractFirLightTr runTest("compiler/testData/codegen/box/delegatedProperty/delegateToAnother/kt50019_noOptimizedCallableReferences.kt"); } + @Test + @TestMetadata("kt57955.kt") + public void testKt57955() throws Exception { + runTest("compiler/testData/codegen/box/delegatedProperty/delegateToAnother/kt57955.kt"); + } + @Test @TestMetadata("mutable.kt") public void testMutable() throws Exception { diff --git a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirPsiBlackBoxCodegenTestGenerated.java b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirPsiBlackBoxCodegenTestGenerated.java index 67bdcc9df28..d4e83b745d9 100644 --- a/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirPsiBlackBoxCodegenTestGenerated.java +++ b/compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirPsiBlackBoxCodegenTestGenerated.java @@ -15142,6 +15142,12 @@ public class FirPsiBlackBoxCodegenTestGenerated extends AbstractFirPsiBlackBoxCo runTest("compiler/testData/codegen/box/delegatedProperty/delegateToAnother/kt50019_noOptimizedCallableReferences.kt"); } + @Test + @TestMetadata("kt57955.kt") + public void testKt57955() throws Exception { + runTest("compiler/testData/codegen/box/delegatedProperty/delegateToAnother/kt57955.kt"); + } + @Test @TestMetadata("mutable.kt") public void testMutable() throws Exception { diff --git a/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/PropertyReferenceDelegationLowering.kt b/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/PropertyReferenceDelegationLowering.kt index 03ebb2bf2a2..c4b7bfd978d 100644 --- a/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/PropertyReferenceDelegationLowering.kt +++ b/compiler/ir/backend.jvm/lower/src/org/jetbrains/kotlin/backend/jvm/lower/PropertyReferenceDelegationLowering.kt @@ -11,7 +11,7 @@ import org.jetbrains.kotlin.backend.common.lower.parents import org.jetbrains.kotlin.backend.common.phaser.makeIrFilePhase import org.jetbrains.kotlin.backend.jvm.JvmBackendContext import org.jetbrains.kotlin.backend.jvm.ir.createJvmIrBuilder -import org.jetbrains.kotlin.backend.jvm.ir.fileParent +import org.jetbrains.kotlin.backend.jvm.ir.fileParentOrNull import org.jetbrains.kotlin.backend.jvm.lower.JvmPropertiesLowering.Companion.createSyntheticMethodForPropertyDelegate import org.jetbrains.kotlin.builtins.StandardNames import org.jetbrains.kotlin.descriptors.Modality @@ -21,7 +21,8 @@ import org.jetbrains.kotlin.ir.builders.declarations.buildField import org.jetbrains.kotlin.ir.builders.declarations.buildVariable import org.jetbrains.kotlin.ir.declarations.* import org.jetbrains.kotlin.ir.expressions.* -import org.jetbrains.kotlin.ir.expressions.impl.* +import org.jetbrains.kotlin.ir.expressions.impl.IrCompositeImpl +import org.jetbrains.kotlin.ir.expressions.impl.IrPropertyReferenceImpl import org.jetbrains.kotlin.ir.symbols.impl.IrAnonymousInitializerSymbolImpl import org.jetbrains.kotlin.ir.util.* import org.jetbrains.kotlin.ir.visitors.IrElementTransformerVoid @@ -89,15 +90,28 @@ private class PropertyReferenceDelegationTransformer(val context: JvmBackendCont // Some receivers don't need to be stored in fields and can be reevaluated every time an accessor is called: private fun IrExpression.canInline(visibleScopes: Set): Boolean = when (this) { - // Reads of immutable variables are stable, but value parameters of the constructor are not in scope: - is IrGetValue -> symbol.owner.let { !(it is IrVariable && it.isVar) && it.parent in visibleScopes } - // Reads of final fields of stable values are stable, but fields in other files can become non-final: - is IrGetField -> symbol.owner.let { it.isFinal && it.fileParent in visibleScopes } && receiver?.canInline(visibleScopes) != false - // Same applies to reads of properties with default getters, but non-final properties may be overridden by `var`s: - is IrCall -> symbol.owner.let { it.isFinalDefaultValGetter && it.fileParent in visibleScopes } && - dispatchReceiver?.canInline(visibleScopes) != false && extensionReceiver?.canInline(visibleScopes) != false - // Constants and singleton object accesses are always stable: - else -> isTrivial() + is IrGetValue -> { + // Reads of immutable variables are stable, but value parameters of the constructor are not in scope: + val value = symbol.owner + !(value is IrVariable && value.isVar) && value.parent in visibleScopes + } + is IrGetField -> { + // Reads of final fields of stable values are stable, but fields in other files can become non-final: + val field = symbol.owner + field.isFinal && field.fileParentOrNull.let { it != null && it in visibleScopes } + && receiver?.canInline(visibleScopes) != false + } + is IrCall -> { + // Same applies to reads of properties with default getters, but non-final properties may be overridden by `var`s: + val callee = symbol.owner + callee.isFinalDefaultValGetter && callee.fileParentOrNull.let { it != null && it in visibleScopes } + && dispatchReceiver?.canInline(visibleScopes) != false + && extensionReceiver?.canInline(visibleScopes) != false + } + else -> { + // Constants and singleton object accesses are always stable: + isTrivial() + } } private val IrSimpleFunction.isFinalDefaultValGetter: Boolean diff --git a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/ir/JvmIrUtils.kt b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/ir/JvmIrUtils.kt index 1ae50ce691f..d2f27f8adf6 100644 --- a/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/ir/JvmIrUtils.kt +++ b/compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/ir/JvmIrUtils.kt @@ -441,11 +441,14 @@ fun IrMemberAccessExpression<*>.getBooleanConstArgument(i: Int): Boolean = } ?: throw AssertionError("Value argument #$i should be a Boolean const: ${dump()}") val IrDeclaration.fileParent: IrFile - get() { - return when (val myParent = parent) { - is IrFile -> myParent - else -> (myParent as IrDeclaration).fileParent - } + get() = fileParentOrNull ?: error("No file parent: $this") + +@Suppress("RecursivePropertyAccessor") +val IrDeclaration.fileParentOrNull: IrFile? + get() = when (val myParent = parent) { + is IrFile -> myParent + is IrDeclaration -> myParent.fileParentOrNull + else -> null } private val RETENTION_PARAMETER_NAME = Name.identifier("value") diff --git a/compiler/testData/codegen/box/delegatedProperty/delegateToAnother/kt57955.kt b/compiler/testData/codegen/box/delegatedProperty/delegateToAnother/kt57955.kt new file mode 100644 index 00000000000..932a386434e --- /dev/null +++ b/compiler/testData/codegen/box/delegatedProperty/delegateToAnother/kt57955.kt @@ -0,0 +1,16 @@ +// TARGET_BACKEND: JVM +// WITH_STDLIB +// FILE: box.kt + +class C { + val x: Int? by J().s::length +} + +fun box(): String = + if (C().x == 0) "OK" else "Fail" + +// FILE: J.java + +public class J { + public final String s = ""; +} diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java index 6413edb7e0b..79e35333019 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java @@ -14806,6 +14806,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest { runTest("compiler/testData/codegen/box/delegatedProperty/delegateToAnother/kt50019_noOptimizedCallableReferences.kt"); } + @Test + @TestMetadata("kt57955.kt") + public void testKt57955() throws Exception { + runTest("compiler/testData/codegen/box/delegatedProperty/delegateToAnother/kt57955.kt"); + } + @Test @TestMetadata("mutable.kt") public void testMutable() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java index 616b9d082d5..7704a2b9a2e 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java @@ -15142,6 +15142,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes runTest("compiler/testData/codegen/box/delegatedProperty/delegateToAnother/kt50019_noOptimizedCallableReferences.kt"); } + @Test + @TestMetadata("kt57955.kt") + public void testKt57955() throws Exception { + runTest("compiler/testData/codegen/box/delegatedProperty/delegateToAnother/kt57955.kt"); + } + @Test @TestMetadata("mutable.kt") public void testMutable() throws Exception { diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenWithIrInlinerTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenWithIrInlinerTestGenerated.java index 89b1bf1ff36..f13e0c1539a 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenWithIrInlinerTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenWithIrInlinerTestGenerated.java @@ -15142,6 +15142,12 @@ public class IrBlackBoxCodegenWithIrInlinerTestGenerated extends AbstractIrBlack runTest("compiler/testData/codegen/box/delegatedProperty/delegateToAnother/kt50019_noOptimizedCallableReferences.kt"); } + @Test + @TestMetadata("kt57955.kt") + public void testKt57955() throws Exception { + runTest("compiler/testData/codegen/box/delegatedProperty/delegateToAnother/kt57955.kt"); + } + @Test @TestMetadata("mutable.kt") public void testMutable() throws Exception { diff --git a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java index 96f92aee175..44ec3bb88d8 100644 --- a/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java +++ b/compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java @@ -12115,6 +12115,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes runTest("compiler/testData/codegen/box/delegatedProperty/delegateToAnother/kt50019_noOptimizedCallableReferences.kt"); } + @TestMetadata("kt57955.kt") + public void testKt57955() throws Exception { + runTest("compiler/testData/codegen/box/delegatedProperty/delegateToAnother/kt57955.kt"); + } + @TestMetadata("mutable.kt") public void testMutable() throws Exception { runTest("compiler/testData/codegen/box/delegatedProperty/delegateToAnother/mutable.kt");