From 78f10d9142dace0cf964db61f745bf7303caaeef Mon Sep 17 00:00:00 2001 From: Hung Nguyen Date: Mon, 20 Dec 2021 09:37:23 +0000 Subject: [PATCH] KT-45777: Keep only package facades whose members are looked up to reduce the size of the shrunk classpath snapshot further. --- .../kotlin/incremental/ChangesCollector.kt | 23 +++-- .../kotlin/incremental/IncrementalJvmCache.kt | 6 +- .../classpathDiff/ClasspathChangesComputer.kt | 53 ++++-------- .../classpathDiff/ClasspathSnapshot.kt | 12 ++- .../ClasspathSnapshotSerializer.kt | 19 ++++- .../ClasspathSnapshotShrinker.kt | 53 ++++++------ .../classpathDiff/ClasspathSnapshotter.kt | 20 +++-- .../classpathDiff/ProgramSymbol.kt | 85 +++++++++++++++++++ .../ClasspathChangesComputerTest.kt | 5 +- .../classpathDiff/ClasspathSnapshotterTest.kt | 6 +- 10 files changed, 193 insertions(+), 89 deletions(-) create mode 100644 compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ProgramSymbol.kt diff --git a/build-common/src/org/jetbrains/kotlin/incremental/ChangesCollector.kt b/build-common/src/org/jetbrains/kotlin/incremental/ChangesCollector.kt index 8d335a83c47..44b1d485c9c 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/ChangesCollector.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/ChangesCollector.kt @@ -19,7 +19,6 @@ package org.jetbrains.kotlin.incremental import org.jetbrains.kotlin.metadata.ProtoBuf import org.jetbrains.kotlin.metadata.deserialization.Flags import org.jetbrains.kotlin.metadata.deserialization.NameResolver -import org.jetbrains.kotlin.metadata.deserialization.supertypes import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.protobuf.MessageLite import org.jetbrains.kotlin.serialization.deserialization.getClassId @@ -45,11 +44,23 @@ class ChangesCollector { fun ClassProtoData.getNonPrivateMemberNames(): Set { return proto.getNonPrivateNames( nameResolver, + // The types below should match the logic at `DifferenceCalculatorForClass.difference` ProtoBuf.Class::getConstructorList, ProtoBuf.Class::getFunctionList, - ProtoBuf.Class::getPropertyList + ProtoBuf.Class::getPropertyList, + ProtoBuf.Class::getTypeAliasList ) + proto.enumEntryList.map { nameResolver.getString(it.name) } } + + fun PackagePartProtoData.getNonPrivateMemberNames(): Set { + return proto.getNonPrivateNames( + nameResolver, + // The types below should match the logic at `DifferenceCalculatorForPackageFacade.difference` + ProtoBuf.Package::getFunctionList, + ProtoBuf.Package::getPropertyList, + ProtoBuf.Package::getTypeAliasList + ) + } } fun changes(): List { @@ -182,13 +193,7 @@ class ChangesCollector { } private fun PackagePartProtoData.collectAllFromPackage(isRemoved: Boolean) { - val memberNames = - proto.getNonPrivateNames( - nameResolver, - ProtoBuf.Package::getFunctionList, - ProtoBuf.Package::getPropertyList - ) - + val memberNames = getNonPrivateMemberNames() if (isRemoved) { collectRemovedMembers(packageFqName, memberNames) } else { diff --git a/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJvmCache.kt b/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJvmCache.kt index 22e114ce433..78b7c4297bf 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJvmCache.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJvmCache.kt @@ -612,7 +612,7 @@ class KotlinClassInfo constructor( val classKind: KotlinClassHeader.Kind, val classHeaderData: Array, // Can be empty val classHeaderStrings: Array, // Can be empty - val multifileClassName: String?, + val multifileClassName: String?, // Not null iff classKind == KotlinClassHeader.Kind.MULTIFILE_CLASS_PART val constantsMap: LinkedHashMap, val inlineFunctionsMap: LinkedHashMap ) { @@ -637,7 +637,7 @@ class KotlinClassInfo constructor( check(classKind != KotlinClassHeader.Kind.MULTIFILE_CLASS) { "Proto data is not available for KotlinClassHeader.Kind.MULTIFILE_CLASS: $classId" } - protoMapValue.toProtoData(className.packageFqName) + protoMapValue.toProtoData(classId.packageFqName) } companion object { @@ -698,7 +698,7 @@ private class ConstantsClassVisitor : ClassVisitor(Opcodes.API_VERSION) { private class InlineFunctionsClassVisitor( private val inlineFunctionNames: Set, - cv: ClassVisitor // Note: cv must not override the visitMethod (it will not be called with the current implementation below) + cv: ConstantsClassVisitor // Note: cv must not override the visitMethod (it will not be called with the current implementation below) ) : ClassVisitor(Opcodes.API_VERSION, cv) { private val result = LinkedHashMap() diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputer.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputer.kt index 362465c67e0..0480d957a30 100644 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputer.kt +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputer.kt @@ -201,38 +201,16 @@ object ClasspathChangesComputer { } private fun DirtyData.normalize(currentClassSnapshots: List, previousClassSnapshots: List): ChangeSet { - val allClassIds = currentClassSnapshots.map { it.getClassId() }.toSet() + previousClassSnapshots.map { it.getClassId() } - val fqNameToClassId = HashMap(allClassIds.size) - allClassIds.forEach { classId -> - val fqName = classId.asSingleFqName() - check(!fqNameToClassId.contains(fqName)) { - "Ambiguous FqName $fqName corresponds to two different `ClassId`s: ${fqNameToClassId[fqName]} and $classId" - } - fqNameToClassId[fqName] = classId - } + val changedLookupSymbols = + dirtyLookupSymbols.filterLookupSymbols(currentClassSnapshots).toSet() + + dirtyLookupSymbols.filterLookupSymbols(previousClassSnapshots) val changes = ChangeSet.Collector().run { - dirtyLookupSymbols.forEach { - val lookupSymbolFqName = if (it.scope.isEmpty()) FqName(it.name) else FqName("${it.scope}.${it.name}") - val lookupSymbolClassId: ClassId? = fqNameToClassId[lookupSymbolFqName] - if (lookupSymbolClassId != null) { - addChangedClass(lookupSymbolClassId) - } else { - // When lookupSymbolClassId == null, it means that either (1) the LookupSymbol does not refer to a class (it refers to - // a class member or a package-level member), or (2) it refers to a class outside allClassIds. In the following, we - // assume that (1) is the case. - // (2) should typically never happen. In the case that it happens, we will collect incorrect changes, but it's - // acceptable to collect more changes than necessary; also, we do not need to collect changes outside allClassIds, so - // we're not collecting fewer changes than required. - val scopeClassId: ClassId? = fqNameToClassId[FqName(it.scope)] - if (scopeClassId != null) { - addChangedClassMember(scopeClassId, it.name) - } else { - // Similarly, when scopeClassId == null, we assume that LookupSymbol.scope does not refer to a class outside - // allClassIds. It means that the LookupSymbol does not refer to a class member. Therefore, it must refer to a - // package-level member. - addChangedTopLevelMember(FqName(it.scope), it.name) - } + changedLookupSymbols.forEach { + when (it) { + is ClassSymbol -> addChangedClass(it.classId) + is ClassMember -> addChangedClassMember(it.classId, it.memberName) + is PackageMember -> addChangedTopLevelMember(it.packageFqName, it.memberName) } } getChanges() @@ -243,14 +221,13 @@ object ClasspathChangesComputer { // 2. dirtyClassesFqNames => This should be derived from dirtyLookupSymbols. // 3. dirtyClassesFqNamesForceRecompile => Should be irrelevant. // Double-check that the assumption at bullet 2 above is correct. - val changedFqNames: Set = - changes.changedClasses.map { it.asSingleFqName() }.toSet() + - changes.changedClassMembers.keys.map { it.asSingleFqName() } + - changes.changedTopLevelMembers.keys - check(dirtyClassesFqNames.toSet() == changedFqNames) { - "Two sets differ:\n" + - "dirtyClassesFqNames: $dirtyClassesFqNames\n" + - "changedFqNames: $changedFqNames" + val derivedDirtyFqNames: Set = dirtyLookupSymbols.flatMap { + val lookupSymbolFqName = if (it.scope.isEmpty()) FqName(it.name) else FqName("${it.scope}.${it.name}") + val scopeFqName = FqName(it.scope) + listOf(lookupSymbolFqName, scopeFqName) + }.toSet() + (dirtyClassesFqNames - derivedDirtyFqNames).let { + check(it.isEmpty()) { "FqNames found in dirtyClassesFqNames but not in dirtyLookupSymbols: $it" } } return changes diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshot.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshot.kt index 48edb4353c1..48802b1438f 100644 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshot.kt +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshot.kt @@ -55,7 +55,17 @@ class ClassSnapshotWithHash(val classSnapshot: ClassSnapshot, val hash: Long) { /** [ClassSnapshot] of a Kotlin class. */ class KotlinClassSnapshot( val classInfo: KotlinClassInfo, - val supertypes: List + val supertypes: List, + + /** + * Package-level members if this class is a package facade (classInfo.classKind != KotlinClassHeader.Kind.CLASS). + * + * Note that MULTIFILE_CLASS classes do not have proto data, so we can't extract their package-level members even though they are + * package facades. + * + * Therefore, the [packageMembers] property below is not null iff classInfo.classKind !in setOf(CLASS, MULTIFILE_CLASS) + */ + val packageMembers: List? ) : ClassSnapshot() { override fun toString() = classInfo.classId.toString() diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotSerializer.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotSerializer.kt index a70cbd4c0a9..fcda029d986 100644 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotSerializer.kt +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotSerializer.kt @@ -111,12 +111,14 @@ object KotlinClassSnapshotExternalizer : DataExternalizer { override fun save(output: DataOutput, snapshot: KotlinClassSnapshot) { KotlinClassInfoExternalizer.save(output, snapshot.classInfo) ListExternalizer(JvmClassNameExternalizer).save(output, snapshot.supertypes) + NullableValueExternalizer(ListExternalizer(PackageMemberExternalizer)).save(output, snapshot.packageMembers) } override fun read(input: DataInput): KotlinClassSnapshot { return KotlinClassSnapshot( classInfo = KotlinClassInfoExternalizer.read(input), - supertypes = ListExternalizer(JvmClassNameExternalizer).read(input) + supertypes = ListExternalizer(JvmClassNameExternalizer).read(input), + packageMembers = NullableValueExternalizer(ListExternalizer(PackageMemberExternalizer)).read(input) ) } } @@ -146,6 +148,21 @@ object KotlinClassInfoExternalizer : DataExternalizer { } } +object PackageMemberExternalizer : DataExternalizer { + + override fun save(output: DataOutput, packageMember: PackageMember) { + FqNameExternalizer.save(output, packageMember.packageFqName) + StringExternalizer.save(output, packageMember.memberName) + } + + override fun read(input: DataInput): PackageMember { + return PackageMember( + packageFqName = FqNameExternalizer.read(input), + memberName = StringExternalizer.read(input) + ) + } +} + object JavaClassSnapshotExternalizer : DataExternalizer { override fun save(output: DataOutput, snapshot: JavaClassSnapshot) { diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotShrinker.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotShrinker.kt index ca09c6cdd74..8511a454863 100644 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotShrinker.kt +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotShrinker.kt @@ -12,6 +12,7 @@ import org.jetbrains.kotlin.incremental.ClasspathChanges.ClasspathSnapshotEnable import org.jetbrains.kotlin.incremental.ClasspathChanges.ClasspathSnapshotEnabled.NotAvailableDueToMissingClasspathSnapshot import org.jetbrains.kotlin.incremental.ClasspathChanges.ClasspathSnapshotEnabled.NotAvailableForNonIncrementalRun import org.jetbrains.kotlin.incremental.LookupStorage +import org.jetbrains.kotlin.incremental.LookupSymbol import org.jetbrains.kotlin.incremental.classpathDiff.ClasspathSnapshotShrinker.shrink import org.jetbrains.kotlin.incremental.storage.ListExternalizer import org.jetbrains.kotlin.incremental.storage.LookupSymbolKey @@ -19,7 +20,6 @@ import org.jetbrains.kotlin.incremental.storage.loadFromFile import org.jetbrains.kotlin.incremental.storage.saveToFile import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader import org.jetbrains.kotlin.name.ClassId -import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.resolve.jvm.JvmClassName object ClasspathSnapshotShrinker { @@ -34,6 +34,8 @@ object ClasspathSnapshotShrinker { ): List { val lookupSymbols = metrics.measure(BuildTime.GET_LOOKUP_SYMBOLS) { lookupStorage.lookupSymbols + .map { LookupSymbol(it.name, it.scope) } + .filterLookupSymbols(allClasses.map { it.classSnapshot }) } return shrink(allClasses, lookupSymbols, metrics) } @@ -41,7 +43,7 @@ object ClasspathSnapshotShrinker { /** Shrinks the given classes by retaining only classes that are referenced by the given lookup symbols. */ fun shrink( allClasses: List, - lookupSymbols: Collection, + lookupSymbols: List, metrics: BuildMetricsReporter = DoNothingBuildMetricsReporter ): List { val referencedClasses = metrics.measure(BuildTime.FIND_REFERENCED_CLASSES) { @@ -59,28 +61,28 @@ object ClasspathSnapshotShrinker { */ private fun findReferencedClasses( allClasses: List, - lookupSymbols: Collection + lookupSymbols: List ): List { - val potentialClassNamesOfReferencedClasses = - lookupSymbols.flatMap { - val lookupSymbolFqName = if (it.scope.isEmpty()) FqName(it.name) else FqName("${it.scope}.${it.name}") - listOf( - lookupSymbolFqName, // If LookupSymbol refers to a class, the class's FqName will be captured here. - FqName(it.scope) // If LookupSymbol refers to a class member, the class's FqName will be captured here. - ) - }.toSet() // Use Set for presence check - val potentialPackageNamesOfReferencedPackageLevelMembers = - lookupSymbols.map { - FqName(it.scope) // If LookupSymbol refers to a package-level member, the package's FqName will be captured here. - }.toSet() // Use Set for presence check + val lookedUpClassIds: Set = lookupSymbols.mapNotNullTo(mutableSetOf()) { + when (it) { + is ClassSymbol -> it.classId + is ClassMember -> it.classId + is PackageMember -> null + } + } + val lookedUpPackageMembers: Set = lookupSymbols.filterIsInstanceTo(mutableSetOf()) return allClasses.filter { - val classId = it.classSnapshot.getClassId() - - (classId.asSingleFqName() in potentialClassNamesOfReferencedClasses) || - (it.classSnapshot is KotlinClassSnapshot - && it.classSnapshot.classInfo.classKind != KotlinClassHeader.Kind.CLASS - && classId.packageFqName in potentialPackageNamesOfReferencedPackageLevelMembers) + val isPackageFacade = + it.classSnapshot is KotlinClassSnapshot && it.classSnapshot.classInfo.classKind != KotlinClassHeader.Kind.CLASS + if (isPackageFacade) { + // If packageMembers == null (e.g., if classKind == KotlinClassHeader.Kind.MULTIFILE_CLASS -- see + // `KotlinClassSnapshot.packageMembers`'s kdoc), it means that we don't have the information, so we will always include the + // class (it's okay to over-approximate the result). + (it.classSnapshot as KotlinClassSnapshot).packageMembers?.any { member -> member in lookedUpPackageMembers } ?: true + } else { + it.classSnapshot.getClassId() in lookedUpClassIds + } } } @@ -210,9 +212,7 @@ internal fun shrinkAndSaveClasspathSnapshot( if (addedLookupSymbols.isEmpty()) { ShrinkMode.IncrementalNoNewLookups(shrunkCurrentClasspathAgainstPreviousLookups!!) } else { - ShrinkMode.Incremental( - currentClasspathSnapshot!!, shrunkCurrentClasspathAgainstPreviousLookups!!, addedLookupSymbols - ) + ShrinkMode.Incremental(currentClasspathSnapshot!!, shrunkCurrentClasspathAgainstPreviousLookups!!, addedLookupSymbols) } } is NotAvailableDueToMissingClasspathSnapshot, is NotAvailableForNonIncrementalRun -> ShrinkMode.NonIncremental @@ -229,8 +229,11 @@ internal fun shrinkAndSaveClasspathSnapshot( is ShrinkMode.Incremental -> metrics.measure(BuildTime.SHRINK_CURRENT_CLASSPATH_SNAPSHOT_AFTER_COMPILATION) { val shrunkClasses = shrinkMode.shrunkCurrentClasspathAgainstPreviousLookups.map { it.classSnapshot.getClassId() }.toSet() val notYetShrunkClasses = shrinkMode.currentClasspathSnapshot.filter { it.classSnapshot.getClassId() !in shrunkClasses } + val addedLookupSymbols = shrinkMode.addedLookupSymbols + .map { LookupSymbol(it.name, it.scope) } + .filterLookupSymbols(shrinkMode.currentClasspathSnapshot.map { it.classSnapshot }) // Don't provide a BuildMetricsReporter for the following call as the sub-BuildTimes in it have a different parent - val shrunkRemainingClassesAgainstNewLookups = shrink(notYetShrunkClasses, shrinkMode.addedLookupSymbols) + val shrunkRemainingClassesAgainstNewLookups = shrink(notYetShrunkClasses, addedLookupSymbols) shrinkMode.shrunkCurrentClasspathAgainstPreviousLookups + shrunkRemainingClassesAgainstNewLookups } diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotter.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotter.kt index a077624a14b..a68dba8e96f 100644 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotter.kt +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotter.kt @@ -5,12 +5,10 @@ package org.jetbrains.kotlin.incremental.classpathDiff -import org.jetbrains.kotlin.incremental.JavaClassDescriptorCreator -import org.jetbrains.kotlin.incremental.KotlinClassInfo -import org.jetbrains.kotlin.incremental.md5 -import org.jetbrains.kotlin.incremental.toSerializedJavaClass +import org.jetbrains.kotlin.incremental.* +import org.jetbrains.kotlin.incremental.ChangesCollector.Companion.getNonPrivateMemberNames import org.jetbrains.kotlin.load.java.descriptors.JavaClassDescriptor -import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader +import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader.Kind.* import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.utils.DFS import java.io.File @@ -112,7 +110,13 @@ object ClassSnapshotter { return if (classFile.classInfo.isKotlinClass) { val kotlinClassInfo = KotlinClassInfo.createFrom(classFile.classInfo.classId, classFile.classInfo.kotlinClassHeader!!, classFile.contents) - KotlinClassSnapshot(kotlinClassInfo, classFile.classInfo.supertypes) + val packageMembers = when (kotlinClassInfo.classKind) { + CLASS, MULTIFILE_CLASS -> null // See `KotlinClassSnapshot.packageMembers`'s kdoc + else -> (kotlinClassInfo.protoData as PackagePartProtoData).getNonPrivateMemberNames().map { + PackageMember(kotlinClassInfo.classId.packageFqName, it) + } + } + KotlinClassSnapshot(kotlinClassInfo, classFile.classInfo.supertypes, packageMembers) } else null } @@ -172,8 +176,8 @@ object ClassSnapshotter { fun BasicClassInfo.isInaccessible(): Boolean { return if (this.isKotlinClass) { when (this.kotlinClassHeader!!.kind) { - KotlinClassHeader.Kind.CLASS -> isPrivate || isLocal || isAnonymous || isSynthetic - KotlinClassHeader.Kind.SYNTHETIC_CLASS -> true + CLASS -> isPrivate || isLocal || isAnonymous || isSynthetic + SYNTHETIC_CLASS -> true // We're not sure about the other kinds of Kotlin classes, so we assume it's accessible (see this method's kdoc) else -> false } diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ProgramSymbol.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ProgramSymbol.kt new file mode 100644 index 00000000000..bd09ad9b271 --- /dev/null +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ProgramSymbol.kt @@ -0,0 +1,85 @@ +/* + * Copyright 2010-2021 JetBrains s.r.o. and Kotlin Programming Language contributors. + * Use of this source code is governed by the Apache 2.0 license that can be found in the license/LICENSE.txt file. + */ + +package org.jetbrains.kotlin.incremental.classpathDiff + +import org.jetbrains.kotlin.incremental.LookupSymbol +import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader +import org.jetbrains.kotlin.name.ClassId +import org.jetbrains.kotlin.name.FqName + +/** + * Similar to [LookupSymbol] but with information about its kind (i.e., whether it's a [ClassSymbol], [ClassMember], or [PackageMember]). + */ +sealed class ProgramSymbol + +data class ClassSymbol(val classId: ClassId) : ProgramSymbol() + +data class ClassMember(val classId: ClassId, val memberName: String) : ProgramSymbol() + +data class PackageMember(val packageFqName: FqName, val memberName: String) : ProgramSymbol() + +/** + * Finds [LookupSymbol]s that potentially refer to classes on the given classpath, and returns the [ProgramSymbol]s corresponding to those + * [LookupSymbol]s. + * + * Note: Some [LookupSymbol]s may refer to a class outside the classpath (e.g., `java/lang/Object`, or a class in the sources being + * compiled). The returned result will not include those symbols. + * + * The given classpath must not contain duplicate classes. + * + * It's okay if the returned result is an over-approximation. + */ +internal fun Collection.filterLookupSymbols(classpath: List): List { + val (packageFacades, regularClasses) = classpath.partition { + it is KotlinClassSnapshot && it.classInfo.classKind != KotlinClassHeader.Kind.CLASS + } + val regularClassesOnClasspath: List = regularClasses.map { it.getClassId() } + val packageMembersOnClasspath: Set = + packageFacades.flatMap { + if ((it as KotlinClassSnapshot).classInfo.classKind == KotlinClassHeader.Kind.MULTIFILE_CLASS) { + // If classKind == MULTIFILE_CLASS, we don't have the information about its package members (see + // `KotlinClassSnapshot.packageMembers`'s kdoc). However, package members in a MULTIFILE_CLASS should be found in + // MULTIFILE_CLASS_PART classes, so it's okay to ignore MULTIFILE_CLASS here. + emptyList() + } else { + it.packageMembers!! + } + }.toSet() // Use Set for presence check + + // It's rare but possible for 2 ClassIds to have the same FqName (e.g., ClassId `com/example/Foo` and ClassId `com/example.Foo` both + // have FqName `com.example.Foo`. ClassId `com/example/Foo` indicates class `Foo` in package `com/example', whereas ClassId + // `com/example.Foo` indicates nested class `Foo` of class `example` in package `com`). + val fqNameToClassIds: Map> = regularClassesOnClasspath.groupBy { it.asSingleFqName() } + + return this.flatMap { + val lookupSymbolFqName = if (it.scope.isEmpty()) FqName(it.name) else FqName("${it.scope}.${it.name}") + val lookupSymbolClassIds: List = fqNameToClassIds[lookupSymbolFqName] ?: emptyList() + + val scopeFqName = FqName(it.scope) + val scopeClassIds: List = fqNameToClassIds[scopeFqName] ?: emptyList() + + val packageMember = PackageMember(scopeFqName, it.name) + + // A LookupSymbol may refer to one of the following types: + // 1. A class + // 2. A class member + // 3. A package member + // Note: It's also possible that a LookupSymbol may refer to more than one type (e.g., LookupSymbol(scope = "com.example.Foo", + // name = "Bar") may refer to a nested class or a class property/function; LookupSymbol(scope = "com.example", name = "Foo") may + // refer to a class or a package-level property/function). In the following, we will collect all possible cases (it's okay to + // over-approximate the result). + val classSymbols = lookupSymbolClassIds.map { classId -> ClassSymbol(classId) } + + // To check if a LookupSymbol refers to a class member, we'll need to check that (1) its scope refers to a class, and (2) its name + // actually refers to a member of that class. Because checking (2) is expensive, and it's okay to over-approximate the result, we're + // checking (1) only. + val classMembers = scopeClassIds.map { classId -> ClassMember(classId, it.name) } + + val packageMembers = if (packageMember in packageMembersOnClasspath) listOf(packageMember) else emptyList() + + return@flatMap classSymbols + classMembers + packageMembers + } +} diff --git a/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputerTest.kt b/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputerTest.kt index 5960d19f1ed..8c56f227f95 100644 --- a/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputerTest.kt +++ b/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputerTest.kt @@ -118,7 +118,6 @@ class KotlinOnlyClasspathChangesComputerTest : ClasspathChangesComputerTest() { LookupSymbol(name = "addedTopLevelFunction", scope = "com.example"), LookupSymbol(name = "removedTopLevelFunction", scope = "com.example"), LookupSymbol(name = "movedTopLevelFunction", scope = "com.example"), - LookupSymbol(name = SAM_LOOKUP_NAME.asString(), scope = "com.example") ), fqNames = setOf("com.example") ).assertEquals(changes) @@ -154,7 +153,6 @@ class KotlinOnlyClasspathChangesComputerTest : ClasspathChangesComputerTest() { LookupSymbol(name = SAM_LOOKUP_NAME.asString(), scope = "com.example.NormalClass"), LookupSymbol(name = SAM_LOOKUP_NAME.asString(), scope = "com.example.NormalClass.CompanionObject"), LookupSymbol(name = SAM_LOOKUP_NAME.asString(), scope = "com.example.NormalClass.NestedClass"), - LookupSymbol(name = SAM_LOOKUP_NAME.asString(), scope = "com.example"), ), fqNames = setOf( "com.example.NormalClass", @@ -267,6 +265,9 @@ class JavaOnlyClasspathChangesComputerTest(private val protoBased: Boolean) : Cl class KotlinAndJavaClasspathChangesComputerTest : ClasspathSnapshotTestCommon() { + // TODO Add more test cases: + // - Java class converted to Kotlin class + @Test fun testImpactAnalysis() { val changes = diff --git a/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotterTest.kt b/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotterTest.kt index 9691ac2f4d5..b7d20a8b587 100644 --- a/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotterTest.kt +++ b/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotterTest.kt @@ -25,8 +25,10 @@ abstract class ClasspathSnapshotterTest : ClasspathSnapshotTestCommon() { protected abstract val sourceFileWithNonAbiChange: TestSourceFile private val expectedSnapshotFile: File - get() = sourceFile.asFile().path.let { - File(it.substringBeforeLast("src") + "expected-snapshot" + it.substringAfterLast("src").substringBeforeLast('.') + ".json") + get() = sourceFile.asFile().let { + val srcDir = File(it.path.substringBeforeLast("src") + "src") + val relativePath = it.relativeTo(srcDir) + testDataDir.resolve("expected-snapshot").resolve(relativePath.parent).resolve(relativePath.nameWithoutExtension + ".json") } @Test