K2: Adjust invoke priorities with K1

- At first, get rid of a kind of interceptTowerGroup callback and use explicit towerGroup construction instead - Get rid of INVOKE_RECEIVER (not sure exactly why it was needed) - Make comparison consistent with tower-levels priority of K2 (see the comments in the compareTo code) ^KT-37375 Fixed ^KT-58940 Open
2023-05-25 19:09:11 +02:00
parent 04dd209f8d
commit 1f120ecd20
13 changed files with 232 additions and 68 deletions
@@ -27449,6 +27449,12 @@ public class DiagnosticCompilerTestFE10TestdataTestGenerated extends AbstractDia
                    KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/resolve/invoke"), Pattern.compile("^(.+)\\.kt$"), Pattern.compile("^(.+)\\.(reversed|fir|ll)\\.kts?$"), true, "multiplatform");
                }

+                @Test
+                @TestMetadata("avoidTypeCheckerRecursion.kt")
+                public void testAvoidTypeCheckerRecursion() throws Exception {
+                    runTest("compiler/testData/diagnostics/tests/resolve/invoke/avoidTypeCheckerRecursion.kt");
+                }
+
                @Test
                @TestMetadata("closeInvokesFarVariable.kt")
                public void testCloseInvokesFarVariable() throws Exception {
@@ -27449,6 +27449,12 @@ public class LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated
                    KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/resolve/invoke"), Pattern.compile("^(.+)\\.kt$"), Pattern.compile("^(.+)\\.(reversed|fir|ll)\\.kts?$"), true, "multiplatform");
                }

+                @Test
+                @TestMetadata("avoidTypeCheckerRecursion.kt")
+                public void testAvoidTypeCheckerRecursion() throws Exception {
+                    runTest("compiler/testData/diagnostics/tests/resolve/invoke/avoidTypeCheckerRecursion.kt");
+                }
+
                @Test
                @TestMetadata("closeInvokesFarVariable.kt")
                public void testCloseInvokesFarVariable() throws Exception {
@@ -27449,6 +27449,12 @@ public class FirLightTreeOldFrontendDiagnosticsTestGenerated extends AbstractFir
                    KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/resolve/invoke"), Pattern.compile("^(.+)\\.kt$"), Pattern.compile("^(.+)\\.(reversed|fir|ll)\\.kts?$"), true, "multiplatform");
                }

+                @Test
+                @TestMetadata("avoidTypeCheckerRecursion.kt")
+                public void testAvoidTypeCheckerRecursion() throws Exception {
+                    runTest("compiler/testData/diagnostics/tests/resolve/invoke/avoidTypeCheckerRecursion.kt");
+                }
+
                @Test
                @TestMetadata("closeInvokesFarVariable.kt")
                public void testCloseInvokesFarVariable() throws Exception {
@@ -27461,6 +27461,12 @@ public class FirPsiOldFrontendDiagnosticsTestGenerated extends AbstractFirPsiDia
                    KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/resolve/invoke"), Pattern.compile("^(.*)\\.kts?$"), Pattern.compile("^(.+)\\.(reversed|fir|ll)\\.kts?$"), true, "multiplatform");
                }

