From 12552d2fc13c6aed3ebde602df0e19a7b9f73ed3 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Wed, 10 Feb 2016 16:14:34 +0300 Subject: [PATCH] Refine special bridges generation #KT-10958 Fixed --- .../kotlin/codegen/builtinSpecialBridges.kt | 43 +++------ .../removeAtTwoSpecialBridges.kt | 96 +++++++++++++++++++ .../removeAtTwoSpecialBridges.kt | 90 +++++++++++++++++ .../removeAtTwoSpecialBridges.txt | 66 +++++++++++++ .../codegen/BytecodeListingTestGenerated.java | 6 ++ .../BlackBoxCodegenTestGenerated.java | 6 ++ .../kotlin/resolve/DescriptorUtils.kt | 5 + 7 files changed, 282 insertions(+), 30 deletions(-) create mode 100644 compiler/testData/codegen/box/specialBuiltins/removeAtTwoSpecialBridges.kt create mode 100644 compiler/testData/codegen/bytecodeListing/specialBridges/removeAtTwoSpecialBridges.kt create mode 100644 compiler/testData/codegen/bytecodeListing/specialBridges/removeAtTwoSpecialBridges.txt diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/builtinSpecialBridges.kt b/compiler/backend/src/org/jetbrains/kotlin/codegen/builtinSpecialBridges.kt index ab877bec878..df83251ebf9 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/builtinSpecialBridges.kt +++ b/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(), 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 = findAllReachableDeclarations(DescriptorBasedFunctionHandle(functionDescriptor)).map { it.descriptor }.toMutableSet() -private fun needGenerateSpecialBridge( - functionDescriptor: FunctionDescriptor, - reachableDeclarations: Collection, - signatureByDescriptor: (FunctionDescriptor) -> Signature, - overriddenBuiltinSignature: Signature -): Boolean { - // We do not generate special bridge unless it has different JVM descriptor - // e.g. `containsAll(Collection c)` in ListImpl 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 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 CallableMemberDescriptor.getSpecialBridgeSignatureIfExists( signatureByDescriptor: (FunctionDescriptor) -> Signature ): Signature? { @@ -148,7 +131,7 @@ private fun 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 diff --git a/compiler/testData/codegen/box/specialBuiltins/removeAtTwoSpecialBridges.kt b/compiler/testData/codegen/box/specialBuiltins/removeAtTwoSpecialBridges.kt new file mode 100644 index 00000000000..86f4b09d05c --- /dev/null +++ b/compiler/testData/codegen/box/specialBuiltins/removeAtTwoSpecialBridges.kt @@ -0,0 +1,96 @@ +open class A0 : MutableList { + override fun add(element: E): Boolean { + throw UnsupportedOperationException() + } + + override fun add(index: Int, element: E) { + throw UnsupportedOperationException() + } + + override fun addAll(index: Int, elements: Collection): Boolean { + throw UnsupportedOperationException() + } + + override fun addAll(elements: Collection): Boolean { + throw UnsupportedOperationException() + } + + override fun clear() { + throw UnsupportedOperationException() + } + + override fun listIterator(): MutableListIterator { + throw UnsupportedOperationException() + } + + override fun listIterator(index: Int): MutableListIterator { + throw UnsupportedOperationException() + } + + override fun remove(element: E): Boolean { + throw UnsupportedOperationException() + } + + override fun removeAll(elements: Collection): Boolean { + throw UnsupportedOperationException() + } + + override fun removeAt(index: Int): E = "K" as E + + override fun retainAll(elements: Collection): Boolean { + throw UnsupportedOperationException() + } + + override fun set(index: Int, element: E): E { + throw UnsupportedOperationException() + } + + override fun subList(fromIndex: Int, toIndex: Int): MutableList { + throw UnsupportedOperationException() + } + + override val size: Int + get() = throw UnsupportedOperationException() + + override fun contains(element: E): Boolean { + throw UnsupportedOperationException() + } + + override fun containsAll(elements: Collection): 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 { + throw UnsupportedOperationException() + } +} + +class A1() : A0() { + override fun removeAt(p0: Int): String = "O" +} + +class A2 : A0() + +// 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) +} diff --git a/compiler/testData/codegen/bytecodeListing/specialBridges/removeAtTwoSpecialBridges.kt b/compiler/testData/codegen/bytecodeListing/specialBridges/removeAtTwoSpecialBridges.kt new file mode 100644 index 00000000000..89daa48784d --- /dev/null +++ b/compiler/testData/codegen/bytecodeListing/specialBridges/removeAtTwoSpecialBridges.kt @@ -0,0 +1,90 @@ +open class A0 : MutableList { + override fun add(element: E): Boolean { + throw UnsupportedOperationException() + } + + override fun add(index: Int, element: E) { + throw UnsupportedOperationException() + } + + override fun addAll(index: Int, elements: Collection): Boolean { + throw UnsupportedOperationException() + } + + override fun addAll(elements: Collection): Boolean { + throw UnsupportedOperationException() + } + + override fun clear() { + throw UnsupportedOperationException() + } + + override fun listIterator(): MutableListIterator { + throw UnsupportedOperationException() + } + + override fun listIterator(index: Int): MutableListIterator { + throw UnsupportedOperationException() + } + + override fun remove(element: E): Boolean { + throw UnsupportedOperationException() + } + + override fun removeAll(elements: Collection): Boolean { + throw UnsupportedOperationException() + } + + override fun removeAt(index: Int): E { + throw UnsupportedOperationException() + } + + override fun retainAll(elements: Collection): Boolean { + throw UnsupportedOperationException() + } + + override fun set(index: Int, element: E): E { + throw UnsupportedOperationException() + } + + override fun subList(fromIndex: Int, toIndex: Int): MutableList { + throw UnsupportedOperationException() + } + + override val size: Int + get() = throw UnsupportedOperationException() + + override fun contains(element: E): Boolean { + throw UnsupportedOperationException() + } + + override fun containsAll(elements: Collection): 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 { + throw UnsupportedOperationException() + } +} + +class A1() : A0() { + override fun removeAt(p0: Int): String = "abc" +} + +class A2 : A0() diff --git a/compiler/testData/codegen/bytecodeListing/specialBridges/removeAtTwoSpecialBridges.txt b/compiler/testData/codegen/bytecodeListing/specialBridges/removeAtTwoSpecialBridges.txt new file mode 100644 index 00000000000..1d6ec1a61d3 --- /dev/null +++ b/compiler/testData/codegen/bytecodeListing/specialBridges/removeAtTwoSpecialBridges.txt @@ -0,0 +1,66 @@ +@kotlin.Metadata +public class A0 { + public method (): 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 (): 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 (): 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[] +} diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeListingTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeListingTestGenerated.java index a9bb4387c44..254fcf3100b 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/BytecodeListingTestGenerated.java +++ b/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); + } } } diff --git a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java index af52675072b..43b36cde54d 100644 --- a/compiler/tests/org/jetbrains/kotlin/codegen/generated/BlackBoxCodegenTestGenerated.java +++ b/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"); diff --git a/core/descriptors/src/org/jetbrains/kotlin/resolve/DescriptorUtils.kt b/core/descriptors/src/org/jetbrains/kotlin/resolve/DescriptorUtils.kt index 7407f21da5c..2ea15247fec 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/resolve/DescriptorUtils.kt +++ b/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 = + with(if (useOriginal) original else this) { + sequenceOf(this) + overriddenDescriptors.asSequence().flatMap { it.overriddenTreeAsSequence(useOriginal) } + }