JVM IR: fix inheritance from mutable collections of Int

For subclasses of `AbstractMutableList<Int>` which are not inline
classes, the special bridge `remove` had a parameter of type `Int`
(mapped to JVM primitive int) before this fix. The hack in
`MethodSignatureMapper` changed this type to `Int?`, yet the body of the
special bridge still loaded it as non-nullable, which resulted in
incorrect bytecode.

It looks like a part of this hack in `BridgeLowering` was made only for
inline classes which are subclasses of mutable collections. Supposedly
it should be extended to non-inline classes, so that `remove` special
bridge would have consistent IR by the time it reaches codegen.

 #KT-46516 Fixed
This commit is contained in:
Alexander Udalov
2021-05-06 20:41:22 +02:00
parent f30980806f
commit e079fb665e
13 changed files with 113 additions and 10 deletions
@@ -6004,6 +6004,12 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT
runTest("compiler/testData/codegen/box/collections/inSetWithSmartCast.kt");
}
@Test
@TestMetadata("inheritFromAbstractMutableListInt.kt")
public void testInheritFromAbstractMutableListInt() throws Exception {
runTest("compiler/testData/codegen/box/collections/inheritFromAbstractMutableListInt.kt");
}
@Test
@TestMetadata("inheritFromHashtable.kt")
public void testInheritFromHashtable() throws Exception {
@@ -205,22 +205,25 @@ internal class BridgeLowering(val context: JvmBackendContext) : FileLoweringPass
// can contain a static replacement for a function 'remove', which forces value parameter boxing
// in order to avoid signature clash with 'remove(int)' method in 'java.util.List'.
// We should rewrite this static replacement as well ('remove' function itself is handled during special bridge processing).
for (irFunction in declaration.functions) {
val originalFunction = context.inlineClassReplacements.originalFunctionForStaticReplacement[irFunction]
?: continue
if (context.methodSignatureMapper.shouldBoxSingleValueParameterForSpecialCaseOfRemove(originalFunction)) {
val oldValueParameter1 = irFunction.valueParameters[1]
val newValueParameter1 = oldValueParameter1.copyTo(irFunction, type = oldValueParameter1.type.makeNullable())
irFunction.valueParameters = listOf(irFunction.valueParameters[0], newValueParameter1)
irFunction.body?.transform(VariableRemapper(mapOf(oldValueParameter1 to newValueParameter1)), null)
break
}
val remove = declaration.functions.find {
val original = context.inlineClassReplacements.originalFunctionForStaticReplacement[it]
original != null && context.methodSignatureMapper.shouldBoxSingleValueParameterForSpecialCaseOfRemove(original)
}
if (remove != null) {
makeLastParameterNullable(remove)
}
}
return super.visitClass(declaration)
}
private fun makeLastParameterNullable(irFunction: IrSimpleFunction) {
val oldValueParameter = irFunction.valueParameters.last()
val newValueParameter = oldValueParameter.copyTo(irFunction, type = oldValueParameter.type.makeNullable())
irFunction.valueParameters = irFunction.valueParameters.dropLast(1) + newValueParameter
irFunction.body?.transform(VariableRemapper(mapOf(oldValueParameter to newValueParameter)), null)
}
private fun generateBridges() {
for (member in potentialBridgeTargets) {
val parent = member.parentAsClass
@@ -479,6 +482,10 @@ internal class BridgeLowering(val context: JvmBackendContext) : FileLoweringPass
if (specialBridge.isOverriding) {
overriddenSymbols = listOf(specialBridge.overridden.symbol)
}
if (context.methodSignatureMapper.shouldBoxSingleValueParameterForSpecialCaseOfRemove(this)) {
makeLastParameterNullable(this)
}
}
private fun IrSimpleFunction.rewriteSpecialMethodBody(
@@ -0,0 +1,18 @@
// WITH_RUNTIME
// IGNORE_BACKEND: WASM
abstract class A : AbstractMutableList<Int>()
class B : A() {
override fun iterator(): MutableIterator<Int> = mutableListOf(0).iterator()
override val size = 0
override fun add(index: Int, element: Int) {}
override fun get(index: Int) = index
override fun removeAt(index: Int) = index
override fun set(index: Int, element: Int) = index
}
fun box(): String {
val b = B()
return if (b.remove(0)) "OK" else "Fail"
}
@@ -1,3 +1,5 @@
// WITH_RUNTIME
abstract class AMListD : AbstractMutableList<Double>()
abstract class AMListI : AbstractMutableList<Int>()
@@ -13,3 +13,19 @@ public abstract class AMListD {
public bridge final method remove(p0: java.lang.Object): boolean
public synthetic bridge method removeAt(p0: int): java.lang.Object
}
@kotlin.Metadata
public abstract class AMListI {
// source: 'inheritingFromAbstractMutableList.kt'
public method <init>(): void
public bridge method contains(p0: int): boolean
public bridge final method contains(p0: java.lang.Object): boolean
public bridge method indexOf(p0: int): int
public bridge final method indexOf(p0: java.lang.Object): int
public bridge method lastIndexOf(p0: int): int
public bridge final method lastIndexOf(p0: java.lang.Object): int
public bridge final method remove(p0: int): int
public bridge method remove(p0: java.lang.Integer): boolean
public bridge final method remove(p0: java.lang.Object): boolean
public synthetic bridge method removeAt(p0: int): java.lang.Object
}
@@ -14,3 +14,20 @@ public abstract class AMListD {
public abstract method removeAt(p0: int): java.lang.Double
public synthetic bridge method removeAt(p0: int): java.lang.Object
}
@kotlin.Metadata
public abstract class AMListI {
// source: 'inheritingFromAbstractMutableList.kt'
public method <init>(): void
public bridge method contains(p0: int): boolean
public bridge final method contains(p0: java.lang.Object): boolean
public bridge method indexOf(p0: int): int
public bridge final method indexOf(p0: java.lang.Object): int
public bridge method lastIndexOf(p0: int): int
public bridge final method lastIndexOf(p0: java.lang.Object): int
public bridge final method remove(p0: int): int
public bridge method remove(p0: java.lang.Integer): boolean
public bridge final method remove(p0: java.lang.Object): boolean
public abstract method removeAt(p0: int): java.lang.Integer
public synthetic bridge method removeAt(p0: int): java.lang.Object
}
@@ -6004,6 +6004,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
runTest("compiler/testData/codegen/box/collections/inSetWithSmartCast.kt");
}
@Test
@TestMetadata("inheritFromAbstractMutableListInt.kt")
public void testInheritFromAbstractMutableListInt() throws Exception {
runTest("compiler/testData/codegen/box/collections/inheritFromAbstractMutableListInt.kt");
}
@Test
@TestMetadata("inheritFromHashtable.kt")
public void testInheritFromHashtable() throws Exception {
@@ -6004,6 +6004,12 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
runTest("compiler/testData/codegen/box/collections/inSetWithSmartCast.kt");
}
@Test
@TestMetadata("inheritFromAbstractMutableListInt.kt")
public void testInheritFromAbstractMutableListInt() throws Exception {
runTest("compiler/testData/codegen/box/collections/inheritFromAbstractMutableListInt.kt");
}
@Test
@TestMetadata("inheritFromHashtable.kt")
public void testInheritFromHashtable() throws Exception {
@@ -5246,6 +5246,11 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes
runTest("compiler/testData/codegen/box/collections/inSetWithSmartCast.kt");
}
@TestMetadata("inheritFromAbstractMutableListInt.kt")
public void testInheritFromAbstractMutableListInt() throws Exception {
runTest("compiler/testData/codegen/box/collections/inheritFromAbstractMutableListInt.kt");
}
@TestMetadata("inheritFromHashtable.kt")
public void testInheritFromHashtable() throws Exception {
runTest("compiler/testData/codegen/box/collections/inheritFromHashtable.kt");
@@ -4171,6 +4171,11 @@ public class IrJsCodegenBoxES6TestGenerated extends AbstractIrJsCodegenBoxES6Tes
runTest("compiler/testData/codegen/box/collections/inSetWithSmartCast.kt");
}
@TestMetadata("inheritFromAbstractMutableListInt.kt")
public void testInheritFromAbstractMutableListInt() throws Exception {
runTest("compiler/testData/codegen/box/collections/inheritFromAbstractMutableListInt.kt");
}
@TestMetadata("internalRemove.kt")
public void testInternalRemove() throws Exception {
runTest("compiler/testData/codegen/box/collections/internalRemove.kt");
@@ -4171,6 +4171,11 @@ public class IrJsCodegenBoxTestGenerated extends AbstractIrJsCodegenBoxTest {
runTest("compiler/testData/codegen/box/collections/inSetWithSmartCast.kt");
}
@TestMetadata("inheritFromAbstractMutableListInt.kt")
public void testInheritFromAbstractMutableListInt() throws Exception {
runTest("compiler/testData/codegen/box/collections/inheritFromAbstractMutableListInt.kt");
}
@TestMetadata("internalRemove.kt")
public void testInternalRemove() throws Exception {
runTest("compiler/testData/codegen/box/collections/internalRemove.kt");
@@ -4171,6 +4171,11 @@ public class JsCodegenBoxTestGenerated extends AbstractJsCodegenBoxTest {
runTest("compiler/testData/codegen/box/collections/inSetWithSmartCast.kt");
}
@TestMetadata("inheritFromAbstractMutableListInt.kt")
public void testInheritFromAbstractMutableListInt() throws Exception {
runTest("compiler/testData/codegen/box/collections/inheritFromAbstractMutableListInt.kt");
}
@TestMetadata("internalRemove.kt")
public void testInternalRemove() throws Exception {
runTest("compiler/testData/codegen/box/collections/internalRemove.kt");
@@ -2992,6 +2992,11 @@ public class IrCodegenBoxWasmTestGenerated extends AbstractIrCodegenBoxWasmTest
KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/codegen/box/collections"), Pattern.compile("^([^_](.+))\\.kt$"), null, TargetBackend.WASM, true);
}
@TestMetadata("inheritFromAbstractMutableListInt.kt")
public void testInheritFromAbstractMutableListInt() throws Exception {
runTest("compiler/testData/codegen/box/collections/inheritFromAbstractMutableListInt.kt");
}
@TestMetadata("internalRemove.kt")
public void testInternalRemove() throws Exception {
runTest("compiler/testData/codegen/box/collections/internalRemove.kt");