FIR: refactor FirOverrideService to get a list of most specific members

There may be several if neither of them returns a subtype of the other's
returned type.

compiler/fir/providers/src/org/jetbrains/kotlin/fir/scopes/FirOverrideService.kt

@@ -6,11 +6,9 @@
 package org.jetbrains.kotlin.fir.scopes
 
 import org.jetbrains.kotlin.descriptors.Visibilities
-import org.jetbrains.kotlin.descriptors.Visibility
 import org.jetbrains.kotlin.fir.FirSession
 import org.jetbrains.kotlin.fir.FirSessionComponent
 import org.jetbrains.kotlin.fir.declarations.FirProperty
-import org.jetbrains.kotlin.fir.declarations.FirPropertyAccessor
 import org.jetbrains.kotlin.fir.declarations.FirSimpleFunction
 import org.jetbrains.kotlin.fir.declarations.utils.visibility
 import org.jetbrains.kotlin.fir.resolve.transformers.ReturnTypeCalculator
@@ -19,10 +17,8 @@ import org.jetbrains.kotlin.fir.scopes.impl.similarFunctionsOrBothProperties
 import org.jetbrains.kotlin.fir.symbols.impl.FirCallableSymbol
 import org.jetbrains.kotlin.fir.types.ConeFlexibleType
 import org.jetbrains.kotlin.fir.types.ConeKotlinType
-import org.jetbrains.kotlin.fir.types.coneTypeSafe
 import org.jetbrains.kotlin.fir.types.typeContext
 import org.jetbrains.kotlin.types.AbstractTypeChecker
-import org.jetbrains.kotlin.types.TypeCheckerState
 import org.jetbrains.kotlin.utils.SmartSet
 import java.util.*
 
