From 39f6b50836126b6b337b2eb47341c7fe90e3a448 Mon Sep 17 00:00:00 2001 From: pyos Date: Tue, 8 Nov 2022 18:45:47 +0100 Subject: [PATCH] FIR DFA: slightly simplify {and,or}ForTypeStatements And use them less in boolean expression analysis. This uncovers a fun new case that doesn't work: interface I { val x: String } class C(override val x: String) : I class D(override val x: String) : I fun foo(x: Any?) { if (x is D || (x as C).x != "") { println(x.x.length) // actually OK (x is D || x is C => x is I) } } --- .../analysis/cfa/FirReturnsImpliesAnalyzer.kt | 8 ++- .../fir/resolve/dfa/FirDataFlowAnalyzer.kt | 52 ++++++++---------- .../kotlin/fir/resolve/dfa/LogicSystem.kt | 53 ++++++++----------- .../fir/resolve/dfa/PersistentLogicSystem.kt | 15 ++---- .../jetbrains/kotlin/fir/resolve/dfa/model.kt | 3 -- 5 files changed, 50 insertions(+), 81 deletions(-) diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/FirReturnsImpliesAnalyzer.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/FirReturnsImpliesAnalyzer.kt index ebb664f1dda..ffc2e51ad03 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/FirReturnsImpliesAnalyzer.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/FirReturnsImpliesAnalyzer.kt @@ -138,10 +138,8 @@ object FirReturnsImpliesAnalyzer : FirControlFlowChecker() { flow: PersistentFlow, statement: OperationStatement, builtinTypes: BuiltinTypes - ): MutableTypeStatements { - val newTypeStatements = flow.approvedTypeStatements.asMutableStatements() - approveStatementsTo(newTypeStatements, flow, statement, flow.logicStatements.flatMap { it.value }) - + ): TypeStatements { + val newTypeStatements = andForTypeStatements(flow.approvedTypeStatements, approveOperationStatement(flow, statement, null)) val variable = statement.variable if (!variable.isReal()) return newTypeStatements val extraStatement = when (statement.operation) { @@ -158,7 +156,7 @@ object FirReturnsImpliesAnalyzer : FirControlFlowChecker() { variableStorage: VariableStorageImpl, flow: Flow, context: CheckerContext - ): MutableTypeStatements? { + ): TypeStatements? { fun buildTypeStatements(arg: ConeValueParameterReference, exactType: Boolean, type: ConeKotlinType): MutableTypeStatements? { val parameterSymbol = function.getParameterSymbol(arg.parameterIndex, context) diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt index 4315a9e0eb1..e0dfda2e7b1 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt @@ -1232,51 +1232,41 @@ abstract class FirDataFlowAnalyzer( * TODO: Here we should handle case when one of arguments is dead (e.g. in cases `false && expr` or `true || expr`) * But since conditions with const are rare it can be delayed */ - val leftVariable = variableStorage.getOrCreateVariable(flow, binaryLogicExpression.leftOperand) val rightVariable = variableStorage.getOrCreateVariable(flow, binaryLogicExpression.rightOperand) val operatorVariable = variableStorage.getOrCreateVariable(flow, binaryLogicExpression) if (!node.leftOperandNode.isDead && node.rightOperandNode.isDead) { - /* - * If there was a jump from right argument then we know that we well exit from - * boolean operator only if right operand was not executed - * - * a && return => a == false - * a || return => a == true - */ + // If the right operand does not terminate, then we know that the value of the entire expression + // has to be `onlyLeftEvaluated`, and it has to be produced by the left operand. logicSystem.approveStatementsInsideFlow( flow, - leftVariable eq !isAnd, + leftVariable eq onlyLeftEvaluated, shouldForkFlow = false, shouldRemoveSynthetics = true ) } else { - val (conditionalFromLeft, conditionalFromRight, approvedFromRight) = logicSystem.collectInfoForBooleanOperator( - flowFromLeft, - leftVariable, - flowFromRight, - rightVariable - ) + val (conditionalFromLeft, conditionalFromRight, approvedFromRight) = + logicSystem.collectInfoForBooleanOperator(flowFromLeft, leftVariable, flowFromRight, rightVariable) - // left && right == True - // left || right == False - val approvedIfTrue: MutableTypeStatements = mutableMapOf() - logicSystem.approveStatementsTo(approvedIfTrue, flowFromRight, leftVariable eq bothEvaluated, conditionalFromLeft) - logicSystem.approveStatementsTo(approvedIfTrue, flowFromRight, rightVariable eq bothEvaluated, conditionalFromRight) - logicSystem.andForTypeStatements(approvedIfTrue, approvedFromRight).values.forEach { - flow.addImplication((operatorVariable eq bothEvaluated) implies it) + // 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), + ).forEach { statements -> + statements.values.forEach { flow.addImplication((operatorVariable eq bothEvaluated) implies it) } } - // left && right == False - // left || right == True - val approvedIfFalse: MutableTypeStatements = mutableMapOf() - val leftIsFalse = logicSystem.approveOperationStatement(flowFromLeft, leftVariable eq onlyLeftEvaluated, conditionalFromLeft) - val rightIsFalse = - logicSystem.approveOperationStatement(flowFromRight, rightVariable eq onlyLeftEvaluated, conditionalFromRight) - logicSystem.andForTypeStatements(approvedIfFalse, logicSystem.orForTypeStatements(leftIsFalse, rightIsFalse)).values.forEach { - flow.addImplication((operatorVariable eq onlyLeftEvaluated) implies it) - } + // 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. + // if (x is A || whatever(x as B)) { /* x is (A | B) */ } + logicSystem.approveOperationStatement(flowFromRight, rightVariable eq onlyLeftEvaluated, conditionalFromRight), + ).values.forEach { flow.addImplication((operatorVariable eq onlyLeftEvaluated) implies it) } } logicSystem.updateAllReceivers(flow) diff --git a/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/LogicSystem.kt b/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/LogicSystem.kt index 4e00e9a6ac8..9d53fdd4fbf 100644 --- a/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/LogicSystem.kt +++ b/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/LogicSystem.kt @@ -65,38 +65,37 @@ abstract class LogicSystem(protected val context: ConeInferenceCont rightVariable: DataFlowVariable, ): InfoForBooleanOperator - abstract fun approveStatementsTo( - destination: MutableTypeStatements, + abstract fun approveOperationStatement( flow: FLOW, approvedStatement: OperationStatement, - statements: Collection, - ) + statementsForVariable: Collection? + ): TypeStatements - fun orForTypeStatements(left: TypeStatements, right: TypeStatements): MutableTypeStatements { - if (left.isEmpty() || right.isEmpty()) return mutableMapOf() - val map = mutableMapOf() - for (variable in left.keys.intersect(right.keys)) { - val leftStatement = left.getValue(variable) - val rightStatement = right.getValue(variable) - map[variable] = or(listOf(leftStatement, rightStatement)) + fun orForTypeStatements(left: TypeStatements, right: TypeStatements): TypeStatements = when { + left.isEmpty() -> left + right.isEmpty() -> right + else -> buildMap { + for ((variable, leftStatement) in left) { + put(variable, or(listOf(leftStatement, right[variable] ?: continue))) + } } - return map } - fun andForTypeStatements(left: TypeStatements, right: TypeStatements): MutableTypeStatements { - if (left.isEmpty() && right.isEmpty()) return mutableMapOf() - val map = left.asMutableStatements() - for ((variable, rightStatement) in right) { - map[variable] = and(listOfNotNull(map[variable], rightStatement)) + fun andForTypeStatements(left: TypeStatements, right: TypeStatements): TypeStatements = when { + left.isEmpty() -> right + right.isEmpty() -> left + else -> left.toMutableMap().apply { + for ((variable, rightStatement) in right) { + put(variable, { rightStatement }, { and(listOf(it, rightStatement)) }) + } } - return map } // ------------------------------- Util functions ------------------------------- - private fun foldStatements(statements: Collection, all: Boolean): MutableTypeStatement { + private fun foldStatements(statements: Collection, all: Boolean): TypeStatement { require(statements.isNotEmpty()) - statements.singleOrNull()?.let { return it.asMutableStatement() } + statements.singleOrNull()?.let { return it } val variable = statements.first().variable assert(statements.all { it.variable == variable }) // TypeStatement(variable, exactType, exactNotType) = @@ -127,23 +126,13 @@ abstract class LogicSystem(protected val context: ConeInferenceCont }.takeIf { !onlyInputTypes || it in intersected } } - protected fun and(statements: Collection): MutableTypeStatement = + protected fun and(statements: Collection): TypeStatement = foldStatements(statements, all = true) - protected fun or(statements: Collection): MutableTypeStatement = + protected fun or(statements: Collection): TypeStatement = foldStatements(statements, all = false) } -fun LogicSystem.approveOperationStatement( - flow: FLOW, - approvedStatement: OperationStatement, - statements: Collection, -): MutableTypeStatements { - return mutableMapOf().apply { - approveStatementsTo(this, flow, approvedStatement, statements) - } -} - /* * used for: * 1. val b = x is String diff --git a/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/PersistentLogicSystem.kt b/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/PersistentLogicSystem.kt index 433057b2c8b..37474a8c2c1 100644 --- a/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/PersistentLogicSystem.kt +++ b/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/PersistentLogicSystem.kt @@ -339,18 +339,13 @@ abstract class PersistentLogicSystem(context: ConeInferenceContext) : LogicSyste } } - override fun approveStatementsTo( - destination: MutableTypeStatements, + override fun approveOperationStatement( flow: PersistentFlow, approvedStatement: OperationStatement, - statements: Collection - ) { - val approveOperationStatements = - approveOperationStatementsInternal(flow, approvedStatement, statements, shouldRemoveSynthetics = false) - approveOperationStatements.asMap().forEach { (variable, infos) -> - destination.put(variable, { and(infos) }, { and(listOf(it) + infos) }) - } - } + statementsForVariable: Collection? + ): TypeStatements = + approveOperationStatementsInternal(flow, approvedStatement, statementsForVariable, shouldRemoveSynthetics = false) + .asMap().mapValues { and(it.value) } override fun collectInfoForBooleanOperator( leftFlow: PersistentFlow, diff --git a/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/model.kt b/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/model.kt index c4cdc8024aa..26e551bc764 100644 --- a/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/model.kt +++ b/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/model.kt @@ -35,9 +35,6 @@ fun Implication.invertCondition(): Implication = Implication(condition.invert(), typealias TypeStatements = Map typealias MutableTypeStatements = MutableMap -fun TypeStatements.asMutableStatements(): MutableTypeStatements = - mapValuesTo(mutableMapOf()) { it.value.asMutableStatement() } - // --------------------------------------- DSL --------------------------------------- infix fun DataFlowVariable.eq(constant: Boolean?): OperationStatement {