From b8625cd23353f5bc5f60202ae4ae173b7a241bf3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Steven=20Sch=C3=A4fer?= Date: Tue, 30 Nov 2021 20:45:53 +0100 Subject: [PATCH] jvm-abi-gen: Do not sort stripped methods and fields The order is visible to annotation processors and should be considered part of the ABI of a class. --- .../kotlin/jvm/abi/JvmAbiOutputExtension.kt | 177 ++++-------------- .../jvm/abi/CompareJvmAbiTestGenerated.java | 10 - .../jvm/abi/IrCompareJvmAbiTestGenerated.java | 10 - .../abi/LegacyCompareJvmAbiTestGenerated.java | 10 - .../compare/declarationOrder/base/test.kt | 9 - .../compare/declarationOrder/directives.txt | 1 - .../compare/declarationOrder/sameAbi/test.kt | 9 - .../base/test.kt | 6 - .../directives.txt | 1 - .../sameAbi/test.kt | 6 - 10 files changed, 33 insertions(+), 206 deletions(-) delete mode 100644 plugins/jvm-abi-gen/testData/compare/declarationOrder/base/test.kt delete mode 100644 plugins/jvm-abi-gen/testData/compare/declarationOrder/directives.txt delete mode 100644 plugins/jvm-abi-gen/testData/compare/declarationOrder/sameAbi/test.kt delete mode 100644 plugins/jvm-abi-gen/testData/compare/declarationOrderInlineNonInline/base/test.kt delete mode 100644 plugins/jvm-abi-gen/testData/compare/declarationOrderInlineNonInline/directives.txt delete mode 100644 plugins/jvm-abi-gen/testData/compare/declarationOrderInlineNonInline/sameAbi/test.kt 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 1b473ca40b3..781173ed4e7 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 @@ -16,9 +16,6 @@ import org.jetbrains.kotlin.codegen.extensions.ClassFileFactoryFinalizerExtensio import org.jetbrains.kotlin.load.java.JvmAnnotationNames import org.jetbrains.org.objectweb.asm.* import org.jetbrains.org.objectweb.asm.commons.Method -import org.jetbrains.org.objectweb.asm.tree.AnnotationNode -import org.jetbrains.org.objectweb.asm.tree.FieldNode -import org.jetbrains.org.objectweb.asm.tree.ParameterNode import java.io.File class JvmAbiOutputExtension( @@ -46,7 +43,8 @@ class JvmAbiOutputExtension( } } - private class AbiOutputFiles(val abiClassInfos: Map, val outputFiles: OutputFileCollection) : OutputFileCollection { + private class AbiOutputFiles(val abiClassInfos: Map, val outputFiles: OutputFileCollection) : + OutputFileCollection { override fun get(relativePath: String): OutputFile? { error("AbiOutputFiles does not implement `get`.") } @@ -72,29 +70,46 @@ class JvmAbiOutputExtension( val methodInfo = (abiInfo as AbiClassInfo.Stripped).methodInfo val writer = ClassWriter(0) ClassReader(outputFile.asByteArray()).accept(object : ClassVisitor(Opcodes.API_VERSION, writer) { - // We remember all non-inline declarations in the stripped class so that we can - // order them deterministically after all inline methods. - private val fieldNodes = mutableListOf() - private val methodNodes = mutableListOf() - // Strip private fields. - override fun visitField(access: Int, name: String?, descriptor: String?, signature: String?, value: Any?): FieldVisitor? { + override fun visitField( + access: Int, + name: String?, + descriptor: String?, + signature: String?, + value: Any? + ): FieldVisitor? { if (access and Opcodes.ACC_PRIVATE != 0) return null - return FieldNode(Opcodes.API_VERSION, access, name, descriptor, signature, value).also { - fieldNodes += it - } + return super.visitField(access, name, descriptor, signature, value) } - override fun visitMethod(access: Int, name: String, descriptor: String, signature: String?, exceptions: Array?): MethodVisitor? { + override fun visitMethod( + access: Int, + name: String, + descriptor: String, + signature: String?, + exceptions: Array? + ): MethodVisitor? { val info = methodInfo[Method(name, descriptor)] ?: return null - if (info == AbiMethodInfo.KEEP) - return super.visitMethod(access, name, descriptor, signature, exceptions) + val visitor = super.visitMethod(access, name, descriptor, signature, exceptions) - return StrippedMethodNode(access, name, descriptor, signature, exceptions).also { - methodNodes += it + if (info == AbiMethodInfo.KEEP || access and (Opcodes.ACC_NATIVE or Opcodes.ACC_ABSTRACT) != 0) + return visitor + + return object : MethodVisitor(Opcodes.API_VERSION, visitor) { + override fun visitCode() { + with(mv) { + visitCode() + visitInsn(Opcodes.ACONST_NULL) + visitInsn(Opcodes.ATHROW) + visitMaxs(0, 0) + visitEnd() + } + // Only instructions and locals follow after `visitCode`. + mv = null + } } } @@ -121,18 +136,6 @@ class JvmAbiOutputExtension( return delegate return abiMetadataProcessor(delegate) } - - override fun visitEnd() { - fieldNodes.sortBy { "${it.name}${it.desc}" } - methodNodes.sortBy { "${it.name}${it.desc}" } - for (field in fieldNodes) { - field.accept(writer) - } - for (method in methodNodes) { - method.accept(writer) - } - super.visitEnd() - } }, 0) SimpleOutputBinaryFile(outputFile.sourceFiles, outputFile.relativePath, writer.toByteArray()) } @@ -143,117 +146,3 @@ class JvmAbiOutputExtension( } } } - -private class StrippedMethodNode( - val access: Int, - val name: String, - val desc: String, - val signature: String?, - val exceptions: Array?, -) : MethodVisitor(Opcodes.API_VERSION) { - private class MethodAnnotation( - val visible: Boolean, - val node: AnnotationNode, - ) - - private class MethodTypeAnnotation( - val typeRef: Int, - val typePath: TypePath?, - val visible: Boolean, - val node: AnnotationNode, - ) - - private class MethodParameterAnnotation( - val parameter: Int, - val visible: Boolean, - val node: AnnotationNode, - ) - - private val parameters = mutableListOf() - private val annotations = mutableListOf() - private val typeAnnotations = mutableListOf() - private var visibleAnnotableParameterCount = 0 - private var invisibleAnnotableParameterCount = 0 - private val parameterAnnotations = mutableListOf() - private val attributes = mutableListOf() - - override fun visitParameter(name: String?, access: Int) { - parameters.add(ParameterNode(name, access)) - } - - override fun visitAnnotation(descriptor: String, visible: Boolean): AnnotationVisitor = - AnnotationNode(Opcodes.API_VERSION, descriptor).also { - annotations.add(MethodAnnotation(visible, it)) - } - - override fun visitTypeAnnotation(typeRef: Int, typePath: TypePath?, descriptor: String, visible: Boolean): AnnotationVisitor = - AnnotationNode(Opcodes.API_VERSION, descriptor).also { - typeAnnotations.add(MethodTypeAnnotation(typeRef, typePath, visible, it)) - } - - override fun visitAttribute(attribute: Attribute) { - attributes.add(attribute) - } - - override fun visitAnnotableParameterCount(parameterCount: Int, visible: Boolean) { - if (visible) { - visibleAnnotableParameterCount = parameterCount - } else { - invisibleAnnotableParameterCount = parameterCount - } - } - - override fun visitParameterAnnotation(parameter: Int, descriptor: String, visible: Boolean): AnnotationVisitor = - AnnotationNode(Opcodes.API_VERSION, descriptor).also { - parameterAnnotations.add(MethodParameterAnnotation(parameter, visible, it)) - } - - fun accept(classVisitor: ClassVisitor) { - val visitor = classVisitor.visitMethod(access, name, desc, signature, exceptions) - - for (parameter in parameters) { - visitor.visitParameter(parameter.name, parameter.access) - } - - for (annotation in annotations) { - visitor.visitAnnotation(annotation.node.desc, annotation.visible)?.let { - annotation.node.accept(it) - } - } - - for (typeAnnotation in typeAnnotations) { - visitor.visitTypeAnnotation( - typeAnnotation.typeRef, - typeAnnotation.typePath, - typeAnnotation.node.desc, - typeAnnotation.visible, - )?.let { - typeAnnotation.node.accept(it) - } - } - - visitor.visitAnnotableParameterCount(visibleAnnotableParameterCount, true) - visitor.visitAnnotableParameterCount(invisibleAnnotableParameterCount, false) - for (parameterAnnotation in parameterAnnotations) { - visitor.visitParameterAnnotation( - parameterAnnotation.parameter, - parameterAnnotation.node.desc, - parameterAnnotation.visible - )?.let { - parameterAnnotation.node.accept(it) - } - } - - for (attribute in attributes) { - visitor.visitAttribute(attribute) - } - - if (access and (Opcodes.ACC_NATIVE or Opcodes.ACC_ABSTRACT) == 0) { - visitor.visitCode() - visitor.visitInsn(Opcodes.ACONST_NULL) - visitor.visitInsn(Opcodes.ATHROW) - visitor.visitMaxs(0, parameters.size) - } - visitor.visitEnd() - } -} 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 cb2e6440492..73143a82b3a 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 @@ -55,11 +55,6 @@ public class CompareJvmAbiTestGenerated extends AbstractCompareJvmAbiTest { runTest("plugins/jvm-abi-gen/testData/compare/constant/"); } - @TestMetadata("declarationOrder") - public void testDeclarationOrder() throws Exception { - runTest("plugins/jvm-abi-gen/testData/compare/declarationOrder/"); - } - @TestMetadata("declarationOrderInline") public void testDeclarationOrderInline() throws Exception { runTest("plugins/jvm-abi-gen/testData/compare/declarationOrderInline/"); @@ -70,11 +65,6 @@ public class CompareJvmAbiTestGenerated extends AbstractCompareJvmAbiTest { runTest("plugins/jvm-abi-gen/testData/compare/declarationOrderInlineCall/"); } - @TestMetadata("declarationOrderInlineNonInline") - public void testDeclarationOrderInlineNonInline() throws Exception { - runTest("plugins/jvm-abi-gen/testData/compare/declarationOrderInlineNonInline/"); - } - @TestMetadata("declarationOrderPrivateInline") public void testDeclarationOrderPrivateInline() throws Exception { runTest("plugins/jvm-abi-gen/testData/compare/declarationOrderPrivateInline/"); diff --git a/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/IrCompareJvmAbiTestGenerated.java b/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/IrCompareJvmAbiTestGenerated.java index 91bef552f82..5d038f71020 100644 --- a/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/IrCompareJvmAbiTestGenerated.java +++ b/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/IrCompareJvmAbiTestGenerated.java @@ -55,11 +55,6 @@ public class IrCompareJvmAbiTestGenerated extends AbstractIrCompareJvmAbiTest { runTest("plugins/jvm-abi-gen/testData/compare/constant/"); } - @TestMetadata("declarationOrder") - public void testDeclarationOrder() throws Exception { - runTest("plugins/jvm-abi-gen/testData/compare/declarationOrder/"); - } - @TestMetadata("declarationOrderInline") public void testDeclarationOrderInline() throws Exception { runTest("plugins/jvm-abi-gen/testData/compare/declarationOrderInline/"); @@ -70,11 +65,6 @@ public class IrCompareJvmAbiTestGenerated extends AbstractIrCompareJvmAbiTest { runTest("plugins/jvm-abi-gen/testData/compare/declarationOrderInlineCall/"); } - @TestMetadata("declarationOrderInlineNonInline") - public void testDeclarationOrderInlineNonInline() throws Exception { - runTest("plugins/jvm-abi-gen/testData/compare/declarationOrderInlineNonInline/"); - } - @TestMetadata("declarationOrderPrivateInline") public void testDeclarationOrderPrivateInline() throws Exception { runTest("plugins/jvm-abi-gen/testData/compare/declarationOrderPrivateInline/"); diff --git a/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/LegacyCompareJvmAbiTestGenerated.java b/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/LegacyCompareJvmAbiTestGenerated.java index e6e5ff145f3..865bc7d0e73 100644 --- a/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/LegacyCompareJvmAbiTestGenerated.java +++ b/plugins/jvm-abi-gen/test/org/jetbrains/kotlin/jvm/abi/LegacyCompareJvmAbiTestGenerated.java @@ -55,11 +55,6 @@ public class LegacyCompareJvmAbiTestGenerated extends AbstractLegacyCompareJvmAb runTest("plugins/jvm-abi-gen/testData/compare/constant/"); } - @TestMetadata("declarationOrder") - public void testDeclarationOrder() throws Exception { - runTest("plugins/jvm-abi-gen/testData/compare/declarationOrder/"); - } - @TestMetadata("declarationOrderInline") public void testDeclarationOrderInline() throws Exception { runTest("plugins/jvm-abi-gen/testData/compare/declarationOrderInline/"); @@ -70,11 +65,6 @@ public class LegacyCompareJvmAbiTestGenerated extends AbstractLegacyCompareJvmAb runTest("plugins/jvm-abi-gen/testData/compare/declarationOrderInlineCall/"); } - @TestMetadata("declarationOrderInlineNonInline") - public void testDeclarationOrderInlineNonInline() throws Exception { - runTest("plugins/jvm-abi-gen/testData/compare/declarationOrderInlineNonInline/"); - } - @TestMetadata("declarationOrderPrivateInline") public void testDeclarationOrderPrivateInline() throws Exception { runTest("plugins/jvm-abi-gen/testData/compare/declarationOrderPrivateInline/"); diff --git a/plugins/jvm-abi-gen/testData/compare/declarationOrder/base/test.kt b/plugins/jvm-abi-gen/testData/compare/declarationOrder/base/test.kt deleted file mode 100644 index 65c5834e6d9..00000000000 --- a/plugins/jvm-abi-gen/testData/compare/declarationOrder/base/test.kt +++ /dev/null @@ -1,9 +0,0 @@ -package test - -class A { - fun f() = 1 - val x = 2 -} - -val y = 3 -fun g() = 4 \ No newline at end of file diff --git a/plugins/jvm-abi-gen/testData/compare/declarationOrder/directives.txt b/plugins/jvm-abi-gen/testData/compare/declarationOrder/directives.txt deleted file mode 100644 index 3f7eb8b3500..00000000000 --- a/plugins/jvm-abi-gen/testData/compare/declarationOrder/directives.txt +++ /dev/null @@ -1 +0,0 @@ -// IGNORE_BACKEND_LEGACY: JVM \ No newline at end of file diff --git a/plugins/jvm-abi-gen/testData/compare/declarationOrder/sameAbi/test.kt b/plugins/jvm-abi-gen/testData/compare/declarationOrder/sameAbi/test.kt deleted file mode 100644 index 7bf7429c9bc..00000000000 --- a/plugins/jvm-abi-gen/testData/compare/declarationOrder/sameAbi/test.kt +++ /dev/null @@ -1,9 +0,0 @@ -package test - -fun g() = 4 -val y = 3 - -class A { - val x = 2 - fun f() = 1 -} diff --git a/plugins/jvm-abi-gen/testData/compare/declarationOrderInlineNonInline/base/test.kt b/plugins/jvm-abi-gen/testData/compare/declarationOrderInlineNonInline/base/test.kt deleted file mode 100644 index 4f563379187..00000000000 --- a/plugins/jvm-abi-gen/testData/compare/declarationOrderInlineNonInline/base/test.kt +++ /dev/null @@ -1,6 +0,0 @@ -package test - -inline fun f() = 1 - -fun g() = 2 -fun h() = 3 diff --git a/plugins/jvm-abi-gen/testData/compare/declarationOrderInlineNonInline/directives.txt b/plugins/jvm-abi-gen/testData/compare/declarationOrderInlineNonInline/directives.txt deleted file mode 100644 index 3f7eb8b3500..00000000000 --- a/plugins/jvm-abi-gen/testData/compare/declarationOrderInlineNonInline/directives.txt +++ /dev/null @@ -1 +0,0 @@ -// IGNORE_BACKEND_LEGACY: JVM \ No newline at end of file diff --git a/plugins/jvm-abi-gen/testData/compare/declarationOrderInlineNonInline/sameAbi/test.kt b/plugins/jvm-abi-gen/testData/compare/declarationOrderInlineNonInline/sameAbi/test.kt deleted file mode 100644 index ba3025ffcac..00000000000 --- a/plugins/jvm-abi-gen/testData/compare/declarationOrderInlineNonInline/sameAbi/test.kt +++ /dev/null @@ -1,6 +0,0 @@ -package test - -inline fun f() = 1 - -fun h() = 3 -fun g() = 2