FIR DFA: do not re-approve statements from LHS of and/or

If the right-hand side is evaluated at all, then in its flow those
statements were already approved. Re-approving them erases the effect of
reassignments.

^KT-28369 tag fixed-in-k2

compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/FirReturnsImpliesAnalyzer.kt

@@ -139,7 +139,7 @@ object FirReturnsImpliesAnalyzer : FirControlFlowChecker() {
         statement: OperationStatement,
         builtinTypes: BuiltinTypes
     ): TypeStatements {
-        val newTypeStatements = andForTypeStatements(flow.approvedTypeStatements, approveOperationStatement(flow, statement, null))
+        val newTypeStatements = andForTypeStatements(flow.approvedTypeStatements, approveOperationStatement(flow, statement))
         val variable = statement.variable
         if (!variable.isReal()) return newTypeStatements
         val extraStatement = when (statement.operation) {

compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt

@@ -1176,15 +1176,12 @@ abstract class FirDataFlowAnalyzer<FLOW : Flow>(
             // has to be `onlyLeftEvaluated`, and it has to be produced by the left operand.
             logicSystem.commitOperationStatement(flow, leftVariable eq onlyLeftEvaluated, shouldRemoveSynthetics = true)
         } else {
-            val (conditionalFromLeft, conditionalFromRight, approvedFromRight) =
-                logicSystem.collectInfoForBooleanOperator(flowFromLeft, leftVariable, flowFromRight, rightVariable)
-
             // If `left && right` is true, then both are true (and evaluated).
             // If `left || right` is false, then both are false.
             arrayOf(
-                approvedFromRight,
-                logicSystem.approveOperationStatement(flowFromRight, leftVariable eq bothEvaluated, conditionalFromLeft),
-                logicSystem.approveOperationStatement(flowFromRight, rightVariable eq bothEvaluated, conditionalFromRight),
+                flowFromRight.approvedTypeStatements,
+                // `leftVariable eq bothEvaluated` already approved in flowFromRight.
+                logicSystem.approveOperationStatement(flowFromRight, rightVariable eq bothEvaluated),
             ).forEach { statements ->
                 statements.values.forEach { flow.addImplication((operatorVariable eq bothEvaluated) implies it) }
             }
@@ -1192,10 +1189,10 @@ abstract class FirDataFlowAnalyzer<FLOW : Flow>(
             // If `left && right` is false, then either `left` is false, or both were evaluated and `right` is false.
             // If `left || right` is true, then either `left` is true, or both were evaluated and `right` is true.
             logicSystem.orForTypeStatements(
-                logicSystem.approveOperationStatement(flowFromLeft, leftVariable eq onlyLeftEvaluated, conditionalFromLeft),
-                // TODO: and(approvedFromRight, ...)? FE1.0 doesn't seem to handle that correctly either.
+                logicSystem.approveOperationStatement(flowFromLeft, leftVariable eq onlyLeftEvaluated),
+                // TODO: and(approved from right, ...)? FE1.0 doesn't seem to handle that correctly either.
                 //   if (x is A || whatever(x as B)) { /* x is (A | B) */ }
-                logicSystem.approveOperationStatement(flowFromRight, rightVariable eq onlyLeftEvaluated, conditionalFromRight),
+                logicSystem.approveOperationStatement(flowFromRight, rightVariable eq onlyLeftEvaluated),
             ).values.forEach { flow.addImplication((operatorVariable eq onlyLeftEvaluated) implies it) }
         }
 

compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/Flow.kt

@@ -8,6 +8,7 @@ package org.jetbrains.kotlin.fir.resolve.dfa
 import org.jetbrains.kotlin.fir.types.ConeKotlinType
 
 abstract class Flow {
+    abstract val approvedTypeStatements: TypeStatements
     abstract fun unwrapVariable(variable: RealVariable): RealVariable
     abstract fun getType(variable: RealVariable): Set<ConeKotlinType>?
 }

compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/LogicSystem.kt

@@ -49,24 +49,7 @@ abstract class LogicSystem<FLOW : Flow>(protected val context: ConeInferenceCont
 
     // ------------------------------- Public TypeStatement util functions -------------------------------
 
-    data class InfoForBooleanOperator(
-        val conditionalFromLeft: Collection<Implication>,
-        val conditionalFromRight: Collection<Implication>,
-        val knownFromRight: TypeStatements,
-    )
-
-    abstract fun collectInfoForBooleanOperator(
-        leftFlow: FLOW,
-        leftVariable: DataFlowVariable,
-        rightFlow: FLOW,
-        rightVariable: DataFlowVariable,
-    ): InfoForBooleanOperator
-
-    abstract fun approveOperationStatement(
-        flow: FLOW,
-        approvedStatement: OperationStatement,
-        statementsForVariable: Collection<Implication>?
-    ): TypeStatements
+    abstract fun approveOperationStatement(flow: FLOW, approvedStatement: OperationStatement): TypeStatements
 
     fun orForTypeStatements(left: TypeStatements, right: TypeStatements): TypeStatements = when {
         left.isEmpty() -> left

compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/PersistentLogicSystem.kt

@@ -23,7 +23,7 @@ typealias PersistentImplications = PersistentMap<DataFlowVariable, PersistentLis
 
 class PersistentFlow : Flow {
     val previousFlow: PersistentFlow?
-    var approvedTypeStatements: PersistentApprovedTypeStatements
+    override var approvedTypeStatements: PersistentApprovedTypeStatements
     var logicStatements: PersistentImplications
     val level: Int
 
@@ -257,7 +257,7 @@ abstract class PersistentLogicSystem(context: ConeInferenceContext) : LogicSyste
     }
 
     override fun commitOperationStatement(flow: PersistentFlow, statement: OperationStatement, shouldRemoveSynthetics: Boolean) {
-        approveOperationStatementsInternal(flow, statement, initialStatements = null, shouldRemoveSynthetics).values.forEach {
+        approveOperationStatementsInternal(flow, statement, shouldRemoveSynthetics).values.forEach {
             addTypeStatement(flow, it)
         }
     }
@@ -265,21 +265,17 @@ abstract class PersistentLogicSystem(context: ConeInferenceContext) : LogicSyste
     private fun approveOperationStatementsInternal(
         flow: PersistentFlow,
         approvedStatement: OperationStatement,
-        initialStatements: Collection<Implication>?,
         shouldRemoveSynthetics: Boolean
     ): TypeStatements {
         val approvedTypeStatements: ArrayListMultimap<RealVariable, TypeStatement> = ArrayListMultimap.create()
         val queue = LinkedList<OperationStatement>().apply { this += approvedStatement }
         val approved = mutableSetOf<OperationStatement>()
-        var firstIteration = true
         while (queue.isNotEmpty()) {
             val next: OperationStatement = queue.removeFirst()
             // Defense from cycles in facts
             if (!approved.add(next)) continue
             val variable = next.variable
-            val statements = initialStatements?.takeIf { firstIteration }
-                ?: flow.logicStatements[variable]?.takeIf { it.isNotEmpty() }
-                ?: continue
+            val statements = flow.logicStatements[variable]?.takeIf { it.isNotEmpty() } ?: continue
             if (shouldRemoveSynthetics && variable.isSynthetic()) {
                 flow.logicStatements -= variable
             }
@@ -291,7 +287,6 @@ abstract class PersistentLogicSystem(context: ConeInferenceContext) : LogicSyste
                     }
                 }
             }
-            firstIteration = false
         }
         return approvedTypeStatements.asMap().mapValues { and(it.value) }
     }
@@ -299,21 +294,7 @@ abstract class PersistentLogicSystem(context: ConeInferenceContext) : LogicSyste
     override fun approveOperationStatement(
         flow: PersistentFlow,
         approvedStatement: OperationStatement,
-        statementsForVariable: Collection<Implication>?
-    ): TypeStatements = approveOperationStatementsInternal(flow, approvedStatement, statementsForVariable, shouldRemoveSynthetics = false)
-
-    override fun collectInfoForBooleanOperator(
-        leftFlow: PersistentFlow,
-        leftVariable: DataFlowVariable,
-        rightFlow: PersistentFlow,
-        rightVariable: DataFlowVariable
-    ): InfoForBooleanOperator {
-        return InfoForBooleanOperator(
-            leftFlow.logicStatements[leftVariable] ?: emptyList(),
-            rightFlow.logicStatements[rightVariable] ?: emptyList(),
-            rightFlow.approvedTypeStatements
-        )
-    }
+    ): TypeStatements = approveOperationStatementsInternal(flow, approvedStatement, shouldRemoveSynthetics = false)
 
     override fun getImplicationsWithVariable(flow: PersistentFlow, variable: DataFlowVariable): Collection<Implication> {
         return flow.logicStatements[variable] ?: emptyList()

compiler/tests-spec/testData/diagnostics/notLinked/dfa/neg/11.fir.kt

@@ -5,8 +5,8 @@
 fun case_1() {
     var x: Boolean? = true
     if (<!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean?")!>x<!> is Boolean && if (true) { x = null; true } else { false }) {
-        <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean? & kotlin.Boolean")!>x<!>
-        <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean? & kotlin.Boolean")!>x<!>.not()
+        <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean?")!>x<!>
+        <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean?")!>x<!><!UNSAFE_CALL!>.<!>not()
     }
 }
 
@@ -14,8 +14,8 @@ fun case_1() {
 fun case_2() {
     var x: Boolean? = true
     if (<!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean?")!>x<!> != null && try { x = null; true } catch (e: Exception) { false }) {
-        <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean? & kotlin.Boolean")!>x<!>
-        <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean? & kotlin.Boolean")!>x<!>.not()
+        <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean?")!>x<!>
+        <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean?")!>x<!><!UNSAFE_CALL!>.<!>not()
     }
 }
 

compiler/tests-spec/testData/diagnostics/notLinked/dfa/neg/15.fir.kt

@@ -9,8 +9,8 @@
 fun case_1() {
     var x: Boolean? = true
     if (x is Boolean && if (true) { x = null; true } else { false }) {
-        <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean? & kotlin.Boolean")!>x<!>
-        <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean? & kotlin.Boolean")!>x<!>.not()
+        <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean?")!>x<!>
+        <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean?")!>x<!><!UNSAFE_CALL!>.<!>not()
     }
 }
 
@@ -22,8 +22,8 @@ fun case_1() {
 fun case_2() {
     var x: Boolean? = true
     if (x !== null && try { x = null; true } catch (e: Exception) { false }) {
-        <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean? & kotlin.Boolean")!>x<!>
-        <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean? & kotlin.Boolean")!>x<!>.not()
+        <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean?")!>x<!>
+        <!DEBUG_INFO_EXPRESSION_TYPE("kotlin.Boolean?")!>x<!><!UNSAFE_CALL!>.<!>not()
     }
 }