From ad9c1001371be83aa62ab4eb520872c88f511737 Mon Sep 17 00:00:00 2001 From: Vladimir Tagakov Date: Tue, 26 Dec 2023 17:44:12 -0700 Subject: [PATCH] Sort class members in jvm-abi-gen #KT-64589 Fixed --- .../kotlin/jvm/abi/JvmAbiMetadataProcessor.kt | 8 +-- .../kotlin/jvm/abi/JvmAbiOutputExtension.kt | 52 ++++++++++++------- .../jvm/abi/CompareJvmAbiTestGenerated.java | 10 ++++ .../testData/compare/fieldOrder/base/test.kt | 11 ++++ .../compare/fieldOrder/sameAbi/test.kt | 10 ++++ .../testData/compare/funOrder/base/test.kt | 8 +++ .../testData/compare/funOrder/sameAbi/test.kt | 7 +++ .../anonymousWhenMapping/signatures.txt | 4 +- .../testData/content/class/signatures.txt | 8 +-- .../testData/content/kt50005/signatures.txt | 10 ++-- .../content/whenMapping/signatures.txt | 4 +- 11 files changed, 95 insertions(+), 37 deletions(-) create mode 100644 plugins/jvm-abi-gen/testData/compare/fieldOrder/base/test.kt create mode 100644 plugins/jvm-abi-gen/testData/compare/fieldOrder/sameAbi/test.kt create mode 100644 plugins/jvm-abi-gen/testData/compare/funOrder/base/test.kt create mode 100644 plugins/jvm-abi-gen/testData/compare/funOrder/sameAbi/test.kt diff --git a/plugins/jvm-abi-gen/src/org/jetbrains/kotlin/jvm/abi/JvmAbiMetadataProcessor.kt b/plugins/jvm-abi-gen/src/org/jetbrains/kotlin/jvm/abi/JvmAbiMetadataProcessor.kt index fdf9774719a..1e31406f51c 100644 --- a/plugins/jvm-abi-gen/src/org/jetbrains/kotlin/jvm/abi/JvmAbiMetadataProcessor.kt +++ b/plugins/jvm-abi-gen/src/org/jetbrains/kotlin/jvm/abi/JvmAbiMetadataProcessor.kt @@ -6,10 +6,7 @@ package org.jetbrains.kotlin.jvm.abi import kotlinx.metadata.* -import kotlinx.metadata.jvm.JvmMetadataVersion -import kotlinx.metadata.jvm.KotlinClassMetadata -import kotlinx.metadata.jvm.Metadata -import kotlinx.metadata.jvm.localDelegatedProperties +import kotlinx.metadata.jvm.* import org.jetbrains.kotlin.load.java.JvmAnnotationNames.* import org.jetbrains.org.objectweb.asm.AnnotationVisitor import org.jetbrains.org.objectweb.asm.Opcodes @@ -166,6 +163,9 @@ private fun KmDeclarationContainer.removePrivateDeclarations() { functions.removeIf { it.visibility.isPrivate } properties.removeIf { it.visibility.isPrivate } + functions.sortWith(compareBy(KmFunction::name, { it.signature.toString() })) + properties.sortWith(compareBy(KmProperty::name, { it.getterSignature.toString() })) + for (property in properties) { // Whether or not the *non-const* property is initialized by a compile-time constant is not a part of the ABI. if (!property.isConst) { diff --git a/plugins/jvm-abi-gen/src/org/jetbrains/kotlin/jvm/abi/JvmAbiOutputExtension.kt b/plugins/jvm-abi-gen/src/org/jetbrains/kotlin/jvm/abi/JvmAbiOutputExtension.kt index 0a0e8b5843b..c494ad77462 100644 --- a/plugins/jvm-abi-gen/src/org/jetbrains/kotlin/jvm/abi/JvmAbiOutputExtension.kt +++ b/plugins/jvm-abi-gen/src/org/jetbrains/kotlin/jvm/abi/JvmAbiOutputExtension.kt @@ -18,6 +18,8 @@ import org.jetbrains.org.objectweb.asm.* import org.jetbrains.org.objectweb.asm.commons.ClassRemapper import org.jetbrains.org.objectweb.asm.commons.Method import org.jetbrains.org.objectweb.asm.commons.Remapper +import org.jetbrains.org.objectweb.asm.tree.FieldNode +import org.jetbrains.org.objectweb.asm.tree.MethodNode import java.io.File class JvmAbiOutputExtension( @@ -76,7 +78,6 @@ class JvmAbiOutputExtension( else -> /* abiInfo is AbiClassInfo.Stripped */ { val methodInfo = (abiInfo as AbiClassInfo.Stripped).methodInfo - val innerClassInfos = mutableMapOf() val innerClassesToKeep = mutableSetOf() val writer = ClassWriter(0) val remapper = ClassRemapper(writer, object : Remapper() { @@ -84,6 +85,10 @@ class JvmAbiOutputExtension( internalName.also { innerClassesToKeep.add(it) } }) ClassReader(outputFile.asByteArray()).accept(object : ClassVisitor(Opcodes.API_VERSION, remapper) { + private val keptFields = mutableListOf() + private val keptMethods = mutableListOf() + private val innerClassInfos = mutableMapOf() + // Strip private fields. override fun visitField( access: Int, @@ -94,7 +99,9 @@ class JvmAbiOutputExtension( ): FieldVisitor? { if (access and Opcodes.ACC_PRIVATE != 0) return null - return super.visitField(access, name, descriptor, signature, value) + return FieldNode(access, name, descriptor, signature, value).also { + keptFields += it + } } override fun visitMethod( @@ -104,15 +111,17 @@ class JvmAbiOutputExtension( signature: String?, exceptions: Array? ): MethodVisitor? { - val info = methodInfo[Method(name, descriptor)] - ?: return null + val info = methodInfo[Method(name, descriptor)] ?: return null - val visitor = super.visitMethod(access, name, descriptor, signature, exceptions) - if (info == AbiMethodInfo.KEEP || access and (Opcodes.ACC_NATIVE or Opcodes.ACC_ABSTRACT) != 0) { - return if (removeDebugInfo) DebugInfoRemovingMethodVisitor(visitor) else visitor + val node = MethodNode(access, name, descriptor, signature, exceptions).also { + keptMethods += it } - return BodyStrippingMethodVisitor(visitor) + if (info == AbiMethodInfo.KEEP || access and (Opcodes.ACC_NATIVE or Opcodes.ACC_ABSTRACT) != 0) { + return if (removeDebugInfo) DebugInfoRemovingMethodVisitor(node) else node + } + + return BodyStrippingMethodVisitor(node) } override fun visitSource(source: String?, debug: String?) { @@ -145,20 +154,23 @@ class JvmAbiOutputExtension( return abiMetadataProcessor(delegate) } - override fun visitEnd() {} + override fun visitEnd() { + // Output class members in sorted order so that changes in original ordering don't affect the ABI JAR. + + keptFields.sortedWith(compareBy(FieldNode::name, FieldNode::desc)).forEach { it.accept(cv) } + keptMethods.sortedWith(compareBy(MethodNode::name, MethodNode::desc)).forEach { it.accept(cv) } + + innerClassesToKeep.addInnerClasses(innerClassInfos, internalName) + innerClassesToKeep.addOuterClasses(innerClassInfos) + + for (name in innerClassesToKeep.sorted()) { + innerClassInfos[name]?.let { cv.visitInnerClass(it.name, it.outerName, it.innerName, it.access) } + } + + super.visitEnd() + } }, 0) - innerClassesToKeep.addInnerClasses(innerClassInfos, internalName) - innerClassesToKeep.addOuterClasses(innerClassInfos) - - // Output classes in sorted order so that changes in original ordering due to method bodies, etc. - // don't affect the ABI JAR. - for (name in innerClassesToKeep.sorted()) { - innerClassInfos[name]?.let { writer.visitInnerClass(it.name, it.outerName, it.innerName, it.access) } - } - - writer.visitEnd() - SimpleOutputBinaryFile(outputFile.sourceFiles, outputFile.relativePath, writer.toByteArray()) } } diff --git a/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/CompareJvmAbiTestGenerated.java b/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/CompareJvmAbiTestGenerated.java index f2254fc5614..886cc62cb89 100644 --- a/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/CompareJvmAbiTestGenerated.java +++ b/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/CompareJvmAbiTestGenerated.java @@ -90,6 +90,16 @@ public class CompareJvmAbiTestGenerated extends AbstractCompareJvmAbiTest { runTest("plugins/jvm-abi-gen/testData/compare/declarationOrderPrivateInline/"); } + @TestMetadata("fieldOrder") + public void testFieldOrder() throws Exception { + runTest("plugins/jvm-abi-gen/testData/compare/fieldOrder/"); + } + + @TestMetadata("funOrder") + public void testFunOrder() throws Exception { + runTest("plugins/jvm-abi-gen/testData/compare/funOrder/"); + } + @TestMetadata("functionBody") public void testFunctionBody() throws Exception { runTest("plugins/jvm-abi-gen/testData/compare/functionBody/"); diff --git a/plugins/jvm-abi-gen/testData/compare/fieldOrder/base/test.kt b/plugins/jvm-abi-gen/testData/compare/fieldOrder/base/test.kt new file mode 100644 index 00000000000..98e85966d28 --- /dev/null +++ b/plugins/jvm-abi-gen/testData/compare/fieldOrder/base/test.kt @@ -0,0 +1,11 @@ +package test + + +class Class { + @JvmField + val first = Unit + @JvmField + val second = Unit + val String.first + get() = this +} diff --git a/plugins/jvm-abi-gen/testData/compare/fieldOrder/sameAbi/test.kt b/plugins/jvm-abi-gen/testData/compare/fieldOrder/sameAbi/test.kt new file mode 100644 index 00000000000..41873015700 --- /dev/null +++ b/plugins/jvm-abi-gen/testData/compare/fieldOrder/sameAbi/test.kt @@ -0,0 +1,10 @@ +package test + +class Class { + val String.first + get() = this + @JvmField + val second = Unit + @JvmField + val first = Unit +} diff --git a/plugins/jvm-abi-gen/testData/compare/funOrder/base/test.kt b/plugins/jvm-abi-gen/testData/compare/funOrder/base/test.kt new file mode 100644 index 00000000000..7afa27f9612 --- /dev/null +++ b/plugins/jvm-abi-gen/testData/compare/funOrder/base/test.kt @@ -0,0 +1,8 @@ +package test + + +class Class { + fun first() = Unit + fun first(a: Any) = Unit + fun second() = Unit +} diff --git a/plugins/jvm-abi-gen/testData/compare/funOrder/sameAbi/test.kt b/plugins/jvm-abi-gen/testData/compare/funOrder/sameAbi/test.kt new file mode 100644 index 00000000000..1abd4dae381 --- /dev/null +++ b/plugins/jvm-abi-gen/testData/compare/funOrder/sameAbi/test.kt @@ -0,0 +1,7 @@ +package test + +class Class { + fun second() = Unit + fun first(a: Any) = Unit + fun first() = Unit +} diff --git a/plugins/jvm-abi-gen/testData/content/anonymousWhenMapping/signatures.txt b/plugins/jvm-abi-gen/testData/content/anonymousWhenMapping/signatures.txt index f56ede30977..78de3c0f0a9 100644 --- a/plugins/jvm-abi-gen/testData/content/anonymousWhenMapping/signatures.txt +++ b/plugins/jvm-abi-gen/testData/content/anonymousWhenMapping/signatures.txt @@ -3,9 +3,9 @@ public final enum class test/E { // source: 'test.kt' public final enum static field A: test.E public final enum static field B: test.E - public static method values(): test.E[] - public static method valueOf(p0: java.lang.String): test.E public static @org.jetbrains.annotations.NotNull method getEntries(): kotlin.enums.EnumEntries + public static method valueOf(p0: java.lang.String): test.E + public static method values(): test.E[] } @kotlin.Metadata public final class test/Test { diff --git a/plugins/jvm-abi-gen/testData/content/class/signatures.txt b/plugins/jvm-abi-gen/testData/content/class/signatures.txt index 9c5e866a3d1..4d3fd69f9c1 100644 --- a/plugins/jvm-abi-gen/testData/content/class/signatures.txt +++ b/plugins/jvm-abi-gen/testData/content/class/signatures.txt @@ -11,12 +11,12 @@ public class test/BaseClass { public final static @org.jetbrains.annotations.NotNull field Companion: test.BaseClass$Companion public final static field basePublicConst: int public method (): void - public method getBaseClassPublicVal(): int + public final method baseClassInternalFun$main(): int + protected final method baseClassProtectedFun(): int public method baseClassPublicFun(): int public final method getBaseClassInternalVal$main(): int - public final method baseClassInternalFun$main(): int protected final method getBaseClassProtectedVal(): int - protected final method baseClassProtectedFun(): int + public method getBaseClassPublicVal(): int } @kotlin.Metadata public final class test/Class$NestedInnerClass$NestedNestedInnerClass { @@ -47,4 +47,4 @@ public interface test/Interface { final class test/PrivateClass { // source: 'classes.kt' public method (): void -} \ No newline at end of file +} diff --git a/plugins/jvm-abi-gen/testData/content/kt50005/signatures.txt b/plugins/jvm-abi-gen/testData/content/kt50005/signatures.txt index f81f9c86161..672befdab25 100644 --- a/plugins/jvm-abi-gen/testData/content/kt50005/signatures.txt +++ b/plugins/jvm-abi-gen/testData/content/kt50005/signatures.txt @@ -1,12 +1,12 @@ @kotlin.Metadata public final class test/A { // source: 'test.kt' - public final @kotlin.jvm.JvmField field b: int public field a: java.lang.String + public final @kotlin.jvm.JvmField field b: int public method (): void - public final @org.jetbrains.annotations.NotNull method getA(): java.lang.String - public final method setA(@org.jetbrains.annotations.NotNull p0: java.lang.String): void - public final method g(): void public final method f(): void + public final method g(): void + public final @org.jetbrains.annotations.NotNull method getA(): java.lang.String public final method h(): void -} \ No newline at end of file + public final method setA(@org.jetbrains.annotations.NotNull p0: java.lang.String): void +} diff --git a/plugins/jvm-abi-gen/testData/content/whenMapping/signatures.txt b/plugins/jvm-abi-gen/testData/content/whenMapping/signatures.txt index 6fc0e7ed439..a555a4a9ba2 100644 --- a/plugins/jvm-abi-gen/testData/content/whenMapping/signatures.txt +++ b/plugins/jvm-abi-gen/testData/content/whenMapping/signatures.txt @@ -3,9 +3,9 @@ public final enum class test/E { // source: 'test.kt' public final enum static field A: test.E public final enum static field B: test.E - public static method values(): test.E[] - public static method valueOf(p0: java.lang.String): test.E public static @org.jetbrains.annotations.NotNull method getEntries(): kotlin.enums.EnumEntries + public static method valueOf(p0: java.lang.String): test.E + public static method values(): test.E[] } @kotlin.Metadata public synthetic final class test/Test$WhenMappings {