From 60a551404a61d02a4c7935ad150b1038de206427 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Tue, 13 Feb 2018 14:25:57 +0100 Subject: [PATCH] Refine modality of fake overrides inherited from abstract expected members In an open expected class inheriting an expected interface, abstract members are now inherited as _open_ fake overrides, not final. Final was technically safer but also stricter and thus could be unexpected by the user. In a final class, abstract members are still inherited as _final_ fake overrides. So, the general rule is now the following: the modality of an expected fake override, which overrides only abstract members, in a non-abstract class is equal to the modality of that class #KT-22031 Fixed --- ...errideMethodsFromInterfaceInCommonCode.txt | 4 +-- .../extendExpectedClassWithAbstractMember.kt | 11 +++++-- .../extendExpectedClassWithAbstractMember.txt | 23 ++++++++++--- .../kotlin/resolve/OverridingUtil.java | 32 +++++++++++-------- 4 files changed, 48 insertions(+), 22 deletions(-) diff --git a/compiler/testData/diagnostics/tests/multiplatform/headerClass/dontOverrideMethodsFromInterfaceInCommonCode.txt b/compiler/testData/diagnostics/tests/multiplatform/headerClass/dontOverrideMethodsFromInterfaceInCommonCode.txt index 06fdcc25f1a..21dbb52f53b 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/headerClass/dontOverrideMethodsFromInterfaceInCommonCode.txt +++ b/compiler/testData/diagnostics/tests/multiplatform/headerClass/dontOverrideMethodsFromInterfaceInCommonCode.txt @@ -17,14 +17,14 @@ public interface Foo { public final expect class ImplicitFoo : Foo { public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean - public abstract override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit + public final override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } public final expect class ImplicitFooCheck : Foo { public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean - public abstract override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit + public final override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } diff --git a/compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithAbstractMember.kt b/compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithAbstractMember.kt index 07fd2ac9e72..984e8fc66ed 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithAbstractMember.kt +++ b/compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithAbstractMember.kt @@ -9,7 +9,7 @@ expect open class BaseAImpl() : BaseA class DerivedA1 : BaseAImpl() class DerivedA2 : BaseAImpl() { - override fun foo() = super.foo() + override fun foo() = super.foo() } @@ -21,7 +21,7 @@ expect open class BaseBImpl() : BaseB class DerivedB1 : BaseBImpl() class DerivedB2 : BaseBImpl() { - override fun foo() = super.foo() + override fun foo() = super.foo() } @@ -53,3 +53,10 @@ expect interface BaseE { sealed class BaseEImpl() : BaseE { fun bar() = super.foo() } + + + +expect interface BaseF { + fun foo() +} +expect class BaseFImpl() : BaseF diff --git a/compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithAbstractMember.txt b/compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithAbstractMember.txt index 97a9b1c3dd0..b476ae9b950 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithAbstractMember.txt +++ b/compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithAbstractMember.txt @@ -11,7 +11,7 @@ public abstract expect class BaseA { public open expect class BaseAImpl : BaseA { public constructor BaseAImpl() public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean - public final expect override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit + public open expect override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } @@ -26,7 +26,7 @@ public expect interface BaseB { public open expect class BaseBImpl : BaseB { public constructor BaseBImpl() public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean - public final expect override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit + public open expect override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } @@ -78,10 +78,25 @@ public sealed class BaseEImpl : BaseE { public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } +public expect interface BaseF { + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public abstract expect fun foo(): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final expect class BaseFImpl : BaseF { + public constructor BaseFImpl() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public final expect override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + public final class DerivedA1 : BaseAImpl { public constructor DerivedA1() public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean - public final expect override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit + public open expect override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } @@ -97,7 +112,7 @@ public final class DerivedA2 : BaseAImpl { public final class DerivedB1 : BaseBImpl { public constructor DerivedB1() public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean - public final expect override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit + public open expect override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit public open override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String } diff --git a/core/descriptors/src/org/jetbrains/kotlin/resolve/OverridingUtil.java b/core/descriptors/src/org/jetbrains/kotlin/resolve/OverridingUtil.java index 1d4594806f0..b558d9e7bd6 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/resolve/OverridingUtil.java +++ b/core/descriptors/src/org/jetbrains/kotlin/resolve/OverridingUtil.java @@ -659,9 +659,7 @@ public class OverridingUtil { // Optimization: avoid creating hash sets in frequent cases when modality can be computed trivially boolean hasOpen = false; boolean hasAbstract = false; - boolean hasExpect = false; for (CallableMemberDescriptor descriptor : descriptors) { - hasExpect |= descriptor.isExpect(); switch (descriptor.getModality()) { case FINAL: return Modality.FINAL; @@ -676,33 +674,39 @@ public class OverridingUtil { } } - if (!hasExpect) { - if (hasOpen && !hasAbstract) return Modality.OPEN; - if (!hasOpen && hasAbstract) return Modality.ABSTRACT; + // Fake overrides of abstract members in non-abstract expected classes should not be abstract, because otherwise it would be + // impossible to inherit a non-expected class from that expected class in common code. + // We're making their modality that of the containing class, because this is the least confusing behavior for the users. + // However, it may cause problems if we reuse resolution results of common code when compiling platform code (see KT-15220) + boolean transformAbstractToClassModality = + current.isExpect() && (current.getModality() != Modality.ABSTRACT && current.getModality() != Modality.SEALED); + + if (hasOpen && !hasAbstract) { + return Modality.OPEN; + } + if (!hasOpen && hasAbstract) { + return transformAbstractToClassModality ? current.getModality() : Modality.ABSTRACT; } Set allOverriddenDeclarations = new HashSet(); for (CallableMemberDescriptor descriptor : descriptors) { allOverriddenDeclarations.addAll(getOverriddenDeclarations(descriptor)); } - // Fake overrides of abstract members in non-abstract expected classes should not be abstract, because otherwise it would be - // impossible to inherit a non-expected class from that expected class in common code. - // We cannot assume that they're open though, because the actual abstract function from an expected super class - // can be implemented with a final function in the actual class for this class. - boolean transformAbstractToFinal = - current.isExpect() && (current.getModality() != Modality.ABSTRACT && current.getModality() != Modality.SEALED); - return getMinimalModality(filterOutOverridden(allOverriddenDeclarations), transformAbstractToFinal); + return getMinimalModality(filterOutOverridden(allOverriddenDeclarations), transformAbstractToClassModality, current.getModality()); } @NotNull private static Modality getMinimalModality( @NotNull Collection descriptors, - boolean transformAbstractToFinal + boolean transformAbstractToClassModality, + @NotNull Modality classModality ) { Modality result = Modality.ABSTRACT; for (CallableMemberDescriptor descriptor : descriptors) { Modality effectiveModality = - transformAbstractToFinal && descriptor.getModality() == Modality.ABSTRACT ? Modality.FINAL : descriptor.getModality(); + transformAbstractToClassModality && descriptor.getModality() == Modality.ABSTRACT + ? classModality + : descriptor.getModality(); if (effectiveModality.compareTo(result) < 0) { result = effectiveModality; }