KT-45777: Clean up handling around LookupSymbols and ProgramSymbols

Make it clearer and more efficient.

^KT-45777 In Progress

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)

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

@@ -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<ClassId>,
-
-    /**
-     * 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<ClassId, Set<String>>,
-
-    /** Map from a package name to the names of changed Kotlin top-level properties/functions within that package. */
-    val changedTopLevelMembers: Map<FqName, Set<String>>
-) {
-    init {
-        check(changedClassMembers.keys.none { it in changedClasses })
-    }
-
-    class Collector {
-        private val changedClasses = mutableSetOf<ClassId>()
-        private val changedClassMembers = mutableMapOf<ClassId, MutableSet<String>>()
-        private val changedTopLevelMembers = mutableMapOf<FqName, MutableSet<String>>()
-
-        fun addChangedClasses(classNames: Collection<ClassId>) = changedClasses.addAll(classNames)
-
-        fun addChangedClass(className: ClassId) = changedClasses.add(className)
-
-        fun addChangedClassMembers(className: ClassId, memberNames: Collection<String>) {
-            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<String>) {
-            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<LookupSymbol>()
-        val fqNames = mutableSetOf<FqName>()
-
-        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)
-    }
-}

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<AccessibleClassSnapshot>,
         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<AccessibleClassSnapshot>,
         previousClassSnapshots: List<AccessibleClassSnapshot>,
         metrics: BuildMetricsReporter
-    ): ChangeSet {
+    ): ProgramSymbolSet {
         val currentClasses: Map<ClassId, AccessibleClassSnapshot> = currentClassSnapshots.associateBy { it.classId }
         val previousClasses: Map<ClassId, AccessibleClassSnapshot> = 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<AccessibleClassSnapshot>,
         previousClassSnapshots: List<AccessibleClassSnapshot>,
         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<KotlinClassSnapshot>,
         previousClassSnapshots: List<KotlinClassSnapshot>
-    ): ChangeSet {
+    ): ProgramSymbolSet {
         val (coarseGrainedCurrentClassSnapshots, fineGrainedCurrentClassSnapshots) =
             currentClassSnapshots.partition { it.classMemberLevelSnapshot == null }
         val (coarseGrainedPreviousClassSnapshots, fineGrainedPreviousClassSnapshots) =
@@ -131,24 +143,24 @@ object ClasspathChangesComputer {
     private fun computeCoarseGrainedKotlinClassChanges(
         currentClassSnapshots: List<KotlinClassSnapshot>,
         previousClassSnapshots: List<KotlinClassSnapshot>
-    ): 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<KotlinClassSnapshot>,
         previousClassSnapshots: List<KotlinClassSnapshot>
-    ): 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<AccessibleClassSnapshot>,
         previousClassSnapshots: List<AccessibleClassSnapshot>
-    ): 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<LookupSymbol)
+     *   - dirtyClassesFqNames (Collection<FqName>)
+     *   - dirtyClassesFqNamesForceRecompile (Collection<FqName>)
+     *
+     * 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<FqName> = 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=<SAM-CONSTRUCTOR>, 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<AccessibleClassSnapshot>): ChangeSet {
-        val classIdToSubclasses = getClassIdToSubclassesMap(previousClassSnapshots)
+    fun computeImpactedSetInclusive(changes: ProgramSymbolSet, allClasses: Iterable<AccessibleClassSnapshot>): 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<AccessibleClassSnapshot>): Map<ClassId, Set<ClassId>> {
-        val classIds: Set<ClassId> = classSnapshots.map { it.classId }.toSet() // Use Set for presence check
+    private fun getClassIdToSubclassesMap(allClasses: Iterable<AccessibleClassSnapshot>): Map<ClassId, Set<ClassId>> {
+        val classIds: Set<ClassId> = 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<ClassId, MutableSet<ClassId>>()
-        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<ClassId> {
+internal fun AccessibleClassSnapshot.getSupertypes(classIdResolver: (JvmClassName) -> ClassId?): Set<ClassId> {
     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) }
     }
 }

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<String>
 ) : 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<String>
 ) : KotlinClassSnapshot()
 
 /** [ClassSnapshot] of a Java class. */

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

