From 56be83cdd3e9a35b0c23985c62fd4f925082c35d Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Tue, 9 Jan 2018 12:39:19 +0100 Subject: [PATCH] Improve fake override construction for expected classes Fake overrides for abstract members from expected classes should become non-abstract (final, in fact) in non-abstract expected subclasses #KT-22031 Fixed --- .../extendExpectedClassWithAbstractMember.kt | 55 ++++++++ .../extendExpectedClassWithAbstractMember.txt | 127 ++++++++++++++++++ ...edClassWithoutExplicitOverrideOfMethod.txt | 2 +- .../checkers/DiagnosticsTestGenerated.java | 6 + .../DiagnosticsUsingJavacTestGenerated.java | 6 + .../kotlin/resolve/OverridingUtil.java | 38 ++++-- 6 files changed, 223 insertions(+), 11 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithAbstractMember.kt create mode 100644 compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithAbstractMember.txt diff --git a/compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithAbstractMember.kt b/compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithAbstractMember.kt new file mode 100644 index 00000000000..07fd2ac9e72 --- /dev/null +++ b/compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithAbstractMember.kt @@ -0,0 +1,55 @@ +// !LANGUAGE: +MultiPlatformProjects +// MODULE: m1-common +// FILE: common.kt + +expect abstract class BaseA() { + abstract fun foo() +} +expect open class BaseAImpl() : BaseA + +class DerivedA1 : BaseAImpl() +class DerivedA2 : BaseAImpl() { + override fun foo() = super.foo() +} + + + +expect interface BaseB { + fun foo() +} +expect open class BaseBImpl() : BaseB + +class DerivedB1 : BaseBImpl() +class DerivedB2 : BaseBImpl() { + override fun foo() = super.foo() +} + + + +expect interface BaseC { + fun foo() +} +expect abstract class BaseCImpl() : BaseC + +class DerivedC1 : BaseCImpl() +class DerivedC2 : BaseCImpl() { + override fun foo() = super.foo() +} + + + +expect interface BaseD { + fun foo() +} +abstract class BaseDImpl() : BaseD { + fun bar() = super.foo() +} + + + +expect interface BaseE { + fun foo() +} +sealed class BaseEImpl() : BaseE { + fun bar() = super.foo() +} diff --git a/compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithAbstractMember.txt b/compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithAbstractMember.txt new file mode 100644 index 00000000000..97a9b1c3dd0 --- /dev/null +++ b/compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithAbstractMember.txt @@ -0,0 +1,127 @@ +package + +public abstract expect class BaseA { + public constructor BaseA() + 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 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 override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public expect interface BaseB { + 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 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 override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public expect interface BaseC { + 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 abstract expect class BaseCImpl : BaseC { + public constructor BaseCImpl() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public abstract 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 expect interface BaseD { + 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 abstract class BaseDImpl : BaseD { + public constructor BaseDImpl() + public final fun bar(): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public abstract 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 expect interface BaseE { + 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 sealed class BaseEImpl : BaseE { + private constructor BaseEImpl() + public final fun bar(): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public abstract 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 override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class DerivedA2 : BaseAImpl { + public constructor DerivedA2() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ 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 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 override /*1*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class DerivedB2 : BaseBImpl { + public constructor DerivedB2() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ 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 DerivedC1 : BaseCImpl { + public constructor DerivedC1() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public abstract 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 DerivedC2 : BaseCImpl { + public constructor DerivedC2() + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*1*/ 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/extendExpectedClassWithoutExplicitOverrideOfMethod.txt b/compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithoutExplicitOverrideOfMethod.txt index 9c8908e708b..988948c5281 100644 --- a/compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithoutExplicitOverrideOfMethod.txt +++ b/compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithoutExplicitOverrideOfMethod.txt @@ -24,7 +24,7 @@ public final expect class DerivedExplicitCheck : Base { public final expect class DerivedImplicit : Base { public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean - public abstract expect override /*1*/ /*fake_override*/ fun foo(): kotlin.Unit + 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 } diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 5dea44e4a87..ba47286318a 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -14452,6 +14452,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } + @TestMetadata("extendExpectedClassWithAbstractMember.kt") + public void testExtendExpectedClassWithAbstractMember() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithAbstractMember.kt"); + doTest(fileName); + } + @TestMetadata("extendExpectedClassWithoutExplicitOverrideOfMethod.kt") public void testExtendExpectedClassWithoutExplicitOverrideOfMethod() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithoutExplicitOverrideOfMethod.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java index d592a257a8e..959a928d9e7 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/javac/DiagnosticsUsingJavacTestGenerated.java @@ -14452,6 +14452,12 @@ public class DiagnosticsUsingJavacTestGenerated extends AbstractDiagnosticsUsing doTest(fileName); } + @TestMetadata("extendExpectedClassWithAbstractMember.kt") + public void testExtendExpectedClassWithAbstractMember() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithAbstractMember.kt"); + doTest(fileName); + } + @TestMetadata("extendExpectedClassWithoutExplicitOverrideOfMethod.kt") public void testExtendExpectedClassWithoutExplicitOverrideOfMethod() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/multiplatform/headerClass/extendExpectedClassWithoutExplicitOverrideOfMethod.kt"); diff --git a/core/descriptors/src/org/jetbrains/kotlin/resolve/OverridingUtil.java b/core/descriptors/src/org/jetbrains/kotlin/resolve/OverridingUtil.java index 539d8078efb..1d4594806f0 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/resolve/OverridingUtil.java +++ b/core/descriptors/src/org/jetbrains/kotlin/resolve/OverridingUtil.java @@ -625,7 +625,7 @@ public class OverridingUtil { boolean allInvisible = visibleOverridables.isEmpty(); Collection effectiveOverridden = allInvisible ? overridables : visibleOverridables; - Modality modality = determineModality(effectiveOverridden); + Modality modality = determineModalityForFakeOverride(effectiveOverridden, current); Visibility visibility = allInvisible ? Visibilities.INVISIBLE_FAKE : Visibilities.INHERITED; // FIXME doesn't work as expected for flexible types: should create a refined signature. @@ -652,11 +652,16 @@ public class OverridingUtil { } @NotNull - private static Modality determineModality(@NotNull Collection descriptors) { + private static Modality determineModalityForFakeOverride( + @NotNull Collection descriptors, + @NotNull ClassDescriptor current + ) { // 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; @@ -671,25 +676,38 @@ public class OverridingUtil { } } - if (hasOpen && !hasAbstract) return Modality.OPEN; - if (!hasOpen && hasAbstract) return Modality.ABSTRACT; + if (!hasExpect) { + if (hasOpen && !hasAbstract) return Modality.OPEN; + if (!hasOpen && hasAbstract) return Modality.ABSTRACT; + } Set allOverriddenDeclarations = new HashSet(); for (CallableMemberDescriptor descriptor : descriptors) { allOverriddenDeclarations.addAll(getOverriddenDeclarations(descriptor)); } - return getMinimalModality(filterOutOverridden(allOverriddenDeclarations)); + // 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); } @NotNull - private static Modality getMinimalModality(@NotNull Collection descriptors) { - Modality modality = Modality.ABSTRACT; + private static Modality getMinimalModality( + @NotNull Collection descriptors, + boolean transformAbstractToFinal + ) { + Modality result = Modality.ABSTRACT; for (CallableMemberDescriptor descriptor : descriptors) { - if (descriptor.getModality().compareTo(modality) < 0) { - modality = descriptor.getModality(); + Modality effectiveModality = + transformAbstractToFinal && descriptor.getModality() == Modality.ABSTRACT ? Modality.FINAL : descriptor.getModality(); + if (effectiveModality.compareTo(result) < 0) { + result = effectiveModality; } } - return modality; + return result; } @NotNull