From 1cb509d5293d762fa0c8e7a0d1d191e2ec5eccc5 Mon Sep 17 00:00:00 2001 From: Hung Nguyen Date: Tue, 11 Jan 2022 19:30:32 +0000 Subject: [PATCH] KT-45777: Remove proto-based approach to compute Java class snapshots as the ASM-based approach is better, and we have switched to the ASM-based approach for a while now. --- .../incremental/JavaClassDescriptorCreator.kt | 224 ------------------ .../classpathDiff/ClasspathChangesComputer.kt | 103 +++----- .../classpathDiff/ClasspathSnapshot.kt | 18 +- .../ClasspathSnapshotSerializer.kt | 48 +--- .../classpathDiff/ClasspathSnapshotter.kt | 222 +++-------------- .../classpathDiff/JavaClassChangesComputer.kt | 12 +- .../classpathDiff/JavaClassSnapshotter.kt | 2 +- .../ClasspathChangesComputerTest.kt | 27 +-- .../ClasspathSnapshotTestCommon.kt | 6 +- .../classpathDiff/ClasspathSnapshotterTest.kt | 2 +- .../impl/classFiles/BinaryJavaClass.kt | 28 +-- 11 files changed, 96 insertions(+), 596 deletions(-) delete mode 100644 build-common/src/org/jetbrains/kotlin/incremental/JavaClassDescriptorCreator.kt diff --git a/build-common/src/org/jetbrains/kotlin/incremental/JavaClassDescriptorCreator.kt b/build-common/src/org/jetbrains/kotlin/incremental/JavaClassDescriptorCreator.kt deleted file mode 100644 index 0deb8c1c8a0..00000000000 --- a/build-common/src/org/jetbrains/kotlin/incremental/JavaClassDescriptorCreator.kt +++ /dev/null @@ -1,224 +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 - -import org.jetbrains.kotlin.builtins.StandardNames -import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor -import org.jetbrains.kotlin.descriptors.ClassDescriptor -import org.jetbrains.kotlin.descriptors.SourceFile -import org.jetbrains.kotlin.descriptors.findClassAcrossModuleDependencies -import org.jetbrains.kotlin.load.java.JavaClassFinder -import org.jetbrains.kotlin.load.java.descriptors.JavaClassDescriptor -import org.jetbrains.kotlin.load.java.sources.JavaSourceElement -import org.jetbrains.kotlin.load.java.sources.JavaSourceElementFactory -import org.jetbrains.kotlin.load.java.structure.JavaAnnotation -import org.jetbrains.kotlin.load.java.structure.JavaClass -import org.jetbrains.kotlin.load.java.structure.JavaElement -import org.jetbrains.kotlin.load.java.structure.JavaPackage -import org.jetbrains.kotlin.load.java.structure.impl.classFiles.BinaryClassSignatureParser -import org.jetbrains.kotlin.load.java.structure.impl.classFiles.BinaryJavaClass -import org.jetbrains.kotlin.load.java.structure.impl.classFiles.ClassifierResolutionContext -import org.jetbrains.kotlin.load.kotlin.DeserializationComponentsForJava.Companion.createModuleData -import org.jetbrains.kotlin.load.kotlin.KotlinClassFinder -import org.jetbrains.kotlin.name.ClassId -import org.jetbrains.kotlin.name.FqName -import org.jetbrains.kotlin.name.Name -import org.jetbrains.kotlin.serialization.DescriptorSerializer -import org.jetbrains.kotlin.serialization.deserialization.ErrorReporter -import org.jetbrains.kotlin.serialization.deserialization.builtins.BuiltInSerializerProtocol -import org.jetbrains.kotlin.serialization.deserialization.builtins.BuiltInsResourceLoader -import java.io.InputStream - -/** Creates [JavaClassDescriptor]s of Java classes. */ -object JavaClassDescriptorCreator { - - /** - * Creates [JavaClassDescriptor]s of the given Java classes, or returns `null` if it can't be created for some reason. - * - * Note that creating a [JavaClassDescriptor] for a nested class will require accessing the outer class (and possibly vice versa). - * Therefore, outer classes and nested classes must be passed together in one invocation of this method. - */ - fun create(classIds: List, classesContents: List): List { - val binaryJavaClasses = classIds.mapIndexed { index, classId -> - createBinaryJavaClass(classId, classesContents[index]) - } - val moduleDescriptor = createModuleData( - kotlinClassFinder = NoOpKotlinClassFinder, - jvmBuiltInsKotlinClassFinder = JvmBuiltInsKotlinClassFinder(), - javaClassFinder = BinaryJavaClassFinder(binaryJavaClasses), - moduleName = JavaClassDescriptorCreator::class.java.simpleName, - errorReporter = ThrowImmediatelyErrorReporter, - javaSourceElementFactory = NoSourceJavaSourceElementFactory - ).deserializationComponentsForJava.components.moduleDescriptor - - return classIds.map { classId -> - moduleDescriptor.findClassAcrossModuleDependencies(classId) as? JavaClassDescriptor - } - } -} - -private fun createBinaryJavaClass(classId: ClassId, classContents: ByteArray): BinaryJavaClass { - val context = ClassifierResolutionContext { - null // TODO: Implement this? - } - val outerClass: JavaClass? = null // TODO: Compute outer class? - val innerClassFinder: ((Name) -> JavaClass?) = { _ -> - null // TODO: Implement this? - } - - return try { - BinaryJavaClass( - virtualFile = null, - fqName = classId.asSingleFqName(), - context = context, - signatureParser = BinaryClassSignatureParser(), - access = 0, - outerClass = outerClass, - classContent = classContents, - innerClassFinder = innerClassFinder - ) - } catch (e: NoSuchMethodError) { - // When running unit tests, the above call currently fails as JavaClassDescriptorCreator and BinaryJavaClass are located in - // different jars (kotlin-build-common.jar and kotlin-compiler-embeddable.jar), and in the second jar, `com.intellij.*` classes are - // repackaged as `org.jetbrains.kotlin.com.intellij.*`. The `virtualFile` argument above (even though it is null) has type - // `com.intellij.openapi.vfs.VirtualFile`, which has been repackaged, so the method signatures won't match at run time. - // In integrations tests or regular builds, JavaClassDescriptorCreator and BinaryJavaClass are both located in - // kotlin-compiler-embeddable.jar with all references renamed together, so there are no issues. - // We use reflection here as a workaround for unit tests. - val binaryJavaClass = BinaryJavaClass::class.java - val constructor = binaryJavaClass.constructors.sortedBy { it.parameters.size }[0] - return constructor.newInstance( - /* virtualFile */ null, - /* fqName */ classId.asSingleFqName(), - /* context */ context, - /* signatureParser */ BinaryClassSignatureParser(), - /* access */ 0, - /* outerClass */ outerClass, - /* classContent */ classContents, - /* innerClassFinder */ innerClassFinder - ) as BinaryJavaClass - } -} - -/** - * [JavaClassFinder] that returns results based on the given [BinaryJavaClass] list. - * - * Note that some returned results are fake/empty (similar to ReflectJavaClassFinder). - * TODO: Revise and see if there are any correctness issues. - */ -private class BinaryJavaClassFinder(binaryJavaClasses: List) : JavaClassFinder { - - private val nameToJavaClass: Map = binaryJavaClasses.associateBy { it.fqName } - - override fun findClass(request: JavaClassFinder.Request): JavaClass? { - return nameToJavaClass[request.classId.asSingleFqName()] - } - - override fun findPackage(fqName: FqName, mayHaveAnnotations: Boolean): JavaPackage { - return object : JavaPackage { - - override val fqName: FqName - get() = fqName - - override val subPackages: Collection - get() = emptyList() - - override val annotations: Collection - get() = emptyList() - - override val isDeprecatedInJavaDoc: Boolean - get() = false - - override fun getClasses(nameFilter: (Name) -> Boolean): Collection = emptyList() - - override fun findAnnotation(fqName: FqName): JavaAnnotation? = null - } - } - - override fun knownClassNamesInPackage(packageFqName: FqName): Set? = null -} - -/** [KotlinClassFinder] that returns no results (e.g., when we know that the classes are not Kotlin classes). */ -private object NoOpKotlinClassFinder : KotlinClassFinder { - override fun findKotlinClassOrContent(classId: ClassId): KotlinClassFinder.Result? = null - override fun findKotlinClassOrContent(javaClass: JavaClass): KotlinClassFinder.Result? = null - override fun findMetadata(classId: ClassId): InputStream? = null - override fun hasMetadataPackage(fqName: FqName): Boolean = false - override fun findBuiltInsData(packageFqName: FqName): InputStream? = null -} - -/** [KotlinClassFinder] for Kotlin JVM built-in classes. */ -private class JvmBuiltInsKotlinClassFinder : KotlinClassFinder { - override fun findKotlinClassOrContent(classId: ClassId): KotlinClassFinder.Result? = null - override fun findKotlinClassOrContent(javaClass: JavaClass): KotlinClassFinder.Result? = null - override fun findMetadata(classId: ClassId): InputStream? = null - override fun hasMetadataPackage(fqName: FqName): Boolean = false - - private val builtInsResourceLoader by lazy { BuiltInsResourceLoader() } - - override fun findBuiltInsData(packageFqName: FqName): InputStream? { - // Same as ReflectKotlinClassFinder.findBuiltInsData - return if (packageFqName.startsWith(StandardNames.BUILT_INS_PACKAGE_NAME)) { - builtInsResourceLoader.loadResource(BuiltInSerializerProtocol.getBuiltInsFilePath(packageFqName)) - } else null - } -} - -/** [ErrorReporter] that throws errors as soon as they are reported. */ -private object ThrowImmediatelyErrorReporter : ErrorReporter { - - override fun reportIncompleteHierarchy(descriptor: ClassDescriptor, unresolvedSuperClasses: MutableList) { - // BinaryJavaClassFinder doesn't have the whole classpath so it is unable to resolve classes such as java.lang.Object. - // Ignore this error for now. - // TODO: Revisit later to see how we can address this or if we can safely ignore this error. - } - - override fun reportCannotInferVisibility(descriptor: CallableMemberDescriptor) { - error("Cannot infer visibility for $descriptor") - } -} - -/** [JavaSourceElementFactory] that doesn't provide a source file for a [JavaElement] (e.g., because the source file is not available). */ -private object NoSourceJavaSourceElementFactory : JavaSourceElementFactory { - - private class NoSourceJavaElement(override val javaElement: JavaElement) : JavaSourceElement { - override fun getContainingFile(): SourceFile = SourceFile.NO_SOURCE_FILE - } - - override fun source(javaElement: JavaElement): JavaSourceElement = NoSourceJavaElement(javaElement) -} - -/** Similar to [JavaClassDescriptor.convertToProto] but without an associated source file. */ -fun JavaClassDescriptor.toSerializedJavaClass(): SerializedJavaClass { - val extension = JavaClassesSerializerExtension() - - val descriptorSerializer = try { - DescriptorSerializer.create( - descriptor = this, - extension = extension, - parentSerializer = null, - project = null - ) - } catch (e: NoSuchMethodError) { - // When running unit tests, the above call currently fails as the `project` argument above has type - // `com.intellij.openapi.project.Project`, which is repackaged as `org.jetbrains.kotlin.com.intellij.openapi.project.Project` in - // kotlin-compiler-embeddable.jar (see the comment at the createBinaryJavaClass method for more context). - // We use reflection here as a workaround for unit tests. - val createMethod = DescriptorSerializer::class.java.methods.single { it.name == "create" } - createMethod.invoke( - /* static method */ null, - /* descriptor */ this, - /* extension */ extension, - /* parentSerializer */ null, - /* project */ null - ) as DescriptorSerializer - } - - val classProto = descriptorSerializer.classProto(this).build() - val (stringTable, qualifiedNameTable) = extension.stringTable.buildProto() - - return SerializedJavaClass(classProto, stringTable, qualifiedNameTable) -} diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputer.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputer.kt index 0480d957a30..e7096acfc1b 100644 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputer.kt +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathChangesComputer.kt @@ -14,12 +14,9 @@ import org.jetbrains.kotlin.incremental.classpathDiff.ImpactAnalysis.computeImpa 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.metadata.deserialization.TypeTable -import org.jetbrains.kotlin.metadata.deserialization.supertypes import org.jetbrains.kotlin.name.ClassId import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.resolve.jvm.JvmClassName -import org.jetbrains.kotlin.serialization.deserialization.getClassId import java.io.File import java.util.* @@ -92,34 +89,38 @@ object ClasspathChangesComputer { previousClassSnapshots: List, metrics: BuildMetricsReporter ): ChangeSet { - val asmBasedSnapshotPredicate: (ClassSnapshot) -> Boolean = { + val isKotlinClass: (ClassSnapshot) -> Boolean = { when (it) { - is RegularJavaClassSnapshot -> true - is KotlinClassSnapshot, is ProtoBasedJavaClassSnapshot -> false - else -> error("Unexpected type (it should have been handled earlier): ${it.javaClass.name}") + is KotlinClassSnapshot -> true + is JavaClassSnapshot -> false + is InaccessibleClassSnapshot -> error("Unexpected type (it should have been handled earlier): ${it.javaClass.name}") } } - val (currentAsmBasedSnapshots, currentProtoBasedSnapshots) = currentClassSnapshots.partition(asmBasedSnapshotPredicate) - val (previousAsmBasedSnapshots, previousProtoBasedSnapshots) = previousClassSnapshots.partition(asmBasedSnapshotPredicate) + val (currentKotlinClassSnapshots, currentJavaClassSnapshots) = currentClassSnapshots.partition(isKotlinClass) + val (previousKotlinClassSnapshots, previousJavaClassSnapshots) = previousClassSnapshots.partition(isKotlinClass) + @Suppress("UNCHECKED_CAST") val kotlinClassChanges = metrics.measure(BuildTime.COMPUTE_KOTLIN_CLASS_CHANGES) { - computeChangesForProtoBasedSnapshots(currentProtoBasedSnapshots, previousProtoBasedSnapshots) + computeKotlinClassChanges( + currentKotlinClassSnapshots as List, + previousKotlinClassSnapshots as List + ) } @Suppress("UNCHECKED_CAST") val javaClassChanges = metrics.measure(BuildTime.COMPUTE_JAVA_CLASS_CHANGES) { JavaClassChangesComputer.compute( - currentAsmBasedSnapshots as List, - previousAsmBasedSnapshots as List + currentJavaClassSnapshots as List, + previousJavaClassSnapshots as List ) } return kotlinClassChanges + javaClassChanges } - private fun computeChangesForProtoBasedSnapshots( - currentClassSnapshots: List, - previousClassSnapshots: List + private fun computeKotlinClassChanges( + currentClassSnapshots: List, + previousClassSnapshots: List ): ChangeSet { val workingDir = FileUtil.createTempDirectory(this::class.java.simpleName, "_WorkingDir_${UUID.randomUUID()}", /* deleteOnExit */ true) @@ -132,28 +133,13 @@ object ClasspathChangesComputer { // - The ChangesCollector result will contain symbols in the previous classes (we actually don't need them, but it's part of the // API's effects). val unusedChangesCollector = ChangesCollector() - for (previousSnapshot in previousClassSnapshots) { - when (previousSnapshot) { - is KotlinClassSnapshot -> { - incrementalJvmCache.saveClassToCache( - kotlinClassInfo = previousSnapshot.classInfo, - sourceFiles = null, - changesCollector = unusedChangesCollector - ) - incrementalJvmCache.markDirty(previousSnapshot.classInfo.className) - } - is ProtoBasedJavaClassSnapshot -> { - incrementalJvmCache.saveJavaClassProto( - source = null, - serializedJavaClass = previousSnapshot.serializedJavaClass, - collector = unusedChangesCollector - ) - incrementalJvmCache.markDirty(JvmClassName.byClassId(previousSnapshot.serializedJavaClass.classId)) - } - is RegularJavaClassSnapshot, is InaccessibleClassSnapshot, is ContentHashJavaClassSnapshot -> { - error("Unexpected type (it should have been handled earlier): ${previousSnapshot.javaClass.name}") - } - } + previousClassSnapshots.forEach { + incrementalJvmCache.saveClassToCache( + kotlinClassInfo = it.classInfo, + sourceFiles = null, + changesCollector = unusedChangesCollector + ) + incrementalJvmCache.markDirty(it.classInfo.className) } // Step 2: @@ -164,26 +150,12 @@ object ClasspathChangesComputer { // - The intermediate ChangesCollector result will contain symbols in added classes and changed (added/modified/removed) symbols // in modified classes. We will collect symbols in removed classes in step 3. val changesCollector = ChangesCollector() - for (currentSnapshot in currentClassSnapshots) { - when (currentSnapshot) { - is KotlinClassSnapshot -> { - incrementalJvmCache.saveClassToCache( - kotlinClassInfo = currentSnapshot.classInfo, - sourceFiles = null, - changesCollector = changesCollector - ) - } - is ProtoBasedJavaClassSnapshot -> { - incrementalJvmCache.saveJavaClassProto( - source = null, - serializedJavaClass = currentSnapshot.serializedJavaClass, - collector = changesCollector - ) - } - is RegularJavaClassSnapshot, is InaccessibleClassSnapshot, is ContentHashJavaClassSnapshot -> { - error("Unexpected type (it should have been handled earlier): ${currentSnapshot.javaClass.name}") - } - } + currentClassSnapshots.forEach { + incrementalJvmCache.saveClassToCache( + kotlinClassInfo = it.classInfo, + sourceFiles = null, + changesCollector = changesCollector + ) } // Step 3: @@ -303,11 +275,8 @@ internal object ImpactAnalysis { internal fun ClassSnapshot.getClassId(): ClassId { return when (this) { is KotlinClassSnapshot -> classInfo.classId - is RegularJavaClassSnapshot -> classId - is ProtoBasedJavaClassSnapshot -> serializedJavaClass.classId - is InaccessibleClassSnapshot, is ContentHashJavaClassSnapshot -> { - error("Unexpected type (it should have been handled earlier): ${javaClass.name}") - } + is JavaClassSnapshot -> classId + is InaccessibleClassSnapshot -> error("Unexpected type (it should have been handled earlier): ${javaClass.name}") } } @@ -320,13 +289,7 @@ internal fun ClassSnapshot.getClassId(): ClassId { internal fun ClassSnapshot.getSupertypes(classIdResolver: (JvmClassName) -> ClassId?): List { return when (this) { is KotlinClassSnapshot -> supertypes.mapNotNull { classIdResolver.invoke(it) } - is RegularJavaClassSnapshot -> supertypes.mapNotNull { classIdResolver.invoke(it) } - is ProtoBasedJavaClassSnapshot -> { - val (proto, nameResolver) = serializedJavaClass.toProtoData() - proto.supertypes(TypeTable(proto.typeTable)).map { nameResolver.getClassId(it.className) } - } - is InaccessibleClassSnapshot, is ContentHashJavaClassSnapshot -> { - error("Unexpected type (it should have been handled earlier): ${javaClass.name}") - } + is JavaClassSnapshot -> supertypes.mapNotNull { classIdResolver.invoke(it) } + is InaccessibleClassSnapshot -> error("Unexpected type (it should have been handled earlier): ${javaClass.name}") } } diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshot.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshot.kt index 48802b1438f..0af2bb75375 100644 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshot.kt +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshot.kt @@ -6,7 +6,6 @@ package org.jetbrains.kotlin.incremental.classpathDiff import org.jetbrains.kotlin.incremental.KotlinClassInfo -import org.jetbrains.kotlin.incremental.SerializedJavaClass import org.jetbrains.kotlin.incremental.md5 import org.jetbrains.kotlin.incremental.storage.toByteArray import org.jetbrains.kotlin.name.ClassId @@ -72,10 +71,7 @@ class KotlinClassSnapshot( } /** [ClassSnapshot] of a Java class. */ -sealed class JavaClassSnapshot : ClassSnapshot() - -/** [JavaClassSnapshot] of a typical Java class. */ -class RegularJavaClassSnapshot( +class JavaClassSnapshot( /** [ClassId] of the class. It is part of the class's ABI ([classAbiExcludingMembers]). */ val classId: ClassId, @@ -92,7 +88,7 @@ class RegularJavaClassSnapshot( /** [AbiSnapshot]s of the class's methods. */ val methodsAbi: List -) : JavaClassSnapshot() { +) : ClassSnapshot() { val className by lazy { JvmClassName.byClassId(classId).also { @@ -123,9 +119,6 @@ class AbiSnapshotForTests( ) : AbiSnapshot(name, abiHash) -/** [JavaClassSnapshot] of a typical Java class which uses protos internally. */ -class ProtoBasedJavaClassSnapshot(val serializedJavaClass: SerializedJavaClass) : JavaClassSnapshot() - /** * [ClassSnapshot] of an inaccessible class. * @@ -133,10 +126,3 @@ class ProtoBasedJavaClassSnapshot(val serializedJavaClass: SerializedJavaClass) * will not require recompilation of other source files. */ object InaccessibleClassSnapshot : ClassSnapshot() - -/** - * [JavaClassSnapshot] of a Java class where a proper snapshot can't be created for some reason, so we use the hash of the class contents as - * the snapshot instead, so that at least it's still correct when used as an input of the `KotlinCompile` task (when the class contents have - * changed, this snapshot will also change). - */ -class ContentHashJavaClassSnapshot(val contentHash: Long) : JavaClassSnapshot() diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotSerializer.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotSerializer.kt index fcda029d986..0637a4661a2 100644 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotSerializer.kt +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotSerializer.kt @@ -6,7 +6,6 @@ package org.jetbrains.kotlin.incremental.classpathDiff import com.intellij.util.io.DataExternalizer -import org.jetbrains.kotlin.incremental.JavaClassProtoMapValueExternalizer import org.jetbrains.kotlin.incremental.KotlinClassInfo import org.jetbrains.kotlin.incremental.storage.* import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader @@ -166,27 +165,6 @@ object PackageMemberExternalizer : DataExternalizer { object JavaClassSnapshotExternalizer : DataExternalizer { override fun save(output: DataOutput, snapshot: JavaClassSnapshot) { - output.writeString(snapshot.javaClass.name) - when (snapshot) { - is RegularJavaClassSnapshot -> RegularJavaClassSnapshotExternalizer.save(output, snapshot) - is ProtoBasedJavaClassSnapshot -> ProtoBasedJavaClassSnapshotExternalizer.save(output, snapshot) - is ContentHashJavaClassSnapshot -> ContentHashJavaClassSnapshotExternalizer.save(output, snapshot) - } - } - - override fun read(input: DataInput): JavaClassSnapshot { - return when (val className = input.readString()) { - RegularJavaClassSnapshot::class.java.name -> RegularJavaClassSnapshotExternalizer.read(input) - ProtoBasedJavaClassSnapshot::class.java.name -> ProtoBasedJavaClassSnapshotExternalizer.read(input) - ContentHashJavaClassSnapshot::class.java.name -> ContentHashJavaClassSnapshotExternalizer.read(input) - else -> error("Unrecognized class name: $className") - } - } -} - -object RegularJavaClassSnapshotExternalizer : DataExternalizer { - - override fun save(output: DataOutput, snapshot: RegularJavaClassSnapshot) { ClassIdExternalizer.save(output, snapshot.classId) ListExternalizer(JvmClassNameExternalizer).save(output, snapshot.supertypes) AbiSnapshotExternalizer.save(output, snapshot.classAbiExcludingMembers) @@ -194,8 +172,8 @@ object RegularJavaClassSnapshotExternalizer : DataExternalizer { } } -object ProtoBasedJavaClassSnapshotExternalizer : DataExternalizer { - - override fun save(output: DataOutput, snapshot: ProtoBasedJavaClassSnapshot) { - JavaClassProtoMapValueExternalizer.save(output, snapshot.serializedJavaClass) - } - - override fun read(input: DataInput): ProtoBasedJavaClassSnapshot { - return ProtoBasedJavaClassSnapshot(serializedJavaClass = JavaClassProtoMapValueExternalizer.read(input)) - } -} - object InaccessibleClassSnapshotExternalizer : DataExternalizer { override fun save(output: DataOutput, snapshot: InaccessibleClassSnapshot) { @@ -238,14 +205,3 @@ object InaccessibleClassSnapshotExternalizer : DataExternalizer { - - override fun save(output: DataOutput, snapshot: ContentHashJavaClassSnapshot) { - LongExternalizer.save(output, snapshot.contentHash) - } - - override fun read(input: DataInput): ContentHashJavaClassSnapshot { - return ContentHashJavaClassSnapshot(contentHash = LongExternalizer.read(input)) - } -} diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotter.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotter.kt index a68dba8e96f..c8ef77cda7b 100644 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotter.kt +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotter.kt @@ -7,10 +7,8 @@ package org.jetbrains.kotlin.incremental.classpathDiff import org.jetbrains.kotlin.incremental.* import org.jetbrains.kotlin.incremental.ChangesCollector.Companion.getNonPrivateMemberNames -import org.jetbrains.kotlin.load.java.descriptors.JavaClassDescriptor import org.jetbrains.kotlin.load.kotlin.header.KotlinClassHeader.Kind.* import org.jetbrains.kotlin.name.ClassId -import org.jetbrains.kotlin.utils.DFS import java.io.File import java.util.zip.ZipInputStream @@ -20,145 +18,53 @@ object ClasspathEntrySnapshotter { private val DEFAULT_CLASS_FILTER = { unixStyleRelativePath: String, isDirectory: Boolean -> !isDirectory && unixStyleRelativePath.endsWith(".class", ignoreCase = true) - && !unixStyleRelativePath.endsWith("module-info.class", ignoreCase = true) - && !unixStyleRelativePath.startsWith("meta-inf", ignoreCase = true) + && !unixStyleRelativePath.startsWith("meta-inf/", ignoreCase = true) + && !unixStyleRelativePath.equals("module-info.class", ignoreCase = true) } - fun snapshot(classpathEntry: File, protoBased: Boolean? = null): ClasspathEntrySnapshot { - val classes = - DirectoryOrJarContentsReader.read(classpathEntry, DEFAULT_CLASS_FILTER) - .map { (unixStyleRelativePath, contents) -> - ClassFileWithContents(ClassFile(classpathEntry, unixStyleRelativePath), contents) - } + fun snapshot(classpathEntry: File): ClasspathEntrySnapshot { + val classes = DirectoryOrJarContentsReader + .read(classpathEntry, DEFAULT_CLASS_FILTER) + .map { (unixStyleRelativePath, contents) -> + ClassFileWithContents(ClassFile(classpathEntry, unixStyleRelativePath), contents) + } - val snapshots = try { - ClassSnapshotter.snapshot(classes, protoBased).map { it.withHash } - } catch (e: Throwable) { - if ((protoBased ?: protoBasedDefaultValue) && isKnownProblematicClasspathEntry(classpathEntry)) { - classes.map { ContentHashJavaClassSnapshot(it.contents.md5()).withHash } - } else throw e - } + val snapshots = ClassSnapshotter.snapshot(classes).map { it.withHash } - val relativePathsToSnapshotsMap = classes.map { it.classFile.unixStyleRelativePath }.zipToMap(snapshots) + val relativePathsToSnapshotsMap = classes.map { it.classFile.unixStyleRelativePath }.zip(snapshots).toMap(LinkedHashMap()) return ClasspathEntrySnapshot(relativePathsToSnapshotsMap) } - - /** Returns `true` if it is known that the snapshot of the given classpath entry can't be created for some reason. */ - private fun isKnownProblematicClasspathEntry(classpathEntry: File): Boolean { - if (classpathEntry.name.startsWith("tools-jar-api")) { - // [FAULTY JAR] kotlin/dependencies/tools-jar-api/build/libs/tools-jar-api-1.6.255-SNAPSHOT.jar contains class - // com/sun/tools/javac/comp/Infer$GraphStrategy$NodeNotFoundException, but doesn't contain its outer class - // com/sun/tools/javac/comp/Infer$GraphStrategy. - // This happens with a few other similar classes in this jar. - // Therefore, this is a faulty jar, and our snapshotting logic cannot process it. - return true - } - if (classpathEntry.name.startsWith("platform-impl")) { - // ~/.gradle/kotlin-build-dependencies/repo/kotlin.build/ideaIC/203.8084.24/artifacts/lib/platform-impl.jar contains class - // com/intellij/application/options/codeStyle/OptionTableWithPreviewPanel.IntOption. When processing that class, - // BinaryJavaAnnotation$Companion.computeTargetType$resolution_common_jvm in Annotations.kt requires the targetType to be - // JavaClassifierType, but the actual type is PlainJavaPrimitiveType. - // TODO: It's likely that this requirement is incorrect, but let's fix it later. - return true - } - return false - } } /** Creates [ClassSnapshot]s of classes. */ object ClassSnapshotter { /** Creates [ClassSnapshot]s of the given classes. */ - fun snapshot( - classes: List, - protoBased: Boolean? = null, - includeDebugInfoInSnapshot: Boolean? = null - ): List { - // Find inaccessible classes first, their snapshots will be `InaccessibleClassSnapshot`s. + fun snapshot(classes: List, includeDebugInfoInJavaSnapshot: Boolean? = null): List { + // Find inaccessible classes first val classesInfo: List = classes.map { it.classInfo } val inaccessibleClassesInfo: Set = getInaccessibleClasses(classesInfo).toSet() - // Snapshot the remaining accessible classes - val accessibleClasses: List = classes.filter { it.classInfo !in inaccessibleClassesInfo } - val accessibleSnapshots: List = doSnapshot(accessibleClasses, protoBased, includeDebugInfoInSnapshot) - val accessibleClassSnapshots: Map = accessibleClasses.zipToMap(accessibleSnapshots) - - return classes.map { accessibleClassSnapshots[it] ?: InaccessibleClassSnapshot } - } - - private fun doSnapshot( - classes: List, - protoBased: Boolean? = null, - includeDebugInfoInSnapshot: Boolean? = null - ): List { - // Snapshot Kotlin classes first - val kotlinSnapshots: List = classes.map { clazz -> - trySnapshotKotlinClass(clazz) - } - val kotlinClassSnapshots: Map = classes.zipToMap(kotlinSnapshots) - - // Snapshot the remaining Java classes - val javaClasses: List = classes.filter { kotlinClassSnapshots[it] == null } - val javaSnapshots: List = snapshotJavaClasses(javaClasses, protoBased, includeDebugInfoInSnapshot) - val javaClassSnapshots: Map = javaClasses.zipToMap(javaSnapshots) - - return classes.map { kotlinClassSnapshots[it] ?: javaClassSnapshots[it]!! } - } - - /** Creates [KotlinClassSnapshot] of the given class, or returns `null` if the class is not a Kotlin class. */ - private fun trySnapshotKotlinClass(classFile: ClassFileWithContents): KotlinClassSnapshot? { - return if (classFile.classInfo.isKotlinClass) { - val kotlinClassInfo = - KotlinClassInfo.createFrom(classFile.classInfo.classId, classFile.classInfo.kotlinClassHeader!!, classFile.contents) - val packageMembers = when (kotlinClassInfo.classKind) { - CLASS, MULTIFILE_CLASS -> null // See `KotlinClassSnapshot.packageMembers`'s kdoc - else -> (kotlinClassInfo.protoData as PackagePartProtoData).getNonPrivateMemberNames().map { - PackageMember(kotlinClassInfo.classId.packageFqName, it) - } - } - KotlinClassSnapshot(kotlinClassInfo, classFile.classInfo.supertypes, packageMembers) - } else null - } - - /** Creates [JavaClassSnapshot]s of the given Java classes. */ - private fun snapshotJavaClasses( - classes: List, - protoBased: Boolean? = null, - includeDebugInfoInSnapshot: Boolean? = null - ): List { - return if (protoBased ?: protoBasedDefaultValue) { - snapshotJavaClassesProtoBased(classes) - } else { - classes.map { JavaClassSnapshotter.snapshot(it, includeDebugInfoInSnapshot) } - } - } - - private fun snapshotJavaClassesProtoBased(classFilesWithContents: List): List { - val classIds = classFilesWithContents.map { it.classInfo.classId } - val classesContents = classFilesWithContents.map { it.contents } - val descriptors: List = JavaClassDescriptorCreator.create(classIds, classesContents) - val snapshots: List = descriptors.mapIndexed { index, descriptor -> - val classFileWithContents = classFilesWithContents[index] - if (descriptor != null) { - try { - ProtoBasedJavaClassSnapshot(descriptor.toSerializedJavaClass()) - } catch (e: Throwable) { - if (isKnownExceptionWhenReadingDescriptor(e)) { - ContentHashJavaClassSnapshot(classFileWithContents.contents.md5()) - } else throw e - } - } else { - if (isKnownProblematicClass(classFileWithContents.classFile)) { - ContentHashJavaClassSnapshot(classFileWithContents.contents.md5()) - } else { - error( - "Failed to create JavaClassDescriptor for class '${classFileWithContents.classFile.unixStyleRelativePath}'" + - " in '${classFileWithContents.classFile.classRoot.path}'" - ) - } + return classes.map { + when { + it.classInfo in inaccessibleClassesInfo -> InaccessibleClassSnapshot + it.classInfo.isKotlinClass -> snapshotKotlinClass(it) + else -> JavaClassSnapshotter.snapshot(it, includeDebugInfoInJavaSnapshot) } } - return snapshots + } + + /** Creates [KotlinClassSnapshot] of the given Kotlin class (the caller must ensure that the given class is a Kotlin class). */ + private fun snapshotKotlinClass(classFile: ClassFileWithContents): KotlinClassSnapshot { + val kotlinClassInfo = + KotlinClassInfo.createFrom(classFile.classInfo.classId, classFile.classInfo.kotlinClassHeader!!, classFile.contents) + val packageMembers = when (kotlinClassInfo.classKind) { + CLASS, MULTIFILE_CLASS -> null // See `KotlinClassSnapshot.packageMembers`'s kdoc + else -> (kotlinClassInfo.protoData as PackagePartProtoData).getNonPrivateMemberNames().map { + PackageMember(kotlinClassInfo.classId.packageFqName, it) + } + } + return KotlinClassSnapshot(kotlinClassInfo, classFile.classInfo.supertypes, packageMembers) } /** @@ -208,57 +114,6 @@ object ClassSnapshotter { return classesInfo.filter { it.isTransitivelyInaccessible() } } - - /** Returns `true` if it is known that the given exception can be thrown when calling [JavaClassDescriptor.toSerializedJavaClass]. */ - private fun isKnownExceptionWhenReadingDescriptor(throwable: Throwable): Boolean { - // When building the Kotlin repo with `./gradlew publish -Pbootstrap.local=true - // -Pbootstrap.local.path=/path/to/kotlin/build/repo -Pkotlin.incremental.useClasspathSnapshot=true`, the build can fail with: - // org.gradle.api.internal.artifacts.transform.TransformException: Execution failed for ClasspathEntrySnapshotTransform: ~/.gradle/wrapper/dists/gradle-6.9-bin/2ecsmyp3bolyybemj56vfn4mt/gradle-6.9/lib/kotlin-reflect-1.4.20.jar - // Caused by: java.lang.IncompatibleClassChangeError: Expected static method 'java.lang.Object org.jetbrains.kotlin.utils.DFS.dfsFromNode(java.lang.Object, org.jetbrains.kotlin.utils.DFS$Neighbors, org.jetbrains.kotlin.utils.DFS$Visited, org.jetbrains.kotlin.utils.DFS$NodeHandler)' - // at org.jetbrains.kotlin.builtins.FunctionTypesKt.isTypeOrSubtypeOf(functionTypes.kt:31) - // (... at JavaClassDescriptor.toSerializedJavaClass) - // The reason is that: - // - org/jetbrains/kotlin/builtins/FunctionTypesKt.class is located in ~/.gradle/caches/jars-8/66425fb82fd14126e9aa07dcd3100b42/kotlin-compiler-embeddable-1.6.255-20210909.213620-55.jar - // - org/jetbrains/kotlin/utils/DFS.class is located in ~/.gradle/caches/jars-8/c716e2e2d26b16f6f1462e59ba44cf3b/buildSrc.jar - // And somehow the two classes are incompatible (probably similar to the NoSuchMethodError documented at JavaClassDescriptorCreatorKt.createBinaryJavaClass). - // This happens to a few other jars inside gradle-6.9/lib. - // However, outside the Kotlin repo build, we don't have this issue (org/jetbrains/kotlin/builtins/FunctionTypesKt.class and - // org/jetbrains/kotlin/utils/DFS.class will be located in the same kotlin-compiler-embeddable.jar). - // Therefore, we special-case the Kotlin repo build below. - // TODO: See how we can address this issue. - return (throwable is IncompatibleClassChangeError && - DFS::class.java.classLoader.getResource(DFS::class.java.name.replace('.', '/') + ".class") - ?.path?.contains("buildSrc.jar") == true - ) - } - - /** Returns `true` if it is known that the snapshot of the given class can't be created for some reason. */ - private fun isKnownProblematicClass(classFile: ClassFile): Boolean { - if (classFile.classRoot.name.startsWith("groovy") - && classFile.unixStyleRelativePath.endsWith("\$CollectorHelper.class") - ) { - // [FAULTY JAR] In groovy-all-1.3-2.5.12.jar and groovy-2.5.11.jar, the bytecode of - // groovy/cli/OptionField\$CollectorHelper.class indicates that its outer class is groovy/cli/OptionField, but the bytecode of - // groovy/cli/OptionField.class does not list any nested classes. - // This happens with a few other CollectorHelper classes in these jars. - // Therefore, these are faulty jars, and our snapshotting logic cannot process it. - return true - } - if (classFile.classRoot.name.startsWith("gradle-api") - && classFile.unixStyleRelativePath.startsWith("org/gradle/internal/impldep/META-INF/versions") - ) { - // [FAULTY JAR] gradle-api-6.9.jar has the following entries: - // - org/gradle/internal/impldep/org/junit/platform/commons/util/ModuleUtils.class - // - org/gradle/internal/impldep/META-INF/versions/9/org/junit/platform/commons/util/ModulesUtils.class - // - org/gradle/internal/impldep/META-INF/versions/9/org/junit/platform/commons/util/ModuleUtils$ModuleReferenceScanner.class - // The META-INF directories are located not at the top level (which is not expected), and those directories escaped our filter - // which filters out top-level META-INF directories. We then failed to snapshot ModuleUtils$ModuleReferenceScanner.class as - // there are 2 versions of ModuleUtils.class, and the one outside the META-INF directory doesn't have any nested classes. - // Therefore, this is a faulty jar, and our snapshotting logic cannot process it. - return true - } - return false - } } /** Utility to read the contents of a directory or jar. */ @@ -317,20 +172,3 @@ private object DirectoryOrJarContentsReader { return relativePathsToContents.toSortedMap().toMap(LinkedHashMap()) } } - -/** - * Combines two lists of the same size into a map. - * - * This method is more efficient than calling `[Iterable.zip].toMap()` as it doesn't create short-lived intermediate [Pair]s as done by - * [Iterable.zip]. - */ -private fun List.zipToMap(other: List): LinkedHashMap { - check(this.size == other.size) - val map = LinkedHashMap(size) - indices.forEach { index -> - map[this[index]] = other[index] - } - return map -} - -private const val protoBasedDefaultValue = false \ No newline at end of file diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/JavaClassChangesComputer.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/JavaClassChangesComputer.kt index 0af4f04c385..57a3d1fc7ec 100644 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/JavaClassChangesComputer.kt +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/JavaClassChangesComputer.kt @@ -18,11 +18,11 @@ object JavaClassChangesComputer { * Each list must not contain duplicate classes (having the same [JvmClassName]/[ClassId]). */ fun compute( - currentJavaClassSnapshots: List, - previousJavaClassSnapshots: List + currentJavaClassSnapshots: List, + previousJavaClassSnapshots: List ): ChangeSet { - val currentClasses: Map = currentJavaClassSnapshots.associateBy { it.classId } - val previousClasses: Map = previousJavaClassSnapshots.associateBy { it.classId } + val currentClasses: Map = currentJavaClassSnapshots.associateBy { it.classId } + val previousClasses: Map = previousJavaClassSnapshots.associateBy { it.classId } // Note: Added classes can also impact recompilation. // For example, suppose a source file uses `SomeClass` through `*` imports: @@ -50,8 +50,8 @@ object JavaClassChangesComputer { * The two classes must have the same [ClassId]. */ private fun collectClassChanges( - currentClassSnapshot: RegularJavaClassSnapshot, - previousClassSnapshot: RegularJavaClassSnapshot, + currentClassSnapshot: JavaClassSnapshot, + previousClassSnapshot: JavaClassSnapshot, changes: ChangeSet.Collector ) { val classId = currentClassSnapshot.classId.also { check(it == previousClassSnapshot.classId) } diff --git a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/JavaClassSnapshotter.kt b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/JavaClassSnapshotter.kt index 92a10707aa7..2f09e48757e 100644 --- a/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/JavaClassSnapshotter.kt +++ b/compiler/incremental-compilation-impl/src/org/jetbrains/kotlin/incremental/classpathDiff/JavaClassSnapshotter.kt @@ -52,7 +52,7 @@ object JavaClassSnapshotter { abiClass.methods.clear() val classAbiExcludingMembers = abiClass.let { snapshotJavaElement(it, it.name, includeDebugInfoInSnapshot) } - return RegularJavaClassSnapshot( + return JavaClassSnapshot( classFile.classInfo.classId, classFile.classInfo.supertypes, classAbiExcludingMembers, fieldsAbi, methodsAbi ) 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 8c56f227f95..9985351fd1d 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 @@ -13,8 +13,6 @@ import org.jetbrains.kotlin.incremental.classpathDiff.ClasspathSnapshotTestCommo import org.jetbrains.kotlin.resolve.sam.SAM_LOOKUP_NAME import org.junit.Test import org.junit.rules.TemporaryFolder -import org.junit.runner.RunWith -import org.junit.runners.Parameterized import java.io.File import kotlin.test.fail @@ -185,18 +183,11 @@ class KotlinOnlyClasspathChangesComputerTest : ClasspathChangesComputerTest() { } } -@RunWith(Parameterized::class) -class JavaOnlyClasspathChangesComputerTest(private val protoBased: Boolean) : ClasspathChangesComputerTest() { - - companion object { - @Parameterized.Parameters(name = "protoBased={0}") - @JvmStatic - fun parameters() = listOf(true, false) - } +class JavaOnlyClasspathChangesComputerTest : ClasspathChangesComputerTest() { @Test override fun testAbiVersusNonAbiChanges() { - val changes = computeClasspathChanges(File(testDataDir, "testAbiVersusNonAbiChanges/src/java"), tmpDir, protoBased) + val changes = computeClasspathChanges(File(testDataDir, "testAbiVersusNonAbiChanges/src/java"), tmpDir) Changes( lookupSymbols = setOf( LookupSymbol(name = "publicFieldChangedType", scope = "com.example.SomeClass"), @@ -209,7 +200,7 @@ class JavaOnlyClasspathChangesComputerTest(private val protoBased: Boolean) : Cl @Test override fun testModifiedAddedRemovedElements() { - val changes = computeClasspathChanges(File(testDataDir, "testModifiedAddedRemovedElements/src/java"), tmpDir, protoBased) + val changes = computeClasspathChanges(File(testDataDir, "testModifiedAddedRemovedElements/src/java"), tmpDir) Changes( lookupSymbols = setOf( // ModifiedClassUnchangedMembers @@ -241,7 +232,7 @@ class JavaOnlyClasspathChangesComputerTest(private val protoBased: Boolean) : Cl @Test override fun testImpactAnalysis() { - val changes = computeClasspathChanges(File(testDataDir, "testImpactAnalysis_JavaOnly/src"), tmpDir, protoBased) + val changes = computeClasspathChanges(File(testDataDir, "testImpactAnalysis_JavaOnly/src"), tmpDir) Changes( lookupSymbols = setOf( LookupSymbol(name = "changedField", scope = "com.example.ChangedSuperClass"), @@ -305,20 +296,20 @@ class KotlinAndJavaClasspathChangesComputerTest : ClasspathSnapshotTestCommon() } } -private fun computeClasspathChanges(classpathSourceDir: File, tmpDir: TemporaryFolder, protoBased: Boolean = false): Changes { - val currentSnapshot = snapshotClasspath(File(classpathSourceDir, "current-classpath"), tmpDir, protoBased) - val previousSnapshot = snapshotClasspath(File(classpathSourceDir, "previous-classpath"), tmpDir, protoBased) +private fun computeClasspathChanges(classpathSourceDir: File, tmpDir: TemporaryFolder): Changes { + val currentSnapshot = snapshotClasspath(File(classpathSourceDir, "current-classpath"), tmpDir) + val previousSnapshot = snapshotClasspath(File(classpathSourceDir, "previous-classpath"), tmpDir) return computeClasspathChanges(currentSnapshot, previousSnapshot) } -private fun snapshotClasspath(classpathSourceDir: File, tmpDir: TemporaryFolder, protoBased: Boolean): ClasspathSnapshot { +private fun snapshotClasspath(classpathSourceDir: File, tmpDir: TemporaryFolder): ClasspathSnapshot { val classpath = mutableListOf() val classpathEntrySnapshots = classpathSourceDir.listFiles()!!.sortedBy { it.name }.map { classpathEntrySourceDir -> val classFiles = compileAll(classpathEntrySourceDir, classpath, tmpDir) classpath.addAll(listOfNotNull(classFiles.firstOrNull()?.classRoot)) val relativePaths = classFiles.map { it.unixStyleRelativePath } - val classSnapshots = classFiles.snapshot(protoBased).map { it.withHash } + val classSnapshots = classFiles.snapshot().map { it.withHash } ClasspathEntrySnapshot( classSnapshots = relativePaths.zip(classSnapshots).toMap(LinkedHashMap()) ) diff --git a/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotTestCommon.kt b/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotTestCommon.kt index 7997dde853d..8d728b6f0f3 100644 --- a/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotTestCommon.kt +++ b/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotTestCommon.kt @@ -164,11 +164,11 @@ abstract class ClasspathSnapshotTestCommon { fun ClassFile.readBytes() = asFile().readBytes() - fun ClassFile.snapshot(protoBased: Boolean? = null): ClassSnapshot = listOf(this).snapshot(protoBased).single() + fun ClassFile.snapshot(): ClassSnapshot = listOf(this).snapshot().single() - fun List.snapshot(protoBased: Boolean? = null): List { + fun List.snapshot(): List { val classFilesWithContents = this.map { ClassFileWithContents(it, it.readBytes()) } - return ClassSnapshotter.snapshot(classFilesWithContents, protoBased) + return ClassSnapshotter.snapshot(classFilesWithContents) } } } diff --git a/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotterTest.kt b/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotterTest.kt index b7d20a8b587..c3c26fe3b49 100644 --- a/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotterTest.kt +++ b/compiler/incremental-compilation-impl/test/org/jetbrains/kotlin/incremental/classpathDiff/ClasspathSnapshotterTest.kt @@ -34,7 +34,7 @@ abstract class ClasspathSnapshotterTest : ClasspathSnapshotTestCommon() { @Test fun `test ClassSnapshotter's result against expected snapshot`() { val classSnapshot = sourceFile.compileSingle().let { - ClassSnapshotter.snapshot(listOf(ClassFileWithContents(it, it.readBytes())), includeDebugInfoInSnapshot = true) + ClassSnapshotter.snapshot(listOf(ClassFileWithContents(it, it.readBytes())), includeDebugInfoInJavaSnapshot = true) }.single() assertEquals(expectedSnapshotFile.readText(), classSnapshot.toGson()) diff --git a/compiler/resolution.common.jvm/src/org/jetbrains/kotlin/load/java/structure/impl/classFiles/BinaryJavaClass.kt b/compiler/resolution.common.jvm/src/org/jetbrains/kotlin/load/java/structure/impl/classFiles/BinaryJavaClass.kt index e8dcebf51e7..af5f21e3a23 100644 --- a/compiler/resolution.common.jvm/src/org/jetbrains/kotlin/load/java/structure/impl/classFiles/BinaryJavaClass.kt +++ b/compiler/resolution.common.jvm/src/org/jetbrains/kotlin/load/java/structure/impl/classFiles/BinaryJavaClass.kt @@ -32,15 +32,13 @@ import java.text.CharacterIterator import java.text.StringCharacterIterator class BinaryJavaClass( - /** If virtualFile is not available, provide classContent & innerClassFinder below. */ - override val virtualFile: VirtualFile? = null, + override val virtualFile: VirtualFile, override val fqName: FqName, internal val context: ClassifierResolutionContext, private val signatureParser: BinaryClassSignatureParser, override var access: Int = 0, override val outerClass: JavaClass?, - classContent: ByteArray? = null, - private val innerClassFinder: ((Name) -> JavaClass?)? = null + classContent: ByteArray? = null ) : ClassVisitor(ASM_API_VERSION_FOR_CLASS_READING), VirtualFileBoundJavaClass, BinaryJavaModifierListOwner, MutableJavaAnnotationOwner { override val annotations: MutableCollection = SmartList() @@ -113,17 +111,13 @@ class BinaryJavaClass( } init { - if (virtualFile == null) { - checkNotNull(classContent) { "classContent must be provided when virtualFile is not available" } - checkNotNull(innerClassFinder) { "innerClassFinder must be provided when virtualFile is not available" } - } try { - ClassReader(classContent ?: virtualFile!!.contentsToByteArray()).accept( + ClassReader(classContent ?: virtualFile.contentsToByteArray()).accept( this, ClassReader.SKIP_CODE or ClassReader.SKIP_FRAMES ) } catch (e: Throwable) { - throw IllegalStateException("Could not read class " + (virtualFile ?: ""), e) + throw IllegalStateException("Could not read class: $virtualFile", e) } } @@ -253,15 +247,11 @@ class BinaryJavaClass( fun findInnerClass(name: Name, classFileContent: ByteArray?): JavaClass? { val access = ownInnerClassNameToAccess[name] ?: return null - return if (virtualFile != null) { - virtualFile.parent.findChild("${virtualFile.nameWithoutExtension}$$name.class")?.let { - BinaryJavaClass( - it, fqName.child(name), context.copyForMember(), signatureParser, access, this, - classFileContent - ) - } - } else { - innerClassFinder!!(name) + return virtualFile.parent.findChild("${virtualFile.nameWithoutExtension}$$name.class")?.let { + BinaryJavaClass( + it, fqName.child(name), context.copyForMember(), signatureParser, access, this, + classFileContent + ) } }