+                @Test
+                @TestMetadata("avoidTypeCheckerRecursion.kt")
+                public void testAvoidTypeCheckerRecursion() throws Exception {
+                    runTest("compiler/testData/diagnostics/tests/resolve/invoke/avoidTypeCheckerRecursion.kt");
+                }
+
                @Test
                @TestMetadata("closeInvokesFarVariable.kt")
                public void testCloseInvokesFarVariable() throws Exception {
@@ -240,7 +240,7 @@ internal class FirInvokeResolveTowerExtension(
    // For calls having a form of "x.(f)()"
    fun enqueueResolveTasksForImplicitInvokeCall(info: CallInfo, receiverExpression: FirExpression) {
        val explicitReceiverValue = ExpressionReceiverValue(receiverExpression)
-        val task = createInvokeFunctionResolveTask(info, TowerGroup.EmptyRoot)
+        val task = createInvokeFunctionResolveTask(info, TowerGroup.EmptyRootForInvokeReceiver)
        manager.enqueueResolverTask {
            task.runResolverForInvoke(
                info, explicitReceiverValue,
@@ -369,11 +369,8 @@ private class InvokeFunctionResolveTask(
    candidateFactory,
 ) {

-    override fun interceptTowerGroup(towerGroup: TowerGroup): TowerGroup {
-        val invokeGroup = towerGroup.InvokeResolvePriority(InvokeResolvePriority.COMMON_INVOKE)
-        val max = maxOf(invokeGroup, receiverGroup)
-        return max.InvokeReceiver(receiverGroup)
-    }
+    private fun TowerGroup.withGivenInvokeReceiverGroup(invokeResolvePriority: InvokeResolvePriority): TowerGroup =
+        InvokeReceiver(receiverGroup, invokeResolvePriority)

    suspend fun runResolverForInvoke(
        info: CallInfo,
@@ -418,7 +415,7 @@ private class InvokeFunctionResolveTask(
    ) {
        processLevel(
            invokeReceiverValue.toMemberScopeTowerLevel(),
-            info, TowerGroup.Member.InvokeResolvePriority(InvokeResolvePriority.INVOKE_EXTENSION),
+            info, TowerGroup.Member.withGivenInvokeReceiverGroup(InvokeResolvePriority.INVOKE_EXTENSION),
            ExplicitReceiverKind.DISPATCH_RECEIVER
        )
    }
@@ -434,8 +431,8 @@ private class InvokeFunctionResolveTask(
            val towerGroup =
                TowerGroup
                    .Implicit(depth)
-                    .InvokeExtension
-                    .InvokeResolvePriority(InvokeResolvePriority.INVOKE_EXTENSION)
+                    .InvokeExtensionWithImplicitReceiver
+                    .withGivenInvokeReceiverGroup(InvokeResolvePriority.INVOKE_EXTENSION)

            processLevel(
                invokeReceiverValue.toMemberScopeTowerLevel(),
@@ -453,7 +450,7 @@ private class InvokeFunctionResolveTask(
        explicitReceiverKind: ExplicitReceiverKind
    ) = processLevel(
        towerLevel, callInfo,
-        group.InvokeResolvePriority(InvokeResolvePriority.COMMON_INVOKE),
+        group.withGivenInvokeReceiverGroup(InvokeResolvePriority.COMMON_INVOKE),
        explicitReceiverKind
    )
 }
@@ -6,6 +6,7 @@
 package org.jetbrains.kotlin.fir.resolve.calls.tower

 import java.lang.Long.toBinaryString
+import java.lang.Long.compareUnsigned

 sealed class TowerGroupKind(val index: Byte) : Comparable<TowerGroupKind> {
    abstract class WithDepth(index: Byte, val depth: Int) : TowerGroupKind(index) {
@@ -42,7 +43,7 @@ sealed class TowerGroupKind(val index: Byte) : Comparable<TowerGroupKind> {

    class ContextReceiverGroup(depth: Int) : WithDepth(7, depth)

-    data object InvokeExtension : TowerGroupKind(8)
+    data object InvokeExtensionWithImplicitReceiver : TowerGroupKind(8)

    data object QualifierValue : TowerGroupKind(9)

@@ -145,11 +146,29 @@ private constructor(
            }
        }

+        private fun compareDebugKinds(aDebugKinds: Array<TowerGroupKind>, bDebugKinds: Array<TowerGroupKind>): Int {
+            var index = 0
+            while (index < aDebugKinds.size) {
+                if (index >= bDebugKinds.size) return 1
+                when {
+                    aDebugKinds[index] < bDebugKinds[index] -> return -1
+                    aDebugKinds[index] > bDebugKinds[index] -> return 1
+                }
+                index++
+            }
+            if (index < bDebugKinds.size) return -1
+
+            return 0
+        }
+
+        val DEBUG_KINDS_COMPARATOR: Comparator<Array<TowerGroupKind>> = Comparator(::compareDebugKinds)
+
        private fun kindOf(kind: TowerGroupKind): TowerGroup {
            return TowerGroup(subscript(0, kind), debugKindArrayOf(kind))
        }

        val EmptyRoot = TowerGroup(0, EMPTY_KIND_ARRAY)
+        val EmptyRootForInvokeReceiver = TowerGroup(0, EMPTY_KIND_ARRAY, InvokeResolvePriority.INVOKE_RECEIVER)

        val Start = kindOf(TowerGroupKind.Start)

@@ -186,11 +205,28 @@ private constructor(

    fun ContextReceiverGroup(depth: Int) = kindOf(TowerGroupKind.ContextReceiverGroup(depth))

-    val InvokeExtension get() = kindOf(TowerGroupKind.InvokeExtension)
+    val InvokeExtensionWithImplicitReceiver get() = kindOf(TowerGroupKind.InvokeExtensionWithImplicitReceiver)

    fun TopPrioritized(depth: Int) = kindOf(TowerGroupKind.TopPrioritized(depth))

-    fun InvokeReceiver(receiverGroup: TowerGroup) = TowerGroup(code, debugKinds, invokeResolvePriority, receiverGroup)
+    fun InvokeReceiver(
+        receiverGroup: TowerGroup,
+        invokeResolvePriority: InvokeResolvePriority
+    ): TowerGroup {
+        require(receiverGroup.invokeResolvePriority == InvokeResolvePriority.INVOKE_RECEIVER) {
+            "Receivers for invoke should be resolved with INVOKE_RECEIVER, but ${receiverGroup.invokeResolvePriority} found"
+        }
+
+        require(invokeResolvePriority != InvokeResolvePriority.NONE && invokeResolvePriority != InvokeResolvePriority.INVOKE_RECEIVER) {
+            "invokeResolvePriority should be non-trivial when receiverGroup is specified"
+        }
+
+        require(receiverGroup.receiverGroup == null) {
+            "receiverGroup should be trivial, but ${receiverGroup.receiverGroup} was found"
+        }
+
+        return TowerGroup(code, debugKinds, invokeResolvePriority, receiverGroup)
+    }

    // Treating `a.foo()` common calls as more prioritized than `a.foo.invoke()`
    // It's not the same as TowerGroupKind because it's not about tower levels, but rather a different dimension semantically.
@@ -201,38 +237,94 @@ private constructor(
    }

    private fun debugCompareTo(other: TowerGroup): Int {
-        var index = 0
-        while (index < debugKinds.size) {
-            if (index >= other.debugKinds.size) return 1
-            when {
-                debugKinds[index] < other.debugKinds[index] -> return -1
-                debugKinds[index] > other.debugKinds[index] -> return 1
-            }
-            index++
-        }
-        if (index < other.debugKinds.size) return -1
+        // In case of `receiverGroup` presence, it means that candidate is invoke+variable or invokeExtension+variable.
+        // In both cases `receiverGroup` means the tower group of a receiver variable.
+        // For regular invoke+variable, this@debugKinds mean the tower level where the "invoke" function has been found.
+        // For invokeExtension, this@debugKinds is either TowerGroup.Member in case of explicit extension receiver value,
+        // like `explicitReceiver.myPropertyOfExtensionFunctionType()`
+        // or TowerGroup.Implicit(depth).InvokeExtensionWithImplicitReceiver in case of implicit extension receiver value
+        // like `with(myReceiver) { myPropertyOfExtensionFunctionType() }`
+        val receiverDebugKinds = receiverGroup?.debugKinds ?: emptyArray()
+        val otherReceiverDebugKinds = other.receiverGroup?.debugKinds ?: emptyArray()

-        val actualResult = invokeResolvePriority.compareTo(other.invokeResolvePriority)
-        if (actualResult == 0) {
-            return if (receiverGroup == null || other.receiverGroup == null) 0
-            else receiverGroup.debugCompareTo(other.receiverGroup)
+        // Maximums define how far resolution algorithm should go to find the candidate
+        val thisMax = maxOf(debugKinds, receiverDebugKinds, DEBUG_KINDS_COMPARATOR)
+        val otherMax = maxOf(other.debugKinds, otherReceiverDebugKinds, DEBUG_KINDS_COMPARATOR)
+
+        // If maximum tower groups are different, then just use more prioritized of maximums.
+        // It seems more or less obvious, because if one candidate uses X group and other uses Y and X > Y,
+        // then for the second candidate we wouldn't even need to continue resolution to the level of X, like we would already found
+        // complete and probably successful candidate on Y.
+        val maxResult = compareDebugKinds(thisMax, otherMax)
+        if (maxResult != 0) return maxResult
+
+        // If any of the candidate is not invoke/invokeExtension, choose it
+        // Otherwise, prefer invoke and then invokeExtension ones
+        val invokeKindPriority = invokeResolvePriority.compareTo(other.invokeResolvePriority)
+        if (invokeKindPriority != 0) return invokeKindPriority
+
+        // NB: thisMax == otherMax and kinds of invokes are the same
+        return if (compareDebugKinds(receiverDebugKinds, thisMax) == 0 && compareDebugKinds(otherReceiverDebugKinds, thisMax) == 0) {
+            // If both variables/receivers are obtained from the maximum current level, compare invoke/invokeExtension levels
+            // Also, here it might be viewed as comparing minimums of both candidates
+            compareDebugKinds(debugKinds, other.debugKinds)
+        } else {
+            // Otherwise prefer, the one with the closest variable/receiver
+            // See how groups are prioritized at org.jetbrains.kotlin.resolve.calls.tower.AbstractInvokeTowerProcessor.process
+            // Also, see the test testData/diagnostics/tests/resolve/invoke/closerVariableMatterMore.kt
+            // There, we have two candidates A, B for which maxTowerGroup(A) == maxTowerGroup(B) && minTowerGroup(A) == minTowerGroup(B)
+            // But we don't assume them as equally placed preferring one with the closest receiver.
+            //
+            // NB: receiverDebugKinds == otherReceiverDebugKinds => this == other
+            // Proof:
+            // - Let's assume receiverDebugKinds == otherReceiverDebugKinds
+            // - We know that thisMax == otherMax
+            // - If receiverDebugKinds == thisMax then otherReceiverDebugKinds == thisMax, so we wouldn't come to the `else` section
+            // - That means that receiverDebugKinds != thisMax && otherReceiverDebugKinds != thisMax
+            // - Thus we have thisMax == debugKinds (because thisMax = maxOf(debugKinds, receiverDebugKinds)
+            // - And the same for otherMax == otherDebugKinds
+            //
+            // Considering thisMax == otherMax we have debugKinds == otherDebugKinds
+            // And with initial condition of receiverDebugKinds == otherReceiverDebugKinds we've got candidates from completely the same level
+            compareDebugKinds(receiverDebugKinds, otherReceiverDebugKinds)
        }
-        return actualResult
    }

    override fun compareTo(other: TowerGroup): Int = run {
-        val result = java.lang.Long.compareUnsigned(code, other.code)
-        if (result != 0) return@run result
-        val actualResult = invokeResolvePriority.compareTo(other.invokeResolvePriority)
-        if (actualResult == 0) {
-            return@run if (receiverGroup == null || other.receiverGroup == null) 0
-            else receiverGroup.compareTo(other.receiverGroup)
+        // See detailed algorithm description at `debugCompareTo`
+        // Fast-path
+        if (receiverGroup == null && other.receiverGroup == null &&
+            invokeResolvePriority == InvokeResolvePriority.NONE && other.invokeResolvePriority == InvokeResolvePriority.NONE
+        ) {
+            return@run compareUnsigned(code, other.code)
+        }
+
+        val receiverCode = receiverGroup?.code ?: 0 // TowerGroup.Start.code
+        val otherReceiverCode = other.receiverGroup?.code ?: 0
+
+        val thisMax: Long = if (compareUnsigned(code, receiverCode) >= 0) code else receiverCode
+        val otherMax: Long = if (compareUnsigned(other.code, otherReceiverCode) >= 0) other.code else otherReceiverCode
+
+        val resultMax = compareUnsigned(thisMax, otherMax)
+        if (resultMax != 0) return@run resultMax
+
+        val invokeKindPriority = invokeResolvePriority.compareTo(other.invokeResolvePriority)
+        if (invokeKindPriority != 0) return invokeKindPriority
+
+        return if (compareUnsigned(receiverCode, thisMax) == 0 && compareUnsigned(otherReceiverCode, thisMax) == 0) {
+            compareUnsigned(code, other.code)
+        } else {
+            compareUnsigned(receiverCode, otherReceiverCode)
        }
-        return@run actualResult
    }.also {
        if (DEBUG) {
            val debugResult = debugCompareTo(other)
-            require(debugResult == it) { "Kind comparison incorrect: $this vs $other, expected: $it, $debugResult" }
+            require(debugResult == it) {
+                "Kind comparison incorrect: $this vs $other, expected: $it, $debugResult"
+            }
+            require((this == other) == (debugResult == 0)) {
+                "Equality and compareTo should work consistently, but $debugResult found"
+            }
        }
    }

@@ -266,5 +358,33 @@ private constructor(
 }

 enum class InvokeResolvePriority {
-    NONE, INVOKE_RECEIVER, COMMON_INVOKE, INVOKE_EXTENSION;
+    /**
+     * Looking for regular functions (or variables when the callee is a variable access)
+     */
+    NONE,
+
+    /**
+     * When resolving a function call, that kind of priority signifies looking for a variable candidates that might serve as a receivers
+     * for "invoke" calls.
+     *
+     * Semantically, that kind of priority is redundant and using NONE works just fine, but having it, helps not to trigger computation of
+     * property return type when we've already found some successful regular candidate.
+     *
+     * See testData/diagnostics/tests/resolve/invoke/errors/typeCheckerRanRecursive.kt as an example which would fail if we use NONE instead
+     * of INVOKE_RECEIVER
+     */
+    INVOKE_RECEIVER,
+
+    /**
+     * Looking for "invoke()" function member or extension that would match for already found variable that would be used as a receiver
+     */
+    COMMON_INVOKE,
+
+    /**
+     * Resolving "invoke()" call when the found variable candidate has a type of extension function type, and we've got a matching extension
+     * receiver (whether explicit or implicit one).
+     *
+     * It should be de-prioritized comparing with regular "invoke()" (COMMON_INVOKE).
+     */
+    INVOKE_EXTENSION
 }
@@ -0,0 +1,23 @@
+// ISSUE: KT-58940
+
+// FILE: JavaIndex.java
+public class JavaIndex {
+    public String getIndexer() { return ""; }
+}
+
+// FILE: main.kt
+abstract class KotlinIndex : JavaIndex() {
+    fun indexer(x: Int): String = ""
+}
+
+class MyKotlinIndex : KotlinIndex() {
+    // `indexer(1)` call should just request the member function that might be resolved successfully
+    // and should not request `indexer` variable (as invoke candidates are anyway less prioritized), thus avoiding TYPECHECKER_HAS_RUN_INTO_RECURSIVE_PROBLEM
+    val INDEXER = indexer(1)
+
+    override fun getIndexer() = INDEXER
+}
+
+fun main() {
+    MyKotlinIndex().getIndexer().length
+}
@@ -0,0 +1,23 @@
+// ISSUE: KT-58940
+
+// FILE: JavaIndex.java
+public class JavaIndex {
+    public String getIndexer() { return ""; }
+}
+
+// FILE: main.kt
+abstract class KotlinIndex : JavaIndex() {
+    fun indexer(x: Int): String = ""
+}
+
+class MyKotlinIndex : KotlinIndex() {
+    // `indexer(1)` call should just request the member function that might be resolved successfully
+    // and should not request `indexer` variable (as invoke candidates are anyway less prioritized), thus avoiding TYPECHECKER_HAS_RUN_INTO_RECURSIVE_PROBLEM
+    val INDEXER = <!DEBUG_INFO_LEAKING_THIS!>indexer<!>(1)
+
+    override fun getIndexer() = INDEXER
+}
+
+fun main() {
+    MyKotlinIndex().getIndexer().length
+}
@@ -1,17 +0,0 @@
-// ISSUE: KT-37375
-
-val foo: Any = Any()
-
-fun bar() {
-    operator fun Any.invoke(): String = ""
-
-    fun baz() {
-        operator fun Any.invoke(): Int = 1
-
-        fun barbaz() {
-            takeInt(<!OVERLOAD_RESOLUTION_AMBIGUITY!>foo<!>())
-        }
-    }
-}
-
-fun takeInt(x: Int) {}
@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 // ISSUE: KT-37375

 val foo: Any = Any()
@@ -1,14 +0,0 @@
-// ISSUE: KT-37375
-
-fun takeDouble(x: Double) {}
-
-val bar: Int = 1
-operator fun Double.invoke(): Double = 1.0 // (1)
-
-fun test_1() {
-    val bar: Double = 2.0
-    operator fun Int.invoke(): Int = 1  // (2)
-
-    val res = bar() // should resolve to (1)
-    takeDouble(<!ARGUMENT_TYPE_MISMATCH!>res<!>) // should be OK
-}
@@ -1,3 +1,4 @@
+// FIR_IDENTICAL
 // ISSUE: KT-37375

 fun takeDouble(x: Double) {}
@@ -28293,6 +28293,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest {
                    KtTestUtil.assertAllTestsPresentByMetadataWithExcluded(this.getClass(), new File("compiler/testData/diagnostics/tests/resolve/invoke"), Pattern.compile("^(.*)\\.kts?$"), Pattern.compile("^(.+)\\.(reversed|fir|ll)\\.kts?$"), true);
                }

+                @Test
+                @TestMetadata("avoidTypeCheckerRecursion.kt")
+                public void testAvoidTypeCheckerRecursion() throws Exception {
+                    runTest("compiler/testData/diagnostics/tests/resolve/invoke/avoidTypeCheckerRecursion.kt");
+                }
+
                @Test
                @TestMetadata("closeInvokesFarVariable.kt")
                public void testCloseInvokesFarVariable() throws Exception {