Handle changes to inline functions/property accessors with @JvmNames

If we detect a change in an inline function `foo` with @JvmName `fooJvmName`, we have two options: 1. Report that function `foo` has changed 2. Report that method `fooJvmName` has changed Similarly, if we detect a change in an inline property accessor with JvmName `getFoo` of property `foo`, we have two options: 1. Report that property `foo` has changed 2. Report that property accessor `getFoo` has changed The compiler is guaranteed to generate `LookupSymbol`s corresponding to option 1 when referencing inline functions/property accessors, but it is not guaranteed to generate `LookupSymbol`s corresponding to option 2. (Currently the compiler seems to support option 2 for *inline* functions/property accessors, but that may change.) Therefore, we will choose option 1 as it is cleaner and safer. ^KT-54144 In progress Small cleanup in IncrementalCompilerRunner - Add comment for closing caches - Rename providedChangedFiles to changedFiles - Tiny clean up in `performWorkBeforeCompilation` - Count directories to delete in debug logs ^KT-53015 In progress Extract KotlinClassInfo to a separate class to reduce the size of IncrementalJvmCache and prepare for the next change. ^KT-54144 In progress Ignore inline functions that are not found in the bytecode ^KT-54144 In progress Add unit test for handling `@JvmName`s Test: KotlinOnlyClasspathChangesComputerTest #testFunctionsAndPropertyAccessorsWithJvmNames ^KT-54144 Fixed Update unit tests for handling `@JvmName`s In a previous commit, we made a behavior change for inline property accessors: The existing behavior is that if the implementation of an inline getter has changed, only usages of the getter will be impacted but not usages of the setter (and vice versa). After that previous commit, usages of *both* the getter and setter will now be impacted (i.e., we might compile slightly more files). This is because a change to either the getter or the setter will now be considered a change to the property, which will help simplify our change analysis. This commit updates the relevant unit tests to reflect the new behavior. Test: Updated Incremental*TestGenerated.PureKotlin#testInlinePropertyInClass and Incremental*TestGenerated.PureKotlin#testInlinePropertyOnTopLevel ^KT-54144 Fixed
2022-10-13 19:32:14 +01:00
parent e1e07d2094
commit cdbbead157
21 changed files with 515 additions and 366 deletions
@@ -5,6 +5,7 @@

 package org.jetbrains.kotlin.incremental

+import org.jetbrains.kotlin.incremental.DifferenceCalculatorForClass.Companion.getNonPrivateMembers
 import org.jetbrains.kotlin.metadata.ProtoBuf.Visibility.PRIVATE
 import org.jetbrains.kotlin.metadata.deserialization.Flags
 import org.jetbrains.kotlin.name.FqName
