From b72293883d94e3b4c5281fff08ce78234cf3b393 Mon Sep 17 00:00:00 2001 From: Alexander Udalov Date: Mon, 11 Jul 2016 20:19:53 +0300 Subject: [PATCH] Fix algorithm of determining modality for fake overrides Previously we inferred "open" if there was at least one open member in the hierarchy. However, that's not correct when that member is overridden by another member in the hierarchy which is abstract. This led to incorrect code being accepted by the front-end, and an exception during the bridge generation #KT-12467 Fixed --- .../tests/override/FakeOverrideModality1.kt | 14 +++++++ .../tests/override/FakeOverrideModality1.txt | 30 +++++++++++++++ .../tests/override/FakeOverrideModality2.kt | 18 +++++++++ .../tests/override/FakeOverrideModality2.txt | 37 +++++++++++++++++++ .../tests/override/FakeOverrideModality3.kt | 16 ++++++++ .../tests/override/FakeOverrideModality3.txt | 37 +++++++++++++++++++ .../diagnostics/tests/override/kt12358.kt | 2 +- .../diagnostics/tests/override/kt12358.txt | 4 +- .../diagnostics/tests/override/kt12467.kt | 14 +++++++ .../diagnostics/tests/override/kt12467.txt | 37 +++++++++++++++++++ .../checkers/DiagnosticsTestGenerated.java | 24 ++++++++++++ .../kotlin/resolve/OverridingUtil.java | 35 +++++++++++++++++- 12 files changed, 263 insertions(+), 5 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/override/FakeOverrideModality1.kt create mode 100644 compiler/testData/diagnostics/tests/override/FakeOverrideModality1.txt create mode 100644 compiler/testData/diagnostics/tests/override/FakeOverrideModality2.kt create mode 100644 compiler/testData/diagnostics/tests/override/FakeOverrideModality2.txt create mode 100644 compiler/testData/diagnostics/tests/override/FakeOverrideModality3.kt create mode 100644 compiler/testData/diagnostics/tests/override/FakeOverrideModality3.txt create mode 100644 compiler/testData/diagnostics/tests/override/kt12467.kt create mode 100644 compiler/testData/diagnostics/tests/override/kt12467.txt diff --git a/compiler/testData/diagnostics/tests/override/FakeOverrideModality1.kt b/compiler/testData/diagnostics/tests/override/FakeOverrideModality1.kt new file mode 100644 index 00000000000..294e0f4971e --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/FakeOverrideModality1.kt @@ -0,0 +1,14 @@ +interface A { + fun foo() {} +} + +interface B : A { + abstract override fun foo() +} + +interface C { + abstract fun foo() +} + +// Fake override Z#foo should be abstract +class Z : B, C diff --git a/compiler/testData/diagnostics/tests/override/FakeOverrideModality1.txt b/compiler/testData/diagnostics/tests/override/FakeOverrideModality1.txt new file mode 100644 index 00000000000..6cdbb62af39 --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/FakeOverrideModality1.txt @@ -0,0 +1,30 @@ +package + +public interface A { + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open 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 interface B : A { + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public abstract 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 interface C { + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public abstract 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 Z : B, C { + public constructor Z() + public open override /*2*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public abstract override /*2*/ /*fake_override*/ fun foo(): kotlin.Unit + public open override /*2*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/diagnostics/tests/override/FakeOverrideModality2.kt b/compiler/testData/diagnostics/tests/override/FakeOverrideModality2.kt new file mode 100644 index 00000000000..77d7303a742 --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/FakeOverrideModality2.kt @@ -0,0 +1,18 @@ +interface A { + fun foo() {} +} + +interface B : A { + abstract override fun foo() +} + +interface C : A { + abstract override fun foo() +} + +interface D : A { + override fun foo() = super.foo() +} + +// Fake override Z#foo should be open +class Z : B, C, D diff --git a/compiler/testData/diagnostics/tests/override/FakeOverrideModality2.txt b/compiler/testData/diagnostics/tests/override/FakeOverrideModality2.txt new file mode 100644 index 00000000000..c3d749325e0 --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/FakeOverrideModality2.txt @@ -0,0 +1,37 @@ +package + +public interface A { + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open 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 interface B : A { + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public abstract 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 interface C : A { + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public abstract 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 interface D : A { + 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 Z : B, C, D { + public constructor Z() + public open override /*3*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open override /*3*/ /*fake_override*/ fun foo(): kotlin.Unit + public open override /*3*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*3*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/diagnostics/tests/override/FakeOverrideModality3.kt b/compiler/testData/diagnostics/tests/override/FakeOverrideModality3.kt new file mode 100644 index 00000000000..485981df58d --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/FakeOverrideModality3.kt @@ -0,0 +1,16 @@ +interface A { + fun foo() {} +} + +interface B : A { + abstract override fun foo() +} + +interface C : A { + abstract override fun foo() +} + +interface D : A + +// Fake override Z#foo should be abstract +class Z : B, C, D diff --git a/compiler/testData/diagnostics/tests/override/FakeOverrideModality3.txt b/compiler/testData/diagnostics/tests/override/FakeOverrideModality3.txt new file mode 100644 index 00000000000..d244b2f4dc1 --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/FakeOverrideModality3.txt @@ -0,0 +1,37 @@ +package + +public interface A { + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open 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 interface B : A { + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public abstract 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 interface C : A { + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public abstract 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 interface D : A { + public open override /*1*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public open 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 Z : B, C, D { + public constructor Z() + public open override /*3*/ /*fake_override*/ fun equals(/*0*/ other: kotlin.Any?): kotlin.Boolean + public abstract override /*3*/ /*fake_override*/ fun foo(): kotlin.Unit + public open override /*3*/ /*fake_override*/ fun hashCode(): kotlin.Int + public open override /*3*/ /*fake_override*/ fun toString(): kotlin.String +} diff --git a/compiler/testData/diagnostics/tests/override/kt12358.kt b/compiler/testData/diagnostics/tests/override/kt12358.kt index 8cd9370c5f1..3b815e8408f 100644 --- a/compiler/testData/diagnostics/tests/override/kt12358.kt +++ b/compiler/testData/diagnostics/tests/override/kt12358.kt @@ -6,4 +6,4 @@ interface B abstract class C : A(), B -class Test : C() +class Test : C() diff --git a/compiler/testData/diagnostics/tests/override/kt12358.txt b/compiler/testData/diagnostics/tests/override/kt12358.txt index 9f617cafbf3..52bc687c530 100644 --- a/compiler/testData/diagnostics/tests/override/kt12358.txt +++ b/compiler/testData/diagnostics/tests/override/kt12358.txt @@ -17,12 +17,12 @@ public abstract class C : A, B { public constructor C() 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*/ /*fake_override*/ fun toString(): kotlin.String + public abstract override /*2*/ /*fake_override*/ fun toString(): kotlin.String } public final class Test : C { public constructor Test() 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 toString(): kotlin.String + public abstract override /*1*/ /*fake_override*/ fun toString(): kotlin.String } diff --git a/compiler/testData/diagnostics/tests/override/kt12467.kt b/compiler/testData/diagnostics/tests/override/kt12467.kt new file mode 100644 index 00000000000..0e1e026b3d2 --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/kt12467.kt @@ -0,0 +1,14 @@ +interface A { + fun test() { + } +} + +interface B : A { + override fun test() +} + +interface C : A + +interface D : C, B + +class K : D diff --git a/compiler/testData/diagnostics/tests/override/kt12467.txt b/compiler/testData/diagnostics/tests/override/kt12467.txt new file mode 100644 index 00000000000..a4d3eeed6e4 --- /dev/null +++ b/compiler/testData/diagnostics/tests/override/kt12467.txt @@ -0,0 +1,37 @@ +package + +public interface A { + 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.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public interface B : A { + 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 abstract override /*1*/ fun test(): kotlin.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public interface C : A { + 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.Unit + public open override /*1*/ /*fake_override*/ fun toString(): kotlin.String +} + +public interface D : C, B { + 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 abstract override /*2*/ /*fake_override*/ fun test(): kotlin.Unit + public open override /*2*/ /*fake_override*/ fun toString(): kotlin.String +} + +public final class K : D { + public constructor K() + 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 abstract override /*1*/ /*fake_override*/ fun test(): kotlin.Unit + 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 44930ab48fb..28b45bf4d52 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -12807,6 +12807,24 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } + @TestMetadata("FakeOverrideModality1.kt") + public void testFakeOverrideModality1() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/override/FakeOverrideModality1.kt"); + doTest(fileName); + } + + @TestMetadata("FakeOverrideModality2.kt") + public void testFakeOverrideModality2() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/override/FakeOverrideModality2.kt"); + doTest(fileName); + } + + @TestMetadata("FakeOverrideModality3.kt") + public void testFakeOverrideModality3() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/override/FakeOverrideModality3.kt"); + doTest(fileName); + } + @TestMetadata("Generics.kt") public void testGenerics() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/override/Generics.kt"); @@ -12825,6 +12843,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } + @TestMetadata("kt12467.kt") + public void testKt12467() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/override/kt12467.kt"); + doTest(fileName); + } + @TestMetadata("kt12482.kt") public void testKt12482() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/override/kt12482.kt"); diff --git a/core/descriptors/src/org/jetbrains/kotlin/resolve/OverridingUtil.java b/core/descriptors/src/org/jetbrains/kotlin/resolve/OverridingUtil.java index ca6cea6e1bb..c96d2e20cbc 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/resolve/OverridingUtil.java +++ b/core/descriptors/src/org/jetbrains/kotlin/resolve/OverridingUtil.java @@ -612,6 +612,9 @@ public class OverridingUtil { boolean allInvisible = visibleOverridables.isEmpty(); Collection effectiveOverridden = allInvisible ? overridables : visibleOverridables; + Modality modality = determineModality(effectiveOverridden); + Visibility visibility = allInvisible ? Visibilities.INVISIBLE_FAKE : Visibilities.INHERITED; + // FIXME doesn't work as expected for flexible types: should create a refined signature. // Current algorithm produces bad results in presence of annotated Java signatures such as: // J: foo(s: String!): String -- @NotNull String foo(String s); @@ -619,8 +622,6 @@ public class OverridingUtil { // --> 'foo(s: String!): String' as an inherited signature with most specific return type. // This is bad because it can be overridden by 'foo(s: String?): String', which is not override-equivalent with K::foo above. // Should be 'foo(s: String): String'. - Modality modality = getMinimalModality(effectiveOverridden); - Visibility visibility = allInvisible ? Visibilities.INVISIBLE_FAKE : Visibilities.INHERITED; CallableMemberDescriptor mostSpecific = selectMostSpecificMember(effectiveOverridden, new Function1() { @@ -637,6 +638,36 @@ public class OverridingUtil { strategy.addFakeOverride(fakeOverride); } + @NotNull + private static Modality determineModality(@NotNull Collection descriptors) { + // Optimization: avoid creating hash sets in frequent cases when modality can be computed trivially + boolean hasOpen = false; + boolean hasAbstract = false; + for (CallableMemberDescriptor descriptor : descriptors) { + switch (descriptor.getModality()) { + case FINAL: + return Modality.FINAL; + case SEALED: + throw new IllegalStateException("Member cannot have SEALED modality: " + descriptor); + case OPEN: + hasOpen = true; + break; + case ABSTRACT: + hasAbstract = true; + break; + } + } + + 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)); + } + @NotNull private static Modality getMinimalModality(@NotNull Collection descriptors) { Modality modality = Modality.ABSTRACT;