JVM_IR special handling for 'remove' collection method stub

compiler/fir/fir2ir/tests/org/jetbrains/kotlin/codegen/ir/FirBlackBoxCodegenTestGenerated.java

@@ -5008,6 +5008,16 @@ public class FirBlackBoxCodegenTestGenerated extends AbstractFirBlackBoxCodegenT
             runTest("compiler/testData/codegen/box/collections/removeClash.kt");
         }
 
+        @TestMetadata("removeOverriddenInJava.kt")
+        public void testRemoveOverriddenInJava() throws Exception {
+            runTest("compiler/testData/codegen/box/collections/removeOverriddenInJava.kt");
+        }
+
+        @TestMetadata("removeOverriddenInJava_Map.kt")
+        public void testRemoveOverriddenInJava_Map() throws Exception {
+            runTest("compiler/testData/codegen/box/collections/removeOverriddenInJava_Map.kt");
+        }
+
         @TestMetadata("strList.kt")
         public void testStrList() throws Exception {
             runTest("compiler/testData/codegen/box/collections/strList.kt");

compiler/ir/backend.jvm/src/org/jetbrains/kotlin/backend/jvm/lower/CollectionStubMethodLowering.kt

@@ -7,7 +7,6 @@ package org.jetbrains.kotlin.backend.jvm.lower
 
 import org.jetbrains.kotlin.backend.common.ClassLoweringPass
 import org.jetbrains.kotlin.backend.common.lower.createIrBuilder
-import org.jetbrains.kotlin.backend.common.lower.irThrow
 import org.jetbrains.kotlin.backend.common.phaser.makeIrFilePhase
 import org.jetbrains.kotlin.backend.jvm.JvmBackendContext
 import org.jetbrains.kotlin.backend.jvm.codegen.isJvmInterface
@@ -50,8 +49,8 @@ internal class CollectionStubMethodLowering(val context: JvmBackendContext) : Cl
             it.modality == Modality.ABSTRACT && it.isFakeOverride
         }.associateBy { it.toSignature() }
 
