From 6fb7a79a1f40f88532f906d1b24d0f16f5e6bd40 Mon Sep 17 00:00:00 2001 From: "Pavel V. Talanov" Date: Tue, 17 Sep 2013 13:24:06 +0400 Subject: [PATCH] Refactor DelegationResolver Fix an issue in lazy resolve : filter out methods from delegated traits superclass --- .../jet/lang/resolve/DelegationResolver.java | 174 ++++++++++++------ .../jet/lang/resolve/OverrideResolver.java | 6 +- .../descriptors/LazyClassMemberScope.java | 6 +- ...ForFunctionInheritedFromTraitSuperClass.kt | 23 +++ ...orFunctionInheritedFromTraitSuperClass.txt | 30 +++ .../DescriptorSerializationTestGenerated.java | 5 + .../LoadCompiledKotlinTestGenerated.java | 5 + ...esolveNamespaceComparingTestGenerated.java | 5 + 8 files changed, 192 insertions(+), 62 deletions(-) create mode 100644 compiler/testData/loadKotlin/fun/NoDelegationForFunctionInheritedFromTraitSuperClass.kt create mode 100644 compiler/testData/loadKotlin/fun/NoDelegationForFunctionInheritedFromTraitSuperClass.txt diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DelegationResolver.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DelegationResolver.java index 70ef3758692..c49c16a15dd 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DelegationResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/DelegationResolver.java @@ -32,67 +32,126 @@ import org.jetbrains.jet.lang.types.TypeUtils; import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import static org.jetbrains.jet.lang.diagnostics.Errors.MANY_IMPL_MEMBER_NOT_IMPLEMENTED; +import static org.jetbrains.jet.lang.resolve.OverridingUtil.OverrideCompatibilityInfo.Result.*; public class DelegationResolver { - private DelegationResolver() {} + private DelegationResolver() { + } - public static void addDelegatedMembers(@NotNull BindingTrace trace, @NotNull JetClassOrObject jetClass, @NotNull MutableClassDescriptor classDescriptor) { + @NotNull + public static Collection generateDelegatedMembers( + @NotNull BindingTrace trace, + @NotNull JetClassOrObject jetClass, + @NotNull MutableClassDescriptor classDescriptor + ) { + Collection delegatedMembers = new HashSet(); for (JetDelegationSpecifier delegationSpecifier : jetClass.getDelegationSpecifiers()) { - if (delegationSpecifier instanceof JetDelegatorByExpressionSpecifier) { - JetDelegatorByExpressionSpecifier specifier = (JetDelegatorByExpressionSpecifier) delegationSpecifier; - JetType type = trace.get(BindingContext.TYPE, specifier.getTypeReference()); - if (type != null) { - final Collection membersToSkip = getMembersFromClassSupertype(type); - Collection descriptorsToDelegate = Collections2.filter(extractCallableMembers(type), - new Predicate() { - @Override - public boolean apply(@Nullable CallableMemberDescriptor descriptor) { - for (CallableMemberDescriptor memberToSkip : membersToSkip) { - if (haveSameSignatures(memberToSkip, descriptor)) { - return false; - } - } - return true; - } - }); - - Collection generatedDescriptors = generateDelegatedMembers(classDescriptor, descriptorsToDelegate); - outer: - for (CallableMemberDescriptor descriptor : generatedDescriptors) { - for (CallableMemberDescriptor existingDescriptor : classDescriptor.getAllCallableMembers()) { - if (OverridingUtil.isOverridableBy(existingDescriptor, descriptor).getResult() == OverridingUtil.OverrideCompatibilityInfo.Result.OVERRIDABLE) { - if (existingDescriptor.getKind() == CallableMemberDescriptor.Kind.DELEGATION) { - //trying to delegate to many traits with the same methods - trace.report(MANY_IMPL_MEMBER_NOT_IMPLEMENTED.on(jetClass.getNameIdentifier(), jetClass, existingDescriptor)); - } - continue outer; - } - } - - if (descriptor instanceof PropertyDescriptor) { - PropertyDescriptor propertyDescriptor = (PropertyDescriptor) descriptor; - classDescriptor.getBuilder().addPropertyDescriptor(propertyDescriptor); - } - else if (descriptor instanceof SimpleFunctionDescriptor) { - SimpleFunctionDescriptor functionDescriptor = (SimpleFunctionDescriptor) descriptor; - classDescriptor.getBuilder().addFunctionDescriptor(functionDescriptor); - } - } + if (!(delegationSpecifier instanceof JetDelegatorByExpressionSpecifier)) { + continue; + } + JetDelegatorByExpressionSpecifier specifier = (JetDelegatorByExpressionSpecifier) delegationSpecifier; + JetType type = trace.get(BindingContext.TYPE, specifier.getTypeReference()); + if (type == null) { + continue; + } + Collection candidates = + generateDelegationCandidates(classDescriptor, type, extractCallableMembers(type)); + for (CallableMemberDescriptor candidate : candidates) { + if (existingMemberOverridesDelegatedMember(classDescriptor, candidate)) { + continue; } + //only leave the first delegated member + if (checkClashWithOtherDelegatedMember(trace, jetClass, delegatedMembers, candidate)) { + continue; + } + + delegatedMembers.add(candidate); } } + return delegatedMembers; } - private static boolean haveSameSignatures(CallableDescriptor memberOne, CallableDescriptor memberTwo) { - return OverridingUtil.isOverridableBy(memberOne, memberTwo).getResult() == OverridingUtil.OverrideCompatibilityInfo.Result.OVERRIDABLE; + private static boolean checkClashWithOtherDelegatedMember( + @NotNull BindingTrace trace, + @NotNull JetClassOrObject jetClass, + @NotNull Collection delegatedMembers, + @NotNull CallableMemberDescriptor candidate + ) { + for (CallableMemberDescriptor alreadyDelegatedMember : delegatedMembers) { + if (haveSameSignatures(alreadyDelegatedMember, candidate)) { + //trying to delegate to many traits with the same methods + trace.report(MANY_IMPL_MEMBER_NOT_IMPLEMENTED.on(jetClass.getNameIdentifier(), jetClass, alreadyDelegatedMember)); + return true; + } + } + return false; } - private static Collection getMembersFromClassSupertype(JetType type) { + private static boolean existingMemberOverridesDelegatedMember( + MutableClassDescriptor classDescriptor, + CallableMemberDescriptor candidate + ) { + for (CallableMemberDescriptor existingDescriptor : classDescriptor.getAllCallableMembers()) { + if (haveSameSignatures(existingDescriptor, candidate)) { + return true; + } + } + return false; + } + + @NotNull + public static Collection generateDelegationCandidates( + @NotNull ClassDescriptor classDescriptor, + @NotNull JetType delegatedTraitType, + @NotNull Collection membersFromTrait + ) { + Collection descriptorsToDelegate = filterMembersFromSuperClassOfDelegatedTrait(delegatedTraitType, membersFromTrait); + Collection result = Lists.newArrayList(); + for (T memberDescriptor : descriptorsToDelegate) { + if (memberDescriptor.getModality().isOverridable()) { + Modality modality = DescriptorUtils.convertModality(memberDescriptor.getModality(), true); + @SuppressWarnings("unchecked") + T copy = (T) memberDescriptor.copy(classDescriptor, modality, memberDescriptor.getVisibility(), + CallableMemberDescriptor.Kind.DELEGATION, false); + result.add(copy); + } + } + return result; + } + + @NotNull + private static Collection filterMembersFromSuperClassOfDelegatedTrait( + @NotNull JetType delegatedTraitType, + @NotNull Collection membersFromTrait + ) { + final Collection membersToSkip = getMembersFromClassSupertypeOfTrait(delegatedTraitType); + return Collections2.filter( + membersFromTrait, + new Predicate() { + @Override + public boolean apply(@Nullable CallableMemberDescriptor descriptor) { + for (CallableMemberDescriptor memberToSkip : membersToSkip) { + if (haveSameSignatures(memberToSkip, descriptor)) { + return false; + } + } + return true; + } + }); + } + + private static boolean haveSameSignatures(@NotNull CallableDescriptor memberOne, @NotNull CallableDescriptor memberTwo) { + return OverridingUtil.isOverridableBy(memberOne, memberTwo).getResult() == OVERRIDABLE; + } + + @NotNull + private static Collection getMembersFromClassSupertypeOfTrait(@NotNull JetType delegateTraitType) { JetType classSupertype = null; - for (JetType supertype : TypeUtils.getAllSupertypes(type)) { + for (JetType supertype : TypeUtils.getAllSupertypes(delegateTraitType)) { if (isNotTrait(supertype.getConstructor().getDeclarationDescriptor())) { classSupertype = supertype; break; @@ -116,17 +175,20 @@ public class DelegationResolver { return false; } - public static Collection generateDelegatedMembers(DeclarationDescriptor newOwner, Collection delegatedDescriptors) { - Collection result = Lists.newArrayList(); - for (CallableMemberDescriptor memberDescriptor : delegatedDescriptors) { - if (memberDescriptor.getModality().isOverridable()) { - Modality modality = DescriptorUtils.convertModality(memberDescriptor.getModality(), true); - CallableMemberDescriptor copy = - memberDescriptor.copy(newOwner, modality, memberDescriptor.getVisibility(), CallableMemberDescriptor.Kind.DELEGATION, false); - result.add(copy); + public static void generateDelegatesInAClass( + @NotNull MutableClassDescriptor classDescriptor, + @NotNull BindingTrace trace, + @NotNull JetClassOrObject jetClassOrObject + ) { + for (CallableMemberDescriptor descriptor : generateDelegatedMembers(trace, jetClassOrObject, classDescriptor)) { + if (descriptor instanceof PropertyDescriptor) { + PropertyDescriptor propertyDescriptor = (PropertyDescriptor) descriptor; + classDescriptor.getBuilder().addPropertyDescriptor(propertyDescriptor); + } + else if (descriptor instanceof SimpleFunctionDescriptor) { + SimpleFunctionDescriptor functionDescriptor = (SimpleFunctionDescriptor) descriptor; + classDescriptor.getBuilder().addFunctionDescriptor(functionDescriptor); } } - //noinspection unchecked - return (Collection) result; } } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java index baa02e7ab01..9ed9bd945c7 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/OverrideResolver.java @@ -114,9 +114,9 @@ public class OverrideResolver { } } - PsiElement declaration = BindingContextUtils.classDescriptorToDeclaration(trace.getBindingContext(), classDescriptor); - DelegationResolver.addDelegatedMembers(trace, (JetClassOrObject) declaration, classDescriptor); - + JetClassOrObject classOrObject = (JetClassOrObject) BindingContextUtils + .classDescriptorToDeclaration(trace.getBindingContext(), classDescriptor); + DelegationResolver.generateDelegatesInAClass(classDescriptor, trace, classOrObject); generateOverridesInAClass(classDescriptor); } diff --git a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/lazy/descriptors/LazyClassMemberScope.java b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/lazy/descriptors/LazyClassMemberScope.java index d7158884c88..0e08d4d9af8 100644 --- a/compiler/frontend/src/org/jetbrains/jet/lang/resolve/lazy/descriptors/LazyClassMemberScope.java +++ b/compiler/frontend/src/org/jetbrains/jet/lang/resolve/lazy/descriptors/LazyClassMemberScope.java @@ -42,6 +42,8 @@ import java.util.Collections; import java.util.List; import java.util.Set; +import static org.jetbrains.jet.lang.resolve.DelegationResolver.generateDelegationCandidates; + public class LazyClassMemberScope extends AbstractLazyMemberScope { @NotNull @@ -268,9 +270,7 @@ public class LazyClassMemberScope extends AbstractLazyMemberScope descriptors = - DelegationResolver.generateDelegatedMembers(thisDescriptor, extractor.extract(supertype, name)); - result.addAll(descriptors); + result.addAll(generateDelegationCandidates(thisDescriptor, supertype, extractor.extract(supertype, name))); } } } diff --git a/compiler/testData/loadKotlin/fun/NoDelegationForFunctionInheritedFromTraitSuperClass.kt b/compiler/testData/loadKotlin/fun/NoDelegationForFunctionInheritedFromTraitSuperClass.kt new file mode 100644 index 00000000000..1bc312bf1ed --- /dev/null +++ b/compiler/testData/loadKotlin/fun/NoDelegationForFunctionInheritedFromTraitSuperClass.kt @@ -0,0 +1,23 @@ +package test + +abstract class A { + abstract fun foo() +} + +trait X : A { + fun bar() { + } +} + +open class B() : A() { + override fun foo() { + } +} + +class C() : A(), X { + override fun foo() { + } +} + +class D(val c: C) : B(), X by c { +} \ No newline at end of file diff --git a/compiler/testData/loadKotlin/fun/NoDelegationForFunctionInheritedFromTraitSuperClass.txt b/compiler/testData/loadKotlin/fun/NoDelegationForFunctionInheritedFromTraitSuperClass.txt new file mode 100644 index 00000000000..0b9157d14e3 --- /dev/null +++ b/compiler/testData/loadKotlin/fun/NoDelegationForFunctionInheritedFromTraitSuperClass.txt @@ -0,0 +1,30 @@ +package test + +internal abstract class A { + /*primary*/ public constructor A() + internal abstract fun foo(): jet.Unit +} + +internal open class B : test.A { + /*primary*/ public constructor B() + internal open override /*1*/ fun foo(): jet.Unit +} + +internal final class C : test.A, test.X { + /*primary*/ public constructor C() + internal open override /*1*/ /*fake_override*/ fun bar(): jet.Unit + internal open override /*2*/ fun foo(): jet.Unit +} + +internal final class D : test.B, test.X { + /*primary*/ public constructor D(/*0*/ c: test.C) + internal final val c: test.C + internal final fun (): test.C + internal open override /*1*/ /*delegation*/ fun bar(): jet.Unit + internal open override /*2*/ /*fake_override*/ fun foo(): jet.Unit +} + +internal trait X : test.A { + internal open fun bar(): jet.Unit + internal abstract override /*1*/ /*fake_override*/ fun foo(): jet.Unit +} diff --git a/compiler/tests/org/jetbrains/jet/descriptors/serialization/DescriptorSerializationTestGenerated.java b/compiler/tests/org/jetbrains/jet/descriptors/serialization/DescriptorSerializationTestGenerated.java index 60dfde0f3ce..3f9bd383b33 100644 --- a/compiler/tests/org/jetbrains/jet/descriptors/serialization/DescriptorSerializationTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/descriptors/serialization/DescriptorSerializationTestGenerated.java @@ -426,6 +426,11 @@ public class DescriptorSerializationTestGenerated extends AbstractDescriptorSeri doTest("compiler/testData/loadKotlin/fun/InheritValsDifferentTypes.kt"); } + @TestMetadata("NoDelegationForFunctionInheritedFromTraitSuperClass.kt") + public void testNoDelegationForFunctionInheritedFromTraitSuperClass() throws Exception { + doTest("compiler/testData/loadKotlin/fun/NoDelegationForFunctionInheritedFromTraitSuperClass.kt"); + } + @TestMetadata("NoSamAdapter.kt") public void testNoSamAdapter() throws Exception { doTest("compiler/testData/loadKotlin/fun/NoSamAdapter.kt"); diff --git a/compiler/tests/org/jetbrains/jet/jvm/compiler/LoadCompiledKotlinTestGenerated.java b/compiler/tests/org/jetbrains/jet/jvm/compiler/LoadCompiledKotlinTestGenerated.java index 06d7020a7be..444cc2b342b 100644 --- a/compiler/tests/org/jetbrains/jet/jvm/compiler/LoadCompiledKotlinTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/jvm/compiler/LoadCompiledKotlinTestGenerated.java @@ -645,6 +645,11 @@ public class LoadCompiledKotlinTestGenerated extends AbstractLoadCompiledKotlinT doTestWithAccessors("compiler/testData/loadKotlin/fun/InheritValsDifferentTypes.kt"); } + @TestMetadata("NoDelegationForFunctionInheritedFromTraitSuperClass.kt") + public void testNoDelegationForFunctionInheritedFromTraitSuperClass() throws Exception { + doTestWithAccessors("compiler/testData/loadKotlin/fun/NoDelegationForFunctionInheritedFromTraitSuperClass.kt"); + } + @TestMetadata("NoSamAdapter.kt") public void testNoSamAdapter() throws Exception { doTestWithAccessors("compiler/testData/loadKotlin/fun/NoSamAdapter.kt"); diff --git a/compiler/tests/org/jetbrains/jet/lang/resolve/lazy/LazyResolveNamespaceComparingTestGenerated.java b/compiler/tests/org/jetbrains/jet/lang/resolve/lazy/LazyResolveNamespaceComparingTestGenerated.java index a39a74f3197..a7b325dc2a6 100644 --- a/compiler/tests/org/jetbrains/jet/lang/resolve/lazy/LazyResolveNamespaceComparingTestGenerated.java +++ b/compiler/tests/org/jetbrains/jet/lang/resolve/lazy/LazyResolveNamespaceComparingTestGenerated.java @@ -647,6 +647,11 @@ public class LazyResolveNamespaceComparingTestGenerated extends AbstractLazyReso doTestCheckingPrimaryConstructorsAndAccessors("compiler/testData/loadKotlin/fun/InheritValsDifferentTypes.kt"); } + @TestMetadata("NoDelegationForFunctionInheritedFromTraitSuperClass.kt") + public void testNoDelegationForFunctionInheritedFromTraitSuperClass() throws Exception { + doTestCheckingPrimaryConstructorsAndAccessors("compiler/testData/loadKotlin/fun/NoDelegationForFunctionInheritedFromTraitSuperClass.kt"); + } + @TestMetadata("NoSamAdapter.kt") public void testNoSamAdapter() throws Exception { doTestCheckingPrimaryConstructorsAndAccessors("compiler/testData/loadKotlin/fun/NoSamAdapter.kt");