@@ -133,7 +133,7 @@ object PackageFacadeKotlinClassSnapshotExternalizer : DataExternalizer<PackageFa
         ClassIdExternalizer.save(output, snapshot.classId)
         LongExternalizer.save(output, snapshot.classAbiHash)
         NullableValueExternalizer(KotlinClassInfoExternalizer).save(output, snapshot.classMemberLevelSnapshot)
-        PackageMemberSetExternalizer.save(output, snapshot.packageMembers)
+        SetExternalizer(StringExternalizer).save(output, snapshot.packageMemberNames)
     }
 
     override fun read(input: DataInput): PackageFacadeKotlinClassSnapshot {
@@ -141,7 +141,7 @@ object PackageFacadeKotlinClassSnapshotExternalizer : DataExternalizer<PackageFa
             classId = ClassIdExternalizer.read(input),
             classAbiHash = LongExternalizer.read(input),
             classMemberLevelSnapshot = NullableValueExternalizer(KotlinClassInfoExternalizer).read(input),
-            packageMembers = PackageMemberSetExternalizer.read(input)
+            packageMemberNames = SetExternalizer(StringExternalizer).read(input)
         )
     }
 }
@@ -152,7 +152,7 @@ object MultifileClassKotlinClassSnapshotExternalizer : DataExternalizer<Multifil
         ClassIdExternalizer.save(output, snapshot.classId)
         LongExternalizer.save(output, snapshot.classAbiHash)
         NullableValueExternalizer(KotlinClassInfoExternalizer).save(output, snapshot.classMemberLevelSnapshot)
-        PackageMemberSetExternalizer.save(output, snapshot.constants)
+        SetExternalizer(StringExternalizer).save(output, snapshot.constantNames)
     }
 
     override fun read(input: DataInput): MultifileClassKotlinClassSnapshot {
@@ -160,7 +160,7 @@ object MultifileClassKotlinClassSnapshotExternalizer : DataExternalizer<Multifil
             classId = ClassIdExternalizer.read(input),
             classAbiHash = LongExternalizer.read(input),
             classMemberLevelSnapshot = NullableValueExternalizer(KotlinClassInfoExternalizer).read(input),
-            constants = PackageMemberSetExternalizer.read(input)
+            constantNames = SetExternalizer(StringExternalizer).read(input)
         )
     }
 }
@@ -190,19 +190,6 @@ object KotlinClassInfoExternalizer : DataExternalizer<KotlinClassInfo> {
     }
 }
 
