diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalJvmCompilerRunner.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalJvmCompilerRunner.kt index b300724c62b..f26020e13cd 100644 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalJvmCompilerRunner.kt +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/IncrementalJvmCompilerRunner.kt @@ -259,7 +259,7 @@ class IncrementalJvmCompilerRunner( shrunkCurrentClasspathAgainstPreviousLookups!!, classpathChanges.classpathSnapshotFiles.shrunkPreviousClasspathSnapshotFile, reporter - ).getChanges() + ).toChangesEither() } is NotAvailableDueToMissingClasspathSnapshot -> ChangesEither.Unknown(BuildAttribute.CLASSPATH_SNAPSHOT_NOT_FOUND) is NotAvailableForNonIncrementalRun -> ChangesEither.Unknown(BuildAttribute.UNKNOWN_CHANGES_IN_GRADLE_INPUTS) diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ChangeSet.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ChangeSet.kt deleted file mode 100644 index 1b57037b7b3..00000000000 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ChangeSet.kt +++ /dev/null @@ -1,109 +0,0 @@ -/* - * 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.ChangesEither -import org.jetbrains.kotlin.incremental.LookupSymbol -import org.jetbrains.kotlin.name.ClassId -import org.jetbrains.kotlin.name.FqName - -/** Set of classes, class members, and top-level members that are changed (or impacted by a change). */ -class ChangeSet( - - /** Set of changed classes, preferably ordered by not required. */ - val changedClasses: Set, - - /** - * Map from a [ClassId] to the names of changed Java fields/methods or Kotlin properties/functions within that class. - * - * The map and sets are preferably ordered but not required. - * - * The [ClassId]s must not appear in [changedClasses] to avoid redundancy. - */ - val changedClassMembers: Map>, - - /** Map from a package name to the names of changed Kotlin top-level properties/functions within that package. */ - val changedTopLevelMembers: Map> -) { - init { - check(changedClassMembers.keys.none { it in changedClasses }) - } - - class Collector { - private val changedClasses = mutableSetOf() - private val changedClassMembers = mutableMapOf>() - private val changedTopLevelMembers = mutableMapOf>() - - fun addChangedClasses(classNames: Collection) = changedClasses.addAll(classNames) - - fun addChangedClass(className: ClassId) = changedClasses.add(className) - - fun addChangedClassMembers(className: ClassId, memberNames: Collection) { - if (memberNames.isNotEmpty()) { - changedClassMembers.computeIfAbsent(className) { mutableSetOf() }.addAll(memberNames) - } - } - - fun addChangedClassMember(className: ClassId, memberName: String) = addChangedClassMembers(className, listOf(memberName)) - - fun addChangedTopLevelMembers(packageMemberSet: PackageMemberSet) { - packageMemberSet.packageToMembersMap.forEach { (packageName, memberNames) -> - changedTopLevelMembers.computeIfAbsent(packageName) { mutableSetOf() }.addAll(memberNames) - } - } - - fun addChangedTopLevelMembers(packageName: FqName, topLevelMembers: Collection) { - if (topLevelMembers.isNotEmpty()) { - changedTopLevelMembers.computeIfAbsent(packageName) { mutableSetOf() }.addAll(topLevelMembers) - } - } - - fun addChangedTopLevelMember(packageName: FqName, topLevelMember: String) = - addChangedTopLevelMembers(packageName, listOf(topLevelMember)) - - fun getChanges(): ChangeSet { - // Remove redundancy in the change set first - changedClassMembers.keys.intersect(changedClasses).forEach { changedClassMembers.remove(it) } - return ChangeSet(changedClasses.toSet(), changedClassMembers.toMap(), changedTopLevelMembers.toMap()) - } - } - - fun isEmpty() = changedClasses.isEmpty() && changedClassMembers.isEmpty() && changedTopLevelMembers.isEmpty() - - operator fun plus(other: ChangeSet) = ChangeSet( - changedClasses + other.changedClasses, - changedClassMembers + other.changedClassMembers, - changedTopLevelMembers + other.changedTopLevelMembers - ) - - internal fun getChanges(): ChangesEither.Known { - val lookupSymbols = mutableSetOf() - val fqNames = mutableSetOf() - - changedClasses.forEach { - val classFqName = it.asSingleFqName() - lookupSymbols.add(LookupSymbol(name = classFqName.shortName().asString(), scope = classFqName.parent().asString())) - fqNames.add(classFqName) - } - - for ((changedClass, changedClassMembers) in changedClassMembers) { - val classFqName = changedClass.asSingleFqName() - changedClassMembers.forEach { - lookupSymbols.add(LookupSymbol(name = it, scope = classFqName.asString())) - } - fqNames.add(classFqName) - } - - for ((changedPackage, changedTopLevelMembers) in changedTopLevelMembers) { - changedTopLevelMembers.forEach { - lookupSymbols.add(LookupSymbol(name = it, scope = changedPackage.asString())) - } - fqNames.add(changedPackage) - } - - return ChangesEither.Known(lookupSymbols, fqNames) - } -} 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 0dedebb8109..1c0acb59e36 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 @@ -10,13 +10,13 @@ import org.jetbrains.kotlin.build.report.metrics.BuildMetricsReporter import org.jetbrains.kotlin.build.report.metrics.BuildTime import org.jetbrains.kotlin.build.report.metrics.measure import org.jetbrains.kotlin.incremental.* -import org.jetbrains.kotlin.incremental.classpathDiff.ImpactAnalysis.computeImpactedSet +import org.jetbrains.kotlin.incremental.classpathDiff.ImpactAnalysis.computeImpactedSetInclusive import org.jetbrains.kotlin.incremental.storage.FileToCanonicalPathConverter import org.jetbrains.kotlin.incremental.storage.ListExternalizer import org.jetbrains.kotlin.incremental.storage.loadFromFile import org.jetbrains.kotlin.name.ClassId -import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.resolve.jvm.JvmClassName +import org.jetbrains.kotlin.resolve.sam.SAM_LOOKUP_NAME import java.io.File import java.util.* @@ -33,7 +33,7 @@ object ClasspathChangesComputer { shrunkCurrentClasspathSnapshot: List, shrunkPreviousClasspathSnapshotFile: File, metrics: BuildMetricsReporter - ): ChangeSet { + ): ProgramSymbolSet { val shrunkPreviousClasspathSnapshot = metrics.measure(BuildTime.LOAD_SHRUNK_PREVIOUS_CLASSPATH_SNAPSHOT) { ListExternalizer(AccessibleClassSnapshotExternalizer).loadFromFile(shrunkPreviousClasspathSnapshotFile) } @@ -51,7 +51,7 @@ object ClasspathChangesComputer { currentClassSnapshots: List, previousClassSnapshots: List, metrics: BuildMetricsReporter - ): ChangeSet { + ): ProgramSymbolSet { val currentClasses: Map = currentClassSnapshots.associateBy { it.classId } val previousClasses: Map = previousClassSnapshots.associateBy { it.classId } @@ -78,7 +78,19 @@ object ClasspathChangesComputer { } return metrics.measure(BuildTime.COMPUTE_IMPACTED_SET) { - computeImpactedSet(classChanges, previousClassSnapshots) + // Note that changes may contain added symbols (they can also impact recompilation -- see examples in JavaClassChangesComputer). + // So ideally, the result should be: + // computeImpactedSetInclusive(changes = changesOnPreviousClasspath, allClasses = classesOnPreviousClasspath) + + // computeImpactedSetInclusive(changes = changesOnCurrentClasspath, allClasses = classesOnCurrentClasspath) + // However, here we only have the combined changes on both the previous and current classpath, and because it's okay to + // over-approximate the result, we will modify the above computation into: + // computeImpactedSetInclusive(changes, allClasses = classesOnPreviousClasspath + classesOnCurrentClasspath) + // Note: `allClasses` may contain overlapping ClassIds, but it won't be an issue. Also, we will replace + // classesOnCurrentClasspath with changedClassesOnCurrentClasspath to avoid listing unchanged classes twice. + computeImpactedSetInclusive( + changes = classChanges, + allClasses = (previousClassSnapshots.asSequence() + changedCurrentClasses.asSequence()).asIterable() + ) } } @@ -92,7 +104,7 @@ object ClasspathChangesComputer { currentClassSnapshots: List, previousClassSnapshots: List, metrics: BuildMetricsReporter - ): ChangeSet { + ): ProgramSymbolSet { val (currentKotlinClassSnapshots, currentJavaClassSnapshots) = currentClassSnapshots.partition { it is KotlinClassSnapshot } val (previousKotlinClassSnapshots, previousJavaClassSnapshots) = previousClassSnapshots.partition { it is KotlinClassSnapshot } @@ -118,7 +130,7 @@ object ClasspathChangesComputer { private fun computeKotlinClassChanges( currentClassSnapshots: List, previousClassSnapshots: List - ): ChangeSet { + ): ProgramSymbolSet { val (coarseGrainedCurrentClassSnapshots, fineGrainedCurrentClassSnapshots) = currentClassSnapshots.partition { it.classMemberLevelSnapshot == null } val (coarseGrainedPreviousClassSnapshots, fineGrainedPreviousClassSnapshots) = @@ -131,24 +143,24 @@ object ClasspathChangesComputer { private fun computeCoarseGrainedKotlinClassChanges( currentClassSnapshots: List, previousClassSnapshots: List - ): ChangeSet { + ): ProgramSymbolSet { // Note: We have removed unchanged classes earlier in computeChangedAndImpactedSet method, so here we only have changed classes. - return ChangeSet.Collector().run { + return ProgramSymbolSet.Collector().run { (currentClassSnapshots + previousClassSnapshots).forEach { when (it) { - is RegularKotlinClassSnapshot -> addChangedClass(it.classId) - is PackageFacadeKotlinClassSnapshot -> addChangedTopLevelMembers(it.packageMembers) - is MultifileClassKotlinClassSnapshot -> addChangedTopLevelMembers(it.constants) + is RegularKotlinClassSnapshot -> addClass(it.classId) + is PackageFacadeKotlinClassSnapshot -> addPackageMembers(it.classId.packageFqName, it.packageMemberNames) + is MultifileClassKotlinClassSnapshot -> addPackageMembers(it.classId.packageFqName, it.constantNames) } } - getChanges() + getResult() } } private fun computeFineGrainedKotlinClassChanges( currentClassSnapshots: List, previousClassSnapshots: List - ): ChangeSet { + ): ProgramSymbolSet { val workingDir = FileUtil.createTempDirectory(this::class.java.simpleName, "_WorkingDir_${UUID.randomUUID()}", /* deleteOnExit */ true) val incrementalJvmCache = IncrementalJvmCache(workingDir, /* targetOutputDir */ null, FileToCanonicalPathConverter) @@ -196,87 +208,114 @@ object ClasspathChangesComputer { val dirtyData = changesCollector.getDirtyData(listOf(incrementalJvmCache), EmptyICReporter) workingDir.deleteRecursively() - return dirtyData.normalize(currentClassSnapshots, previousClassSnapshots) + return dirtyData.toProgramSymbols(currentClassSnapshots, previousClassSnapshots) } - private fun DirtyData.normalize( + private fun DirtyData.toProgramSymbols( currentClassSnapshots: List, previousClassSnapshots: List - ): ChangeSet { - val changedLookupSymbols = - dirtyLookupSymbols.filterLookupSymbols(currentClassSnapshots).toSet() + - dirtyLookupSymbols.filterLookupSymbols(previousClassSnapshots) + ): ProgramSymbolSet { + // Note that dirtyLookupSymbols may contain added symbols (they can also impact recompilation -- see examples in + // JavaClassChangesComputer). Therefore, we need to consider classes on both the previous and current classpath. The combined list + // may contain overlapping ClassIds, but it won't be an issue. + val allClasses = (previousClassSnapshots.asSequence() + currentClassSnapshots.asSequence()).asIterable() + return dirtyLookupSymbols.toProgramSymbolSet(allClasses).also { + checkDirtyDataNormalization(this, it) + } + } - val changes = ChangeSet.Collector().run { - changedLookupSymbols.forEach { - when (it) { - is ClassSymbol -> addChangedClass(it.classId) - is ClassMember -> addChangedClassMember(it.classId, it.memberName) - is PackageMember -> addChangedTopLevelMember(it.packageFqName, it.memberName) - } - } - getChanges() + /** + * Checks whether [DirtyData.toProgramSymbols] completed without any inconsistencies. + * + * Specifically, [DirtyData] consists of: + * - dirtyLookupSymbols (Collection) + * - dirtyClassesFqNamesForceRecompile (Collection) + * + * In [DirtyData.toProgramSymbols], we converted only dirtyLookupSymbols to [ProgramSymbol]s as dirtyLookupSymbols should contain all + * the changes. + * + * In the following, we'll check that: + * 1. There are no items in dirtyLookupSymbols that have not yet been converted to [ProgramSymbol]s. + * 2. dirtyClassesFqNames and dirtyClassesFqNamesForceRecompile do not contain new information that can't be derived from + * dirtyLookupSymbols. + */ + private fun checkDirtyDataNormalization(dirtyData: DirtyData, programSymbols: ProgramSymbolSet) { + val changes = programSymbols.toChangesEither() + + val unmatchedLookupSymbols = dirtyData.dirtyLookupSymbols.toMutableSet().also { + it.removeAll(changes.lookupSymbols.toSet()) + } + val unmatchedFqNames = (dirtyData.dirtyClassesFqNames + dirtyData.dirtyClassesFqNamesForceRecompile).toMutableSet().also { + it.removeAll(changes.fqNames.toSet()) } - // DirtyData contains: - // 1. dirtyLookupSymbols => This contains all info we need (extracted above). - // 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 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" } + // Some LookupSymbols (reported by IncrementalJvmCache) are invalid. Examples: + // - LookupSymbol(name=, scope=com.example) (detected by + // KotlinOnlyClasspathChangesComputerTest.testTopLevelMembers): SAM-CONSTRUCTOR should have a class scope not a package scope. + // - LookupSymbol(name=BarUseABKt, scope=bar) (detected by + // IncrementalCompilationClasspathSnapshotJvmMultiProjectIT.testMoveFunctionFromLibToApp): The name of a LookupSymbol should not + // end with "Kt" (unless there is a Kotlin class (CLASS kind) whose name ends with "Kt", which is almost never the case). + // Ignore these for now. + // TODO: Fix them later + if (unmatchedLookupSymbols.isNotEmpty()) { + unmatchedLookupSymbols.removeAll { it.name == SAM_LOOKUP_NAME.asString() || it.name.endsWith("Kt") } + } + if (unmatchedFqNames.isNotEmpty()) { + unmatchedFqNames.removeAll { it.asString().endsWith("Kt") } } - return changes + check(unmatchedLookupSymbols.isEmpty()) { + "The following LookupSymbols are not yet converted to ProgramSymbols: ${unmatchedLookupSymbols.joinToString(", ")}" + } + check(unmatchedFqNames.isEmpty()) { + "The following FqNames are not found in DirtyData.dirtyLookupSymbols: ${unmatchedFqNames.joinToString(", ")}.\n" + + "DirtyData = $dirtyData" + } } } internal object ImpactAnalysis { /** - * Computes the set of classes, class members, and top-level members that are impacted by the given changes. + * Computes the set of [ProgramSymbol]s that are impacted by the given changes. * * For example, if a superclass has changed, any of its subclasses will be impacted even if it has not changed because unchanged source * files in the previous compilation that depended on the subclasses will need to be recompiled. * - * The returned set is also a [ChangeSet], which includes the given changes plus the impacted ones. + * The returned set includes the given changes plus the impacted ones. */ - fun computeImpactedSet(changes: ChangeSet, previousClassSnapshots: List): ChangeSet { - val classIdToSubclasses = getClassIdToSubclassesMap(previousClassSnapshots) + fun computeImpactedSetInclusive(changes: ProgramSymbolSet, allClasses: Iterable): ProgramSymbolSet { + val classIdToSubclasses = getClassIdToSubclassesMap(allClasses) val impactedClassesResolver = { classId: ClassId -> classIdToSubclasses[classId] ?: emptySet() } - return ChangeSet.Collector().run { - addChangedClasses(findImpactedClassesInclusive(changes.changedClasses, impactedClassesResolver)) - for ((changedClass, changedClassMembers) in changes.changedClassMembers) { - findImpactedClassesInclusive(setOf(changedClass), impactedClassesResolver).forEach { - addChangedClassMembers(it, changedClassMembers) + return ProgramSymbolSet.Collector().run { + addClasses(findImpactedClassesInclusive(changes.classes, impactedClassesResolver)) + + for ((classId, memberNames) in changes.classMembers) { + findImpactedClassesInclusive(setOf(classId), impactedClassesResolver).forEach { impactedClassId -> + addClassMembers(impactedClassId, memberNames) } } - for ((changedPackage, changedClassMembers) in changes.changedTopLevelMembers) { - addChangedTopLevelMembers(changedPackage, changedClassMembers) + + for ((packageFqName, memberNames) in changes.packageMembers) { + addPackageMembers(packageFqName, memberNames) } - getChanges() + + getResult() } } - private fun getClassIdToSubclassesMap(classSnapshots: List): Map> { - val classIds: Set = classSnapshots.map { it.classId }.toSet() // Use Set for presence check + private fun getClassIdToSubclassesMap(allClasses: Iterable): Map> { + val classIds: Set = allClasses.mapTo(mutableSetOf()) { it.classId } // Use Set for presence check val classNameToClassId = classIds.associateBy { JvmClassName.byClassId(it) } val classNameToClassIdResolver = { className: JvmClassName -> classNameToClassId[className] } val classIdToSubclasses = mutableMapOf>() - classSnapshots.forEach { classSnapshot -> - val classId = classSnapshot.classId - classSnapshot.getSupertypes(classNameToClassIdResolver).forEach { supertype -> - // No need to collect supertypes outside the given set of classes (e.g., "java/lang/Object") - if (supertype in classIds) { - classIdToSubclasses.computeIfAbsent(supertype) { mutableSetOf() }.add(classId) - } + allClasses.forEach { classSnapshot -> + // No need to collect supertypes outside the given set of classes (e.g., "java/lang/Object") + classSnapshot.getSupertypes(classNameToClassIdResolver).intersect(classIds).forEach { supertype -> + classIdToSubclasses.getOrPut(supertype) { mutableSetOf() }.add(classSnapshot.classId) } } return classIdToSubclasses @@ -307,16 +346,16 @@ internal object ImpactAnalysis { * @param classIdResolver Resolves the [ClassId] from the [JvmClassName] of a supertype. It may return null if the supertype is outside the * considered set of classes (e.g., "java/lang/Object"). Those supertypes do not need to be included in the returned result. */ -internal fun AccessibleClassSnapshot.getSupertypes(classIdResolver: (JvmClassName) -> ClassId?): List { +internal fun AccessibleClassSnapshot.getSupertypes(classIdResolver: (JvmClassName) -> ClassId?): Set { return when (this) { - is RegularKotlinClassSnapshot -> supertypes.mapNotNull { classIdResolver.invoke(it) } + is RegularKotlinClassSnapshot -> supertypes.mapNotNullTo(mutableSetOf()) { classIdResolver.invoke(it) } is PackageFacadeKotlinClassSnapshot, is MultifileClassKotlinClassSnapshot -> { // These classes may have supertypes (e.g., kotlin/collections/ArraysKt (MULTIFILE_CLASS) extends // kotlin/collections/ArraysKt___ArraysKt (MULTIFILE_CLASS_PART)), but we don't have to use that info during impact analysis // because those inheritors and supertypes should have the same package names, and in package facades only the package names and // member names matter. - emptyList() + emptySet() } - is JavaClassSnapshot -> supertypes.mapNotNull { classIdResolver.invoke(it) } + is JavaClassSnapshot -> supertypes.mapNotNullTo(mutableSetOf()) { classIdResolver.invoke(it) } } } 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 98478fa7088..59a05789422 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 @@ -69,7 +69,7 @@ class PackageFacadeKotlinClassSnapshot( override val classId: ClassId, override val classAbiHash: Long, override val classMemberLevelSnapshot: KotlinClassInfo?, - val packageMembers: PackageMemberSet + val packageMemberNames: Set ) : KotlinClassSnapshot() /** @@ -88,7 +88,7 @@ class MultifileClassKotlinClassSnapshot( override val classId: ClassId, override val classAbiHash: Long, override val classMemberLevelSnapshot: KotlinClassInfo?, - val constants: PackageMemberSet + val constantNames: Set ) : KotlinClassSnapshot() /** [ClassSnapshot] of a Java class. */ 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 f7d73f03267..a138e974bba 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 @@ -133,7 +133,7 @@ object PackageFacadeKotlinClassSnapshotExternalizer : DataExternalizer { } } -object PackageMemberSetExternalizer : DataExternalizer { - - override fun save(output: DataOutput, set: PackageMemberSet) { - MapExternalizer(FqNameExternalizer, SetExternalizer(StringExternalizer)).save(output, set.packageToMembersMap) - } - - override fun read(input: DataInput): PackageMemberSet { - return PackageMemberSet( - packageToMembersMap = MapExternalizer(FqNameExternalizer, SetExternalizer(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 031a7050331..acae8cae45b 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 @@ -37,8 +37,6 @@ object ClasspathSnapshotShrinker { ): List { val lookupSymbols = metrics.getLookupSymbols { lookupStorage.lookupSymbols - .map { LookupSymbol(name = it.name, scope = it.scope) } - .filterLookupSymbols(allClasses) } return shrinkClasses(allClasses, lookupSymbols, metrics) } @@ -46,7 +44,7 @@ object ClasspathSnapshotShrinker { /** Shrinks the given classes by retaining only classes that are referenced by the given lookup symbols. */ fun shrinkClasses( allClasses: List, - lookupSymbols: List, + lookupSymbols: Collection, metrics: MetricsReporter = MetricsReporter() ): List { val referencedClasses = metrics.findReferencedClasses { @@ -64,25 +62,30 @@ object ClasspathSnapshotShrinker { */ private fun findReferencedClasses( allClasses: List, - lookupSymbols: List + lookupSymbolKeys: Collection ): List { - val lookedUpClassIds: Set = lookupSymbols.mapNotNullTo(mutableSetOf()) { - when (it) { - is ClassSymbol -> it.classId - is ClassMember -> it.classId - is PackageMember -> null - } - } - val lookedUpPackageMembers: PackageMemberSet = - lookupSymbols.filterIsInstanceTo>(mutableSetOf()).compact() + // Use LookupSymbolSet for efficiency + val lookupSymbols = + LookupSymbolSet(lookupSymbolKeys.asSequence().map { LookupSymbol(name = it.name, scope = it.scope) }.asIterable()) - return allClasses.filter { - when (it) { - is RegularKotlinClassSnapshot, is JavaClassSnapshot -> it.classId in lookedUpClassIds - is PackageFacadeKotlinClassSnapshot -> it.packageMembers.containsElementsIn(lookedUpPackageMembers) - is MultifileClassKotlinClassSnapshot -> it.constants.containsElementsIn(lookedUpPackageMembers) + val referencedClasses = allClasses.filter { clazz -> + when (clazz) { + is RegularKotlinClassSnapshot, is JavaClassSnapshot -> { + ClassSymbol(clazz.classId).toLookupSymbol() in lookupSymbols + || lookupSymbols.getLookupNamesInScope(clazz.classId.asSingleFqName()).isNotEmpty() + } + is PackageFacadeKotlinClassSnapshot, is MultifileClassKotlinClassSnapshot -> { + val lookupNamesInScope = lookupSymbols.getLookupNamesInScope(clazz.classId.packageFqName) + if (lookupNamesInScope.isEmpty()) return@filter false + val packageMemberNames = when (clazz) { + is PackageFacadeKotlinClassSnapshot -> clazz.packageMemberNames + else -> (clazz as MultifileClassKotlinClassSnapshot).constantNames + } + packageMemberNames.any { it in lookupNamesInScope } + } } } + return referencedClasses } /** @@ -104,13 +107,13 @@ object ClasspathSnapshotShrinker { // No need to collect supertypes outside the given set of classes (e.g., "java/lang/Object") @Suppress("SimpleRedundantLet") classIdToClassSnapshot[classId]?.let { - it.getSupertypes(classNameToClassIdResolver).filterTo(mutableSetOf()) { supertype -> supertype in classIds } + it.getSupertypes(classNameToClassIdResolver).intersect(classIds) } ?: emptySet() } val referencedClassIds = referencedClasses.mapTo(mutableSetOf()) { it.classId } - val transitivelyReferencedClassIds: Set = - ImpactAnalysis.findImpactedClassesInclusive(referencedClassIds, supertypesResolver) // Use Set for presence check + val transitivelyReferencedClassIds: Set = /* Use Set for presence check */ + ImpactAnalysis.findImpactedClassesInclusive(referencedClassIds, supertypesResolver) return allClasses.filter { it.classId in transitivelyReferencedClassIds } } @@ -244,8 +247,6 @@ internal fun shrinkAndSaveClasspathSnapshot( val shrunkClasses = shrinkMode.shrunkCurrentClasspathAgainstPreviousLookups.mapTo(mutableSetOf()) { it.classId } val notYetShrunkClasses = shrinkMode.currentClasspathSnapshot.filter { it.classId !in shrunkClasses } val addedLookupSymbols = shrinkMode.addedLookupSymbols - .map { LookupSymbol(name = it.name, scope = it.scope) } - .filterLookupSymbols(notYetShrunkClasses) val shrunkRemainingClassesAgainstNewLookups = shrinkClasses(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 29b4f2ee0ce..202cfa97f9f 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,9 +5,11 @@ package org.jetbrains.kotlin.incremental.classpathDiff -import org.jetbrains.kotlin.incremental.* import org.jetbrains.kotlin.incremental.ChangesCollector.Companion.getNonPrivateMemberNames -import org.jetbrains.kotlin.incremental.classpathDiff.ClassSnapshotGranularity.* +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.md5 import org.jetbrains.kotlin.incremental.storage.toByteArray import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader.Kind.* import org.jetbrains.kotlin.name.ClassId @@ -72,13 +74,11 @@ object ClassSnapshotter { CLASS -> RegularKotlinClassSnapshot(classId, classAbiHash, classMemberLevelSnapshot, classFile.classInfo.supertypes) FILE_FACADE, MULTIFILE_CLASS_PART -> PackageFacadeKotlinClassSnapshot( classId, classAbiHash, classMemberLevelSnapshot, - packageMembers = PackageMemberSet( - mapOf(classId.packageFqName to (kotlinClassInfo.protoData as PackagePartProtoData).getNonPrivateMemberNames()) - ) + packageMemberNames = (kotlinClassInfo.protoData as PackagePartProtoData).getNonPrivateMemberNames() ) MULTIFILE_CLASS -> MultifileClassKotlinClassSnapshot( classId, classAbiHash, classMemberLevelSnapshot, - constants = PackageMemberSet(mapOf(classId.packageFqName to kotlinClassInfo.constantsMap.keys)) + constantNames = kotlinClassInfo.constantsMap.keys ) SYNTHETIC_CLASS -> error("Unexpected class $classId with class kind ${SYNTHETIC_CLASS.name} (synthetic classes should have been removed earlier)") UNKNOWN -> error("Can't handle class $classId with class kind ${UNKNOWN.name}") @@ -183,7 +183,7 @@ private object DirectoryOrJarContentsReader { val entry = zipInputStream.nextEntry ?: break val unixStyleRelativePath = entry.name if (entryFilter == null || entryFilter(unixStyleRelativePath, entry.isDirectory)) { - relativePathsToContents.computeIfAbsent(unixStyleRelativePath) { zipInputStream.readBytes() } + relativePathsToContents.getOrPut(unixStyleRelativePath) { zipInputStream.readBytes() } } } } diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/JavaClassChangesComputer.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/JavaClassChangesComputer.kt index fd76e279be5..b6e646dc3bc 100644 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/JavaClassChangesComputer.kt +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/JavaClassChangesComputer.kt @@ -9,38 +9,38 @@ import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.resolve.jvm.JvmClassName import org.jetbrains.kotlin.resolve.sam.SAM_LOOKUP_NAME -/** Computes [ChangeSet] between two lists of [JavaClassSnapshot]s .*/ +/** Computes changes between two lists of [JavaClassSnapshot]s .*/ object JavaClassChangesComputer { /** - * Computes [ChangeSet] between two lists of [JavaClassSnapshot]s. + * Computes changes between two lists of [JavaClassSnapshot]s. * - * Each list must not contain duplicate classes (having the same [JvmClassName]/[ClassId]). + * NOTE: Each list of classes must not contain duplicates (having the same [JvmClassName]/[ClassId]). */ fun compute( currentJavaClassSnapshots: List, previousJavaClassSnapshots: List - ): ChangeSet { + ): ProgramSymbolSet { val currentClasses: Map = currentJavaClassSnapshots.associateBy { it.classId } val previousClasses: Map = previousJavaClassSnapshots.associateBy { it.classId } // Note: Added classes can also impact recompilation. // For example, suppose a source file uses `SomeClass` through `*` imports: - // import foo.* // foo.SomeClass is added in the second build - // import bar.* // bar.SomeClass is present in the first build and unchanged in the second build - // In the second build, the source file needs to be recompiled as `SomeClass` is ambiguous now (in this example, the recompilation - // will fail, but recompilation needs to happen). + // import foo.* // `foo.SomeClass` is present in both the first build and the second build + // import bar.* // `bar.SomeClass` is added in the second build + // The addition of `bar.SomeClass` will require the source file to be recompiled as `SomeClass` will be ambiguous. (In this example, + // the recompilation will fail, but recompilation needs to happen.) val addedClasses = currentClasses.keys - previousClasses.keys val removedClasses = previousClasses.keys - currentClasses.keys val unchangedOrModifiedClasses = currentClasses.keys - addedClasses - return ChangeSet.Collector().run { - addChangedClasses(addedClasses) - addChangedClasses(removedClasses) + return ProgramSymbolSet.Collector().run { + addClasses(addedClasses) + addClasses(removedClasses) unchangedOrModifiedClasses.forEach { collectClassChanges(currentClasses[it]!!, previousClasses[it]!!, this) } - getChanges() + getResult() } } @@ -52,7 +52,7 @@ object JavaClassChangesComputer { private fun collectClassChanges( currentClassSnapshot: JavaClassSnapshot, previousClassSnapshot: JavaClassSnapshot, - changes: ChangeSet.Collector + changes: ProgramSymbolSet.Collector ) { if (currentClassSnapshot.classAbiHash == previousClassSnapshot.classAbiHash) return @@ -61,7 +61,7 @@ object JavaClassChangesComputer { if (currentClassSnapshot.classMemberLevelSnapshot.classAbiExcludingMembers.abiHash != previousClassSnapshot.classMemberLevelSnapshot.classAbiExcludingMembers.abiHash ) { - changes.addChangedClass(classId) + changes.addClass(classId) } else { collectClassMemberChanges( classId, @@ -77,7 +77,7 @@ object JavaClassChangesComputer { ) } } else { - changes.addChangedClass(classId) + changes.addClass(classId) } } @@ -86,7 +86,7 @@ object JavaClassChangesComputer { classId: ClassId, currentMemberSnapshots: List, previousMemberSnapshots: List, - changes: ChangeSet.Collector + changes: ProgramSymbolSet.Collector ) { val currentMemberHashes: Map = currentMemberSnapshots.associateBy { it.abiHash } val previousMemberHashes: Map = previousMemberSnapshots.associateBy { it.abiHash } @@ -96,19 +96,20 @@ object JavaClassChangesComputer { // Note: // - Added members can also impact recompilation. For example, suppose a source file calls `foo(1)` where `foo` is defined as: + // fun foo(x: Any) { } // Present in both the first build and the second build // fun foo(x: Int) { } // Added in the second build - // fun foo(x: Any) { } // Present in the first build and unchanged in the second build - // In the second build, the source file needs to be recompiled as `foo(1)` will now resolve to `foo(Int)` instead of `foo(Any)`. + // The addition of `foo(x: Int)` will require the source file to be recompiled as `foo(1)` will now be resolved to `foo(Int)` + // instead of `foo(Any)`. // - Modified members will appear in both addedMembers and removedMembers. // - Multiple members may have the same name (but never the same signature (name + desc) or ABI hash). It's okay to report the // same name multiple times. - changes.addChangedClassMembers(classId, addedMembers.map { currentMemberHashes[it]!!.name }) - changes.addChangedClassMembers(classId, removedMembers.map { previousMemberHashes[it]!!.name }) + changes.addClassMembers(classId, addedMembers.map { currentMemberHashes[it]!!.name }) + changes.addClassMembers(classId, removedMembers.map { previousMemberHashes[it]!!.name }) // TODO: Check whether the condition to add SAM_LOOKUP_NAME below is too broad, and correct it if necessary. // Currently, it matches the logic in ChangesCollector.getDirtyData in buildUtil.kt. if (addedMembers.isNotEmpty() || removedMembers.isNotEmpty()) { - changes.addChangedClassMember(classId, SAM_LOOKUP_NAME.asString()) + changes.addClassMember(classId, SAM_LOOKUP_NAME.asString()) } } } 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 index 5484a7886f5..d217fd378e8 100644 --- 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 @@ -5,6 +5,7 @@ package org.jetbrains.kotlin.incremental.classpathDiff +import org.jetbrains.kotlin.incremental.ChangesEither import org.jetbrains.kotlin.incremental.LookupSymbol import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.FqName @@ -20,97 +21,169 @@ data class ClassMember(val classId: ClassId, val memberName: String) : ProgramSy data class PackageMember(val packageFqName: FqName, val memberName: String) : ProgramSymbol() -/** Compact representation for a set of [PackageMember]s. */ -class PackageMemberSet(val packageToMembersMap: Map>) { +/** Compact representation for a set of [ProgramSymbol]s. */ +class ProgramSymbolSet private constructor( - operator fun contains(other: PackageMember): Boolean { - return packageToMembersMap[other.packageFqName]?.let { other.memberName in it } ?: false + /** Compact set of [ClassSymbol]s. */ + val classes: Set, + + /** Compact set of [ClassMember]s (map from a [ClassId] to the class members' names). */ + val classMembers: Map>, + + /** Compact set of [PackageMember]s (map from a package's [FqName] to the package members' names). */ + val packageMembers: Map> +) { + + fun isEmpty() = classes.isEmpty() && classMembers.all { it.value.isEmpty() } && packageMembers.all { it.value.isEmpty() } + + operator fun plus(other: ProgramSymbolSet): ProgramSymbolSet { + return Collector().run { + addClasses(classes) + addClasses(other.classes) + + classMembers.forEach { addClassMembers(it.key, it.value) } + other.classMembers.forEach { addClassMembers(it.key, it.value) } + + packageMembers.forEach { addPackageMembers(it.key, it.value) } + other.packageMembers.forEach { addPackageMembers(it.key, it.value) } + + getResult() + } } - fun containsElementsIn(other: PackageMemberSet): Boolean { - return this.packageToMembersMap.keys.intersect(other.packageToMembersMap.keys).any { commonPackage -> - val otherMembers = other.packageToMembersMap[commonPackage]!! - this.packageToMembersMap[commonPackage]!!.any { it in otherMembers } + fun asSequence(): Sequence { + return classes.asSequence().map { ClassSymbol(it) } + + classMembers.asSequence().flatMap { (classId, memberNames) -> memberNames.asSequence().map { ClassMember(classId, it) } } + + packageMembers.asSequence() + .flatMap { (packageFqName, memberNames) -> memberNames.asSequence().map { PackageMember(packageFqName, it) } } + } + + /** + * Collects [ProgramSymbol]s and returns a [ProgramSymbolSet]. + * + * NOTE: If both class `Foo` and class member `Foo.bar` are to be collected, only class `Foo` will be kept in the resulting + * [ProgramSymbolSet] to avoid redundancy. + */ + class Collector { + private val classes = mutableSetOf() + private val classMembers = mutableMapOf>() + private val packageMembers = mutableMapOf>() + + fun addClass(classId: ClassId) { + classMembers.remove(classId) + classes.add(classId) } + + fun addClassMembers(classId: ClassId, memberNames: Collection) { + if (classId !in classes && memberNames.isNotEmpty()) { + classMembers.getOrPut(classId) { mutableSetOf() }.addAll(memberNames) + } + } + + fun addPackageMembers(packageFqName: FqName, memberNames: Collection) { + if (memberNames.isNotEmpty()) { + packageMembers.getOrPut(packageFqName) { mutableSetOf() }.addAll(memberNames) + } + } + + fun addClasses(classIds: Collection) = classIds.forEach { addClass(it) } + + fun addClassMember(classId: ClassId, memberName: String) = addClassMembers(classId, setOf(memberName)) + + fun getResult() = ProgramSymbolSet(classes, classMembers, packageMembers) } } -fun Set.compact(): PackageMemberSet { - val map = mutableMapOf>() - forEach { - map.getOrPut(it.packageFqName) { mutableSetOf() }.add(it.memberName) - } - return PackageMemberSet(map) -} +/** Compact representation for a set of [LookupSymbol]s. It also allows O(1) operation for [getLookupNamesInScope]. */ +class LookupSymbolSet(lookupSymbols: Iterable) { -fun List.combine(): PackageMemberSet { - val combinedMap = mutableMapOf>() - forEach { - it.packageToMembersMap.forEach { (packageFqName, memberNames) -> - combinedMap.getOrPut(packageFqName) { mutableSetOf() }.addAll(memberNames) + private val scopeToLookupNames: Map> = mutableMapOf>().also { map -> + lookupSymbols.forEach { + map.getOrPut(FqName(it.scope)) { mutableSetOf() }.add(it.name) } } - return PackageMemberSet(combinedMap) + + fun getLookupNamesInScope(scope: FqName): Set = scopeToLookupNames[scope] ?: emptySet() + + operator fun contains(lookupSymbol: LookupSymbol): Boolean { + return scopeToLookupNames[FqName(lookupSymbol.scope)]?.contains(lookupSymbol.name) ?: false + } +} + +internal fun ProgramSymbol.toLookupSymbol(): LookupSymbol { + return when (this) { + is ClassSymbol -> classId.asSingleFqName().let { + LookupSymbol(name = it.shortName().asString(), scope = it.parent().asString()) + } + is ClassMember -> LookupSymbol(name = memberName, scope = classId.asSingleFqName().asString()) + is PackageMember -> LookupSymbol(name = memberName, scope = packageFqName.asString()) + } } /** - * Finds [LookupSymbol]s that potentially refer to classes on the given classpath, and returns the [ProgramSymbol]s corresponding to those - * [LookupSymbol]s. + * Converts [LookupSymbol]s to [ProgramSymbol]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. + * Since [LookupSymbol]s are ambiguous, we need to use the given classes to disambiguate them. * - * The given classpath must not contain duplicate classes. + * A [LookupSymbol] may be converted to more than one [ProgramSymbol]. For example, given this class: + * class Foo { + * class Bar + * fun Bar(x: Int) {} + * } + * LookupSymbol(scope = "Foo", name = "Bar") will be converted to both ClassSymbol("Foo.Bar") and ClassMember("Foo", "Bar"). * - * It's okay to over-approximate the result. + * If a [LookupSymbol] does not refer to any symbols in the given classes, it will be ignored. + * + * Note: It's okay to over-approximate the result. */ -internal fun Collection.filterLookupSymbols(classpath: List): List { - val regularClassesOnClasspath: List = classpath.mapNotNull { - when (it) { - is RegularKotlinClassSnapshot -> it.classId - is JavaClassSnapshot -> it.classId - is PackageFacadeKotlinClassSnapshot, is MultifileClassKotlinClassSnapshot -> null +internal fun Collection.toProgramSymbolSet(allClasses: Iterable): ProgramSymbolSet { + // Use LookupSymbolSet for efficiency + val lookupSymbols = LookupSymbolSet(this) + + val collector = ProgramSymbolSet.Collector() + allClasses.forEach { clazz -> + when (clazz) { + is RegularKotlinClassSnapshot, is JavaClassSnapshot -> { + // Collect ClassSymbols + if (ClassSymbol(clazz.classId).toLookupSymbol() in lookupSymbols) { + collector.addClass(clazz.classId) + } + + // Collect ClassMembers + // We want to get the intersection of clazz.classMemberNames and lookupNamesInScope. However, we currently don't store + // information about clazz.classMemberNames, so we'll take all of lookupNamesInScope (it's okay to over-approximate the + // result). + val lookupNamesInScope = lookupSymbols.getLookupNamesInScope(clazz.classId.asSingleFqName()) + collector.addClassMembers(clazz.classId, lookupNamesInScope) + } + is PackageFacadeKotlinClassSnapshot, is MultifileClassKotlinClassSnapshot -> { + // Collect PackageMembers + val lookupNamesInScope = lookupSymbols.getLookupNamesInScope(clazz.classId.packageFqName) + if (lookupNamesInScope.isEmpty()) return@forEach + val packageMemberNames = when (clazz) { + is PackageFacadeKotlinClassSnapshot -> clazz.packageMemberNames + else -> (clazz as MultifileClassKotlinClassSnapshot).constantNames + } + collector.addPackageMembers(clazz.classId.packageFqName, packageMemberNames.intersect(lookupNamesInScope)) + } } } - val packageMembersOnClasspath: PackageMemberSet = classpath.mapNotNull { - when (it) { - is RegularKotlinClassSnapshot, is JavaClassSnapshot -> null - is PackageFacadeKotlinClassSnapshot -> it.packageMembers - is MultifileClassKotlinClassSnapshot -> it.constants - } - }.combine() - - // 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 - } + return collector.getResult() +} + +internal fun ProgramSymbolSet.toChangesEither(): ChangesEither.Known { + val lookupSymbols = mutableSetOf() + val fqNames = mutableSetOf() + + asSequence().forEach { + lookupSymbols.add(it.toLookupSymbol()) + val fqName = when (it) { + is ClassSymbol -> it.classId.asSingleFqName() + is ClassMember -> it.classId.asSingleFqName() + is PackageMember -> it.packageFqName + } + fqNames.add(fqName) + } + + return ChangesEither.Known(lookupSymbols, fqNames) } 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 1dff5f0775e..74bfcda35b5 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 @@ -444,8 +444,8 @@ private fun computeClasspathChanges( /** Adapted version of [ChangesEither.Known] for readability in this test. */ private data class Changes(val lookupSymbols: Set, val fqNames: Set) -private fun ChangeSet.normalize(): Changes { - val changes: ChangesEither.Known = getChanges() +private fun ProgramSymbolSet.normalize(): Changes { + val changes: ChangesEither.Known = toChangesEither() return Changes(changes.lookupSymbols.toSet(), changes.fqNames.map { it.asString() }.toSet()) } diff --git a/libraries/tools/kotlin-gradle-plugin/src/testData/kotlin.incremental.useClasspathSnapshot/src/original/com/example/SimpleJavaClass-expected-snapshot.json b/libraries/tools/kotlin-gradle-plugin/src/testData/kotlin.incremental.useClasspathSnapshot/src/original/com/example/SimpleJavaClass-expected-snapshot.json deleted file mode 100644 index e69de29bb2d..00000000000