New IC: Add constants-in-companion-objects impact computation

When computing impacted symbols of changed symbols, previously we considered only the supertypes-inheritors type of impact, which is the most common type. This commit adds the constants-in-companion-objects type of impact to address KT-53266. We've also cleaned up impact computation to make it easier to add new types of impact in the future. ^KT-53266 In progress
2022-08-05 14:57:15 +01:00
parent 3f0a93d6dd
commit 2ad047340f
13 changed files with 462 additions and 172 deletions
@@ -17,7 +17,6 @@
 package org.jetbrains.kotlin.incremental

 import org.jetbrains.kotlin.metadata.ProtoBuf
-import org.jetbrains.kotlin.metadata.deserialization.Flags
 import org.jetbrains.kotlin.metadata.deserialization.NameResolver
 import org.jetbrains.kotlin.name.FqName
 import org.jetbrains.kotlin.protobuf.MessageLite
@@ -203,9 +202,8 @@ class ChangesCollector {

    private fun ClassProtoData.collectAllFromClass(isRemoved: Boolean, isAdded: Boolean, collectAllMembersForNewClass: Boolean = false) {
        val classFqName = nameResolver.getClassId(proto.fqName).asSingleFqName()
-        val kind = Flags.CLASS_KIND.get(proto.flags)

-        if (kind == ProtoBuf.Class.Kind.COMPANION_OBJECT) {
+        if (proto.isCompanionObject) {
            val memberNames = getNonPrivateMemberNames()

            val collectMember = if (isRemoved) this@ChangesCollector::collectRemovedMember else this@ChangesCollector::collectChangedMember
@@ -455,27 +455,27 @@ open class IncrementalJvmCache(
            val allConstants = oldMap.keys + newMap.keys
            if (allConstants.isEmpty()) return

-            // If a constant is defined in a companion object, it will be found in the constantsMap of the containing class, not the
-            // companion object's class, so we will need to correct its scope.
-            // (See https://youtrack.jetbrains.com/issue/KT-44741#focus=Comments-27-5659564.0-0 for more details.)
-            // Note: This only applies to a *constant* defined in a *companion object* (it's not an issue for inline functions, or top-level
-            // constants, or constants in non-companion objects).
-            val companionObjectClassId = if (kotlinClassInfo.classKind == KotlinClassHeader.Kind.CLASS) {
-                val protoData = kotlinClassInfo.protoData as ClassProtoData
-                if (protoData.proto.hasCompanionObjectName()) {
-                    val companionObjectName = Name.identifier(protoData.nameResolver.getString(protoData.proto.companionObjectName))
-                    kotlinClassInfo.classId.createNestedClassId(companionObjectName)
-                } else null
-            } else null
-            val scope = companionObjectClassId?.asSingleFqName() ?: kotlinClassInfo.scopeFqName()
-
-            // Here we assume that the old and new classes have the same KotlinClassHeader.Kind, so that the scopes of the old and new
-            // constants are the same and their values can be compared.
-            // If the class kinds are different, the changes will be detected when comparing protos (in that case, the changes collected
-            // here will be a subset of those changes).
+            val scope = kotlinClassInfo.scopeFqName()
            for (const in allConstants) {
                changesCollector.collectMemberIfValueWasChanged(scope, const, oldMap[const], newMap[const])
            }
+
+            // If a constant is defined in a companion object of class A, its name and type will be found in the Kotlin metadata of
+            // `A$Companion.class`, but its value will only be found in the Java bytecode code of `A.class` (see
+            // `org.jetbrains.kotlin.incremental.classpathDiff.ConstantsInCompanionObjectImpact` for more details).
+            // Therefore, if the value of `CONSTANT` in `A.class` has changed, we will report that `A.CONSTANT` has changed in the code
+            // above, and report that `A.Companion.CONSTANT` is impacted in the code below.
+            kotlinClassInfo.companionObject?.let { companionObjectClassId ->
+                // Note that `companionObjectClassId` is the companion object of the current class. Here we assume that the previous class
+                // also has a companion object with the same name. If that is not the case, that change will be detected when comparing
+                // protos, and the report below will be imprecise/redundant, but it's okay to over-approximate the result.
+                val companionObjectFqName = companionObjectClassId.asSingleFqName()
+                for (const in allConstants) {
+                    changesCollector.collectMemberIfValueWasChanged(
+                        scope = companionObjectFqName, name = const, oldMap[const], newMap[const]
+                    )
+                }
+            }
        }

        @Synchronized
@@ -688,7 +688,7 @@ class KotlinClassInfo constructor(
    }

    /**
-     * Returns the [ProtoData] of this class.
+     * The [ProtoData] of this class.
     *
     * NOTE: The caller needs to ensure `classKind != KotlinClassHeader.Kind.MULTIFILE_CLASS` first, as the compiler doesn't write proto
     * data to [KotlinClassHeader.Kind.MULTIFILE_CLASS] classes.
@@ -700,6 +700,25 @@ class KotlinClassInfo constructor(
        protoMapValue.toProtoData(classId.packageFqName)
    }

+    /** Name of the companion object of this class (default is "Companion") iff this class HAS a companion object, or null otherwise. */
+    val companionObject: ClassId? by lazy {
+        if (classKind == KotlinClassHeader.Kind.CLASS) {
+            (protoData as ClassProtoData).getCompanionObjectName()?.let {
+                classId.createNestedClassId(Name.identifier(it))
+            }
+        } else null
+    }
+
+    /** List of constants defined in this class iff this class IS a companion object, or null otherwise. The list could be empty. */
+    val constantsInCompanionObject: List<String>? by lazy {
+        if (classKind == KotlinClassHeader.Kind.CLASS) {
+            val classProtoData = protoData as ClassProtoData
+            if (classProtoData.proto.isCompanionObject) {
+                classProtoData.getConstants()
+            } else null
+        } else null
+    }
+
    companion object {

        fun createFrom(kotlinClass: LocalFileKotlinClass): KotlinClassInfo {
@@ -367,8 +367,23 @@ class DifferenceCalculatorForPackageFacade(
 private val ProtoBuf.Class.isSealed: Boolean
    get() = ProtoBuf.Modality.SEALED == Flags.MODALITY.get(flags)

+internal val ProtoBuf.Class.isCompanionObject: Boolean
+    get() = ProtoBuf.Class.Kind.COMPANION_OBJECT == Flags.CLASS_KIND.get(flags)
+
 val ProtoBuf.Class.typeTableOrNull: ProtoBuf.TypeTable?
    get() = if (hasTypeTable()) typeTable else null

 val ProtoBuf.Package.typeTableOrNull: ProtoBuf.TypeTable?
    get() = if (hasTypeTable()) typeTable else null
+
+internal fun ClassProtoData.getCompanionObjectName(): String? {
+    return if (proto.hasCompanionObjectName()) {
+        nameResolver.getString(proto.companionObjectName)
+    } else null
+}
+
+internal fun ClassProtoData.getConstants(): List<String> {
+    return proto.propertyList
+        .filter { Flags.IS_CONST.get(it.flags) }
+        .map { nameResolver.getString(it.name) }
+}
@@ -12,13 +12,15 @@ 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.BreadthFirstSearch.findReachableNodes
 import org.jetbrains.kotlin.incremental.classpathDiff.ClasspathSnapshotShrinker.shrinkClasspath
-import org.jetbrains.kotlin.incremental.classpathDiff.ImpactAnalysis.computeImpactedSetInclusive
+import org.jetbrains.kotlin.incremental.classpathDiff.ImpactedSymbolsComputer.computeImpactedSymbols
 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.resolve.jvm.JvmClassName
+import org.jetbrains.kotlin.name.FqName
+import org.jetbrains.kotlin.name.Name
 import org.jetbrains.kotlin.resolve.sam.SAM_LOOKUP_NAME
 import java.util.*

@@ -110,14 +112,16 @@ object ClasspathChangesComputer {
        val changedAndImpactedSet = reporter.measure(BuildTime.COMPUTE_IMPACTED_SET) {
            // 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)
+            //     computeImpactedSymbols(changes = changesOnPreviousClasspath, allClasses = classesOnPreviousClasspath) +
+            //         computeImpactedSymbols(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(
+            //     computeImpactedSet(
+            //         changes = changesOnPreviousAndCurrentClasspath,
+            //         allClasses = classesOnPreviousClasspath + classesOnCurrentClasspath)
+            // Note: We will replace `classesOnCurrentClasspath` with `changedClassesOnCurrentClasspath` to avoid listing unchanged classes
+            // twice. `allClasses` may contain overlapping ClassIds of modified classes, but it won't be an issue.
+            computeImpactedSymbols(
                changes = changedSet,
                allClasses = (previousClassSnapshots.asSequence() + changedCurrentClasses.asSequence()).asIterable()
            )
@@ -240,158 +244,143 @@ object ClasspathChangesComputer {
        //     classes, and symbols in removed classes.
        incrementalJvmCache.clearCacheForRemovedClasses(changesCollector)

-        // Normalize the changes and clean up
+        // Get the changes and clean up
        val dirtyData = changesCollector.getDirtyData(listOf(incrementalJvmCache), DoNothingICReporter)
        workingDir.deleteRecursively()

-        return dirtyData.toProgramSymbols(currentClassSnapshots, previousClassSnapshots)
-    }
-
-    private fun DirtyData.toProgramSymbols(
-        currentClassSnapshots: List<AccessibleClassSnapshot>,
-        previousClassSnapshots: List<AccessibleClassSnapshot>
-    ): 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)
-        }
+        // Normalize the changes (convert DirtyData to `ProgramSymbol`s)
+        // Note:
+        //   - DirtyData 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 when converting DirtyData.
+        //   - We have removed unchanged classes earlier in computeChangedAndImpactedSet method, so here we only have changed classes.
+        //   - `changedClasses` may contain overlapping ClassIds of modified classes, but it won't be an issue.
+        val changedClasses = (previousClassSnapshots.asSequence() + currentClassSnapshots.asSequence()).asIterable()
+        return dirtyData.toProgramSymbols(changedClasses)
    }

    /**
-     * Checks whether [DirtyData.toProgramSymbols] completed without any inconsistencies.
+     * Converts this [DirtyData] to [ProgramSymbol]s.
     *
     * 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.
+     * First, we will convert `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.
+     * Then, we will check that:
+     *   1. There are no items in `dirtyLookupSymbols` that have not yet been converted to [ProgramSymbol]s.
+     *   2. `dirtyClassesFqNames` and `dirtyClassesFqNamesForceRecompile` must not contain new information that can't be derived from
+     *      `dirtyLookupSymbols`.
     */
-    private fun checkDirtyDataNormalization(dirtyData: DirtyData, programSymbols: ProgramSymbolSet) {
-        val changes = programSymbols.toChangesEither()
+    private fun DirtyData.toProgramSymbols(changedClasses: Iterable<AccessibleClassSnapshot>): ProgramSymbolSet {
+        val changedProgramSymbols = dirtyLookupSymbols.toProgramSymbolSet(changedClasses)

-        val unmatchedLookupSymbols = dirtyData.dirtyLookupSymbols.toMutableSet().also {
-            it.removeAll(changes.lookupSymbols.toSet())
+        // Check whether there is any info in this DirtyData that has not yet been converted to `changedProgramSymbols`
+        val (changedLookupSymbols, changedFqNames) = changedProgramSymbols.toChangesEither().let {
+            it.lookupSymbols.toSet() to it.fqNames.toSet()
        }
-        val unmatchedFqNames = (dirtyData.dirtyClassesFqNames).toMutableSet().also {
-            it.removeAll(changes.fqNames.toSet())
+        val unmatchedLookupSymbols = this.dirtyLookupSymbols.toMutableSet().also {
+            it.removeAll(changedLookupSymbols)
+        }
+        val unmatchedFqNames = this.dirtyClassesFqNames.toMutableSet().also {
+            it.addAll(this.dirtyClassesFqNamesForceRecompile)
+            it.removeAll(changedFqNames)
        }

-        // 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") }
-        }
+        /* When `unmatchedLookupSymbols` or `unmatchedFqNames` is not empty, there are two cases:
+         *   1. The unmatched LookupSymbols/FqNames are redundant. This is not ideal but because it does not cause incremental compilation
+         *      to be incorrect, we can fix these issues later if they are not easy to fix immediately.
+         *   2. The unmatched LookupSymbols/FqNames are valid changes. Since they are required for incremental compilation to be correct, we
+         *      must fix these issues immediately.
+         * In the following, we'll list the known issues for case 1 (and it must be case 1 only).
+         * TODO: We'll fix these issues later.
+         */

+        // Known issue 1: DirtyData reported by IncrementalJvmCache may include both a class and class member (e.g.,
+        // LookupSymbol("com.example", "A") and LookupSymbol("com.example.A", "someProperty")). When the class LookupSymbol is present, the
+        // class member LookupSymbol is redundant. When converting DirtyData to ProgramSymbols, we remove redundant class member
+        // `ProgramSymbol`s, so here we will find that LookupSymbol("com.example.A", "someProperty") is not yet matched. Ignore these
+        // `LookupSymbol`s for now.
+        val classesFqNames = changedProgramSymbols.classes.mapTo(mutableSetOf()) { it.asSingleFqName() }
+        unmatchedLookupSymbols.removeAll { FqName(it.scope) in classesFqNames }
+
+        // Known issue 2: If class A has a companion object containing a constant `CONSTANT`, and if the value of `CONSTANT` has changed,
+        // then only `A.class` will change, not `A.Companion.class` (see `ConstantsInCompanionObjectImpact`). Since we distinguish between
+        // changed symbols and impacted symbols, we should detect that:
+        //    - A.CONSTANT has changed
+        //    - A.Companion.CONSTANT is unchanged but impacted (this detection happens after the step here)
+        //
+        // However, currently IncrementalJvmCache will report that both `A.CONSTANT` and `A.Companion.CONSTANT` have changed (see
+        // `IncrementalJvmCache.ConstantsMap.process`) as it needs to work with both the old IC and the new IC (in the old IC, changed
+        // symbols and impacted symbols are not clearly separated).
+        //
+        // With the new IC, when converting DirtyData to ProgramSymbols (this method), because we consider only changed classes and
+        // `A.Companion.class` is unchanged, we will not convert `A.Companion.CONSTANT`. Therefore, `A.Companion.CONSTANT` is unmatched,
+        // and we'll need to ignore it here.
+        //
+        // Note: Once we are able to remove this workaround, we can remove RegularKotlinClassSnapshot.companionObjectName as this is the only
+        // usage of that property.
+        val companionObjectFqNames = changedClasses.mapNotNullTo(mutableSetOf()) { clazz ->
+            (clazz as? RegularKotlinClassSnapshot)?.companionObjectName?.let { it ->
+                clazz.classId.createNestedClassId(Name.identifier(it)).asSingleFqName()
+            }
+        }
+        unmatchedLookupSymbols.removeAll { FqName(it.scope) in companionObjectFqNames }
+        unmatchedFqNames.removeAll(companionObjectFqNames)
+
+        // Known issue 3: LookupSymbol(name=<SAM-CONSTRUCTOR>, scope=com.example) reported by IncrementalJvmCache is invalid (detected by
+        // KotlinOnlyClasspathChangesComputerTest.testTopLevelMembers): SAM-CONSTRUCTOR should have a class scope, not a package scope.
+        unmatchedLookupSymbols.removeAll { it.name == SAM_LOOKUP_NAME.asString() && FqName(it.scope) !in classesFqNames }
+
+        // Known issue 4: LookupSymbol(name=BarUseABKt, scope=bar) reported by IncrementalJvmCache is invalid (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).
+        unmatchedLookupSymbols.removeAll { it.name.endsWith("Kt") }
+        unmatchedFqNames.removeAll { it.asString().endsWith("Kt") }
+
+        /*
+         * End of known issues, throw an Exception.
+         */
        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"
+            "The following FqNames can't be derived from DirtyData.dirtyLookupSymbols: ${unmatchedFqNames.joinToString(", ")}.\n" +
+                    "DirtyData = $this"
        }
+
+        return changedProgramSymbols
    }
 }

-internal object ImpactAnalysis {
+private object ImpactedSymbolsComputer {

    /**
-     * Computes the set of [ProgramSymbol]s that are impacted by the given changes.
+     * Computes the set of [ProgramSymbol]s that are *transitively* impacted by the given set of [ProgramSymbol]s. For example, if a
+     * superclass has changed/been impacted, its subclasses will be impacted.
     *
-     * 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 includes the given changes plus the impacted ones.
+     * The returned set is *inclusive* (it contains the given set + the directly/transitively impacted ones).
     */
-    fun computeImpactedSetInclusive(changes: ProgramSymbolSet, allClasses: Iterable<AccessibleClassSnapshot>): ProgramSymbolSet {
-        val classIdToSubclasses = getClassIdToSubclassesMap(allClasses)
-        val impactedClassesResolver = { classId: ClassId -> classIdToSubclasses[classId] ?: emptySet() }
+    fun computeImpactedSymbols(changes: ProgramSymbolSet, allClasses: Iterable<AccessibleClassSnapshot>): ProgramSymbolSet {
+        val impactedSymbolsResolver = AllImpacts.getResolver(allClasses)
+        return ProgramSymbolSet.Collector().apply {
+            // Add impacted classes
+            val impactedClasses = findReachableNodes(changes.classes, impactedSymbolsResolver::getImpactedClasses)
+            addClasses(impactedClasses)

-        return ProgramSymbolSet.Collector().run {
-            addClasses(findImpactedClassesInclusive(changes.classes, impactedClassesResolver))
-
-            for ((classId, memberNames) in changes.classMembers) {
-                findImpactedClassesInclusive(setOf(classId), impactedClassesResolver).forEach { impactedClassId ->
-                    addClassMembers(impactedClassId, memberNames)
-                }
+            // Add impacted class members
+            val classMembers = changes.classMembers.map { ClassMembers(it.key, it.value) }
+            val impactedClassMembers = findReachableNodes(classMembers, impactedSymbolsResolver::getImpactedClassMembers)
+            impactedClassMembers.forEach {
+                addClassMembers(it.classId, it.memberNames)
            }

-            for ((packageFqName, memberNames) in changes.packageMembers) {
+            // Package members are currently not impacted, so we just copy the original set over
+            changes.packageMembers.forEach { (packageFqName, memberNames) ->
                addPackageMembers(packageFqName, memberNames)
            }
-
-            getResult()
-        }
+        }.getResult()
    }

-    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>>()
-        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
-    }
-
-    /**
-     * Finds directly and transitively impacted classes of the given classes. The return set includes both the given classes and the
-     * impacted classes.
-     */
-    fun findImpactedClassesInclusive(classIds: Set<ClassId>, impactedClassesResolver: (ClassId) -> Set<ClassId>): Set<ClassId> {
-        // Standard Breadth-First Search
-        val visitedAndToVisitClasses = classIds.toMutableSet()
-        val classesToVisit = ArrayDeque(classIds)
-
-        while (classesToVisit.isNotEmpty()) {
-            val classToVisit = classesToVisit.removeFirst()
-            val nextClassesToVisit = impactedClassesResolver.invoke(classToVisit) - visitedAndToVisitClasses
-            visitedAndToVisitClasses.addAll(nextClassesToVisit)
-            classesToVisit.addAll(nextClassesToVisit)
-        }
-        return visitedAndToVisitClasses
-    }
-}
-
-/**
- * Returns the [ClassId]s of the supertypes of this class (could be empty in some cases).
- *
- * @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?): Set<ClassId> {
-    return when (this) {
-        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.
-            emptySet()
-        }
-        is JavaClassSnapshot -> supertypes.mapNotNullTo(mutableSetOf()) { classIdResolver.invoke(it) }
-    }
 }
@@ -62,7 +62,14 @@ class RegularKotlinClassSnapshot(
    override val classId: ClassId,
    override val classAbiHash: Long,
    override val classMemberLevelSnapshot: KotlinClassInfo?,
-    val supertypes: List<JvmClassName>
+    val supertypes: List<JvmClassName>,
+
+    /** Name of the companion object of this class (default is "Companion") iff this class HAS a companion object, or null otherwise. */
+    val companionObjectName: String?,
+
+    /** List of constants defined in this class iff this class IS a companion object, or null otherwise. The list could be empty. */
+    val constantsInCompanionObject: List<String>?
+
 ) : KotlinClassSnapshot()

 /** [KotlinClassSnapshot] where class kind == [FILE_FACADE] or [MULTIFILE_CLASS_PART]. */
@@ -120,6 +120,8 @@ private object RegularKotlinClassSnapshotExternalizer : DataExternalizer<Regular
        LongExternalizer.save(output, snapshot.classAbiHash)
        NullableValueExternalizer(KotlinClassInfoExternalizer).save(output, snapshot.classMemberLevelSnapshot)
        ListExternalizer(JvmClassNameExternalizer).save(output, snapshot.supertypes)
+        NullableValueExternalizer(StringExternalizer).save(output, snapshot.companionObjectName)
+        NullableValueExternalizer(ListExternalizer(StringExternalizer)).save(output, snapshot.constantsInCompanionObject)
    }

    override fun read(input: DataInput): RegularKotlinClassSnapshot {
@@ -128,7 +130,9 @@ private object RegularKotlinClassSnapshotExternalizer : DataExternalizer<Regular
            classId = ClassIdExternalizerWithInterning.read(input),
            classAbiHash = LongExternalizer.read(input),
            classMemberLevelSnapshot = NullableValueExternalizer(KotlinClassInfoExternalizer).read(input),
-            supertypes = ListExternalizer(JvmClassNameExternalizerWithInterning).read(input)
+            supertypes = ListExternalizer(JvmClassNameExternalizerWithInterning).read(input),
+            companionObjectName = NullableValueExternalizer(StringExternalizer).read(input),
+            constantsInCompanionObject = NullableValueExternalizer(ListExternalizer(StringExternalizer)).read(input)
        )
    }
 }
@@ -17,6 +17,7 @@ import org.jetbrains.kotlin.incremental.ClasspathChanges.ClasspathSnapshotEnable
 import org.jetbrains.kotlin.incremental.ClasspathChanges.ClasspathSnapshotEnabled.NotAvailableForNonIncrementalRun
 import org.jetbrains.kotlin.incremental.LookupStorage
 import org.jetbrains.kotlin.incremental.LookupSymbol
+import org.jetbrains.kotlin.incremental.classpathDiff.BreadthFirstSearch.findReachableNodes
 import org.jetbrains.kotlin.incremental.classpathDiff.ClasspathSnapshotShrinker.shrinkClasses
 import org.jetbrains.kotlin.incremental.classpathDiff.ClasspathSnapshotShrinker.shrinkClasspath
 import org.jetbrains.kotlin.incremental.storage.ListExternalizer
@@ -24,7 +25,6 @@ import org.jetbrains.kotlin.incremental.storage.LookupSymbolKey
 import org.jetbrains.kotlin.incremental.storage.loadFromFile
 import org.jetbrains.kotlin.incremental.storage.saveToFile
 import org.jetbrains.kotlin.name.ClassId
-import org.jetbrains.kotlin.resolve.jvm.JvmClassName

 object ClasspathSnapshotShrinker {

@@ -42,7 +42,12 @@ object ClasspathSnapshotShrinker {
        return shrinkClasses(allClasses, lookupSymbols, metrics)
    }

-    /** Shrinks the given classes by retaining only classes that are referenced by the given lookup symbols. */
+    /**
+     * Shrinks the given classes by retaining only classes that are referenced by the given lookup symbols.
+     *
+     * Note: We need to retain both directly and transitively referenced classes to compute the impact of classpath changes correctly (see
+     * [ClasspathChangesComputer.computeChangedAndImpactedSet]).
+     */
    fun shrinkClasses(
        allClasses: List<AccessibleClassSnapshot>,
        lookupSymbols: Collection<LookupSymbolKey>,
@@ -57,7 +62,7 @@ object ClasspathSnapshotShrinker {
    }

    /**
-     * Finds classes that are referenced by the given lookup symbols.
+     * Finds classes that are *directly* referenced by the given lookup symbols.
     *
     * Note: It's okay to over-approximate the result.
     */
@@ -90,31 +95,19 @@ object ClasspathSnapshotShrinker {
    }

    /**
-     * Finds classes that are transitively referenced. For example, if a subclass is referenced, its supertypes will potentially be
-     * referenced.
+     * Finds classes that are *transitively* referenced from the given classes. For example, if a subclass is referenced, its supertypes
+     * will be transitively referenced.
     *
-     * The returned list includes the given referenced classes plus the transitively referenced ones.
+     * The returned list is *inclusive* (it contains the given list + the transitively referenced ones).
     */
    private fun findTransitivelyReferencedClasses(
        allClasses: List<AccessibleClassSnapshot>,
        referencedClasses: List<AccessibleClassSnapshot>
    ): List<AccessibleClassSnapshot> {
-        val classIdToClassSnapshot = allClasses.associateBy { it.classId }
-        val classIds: Set<ClassId> = classIdToClassSnapshot.keys // Use Set for presence check
-        val classNameToClassId = classIds.associateBy { JvmClassName.byClassId(it) }
-        val classNameToClassIdResolver = { className: JvmClassName -> classNameToClassId[className] }
-
-        val supertypesResolver = { classId: ClassId ->
-            // No need to collect supertypes outside the given set of classes (e.g., "java/lang/Object")
-            @Suppress("SimpleRedundantLet")
-            classIdToClassSnapshot[classId]?.let {
-                it.getSupertypes(classNameToClassIdResolver).intersect(classIds)
-            } ?: emptySet()
-        }
-
-        val referencedClassIds = referencedClasses.mapTo(mutableSetOf()) { it.classId }
-        val transitivelyReferencedClassIds: Set<ClassId> = /* Use Set for presence check */
-            ImpactAnalysis.findImpactedClassesInclusive(referencedClassIds, supertypesResolver)
+        val referencedClassIds = referencedClasses.map { it.classId }
+        val impactingClassesResolver = AllImpacts.getReverseResolver(allClasses)
+        val transitivelyReferencedClassIds: Set<ClassId> = /* Must be a Set for the presence check below */
+            findReachableNodes(referencedClassIds, impactingClassesResolver::getImpactingClasses)

        return allClasses.filter { it.classId in transitivelyReferencedClassIds }
    }
@@ -89,7 +89,12 @@ object ClassSnapshotter {
        val classMemberLevelSnapshot = kotlinClassInfo.takeIf { granularity == CLASS_MEMBER_LEVEL }

        return when (kotlinClassInfo.classKind) {
-            CLASS -> RegularKotlinClassSnapshot(classId, classAbiHash, classMemberLevelSnapshot, classFile.classInfo.supertypes)
+            CLASS -> RegularKotlinClassSnapshot(
+                classId, classAbiHash, classMemberLevelSnapshot,
+                supertypes = classFile.classInfo.supertypes,
+                companionObjectName = kotlinClassInfo.companionObject?.shortClassName?.identifier,
+                constantsInCompanionObject = kotlinClassInfo.constantsInCompanionObject
+            )
            FILE_FACADE, MULTIFILE_CLASS_PART -> PackageFacadeKotlinClassSnapshot(
                classId, classAbiHash, classMemberLevelSnapshot,
                packageMemberNames = (kotlinClassInfo.protoData as PackagePartProtoData).getNonPrivateMemberNames()
@@ -0,0 +1,240 @@
+/*
+ * Copyright 2010-2022 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.name.ClassId
+import org.jetbrains.kotlin.resolve.jvm.JvmClassName
+import java.util.*
+
+/**
+ * A common interface for all types of impact among classes. For example, if class B extends class A, then class A impacts class B, because
+ * if class A has changed, a source file that references class B will need to be recompiled (even though class B has not changed).
+ */
+internal sealed interface Impact {
+
+    /** Provides an [ImpactedSymbolsResolver] to compute the set of [ProgramSymbol]s impacted by a given set of [ProgramSymbol]s. */
+    fun getResolver(allClasses: Iterable<AccessibleClassSnapshot>): ImpactedSymbolsResolver
+
+    /**
+     * Provides an [ImpactingClassesResolver] to compute the set of classes impacting a given set of classes (the reverse of [getResolver]).
+     */
+    fun getReverseResolver(allClasses: Iterable<AccessibleClassSnapshot>): ImpactingClassesResolver
+}
+
+/**
+ * Computes the set of [ProgramSymbol]s that are *directly* impacted by a given set of [ProgramSymbol]s.
+ *
+ * The returned set is *inclusive* (it contains the given set + the directly impacted ones).
+ *
+ * This is typically used when computing classpath changes: If class A has changed, and it impacts class B, then a source file that
+ * references class B will need to be recompiled (even though class B has not changed).
+ */
+internal interface ImpactedSymbolsResolver {
+    fun getImpactedClasses(classId: ClassId): Set<ClassId>
+    fun getImpactedClassMembers(classMembers: ClassMembers): Set<ClassMembers>
+}
+
+/**
+ * Computes the set of classes *directly* impacting a given set of classes.
+ *
+ * The returned set is *inclusive* (it contains the given set + the directly impacting ones).
+ *
+ * This is typically used when shrinking classpath snapshots: If class A impacts class B, and class B is referenced by a source file, then
+ * class A will need to be retained in the shrunk classpath snapshot because the classpath changes computation will need to see class A
+ * (see [ImpactedSymbolsResolver]).
+ */
+internal interface ImpactingClassesResolver {
+    fun getImpactingClasses(classId: ClassId): Set<ClassId>
+}
+
+/**
+ * A composite [Impact] containing all possible concrete impacts. Currently, the types of impact include:
+ *   1. [SupertypesInheritorsImpact]
+ *   2. [ConstantsInCompanionObjectsImpact]
+ */
+internal object AllImpacts : Impact {
+
+    private val allImpacts = listOf(SupertypesInheritorsImpact, ConstantsInCompanionObjectsImpact)
+
+    override fun getResolver(allClasses: Iterable<AccessibleClassSnapshot>): ImpactedSymbolsResolver {
+        val resolvers = allImpacts.map { it.getResolver(allClasses) }
+        return object : ImpactedSymbolsResolver {
+            override fun getImpactedClasses(classId: ClassId): Set<ClassId> {
+                return resolvers.flatMapTo(mutableSetOf()) { it.getImpactedClasses(classId) }
+            }
+
+            override fun getImpactedClassMembers(classMembers: ClassMembers): Set<ClassMembers> {
+                return resolvers.flatMapTo(mutableSetOf()) { it.getImpactedClassMembers(classMembers) }
+            }
+        }
+    }
+
+    override fun getReverseResolver(allClasses: Iterable<AccessibleClassSnapshot>): ImpactingClassesResolver {
+        val reverseResolvers = allImpacts.map { it.getReverseResolver(allClasses) }
+        return object : ImpactingClassesResolver {
+            override fun getImpactingClasses(classId: ClassId): Set<ClassId> {
+                return reverseResolvers.flatMapTo(mutableSetOf()) { it.getImpactingClasses(classId) }
+            }
+        }
+    }
+}
+
+/**
+ * Describes the impact between supertypes and inheritors: If a superclass/interface has changed, its subclasses/sub-interfaces will be
+ * impacted.
+ */
+private object SupertypesInheritorsImpact : Impact {
+
+    override fun getResolver(allClasses: Iterable<AccessibleClassSnapshot>): ImpactedSymbolsResolver {
+        val classIdToSubclasses: Map<ClassId, Set<ClassId>> = getClassIdToSubclassesMap(allClasses)
+        return object : ImpactedSymbolsResolver {
+            override fun getImpactedClasses(classId: ClassId): Set<ClassId> {
+                return classIdToSubclasses[classId] ?: emptySet()
+            }
+
+            override fun getImpactedClassMembers(classMembers: ClassMembers): Set<ClassMembers> {
+                return classIdToSubclasses[classMembers.classId]?.let { subclasses ->
+                    subclasses.mapTo(mutableSetOf()) { subclass ->
+                        ClassMembers(subclass, classMembers.memberNames)
+                    }
+                } ?: emptySet()
+            }
+        }
+    }
+
+    override fun getReverseResolver(allClasses: Iterable<AccessibleClassSnapshot>): ImpactingClassesResolver {
+        val classIdToSupertypesMap: Map<ClassId, Set<ClassId>> = getClassIdToSupertypesMap(allClasses)
+        return object : ImpactingClassesResolver {
+            override fun getImpactingClasses(classId: ClassId): Set<ClassId> {
+                return classIdToSupertypesMap[classId] ?: emptySet()
+            }
+        }
+    }
+
+    private fun getClassIdToSubclassesMap(allClasses: Iterable<AccessibleClassSnapshot>): Map<ClassId, Set<ClassId>> {
+        val classIdToSubclasses = mutableMapOf<ClassId, MutableSet<ClassId>>()
+        getClassIdToSupertypesMap(allClasses).forEach { (classId, supertypes) ->
+            supertypes.forEach { supertype ->
+                classIdToSubclasses.getOrPut(supertype) { mutableSetOf() }.add(classId)
+            }
+        }
+        return classIdToSubclasses
+    }
+
+    private fun getClassIdToSupertypesMap(allClasses: Iterable<AccessibleClassSnapshot>): Map<ClassId, Set<ClassId>> {
+        val classNameToClassId = allClasses.associate { JvmClassName.byClassId(it.classId) to it.classId }
+        return allClasses.mapNotNull { clazz ->
+            // Find supertypes that are within `allClasses`, we don't care about those outside `allClasses` (e.g., `java/lang/Object`)
+            val supertypes = when (clazz) {
+                is RegularKotlinClassSnapshot -> clazz.supertypes.mapNotNullTo(mutableSetOf()) { classNameToClassId[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.
+                    emptySet()
+                }
+                is JavaClassSnapshot -> clazz.supertypes.mapNotNullTo(mutableSetOf()) { classNameToClassId[it] }
+            }
+            if (supertypes.isNotEmpty()) {
+                clazz.classId to supertypes
+            } else null
+        }.toMap()
+    }
+}
+
+/**
+ * Describes the impact between a class and its companion object when the companion object defines some constants.
+ *
+ * Consider the following source file:
+ *    class A {
+ *       companion object {
+ *          const val CONSTANT = 1
+ *       }
+ *    }
+ *
+ * This source file will compile into 2 .class files:
+ *   - `A.Companion.class`'s Kotlin metadata describes the name and type of `CONSTANT` but not its value. Its Java bytecode does not define
+ *     the constant.
+ *   - `A.class`'s Kotlin metadata does not contain `CONSTANT`. However, its Java bytecode defines the constant as follows:
+ *         public static final int CONSTANT = 1;
+ *
+ * Therefore, if the value of the constant has changed in the source file, we will only see a change in the Java bytecode of `A.class`, not
+ * in `A.Companion.class` or in the Kotlin metadata of either class.
+ *
+ * Hence, we will need to detect that `A.CONSTANT` impacts `A.Companion.CONSTANT` because if a source file references
+ * `A.Companion.CONSTANT`, it will need to be recompiled when `A.CONSTANT`'s value in `A.class` has changed (even though `A.Companion.class`
+ * has not changed).
+ *
+ * Note: This corner case only applies to *constants' values* defined in *companion objects* (it does not apply to constants' names and
+ * types, or top-level constants, or constants in non-companion objects, or inline functions).
+ */
+private object ConstantsInCompanionObjectsImpact : Impact {
+
+    override fun getResolver(allClasses: Iterable<AccessibleClassSnapshot>): ImpactedSymbolsResolver {
+        val companionObjectToConstants: Map<ClassId, List<String>> = allClasses.mapNotNull { clazz ->
+            (clazz as? RegularKotlinClassSnapshot)?.constantsInCompanionObject?.let { constants ->
+                // We only care about companion objects that define some constants
+                if (constants.isNotEmpty()) {
+                    clazz.classId to constants
+                } else null
+            }
+        }.toMap()
+        val classToCompanionObject: Map<ClassId, ClassId> = companionObjectToConstants.keys.associateBy { it.parentClassId!! }
+
+        return object : ImpactedSymbolsResolver {
+            override fun getImpactedClasses(classId: ClassId): Set<ClassId> {
+                return setOfNotNull(classToCompanionObject[classId])
+            }
+
+            override fun getImpactedClassMembers(classMembers: ClassMembers): Set<ClassMembers> {
+                return classToCompanionObject[classMembers.classId]?.let { companionObject ->
+                    val constantsInCompanionObject = companionObjectToConstants[companionObject]!!
+                    val impactedConstants = classMembers.memberNames.intersect(constantsInCompanionObject.toSet())
+                    setOf(ClassMembers(companionObject, impactedConstants))
+                } ?: emptySet()
+            }
+        }
+    }
+
+    override fun getReverseResolver(allClasses: Iterable<AccessibleClassSnapshot>): ImpactingClassesResolver {
+        val companionObjects: Set<ClassId> = allClasses.mapNotNullTo(mutableSetOf()) { clazz ->
+            (clazz as? RegularKotlinClassSnapshot)?.constantsInCompanionObject?.let { constants ->
+                // We only care about companion objects that define some constants
+                if (constants.isNotEmpty()) clazz.classId else null
+            }
+        }
+
+        return object : ImpactingClassesResolver {
+            override fun getImpactingClasses(classId: ClassId): Set<ClassId> {
+                return if (classId in companionObjects) {
+                    setOf(classId.parentClassId!!)
+                } else emptySet()
+            }
+        }
+    }
+}
+
+internal object BreadthFirstSearch {
+
+    /**
+     * Finds the set of nodes that are *transitively* reachable from the given set of nodes.
+     *
+     * The returned set is *inclusive* (it contains the given set + the directly/transitively reachable ones).
+     */
+    fun <T> findReachableNodes(nodes: Iterable<T>, edgesProvider: (T) -> Iterable<T>): Set<T> {
+        val visitedAndToVisitNodes = nodes.toMutableSet()
+        val nodesToVisit = ArrayDeque(nodes.toSet())
+
+        while (nodesToVisit.isNotEmpty()) {
+            val nodeToVisit = nodesToVisit.removeFirst()
+            val nextNodesToVisit = edgesProvider.invoke(nodeToVisit) - visitedAndToVisitNodes
+            visitedAndToVisitNodes.addAll(nextNodesToVisit)
+            nodesToVisit.addAll(nextNodesToVisit)
+        }
+        return visitedAndToVisitNodes
+    }
+}
@@ -21,6 +21,9 @@ data class ClassMember(val classId: ClassId, val memberName: String) : ProgramSy

 data class PackageMember(val packageFqName: FqName, val memberName: String) : ProgramSymbol()

+/** Compact representation for set of [ClassMember]s having the same [ClassId]. */
+data class ClassMembers(val classId: ClassId, val memberNames: Set<String>)
+
 /** Compact representation for a set of [ProgramSymbol]s. */
 class ProgramSymbolSet private constructor(

@@ -217,6 +217,9 @@ class KotlinOnlyClasspathChangesComputerTest : ClasspathChangesComputerTest() {
        val changes = computeClasspathChanges(File(testDataDir, "KotlinOnly/testConstantsAndInlineFunctions/src"), tmpDir)
        Changes(
            lookupSymbols = setOf(
+                LookupSymbol(name = "constantChangedType", scope = "com.example.SomeClass"),
+                LookupSymbol(name = "constantChangedValue", scope = "com.example.SomeClass"),
+
                LookupSymbol(name = "constantChangedType", scope = "com.example.SomeClass.CompanionObject"),
                LookupSymbol(name = "constantChangedValue", scope = "com.example.SomeClass.CompanionObject"),

@@ -12,6 +12,8 @@ import org.jetbrains.kotlin.incremental.classpathDiff.ClasspathSnapshotTestCommo
 import org.jetbrains.kotlin.incremental.classpathDiff.ClasspathSnapshotTestCommon.CompileUtil.compile
 import org.jetbrains.kotlin.incremental.classpathDiff.ClasspathSnapshotTestCommon.CompileUtil.compileAll
 import org.jetbrains.kotlin.incremental.classpathDiff.ClasspathSnapshotTestCommon.SourceFile.KotlinSourceFile
+import org.jetbrains.kotlin.incremental.storage.fromByteArray
+import org.jetbrains.kotlin.incremental.storage.toByteArray
 import org.jetbrains.kotlin.test.KotlinTestUtils
 import org.junit.Rule
 import org.junit.rules.TemporaryFolder
@@ -25,7 +27,11 @@ abstract class ClasspathSnapshotTestCommon {

    // Use Gson to compare objects
    private val gson by lazy { GsonBuilder().setPrettyPrinting().create() }
-    protected fun Any.toGson(): String = gson.toJson(this)
+    protected fun ClassSnapshot.toGson(): String = gson.toJson(
+        // Serialize and deserialize the object to unset lazy properties' values as they are not essential and can add noise when comparing
+        // objects
+        ClassSnapshotExternalizer.fromByteArray(ClassSnapshotExternalizer.toByteArray(this))
+    )

    sealed class SourceFile(val baseDir: File, relativePath: String) {
        val unixStyleRelativePath: String
@@ -56,6 +56,14 @@
    "protoData$delegate": {
      "initializer": {},
      "_value": {}
+    },
+    "companionObject$delegate": {
+      "initializer": {},
+      "_value": {}
+    },
+    "constantsInCompanionObject$delegate": {
+      "initializer": {},
+      "_value": {}
    }
  },
  "supertypes": [