From 482b43631c65526f468a2f6d68be32e65027c31f Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Mon, 19 Dec 2016 12:34:26 +0300 Subject: [PATCH] Refactor DelegationChecker and related code - move getDelegates from CodegenUtil to DelegationResolver, reuse it in DelegationChecker - use getAllOverriddenDescriptors + filterOutOverridden instead of a manual DFS --- .../kotlin/backend/common/CodegenUtil.kt | 41 ------------ .../codegen/ImplementationBodyCodegen.java | 26 ++++---- .../kotlin/resolve/DelegationResolver.kt | 38 ++++++++++++ .../resolve/checkers/DelegationChecker.kt | 62 +++---------------- .../delegationBy/fakeOverrideInTheMiddle.kt | 13 ++++ .../delegationBy/fakeOverrideInTheMiddle.txt | 40 ++++++++++++ .../checkers/DiagnosticsTestGenerated.java | 6 ++ .../declaration/DelegationTranslator.kt | 5 +- 8 files changed, 122 insertions(+), 109 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/delegationBy/fakeOverrideInTheMiddle.kt create mode 100644 compiler/testData/diagnostics/tests/delegationBy/fakeOverrideInTheMiddle.txt diff --git a/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/CodegenUtil.kt b/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/CodegenUtil.kt index cc2b9563bdd..a1c31a9293f 100644 --- a/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/CodegenUtil.kt +++ b/compiler/backend-common/src/org/jetbrains/kotlin/backend/common/CodegenUtil.kt @@ -24,51 +24,10 @@ import org.jetbrains.kotlin.name.Name import org.jetbrains.kotlin.psi.* import org.jetbrains.kotlin.resolve.BindingContext import org.jetbrains.kotlin.resolve.DescriptorUtils -import org.jetbrains.kotlin.resolve.MemberComparator -import org.jetbrains.kotlin.resolve.OverridingUtil import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall import org.jetbrains.kotlin.types.KotlinType -import org.jetbrains.kotlin.types.isDynamic -import org.jetbrains.kotlin.utils.keysToMapExceptNulls object CodegenUtil { - // class Foo : Bar by baz - // descriptor = Foo - // toInterface = Bar - // delegateExpressionType = typeof(baz) - // return Map - @JvmStatic - fun getDelegates( - descriptor: ClassDescriptor, - toInterface: ClassDescriptor, - delegateExpressionType: KotlinType? = null - ): Map { - if (delegateExpressionType?.isDynamic() ?: false) return emptyMap() - - return descriptor.defaultType.memberScope.getContributedDescriptors().asSequence() - .filterIsInstance() - .filter { it.kind == CallableMemberDescriptor.Kind.DELEGATION } - .asIterable() - .sortedWith(MemberComparator.INSTANCE) - .keysToMapExceptNulls { delegatingMember -> - val actualDelegates = DescriptorUtils.getAllOverriddenDescriptors(delegatingMember) - .filter { it.containingDeclaration == toInterface } - .map { overriddenDescriptor -> - val scope = (delegateExpressionType ?: toInterface.defaultType).memberScope - val name = overriddenDescriptor.name - - // this is the actual member of delegateExpressionType that we are delegating to - (scope.getContributedFunctions(name, NoLookupLocation.FROM_BACKEND) + - scope.getContributedVariables(name, NoLookupLocation.FROM_BACKEND)) - .firstOrNull { it == overriddenDescriptor || OverridingUtil.overrides(it, overriddenDescriptor) } - } - - assert(actualDelegates.size <= 1) { "Many delegates found for $delegatingMember: $actualDelegates" } - - actualDelegates.firstOrNull() - } - } - @JvmStatic fun getDelegatePropertyIfAny( expression: KtExpression, classDescriptor: ClassDescriptor, bindingContext: BindingContext diff --git a/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java b/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java index 0855a1b9d19..0a9e6d36295 100644 --- a/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java +++ b/compiler/backend/src/org/jetbrains/kotlin/codegen/ImplementationBodyCodegen.java @@ -46,6 +46,7 @@ import org.jetbrains.kotlin.name.FqName; import org.jetbrains.kotlin.name.Name; import org.jetbrains.kotlin.psi.*; import org.jetbrains.kotlin.resolve.BindingContext; +import org.jetbrains.kotlin.resolve.DelegationResolver; import org.jetbrains.kotlin.resolve.DescriptorToSourceUtils; import org.jetbrains.kotlin.resolve.DescriptorUtils; import org.jetbrains.kotlin.resolve.calls.callUtil.CallUtilKt; @@ -1655,18 +1656,21 @@ public class ImplementationBodyCodegen extends ClassBodyCodegen { fieldInfo.name, fieldInfo.type.getDescriptor(), /*TODO*/null, null); } - private void generateDelegates(ClassDescriptor toTrait, KotlinType delegateExpressionType, DelegationFieldsInfo.Field field) { - for (Map.Entry entry : - CodegenUtil.getDelegates(descriptor, toTrait, delegateExpressionType).entrySet()) { - CallableMemberDescriptor callableMemberDescriptor = entry.getKey(); - CallableDescriptor delegateTo = entry.getValue(); - if (callableMemberDescriptor instanceof PropertyDescriptor) { - propertyCodegen - .genDelegate((PropertyDescriptor) callableMemberDescriptor, (PropertyDescriptor) delegateTo, field.getStackValue()); + private void generateDelegates( + @NotNull ClassDescriptor toInterface, + @Nullable KotlinType delegateExpressionType, + @NotNull DelegationFieldsInfo.Field field + ) { + for (Map.Entry entry : DelegationResolver.Companion.getDelegates( + descriptor, toInterface, delegateExpressionType).entrySet() + ) { + CallableMemberDescriptor member = entry.getKey(); + CallableMemberDescriptor delegateTo = entry.getValue(); + if (member instanceof PropertyDescriptor) { + propertyCodegen.genDelegate((PropertyDescriptor) member, (PropertyDescriptor) delegateTo, field.getStackValue()); } - else if (callableMemberDescriptor instanceof FunctionDescriptor) { - functionCodegen - .genDelegate((FunctionDescriptor) callableMemberDescriptor, (FunctionDescriptor) delegateTo, field.getStackValue()); + else if (member instanceof FunctionDescriptor) { + functionCodegen.genDelegate((FunctionDescriptor) member, (FunctionDescriptor) delegateTo, field.getStackValue()); } } } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DelegationResolver.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DelegationResolver.kt index 328f2038e64..323e78344e4 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DelegationResolver.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DelegationResolver.kt @@ -19,6 +19,7 @@ package org.jetbrains.kotlin.resolve import org.jetbrains.kotlin.descriptors.* import org.jetbrains.kotlin.descriptors.CallableMemberDescriptor.Kind.DELEGATION import org.jetbrains.kotlin.diagnostics.Errors.MANY_IMPL_MEMBER_NOT_IMPLEMENTED +import org.jetbrains.kotlin.incremental.components.NoLookupLocation import org.jetbrains.kotlin.psi.KtClassOrObject import org.jetbrains.kotlin.psi.KtDelegatedSuperTypeEntry import org.jetbrains.kotlin.psi.KtPureClassOrObject @@ -27,6 +28,8 @@ import org.jetbrains.kotlin.resolve.OverridingUtil.OverrideCompatibilityInfo.Res import org.jetbrains.kotlin.resolve.lazy.DelegationFilter import org.jetbrains.kotlin.types.KotlinType import org.jetbrains.kotlin.types.TypeUtils +import org.jetbrains.kotlin.types.isDynamic +import org.jetbrains.kotlin.utils.keysToMapExceptNulls class DelegationResolver private constructor( private val classOrObject: KtPureClassOrObject, @@ -124,5 +127,40 @@ class DelegationResolver private constructor( private fun isOverridableBy(memberOne: CallableDescriptor, memberTwo: CallableDescriptor): Boolean = OverridingUtil.DEFAULT.isOverridableBy(memberOne, memberTwo, null).result == OVERRIDABLE + // class Foo : Bar by baz + // descriptor = Foo + // toInterface = Bar + // delegateExpressionType = typeof(baz) + // return Map + fun getDelegates( + descriptor: ClassDescriptor, + toInterface: ClassDescriptor, + delegateExpressionType: KotlinType? = null + ): Map { + if (delegateExpressionType?.isDynamic() ?: false) return emptyMap() + + val delegatedMembers = descriptor.defaultType.memberScope.getContributedDescriptors().asSequence() + .filterIsInstance() + .filter { it.kind == CallableMemberDescriptor.Kind.DELEGATION } + .asIterable() + .sortedWith(MemberComparator.INSTANCE) + + return delegatedMembers + .keysToMapExceptNulls { delegatingMember -> + val actualDelegates = DescriptorUtils.getAllOverriddenDescriptors(delegatingMember) + .filter { it.containingDeclaration == toInterface } + .map { overriddenDescriptor -> + val scope = (delegateExpressionType ?: toInterface.defaultType).memberScope + val name = overriddenDescriptor.name + + // this is the actual member of delegateExpressionType that we are delegating to + (scope.getContributedFunctions(name, NoLookupLocation.WHEN_CHECK_OVERRIDES) + + scope.getContributedVariables(name, NoLookupLocation.WHEN_CHECK_OVERRIDES)) + .firstOrNull { it == overriddenDescriptor || OverridingUtil.overrides(it, overriddenDescriptor) } + } + + actualDelegates.firstOrNull() + } + } } } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/DelegationChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/DelegationChecker.kt index b5f8c5f204c..ffee1e849e6 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/DelegationChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/checkers/DelegationChecker.kt @@ -22,16 +22,13 @@ import org.jetbrains.kotlin.descriptors.DeclarationDescriptor import org.jetbrains.kotlin.descriptors.Modality import org.jetbrains.kotlin.diagnostics.DiagnosticSink import org.jetbrains.kotlin.diagnostics.Errors.DELEGATED_MEMBER_HIDES_SUPERTYPE_OVERRIDE -import org.jetbrains.kotlin.incremental.components.NoLookupLocation import org.jetbrains.kotlin.psi.KtClassOrObject import org.jetbrains.kotlin.psi.KtDeclaration import org.jetbrains.kotlin.psi.KtDelegatedSuperTypeEntry import org.jetbrains.kotlin.resolve.BindingContext +import org.jetbrains.kotlin.resolve.DelegationResolver import org.jetbrains.kotlin.resolve.DescriptorUtils -import org.jetbrains.kotlin.resolve.MemberComparator import org.jetbrains.kotlin.resolve.OverridingUtil -import org.jetbrains.kotlin.utils.DFS -import org.jetbrains.kotlin.utils.keysToMapExceptNulls class DelegationChecker : SimpleDeclarationChecker { override fun check( @@ -43,16 +40,12 @@ class DelegationChecker : SimpleDeclarationChecker { if (descriptor !is ClassDescriptor) return if (declaration !is KtClassOrObject) return - val delegationDescriptors = descriptor.defaultType.memberScope.getContributedDescriptors(). - filterIsInstance().filter { it.kind == CallableMemberDescriptor.Kind.DELEGATION }.sortedWith(MemberComparator.INSTANCE) - for (specifier in declaration.superTypeListEntries) { if (specifier is KtDelegatedSuperTypeEntry) { val superType = specifier.typeReference?.let { bindingContext.get(BindingContext.TYPE, it) } ?: continue val superTypeDescriptor = superType.constructor.declarationDescriptor as? ClassDescriptor ?: continue - val delegates = getDelegates(delegationDescriptors, superTypeDescriptor) - delegates.forEach { (delegated, delegatedTo) -> + for ((delegated, delegatedTo) in DelegationResolver.getDelegates(descriptor, superTypeDescriptor)) { checkDescriptor(declaration, delegated, delegatedTo, diagnosticHolder) } } @@ -65,57 +58,16 @@ class DelegationChecker : SimpleDeclarationChecker { delegatedToDescriptor: CallableMemberDescriptor, diagnosticHolder: DiagnosticSink ) { - val reachableFromDelegated = findAllReachableDeclarations(delegatedDescriptor) - reachableFromDelegated.remove(delegatedDescriptor.original) - val toRemove = linkedSetOf() - for (declaration in reachableFromDelegated) { - val reachable = findAllReachableDeclarations(declaration.original) - reachable.remove(declaration) - toRemove.addAll(reachable) - } - reachableFromDelegated.removeAll(toRemove) - reachableFromDelegated.remove(DescriptorUtils.unwrapFakeOverride(delegatedToDescriptor).original) + val reachableFromDelegated = + OverridingUtil.filterOutOverridden( + DescriptorUtils.getAllOverriddenDescriptors(delegatedDescriptor).filter { it.kind.isReal }.toSet() + ) - DescriptorUtils.unwrapFakeOverride(delegatedToDescriptor).original val nonAbstractReachable = reachableFromDelegated.filter { it.modality != Modality.ABSTRACT } if (nonAbstractReachable.isNotEmpty()) { /*In case of MANY_IMPL_MEMBER_NOT_IMPLEMENTED error there could be several elements otherwise only one*/ - diagnosticHolder.report(DELEGATED_MEMBER_HIDES_SUPERTYPE_OVERRIDE.on(classDeclaration, delegatedDescriptor, nonAbstractReachable.toList())) + diagnosticHolder.report(DELEGATED_MEMBER_HIDES_SUPERTYPE_OVERRIDE.on(classDeclaration, delegatedDescriptor, nonAbstractReachable)) } } - - fun getDelegates( - delegatedMethods: Iterable, - toInterface: ClassDescriptor - ): Map { - return delegatedMethods - .keysToMapExceptNulls { delegatingMember -> - val actualDelegates = DescriptorUtils.getAllOverriddenDescriptors(delegatingMember) - .filter { it.containingDeclaration == toInterface } - .map { overriddenDescriptor -> - val scope = toInterface.defaultType.memberScope - val name = overriddenDescriptor.name - - // this is the actual member of delegateExpressionType that we are delegating to - (scope.getContributedFunctions(name, NoLookupLocation.WHEN_CHECK_OVERRIDES) + - scope.getContributedVariables(name, NoLookupLocation.WHEN_CHECK_OVERRIDES)) - .firstOrNull { it == overriddenDescriptor || OverridingUtil.overrides(it, overriddenDescriptor) } - } - - actualDelegates.firstOrNull() as? CallableMemberDescriptor - } - } -} - -private fun findAllReachableDeclarations(memberDescriptor: CallableMemberDescriptor): MutableSet { - val collector = object : DFS.NodeHandlerWithListResult() { - override fun afterChildren(current: CallableMemberDescriptor) { - if (current.kind.isReal) { - result.add(current.original) - } - } - } - - DFS.dfs(listOf(memberDescriptor), { it.overriddenDescriptors }, collector) - return java.util.HashSet(collector.result()) } diff --git a/compiler/testData/diagnostics/tests/delegationBy/fakeOverrideInTheMiddle.kt b/compiler/testData/diagnostics/tests/delegationBy/fakeOverrideInTheMiddle.kt new file mode 100644 index 00000000000..b8bad3002ce --- /dev/null +++ b/compiler/testData/diagnostics/tests/delegationBy/fakeOverrideInTheMiddle.kt @@ -0,0 +1,13 @@ +interface Base { + fun test() = "Base" +} + +class Delegate : Base + +abstract class Middle : Base { + override fun test() = "MyClass" +} + +abstract class MyClass : Middle() + +class A : MyClass(), Base by Delegate() diff --git a/compiler/testData/diagnostics/tests/delegationBy/fakeOverrideInTheMiddle.txt b/compiler/testData/diagnostics/tests/delegationBy/fakeOverrideInTheMiddle.txt new file mode 100644 index 00000000000..e1afaa91a80 --- /dev/null +++ b/compiler/testData/diagnostics/tests/delegationBy/fakeOverrideInTheMiddle.txt @@ -0,0 +1,40 @@ +package + +public final class A : MyClass, Base { + public constructor A() + public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*2*/ /*delegation*/ fun test(): kotlin.String + public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String +} + +public interface Base { + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open fun test(): kotlin.String + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class Delegate : Base { + public constructor Delegate() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun test(): kotlin.String + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public abstract class Middle : Base { + public constructor Middle() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ fun test(): kotlin.String + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public abstract class MyClass : Middle { + public constructor MyClass() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun test(): kotlin.String + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index d7ee44497c6..898307a6f55 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -6271,6 +6271,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } + @TestMetadata("fakeOverrideInTheMiddle.kt") + public void testFakeOverrideInTheMiddle() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/delegationBy/fakeOverrideInTheMiddle.kt"); + doTest(fileName); + } + @TestMetadata("generic.kt") public void testGeneric() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/delegationBy/generic.kt"); diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/DelegationTranslator.kt b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/DelegationTranslator.kt index c0214a59179..1cd1e695cca 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/DelegationTranslator.kt +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/declaration/DelegationTranslator.kt @@ -30,6 +30,7 @@ import org.jetbrains.kotlin.js.translate.utils.TranslationUtils.translateFunctio import org.jetbrains.kotlin.psi.KtClassOrObject import org.jetbrains.kotlin.psi.KtDelegatedSuperTypeEntry import org.jetbrains.kotlin.psi.KtSuperTypeListEntry +import org.jetbrains.kotlin.resolve.DelegationResolver import org.jetbrains.kotlin.resolve.DescriptorUtils import org.jetbrains.kotlin.resolve.descriptorUtil.isExtensionProperty @@ -42,7 +43,7 @@ class DelegationTranslator( BindingUtils.getClassDescriptor(context.bindingContext(), classDeclaration) private val delegationBySpecifiers = - classDeclaration.getSuperTypeListEntries().filterIsInstance() + classDeclaration.superTypeListEntries.filterIsInstance() private class Field (val name: JsName, val generateField: Boolean) private val fields = mutableMapOf() @@ -91,7 +92,7 @@ class DelegationTranslator( CodegenUtil.getSuperClassBySuperTypeListEntry(specifier, bindingContext()) private fun generateDelegates(toClass: ClassDescriptor, field: Field) { - for ((descriptor, overriddenDescriptor) in CodegenUtil.getDelegates(classDescriptor, toClass)) { + for ((descriptor, overriddenDescriptor) in DelegationResolver.getDelegates(classDescriptor, toClass)) { when (descriptor) { is PropertyDescriptor -> generateDelegateCallForPropertyMember(descriptor, field.name)