FIR: assume a lambda returns Unit if it ends with a non-expression

While it is theoretically useful to know that `{ while(true) {} }` returns Nothing, CFG node deadness is not precise enough to do that: if the entire lambda is dead, it's no longer possible to find out whether the loop is terminating. Besides, `while (true)` and `if (true)` are pretty much the only constructs like that anyway. Note that this commit does not affect resolution for lambdas that end in a Nothing-returning expression, e.g. `throw`.
2022-12-09 22:11:44 +01:00
parent 803abfeba8
commit 1eccb9aea1
12 changed files with 47 additions and 42 deletions
@@ -65,7 +65,7 @@ import kotlin.contracts.contract
 fun List<FirQualifierPart>.toTypeProjections(): Array<ConeTypeProjection> =
    asReversed().flatMap { it.typeArgumentList.typeArguments.map { typeArgument -> typeArgument.toConeTypeProjection() } }.toTypedArray()

-fun FirAnonymousFunction.shouldReturnUnit(returnStatements: Collection<FirStatement>): Boolean =
+fun FirAnonymousFunction.shouldReturnUnit(returnStatements: Collection<FirExpression>): Boolean =
    isLambda && returnStatements.any { it is FirUnitExpression }

 fun FirAnonymousFunction.isExplicitlySuspend(session: FirSession): Boolean =
@@ -136,10 +136,10 @@ abstract class FirDataFlowAnalyzer(
        return variable.stability to types.toMutableList()
    }

-    fun returnExpressionsOfAnonymousFunctionOrNull(function: FirAnonymousFunction): Collection<FirStatement>? =
+    fun returnExpressionsOfAnonymousFunctionOrNull(function: FirAnonymousFunction): Collection<FirExpression>? =
        graphBuilder.returnExpressionsOfAnonymousFunction(function)

-    fun returnExpressionsOfAnonymousFunction(function: FirAnonymousFunction): Collection<FirStatement> =
+    fun returnExpressionsOfAnonymousFunction(function: FirAnonymousFunction): Collection<FirExpression> =
        returnExpressionsOfAnonymousFunctionOrNull(function)
            ?: error("anonymous function ${function.render()} not analyzed")

@@ -11,6 +11,7 @@ import org.jetbrains.kotlin.fir.declarations.*
 import org.jetbrains.kotlin.fir.declarations.utils.hasExplicitBackingField
 import org.jetbrains.kotlin.fir.declarations.utils.isLocal
 import org.jetbrains.kotlin.fir.expressions.*
+import org.jetbrains.kotlin.fir.expressions.builder.buildUnitExpression
 import org.jetbrains.kotlin.fir.references.FirControlFlowGraphReference
 import org.jetbrains.kotlin.fir.render
 import org.jetbrains.kotlin.fir.resolve.dfa.*