@@ -97,8 +98,7 @@ class AbiSnapshotDiffService() {
            when (protoData) {
                is ClassProtoData -> {
                    fqNames.add(fqName)
-                    symbols.addAll(
-                        protoData.getNonPrivateMemberNames(includeInlineAccessors = true).map { LookupSymbol(it, fqName.asString()) })
+                    symbols.addAll(protoData.getNonPrivateMembers().map { LookupSymbol(it, fqName.asString()) })
                }
                is PackagePartProtoData -> {
                    symbols.addAll(
@@ -18,12 +18,15 @@ package org.jetbrains.kotlin.incremental

 import org.jetbrains.kotlin.build.DEFAULT_KOTLIN_SOURCE_FILES_EXTENSIONS
 import org.jetbrains.kotlin.build.GeneratedFile
-import org.jetbrains.kotlin.build.report.*
+import org.jetbrains.kotlin.build.report.BuildReporter
+import org.jetbrains.kotlin.build.report.debug
+import org.jetbrains.kotlin.build.report.info
 import org.jetbrains.kotlin.build.report.metrics.BuildAttribute
 import org.jetbrains.kotlin.build.report.metrics.BuildAttribute.*
 import org.jetbrains.kotlin.build.report.metrics.BuildPerformanceMetric
 import org.jetbrains.kotlin.build.report.metrics.BuildTime
 import org.jetbrains.kotlin.build.report.metrics.measure
+import org.jetbrains.kotlin.build.report.warn
 import org.jetbrains.kotlin.cli.common.*
 import org.jetbrains.kotlin.cli.common.arguments.CommonCompilerArguments
 import org.jetbrains.kotlin.cli.common.messages.MessageCollector
@@ -80,12 +83,12 @@ abstract class IncrementalCompilerRunner<
        allSourceFiles: List<File>,
        args: Args,
        messageCollector: MessageCollector,
-        // when [providedChangedFiles] is not null, changes are provided by external system (e.g. Gradle)
+        // when `changedFiles` is not null, changes are provided by external system (e.g. Gradle)
        // otherwise we track source files changes ourselves.
-        providedChangedFiles: ChangedFiles?,
+        changedFiles: ChangedFiles?,
        projectDir: File? = null
    ): ExitCode = reporter.measure(BuildTime.INCREMENTAL_COMPILATION_DAEMON) {
-        return when (val result = tryCompileIncrementally(allSourceFiles, providedChangedFiles, args, projectDir, messageCollector)) {
+        return when (val result = tryCompileIncrementally(allSourceFiles, changedFiles, args, projectDir, messageCollector)) {
            is ICResult.Completed -> {
                reporter.debug { "Incremental compilation completed" }
                result.exitCode
@@ -95,7 +98,7 @@ abstract class IncrementalCompilerRunner<
                reporter.addAttribute(result.reason)

                compileNonIncrementally(
-                    result.reason, allSourceFiles, args, projectDir, trackChangedFiles = providedChangedFiles == null, messageCollector
+                    result.reason, allSourceFiles, args, projectDir, trackChangedFiles = changedFiles == null, messageCollector
                )
            }
            is ICResult.Failed -> {
@@ -114,7 +117,7 @@ abstract class IncrementalCompilerRunner<
                reporter.addAttribute(result.reason)

                compileNonIncrementally(
-                    result.reason, allSourceFiles, args, projectDir, trackChangedFiles = providedChangedFiles == null, messageCollector
+                    result.reason, allSourceFiles, args, projectDir, trackChangedFiles = changedFiles == null, messageCollector
                )
            }
        }
@@ -141,23 +144,24 @@ abstract class IncrementalCompilerRunner<
     */
    private fun tryCompileIncrementally(
        allSourceFiles: List<File>,
-        providedChangedFiles: ChangedFiles?,
+        changedFiles: ChangedFiles?,
        args: Args,
        projectDir: File?,
        messageCollector: MessageCollector
    ): ICResult {
-        if (providedChangedFiles is ChangedFiles.Unknown) {
+        if (changedFiles is ChangedFiles.Unknown) {
            return ICResult.RequiresRebuild(UNKNOWN_CHANGES_IN_GRADLE_INPUTS)
        }
-        providedChangedFiles as ChangedFiles.Known?
+        changedFiles as ChangedFiles.Known?

        val caches = createCacheManager(args, projectDir)
        val exitCode: ExitCode
        try {
            // Step 1: Get changed files
-            val changedFiles: ChangedFiles.Known = try {
-                getChangedFiles(providedChangedFiles, allSourceFiles, caches)
+            val knownChangedFiles: ChangedFiles.Known = try {
+                getChangedFiles(changedFiles, allSourceFiles, caches)
            } catch (e: Throwable) {
+                // Don't need to close caches in cases where we return `ICResult.Failed` because we will compile non-incrementally anyway
                return ICResult.Failed(IC_FAILED_TO_GET_CHANGED_FILES, e)
            }

@@ -166,7 +170,7 @@ abstract class IncrementalCompilerRunner<
            // Step 2: Compute files to recompile
            val compilationMode = try {
                reporter.measure(BuildTime.IC_CALCULATE_INITIAL_DIRTY_SET) {
-                    calculateSourcesToCompile(caches, changedFiles, args, messageCollector, classpathAbiSnapshot ?: emptyMap())
+                    calculateSourcesToCompile(caches, knownChangedFiles, args, messageCollector, classpathAbiSnapshot ?: emptyMap())
                }
            } catch (e: Throwable) {
                return ICResult.Failed(IC_FAILED_TO_COMPUTE_FILES_TO_RECOMPILE, e)
@@ -227,7 +231,7 @@ abstract class IncrementalCompilerRunner<
                check(it.containsAll(mainOutputDirs)) { "outputDirs is missing classesDir and workingDir: $it" }
            } ?: mainOutputDirs

-            reporter.debug { "Cleaning output directories" }
+            reporter.debug { "Cleaning ${outputDirsToClean.size} output directories" }
            cleanOrCreateDirectories(outputDirsToClean)
        }
        return createCacheManager(args, projectDir).use { caches ->
@@ -266,20 +270,20 @@ abstract class IncrementalCompilerRunner<
    }

    private fun getChangedFiles(
-        providedChangedFiles: ChangedFiles.Known?,
+        changedFiles: ChangedFiles.Known?,
        allSourceFiles: List<File>,
        caches: CacheManager
    ): ChangedFiles.Known {
        return when {
-            providedChangedFiles == null -> caches.inputsCache.sourceSnapshotMap.compareAndUpdate(allSourceFiles)
-            providedChangedFiles.forDependencies -> {
+            changedFiles == null -> caches.inputsCache.sourceSnapshotMap.compareAndUpdate(allSourceFiles)
+            changedFiles.forDependencies -> {
                val moreChangedFiles = caches.inputsCache.sourceSnapshotMap.compareAndUpdate(allSourceFiles)
                ChangedFiles.Known(
-                    modified = providedChangedFiles.modified + moreChangedFiles.modified,
-                    removed = providedChangedFiles.removed + moreChangedFiles.removed
+                    modified = changedFiles.modified + moreChangedFiles.modified,
+                    removed = changedFiles.removed + moreChangedFiles.removed
                )
            }
-            else -> providedChangedFiles
+            else -> changedFiles
        }
    }

@@ -109,7 +109,7 @@ fun makeIncrementally(
                    classpathChanges = ClasspathSnapshotDisabled
                )
        //TODO set properly
-        compiler.compile(sourceFiles, args, messageCollector, providedChangedFiles = null)
+        compiler.compile(sourceFiles, args, messageCollector, changedFiles = null)
    }
 }

@@ -388,9 +388,7 @@ open class IncrementalJvmCompilerRunner(
        super.performWorkBeforeCompilation(compilationMode, args)

        if (compilationMode is CompilationMode.Incremental) {
-            val destinationDir = args.destinationAsFile
-            destinationDir.mkdirs()
-            args.classpathAsList = listOf(destinationDir) + args.classpathAsList
+            args.classpathAsList = listOf(args.destinationAsFile) + args.classpathAsList
        }
    }

@@ -8,6 +8,7 @@ package org.jetbrains.kotlin.incremental.classpathDiff
 import com.intellij.util.containers.Interner
 import com.intellij.util.io.DataExternalizer
 import org.jetbrains.kotlin.build.report.metrics.BuildPerformanceMetric
+import org.jetbrains.kotlin.incremental.ConstantValueExternalizer
 import org.jetbrains.kotlin.incremental.KotlinClassInfo
 import org.jetbrains.kotlin.incremental.storage.*
 import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader
@@ -53,27 +54,6 @@ object CachedClasspathSnapshotSerializer {
    }
 }

-internal open class DataExternalizerForSealedClass<T>(
-    val baseClass: Class<T>,
-    val inheritorClasses: List<Class<out T>>,
-    val inheritorExternalizers: List<DataExternalizer<*>>
-) : DataExternalizer<T> {
-
-    override fun save(output: DataOutput, objectToExternalize: T) {
-        val inheritorClassIndex =
-            inheritorClasses.indexOfFirst { it.isAssignableFrom(objectToExternalize!!::class.java) }.also { check(it != -1) }
-        output.writeByte(inheritorClassIndex.also { check(it <= Byte.MAX_VALUE) }) // Write byte so the data is smaller
-        @Suppress("UNCHECKED_CAST")
-        (inheritorExternalizers[inheritorClassIndex] as DataExternalizer<T>).save(output, objectToExternalize)
-    }
-
-    override fun read(input: DataInput): T {
-        val inheritorClassIndex = input.readByte().toInt()
-        @Suppress("UNCHECKED_CAST")
-        return inheritorExternalizers[inheritorClassIndex].read(input) as T
-    }
-}
-
 object ClasspathEntrySnapshotExternalizer : DataExternalizer<ClasspathEntrySnapshot> {

    override fun save(output: DataOutput, snapshot: ClasspathEntrySnapshot) {
@@ -87,26 +67,23 @@ object ClasspathEntrySnapshotExternalizer : DataExternalizer<ClasspathEntrySnaps
    }
 }

-internal object ClassSnapshotExternalizer : DataExternalizerForSealedClass<ClassSnapshot>(
-    baseClass = ClassSnapshot::class.java,
-    inheritorClasses = listOf(AccessibleClassSnapshot::class.java, InaccessibleClassSnapshot::class.java),
-    inheritorExternalizers = listOf(AccessibleClassSnapshotExternalizer, InaccessibleClassSnapshotExternalizer)
+internal object ClassSnapshotExternalizer : DataExternalizer<ClassSnapshot> by DelegateDataExternalizer(
+    types = listOf(AccessibleClassSnapshot::class.java, InaccessibleClassSnapshot::class.java),
+    typesExternalizers = listOf(AccessibleClassSnapshotExternalizer, InaccessibleClassSnapshotExternalizer)
 )

-internal object AccessibleClassSnapshotExternalizer : DataExternalizerForSealedClass<AccessibleClassSnapshot>(
-    baseClass = AccessibleClassSnapshot::class.java,
-    inheritorClasses = listOf(KotlinClassSnapshot::class.java, JavaClassSnapshot::class.java),
-    inheritorExternalizers = listOf(KotlinClassSnapshotExternalizer, JavaClassSnapshotExternalizer)
+internal object AccessibleClassSnapshotExternalizer : DataExternalizer<AccessibleClassSnapshot> by DelegateDataExternalizer(
+    types = listOf(KotlinClassSnapshot::class.java, JavaClassSnapshot::class.java),
+    typesExternalizers = listOf(KotlinClassSnapshotExternalizer, JavaClassSnapshotExternalizer)
 )

-private object KotlinClassSnapshotExternalizer : DataExternalizerForSealedClass<KotlinClassSnapshot>(
-    baseClass = KotlinClassSnapshot::class.java,
-    inheritorClasses = listOf(
+private object KotlinClassSnapshotExternalizer : DataExternalizer<KotlinClassSnapshot> by DelegateDataExternalizer(
+    types = listOf(
        RegularKotlinClassSnapshot::class.java,
        PackageFacadeKotlinClassSnapshot::class.java,
        MultifileClassKotlinClassSnapshot::class.java
    ),
-    inheritorExternalizers = listOf(
+    typesExternalizers = listOf(
        RegularKotlinClassSnapshotExternalizer,
        PackageFacadeKotlinClassSnapshotExternalizer,
        MultifileClassKotlinClassSnapshotExternalizer
@@ -185,8 +162,8 @@ internal object KotlinClassInfoExternalizer : DataExternalizer<KotlinClassInfo>
        ListExternalizer(StringExternalizer).save(output, info.classHeaderData.toList())
        ListExternalizer(StringExternalizer).save(output, info.classHeaderStrings.toList())
        NullableValueExternalizer(StringExternalizer).save(output, info.multifileClassName)
-        LinkedHashMapExternalizer(StringExternalizer, ConstantExternalizer).save(output, info.constantsMap)
-        LinkedHashMapExternalizer(StringExternalizer, LongExternalizer).save(output, info.inlineFunctionsAndAccessorsMap)
+        MapExternalizer(StringExternalizer, ConstantValueExternalizer).save(output, info.constantsMap)
+        MapExternalizer(InlineFunctionOrAccessorExternalizer, LongExternalizer).save(output, info.inlineFunctionsAndAccessorsMap)
    }

    override fun read(input: DataInput): KotlinClassInfo {
@@ -197,8 +174,8 @@ internal object KotlinClassInfoExternalizer : DataExternalizer<KotlinClassInfo>
            classHeaderData = ListExternalizer(StringExternalizer).read(input).toTypedArray(),
            classHeaderStrings = ListExternalizer(StringExternalizer).read(input).toTypedArray(),
            multifileClassName = NullableValueExternalizer(StringExternalizer).read(input),
-            constantsMap = LinkedHashMapExternalizer(StringExternalizer, ConstantExternalizer).read(input),
-            inlineFunctionsAndAccessorsMap = LinkedHashMapExternalizer(StringExternalizer, LongExternalizer).read(input)
+            constantsMap = MapExternalizer(StringExternalizer, ConstantValueExternalizer).read(input),
+            inlineFunctionsAndAccessorsMap = MapExternalizer(InlineFunctionOrAccessorExternalizer, LongExternalizer).read(input)
        )
    }
 }
@@ -9,9 +9,10 @@ import org.jetbrains.kotlin.build.report.metrics.BuildMetricsReporter
 import org.jetbrains.kotlin.build.report.metrics.BuildTime
 import org.jetbrains.kotlin.build.report.metrics.DoNothingBuildMetricsReporter
 import org.jetbrains.kotlin.build.report.metrics.measure
+import org.jetbrains.kotlin.incremental.DifferenceCalculatorForPackageFacade.Companion.getNonPrivateMembers
 import org.jetbrains.kotlin.incremental.KotlinClassInfo
+import org.jetbrains.kotlin.incremental.PackagePartProtoData
 import org.jetbrains.kotlin.incremental.classpathDiff.ClassSnapshotGranularity.CLASS_MEMBER_LEVEL
-import org.jetbrains.kotlin.incremental.getNonPrivateMemberNames
 import org.jetbrains.kotlin.incremental.md5
 import org.jetbrains.kotlin.incremental.storage.toByteArray
 import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader.Kind.*
@@ -96,7 +97,7 @@ object ClassSnapshotter {
            )
            FILE_FACADE, MULTIFILE_CLASS_PART -> PackageFacadeKotlinClassSnapshot(
                classId, classAbiHash, classMemberLevelSnapshot,
-                packageMemberNames = kotlinClassInfo.protoData.getNonPrivateMemberNames(includeInlineAccessors = true).toSet()
+                packageMemberNames = (kotlinClassInfo.protoData as PackagePartProtoData).getNonPrivateMembers().toSet()
            )
            MULTIFILE_CLASS -> MultifileClassKotlinClassSnapshot(
                classId, classAbiHash, classMemberLevelSnapshot,
@@ -223,11 +223,9 @@ class KotlinOnlyClasspathChangesComputerTest : ClasspathChangesComputerTest() {

                LookupSymbol(name = "inlineProperty_ChangedType", scope = "com.example"),
                LookupSymbol(name = "inlineProperty_ChangedType_BackingField", scope = "com.example"),
-                LookupSymbol(name = "getInlineProperty_ChangedType", scope = "com.example"),
-                LookupSymbol(name = "setInlineProperty_ChangedType", scope = "com.example"),

-                LookupSymbol(name = "getInlineProperty_ChangedGetterImpl", scope = "com.example"),
-                LookupSymbol(name = "setInlineProperty_ChangedSetterImpl", scope = "com.example"),
+                LookupSymbol(name = "inlineProperty_ChangedGetterImpl", scope = "com.example"),
+                LookupSymbol(name = "inlineProperty_ChangedSetterImpl", scope = "com.example"),

                LookupSymbol(name = SAM_LOOKUP_NAME.asString(), scope = "com.example.SomeClass")
            ),
@@ -238,6 +236,26 @@ class KotlinOnlyClasspathChangesComputerTest : ClasspathChangesComputerTest() {
        ).assertEquals(changes)
    }

+    @Test
+    fun testFunctionsAndPropertyAccessorsWithJvmNames() {
+        val changes = computeClasspathChanges(File(testDataDir, "KotlinOnly/testFunctionsAndPropertyAccessorsWithJvmNames/src"), tmpDir)
+        Changes(
+            lookupSymbols = setOf(
+                LookupSymbol(name = "changedFunction", scope = "com.example.SomeClass"),
+                LookupSymbol(name = "changedPropertyAccessor", scope = "com.example.SomeClass"),
+
+                LookupSymbol(name = "changedInlineFunction", scope = "com.example"),
+                LookupSymbol(name = "changedInlinePropertyAccessor", scope = "com.example"),
+
+                LookupSymbol(name = SAM_LOOKUP_NAME.asString(), scope = "com.example.SomeClass"),
+            ),
+            fqNames = setOf(
+                "com.example",
+                "com.example.SomeClass"
+            )
+        ).assertEquals(changes)
+    }
+
    /** Tests [SupertypesInheritorsImpact]. */
    @Test
    override fun testImpactComputation_SupertypesInheritors() {
@@ -0,0 +1,20 @@
+@file:Suppress("NOTHING_TO_INLINE")
+
+package com.example
+
+class SomeClass {
+
+    @JvmName("changedFunctionJvmName")
+    fun changedFunction(): Long = 0
+
+    val changedPropertyAccessor: Long
+        @JvmName("changedPropertyAccessorJvmName")
+        get() = 0
+}
+
+@JvmName("changedInlineFunctionJvmName")
+inline fun changedInlineFunction(): Long = 0
+
+inline val changedInlinePropertyAccessor: Long
+    @JvmName("changedInlinePropertyAccessorJvmName")
+    get() = 0
@@ -0,0 +1,20 @@
+@file:Suppress("NOTHING_TO_INLINE")
+
+package com.example
+
+class SomeClass {
+
+    @JvmName("changedFunctionJvmName")
+    fun changedFunction(): Int = 0
+
+    val changedPropertyAccessor: Int
+        @JvmName("changedPropertyAccessorJvmName")
+        get() = 0
+}
+
+@JvmName("changedInlineFunctionJvmName")
+inline fun changedInlineFunction(): Int = 0
+
+inline val changedInlinePropertyAccessor: Int
+    @JvmName("changedInlinePropertyAccessorJvmName")
+    get() = 0