From ecdd80769e229c3cef711db733e50f9b2a91708e Mon Sep 17 00:00:00 2001 From: pyos Date: Tue, 23 Aug 2022 14:17:27 +0200 Subject: [PATCH] FIR: do not read a Java nested class if the outer class is Kotlin Example: if C is a Kotlin class and there is an import of C.Companion.f, a call to f will need to check if it is a constructor, for which it will attempt to load C.Companion.f as a Java class. This involves loading all outer classes, including C itself, as BinaryJavaClass - see KotlinCliJavaFileManagerImpl - despite the fact that C and C.Companion have both already been loaded from their Metadata annotations as Kotlin classes, wasting cycles and polluting caches. Thus the theoretical motivation for this change is to marginally speed up FIR. The real motivation is that with the -Xemit-jvm-type-annotations option, kotlinc sometimes generates invalid annotations, and reading them with BinaryJavaClass throws an exception (the Kotlin metadata however is valid): // MODULE: lib // FILE: C.kt // The constructor has an argument of type // List> // and a ParameterName type annotation with path // TYPE_ARGUMENT(0), TYPE_ARGUMENT(0) // which is missing a WILDCARD between the two TYPE_ARGUMENTs class C(val x: List<(name: String) -> Int>) { companion object { fun foo() {} } } // MODULE: main(lib) // FILE: main.kt import C.Companion.f fun bar() = foo() // crashes FIR The parameter annotation should obviously be fixed too, but invalid bytecode may already exist in the wild. Also, did I mention that this change marginally speeds up FIR? --- .../JvmClassFileBasedSymbolProvider.kt | 24 ++++++++++--------- .../org/jetbrains/kotlin/name/ClassId.java | 7 ++++++ 2 files changed, 20 insertions(+), 11 deletions(-) diff --git a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/deserialization/JvmClassFileBasedSymbolProvider.kt b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/deserialization/JvmClassFileBasedSymbolProvider.kt index b4b187895df..5a7cf90d955 100644 --- a/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/deserialization/JvmClassFileBasedSymbolProvider.kt +++ b/compiler/fir/java/src/org/jetbrains/kotlin/fir/java/deserialization/JvmClassFileBasedSymbolProvider.kt @@ -14,6 +14,7 @@ import org.jetbrains.kotlin.fir.declarations.getDeprecationsProvider import org.jetbrains.kotlin.fir.deserialization.* import org.jetbrains.kotlin.fir.expressions.FirAnnotation import org.jetbrains.kotlin.fir.java.FirJavaFacade +import org.jetbrains.kotlin.fir.java.declarations.FirJavaClass import org.jetbrains.kotlin.fir.scopes.FirKotlinScopeProvider import org.jetbrains.kotlin.fir.symbols.impl.FirRegularClassSymbol import org.jetbrains.kotlin.load.kotlin.* @@ -97,23 +98,24 @@ class JvmClassFileBasedSymbolProvider( private val KotlinJvmBinaryClass.isPreReleaseInvisible: Boolean get() = classHeader.isPreRelease - private fun loadJavaClass(classId: ClassId, content: ByteArray?): ClassMetadataFindResult? { - val javaClass = javaFacade.findClass(classId, content) ?: return null - return ClassMetadataFindResult.NoMetadata { symbol -> - javaFacade.convertJavaClassToFir(symbol, classId.outerClassId?.let(::getClass), javaClass) - } - } - override fun extractClassMetadata(classId: ClassId, parentContext: FirDeserializationContext?): ClassMetadataFindResult? { // Kotlin classes are annotated Java classes, so this check also looks for them. if (!javaFacade.hasTopLevelClassOf(classId)) return null val result = kotlinClassFinder.findKotlinClassOrContent(classId) - val kotlinClass = when (result) { - is KotlinClassFinder.Result.KotlinClass -> result.kotlinJvmBinaryClass - is KotlinClassFinder.Result.ClassFileContent -> return loadJavaClass(classId, result.content) - null -> return loadJavaClass(classId, null) + if (result !is KotlinClassFinder.Result.KotlinClass) { + if (parentContext != null || (classId.isNestedClass && getClass(classId.outermostClassId)?.fir !is FirJavaClass)) { + // Nested class of Kotlin class should have been a Kotlin class. + return null + } + val javaClass = javaFacade.findClass(classId, (result as? KotlinClassFinder.Result.ClassFileContent)?.content) + ?: return null + return ClassMetadataFindResult.NoMetadata { symbol -> + javaFacade.convertJavaClassToFir(symbol, classId.outerClassId?.let(::getClass), javaClass) + } } + + val kotlinClass = result.kotlinJvmBinaryClass if (kotlinClass.classHeader.kind != KotlinClassHeader.Kind.CLASS) return null val data = kotlinClass.classHeader.data ?: return null val strings = kotlinClass.classHeader.strings ?: return null diff --git a/core/compiler.common/src/org/jetbrains/kotlin/name/ClassId.java b/core/compiler.common/src/org/jetbrains/kotlin/name/ClassId.java index 03fc88fa9ff..318f9a1e568 100644 --- a/core/compiler.common/src/org/jetbrains/kotlin/name/ClassId.java +++ b/core/compiler.common/src/org/jetbrains/kotlin/name/ClassId.java @@ -89,6 +89,13 @@ public final class ClassId { return parent.isRoot() ? null : new ClassId(getPackageFqName(), parent, local); } + @NotNull + public ClassId getOutermostClassId() { + FqName name = relativeClassName; + while (!name.parent().isRoot()) name = name.parent(); + return new ClassId(packageFqName, name, false); + } + public boolean isNestedClass() { return !relativeClassName.parent().isRoot(); }