JVM_IR: avoid redundant accessors in private inline funs
Their call sites are all in the same file, so we can check whether the declarations used in the inline function are accessible from all the places where it will be inlined. #KT-48736 Fixed
This commit is contained in:
+6
@@ -41884,6 +41884,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT
|
|||||||
runTest("compiler/testData/codegen/box/syntheticAccessors/packagePrivate.kt");
|
runTest("compiler/testData/codegen/box/syntheticAccessors/packagePrivate.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@TestMetadata("packagePrivateInPrivateInline.kt")
|
||||||
|
public void testPackagePrivateInPrivateInline() throws Exception {
|
||||||
|
runTest("compiler/testData/codegen/box/syntheticAccessors/packagePrivateInPrivateInline.kt");
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@TestMetadata("protectedFromLambda.kt")
|
@TestMetadata("protectedFromLambda.kt")
|
||||||
public void testProtectedFromLambda() throws Exception {
|
public void testProtectedFromLambda() throws Exception {
|
||||||
|
|||||||
+6
@@ -367,6 +367,12 @@ public class FirBytecodeTextTestGenerated extends AbstractFirBytecodeTextTest {
|
|||||||
runTest("compiler/testData/codegen/bytecodeText/noAccessorForProtectedInSamePackageCrossinline.kt");
|
runTest("compiler/testData/codegen/bytecodeText/noAccessorForProtectedInSamePackageCrossinline.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@TestMetadata("noAccessorForProtectedInSamePackagePrivateInline.kt")
|
||||||
|
public void testNoAccessorForProtectedInSamePackagePrivateInline() throws Exception {
|
||||||
|
runTest("compiler/testData/codegen/bytecodeText/noAccessorForProtectedInSamePackagePrivateInline.kt");
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@TestMetadata("noFlagAnnotations.kt")
|
@TestMetadata("noFlagAnnotations.kt")
|
||||||
public void testNoFlagAnnotations() throws Exception {
|
public void testNoFlagAnnotations() throws Exception {
|
||||||
|
|||||||
+51
-12
@@ -11,6 +11,7 @@ import org.jetbrains.kotlin.backend.common.descriptors.synthesizedString
|
|||||||
import org.jetbrains.kotlin.backend.common.ir.*
|
import org.jetbrains.kotlin.backend.common.ir.*
|
||||||
import org.jetbrains.kotlin.backend.jvm.JvmBackendContext
|
import org.jetbrains.kotlin.backend.jvm.JvmBackendContext
|
||||||
import org.jetbrains.kotlin.backend.jvm.JvmLoweredDeclarationOrigin
|
import org.jetbrains.kotlin.backend.jvm.JvmLoweredDeclarationOrigin
|
||||||
|
import org.jetbrains.kotlin.backend.jvm.codegen.fileParent
|
||||||
import org.jetbrains.kotlin.backend.jvm.codegen.isJvmInterface
|
import org.jetbrains.kotlin.backend.jvm.codegen.isJvmInterface
|
||||||
import org.jetbrains.kotlin.backend.jvm.intrinsics.receiverAndArgs
|
import org.jetbrains.kotlin.backend.jvm.intrinsics.receiverAndArgs
|
||||||
import org.jetbrains.kotlin.backend.jvm.ir.IrInlineReferenceLocator
|
import org.jetbrains.kotlin.backend.jvm.ir.IrInlineReferenceLocator
|
||||||
@@ -19,6 +20,7 @@ import org.jetbrains.kotlin.codegen.AsmUtil
|
|||||||
import org.jetbrains.kotlin.descriptors.DescriptorVisibilities
|
import org.jetbrains.kotlin.descriptors.DescriptorVisibilities
|
||||||
import org.jetbrains.kotlin.descriptors.DescriptorVisibility
|
import org.jetbrains.kotlin.descriptors.DescriptorVisibility
|
||||||
import org.jetbrains.kotlin.descriptors.Modality
|
import org.jetbrains.kotlin.descriptors.Modality
|
||||||
|
import org.jetbrains.kotlin.ir.IrElement
|
||||||
import org.jetbrains.kotlin.ir.IrStatement
|
import org.jetbrains.kotlin.ir.IrStatement
|
||||||
import org.jetbrains.kotlin.ir.UNDEFINED_OFFSET
|
import org.jetbrains.kotlin.ir.UNDEFINED_OFFSET
|
||||||
import org.jetbrains.kotlin.ir.builders.declarations.addValueParameter
|
import org.jetbrains.kotlin.ir.builders.declarations.addValueParameter
|
||||||
@@ -42,6 +44,7 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle
|
|||||||
|
|
||||||
private val pendingAccessorsToAdd = mutableListOf<IrFunction>()
|
private val pendingAccessorsToAdd = mutableListOf<IrFunction>()
|
||||||
private val inlineLambdaToCallSite = mutableMapOf<IrFunction, LambdaCallSite>()
|
private val inlineLambdaToCallSite = mutableMapOf<IrFunction, LambdaCallSite>()
|
||||||
|
private val inlineFunctionToCallSites = mutableMapOf<IrFunction, Set<IrElement>>()
|
||||||
|
|
||||||
override fun lower(irFile: IrFile) {
|
override fun lower(irFile: IrFile) {
|
||||||
irFile.accept(object : IrInlineReferenceLocator(context) {
|
irFile.accept(object : IrInlineReferenceLocator(context) {
|
||||||
@@ -56,6 +59,24 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle
|
|||||||
inlineLambdaToCallSite[argument.symbol.owner] =
|
inlineLambdaToCallSite[argument.symbol.owner] =
|
||||||
LambdaCallSite(scope, parameter.isCrossinline && !callee.isCoroutineIntrinsic())
|
LambdaCallSite(scope, parameter.isCrossinline && !callee.isCoroutineIntrinsic())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
override fun visitSimpleFunction(declaration: IrSimpleFunction, data: IrDeclaration?) {
|
||||||
|
if (declaration.isPrivateInline) {
|
||||||
|
inlineFunctionToCallSites.putIfAbsent(declaration, mutableSetOf())
|
||||||
|
}
|
||||||
|
super.visitSimpleFunction(declaration, data)
|
||||||
|
}
|
||||||
|
|
||||||
|
override fun visitCall(expression: IrCall, data: IrDeclaration?) {
|
||||||
|
val callee = expression.symbol.owner
|
||||||
|
if (callee.isPrivateInline && callee.fileParent == irFile && data != null) {
|
||||||
|
(inlineFunctionToCallSites.getOrPut(callee) { mutableSetOf() } as MutableSet).add(data)
|
||||||
|
}
|
||||||
|
super.visitCall(expression, data)
|
||||||
|
}
|
||||||
|
|
||||||
|
private inline val IrSimpleFunction.isPrivateInline
|
||||||
|
get() = isInline && DescriptorVisibilities.isPrivate(visibility)
|
||||||
}, null)
|
}, null)
|
||||||
|
|
||||||
irFile.transformChildrenVoid(this)
|
irFile.transformChildrenVoid(this)
|
||||||
@@ -752,15 +773,15 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private fun getScopeClassOrPackage(): IrDeclarationContainer? =
|
||||||
|
getScopeClassOrPackage(currentScope?.irElement, approximateToPackage = false)
|
||||||
|
|
||||||
// Get the class from which all accesses in the current scope will be done after bytecode generation.
|
// Get the class from which all accesses in the current scope will be done after bytecode generation.
|
||||||
// If the current scope is a crossinline lambda, this is not possible, as the lambda maybe inlined
|
// If the current scope is a crossinline lambda, this is not possible, as the lambda maybe inlined
|
||||||
// into some other class; in that case, get at least the package.
|
// into some other class; in that case, get at least the package.
|
||||||
private fun getScopeClassOrPackage(): IrDeclarationContainer? {
|
private tailrec fun getScopeClassOrPackage(context: IrElement?, approximateToPackage: Boolean): IrDeclarationContainer? {
|
||||||
var context = currentScope?.irElement
|
|
||||||
var throughCrossinlineLambda = false
|
|
||||||
while (context is IrDeclaration) {
|
|
||||||
val callSite = inlineLambdaToCallSite[context]
|
val callSite = inlineLambdaToCallSite[context]
|
||||||
when {
|
return when {
|
||||||
// Crossinline lambdas can be inlined into some other class in the same package. However,
|
// Crossinline lambdas can be inlined into some other class in the same package. However,
|
||||||
// classes within crossinline lambdas should not be regenerated, so if we've already found
|
// classes within crossinline lambdas should not be regenerated, so if we've already found
|
||||||
// a class *before* reaching this lambda, it's valid:
|
// a class *before* reaching this lambda, it's valid:
|
||||||
@@ -771,7 +792,7 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle
|
|||||||
// object { val x = f() } // this call is done in C$g$1$1
|
// object { val x = f() } // this call is done in C$g$1$1
|
||||||
// }
|
// }
|
||||||
// }
|
// }
|
||||||
callSite != null -> throughCrossinlineLambda = throughCrossinlineLambda || callSite.crossinline
|
callSite != null -> getScopeClassOrPackage(callSite.scope, approximateToPackage || callSite.crossinline)
|
||||||
// Inline functions can be inlined into anywhere. Not even private inline functions are safe:
|
// Inline functions can be inlined into anywhere. Not even private inline functions are safe:
|
||||||
// class C {
|
// class C {
|
||||||
// fun f() {}
|
// fun f() {}
|
||||||
@@ -779,10 +800,28 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle
|
|||||||
// fun g2() = { g1() } // ...or from C$g2$1 in the same package?
|
// fun g2() = { g1() } // ...or from C$g2$1 in the same package?
|
||||||
// inline fun g3() = g1() // ...or from some other package that calls g3?
|
// inline fun g3() = g1() // ...or from some other package that calls g3?
|
||||||
// }
|
// }
|
||||||
|
// However, for private ones we at least know where they're called, so just like inline lambdas,
|
||||||
|
// we can navigate there.
|
||||||
|
//
|
||||||
// TODO: this has some weird effects for inline functions in local classes, e.g. they
|
// TODO: this has some weird effects for inline functions in local classes, e.g. they
|
||||||
// access the capture fields (package-private) through accessors; this may or may not
|
// access the capture fields (package-private) through accessors; this may or may not
|
||||||
// be necessary - local types should in theory not be usable outside the current file.
|
// be necessary - local types should in theory not be usable outside the current file.
|
||||||
context is IrFunction && context.isInline -> return null
|
context is IrFunction && context.isInline -> {
|
||||||
|
val callSites = inlineFunctionToCallSites[context] ?: return null
|
||||||
|
when {
|
||||||
|
callSites.isEmpty() -> getScopeClassOrPackage(context.parent, approximateToPackage)
|
||||||
|
callSites.size == 1 -> getScopeClassOrPackage(callSites.single(), approximateToPackage)
|
||||||
|
else -> {
|
||||||
|
// TODO: cache the results
|
||||||
|
@Suppress("NON_TAIL_RECURSIVE_CALL")
|
||||||
|
val results = callSites.map { getScopeClassOrPackage(it, approximateToPackage = false) ?: return null }
|
||||||
|
// If all call sites are within a single class, use it. Otherwise, all scopes must be within
|
||||||
|
// the current file's package.
|
||||||
|
val single = results.first().takeIf { results.all { other -> it === other } }
|
||||||
|
getScopeClassOrPackage(single ?: context.parent, approximateToPackage || single == null)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
// TODO: if this class is an object local to an inline function, it could be regenerated,
|
// TODO: if this class is an object local to an inline function, it could be regenerated,
|
||||||
// so the scope depends on the declaration accessed (see KT-48508):
|
// so the scope depends on the declaration accessed (see KT-48508):
|
||||||
// class C {
|
// class C {
|
||||||
@@ -795,15 +834,15 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle
|
|||||||
// }
|
// }
|
||||||
// }
|
// }
|
||||||
// }
|
// }
|
||||||
// Further complicating things, the accessor for `f1` cannot be in `C$foo$1`, as otherwise
|
// Further complicating things, the accessor for `f1` cannot be in `C$inlineFun$1`, as otherwise
|
||||||
// the accessor itself will be regenerated (and thus not work) at `inlineFun` call sites.
|
// the accessor itself will be regenerated (and thus not work) at `inlineFun` call sites.
|
||||||
context is IrClass && !throughCrossinlineLambda -> return context
|
context is IrClass && !approximateToPackage -> context
|
||||||
}
|
|
||||||
// Inline lambdas have already been moved out to the containing class, but we still need to check
|
// Inline lambdas have already been moved out to the containing class, but we still need to check
|
||||||
// the containing function (again, see above), so navigate there instead.
|
// the containing function (again, see above), so navigate there instead.
|
||||||
context = callSite?.scope ?: context.parent
|
context is IrDeclaration -> getScopeClassOrPackage(context.parent, approximateToPackage)
|
||||||
|
// The only non-declaration parent should be the package.
|
||||||
|
else -> context as? IrPackageFragment
|
||||||
}
|
}
|
||||||
return context as? IrPackageFragment
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
+19
@@ -0,0 +1,19 @@
|
|||||||
|
// TARGET_BACKEND: JVM
|
||||||
|
// FILE: test/J.java
|
||||||
|
package test;
|
||||||
|
|
||||||
|
public class J {
|
||||||
|
String packagePrivate = "OK";
|
||||||
|
}
|
||||||
|
|
||||||
|
// FILE: test/main.kt
|
||||||
|
package test
|
||||||
|
|
||||||
|
object O {
|
||||||
|
fun box() = privateInline()
|
||||||
|
|
||||||
|
// no accessor needed - the only call site is in the same package
|
||||||
|
private inline fun privateInline(): String = J().packagePrivate
|
||||||
|
}
|
||||||
|
|
||||||
|
fun box() = O.box()
|
||||||
+17
@@ -0,0 +1,17 @@
|
|||||||
|
package a
|
||||||
|
|
||||||
|
open class A {
|
||||||
|
protected fun protectedFun(): String = "OK"
|
||||||
|
}
|
||||||
|
|
||||||
|
class BSamePackage: A() {
|
||||||
|
// known to only be called within `BSamePackage`, so accessors are redundant
|
||||||
|
private inline fun test(): String = protectedFun()
|
||||||
|
|
||||||
|
fun onlyTestCallSite() = test()
|
||||||
|
}
|
||||||
|
|
||||||
|
// JVM_TEMPLATES
|
||||||
|
// 2 INVOKESTATIC a/BSamePackage.access
|
||||||
|
// JVM_IR_TEMPLATES
|
||||||
|
// 0 INVOKESTATIC a/BSamePackage.access
|
||||||
@@ -39,8 +39,6 @@ suspend fun box() {
|
|||||||
// test.kt:25 box:
|
// test.kt:25 box:
|
||||||
// test.kt:26 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null
|
// test.kt:26 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null
|
||||||
// test.kt:19 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, $i$f$suspendBar:int=0:int
|
// test.kt:19 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, $i$f$suspendBar:int=0:int
|
||||||
// test.kt:19 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, $i$f$suspendBar:int=0:int
|
|
||||||
// test.kt:14 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, $i$f$suspendBar:int=0:int, $this$extensionFun$iv$iv:AtomicInt=AtomicInt, $i$f$extensionFun:int=0:int
|
|
||||||
// test.kt:14 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, $i$f$suspendBar:int=0:int, $this$extensionFun$iv$iv:AtomicInt=AtomicInt, $i$f$extensionFun:int=0:int
|
// test.kt:14 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, $i$f$suspendBar:int=0:int, $this$extensionFun$iv$iv:AtomicInt=AtomicInt, $i$f$extensionFun:int=0:int
|
||||||
// test.kt:15 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, $i$f$suspendBar:int=0:int, $this$extensionFun$iv$iv:AtomicInt=AtomicInt, $i$f$extensionFun:int=0:int
|
// test.kt:15 box: $continuation:kotlin.coroutines.Continuation=TestKt$box$1, $result:java.lang.Object=null, $i$f$suspendBar:int=0:int, $this$extensionFun$iv$iv:AtomicInt=AtomicInt, $i$f$extensionFun:int=0:int
|
||||||
// test.kt:6 getValue:
|
// test.kt:6 getValue:
|
||||||
|
|||||||
+6
@@ -41728,6 +41728,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
|
|||||||
runTest("compiler/testData/codegen/box/syntheticAccessors/packagePrivate.kt");
|
runTest("compiler/testData/codegen/box/syntheticAccessors/packagePrivate.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@TestMetadata("packagePrivateInPrivateInline.kt")
|
||||||
|
public void testPackagePrivateInPrivateInline() throws Exception {
|
||||||
|
runTest("compiler/testData/codegen/box/syntheticAccessors/packagePrivateInPrivateInline.kt");
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@TestMetadata("protectedFromLambda.kt")
|
@TestMetadata("protectedFromLambda.kt")
|
||||||
public void testProtectedFromLambda() throws Exception {
|
public void testProtectedFromLambda() throws Exception {
|
||||||
|
|||||||
+6
@@ -355,6 +355,12 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest {
|
|||||||
runTest("compiler/testData/codegen/bytecodeText/noAccessorForProtectedInSamePackageCrossinline.kt");
|
runTest("compiler/testData/codegen/bytecodeText/noAccessorForProtectedInSamePackageCrossinline.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@TestMetadata("noAccessorForProtectedInSamePackagePrivateInline.kt")
|
||||||
|
public void testNoAccessorForProtectedInSamePackagePrivateInline() throws Exception {
|
||||||
|
runTest("compiler/testData/codegen/bytecodeText/noAccessorForProtectedInSamePackagePrivateInline.kt");
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@TestMetadata("noFlagAnnotations.kt")
|
@TestMetadata("noFlagAnnotations.kt")
|
||||||
public void testNoFlagAnnotations() throws Exception {
|
public void testNoFlagAnnotations() throws Exception {
|
||||||
|
|||||||
+6
@@ -41884,6 +41884,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
|
|||||||
runTest("compiler/testData/codegen/box/syntheticAccessors/packagePrivate.kt");
|
runTest("compiler/testData/codegen/box/syntheticAccessors/packagePrivate.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@TestMetadata("packagePrivateInPrivateInline.kt")
|
||||||
|
public void testPackagePrivateInPrivateInline() throws Exception {
|
||||||
|
runTest("compiler/testData/codegen/box/syntheticAccessors/packagePrivateInPrivateInline.kt");
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@TestMetadata("protectedFromLambda.kt")
|
@TestMetadata("protectedFromLambda.kt")
|
||||||
public void testProtectedFromLambda() throws Exception {
|
public void testProtectedFromLambda() throws Exception {
|
||||||
|
|||||||
+6
@@ -367,6 +367,12 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest {
|
|||||||
runTest("compiler/testData/codegen/bytecodeText/noAccessorForProtectedInSamePackageCrossinline.kt");
|
runTest("compiler/testData/codegen/bytecodeText/noAccessorForProtectedInSamePackageCrossinline.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
@TestMetadata("noAccessorForProtectedInSamePackagePrivateInline.kt")
|
||||||
|
public void testNoAccessorForProtectedInSamePackagePrivateInline() throws Exception {
|
||||||
|
runTest("compiler/testData/codegen/bytecodeText/noAccessorForProtectedInSamePackagePrivateInline.kt");
|
||||||
|
}
|
||||||
|
|
||||||
@Test
|
@Test
|
||||||
@TestMetadata("noFlagAnnotations.kt")
|
@TestMetadata("noFlagAnnotations.kt")
|
||||||
public void testNoFlagAnnotations() throws Exception {
|
public void testNoFlagAnnotations() throws Exception {
|
||||||
|
|||||||
+5
@@ -33568,6 +33568,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes
|
|||||||
runTest("compiler/testData/codegen/box/syntheticAccessors/kt9958Interface.kt");
|
runTest("compiler/testData/codegen/box/syntheticAccessors/kt9958Interface.kt");
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@TestMetadata("packagePrivateInPrivateInline.kt")
|
||||||
|
public void testPackagePrivateInPrivateInline() throws Exception {
|
||||||
|
runTest("compiler/testData/codegen/box/syntheticAccessors/packagePrivateInPrivateInline.kt");
|
||||||
|
}
|
||||||
|
|
||||||
@TestMetadata("protectedFromLambda.kt")
|
@TestMetadata("protectedFromLambda.kt")
|
||||||
public void testProtectedFromLambda() throws Exception {
|
public void testProtectedFromLambda() throws Exception {
|
||||||
runTest("compiler/testData/codegen/box/syntheticAccessors/protectedFromLambda.kt");
|
runTest("compiler/testData/codegen/box/syntheticAccessors/protectedFromLambda.kt");
|
||||||
|
|||||||
Reference in New Issue
Block a user