Refine special bridges generation

#KT-10958 Fixed

compiler/backend/src/org/jetbrains/kotlin/codegen/builtinSpecialBridges.kt

@@ -32,7 +32,7 @@ import org.jetbrains.kotlin.resolve.BindingContext
 import org.jetbrains.kotlin.resolve.DescriptorUtils
 import org.jetbrains.kotlin.resolve.calls.callUtil.getParentCall
 import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
-import org.jetbrains.kotlin.resolve.descriptorUtil.firstOverridden
+import org.jetbrains.kotlin.resolve.descriptorUtil.overriddenTreeAsSequence
 import org.jetbrains.kotlin.utils.singletonOrEmptyList
 import java.util.*
 
@@ -59,18 +59,24 @@ object BuiltinSpecialBridgesUtil {
         // e.g. `size()I`
         val specialBridgeSignature = signatureByDescriptor(overriddenBuiltin)
 
-        val needGenerateSpecialBridge = needGenerateSpecialBridge(
-                function, reachableDeclarations, signatureByDescriptor, specialBridgeSignature)
+        val specialBridgeExists = function.getSpecialBridgeSignatureIfExists(signatureByDescriptor) != null
+        val specialBridgesSignaturesInSuperClass = function.overriddenTreeAsSequence(useOriginal = true).mapNotNull {
+            if (it === function) return@mapNotNull null
+            it.getSpecialBridgeSignatureIfExists(signatureByDescriptor)
+        }
+        val isTherePossibleClashWithSpecialBridge =
+                specialBridgeSignature in specialBridgesSignaturesInSuperClass
+                    || reachableDeclarations.any { it.modality == Modality.FINAL && signatureByDescriptor(it) == specialBridgeSignature }
 
-        val specialBridge = if (needGenerateSpecialBridge)
+        val specialBridge = if (specialBridgeExists && !isTherePossibleClashWithSpecialBridge)
             BridgeForBuiltinSpecial(specialBridgeSignature, methodItself, isSpecial = true)
         else null
 
         val commonBridges = reachableDeclarations.mapTo(LinkedHashSet<Signature>(), signatureByDescriptor)
-        commonBridges.remove(specialBridgeSignature)
+        commonBridges.removeAll(specialBridgesSignaturesInSuperClass + specialBridge?.from.singletonOrEmptyList())
 
         val superImplementationDescriptor = findSuperImplementationForStubDelegation(function, fake)
-        if (superImplementationDescriptor != null || !fake) {
+        if (superImplementationDescriptor != null || !fake || functionHandle.isAbstract) {
             commonBridges.remove(methodItself)
         }
 
@@ -114,29 +120,6 @@ private fun findSuperImplementationForStubDelegation(function: FunctionDescripto
 private fun findAllReachableDeclarations(functionDescriptor: FunctionDescriptor): MutableSet<FunctionDescriptor> =
         findAllReachableDeclarations(DescriptorBasedFunctionHandle(functionDescriptor)).map { it.descriptor }.toMutableSet()
 
-private fun <Signature : Any> needGenerateSpecialBridge(
-        functionDescriptor: FunctionDescriptor,
-        reachableDeclarations: Collection<FunctionDescriptor>,
-        signatureByDescriptor: (FunctionDescriptor) -> Signature,
-        overriddenBuiltinSignature: Signature
-): Boolean {
-    // We do not generate special bridge unless it has different JVM descriptor
-    // e.g. `containsAll(Collection<E> c)` in ListImpl<E> has the same signature as `containsAll(Collection<?> c)`
-    // or `contains(E e)` has the same signature as `contains(Object e)`.
-    // While `contains(String e)` in StringList : List<String> has different JVM descriptor from `contains(Object e)`
-    // and there should be special bridge in latter case.
-    if (signatureByDescriptor(functionDescriptor) == overriddenBuiltinSignature) return false
-
-    // Is there Kotlin superclass that already has generated special bridge
-    if (functionDescriptor.firstOverridden {
-        overridden ->
-        overridden !== functionDescriptor && overridden.getSpecialBridgeSignatureIfExists(signatureByDescriptor) != null
-    } != null) return false
-
-    return reachableDeclarations.none { it.modality == Modality.FINAL
-                                        && signatureByDescriptor(it) == overriddenBuiltinSignature }
-}
-
 private fun <Signature> CallableMemberDescriptor.getSpecialBridgeSignatureIfExists(
         signatureByDescriptor: (FunctionDescriptor) -> Signature
 ): Signature? {
@@ -148,7 +131,7 @@ private fun <Signature> CallableMemberDescriptor.getSpecialBridgeSignatureIfExis
 
     // Getting original is necessary here, because we want to determine JVM signature of descriptor as it was declared in containing class
     val originalOverridden = original
-    val overriddenSpecial = originalOverridden.getOverriddenBuiltinReflectingJvmDescriptor()?.original ?: return null
+    val overriddenSpecial = originalOverridden.getOverriddenBuiltinReflectingJvmDescriptor() ?: return null
     val specialBridgeSignature = signatureByDescriptor(overriddenSpecial)
 
     // Does special bridge has different signature

compiler/testData/codegen/box/specialBuiltins/removeAtTwoSpecialBridges.kt

@@ -0,0 +1,96 @@
+open class A0<E> : MutableList<E> {
+    override fun add(element: E): Boolean {
+        throw UnsupportedOperationException()
+    }
+
+    override fun add(index: Int, element: E) {
+        throw UnsupportedOperationException()
+    }
+
+    override fun addAll(index: Int, elements: Collection<E>): Boolean {
+        throw UnsupportedOperationException()
+    }
+
+    override fun addAll(elements: Collection<E>): Boolean {
+        throw UnsupportedOperationException()
+    }
+
+    override fun clear() {
+        throw UnsupportedOperationException()
+    }
+
+    override fun listIterator(): MutableListIterator<E> {
+        throw UnsupportedOperationException()
+    }
+
+    override fun listIterator(index: Int): MutableListIterator<E> {
+        throw UnsupportedOperationException()
+    }
+
+    override fun remove(element: E): Boolean {
+        throw UnsupportedOperationException()
+    }
+
+    override fun removeAll(elements: Collection<E>): Boolean {
+        throw UnsupportedOperationException()
+    }
+
+    override fun removeAt(index: Int): E = "K" as E
+
+    override fun retainAll(elements: Collection<E>): Boolean {
+        throw UnsupportedOperationException()
+    }
+
+    override fun set(index: Int, element: E): E {
+        throw UnsupportedOperationException()
+    }
+
+    override fun subList(fromIndex: Int, toIndex: Int): MutableList<E> {
+        throw UnsupportedOperationException()
+    }
+
+    override val size: Int
+        get() = throw UnsupportedOperationException()
+
+    override fun contains(element: E): Boolean {
+        throw UnsupportedOperationException()
+    }
+
+    override fun containsAll(elements: Collection<E>): Boolean {
+        throw UnsupportedOperationException()
+    }
+
+    override fun get(index: Int): E {
+        throw UnsupportedOperationException()
+    }
+
+    override fun indexOf(element: E): Int {
+        throw UnsupportedOperationException()
+    }
+
+    override fun isEmpty(): Boolean {
+        throw UnsupportedOperationException()
+    }
+
+    override fun lastIndexOf(element: E): Int {
+        throw UnsupportedOperationException()
+    }
+
+    override fun iterator(): MutableIterator<E> {
+        throw UnsupportedOperationException()
+    }
+}
+
+class A1() : A0<String>() {
+    override fun removeAt(p0: Int): String = "O"
+}
+
+class A2 : A0<String>()
+
+// Basically this test checks that no redundant special bridges were generated (i.e. no VerifyError happens)
+fun box(): String {
+    val a1 = A1()
+    val a2 = A2()
+
+    return a1.removeAt(123) + a2.removeAt(456)
+}

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

@@ -0,0 +1,90 @@
+open class A0<E> : MutableList<E> {
+    override fun add(element: E): Boolean {
+        throw UnsupportedOperationException()
+    }
+
+    override fun add(index: Int, element: E) {
+        throw UnsupportedOperationException()
+    }
+
+    override fun addAll(index: Int, elements: Collection<E>): Boolean {
+        throw UnsupportedOperationException()
+    }
+
+    override fun addAll(elements: Collection<E>): Boolean {
+        throw UnsupportedOperationException()
+    }
+
+    override fun clear() {
+        throw UnsupportedOperationException()
+    }
+
+    override fun listIterator(): MutableListIterator<E> {
+        throw UnsupportedOperationException()
+    }
+
+    override fun listIterator(index: Int): MutableListIterator<E> {
+        throw UnsupportedOperationException()
+    }
+
+    override fun remove(element: E): Boolean {
+        throw UnsupportedOperationException()
+    }
+
+    override fun removeAll(elements: Collection<E>): Boolean {
+        throw UnsupportedOperationException()
+    }
+
+    override fun removeAt(index: Int): E {
+        throw UnsupportedOperationException()
+    }
+
+    override fun retainAll(elements: Collection<E>): Boolean {
+        throw UnsupportedOperationException()
+    }
+
+    override fun set(index: Int, element: E): E {
+        throw UnsupportedOperationException()
+    }
+
+    override fun subList(fromIndex: Int, toIndex: Int): MutableList<E> {
+        throw UnsupportedOperationException()
+    }
+
+    override val size: Int
+        get() = throw UnsupportedOperationException()
+
+    override fun contains(element: E): Boolean {
+        throw UnsupportedOperationException()
+    }
+
+    override fun containsAll(elements: Collection<E>): Boolean {
+        throw UnsupportedOperationException()
+    }
+
+    override fun get(index: Int): E {
+        throw UnsupportedOperationException()
+    }
+
+    override fun indexOf(element: E): Int {
+        throw UnsupportedOperationException()
+    }
+
+    override fun isEmpty(): Boolean {
+        throw UnsupportedOperationException()
+    }
+
+    override fun lastIndexOf(element: E): Int {
+        throw UnsupportedOperationException()
+    }
+
+    override fun iterator(): MutableIterator<E> {
+        throw UnsupportedOperationException()
+    }
+}
+
+class A1() : A0<String>() {
+    override fun removeAt(p0: Int): String = "abc"
+}
+
+class A2 : A0<String>()

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

@@ -0,0 +1,66 @@
+@kotlin.Metadata
+public class A0 {
+    public method <init>(): void
+    public method add(p0: int, p1: java.lang.Object): void
+    public method add(p0: java.lang.Object): boolean
+    public method addAll(@org.jetbrains.annotations.NotNull p0: java.util.Collection): boolean
+    public method addAll(p0: int, @org.jetbrains.annotations.NotNull p1: java.util.Collection): boolean
+    public method clear(): void
+    public method contains(p0: java.lang.Object): boolean
+    public method containsAll(@org.jetbrains.annotations.NotNull p0: java.util.Collection): boolean
+    public method get(p0: int): java.lang.Object
+    public method getSize(): int
+    public method indexOf(p0: java.lang.Object): int
+    public method isEmpty(): boolean
+    public @org.jetbrains.annotations.NotNull method iterator(): java.util.Iterator
+    public method lastIndexOf(p0: java.lang.Object): int
+    public @org.jetbrains.annotations.NotNull method listIterator(): java.util.ListIterator
+    public @org.jetbrains.annotations.NotNull method listIterator(p0: int): java.util.ListIterator
+    public final method remove(p0: int): java.lang.Object
+    public method remove(p0: java.lang.Object): boolean
+    public method removeAll(@org.jetbrains.annotations.NotNull p0: java.util.Collection): boolean
+    public method removeAt(p0: int): java.lang.Object
+    public method retainAll(@org.jetbrains.annotations.NotNull p0: java.util.Collection): boolean
+    public method set(p0: int, p1: java.lang.Object): java.lang.Object
+    public final method size(): int
+    public @org.jetbrains.annotations.NotNull method subList(p0: int, p1: int): java.util.List
+    public method toArray(): java.lang.Object[]
+    public method toArray(p0: java.lang.Object[]): java.lang.Object[]
+}
+
+@kotlin.Metadata
+public final class A1 {
+    public method <init>(): void
+    public final method contains(p0: java.lang.Object): boolean
+    public method contains(p0: java.lang.String): boolean
+    public method getSize(): int
+    public final method indexOf(p0: java.lang.Object): int
+    public method indexOf(p0: java.lang.String): int
+    public final method lastIndexOf(p0: java.lang.Object): int
+    public method lastIndexOf(p0: java.lang.String): int
+    public final method remove(p0: int): java.lang.String
+    public final method remove(p0: java.lang.Object): boolean
+    public method remove(p0: java.lang.String): boolean
+    public synthetic method removeAt(p0: int): java.lang.Object
+    public @org.jetbrains.annotations.NotNull method removeAt(p0: int): java.lang.String
+    public method toArray(): java.lang.Object[]
+    public method toArray(p0: java.lang.Object[]): java.lang.Object[]
+}
+
+@kotlin.Metadata
+public final class A2 {
+    public method <init>(): void
+    public final method contains(p0: java.lang.Object): boolean
+    public method contains(p0: java.lang.String): boolean
+    public method getSize(): int
+    public final method indexOf(p0: java.lang.Object): int
+    public method indexOf(p0: java.lang.String): int
+    public final method lastIndexOf(p0: java.lang.Object): int
+    public method lastIndexOf(p0: java.lang.String): int
+    public final method remove(p0: int): java.lang.String
+    public final method remove(p0: java.lang.Object): boolean
+    public method remove(p0: java.lang.String): boolean
+    public method removeAt(p0: int): java.lang.String
+    public method toArray(): java.lang.Object[]
+    public method toArray(p0: java.lang.Object[]): java.lang.Object[]
+}

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

@@ -117,5 +117,11 @@ public class BytecodeListingTestGenerated extends AbstractBytecodeListingTest {
             String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeListing/specialBridges/contains.kt");
             doTest(fileName);
         }
+
+        @TestMetadata("removeAtTwoSpecialBridges.kt")
+        public void testRemoveAtTwoSpecialBridges() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/bytecodeListing/specialBridges/removeAtTwoSpecialBridges.kt");
+            doTest(fileName);
+        }
     }
 }

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

@@ -7492,6 +7492,12 @@ public class BlackBoxCodegenTestGenerated extends AbstractBlackBoxCodegenTest {
             doTest(fileName);
         }
 
+        @TestMetadata("removeAtTwoSpecialBridges.kt")
+        public void testRemoveAtTwoSpecialBridges() throws Exception {
+            String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/specialBuiltins/removeAtTwoSpecialBridges.kt");
+            doTest(fileName);
+        }
+
         @TestMetadata("throwable.kt")
         public void testThrowable() throws Exception {
             String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/codegen/box/specialBuiltins/throwable.kt");

core/descriptors/src/org/jetbrains/kotlin/resolve/DescriptorUtils.kt

@@ -234,3 +234,8 @@ fun CallableMemberDescriptor.firstOverridden(
 fun CallableMemberDescriptor.setSingleOverridden(overridden: CallableMemberDescriptor) {
     overriddenDescriptors = listOf(overridden)
 }
+
+fun CallableMemberDescriptor.overriddenTreeAsSequence(useOriginal: Boolean): Sequence<CallableMemberDescriptor> =
+    with(if (useOriginal) original else this) {
+        sequenceOf(this) + overriddenDescriptors.asSequence().flatMap { it.overriddenTreeAsSequence(useOriginal) }
+    }