-        for (member in methodStubsToGenerate) {
+        for ((signature, member) in methodStubsToGenerate) {
-            val existingMethod = existingMethodsBySignature[member.toSignature()]
+            val existingMethod = existingMethodsBySignature[signature] // TODO KT-41915
             if (existingMethod != null) {
                 // In the case that we find a defined method that matches the stub signature, we add the overridden symbols to that
                 // defined method, so that bridge lowering can still generate correct bridge for that method
@@ -66,8 +65,8 @@ internal class CollectionStubMethodLowering(val context: JvmBackendContext) : Cl
 
     private fun createStubMethod(
         function: IrSimpleFunction, irClass: IrClass, substitutionMap: Map<IrTypeParameterSymbol, IrType>
-    ): IrSimpleFunction {
+    ): Pair<String, IrSimpleFunction> {
-        return context.irFactory.buildFun {
+        val newMethod = context.irFactory.buildFun {
             name = function.name
             returnType = function.returnType.substitute(substitutionMap)
             visibility = function.visibility
@@ -90,19 +89,41 @@ internal class CollectionStubMethodLowering(val context: JvmBackendContext) : Cl
                 }
             }
         }
+        val signatureToCheckPresence = newMethod.toSignature()
+        // NB 'remove' method requires some special handling (and that's why we need to track both signature and function):
+        // - we check that 'remove(T)' is present in the class, where 'T' is a type argument of the collection type
+        //   (this matches the signature of the method created above);
+        // - if it is absent, we add 'remove(Any?)' member function,
+        //   which corresponds to method 'remove' in java.util.Collection and java.util.Map.
+        if (newMethod.name.asString() == "remove") {
+            // Note that replacing value parameter types with 'Any?' handles both MutableCollection and MutableMap case:
+            // java.util.Collection<E>#remove has signature 'boolean remove(Object)', and
+            // java.util.Map<K, V>#remove has signature 'V remove(Object)'.
+            newMethod.valueParameters = function.valueParameters.map {
+                it.copyWithCustomTypeSubstitution(newMethod) { context.irBuiltIns.anyNType }
+            }
+        }
+        return signatureToCheckPresence to newMethod
     }
 
     // Copy value parameter with type substitution
     private fun IrValueParameter.copyWithSubstitution(
-        target: IrSimpleFunction, substitutionMap: Map<IrTypeParameterSymbol, IrType>
+        target: IrSimpleFunction,
+        substitutionMap: Map<IrTypeParameterSymbol, IrType>
+    ): IrValueParameter =
+        copyWithCustomTypeSubstitution(target) { it.substitute(substitutionMap) }
+
+    private fun IrValueParameter.copyWithCustomTypeSubstitution(
+        target: IrSimpleFunction,
+        substituteType: (IrType) -> IrType
     ): IrValueParameter {
         val parameter = this
         return buildValueParameter(target) {
             origin = IrDeclarationOrigin.IR_BUILTINS_STUB
             name = parameter.name
             index = parameter.index
-            type = parameter.type.substitute(substitutionMap)
+            type = substituteType(parameter.type)
-            varargElementType = parameter.varargElementType?.substitute(substitutionMap)
+            varargElementType = parameter.varargElementType?.let { substituteType(it) }
             isCrossInline = parameter.isCrossinline
             isNoinline = parameter.isNoinline
         }
@@ -129,7 +150,7 @@ internal class CollectionStubMethodLowering(val context: JvmBackendContext) : Cl
     }
 
     // Compute stubs that should be generated, compare based on signature
-    private fun generateRelevantStubMethods(irClass: IrClass): Set<IrSimpleFunction> {
+    private fun generateRelevantStubMethods(irClass: IrClass): Map<String, IrSimpleFunction> {
         val ourStubsForCollectionClasses = collectionStubComputer.stubsForCollectionClasses(irClass)
         val superStubClasses = irClass.superClass?.superClassChain?.map { superClass ->
             collectionStubComputer.stubsForCollectionClasses(superClass).map { it.readOnlyClass }
@@ -148,7 +169,7 @@ internal class CollectionStubMethodLowering(val context: JvmBackendContext) : Cl
             mutableOnlyMethods.map { function ->
                 createStubMethod(function, irClass, substitutionMap)
             }
-        }.toHashSet()
+        }.toMap()
     }
 
     private fun Collection<IrType>.findMostSpecificTypeForClass(classifier: IrClassSymbol): IrType {

compiler/testData/codegen/box/collections/removeOverriddenInJava.kt

@@ -0,0 +1,26 @@
+// TARGET_BACKEND: JVM
+// FILE: removeOverriddenInJava.kt
+
+open class A : Collection<String> {
+    override val size: Int get() = TODO()
+    override fun contains(element: String): Boolean = TODO()
+    override fun containsAll(elements: Collection<String>): Boolean = TODO()
+    override fun isEmpty(): Boolean = TODO()
+    override fun iterator(): Iterator<String> = TODO()
+}
+
+fun box(): String {
+    B().remove("OK")
+    return B.removed as String
+}
+
+// FILE: B.java
+public class B extends A {
+    public static Object removed = null;
+
+    @Override
+    public boolean remove(Object o) {
+        removed = o;
+        return false;
+    }
+}

compiler/testData/codegen/box/collections/removeOverriddenInJava_Map.kt

@@ -0,0 +1,23 @@
+// TARGET_BACKEND: JVM
+// FILE: removeOverriddenInJava_Map.kt
+
+open class MapA : Map<String, String> {
+    override val entries: Set<Map.Entry<String, String>> get() = null!!
+    override val keys: Set<String> get() = null!!
+    override val size: Int get() = null!!
+    override val values: Collection<String> get() = null!!
+    override fun containsKey(key: String): Boolean = null!!
+    override fun containsValue(value: String): Boolean = null!!
+    override fun get(key: String): String? = null!!
+    override fun isEmpty(): Boolean = null!!
+}
+
+fun box() = MapB().remove("OK")
+
+// FILE: MapB.java
+public class MapB extends MapA {
+    @Override
+    public String remove(Object key) {
+        return (String) key;
+    }
+}

compiler/testData/codegen/bytecodeListing/specialBridges/redundantStubForSize.kt

@@ -6,18 +6,18 @@ class A2 : A1(), Collection<String> {
     // No 'getSize()' method should be generated in A2
 
     override fun contains(element: String): Boolean {
-        throw UnsupportedOperationException("not implemented") //To change body of created functions use File | Settings | File Templates.
+        throw UnsupportedOperationException("not implemented")
     }
 
     override fun containsAll(elements: Collection<String>): Boolean {
-        throw UnsupportedOperationException("not implemented") //To change body of created functions use File | Settings | File Templates.
+        throw UnsupportedOperationException("not implemented")
     }
 
     override fun isEmpty(): Boolean {
-        throw UnsupportedOperationException("not implemented") //To change body of created functions use File | Settings | File Templates.
+        throw UnsupportedOperationException("not implemented")
     }
 
     override fun iterator(): Iterator<String> {
-        throw UnsupportedOperationException("not implemented") //To change body of created functions use File | Settings | File Templates.
+        throw UnsupportedOperationException("not implemented")
     }
 }

compiler/testData/codegen/bytecodeListing/specialBridges/redundantStubForSize_ir.txt

@@ -19,8 +19,7 @@ public final class A2 {
     public method containsAll(@org.jetbrains.annotations.NotNull p0: java.util.Collection): boolean
     public method isEmpty(): boolean
     public @org.jetbrains.annotations.NotNull method iterator(): java.util.Iterator
-    public bridge final method remove(p0: java.lang.Object): boolean
+    public method remove(p0: java.lang.Object): boolean
-    public method remove(p0: java.lang.String): boolean
     public method removeAll(p0: java.util.Collection): boolean
     public method retainAll(p0: java.util.Collection): boolean
     public bridge final method size(): int

compiler/tests/org/jetbrains/kotlin/codegen/BlackBoxCodegenTestGenerated.java

@@ -5038,6 +5038,16 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
             runTest("compiler/testData/codegen/box/collections/removeClash.kt");
         }
 
+        @TestMetadata("removeOverriddenInJava.kt")
+        public void testRemoveOverriddenInJava() throws Exception {
+            runTest("compiler/testData/codegen/box/collections/removeOverriddenInJava.kt");
+        }
+
+        @TestMetadata("removeOverriddenInJava_Map.kt")
+        public void testRemoveOverriddenInJava_Map() throws Exception {
+            runTest("compiler/testData/codegen/box/collections/removeOverriddenInJava_Map.kt");
+        }
+
         @TestMetadata("strList.kt")
         public void testStrList() throws Exception {
             runTest("compiler/testData/codegen/box/collections/strList.kt");

compiler/tests/org/jetbrains/kotlin/codegen/LightAnalysisModeTestGenerated.java

@@ -5038,6 +5038,16 @@ public class LightAnalysisModeTestGenerated extends AbstractLightAnalysisModeTes
             runTest("compiler/testData/codegen/box/collections/removeClash.kt");
         }
 
+        @TestMetadata("removeOverriddenInJava.kt")
+        public void testRemoveOverriddenInJava() throws Exception {
+            runTest("compiler/testData/codegen/box/collections/removeOverriddenInJava.kt");
+        }
+
+        @TestMetadata("removeOverriddenInJava_Map.kt")
+        public void testRemoveOverriddenInJava_Map() throws Exception {
+            runTest("compiler/testData/codegen/box/collections/removeOverriddenInJava_Map.kt");
+        }
+
         @TestMetadata("strList.kt")
         public void testStrList() throws Exception {
             runTest("compiler/testData/codegen/box/collections/strList.kt");

compiler/tests/org/jetbrains/kotlin/codegen/ir/IrBlackBoxCodegenTestGenerated.java

@@ -5008,6 +5008,16 @@ public class IrBlackBoxCodegenTestGenerated extends AbstractIrBlackBoxCodegenTes
             runTest("compiler/testData/codegen/box/collections/removeClash.kt");
         }
 
+        @TestMetadata("removeOverriddenInJava.kt")
+        public void testRemoveOverriddenInJava() throws Exception {
+            runTest("compiler/testData/codegen/box/collections/removeOverriddenInJava.kt");
+        }
+
+        @TestMetadata("removeOverriddenInJava_Map.kt")
+        public void testRemoveOverriddenInJava_Map() throws Exception {
+            runTest("compiler/testData/codegen/box/collections/removeOverriddenInJava_Map.kt");
+        }
+
         @TestMetadata("strList.kt")
         public void testStrList() throws Exception {
             runTest("compiler/testData/codegen/box/collections/strList.kt");