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
This commit is contained in:
Alexander Udalov
2016-07-11 20:19:53 +03:00
parent 84a83e5dc2
commit b72293883d
12 changed files with 263 additions and 5 deletions
@@ -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
<!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>class Z<!> : B, C
@@ -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
}
@@ -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
<!MANY_INTERFACES_MEMBER_NOT_IMPLEMENTED!>class Z<!> : B, C, D
@@ -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
}
@@ -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
<!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>class Z<!> : B, C, D
@@ -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
}
+1 -1
View File
@@ -6,4 +6,4 @@ interface B
abstract class C : A(), B
<!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>class Test<!> : C()
<!ABSTRACT_CLASS_MEMBER_NOT_IMPLEMENTED!>class Test<!> : C()
+2 -2
View File
@@ -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
}
+14
View File
@@ -0,0 +1,14 @@
interface A {
fun test() {
}
}
interface B : A {
override fun test()
}
interface C : A
interface D : C, B
<!ABSTRACT_MEMBER_NOT_IMPLEMENTED!>class K<!> : D
@@ -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
}
@@ -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");
@@ -612,6 +612,9 @@ public class OverridingUtil {
boolean allInvisible = visibleOverridables.isEmpty();
Collection<CallableMemberDescriptor> 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<CallableMemberDescriptor, CallableDescriptor>() {
@@ -637,6 +638,36 @@ public class OverridingUtil {
strategy.addFakeOverride(fakeOverride);
}
@NotNull
private static Modality determineModality(@NotNull Collection<CallableMemberDescriptor> 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<CallableMemberDescriptor> allOverriddenDeclarations = new HashSet<CallableMemberDescriptor>();
for (CallableMemberDescriptor descriptor : descriptors) {
allOverriddenDeclarations.addAll(getOverriddenDeclarations(descriptor));
}
return getMinimalModality(filterOutOverridden(allOverriddenDeclarations));
}
@NotNull
private static Modality getMinimalModality(@NotNull Collection<CallableMemberDescriptor> descriptors) {
Modality modality = Modality.ABSTRACT;