From 7f5bc9411714e0b04de62155fd2f0b638839f337 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Thu, 10 Aug 2023 00:38:58 +0200 Subject: [PATCH] Jvm-abi-gen: do not treat KmProperty.hasConstant as part of ABI ... for non-const properties. The meaning of this flag for non-const properties is in reality not well-established, and it can vary from one release to another. In particular, it's different in K2. This commit has two parts: 1) The test (added in 5891617674) about clinit is changed. Original motivation was to check that presence or absence of `` is not a part of the ABI. However, in addition to that, the test also involuntarily checked that the aforementioned "hasConstant" flag for the property y/Object.y is the same in both cases (which I believe was not the intention) because these tests check that Kotlin metadata is _exactly the same_ after applying jvm-abi-gen. So I've changed it to be more straightforward: one version declares an obvious constant, while another declares an obvious non-constant. After this change, the test started to fail (even on K1) because the value of the "hasConstant" flag was different between two versions. 2) Remove "hasConstant" flag for non-const properties when transforming metadata via jvm-abi-gen. This fixes the test and assures it won't fail when the project is migrated to LV 2.0. #KT-60849 Fixed --- .../kotlin/jvm/abi/JvmAbiMetadataProcessor.kt | 23 ++++++++++++------- .../testData/compare/clinit/base/Object.kt | 3 +-- .../compare/clinit/base/topLevelProperties.kt | 1 - .../testData/compare/clinit/sameAbi/Object.kt | 7 +++--- .../clinit/sameAbi/topLevelProperties.kt | 5 ++-- 5 files changed, 23 insertions(+), 16 deletions(-) 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 761403a0507..abedb0862f6 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 @@ -5,13 +5,10 @@ package org.jetbrains.kotlin.jvm.abi -import kotlinx.metadata.KmClass -import kotlinx.metadata.KmPackage -import kotlinx.metadata.Visibility +import kotlinx.metadata.* import kotlinx.metadata.jvm.KotlinClassMetadata import kotlinx.metadata.jvm.Metadata import kotlinx.metadata.jvm.localDelegatedProperties -import kotlinx.metadata.visibility import org.jetbrains.kotlin.load.java.JvmAnnotationNames.* import org.jetbrains.org.objectweb.asm.AnnotationVisitor import org.jetbrains.org.objectweb.asm.Opcodes @@ -162,18 +159,28 @@ private fun AnnotationVisitor.visitKotlinMetadata(header: Metadata) { private fun KmClass.removePrivateDeclarations() { constructors.removeIf { it.visibility.isPrivate } - functions.removeIf { it.visibility.isPrivate } - properties.removeIf { it.visibility.isPrivate } + (this as KmDeclarationContainer).removePrivateDeclarations() localDelegatedProperties.clear() // TODO: do not serialize private type aliases once KT-17229 is fixed. } private fun KmPackage.removePrivateDeclarations() { - functions.removeIf { it.visibility.isPrivate } - properties.removeIf { it.visibility.isPrivate } + (this as KmDeclarationContainer).removePrivateDeclarations() localDelegatedProperties.clear() // TODO: do not serialize private type aliases once KT-17229 is fixed. } +private fun KmDeclarationContainer.removePrivateDeclarations() { + functions.removeIf { it.visibility.isPrivate } + properties.removeIf { it.visibility.isPrivate } + + 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) { + property.hasConstant = false + } + } +} + private val Visibility.isPrivate: Boolean get() = this == Visibility.PRIVATE || this == Visibility.PRIVATE_TO_THIS || this == Visibility.LOCAL diff --git a/plugins/jvm-abi-gen/testData/compare/clinit/base/Object.kt b/plugins/jvm-abi-gen/testData/compare/clinit/base/Object.kt index 0430253cc4d..f99730dc743 100644 --- a/plugins/jvm-abi-gen/testData/compare/clinit/base/Object.kt +++ b/plugins/jvm-abi-gen/testData/compare/clinit/base/Object.kt @@ -2,5 +2,4 @@ package test object Object { val x = 0 - val y = x -} \ No newline at end of file +} diff --git a/plugins/jvm-abi-gen/testData/compare/clinit/base/topLevelProperties.kt b/plugins/jvm-abi-gen/testData/compare/clinit/base/topLevelProperties.kt index 7909a50197d..ded441ca956 100644 --- a/plugins/jvm-abi-gen/testData/compare/clinit/base/topLevelProperties.kt +++ b/plugins/jvm-abi-gen/testData/compare/clinit/base/topLevelProperties.kt @@ -1,4 +1,3 @@ package test val x = 0 -val y = x \ No newline at end of file diff --git a/plugins/jvm-abi-gen/testData/compare/clinit/sameAbi/Object.kt b/plugins/jvm-abi-gen/testData/compare/clinit/sameAbi/Object.kt index 9e35417b699..da8f8f42e56 100644 --- a/plugins/jvm-abi-gen/testData/compare/clinit/sameAbi/Object.kt +++ b/plugins/jvm-abi-gen/testData/compare/clinit/sameAbi/Object.kt @@ -1,6 +1,7 @@ package test object Object { - val x = 0 - val y = 0 -} \ No newline at end of file + val x = foo() + + private fun foo(): Int = 0 +} diff --git a/plugins/jvm-abi-gen/testData/compare/clinit/sameAbi/topLevelProperties.kt b/plugins/jvm-abi-gen/testData/compare/clinit/sameAbi/topLevelProperties.kt index cf62224ec21..4b14a658a0c 100644 --- a/plugins/jvm-abi-gen/testData/compare/clinit/sameAbi/topLevelProperties.kt +++ b/plugins/jvm-abi-gen/testData/compare/clinit/sameAbi/topLevelProperties.kt @@ -1,4 +1,5 @@ package test -val x = 0 -val y = 0 \ No newline at end of file +val x = foo() + +private fun foo(): Int = 0