From e58711cde31e0664180b0ff187fed042e7419d39 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Thu, 5 Dec 2019 14:27:47 +0300 Subject: [PATCH] FIR: Select most specific members from supertypes scope --- .../scopes/impl/AbstractFirOverrideScope.kt | 2 +- .../scopes/impl/FirAbstractOverrideChecker.kt | 22 ++- .../fir/scopes/impl/FirSuperTypeScope.kt | 183 ++++++++++++++++-- .../resolve/overrides/simpleMostSpecific.kt | 41 ++++ .../resolve/overrides/simpleMostSpecific.txt | 86 ++++++++ .../fir/FirDiagnosticsTestGenerated.java | 5 + .../ir/irText/expressions/kt30020.fir.txt | 6 +- 7 files changed, 315 insertions(+), 30 deletions(-) create mode 100644 compiler/fir/resolve/testData/resolve/overrides/simpleMostSpecific.kt create mode 100644 compiler/fir/resolve/testData/resolve/overrides/simpleMostSpecific.txt diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/impl/AbstractFirOverrideScope.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/impl/AbstractFirOverrideScope.kt index 54087d7dff4..8de27c0196c 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/impl/AbstractFirOverrideScope.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/impl/AbstractFirOverrideScope.kt @@ -25,7 +25,7 @@ abstract class AbstractFirOverrideScope(val session: FirSession, protected val o return overrideChecker.isOverriddenProperty(overrideCandidate, baseDeclaration) } - private fun similarFunctionsOrBothProperties( + protected fun similarFunctionsOrBothProperties( overrideCandidate: FirCallableMemberDeclaration<*>, baseDeclaration: FirCallableMemberDeclaration<*> ): Boolean { diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/impl/FirAbstractOverrideChecker.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/impl/FirAbstractOverrideChecker.kt index c79e27633a9..1b6005e9545 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/impl/FirAbstractOverrideChecker.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/impl/FirAbstractOverrideChecker.kt @@ -5,6 +5,7 @@ package org.jetbrains.kotlin.fir.scopes.impl +import org.jetbrains.kotlin.fir.declarations.FirCallableMemberDeclaration import org.jetbrains.kotlin.fir.declarations.FirSimpleFunction import org.jetbrains.kotlin.fir.resolve.substitution.ConeSubstitutor import org.jetbrains.kotlin.fir.resolve.substitution.substitutorByMap @@ -20,12 +21,7 @@ abstract class FirAbstractOverrideChecker : FirOverrideChecker { overrideCandidate: FirSimpleFunction, baseDeclaration: FirSimpleFunction ): ConeSubstitutor? { - if (overrideCandidate.typeParameters.size != baseDeclaration.typeParameters.size) return null - - val types = baseDeclaration.typeParameters.map { - ConeTypeParameterTypeImpl(it.symbol.toLookupTag(), false) - } - val substitutor = substitutorByMap(overrideCandidate.typeParameters.map { it.symbol }.zip(types).toMap()) + val substitutor = buildSubstitutorForOverridesCheck(overrideCandidate, baseDeclaration) ?: return null if (!overrideCandidate.typeParameters.zip(baseDeclaration.typeParameters).all { (a, b) -> a.bounds.size == b.bounds.size && a.bounds.zip(b.bounds).all { (aBound, bBound) -> isEqualTypes(aBound, bBound, substitutor) @@ -34,4 +30,16 @@ abstract class FirAbstractOverrideChecker : FirOverrideChecker { ) return null return substitutor } -} \ No newline at end of file +} + +fun buildSubstitutorForOverridesCheck( + overrideCandidate: FirCallableMemberDeclaration<*>, + baseDeclaration: FirCallableMemberDeclaration<*> +): ConeSubstitutor? { + if (overrideCandidate.typeParameters.size != baseDeclaration.typeParameters.size) return null + + val types = baseDeclaration.typeParameters.map { + ConeTypeParameterTypeImpl(it.symbol.toLookupTag(), false) + } + return substitutorByMap(overrideCandidate.typeParameters.map { it.symbol }.zip(types).toMap()) +} diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/impl/FirSuperTypeScope.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/impl/FirSuperTypeScope.kt index eb9b6f614c1..af2f6e2c241 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/impl/FirSuperTypeScope.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/scopes/impl/FirSuperTypeScope.kt @@ -5,14 +5,28 @@ package org.jetbrains.kotlin.fir.scopes.impl +import org.jetbrains.kotlin.descriptors.Visibilities import org.jetbrains.kotlin.fir.FirSession +import org.jetbrains.kotlin.fir.declarations.FirCallableMemberDeclaration +import org.jetbrains.kotlin.fir.declarations.FirProperty +import org.jetbrains.kotlin.fir.declarations.FirSimpleFunction import org.jetbrains.kotlin.fir.scopes.FirOverrideChecker import org.jetbrains.kotlin.fir.scopes.FirScope import org.jetbrains.kotlin.fir.scopes.ProcessorAction import org.jetbrains.kotlin.fir.symbols.impl.FirCallableSymbol import org.jetbrains.kotlin.fir.symbols.impl.FirClassifierSymbol +import org.jetbrains.kotlin.fir.symbols.impl.FirConstructorSymbol import org.jetbrains.kotlin.fir.symbols.impl.FirFunctionSymbol +import org.jetbrains.kotlin.fir.types.ConeFlexibleType +import org.jetbrains.kotlin.fir.types.ConeKotlinType +import org.jetbrains.kotlin.fir.types.ConeTypeCheckerContext +import org.jetbrains.kotlin.fir.types.coneTypeSafe import org.jetbrains.kotlin.name.Name +import org.jetbrains.kotlin.types.AbstractTypeChecker +import org.jetbrains.kotlin.types.AbstractTypeCheckerContext +import org.jetbrains.kotlin.utils.addToStdlib.flattenTo +import java.util.* +import kotlin.collections.HashSet class FirSuperTypeScope private constructor( session: FirSession, @@ -26,6 +40,8 @@ class FirSuperTypeScope private constructor( private val absentClassifiers = mutableSetOf() + private val typeContext = ConeTypeCheckerContext(isErrorTypeEqualsToAnything = false, isStubTypeEqualsToAnything = false, session) + override fun processFunctionsByName(name: Name, processor: (FirFunctionSymbol<*>) -> ProcessorAction): ProcessorAction { processCallablesByName(name, processor, absentFunctions, FirScope::processFunctionsByName)?.let { return it } return super.processFunctionsByName(name, processor) @@ -46,32 +62,161 @@ class FirSuperTypeScope private constructor( return ProcessorAction.NEXT } - val accepted = HashSet>() - val pending = mutableListOf>() - var empty = true - for (scope in scopes) { - if (scope.processCallables(name) { - empty = false - if (it !in accepted && it.getOverridden(accepted) == null) { - pending += it - processor(it) - } else { - ProcessorAction.NEXT - } - }.stop() - ) { - return ProcessorAction.STOP + val membersByScope = scopes.mapNotNull { scope -> + val resultForScope = mutableListOf() + scope.processCallables(name) { + if (it !is FirConstructorSymbol) { + resultForScope.add(it) + } + ProcessorAction.NEXT } - accepted += pending - pending.clear() + + resultForScope.takeIf { it.isNotEmpty() } } - if (empty) { - absentNames += name + + if (membersByScope.isEmpty()) { + absentNames.add(name) + return null + } + + membersByScope.singleOrNull()?.let { members -> + for (member in members) { + if (processor(member).stop()) return ProcessorAction.STOP + } + + return ProcessorAction.NEXT + } + + val allMembers = membersByScope.flattenTo(LinkedList()) + + while (allMembers.isNotEmpty()) { + val maxByVisibility = findMemberWithMaxVisibility(allMembers) + val extractedOverrides = extractBothWaysOverridable(maxByVisibility, allMembers) + + val mostSpecific = selectMostSpecificMember(extractedOverrides) + + if (processor(mostSpecific).stop()) return ProcessorAction.STOP } return null } + private fun > selectMostSpecificMember( + overridables: Collection + ): D { + require(overridables.isNotEmpty()) { "Should have at least one overridable descriptor" } + if (overridables.size == 1) { + return overridables.first() + } + + val candidates: MutableCollection = ArrayList(2) + var transitivelyMostSpecific: D = overridables.first() + + for (candidate in overridables) { + if (overridables.all { isMoreSpecific(candidate, it) }) { + candidates.add(candidate) + } + + if (isMoreSpecific(candidate, transitivelyMostSpecific) && !isMoreSpecific(transitivelyMostSpecific, candidate)) { + transitivelyMostSpecific = candidate + } + } + + when { + candidates.isEmpty() -> { + return transitivelyMostSpecific + } + candidates.size == 1 -> { + return candidates.first() + } + else -> { + candidates.firstOrNull { + val type = it.fir.returnTypeRef.coneTypeSafe() + type != null && type !is ConeFlexibleType + }?.let { return it } + return candidates.first() + } + } + } + + private fun isMoreSpecific( + a: FirCallableSymbol<*>, + b: FirCallableSymbol<*> + ): Boolean { + val aFir = a.fir + val bFir = b.fir + if (aFir !is FirCallableMemberDeclaration<*> || bFir !is FirCallableMemberDeclaration<*>) return false + + val substitutor = buildSubstitutorForOverridesCheck(aFir, bFir) ?: return false + + val aReturnType = a.fir.returnTypeRef.coneTypeSafe()?.let(substitutor::substituteOrSelf) ?: return false + val bReturnType = b.fir.returnTypeRef.coneTypeSafe() ?: return false + + if (aFir is FirSimpleFunction) { + require(bFir is FirSimpleFunction) { "b is " + b.javaClass } + return isTypeMoreSpecific(aReturnType, bReturnType) + } + if (aFir is FirProperty) { + require(bFir is FirProperty) { "b is " + b.javaClass } + // TODO: if (!OverridingUtil.isAccessorMoreSpecific(pa.getSetter(), pb.getSetter())) return false + return if (aFir.isVar && bFir.isVar) { + AbstractTypeChecker.equalTypes(typeContext as AbstractTypeCheckerContext, aReturnType, bReturnType) + } else { // both vals or var vs val: val can't be more specific then var + !(!aFir.isVar && bFir.isVar) && isTypeMoreSpecific(aReturnType, bReturnType) + } + } + throw IllegalArgumentException("Unexpected callable: " + a.javaClass) + } + + private fun isTypeMoreSpecific(a: ConeKotlinType, b: ConeKotlinType): Boolean = + AbstractTypeChecker.isSubtypeOf(typeContext as AbstractTypeCheckerContext, a, b) + + private fun > findMemberWithMaxVisibility(members: Collection): D { + assert(members.isNotEmpty()) + + var member: D? = null + for (candidate in members) { + if (member == null) { + member = candidate + continue + } + + val result = Visibilities.compare( + (member.fir as FirCallableMemberDeclaration<*>).status.visibility, + (candidate.fir as FirCallableMemberDeclaration<*>).status.visibility + ) + if (result != null && result < 0) { + member = candidate + } + } + return member!! + } + + private fun > extractBothWaysOverridable( + overrider: D, + members: MutableCollection + ): Collection { + val result = mutableListOf().apply { add(overrider) } + + val iterator = members.iterator() + + val overrideCandidate = overrider.fir as FirCallableMemberDeclaration<*> + while (iterator.hasNext()) { + val next = iterator.next() + if (next == overrider) { + iterator.remove() + continue + } + + if (similarFunctionsOrBothProperties(overrideCandidate, next.fir as FirCallableMemberDeclaration<*>)) { + result.add(next) + iterator.remove() + } + } + + return result + } + override fun processClassifiersByName(name: Name, processor: (FirClassifierSymbol<*>) -> ProcessorAction): ProcessorAction { if (name in absentClassifiers) { return ProcessorAction.NEXT diff --git a/compiler/fir/resolve/testData/resolve/overrides/simpleMostSpecific.kt b/compiler/fir/resolve/testData/resolve/overrides/simpleMostSpecific.kt new file mode 100644 index 00000000000..11edfd863f2 --- /dev/null +++ b/compiler/fir/resolve/testData/resolve/overrides/simpleMostSpecific.kt @@ -0,0 +1,41 @@ +// !CHECK_TYPE +interface A { + val x: CharSequence + + fun foo(): String? + + fun bar(x: T, y: F): F? +} + +interface B { + var x: String + + fun foo(): String + + fun bar(x: X, y: Y): Y +} + +interface C { + val x: String +} + +interface D1 : A, B +interface D2 : B, A +interface D3 : A, C + +fun main(d1: D1, d2: D2, d3: D3) { + d1.x.checkType { _() } + d1.x = "" + d1.foo().checkType { _() } + d1.bar(1, "").checkType { _() } + + d2.x.checkType { _() } + d2.x = "" + d2.foo().checkType { _() } + d2.bar(1, "").checkType { _() } + + d3.x.checkType { _() } + d3.x = "" + d3.foo().checkType { _() } + d3.bar(1, "").checkType { _() } +} diff --git a/compiler/fir/resolve/testData/resolve/overrides/simpleMostSpecific.txt b/compiler/fir/resolve/testData/resolve/overrides/simpleMostSpecific.txt new file mode 100644 index 00000000000..977db279c78 --- /dev/null +++ b/compiler/fir/resolve/testData/resolve/overrides/simpleMostSpecific.txt @@ -0,0 +1,86 @@ +FILE: simpleMostSpecific.kt + public abstract interface A : R|kotlin/Any| { + public abstract val x: R|kotlin/CharSequence| + public get(): R|kotlin/CharSequence| + + public abstract fun foo(): R|kotlin/String?| + + public abstract fun bar(x: R|T|, y: R|F|): R|F?| + + } + public abstract interface B : R|kotlin/Any| { + public abstract var x: R|kotlin/String| + public get(): R|kotlin/String| + public set(value: R|kotlin/String|): R|kotlin/Unit| + + public abstract fun foo(): R|kotlin/String| + + public abstract fun bar(x: R|X|, y: R|Y|): R|Y| + + } + public abstract interface C : R|kotlin/Any| { + public abstract val x: R|kotlin/String| + public get(): R|kotlin/String| + + } + public abstract interface D1 : R|A|, R|B| { + } + public abstract interface D2 : R|B|, R|A| { + } + public abstract interface D3 : R|A|, R|C| { + } + public final fun main(d1: R|D1|, d2: R|D2|, d3: R|D3|): R|kotlin/Unit| { + R|/d1|.R|/B.x|.R|tests/_checkType/checkType|( = checkType@fun R|tests/_checkType/Inv|.(): R|kotlin/Unit| { + this@R|special/anonymous|.R|tests/_checkType/_|() + } + ) + R|/d1|.R|/B.x| = String() + R|/d1|.R|/B.foo|().R|tests/_checkType/checkType|( = checkType@fun R|tests/_checkType/Inv|.(): R|kotlin/Unit| { + this@R|special/anonymous|.R|tests/_checkType/_|() + } + ) + R|/d1|.R|/B.bar|(Int(1), String()).R|tests/_checkType/checkType|( = checkType@fun R|tests/_checkType/Inv|.(): R|kotlin/Unit| { + this@R|special/anonymous|.R|tests/_checkType/_|() + } + ) + R|/d2|.R|/B.x|.R|tests/_checkType/checkType|( = checkType@fun R|tests/_checkType/Inv|.(): R|kotlin/Unit| { + this@R|special/anonymous|.R|tests/_checkType/_|() + } + ) + R|/d2|.R|/B.x| = String() + R|/d2|.R|/B.foo|().R|tests/_checkType/checkType|( = checkType@fun R|tests/_checkType/Inv|.(): R|kotlin/Unit| { + this@R|special/anonymous|.R|tests/_checkType/_|() + } + ) + R|/d2|.R|/B.bar|(Int(1), String()).R|tests/_checkType/checkType|( = checkType@fun R|tests/_checkType/Inv|.(): R|kotlin/Unit| { + this@R|special/anonymous|.R|tests/_checkType/_|() + } + ) + R|/d3|.R|/C.x|.R|tests/_checkType/checkType|( = checkType@fun R|tests/_checkType/Inv|.(): R|kotlin/Unit| { + this@R|special/anonymous|.R|tests/_checkType/_|() + } + ) + R|/d3|.R|/C.x| = String() + R|/d3|.R|/A.foo|().R|tests/_checkType/checkType|( = checkType@fun R|tests/_checkType/Inv|.(): R|kotlin/Unit| { + this@R|special/anonymous|.R|tests/_checkType/_|() + } + ) + R|/d3|.R|/A.bar|(Int(1), String()).R|tests/_checkType/checkType|( = checkType@fun R|tests/_checkType/Inv|.(): R|kotlin/Unit| { + this@R|special/anonymous|.R|tests/_checkType/_|() + } + ) + } +FILE: CHECK_TYPE.kt + public final fun checkSubtype(t: R|T|): R|T| { + ^checkSubtype R|/t| + } + public final class Inv : R|kotlin/Any| { + public constructor(): R|tests/_checkType/Inv| { + super() + } + + } + public final fun R|tests/_checkType/Inv|._(): R|kotlin/Unit| { + } + public final infix fun R|T|.checkType(f: R|tests/_checkType/Inv.() -> kotlin/Unit|): R|kotlin/Unit| { + } diff --git a/compiler/fir/resolve/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java b/compiler/fir/resolve/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java index 92278a13e98..4081fe98d77 100644 --- a/compiler/fir/resolve/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java +++ b/compiler/fir/resolve/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java @@ -873,6 +873,11 @@ public class FirDiagnosticsTestGenerated extends AbstractFirDiagnosticsTest { runTest("compiler/fir/resolve/testData/resolve/overrides/simpleFakeOverride.kt"); } + @TestMetadata("simpleMostSpecific.kt") + public void testSimpleMostSpecific() throws Exception { + runTest("compiler/fir/resolve/testData/resolve/overrides/simpleMostSpecific.kt"); + } + @TestMetadata("supertypeGenerics.kt") public void testSupertypeGenerics() throws Exception { runTest("compiler/fir/resolve/testData/resolve/overrides/supertypeGenerics.kt"); diff --git a/compiler/testData/ir/irText/expressions/kt30020.fir.txt b/compiler/testData/ir/irText/expressions/kt30020.fir.txt index 775ae6a79b0..021c9130b67 100644 --- a/compiler/testData/ir/irText/expressions/kt30020.fir.txt +++ b/compiler/testData/ir/irText/expressions/kt30020.fir.txt @@ -201,10 +201,10 @@ FILE fqName: fileName:/kt30020.kt overridden: public abstract fun isEmpty (): kotlin.Boolean declared in kotlin.collections.List $this: VALUE_PARAMETER name: type:kotlin.collections.List - FUN FAKE_OVERRIDE name:iterator visibility:public modality:ABSTRACT <> ($this:kotlin.collections.List) returnType:kotlin.collections.Iterator [fake_override,operator] + FUN FAKE_OVERRIDE name:iterator visibility:public modality:ABSTRACT <> ($this:kotlin.collections.MutableCollection) returnType:kotlin.collections.MutableIterator [fake_override,operator] overridden: - public abstract fun iterator (): kotlin.collections.Iterator [operator] declared in kotlin.collections.List - $this: VALUE_PARAMETER name: type:kotlin.collections.List + public abstract fun iterator (): kotlin.collections.MutableIterator [operator] declared in kotlin.collections.MutableCollection + $this: VALUE_PARAMETER name: type:kotlin.collections.MutableCollection FUN FAKE_OVERRIDE name:lastIndexOf visibility:public modality:ABSTRACT <> ($this:kotlin.collections.List, element:kotlin.Int) returnType:kotlin.Int [fake_override] overridden: public abstract fun lastIndexOf (element: E of kotlin.collections.MutableList): kotlin.Int declared in kotlin.collections.List