-object PackageMemberSetExternalizer : DataExternalizer<PackageMemberSet> {
-
-    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<JavaClassSnapshot> {
 
     override fun save(output: DataOutput, snapshot: JavaClassSnapshot) {

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

@@ -37,8 +37,6 @@ object ClasspathSnapshotShrinker {
     ): List<AccessibleClassSnapshot> {
         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<AccessibleClassSnapshot>,
-        lookupSymbols: List<ProgramSymbol>,
+        lookupSymbols: Collection<LookupSymbolKey>,
         metrics: MetricsReporter = MetricsReporter()
     ): List<AccessibleClassSnapshot> {
         val referencedClasses = metrics.findReferencedClasses {
@@ -64,25 +62,30 @@ object ClasspathSnapshotShrinker {
      */
     private fun findReferencedClasses(
         allClasses: List<AccessibleClassSnapshot>,
-        lookupSymbols: List<ProgramSymbol>
+        lookupSymbolKeys: Collection<LookupSymbolKey>
     ): List<AccessibleClassSnapshot> {
-        val lookedUpClassIds: Set<ClassId> = lookupSymbols.mapNotNullTo(mutableSetOf()) {
-            when (it) {
-                is ClassSymbol -> it.classId
-                is ClassMember -> it.classId
-                is PackageMember -> null
-            }
-        }
-        val lookedUpPackageMembers: PackageMemberSet =
-            lookupSymbols.filterIsInstanceTo<PackageMember, MutableSet<PackageMember>>(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<ClassId> =
-            ImpactAnalysis.findImpactedClassesInclusive(referencedClassIds, supertypesResolver) // Use Set for presence check
+        val transitivelyReferencedClassIds: Set<ClassId> = /* 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

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

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<JavaClassSnapshot>,
         previousJavaClassSnapshots: List<JavaClassSnapshot>
-    ): ChangeSet {
+    ): ProgramSymbolSet {
         val currentClasses: Map<ClassId, JavaClassSnapshot> = currentJavaClassSnapshots.associateBy { it.classId }
         val previousClasses: Map<ClassId, JavaClassSnapshot> = 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<JavaElementSnapshot>,
         previousMemberSnapshots: List<JavaElementSnapshot>,
-        changes: ChangeSet.Collector
+        changes: ProgramSymbolSet.Collector
     ) {
         val currentMemberHashes: Map<Long, JavaElementSnapshot> = currentMemberSnapshots.associateBy { it.abiHash }
         val previousMemberHashes: Map<Long, JavaElementSnapshot> = 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())
         }
     }
 }

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<FqName, Set<String>>) {
+/** 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<ClassId>,
+
+    /** Compact set of [ClassMember]s (map from a [ClassId] to the class members' names). */
+    val classMembers: Map<ClassId, Set<String>>,
+
+    /** Compact set of [PackageMember]s (map from a package's [FqName] to the package members' names). */
+    val packageMembers: Map<FqName, Set<String>>
+) {
+
+    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<ProgramSymbol> {
+        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<ClassId>()
+        private val classMembers = mutableMapOf<ClassId, MutableSet<String>>()
+        private val packageMembers = mutableMapOf<FqName, MutableSet<String>>()
+
+        fun addClass(classId: ClassId) {
+            classMembers.remove(classId)
+            classes.add(classId)
         }
+
+        fun addClassMembers(classId: ClassId, memberNames: Collection<String>) {
+            if (classId !in classes && memberNames.isNotEmpty()) {
+                classMembers.getOrPut(classId) { mutableSetOf() }.addAll(memberNames)
+            }
+        }
+
+        fun addPackageMembers(packageFqName: FqName, memberNames: Collection<String>) {
+            if (memberNames.isNotEmpty()) {
+                packageMembers.getOrPut(packageFqName) { mutableSetOf() }.addAll(memberNames)
+            }
+        }
+
+        fun addClasses(classIds: Collection<ClassId>) = classIds.forEach { addClass(it) }
+
+        fun addClassMember(classId: ClassId, memberName: String) = addClassMembers(classId, setOf(memberName))
+
+        fun getResult() = ProgramSymbolSet(classes, classMembers, packageMembers)
     }
 }
 
-fun Set<PackageMember>.compact(): PackageMemberSet {
-    val map = mutableMapOf<FqName, MutableSet<String>>()
-    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<LookupSymbol>) {
 
-fun List<PackageMemberSet>.combine(): PackageMemberSet {
-    val combinedMap = mutableMapOf<FqName, MutableSet<String>>()
-    forEach {
-        it.packageToMembersMap.forEach { (packageFqName, memberNames) ->
-            combinedMap.getOrPut(packageFqName) { mutableSetOf() }.addAll(memberNames)
+    private val scopeToLookupNames: Map<FqName, Set<String>> = mutableMapOf<FqName, MutableSet<String>>().also { map ->
+        lookupSymbols.forEach {
+            map.getOrPut(FqName(it.scope)) { mutableSetOf() }.add(it.name)
         }
     }
-    return PackageMemberSet(combinedMap)
+
+    fun getLookupNamesInScope(scope: FqName): Set<String> = 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<LookupSymbol>.filterLookupSymbols(classpath: List<AccessibleClassSnapshot>): List<ProgramSymbol> {
-    val regularClassesOnClasspath: List<ClassId> = classpath.mapNotNull {
-        when (it) {
-            is RegularKotlinClassSnapshot -> it.classId
-            is JavaClassSnapshot -> it.classId
-            is PackageFacadeKotlinClassSnapshot, is MultifileClassKotlinClassSnapshot -> null
+internal fun Collection<LookupSymbol>.toProgramSymbolSet(allClasses: Iterable<AccessibleClassSnapshot>): 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<FqName, List<ClassId>> = 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<ClassId> = fqNameToClassIds[lookupSymbolFqName] ?: emptyList()
-
-        val scopeFqName = FqName(it.scope)
-        val scopeClassIds: List<ClassId> = 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<LookupSymbol>()
+    val fqNames = mutableSetOf<FqName>()
+
+    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)
 }

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<LookupSymbol>, val fqNames: Set<String>)
 
-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())
 }