@@ -51,7 +47,7 @@ class FirOverrideService(val session: FirSession) : FirSessionComponent {
             val mostSpecific = selectMostSpecificMember(overridableGroup, returnTypeCalculator)
 
             overridableGroup.filterNotTo(conflictedHandles) {
-                isMoreSpecific(mostSpecific.member, it.member, returnTypeCalculator)
+                isMoreSpecificOrEqual(mostSpecific, it, returnTypeCalculator)
             }
 
             if (conflictedHandles.isNotEmpty()) {
@@ -104,92 +100,125 @@ class FirOverrideService(val session: FirSession) : FirSessionComponent {
         return result
     }
 
-    fun <D : FirCallableSymbol<*>> selectMostSpecificMember(
+    fun <D : FirCallableSymbol<*>> selectMostSpecificMembers(
-        overridables: Collection<MemberWithBaseScope<D>>,
+        overridables: List<MemberWithBaseScope<D>>,
         returnTypeCalculator: ReturnTypeCalculator
-    ): MemberWithBaseScope<D> {
+    ): List<MemberWithBaseScope<D>> {
         require(overridables.isNotEmpty()) { "Should have at least one overridable symbol" }
         if (overridables.size == 1) {
-            return overridables.first()
+            return overridables
         }
 
-        val candidates: MutableCollection<MemberWithBaseScope<D>> = ArrayList(2)
+        val maximums: MutableList<MemberWithBaseScopeAndReturnType<D>> = ArrayList(2)
-        var transitivelyMostSpecific: MemberWithBaseScope<D> = overridables.first()
+        skipCandidate@ for (candidate in overridables) {
-
+            val withReturnType = MemberWithBaseScopeAndReturnType(candidate, returnTypeCalculator)
-        for (candidate in overridables) {
+            // 1. Remove those members that are less specific than the current one;
-            if (overridables.all { isMoreSpecific(candidate.member, it.member, returnTypeCalculator) }) {
+            // 2. Add this member if none of the existing ones are more or equally specific.
-                candidates.add(candidate)
+            // The former, at least in theory, implies the latter, otherwise `compare` does not
+            // define a correct partial order (there are a and b such that a < candidate < b, but
+            // not a < b), so `skip = true` is equivalent to `continue`.
+            var skip = false
+            val toRemove = BooleanArray(maximums.size) { i ->
+                val c = maximums[i].compareTo(withReturnType) ?: return@BooleanArray false
+                if (c >= 0) {
+                    skip = true
+                }
+                c < 0
             }
-
+            maximums.removeFlagged(toRemove)
-            if (isMoreSpecific(candidate.member, transitivelyMostSpecific.member, returnTypeCalculator) &&
+            if (!skip) {
-                !isMoreSpecific(transitivelyMostSpecific.member, candidate.member, returnTypeCalculator)
+                maximums.add(withReturnType)
-            ) {
-                transitivelyMostSpecific = candidate
             }
         }
+        return maximums.map { it.memberWithBaseScope }
+    }
 
-        return when {
+    fun <D : FirCallableSymbol<*>> selectMostSpecificMember(
-            candidates.isEmpty() -> transitivelyMostSpecific
+        overridables: List<MemberWithBaseScope<D>>,
-            candidates.size == 1 -> candidates.first()
+        returnTypeCalculator: ReturnTypeCalculator
-            else -> {
+    ): MemberWithBaseScope<D> = selectMostSpecificMembers(overridables, returnTypeCalculator).first()
-                candidates.firstOrNull {
+
-                    val type = it.member.fir.returnTypeRef.coneTypeSafe<ConeKotlinType>()
+    private fun <E> MutableList<E>.removeFlagged(flags: BooleanArray) {
-                    type != null && type !is ConeFlexibleType
+        var dest = 0
-                }?.let { return it }
+        for (i in flags.indices) {
-                candidates.first()
+            if (!flags[i]) {
+                this[dest++] = this[i]
             }
         }
+        while (size > dest) {
+            removeLast()
+        }
     }
 
-    private fun isMoreSpecific(
+    private fun isMoreSpecificOrEqual(
-        a: FirCallableSymbol<*>,
+        a: MemberWithBaseScope<*>,
-        b: FirCallableSymbol<*>,
+        b: MemberWithBaseScope<*>,
         returnTypeCalculator: ReturnTypeCalculator
-    ): Boolean {
+    ) = MemberWithBaseScopeAndReturnType(a, returnTypeCalculator).compareTo(MemberWithBaseScopeAndReturnType(b, returnTypeCalculator)).let {
-        val aFir = a.fir
+        it != null && it >= 0
-        val bFir = b.fir
+    }
 
-        if (!isVisibilityMoreSpecific(aFir.visibility, bFir.visibility)) return false
+    private class MemberWithBaseScopeAndReturnType<out D : FirCallableSymbol<*>>(
+        val memberWithBaseScope: MemberWithBaseScope<D>,
+        returnTypeCalculator: ReturnTypeCalculator
+    ) {
+        val returnType: ConeKotlinType? = returnTypeCalculator.tryCalculateReturnTypeOrNull(memberWithBaseScope.member.fir)?.type
+    }
 
-        val substitutor = buildSubstitutorForOverridesCheck(aFir, bFir, session) ?: return false
+    private fun MemberWithBaseScopeAndReturnType<*>.compareTo(other: MemberWithBaseScopeAndReturnType<*>): Int? {
+        fun merge(preferA: Boolean, preferB: Boolean, previous: Int): Int? = when {
+            preferA == preferB -> previous
+            preferA && previous >= 0 -> 1
+            preferB && previous <= 0 -> -1
+            else -> null
+        }
+
+        val aFir = memberWithBaseScope.member.fir
+        val bFir = other.memberWithBaseScope.member.fir
+        val byVisibility = Visibilities.compare(aFir.visibility, bFir.visibility) ?: 0
+
+        val substitutor = buildSubstitutorForOverridesCheck(aFir, bFir, session) ?: return null
         // NB: these lines throw CCE in modularized tests when changed to just .coneType (FirImplicitTypeRef)
-        val aReturnType = returnTypeCalculator.tryCalculateReturnTypeOrNull(a.fir)?.type?.let(substitutor::substituteOrSelf) ?: return false
+        //  See also KT-41917 and the corresponding test (compiler/fir/analysis-tests/testData/resolveWithStdlib/delegates/kt41917.kt)
-        val bReturnType = returnTypeCalculator.tryCalculateReturnTypeOrNull(b.fir)?.type ?: return false
+        val aReturnType = returnType?.let(substitutor::substituteOrSelf) ?: return null
+        val bReturnType = other.returnType ?: return null
 
         val typeCheckerState = session.typeContext.newTypeCheckerState(
             errorTypesEqualToAnything = false,
             stubTypesEqualToAnything = false
         )
+        val aSubtypesB = AbstractTypeChecker.isSubtypeOf(typeCheckerState, aReturnType, bReturnType)
+        val bSubtypesA = AbstractTypeChecker.isSubtypeOf(typeCheckerState, bReturnType, aReturnType)
+        val byVisibilityAndType = when {
+            // Could be that one of them is flexible, in which case the types are not equal but still subtypes of one another;
+            // make the inflexible one more specific.
+            aSubtypesB && bSubtypesA -> merge(aReturnType !is ConeFlexibleType, bReturnType !is ConeFlexibleType, byVisibility)
+                ?: return null
 
-        if (aFir is FirSimpleFunction) {
+            aSubtypesB && byVisibility >= 0 -> 1
-            require(bFir is FirSimpleFunction) { "b is " + b.javaClass }
+            bSubtypesA && byVisibility <= 0 -> -1
-            return isTypeMoreSpecific(aReturnType, bReturnType, typeCheckerState)
+            else -> return null // unorderable by types, or visibility disagrees
         }
-        if (aFir is FirProperty) {
-            require(bFir is FirProperty) { "b is " + b.javaClass }
 
-            if (!isAccessorMoreSpecific(aFir.setter, bFir.setter)) return false
+        return when (aFir) {
-
+            is FirSimpleFunction -> {
-            return if (aFir.isVar && bFir.isVar) {
+                require(bFir is FirSimpleFunction) { "b is " + bFir.javaClass }
-                AbstractTypeChecker.equalTypes(typeCheckerState, aReturnType, bReturnType)
+                byVisibilityAndType
-            } else { // both vals or var vs val: val can't be more specific then var
-                !(!aFir.isVar && bFir.isVar) && isTypeMoreSpecific(aReturnType, bReturnType, typeCheckerState)
             }
+
+            is FirProperty -> {
+                require(bFir is FirProperty) { "b is " + bFir.javaClass }
+                // At least one of `subtypes` is true here, so `!xSubtypesY` implies `ySubtypesX`, meaning y's type
+                // is a *strict* subtype of x's. Vars are more specific than vals, so if one is a var and another
+                // has a strict subtype, then they are unorderable - one is a val with a more specific type than
+                // the other var, or both are vars of different types.
+                if (aFir.isVar && !aSubtypesB) return null
+                if (bFir.isVar && !bSubtypesA) return null
+                merge(aFir.isVar, bFir.isVar, byVisibilityAndType)
+            }
+
+            else -> throw IllegalArgumentException("Unexpected callable: " + aFir.javaClass)
         }
-        throw IllegalArgumentException("Unexpected callable: " + a.javaClass)
-    }
-
-    private fun isTypeMoreSpecific(a: ConeKotlinType, b: ConeKotlinType, typeCheckerState: TypeCheckerState): Boolean =
-        AbstractTypeChecker.isSubtypeOf(typeCheckerState, a, b)
-
-    private fun isAccessorMoreSpecific(a: FirPropertyAccessor?, b: FirPropertyAccessor?): Boolean {
-        if (a == null || b == null) return true
-        return isVisibilityMoreSpecific(a.visibility, b.visibility)
-    }
-
-    private fun isVisibilityMoreSpecific(a: Visibility, b: Visibility): Boolean {
-        val result = Visibilities.compare(a, b)
-        return result == null || result >= 0
     }
 }