[IC] Update inline function snapshotting

This is a more robust version with extra tests and better support for
non-lambda local classes

^KT-75883

Author: mazhukinevgeniy

build-common/src/org/jetbrains/kotlin/incremental/impl/ExtraClassInfoGenerator.kt

@@ -25,7 +25,16 @@ open class ExtraClassInfoGenerator() {
         return classNode
     }
 
-    protected open fun calculateInlineMethodHash(methodSignature: JvmMemberSignature.Method, ownMethodHash: Long): Long {
+    /**
+     * @param methodSignature well-typed method signature. doesn't include the containing class' internal name
+     * @param inlinedClassPrefix - includes class internal name and method name. example value is "com/bar/OuterClass$InnerClass$calculate"
+     * @param ownMethodHash - a basic intuition is that it's based on bytecode and debug info
+     */
+    protected open fun calculateInlineMethodHash(
+        methodSignature: JvmMemberSignature.Method,
+        inlinedClassPrefix: String,
+        ownMethodHash: Long
+    ): Long {
         return ownMethodHash
     }
 
@@ -89,8 +98,9 @@ open class ExtraClassInfoGenerator() {
             //     class metadata (also in the source file), but not in the bytecode. However, we can safely ignore those
             //     inline functions/accessors because they are not declared in the bytecode and therefore can't be referenced.
             val methodSignature = JvmMemberSignature.Method(name = methodNode.name, desc = methodNode.desc)
+            val innerClassPrefix = "${classNode.name}\$${methodNode.name}"
             var methodHash = snapshotMethod(methodNode, classNode.version)
-            inlineFunctionsAndAccessors[methodSignature]!! to calculateInlineMethodHash(methodSignature, methodHash)
+            inlineFunctionsAndAccessors[methodSignature]!! to calculateInlineMethodHash(methodSignature, innerClassPrefix, methodHash)
         }
 
         return ExtraInfo(classSnapshotExcludingMembers, constantSnapshots, inlineFunctionOrAccessorSnapshots)

build-common/src/org/jetbrains/kotlin/incremental/impl/InstanceOwnerRecordingClassVisitor.kt

@@ -32,13 +32,26 @@ class InstanceOwnerRecordingClassVisitor(
 
         return object : MethodVisitor(Opcodes.ASM9, super.visitMethod(access, name, descriptor, signature, exceptions)) {
             override fun visitFieldInsn(opcode: Int, owner: String?, name: String?, descriptor: String?) {
-                if (owner != null && opcode == Opcodes.GETSTATIC && name == "INSTANCE") {
-                    val jvmClassName = JvmClassName.byInternalName(owner)
-                    methodToUsedClassesMap?.getOrPut(methodSignature) { mutableSetOf() }?.add(jvmClassName)
-                    allUsedClassesSet?.add(jvmClassName)
+                if (opcode == Opcodes.GETSTATIC && name == "INSTANCE" && owner?.contains("$") == true) {
+                    storeUsage(owner)
                 }
                 super.visitFieldInsn(opcode, owner, name, descriptor)
             }
+
+            override fun visitTypeInsn(opcode: Int, type: String?) {
+                if (opcode == Opcodes.NEW && type?.contains("$") == true) {
+                    // here we might be instantiating a class from another inline function in the same module
+                    // better safe than sorry, so:
+                    storeUsage(type)
+                }
+                super.visitTypeInsn(opcode, type)
+            }
+
+            private fun storeUsage(internalName: String) {
+                val jvmClassName = JvmClassName.byInternalName(internalName)
+                methodToUsedClassesMap?.getOrPut(methodSignature) { mutableSetOf() }?.add(jvmClassName)
+                allUsedClassesSet?.add(jvmClassName)
+            }
         }
     }
 }

compiler/build-tools/kotlin-build-tools-api-tests/src/main/resources/modules/ic-scenarios/inline-local-class/nested-inline-as-anonymous-object/app/callSite.kt

@@ -0,0 +1,5 @@
+val result = calculateA()
+
+fun main(args: Array<String>) {
+    println(result)
+}

compiler/build-tools/kotlin-build-tools-api-tests/src/main/resources/modules/ic-scenarios/inline-local-class/nested-inline-as-anonymous-object/lib/inlinedA.kt

@@ -0,0 +1,3 @@
+inline fun calculateA(): Int {
+    return calculateB()
+}

compiler/build-tools/kotlin-build-tools-api-tests/src/main/resources/modules/ic-scenarios/inline-local-class/nested-inline-as-anonymous-object/lib/inlinedB.kt

@@ -0,0 +1,6 @@
+inline fun calculateB(): Int {
+    val calculator = object {
+        fun calc(): Int = 42
+    }
+    return calculator.calc()
+}

compiler/build-tools/kotlin-build-tools-api-tests/src/main/resources/modules/ic-scenarios/inline-local-class/nested-inline-as-anonymous-object/lib/inlinedB.kt.changeObjectInB

@@ -0,0 +1,6 @@
+inline fun calculateB(): Int {
+    val calculator = object {
+        fun calc(): Int = 42 + 3
+    }
+    return calculator.calc()
+}

compiler/build-tools/kotlin-build-tools-api-tests/src/main/resources/modules/ic-scenarios/inline-local-class/nested-inline-as-typed-object/app/callSite.kt

@@ -0,0 +1,5 @@
+val result = calculateA()
+
+fun main(args: Array<String>) {
+    println(result)
+}

compiler/build-tools/kotlin-build-tools-api-tests/src/main/resources/modules/ic-scenarios/inline-local-class/nested-inline-as-typed-object/lib/inlinedA.kt

@@ -0,0 +1,3 @@
+inline fun calculateA(): Int {
+    return calculateB()
+}

compiler/build-tools/kotlin-build-tools-api-tests/src/main/resources/modules/ic-scenarios/inline-local-class/nested-inline-as-typed-object/lib/inlinedB.kt

@@ -0,0 +1,10 @@
+interface NiceType {
+    fun calc(): Int
+}
+
+inline fun calculateB(): Int {
+    val calculator = object : NiceType {
+        override fun calc(): Int = 42
+    }
+    return calculator.calc()
+}

compiler/build-tools/kotlin-build-tools-api-tests/src/main/resources/modules/ic-scenarios/inline-local-class/nested-inline-as-typed-object/lib/inlinedB.kt.changeObjectInB

@@ -0,0 +1,10 @@
+interface NiceType {
+    fun calc(): Int
+}
+
+inline fun calculateB(): Int {
+    val calculator = object : NiceType {
+        override fun calc(): Int = 42 + 3
+    }
+    return calculator.calc()
+}

compiler/build-tools/kotlin-build-tools-api-tests/src/testCrossModuleIncrementalChanges/kotlin/InlinedLambdaChangeTest.kt

@@ -13,7 +13,6 @@ import org.jetbrains.kotlin.buildtools.api.tests.compilation.util.compile
 import org.jetbrains.kotlin.buildtools.api.tests.compilation.util.execute
 import org.jetbrains.kotlin.buildtools.api.tests.compilation.util.moduleWithInlineSnapshotting
 import org.jetbrains.kotlin.test.TestMetadata
-import org.junit.jupiter.api.Disabled
 import org.junit.jupiter.api.DisplayName
 
 
@@ -190,9 +189,51 @@ class InlinedLambdaChangeTest : BaseCompilationTest() {
 
             lib.replaceFileWithVersion("inlinedB.kt", "changeLambdaInB")
 
-            // It so happens that we don't need to recompile A.
-            // As far as IC is concerned, the compiler can do whatever it wants:
-            // the main criteria for these tests is that the expected output is generated by the built & executed app.
+            // Because of a compiler optimization, we don't need to recompile A:
+            // Inside the lib module inlinedA directly uses a local class from inlinedB without copying it.
+            // It's ok. The main criteria for these tests is that the expected output is generated by the built & executed app.
+            lib.compile(expectedDirtySet = setOf("inlinedB.kt"))
+            app.compile(expectedDirtySet = setOf("callSite.kt"))
+            app.execute(mainClass = "CallSiteKt", exactOutput = WITH_NEW_LAMBDA_BODY)
+        }
+    }
+
+    @DefaultStrategyAgnosticCompilationTest
+    @DisplayName("Changes in anonymous object inside inline function B affect call site of inline function A that calls B - happy version")
+    @TestMetadata("ic-scenarios/inline-local-class/nested-inline-as-anonymous-object/lib")
+    fun testNestedInlineTrulyAnonymousObject(strategyConfig: CompilerExecutionStrategyConfiguration) {
+        scenario(strategyConfig) {
+            val lib = module("ic-scenarios/inline-local-class/nested-inline-as-anonymous-object/lib")
+            val app = moduleWithInlineSnapshotting(
+                "ic-scenarios/inline-local-class/nested-inline-as-anonymous-object/app",
+                dependencies = listOf(lib),
+            )
+
+            app.execute(mainClass = "CallSiteKt", exactOutput = INITIAL_OUTPUT)
+
+            lib.replaceFileWithVersion("inlinedB.kt", "changeObjectInB")
+
+            lib.compile(expectedDirtySet = setOf("inlinedB.kt"))
+            app.compile(expectedDirtySet = setOf("callSite.kt"))
+            app.execute(mainClass = "CallSiteKt", exactOutput = WITH_NEW_LAMBDA_BODY)
+        }
+    }
+
+    @DefaultStrategyAgnosticCompilationTest
+    @DisplayName("Changes in anonymous object inside inline function B affect call site of inline function A that calls B")
+    @TestMetadata("ic-scenarios/inline-local-class/nested-inline-as-typed-object/lib")
+    fun testNestedInlineAnonymousObject(strategyConfig: CompilerExecutionStrategyConfiguration) {
+        scenario(strategyConfig) {
+            val lib = module("ic-scenarios/inline-local-class/nested-inline-as-typed-object/lib")
+            val app = moduleWithInlineSnapshotting(
+                "ic-scenarios/inline-local-class/nested-inline-as-typed-object/app",
+                dependencies = listOf(lib),
+            )
+
+            app.execute(mainClass = "CallSiteKt", exactOutput = INITIAL_OUTPUT)
+
+            lib.replaceFileWithVersion("inlinedB.kt", "changeObjectInB")
+
             lib.compile(expectedDirtySet = setOf("inlinedB.kt"))
             app.compile(expectedDirtySet = setOf("callSite.kt"))
             app.execute(mainClass = "CallSiteKt", exactOutput = WITH_NEW_LAMBDA_BODY)
@@ -284,7 +325,6 @@ class InlinedLambdaChangeTest : BaseCompilationTest() {
         }
     }
 
-    @Disabled("KT-75883 - here callable's code creates a new object so there's no INSTANCE")
     @DefaultStrategyAgnosticCompilationTest
     @DisplayName("Recompilation of call site affected by an anonymous object - slightly evil")
     @TestMetadata("ic-scenarios/inline-local-class/inline-anonymous-object-evil/lib")
@@ -301,7 +341,7 @@ class InlinedLambdaChangeTest : BaseCompilationTest() {
             lib.replaceFileWithVersion("SomeClass.kt", "withOverload")
 
             lib.compile(expectedDirtySet = setOf("SomeClass.kt", "callable.kt"))
-            app.compile(expectedDirtySet = setOf())
+            app.compile(expectedDirtySet = setOf("callSite.kt"))
             app.execute(mainClass = "CallSiteKt", exactOutput = WITH_NEW_LAMBDA_BODY)
         }
     }

compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/impl/ClassListSnapshotter.kt

@@ -142,11 +142,7 @@ internal class ClassListSnapshotterWithInlinedClassSupport(
 
 
     override fun snapshot(): List<ClassSnapshot> {
-        return classes.map {
-            val mapped = makeOrReuseClassSnapshot(it)
-
-            mapped
-        }
+        return classes.map { makeOrReuseClassSnapshot(it) }
     }
 
     private fun makeOrReuseClassSnapshot(classFile: ClassFileWithContentsProvider): ClassSnapshot {
@@ -164,9 +160,7 @@ internal class ClassListSnapshotterWithInlinedClassSupport(
     private fun makeOrReuseClassSnapshot(descriptor: ClassDescriptorForProcessing, classFileWithContents: ClassFileWithContents): ClassSnapshot {
         descriptor.snapshot?.let { return it }
 
-        // loading is an expensive part of ClassListSnapshotter, so it's worth trying to minimize it.
-        // this part of the implementation would be updated by KT-75883
-        val loadedClasses = mutableListOf<Pair<ClassDescriptorForProcessing, ClassFileWithContents>>()
+        // TODO consider reusing loaded classes across the pipeline - it should be benefitial but it makes the process more complicated
 
         val snapshot = if (isInaccessible(classFileWithContents)) {
             InaccessibleClassSnapshot
@@ -177,13 +171,7 @@ internal class ClassListSnapshotterWithInlinedClassSupport(
                  * so inlinedSnapshots calculation must not directly call regular snapshotting to prevent infinite loops
                  */
                 val extraInfo = ExtraInfoGeneratorWithInlinedClassSnapshotting(
-                    classMultiHashProvider = object : ClassMultiHashProvider {
-                        override fun searchAndGetFullAbiHashOfUsedClasses(rootClasses: Set<JvmClassName>): Long {
-                            val outcome = inlinedClassSnapshotter.searchAndGetFullAbiHashOfUsedClasses(rootClasses)
-                            loadedClasses.addAll(outcome.loadedClasses)
-                            return outcome.calculatedHash
-                        }
-                    },
+                    classMultiHashProvider = inlinedClassSnapshotter as ClassMultiHashProvider,
                 ).getExtraInfo(
                     classFileWithContents.classInfo.kotlinClassHeader!!,
                     classFileWithContents.contents,
@@ -204,9 +192,6 @@ internal class ClassListSnapshotterWithInlinedClassSupport(
 
         descriptor.snapshot = snapshot
 
-        for ((descriptor, contents) in loadedClasses) {
-            makeOrReuseClassSnapshot(descriptor, contents)
-        }
         return snapshot
     }