From f5989aa4f220b307db1449d571cb6bb752e0e846 Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Fri, 11 Dec 2015 19:17:15 +0300 Subject: [PATCH] CallableMemberDescriptor.isOverridable / ClassDescriptor.isFinal refactoring Extra JvmField and integer constants tests --- .../cfg/ControlFlowInformationProvider.java | 5 +++-- .../kotlin/resolve/BodyResolver.java | 7 ++++--- .../kotlin/resolve/DeclarationsChecker.kt | 5 +---- .../smartcasts/DataFlowValueFactory.java | 16 +-------------- .../evaluate/ConstantExpressionEvaluator.kt | 3 ++- .../diagnostics/tests/AbstractInClass.kt | 4 ++-- .../jvmField/jvmFieldApplicability.kt | 18 +++++++++++++++++ .../jvmField/jvmFieldApplicability.txt | 20 +++++++++++++++++++ .../jetbrains/kotlin/descriptors/Modality.kt | 10 +++++++++- idea/testData/checker/Abstract.kt | 4 ++-- .../js/translate/utils/JsDescriptorUtils.java | 2 +- .../js/translate/utils/ManglingUtils.java | 2 +- 12 files changed, 64 insertions(+), 32 deletions(-) diff --git a/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.java b/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.java index b421971d110..3cacd7166c9 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/cfg/ControlFlowInformationProvider.java @@ -611,9 +611,10 @@ public class ControlFlowInformationProvider { assert descriptor instanceof FunctionDescriptor : owner.getText(); FunctionDescriptor functionDescriptor = (FunctionDescriptor) descriptor; String functionName = functionDescriptor.getName().asString(); + KtFunction function = (KtFunction) owner; if (isMain - || ModalityKt.isOverridable(functionDescriptor) - || !functionDescriptor.getOverriddenDescriptors().isEmpty() + || ModalityKt.isOverridableOrOverrides(functionDescriptor) + || function.hasModifier(KtTokens.OVERRIDE_KEYWORD) || "getValue".equals(functionName) || "setValue".equals(functionName) || "propertyDelegated".equals(functionName) ) { diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/BodyResolver.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/BodyResolver.java index 2e63b8c50b2..9a06f3accf7 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/BodyResolver.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/BodyResolver.java @@ -483,13 +483,14 @@ public class BodyResolver { trace.report(SUPERTYPE_APPEARS_TWICE.on(typeReference)); } - if (classDescriptor != null && classDescriptor.getKind().isSingleton()) { + if (classDescriptor == null) return; + if (classDescriptor.getKind().isSingleton()) { if (!DescriptorUtils.isEnumEntry(classDescriptor)) { trace.report(SINGLETON_IN_SUPERTYPE.on(typeReference)); } } else if (!allowedFinalSupertypes.contains(constructor)) { - if (classDescriptor != null && classDescriptor.getModality() == Modality.SEALED) { + if (classDescriptor.getModality() == Modality.SEALED) { DeclarationDescriptor containingDescriptor = supertypeOwner.getContainingDeclaration(); while (containingDescriptor != null && containingDescriptor != classDescriptor) { containingDescriptor = containingDescriptor.getContainingDeclaration(); @@ -501,7 +502,7 @@ public class BodyResolver { trace.report(SEALED_SUPERTYPE_IN_LOCAL_CLASS.on(typeReference)); } } - else if (constructor.isFinal()) { + else if (ModalityKt.isFinalOrEnum(classDescriptor)) { trace.report(FINAL_SUPERTYPE.on(typeReference)); } } diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DeclarationsChecker.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DeclarationsChecker.kt index 219f68b7111..989c98c775c 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/DeclarationsChecker.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/DeclarationsChecker.kt @@ -712,10 +712,7 @@ class DeclarationsChecker( reportVisibilityModifierDiagnostics(tokens.values, Errors.PRIVATE_SETTER_FOR_ABSTRACT_PROPERTY) } else { - val parentDescriptor = propertyDescriptor.containingDeclaration - if (parentDescriptor !is ClassDescriptor || !parentDescriptor.isFinal) { - reportVisibilityModifierDiagnostics(tokens.values, Errors.PRIVATE_SETTER_FOR_OPEN_PROPERTY) - } + reportVisibilityModifierDiagnostics(tokens.values, Errors.PRIVATE_SETTER_FOR_OPEN_PROPERTY) } } else if (propertyDescriptor.isLateInit && accessorDescriptor.visibility != propertyDescriptor.visibility) { diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DataFlowValueFactory.java b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DataFlowValueFactory.java index ce007f0c897..dab09829279 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DataFlowValueFactory.java +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/calls/smartcasts/DataFlowValueFactory.java @@ -395,7 +395,7 @@ public class DataFlowValueFactory { private static Kind propertyKind(@NotNull PropertyDescriptor propertyDescriptor, @Nullable ModuleDescriptor usageModule) { if (propertyDescriptor.isVar()) return MUTABLE_PROPERTY; - if (!isFinal(propertyDescriptor)) return PROPERTY_WITH_GETTER; + if (ModalityKt.isOverridable(propertyDescriptor)) return PROPERTY_WITH_GETTER; if (!hasDefaultGetter(propertyDescriptor)) return PROPERTY_WITH_GETTER; if (!invisibleFromOtherModules(propertyDescriptor)) { ModuleDescriptor declarationModule = DescriptorUtils.getContainingModule(propertyDescriptor); @@ -464,20 +464,6 @@ public class DataFlowValueFactory { return true; } - private static boolean isFinal(PropertyDescriptor propertyDescriptor) { - DeclarationDescriptor containingDeclaration = propertyDescriptor.getContainingDeclaration(); - if (containingDeclaration instanceof ClassDescriptor) { - ClassDescriptor classDescriptor = (ClassDescriptor) containingDeclaration; - if (!ModalityKt.isFinal(classDescriptor) && ModalityKt.isOverridable(propertyDescriptor)) return false; - } - else { - if (ModalityKt.isOverridable(propertyDescriptor)) { - throw new IllegalStateException("Property outside a class must not be overridable: " + propertyDescriptor.getName()); - } - } - return true; - } - private static boolean invisibleFromOtherModules(@NotNull DeclarationDescriptorWithVisibility descriptor) { if (Visibilities.INVISIBLE_FROM_OTHER_MODULES.contains(descriptor.getVisibility())) return true; diff --git a/compiler/frontend/src/org/jetbrains/kotlin/resolve/constants/evaluate/ConstantExpressionEvaluator.kt b/compiler/frontend/src/org/jetbrains/kotlin/resolve/constants/evaluate/ConstantExpressionEvaluator.kt index 53200faae24..6f7c95e6bf6 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/resolve/constants/evaluate/ConstantExpressionEvaluator.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/resolve/constants/evaluate/ConstantExpressionEvaluator.kt @@ -531,7 +531,8 @@ private class ConstantExpressionEvaluatorVisitor( if (resolvedCall != null) { val callableDescriptor = resolvedCall.resultingDescriptor if (callableDescriptor is VariableDescriptor) { - if (callableDescriptor is PropertyDescriptor && callableDescriptor.isOverridable) return null + // TODO: FIXME: see KT-10425 + if (callableDescriptor is PropertyDescriptor && callableDescriptor.modality != Modality.FINAL) return null val variableInitializer = callableDescriptor.compileTimeInitializer ?: return null diff --git a/compiler/testData/diagnostics/tests/AbstractInClass.kt b/compiler/testData/diagnostics/tests/AbstractInClass.kt index d25be520867..0399873e4c1 100644 --- a/compiler/testData/diagnostics/tests/AbstractInClass.kt +++ b/compiler/testData/diagnostics/tests/AbstractInClass.kt @@ -9,8 +9,8 @@ class MyClass() { var b: Int private set var b1: Int = 0; private set - abstract var b2: Int private set - abstract var b3: Int = 0; private set + abstract var b2: Int private set + abstract var b3: Int = 0; private set var c: Int set(v: Int) { field = v } var c1: Int = 0; set(v: Int) { field = v } diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmField/jvmFieldApplicability.kt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmField/jvmFieldApplicability.kt index edf7042a852..1edd19c8e59 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmField/jvmFieldApplicability.kt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmField/jvmFieldApplicability.kt @@ -65,12 +65,30 @@ class H { } interface K { + + val i: Int + val j: Int + companion object { @JvmField var c = 3 } } +class KK : K { + @JvmField + override val i: Int = 0 + @JvmField + override final val j: Int = 0 +} + +open class KKK : K { + @JvmField + override val i: Int = 0 + @JvmField + override final val j: Int = 0 +} + object O { @JvmField val c = 3 diff --git a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmField/jvmFieldApplicability.txt b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmField/jvmFieldApplicability.txt index f8c4b318ba2..a493bc6c1ce 100644 --- a/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmField/jvmFieldApplicability.txt +++ b/compiler/testData/diagnostics/testsWithStdLib/annotations/jvmField/jvmFieldApplicability.txt @@ -54,6 +54,8 @@ public interface I { } public interface K { + public abstract val i: kotlin.Int + public abstract val j: kotlin.Int 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 @@ -67,6 +69,24 @@ public interface K { } } +public final class KK : K { + public constructor KK() + @kotlin.jvm.JvmField() public open override /*1*/ val i: kotlin.Int = 0 + @kotlin.jvm.JvmField() public final override /*1*/ val j: kotlin.Int = 0 + 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 open class KKK : K { + public constructor KKK() + @kotlin.jvm.JvmField() public open override /*1*/ val i: kotlin.Int = 0 + @kotlin.jvm.JvmField() public final override /*1*/ val j: kotlin.Int = 0 + 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 object O { private constructor O() @kotlin.jvm.JvmField() public final val c: kotlin.Int = 3 diff --git a/core/descriptors/src/org/jetbrains/kotlin/descriptors/Modality.kt b/core/descriptors/src/org/jetbrains/kotlin/descriptors/Modality.kt index adc7c9a13ab..fe0eff77140 100644 --- a/core/descriptors/src/org/jetbrains/kotlin/descriptors/Modality.kt +++ b/core/descriptors/src/org/jetbrains/kotlin/descriptors/Modality.kt @@ -16,6 +16,8 @@ package org.jetbrains.kotlin.descriptors +import org.jetbrains.kotlin.resolve.DescriptorUtils + // For sealed classes, isOverridable is false but isOverridableByMembers is true enum class Modality { // THE ORDER OF ENTRIES MATTERS HERE @@ -37,7 +39,13 @@ enum class Modality { } val CallableMemberDescriptor.isOverridable: Boolean - get() = modality != Modality.FINAL + get() = modality != Modality.FINAL && (containingDeclaration as? ClassDescriptor)?.isFinal != true + +val CallableMemberDescriptor.isOverridableOrOverrides: Boolean + get() = isOverridable || DescriptorUtils.isOverride(this) val ClassDescriptor.isFinal: Boolean + get() = modality == Modality.FINAL && kind != ClassKind.ENUM_CLASS + +val ClassDescriptor.isFinalOrEnum: Boolean get() = modality == Modality.FINAL diff --git a/idea/testData/checker/Abstract.kt b/idea/testData/checker/Abstract.kt index 2a3cd88b599..82692a9dd69 100644 --- a/idea/testData/checker/Abstract.kt +++ b/idea/testData/checker/Abstract.kt @@ -10,8 +10,8 @@ class MyClass() { var b: Int private set var b1: Int = 0; private set - abstract var b2: Int private set - abstract var b3: Int = 0; private set + abstract var b2: Int private set + abstract var b3: Int = 0; private set var c: Int set(v: Int) { field = v } var c1: Int = 0; set(v: Int) { field = v } diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/JsDescriptorUtils.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/JsDescriptorUtils.java index 448c6d4667f..74a1aadc569 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/JsDescriptorUtils.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/JsDescriptorUtils.java @@ -151,7 +151,7 @@ public final class JsDescriptorUtils { return !isExtension(propertyDescriptor) && isDefaultAccessor(propertyDescriptor.getGetter()) && isDefaultAccessor(propertyDescriptor.getSetter()) && - !ModalityKt.isOverridable(propertyDescriptor); + !ModalityKt.isOverridableOrOverrides(propertyDescriptor); } public static boolean isBuiltin(@NotNull DeclarationDescriptor descriptor) { diff --git a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/ManglingUtils.java b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/ManglingUtils.java index b90e7515074..e749fcec75e 100644 --- a/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/ManglingUtils.java +++ b/js/js.translator/src/org/jetbrains/kotlin/js/translate/utils/ManglingUtils.java @@ -86,7 +86,7 @@ public class ManglingUtils { ClassDescriptor classDescriptor = (ClassDescriptor) containingDeclaration; // Use stable mangling when it's inside an overridable declaration to avoid clashing names on inheritance. - if (!ModalityKt.isFinal(classDescriptor)) { + if (!ModalityKt.isFinalOrEnum(classDescriptor)) { return true; }