diff --git a/build-common/src/org/jetbrains/kotlin/incremental/AbstractIncrementalCache.kt b/build-common/src/org/jetbrains/kotlin/incremental/AbstractIncrementalCache.kt index 5e073dc6395..67bde4055ee 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/AbstractIncrementalCache.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/AbstractIncrementalCache.kt @@ -20,7 +20,6 @@ import com.intellij.util.io.EnumeratorStringDescriptor import org.jetbrains.kotlin.incremental.storage.* import org.jetbrains.kotlin.metadata.ProtoBuf import org.jetbrains.kotlin.metadata.deserialization.Flags -import org.jetbrains.kotlin.metadata.deserialization.NameResolver import org.jetbrains.kotlin.metadata.deserialization.TypeTable import org.jetbrains.kotlin.metadata.deserialization.supertypes import org.jetbrains.kotlin.name.FqName @@ -125,7 +124,9 @@ abstract class AbstractIncrementalCache( * * The `srcFile` argument may be `null` (e.g., if we are processing .class files in jars where source files are not available). */ - protected fun addToClassStorage(proto: ProtoBuf.Class, nameResolver: NameResolver, srcFile: File?) { + protected fun addToClassStorage(classProtoData: ClassProtoData, srcFile: File?) { + val (proto, nameResolver) = classProtoData + val supertypes = proto.supertypes(TypeTable(proto.typeTable)) val parents = supertypes.map { nameResolver.getClassId(it.className).asSingleFqName() } .filter { it.asString() != "kotlin.Any" } diff --git a/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJsCache.kt b/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJsCache.kt index 84e35901390..6c42a688834 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJsCache.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJsCache.kt @@ -117,7 +117,7 @@ open class IncrementalJsCache( registerOutputForFile(srcFile, classId.asSingleFqName()) if (protoData is ClassProtoData) { - addToClassStorage(protoData.proto, protoData.nameResolver, srcFile) + addToClassStorage(protoData, srcFile) } } diff --git a/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJvmCache.kt b/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJvmCache.kt index 69c7b915c21..dfeb8f17386 100644 --- a/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJvmCache.kt +++ b/build-common/src/org/jetbrains/kotlin/incremental/IncrementalJvmCache.kt @@ -30,11 +30,10 @@ import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader import org.jetbrains.kotlin.load.kotlin.incremental.components.IncrementalCache import org.jetbrains.kotlin.load.kotlin.incremental.components.JvmPackagePartProto import org.jetbrains.kotlin.metadata.jvm.deserialization.BitEncoding -import org.jetbrains.kotlin.metadata.jvm.deserialization.JvmProtoBufUtil import org.jetbrains.kotlin.metadata.jvm.deserialization.ModuleMapping import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.FqName -import org.jetbrains.kotlin.name.SpecialNames.DEFAULT_NAME_FOR_COMPANION_OBJECT +import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.resolve.jvm.AsmTypes import org.jetbrains.kotlin.resolve.jvm.JvmClassName import org.jetbrains.org.objectweb.asm.* @@ -182,12 +181,7 @@ open class IncrementalJvmCache( inlineFunctionsMap.process(kotlinClassInfo, changesCollector) } KotlinClassHeader.Kind.CLASS -> { - if (sourceFiles != null) { - assert(sourceFiles.size == 1) { "Class is expected to have only one source file: $sourceFiles" } - addToClassStorage(kotlinClassInfo, sourceFiles.single()) - } else { - addToClassStorage(kotlinClassInfo, null) - } + addToClassStorage(kotlinClassInfo.protoData as ClassProtoData, sourceFiles?.let { sourceFiles.single() }) protoMap.process(kotlinClassInfo, changesCollector) constantsMap.process(kotlinClassInfo, changesCollector) @@ -202,8 +196,7 @@ open class IncrementalJvmCache( val jvmClassName = JvmClassName.byClassId(serializedJavaClass.classId) javaSourcesProtoMap.process(jvmClassName, serializedJavaClass, collector) source?.let { sourceToClassesMap.add(source, jvmClassName) } - val (proto, nameResolver) = serializedJavaClass.toProtoData() - addToClassStorage(proto, nameResolver, source) + addToClassStorage(serializedJavaClass.toProtoData(), source) // collector.addJavaProto(ClassProtoData(proto, nameResolver)) dirtyOutputClassesMap.notDirty(jvmClassName) } @@ -317,15 +310,11 @@ open class IncrementalJvmCache( private fun put(kotlinClassInfo: KotlinClassInfo, changesCollector: ChangesCollector) { val key = kotlinClassInfo.className.internalName val oldData = storage[key] - val newData = ProtoMapValue( - kotlinClassInfo.classKind != KotlinClassHeader.Kind.CLASS, - BitEncoding.decodeBytes(kotlinClassInfo.classHeaderData), - kotlinClassInfo.classHeaderStrings - ) + val newData = kotlinClassInfo.protoMapValue storage[key] = newData val packageFqName = kotlinClassInfo.className.packageFqName - changesCollector.collectProtoChanges(oldData?.toProtoData(packageFqName), newData.toProtoData(packageFqName), packageProtoKey = key) + changesCollector.collectProtoChanges(oldData?.toProtoData(packageFqName), kotlinClassInfo.protoData, packageProtoKey = key) } operator fun contains(className: JvmClassName): Boolean = @@ -402,20 +391,29 @@ open class IncrementalJvmCache( storage.remove(key) } - for (const in oldMap.keys + newMap.keys) { - //Constant can be declared via companion object or via const field declaration - changesCollector.collectMemberIfValueWasChanged( - kotlinClassInfo.scopeFqName(companion = true), - const, - oldMap[const], - newMap[const] - ) - changesCollector.collectMemberIfValueWasChanged( - kotlinClassInfo.scopeFqName(companion = false), - const, - oldMap[const], - newMap[const] - ) + 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). + for (const in allConstants) { + changesCollector.collectMemberIfValueWasChanged(scope, const, oldMap[const], newMap[const]) } } @@ -503,11 +501,6 @@ open class IncrementalJvmCache( value.dumpCollection() } - private fun addToClassStorage(classInfo: KotlinClassInfo, srcFile: File?) { - val (nameResolver, proto) = JvmProtoBufUtil.readClassDataFrom(classInfo.classHeaderData, classInfo.classHeaderStrings) - addToClassStorage(proto, nameResolver, srcFile) - } - private inner class InlineFunctionsMap(storageFile: File) : BasicStringMap>(storageFile, LinkedHashMapExternalizer(StringExternalizer, LongExternalizer)) { @@ -545,6 +538,11 @@ open class IncrementalJvmCache( override fun dumpValue(value: LinkedHashMap): String = value.dumpMap { java.lang.Long.toHexString(it) } } + + private fun KotlinClassInfo.scopeFqName() = when (classKind) { + KotlinClassHeader.Kind.CLASS -> classId.asSingleFqName() + else -> classId.packageFqName + } } private object PathCollectionExternalizer : @@ -613,18 +611,32 @@ class KotlinClassInfo constructor( val classKind: KotlinClassHeader.Kind, val classHeaderData: Array, // Can be empty val classHeaderStrings: Array, // Can be empty - @Suppress("SpellCheckingInspection") val multifileClassName: String?, + val multifileClassName: String?, val constantsMap: LinkedHashMap, val inlineFunctionsMap: LinkedHashMap ) { val className: JvmClassName by lazy { JvmClassName.byClassId(classId) } - fun scopeFqName(companion: Boolean = false) = when (classKind) { - KotlinClassHeader.Kind.CLASS -> { - className.fqNameForClassNameWithoutDollars.let { if (companion) it.child(DEFAULT_NAME_FOR_COMPANION_OBJECT) else it } + val protoMapValue: ProtoMapValue by lazy { + ProtoMapValue( + isPackageFacade = classKind != KotlinClassHeader.Kind.CLASS, + BitEncoding.decodeBytes(classHeaderData), + classHeaderStrings + ) + } + + /** + * Returns 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. + */ + val protoData: ProtoData by lazy { + check(classKind != KotlinClassHeader.Kind.MULTIFILE_CLASS) { + "Proto data is not available for KotlinClassHeader.Kind.MULTIFILE_CLASS: $classId" } - else -> className.packageFqName + protoMapValue.toProtoData(className.packageFqName) } companion object { diff --git a/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputerTest.kt b/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputerTest.kt index 58459ebc57c..5960d19f1ed 100644 --- a/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputerTest.kt +++ b/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputerTest.kt @@ -171,27 +171,17 @@ class KotlinOnlyClasspathChangesComputerTest : ClasspathChangesComputerTest() { Changes( lookupSymbols = setOf( LookupSymbol(name = "constantChangedType", scope = "com.example.SomeClass.CompanionObject"), - // TODO (Fix in next commit). Missing: - // LookupSymbol(name = "constantChangedValue", scope = "com.example.SomeClass.CompanionObject") + LookupSymbol(name = "constantChangedValue", scope = "com.example.SomeClass.CompanionObject"), LookupSymbol(name = "inlineFunctionChangedSignature", scope = "com.example.SomeClass"), LookupSymbol(name = "inlineFunctionChangedImplementation", scope = "com.example.SomeClass"), LookupSymbol(name = SAM_LOOKUP_NAME.asString(), scope = "com.example.SomeClass"), LookupSymbol(name = SAM_LOOKUP_NAME.asString(), scope = "com.example.SomeClass.CompanionObject"), - - // TODO (Fix in next commit). Incorrect: - LookupSymbol(name = "constantChangedType", scope = "com.example.SomeClass"), - LookupSymbol(name = "constantChangedType", scope = "com.example.SomeClass.Companion"), - LookupSymbol(name = "constantChangedValue", scope = "com.example.SomeClass"), - LookupSymbol(name = "constantChangedValue", scope = "com.example.SomeClass.Companion"), - LookupSymbol(name = SAM_LOOKUP_NAME.asString(), scope = "com.example.SomeClass.Companion"), ), fqNames = setOf( "com.example.SomeClass", "com.example.SomeClass.CompanionObject", - // TODO (Fix in next commit). Incorrect: - "com.example.SomeClass.Companion" ) ).assertEquals(changes) } diff --git a/compiler/incremental-compilation-impl/testData/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotterTest/expected-snapshot/kotlin/com/example/SimpleClass.json b/compiler/incremental-compilation-impl/testData/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotterTest/expected-snapshot/kotlin/com/example/SimpleClass.json index 5f8fbbf4728..5c582246606 100644 --- a/compiler/incremental-compilation-impl/testData/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotterTest/expected-snapshot/kotlin/com/example/SimpleClass.json +++ b/compiler/incremental-compilation-impl/testData/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotterTest/expected-snapshot/kotlin/com/example/SimpleClass.json @@ -34,6 +34,14 @@ "className$delegate": { "initializer": {}, "_value": {} + }, + "protoMapValue$delegate": { + "initializer": {}, + "_value": {} + }, + "protoData$delegate": { + "initializer": {}, + "_value": {} } }, "supertypes": [