@@ -96,19 +97,22 @@ class ControlFlowGraphBuilder {

    // ----------------------------------- Public API -----------------------------------

-    fun returnExpressionsOfAnonymousFunction(function: FirAnonymousFunction): Collection<FirStatement>? {
+    fun returnExpressionsOfAnonymousFunction(function: FirAnonymousFunction): Collection<FirExpression>? {
        val exitNode = function.controlFlowGraphReference?.controlFlowGraph?.exitNode ?: return null

-        fun FirElement.extractArgument(): FirElement = when {
-            this is FirReturnExpression && target.labeledElement.symbol == function.symbol -> result.extractArgument()
-            else -> this
-        }
-
-        fun CFGNode<*>.extractArgument(): FirStatement? = when (this) {
-            is FunctionEnterNode, is TryMainBlockEnterNode, is FinallyBlockExitNode, is CatchClauseEnterNode -> null
-            is BlockExitNode -> if (function.isLambda || isDead) firstPreviousNode.extractArgument() else null
-            is StubNode -> firstPreviousNode.extractArgument()
-            else -> fir.extractArgument() as FirStatement?
+        fun CFGNode<*>.returnExpression(): FirExpression? = when (this) {
+            is BlockExitNode -> when {
+                // lambda@{ x } -> x
+                // lambda@{ class C } -> Unit-returning stub
+                function.isLambda -> fir.statements.lastOrNull() as? FirExpression
+                    ?: buildUnitExpression { source = fir.statements.lastOrNull()?.source ?: fir.source }
+                // fun() { terminatingExpression } -> nothing (checker will emit an error if return type is not Unit)
+                // fun() { throw } or fun() { returnsNothing() } -> Nothing-returning stub
+                else -> FirStub.takeIf { _ -> previousNodes.all { it is StubNode } }
+            }
+            // lambda@{ return@lambda x } -> x
+            is JumpNode -> (fir as? FirReturnExpression)?.takeIf { it.target.labeledElement.symbol == function.symbol }?.result
+            else -> null // shouldn't happen? expression bodies are implicitly wrapped in `FirBlock`s
        }

        val returnValues = exitNode.previousNodes.mapNotNullTo(mutableSetOf()) {
@@ -117,9 +121,9 @@ class ControlFlowGraphBuilder {
            // * UncaughtExceptionPath: last expression = whatever threw, *not* a return value
            // * ReturnPath(this lambda): these go from `finally` blocks, so that's not the return value;
            //   look in `nonDirectJumps` instead.
-            it.takeIf { edge.kind.usedInCfa && edge.label == NormalPath }?.extractArgument()
+            it.takeIf { edge.kind.usedInCfa && edge.label == NormalPath }?.returnExpression()
        }
-        return nonDirectJumps[exitNode].mapNotNullTo(returnValues) { it.extractArgument() }
+        return nonDirectJumps[exitNode].mapNotNullTo(returnValues) { it.returnExpression() }
    }

    @OptIn(CfgBuilderInternals::class)
@@ -7,7 +7,6 @@ package org.jetbrains.kotlin.fir.resolve.inference

 import org.jetbrains.kotlin.fir.FirCallResolver
 import org.jetbrains.kotlin.fir.expressions.FirExpression
-import org.jetbrains.kotlin.fir.expressions.FirStatement
 import org.jetbrains.kotlin.fir.lookupTracker
 import org.jetbrains.kotlin.fir.recordTypeResolveAsLookup
 import org.jetbrains.kotlin.fir.references.builder.buildErrorNamedReference
@@ -28,7 +27,7 @@ import org.jetbrains.kotlin.resolve.calls.inference.model.BuilderInferencePositi
 import org.jetbrains.kotlin.types.model.*

 data class ReturnArgumentsAnalysisResult(
-    val returnArguments: Collection<FirStatement>,
+    val returnArguments: Collection<FirExpression>,
    val inferenceSession: FirInferenceSession?
 )

@@ -178,7 +177,6 @@ class PostponedArgumentsAnalyzer(
        val lambdaReturnType = lambda.returnType.let(substitute)
        returnArguments.forEach {
            val haveSubsystem = c.addSubsystemFromExpression(it)
-            if (it !is FirExpression) return@forEach
            // If the lambda returns Unit, the last expression is not returned and should not be constrained.
            // TODO (KT-55837) questionable moment inherited from FE1.0 (the `haveSubsystem` case):
            //    fun <T> foo(): T
@@ -10,7 +10,7 @@ import org.jetbrains.kotlin.fir.FirSession
 import org.jetbrains.kotlin.fir.declarations.FirAnonymousFunction
 import org.jetbrains.kotlin.fir.expressions.FirAnonymousFunctionExpression
 import org.jetbrains.kotlin.fir.expressions.FirCallableReferenceAccess
-import org.jetbrains.kotlin.fir.expressions.FirStatement
+import org.jetbrains.kotlin.fir.expressions.FirExpression
 import org.jetbrains.kotlin.fir.references.FirNamedReference
 import org.jetbrains.kotlin.fir.resolve.DoubleColonLHS
 import org.jetbrains.kotlin.fir.resolve.calls.Candidate
@@ -60,7 +60,7 @@ class ResolvedLambdaAtom(
    override var expectedType = expectedType
        private set

-    lateinit var returnStatements: Collection<FirStatement>
+    lateinit var returnStatements: Collection<FirExpression>

    override val inputTypes: Collection<ConeKotlinType>
        get() {
@@ -577,7 +577,7 @@ class FirCallCompletionResultsWriterTransformer(
        // The case where we can't find any return expressions not common, and happens when there are anonymous function arguments
        // that aren't mapped to any parameter in the call. So, we don't run body resolve transformation for them, thus there's
        // no control flow info either. Example: second lambda in the call like list.filter({}, {})
-        val returnStatements = dataFlowAnalyzer.returnExpressionsOfAnonymousFunctionOrNull(anonymousFunction)
+        val returnExpressions = dataFlowAnalyzer.returnExpressionsOfAnonymousFunctionOrNull(anonymousFunction)
            ?: return transformImplicitTypeRefInAnonymousFunction(anonymousFunction)

        val expectedType = data?.getExpectedType(anonymousFunction)?.let { expectedArgumentType ->
@@ -623,14 +623,14 @@ class FirCallCompletionResultsWriterTransformer(

        val newData = expectedReturnType?.toExpectedType()
        val result = transformElement(anonymousFunction, newData)
-        for (expression in returnStatements) {
+        for (expression in returnExpressions) {
            expression.transformSingle(this, newData)
        }

        // Prefer the expected type over the inferred one - the latter is a subtype of the former in valid code,
        // and there will be ARGUMENT_TYPE_MISMATCH errors on the lambda's return expressions in invalid code.
        val resultReturnType = expectedReturnType
-            ?: session.typeContext.commonSuperTypeOrNull(returnStatements.mapNotNull { (it as? FirExpression)?.resultType?.coneType })
+            ?: session.typeContext.commonSuperTypeOrNull(returnExpressions.map { it.resultType.coneType })
            ?: session.builtinTypes.unitType.type

        if (initialReturnType != resultReturnType) {
@@ -845,15 +845,14 @@ open class FirDeclarationsResolveTransformer(transformer: FirAbstractBodyResolve
        // Any lambda expression assigned to `(...) -> Unit` returns Unit
        if (isLambda && expected?.type?.isUnit == true) return expected
        // `lambda@ { return@lambda }` always returns Unit
-        val returnStatements = dataFlowAnalyzer.returnExpressionsOfAnonymousFunction(this)
-        if (shouldReturnUnit(returnStatements)) return session.builtinTypes.unitType
+        val returnExpressions = dataFlowAnalyzer.returnExpressionsOfAnonymousFunction(this)
+        if (shouldReturnUnit(returnExpressions)) return session.builtinTypes.unitType
        // Here is a questionable moment where we could prefer the expected type over an inferred one.
        // In correct code this doesn't matter, as all return expression types should be subtypes of the expected type.
        // In incorrect code, this would change diagnostics: we can get errors either on the entire lambda, or only on its
        // return statements. The former kind of makes more sense, but the latter is more readable.
-        val inferredFromReturnStatements =
-            session.typeContext.commonSuperTypeOrNull(returnStatements.mapNotNull { (it as? FirExpression)?.resultType?.coneType })
-        return inferredFromReturnStatements?.let { returnTypeRef.resolvedTypeFromPrototype(it) }
+        val inferredFromReturnExpressions = session.typeContext.commonSuperTypeOrNull(returnExpressions.map { it.resultType.coneType })
+        return inferredFromReturnExpressions?.let { returnTypeRef.resolvedTypeFromPrototype(it) }
            ?: session.builtinTypes.unitType // Empty lambda returns Unit
    }

@@ -13,6 +13,9 @@ import org.jetbrains.kotlin.fir.declarations.*
 import org.jetbrains.kotlin.fir.expressions.*
 import org.jetbrains.kotlin.fir.resolve.dfa.PersistentFlow
 import org.jetbrains.kotlin.fir.resolve.dfa.controlFlowGraph
+import org.jetbrains.kotlin.fir.types.FirTypeRef
+import org.jetbrains.kotlin.fir.types.impl.FirImplicitNothingTypeRef
+import org.jetbrains.kotlin.fir.types.isNothing
 import org.jetbrains.kotlin.fir.visitors.FirTransformer
 import org.jetbrains.kotlin.fir.visitors.FirVisitor
 import org.jetbrains.kotlin.utils.SmartList
@@ -891,17 +894,18 @@ class AnnotationExitNode(owner: ControlFlowGraph, override val fir: FirAnnotatio

 // ----------------------------------- Stub -----------------------------------

-object FirStub : FirElement {
+object FirStub : FirExpression() {
    override val source: KtSourceElement? get() = null
+    override val typeRef: FirTypeRef = FirImplicitNothingTypeRef(null)
+    override val annotations: List<FirAnnotation> get() = listOf()

    override fun <R, D> acceptChildren(visitor: FirVisitor<R, D>, data: D) {}
-
-    override fun <D> transformChildren(transformer: FirTransformer<D>, data: D): FirElement {
-        return this
-    }
+    override fun <D> transformAnnotations(transformer: FirTransformer<D>, data: D): FirExpression = this
+    override fun <D> transformChildren(transformer: FirTransformer<D>, data: D): FirElement = this
+    override fun replaceAnnotations(newAnnotations: List<FirAnnotation>) { assert(newAnnotations.isEmpty()) }
+    override fun replaceTypeRef(newTypeRef: FirTypeRef) { assert(newTypeRef.isNothing) }
 }

-
 // ----------------------------------- Smart-cast node -----------------------------------

 class SmartCastExpressionExitNode(owner: ControlFlowGraph, override val fir: FirSmartCastExpression, level: Int, id: Int) : CFGNode<FirSmartCastExpression>(owner, level, id) {
@@ -11,5 +11,5 @@ fun test(bal: Array<Int>) {

    val e: Unit = run { bar += 4 }

-    val f: Int = <!INITIALIZER_TYPE_MISMATCH!>run <!ARGUMENT_TYPE_MISMATCH!>{ bar += 4 }<!><!>
+    val f: Int = run { <!ARGUMENT_TYPE_MISMATCH!>bar += 4<!> }
 }
@@ -65,7 +65,7 @@ fun testParameter() {
    takeFnToParameter(fun(): Unit { return Unit })
    takeFnToParameter(fun() { if (true) return })
    takeFnToParameter(fun() { if (true) return Unit })
-    <!NEW_INFERENCE_NO_INFORMATION_FOR_PARAMETER!>takeFnToParameter<!>(fun() = <!UNRESOLVED_REFERENCE!>unresolved<!>())
+    takeFnToParameter(fun() = <!UNRESOLVED_REFERENCE!>unresolved<!>())
    takeFnToParameter(fun() { <!UNRESOLVED_REFERENCE!>unresolved<!>() })
    takeFnToParameter(fun(): Unit { <!UNRESOLVED_REFERENCE!>unresolved<!>() })
    takeFnToParameter(fun() { return <!UNRESOLVED_REFERENCE!>unresolved<!>() })
@@ -3,5 +3,5 @@
 fun foo(l: () -> Unit) {}
 fun bar(l: () -> String) {}

-val a = foo { <!ARGUMENT_TYPE_MISMATCH, UNSUPPORTED!>[]<!> }
+val a = foo { <!UNSUPPORTED!>[]<!> }
 val b = bar { <!ARGUMENT_TYPE_MISMATCH, UNSUPPORTED!>[]<!> }
@@ -170,8 +170,8 @@ fun case_15(z: Any?) {
        return@let it as Int
        while (true) { println(1) }
    }
-    <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Int")!>y<!>
-    <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Int")!>y<!>.equals(10)
+    <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Any")!>y<!>
+    <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Any")!>y<!>.equals(10)
 }

 /*
@@ -183,8 +183,8 @@ fun case_16(z: Any?) {
        return@run this as Int
        while (true) { println(1) }
    }
-    <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Int")!>y<!>
-    <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Int")!>y<!>.equals(10)
+    <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Any")!>y<!>
+    <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Any")!>y<!>.equals(10)
 }

 /*