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

compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBlackBoxCodegenTestGenerated.java

@@ -41884,6 +41884,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT
             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
         @TestMetadata("protectedFromLambda.kt")
         public void testProtectedFromLambda() throws Exception {

compiler/fir/fir2ir/tests-gen/org/jetbrains/kotlin/test/runners/codegen/FirBytecodeTextTestGenerated.java

@@ -367,6 +367,12 @@ public class FirBytecodeTextTestGenerated extends AbstractFirBytecodeTextTest {
         runTest("compiler/testData/codegen/bytecodeText/noAccessorForProtectedInSamePackageCrossinline.kt");
     }
 
+    @Test
+    @TestMetadata("noAccessorForProtectedInSamePackagePrivateInline.kt")
+    public void testNoAccessorForProtectedInSamePackagePrivateInline() throws Exception {
+        runTest("compiler/testData/codegen/bytecodeText/noAccessorForProtectedInSamePackagePrivateInline.kt");
+    }
+
     @Test
     @TestMetadata("noFlagAnnotations.kt")
     public void testNoFlagAnnotations() throws Exception {

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

@@ -11,6 +11,7 @@ import org.jetbrains.kotlin.backend.common.descriptors.synthesizedString
 import org.jetbrains.kotlin.backend.common.ir.*
 import org.jetbrains.kotlin.backend.jvm.JvmBackendContext
 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.intrinsics.receiverAndArgs
 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.DescriptorVisibility
 import org.jetbrains.kotlin.descriptors.Modality
+import org.jetbrains.kotlin.ir.IrElement
 import org.jetbrains.kotlin.ir.IrStatement
 import org.jetbrains.kotlin.ir.UNDEFINED_OFFSET
 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 inlineLambdaToCallSite = mutableMapOf<IrFunction, LambdaCallSite>()
+    private val inlineFunctionToCallSites = mutableMapOf<IrFunction, Set<IrElement>>()
 
     override fun lower(irFile: IrFile) {
         irFile.accept(object : IrInlineReferenceLocator(context) {
@@ -56,6 +59,24 @@ internal class SyntheticAccessorLowering(val context: JvmBackendContext) : IrEle
                 inlineLambdaToCallSite[argument.symbol.owner] =
                     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)
 
         irFile.transformChildrenVoid(this)
@@ -752,58 +773,76 @@ 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.
     // 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.
-    private fun getScopeClassOrPackage(): IrDeclarationContainer? {
+    private tailrec fun getScopeClassOrPackage(context: IrElement?, approximateToPackage: Boolean): IrDeclarationContainer? {
-        var context = currentScope?.irElement
+        val callSite = inlineLambdaToCallSite[context]
-        var throughCrossinlineLambda = false
+        return when {
-        while (context is IrDeclaration) {
+            // Crossinline lambdas can be inlined into some other class in the same package. However,
-            val callSite = inlineLambdaToCallSite[context]
+            // classes within crossinline lambdas should not be regenerated, so if we've already found
-            when {
+            // a class *before* reaching this lambda, it's valid:
-                // Crossinline lambdas can be inlined into some other class in the same package. However,
+            //     class C {
-                // classes within crossinline lambdas should not be regenerated, so if we've already found
+            //         fun f() {}
-                // a class *before* reaching this lambda, it's valid:
+            //         fun g() = inlineFunctionWithCrossinlineArgument {
-                //     class C {
+            //             f() // this call is done in some unknown class within C's package
-                //         fun f() {}
+            //             object { val x = f() } // this call is done in C$g$1$1
-                //         fun g() = inlineFunctionWithCrossinlineArgument {
+            //         }
-                //             f() // this call is done in some unknown class within C's package
+            //     }
-                //             object { val x = f() } // this call is done in C$g$1$1
+            callSite != null -> getScopeClassOrPackage(callSite.scope, approximateToPackage || callSite.crossinline)
-                //         }
+            // Inline functions can be inlined into anywhere. Not even private inline functions are safe:
-                //     }
+            //     class C {
-                callSite != null -> throughCrossinlineLambda = throughCrossinlineLambda || callSite.crossinline
+            //         fun f() {}
-                // Inline functions can be inlined into anywhere. Not even private inline functions are safe:
+            //         private inline fun g1() = f() // `f` is called from C?
-                //     class C {
+            //         fun g2() = { g1() } // ...or from C$g2$1 in the same package?
-                //         fun f() {}
+            //         inline fun g3() = g1() // ...or from some other package that calls g3?
-                //         private inline fun g1() = f() // `f` is called from C?
+            //     }
-                //         fun g2() = { g1() } // ...or from C$g2$1 in the same package?
+            // However, for private ones we at least know where they're called, so just like inline lambdas,
-                //         inline fun g3() = g1() // ...or from some other package that calls g3?
+            // 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 -> {
-                // TODO: if this class is an object local to an inline function, it could be regenerated,
+                val callSites = inlineFunctionToCallSites[context] ?: return null
-                //  so the scope depends on the declaration accessed (see KT-48508):
+                when {
-                //     class C {
+                    callSites.isEmpty() -> getScopeClassOrPackage(context.parent, approximateToPackage)
-                //         fun f1()
+                    callSites.size == 1 -> getScopeClassOrPackage(callSites.single(), approximateToPackage)
-                //         inline fun inlineFun() = object {
+                    else -> {
-                //             fun f2() {}
+                        // TODO: cache the results
-                //             fun g1() {
+                        @Suppress("NON_TAIL_RECURSIVE_CALL")
-                //                 f1() // this access can be anywhere
+                        val results = callSites.map { getScopeClassOrPackage(it, approximateToPackage = false) ?: return null }
-                //                 f2() // can pretend this access is from C$foo$1
+                        // 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)
-                //  Further complicating things, the accessor for `f1` cannot be in `C$foo$1`, as otherwise
+                    }
-                //  the accessor itself will be regenerated (and thus not work) at `inlineFun` call sites.
+                }
-                context is IrClass && !throughCrossinlineLambda -> return context
             }
+            // 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):
+            //     class C {
+            //         fun f1()
+            //         inline fun inlineFun() = object {
+            //             fun f2() {}
+            //             fun g1() {
+            //                 f1() // this access can be anywhere
+            //                 f2() // can pretend this access is from C$foo$1
+            //             }
+            //         }
+            //     }
+            //  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.
+            context is IrClass && !approximateToPackage -> context
             // 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.
-            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
     }
 }
 

compiler/testData/codegen/box/syntheticAccessors/packagePrivateInPrivateInline.kt

@@ -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()

compiler/testData/codegen/bytecodeText/noAccessorForProtectedInSamePackagePrivateInline.kt

@@ -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

compiler/testData/debug/localVariables/suspend/mergeLvt.kt

@@ -39,8 +39,6 @@ suspend fun box() {
 // test.kt:25 box:
 // 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: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:6 getValue:

compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BlackBoxCodegenTestGenerated.java

@@ -41728,6 +41728,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
             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
         @TestMetadata("protectedFromLambda.kt")
         public void testProtectedFromLambda() throws Exception {

compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/BytecodeTextTestGenerated.java

@@ -355,6 +355,12 @@ public class BytecodeTextTestGenerated extends AbstractBytecodeTextTest {
         runTest("compiler/testData/codegen/bytecodeText/noAccessorForProtectedInSamePackageCrossinline.kt");
     }
 
+    @Test
+    @TestMetadata("noAccessorForProtectedInSamePackagePrivateInline.kt")
+    public void testNoAccessorForProtectedInSamePackagePrivateInline() throws Exception {
+        runTest("compiler/testData/codegen/bytecodeText/noAccessorForProtectedInSamePackagePrivateInline.kt");
+    }
+
     @Test
     @TestMetadata("noFlagAnnotations.kt")
     public void testNoFlagAnnotations() throws Exception {

compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBlackBoxCodegenTestGenerated.java

@@ -41884,6 +41884,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
             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
         @TestMetadata("protectedFromLambda.kt")
         public void testProtectedFromLambda() throws Exception {

compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/codegen/IrBytecodeTextTestGenerated.java

@@ -367,6 +367,12 @@ public class IrBytecodeTextTestGenerated extends AbstractIrBytecodeTextTest {
         runTest("compiler/testData/codegen/bytecodeText/noAccessorForProtectedInSamePackageCrossinline.kt");
     }
 
+    @Test
+    @TestMetadata("noAccessorForProtectedInSamePackagePrivateInline.kt")
+    public void testNoAccessorForProtectedInSamePackagePrivateInline() throws Exception {
+        runTest("compiler/testData/codegen/bytecodeText/noAccessorForProtectedInSamePackagePrivateInline.kt");
+    }
+
     @Test
     @TestMetadata("noFlagAnnotations.kt")
     public void testNoFlagAnnotations() throws Exception {

compiler/tests-gen/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java

@@ -33568,6 +33568,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes
             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")
         public void testProtectedFromLambda() throws Exception {
             runTest("compiler/testData/codegen/box/syntheticAccessors/protectedFromLambda.kt");