IC: Fix regression in detecting constants in KotlinClassInfo.kt

A constant is a static final field with non-null value. In a previous
commit (0b09be7), we accidentally removed the *non-null value*
filter when looking for constants in the bytecode.

This commit re-adds that filter to make sure the detection is correct.

Test: Added KotlinOnlyClasspathChangesComputerTest.testDelegatedProperties

^KT-58986: Fixed
This commit is contained in:
Hung Nguyen
2023-06-16 15:28:39 +01:00
committed by Space Team
parent ff612f15cf
commit 078356164b
7 changed files with 47 additions and 2 deletions
@@ -180,7 +180,7 @@ private fun getExtraInfo(classHeader: KotlinClassHeader, classContents: ByteArra
val constantSnapshots: Map<String, Long> = classNode.fields.associate { fieldNode ->
// Note: `fieldNode` is a constant because we kept only fields that are (non-private) constants in `classNode`
fieldNode.name to (fieldNode.value?.let { ConstantValueExternalizer.toByteArray(it).hashToLong() } ?: 0L)
fieldNode.name to ConstantValueExternalizer.toByteArray(fieldNode.value!!).hashToLong()
}
val inlineFunctionOrAccessorSnapshots: Map<InlineFunctionOrAccessor, Long> = classNode.methods.associate { methodNode ->
@@ -210,7 +210,11 @@ class SelectiveClassVisitor(
) : ClassVisitor(Opcodes.API_VERSION, cv) {
override fun visitField(access: Int, name: String, desc: String, signature: String?, value: Any?): FieldVisitor? {
return if (shouldVisitField(JvmMemberSignature.Field(name, desc), access.isPrivate(), access.isStaticFinal())) {
// Note: A constant's value must be not-null. A static final field with a `null` value at the bytecode declaration is not a constant
// (whether the value is initialized later in the static initializer or not, it won't be inlined by the compiler).
val isConstant = access.isStaticFinal() && value != null
return if (shouldVisitField(JvmMemberSignature.Field(name, desc), access.isPrivate(), isConstant)) {
cv.visitField(access, name, desc, signature, value)
} else null
}
@@ -290,6 +290,19 @@ class KotlinOnlyClasspathChangesComputerTest : ClasspathChangesComputerTest() {
).assertEquals(changes)
}
/** Regression test for KT-58986.*/
@Test
fun testDelegatedProperties() {
// Check that classpath changes computation doesn't fail
val changes = computeClasspathChanges(File(testDataDir, "KotlinOnly/testDelegatedProperties/src"), tmpDir)
Changes(
lookupSymbols = setOf(
LookupSymbol(name = "delegatedProperty", scope = "com.example"),
),
fqNames = setOf("com.example")
).assertEquals(changes)
}
/** Tests [SupertypesInheritorsImpact]. */
@Test
override fun testImpactComputation_SupertypesInheritors() {
@@ -0,0 +1,15 @@
package com.example
import kotlin.reflect.KProperty
var delegatedProperty: String by Delegate() // Added
class Delegate {
operator fun getValue(thisRef: Any?, property: KProperty<*>): String {
return "<Delegated>"
}
operator fun setValue(thisRef: Any?, property: KProperty<*>, value: String) {
}
}
@@ -0,0 +1,13 @@
package com.example
import kotlin.reflect.KProperty
class Delegate {
operator fun getValue(thisRef: Any?, property: KProperty<*>): String {
return "<Delegated>"
}
operator fun setValue(thisRef: Any?, property: KProperty<*>, value: String) {
}
}