From 639a454d6cf3064a4977db56b80108e4e50ab0d8 Mon Sep 17 00:00:00 2001 From: pyos Date: Tue, 1 Nov 2022 14:31:52 +0100 Subject: [PATCH] FIR DFA: when joining flows, clear reassigned vars with no statements For example: val c = C("...") val x = c.x // alias to variable for c.x, which depends on c if (x == null) return // now c.x has type Any c = C(null) // c has been reassigned => info for c.x is no longer valid; // c itself has never had any statements made about it, but // we must still call removeAllAboutVariable to clear the // dependents --- .../kotlin/fir/resolve/dfa/LogicSystem.kt | 3 +- .../fir/resolve/dfa/PersistentLogicSystem.kt | 91 +++++++------------ .../variables/reassignedDependency.fir.kt | 2 +- 3 files changed, 35 insertions(+), 61 deletions(-) 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 12382246a6f..1b54777a95b 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 @@ -22,8 +22,7 @@ abstract class LogicSystem(protected val context: ConeInferenceCont abstract fun addImplication(flow: FLOW, implication: Implication) - fun removeAllAboutVariable(flow: FLOW, variable: RealVariable?) { - if (variable == null) return + fun removeAllAboutVariable(flow: FLOW, variable: RealVariable) { removeAliasInformationAboutVariable(flow, variable) removeTypeStatementsAboutVariable(flow, variable) removeLogicStatementsAboutVariable(flow, variable) 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 b95c6643b5d..0f992a9eb3b 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 @@ -108,10 +108,15 @@ abstract class PersistentLogicSystem(context: ConeInferenceContext) : LogicSyste } override fun joinFlow(flows: Collection): PersistentFlow { + // One input flow executes - one set of statements is true, others might be false. return foldFlow(flows) { variable -> or(flows.map { it.getApprovedTypeStatements(variable) }).takeIf { it.isNotEmpty } } } override fun unionFlow(flows: Collection): PersistentFlow { + // All input flows execute in some order. If none of them reassign the variable, then + // *all* sets of statements are true; otherwise the assignments might have happened in + // an arbitrary order, so only one set of statements is true depending on which assignment + // happened last (and the flows that don't reassign may or may not have executed after that). return foldFlow(flows) { variable -> or(flows.groupBy { it.assignmentIndex[variable] ?: -1 }.values.map { flowSubset -> and(flowSubset.map { it.getApprovedTypeStatements(variable) }) @@ -119,45 +124,39 @@ abstract class PersistentLogicSystem(context: ConeInferenceContext) : LogicSyste } } - private inline fun foldFlow( - flows: Collection, - mergeOperation: (RealVariable) -> MutableTypeStatement?, - ): PersistentFlow { - if (flows.isEmpty()) return createEmptyFlow() - flows.singleOrNull()?.let { return it } + private inline fun foldFlow(flows: Collection, mergeOperation: (RealVariable) -> TypeStatement?): PersistentFlow = + when (flows.size) { + 0 -> createEmptyFlow() + 1 -> flows.first() + else -> foldFlow(flows, flows.flatMapTo(mutableSetOf()) { it.approvedTypeStatements.keys }.mapNotNull(mergeOperation)) + } + private fun foldFlow(flows: Collection, statements: Collection): PersistentFlow { val aliasedVariablesThatDontChangeAlias = computeAliasesThatDontChange(flows) val commonFlow = flows.reduce(::lowestCommonFlow) - val variables = flows.flatMap { it.approvedTypeStatements.keys }.toSet() - for (variable in variables) { - val info = mergeOperation(variable) ?: continue - commonFlow.approvedTypeStatements -= variable - commonFlow.approvedTypeStatementsDiff -= variable - val thereWereReassignments = variable.hasDifferentReassignments(flows) - if (thereWereReassignments) { - removeAllAboutVariable(commonFlow, variable) + for (flow in flows) { + for ((variable, index) in flow.assignmentIndex) { + if (commonFlow.assignmentIndex[variable] != index) { + removeAllAboutVariable(commonFlow, variable) + } } - commonFlow.addApprovedStatements(info) } - commonFlow.addVariableAliases(aliasedVariablesThatDontChangeAlias) + val toReplace = statements.map { it.variable to it.toPersistent() } + commonFlow.approvedTypeStatements += toReplace + if (commonFlow.previousFlow != null) { + commonFlow.approvedTypeStatementsDiff += toReplace + } + + for ((alias, underlyingVariable) in aliasedVariablesThatDontChangeAlias) { + addLocalVariableAlias(commonFlow, alias, underlyingVariable) + } return commonFlow } - private fun RealVariable.hasDifferentReassignments(flows: Collection): Boolean { - val firstIndex = flows.first().assignmentIndex[this] ?: -1 - for (flow in flows) { - val index = flow.assignmentIndex[this] ?: -1 - if (index != firstIndex) return true - } - return false - } - - private fun computeAliasesThatDontChange( - flows: Collection - ): MutableMap { + private fun computeAliasesThatDontChange(flows: Collection): MutableMap { val flowsSize = flows.size val aliasedVariablesThatDontChangeAlias = mutableMapOf() @@ -173,23 +172,6 @@ abstract class PersistentLogicSystem(context: ConeInferenceContext) : LogicSyste return aliasedVariablesThatDontChangeAlias } - private fun PersistentFlow.addVariableAliases( - aliasedVariablesThatDontChangeAlias: MutableMap - ) { - for ((alias, underlyingVariable) in aliasedVariablesThatDontChangeAlias) { - addLocalVariableAlias(this, alias, underlyingVariable) - } - } - - private fun PersistentFlow.addApprovedStatements( - info: MutableTypeStatement - ) { - approvedTypeStatements = approvedTypeStatements.addTypeStatement(info) - if (previousFlow != null) { - approvedTypeStatementsDiff = approvedTypeStatementsDiff.addTypeStatement(info) - } - } - override fun addLocalVariableAlias(flow: PersistentFlow, alias: RealVariable, underlyingVariable: RealVariableAndType) { removeLocalVariableAlias(flow, alias) flow.directAliasMap = flow.directAliasMap.put(alias, underlyingVariable) @@ -527,22 +509,15 @@ private fun lowestCommonFlow(left: PersistentFlow, right: PersistentFlow): Persi private fun PersistentApprovedTypeStatements.addTypeStatement(info: TypeStatement): PersistentApprovedTypeStatements { val variable = info.variable val existingInfo = this[variable] - return if (existingInfo == null) { - val persistentInfo = if (info is PersistentTypeStatement) info else info.toPersistent() - put(variable, persistentInfo) - } else { - put(variable, existingInfo + info) - } + return put(variable, if (existingInfo != null) existingInfo + info else info.toPersistent()) } -private fun TypeStatement.toPersistent(): PersistentTypeStatement = PersistentTypeStatement( - variable, - exactType.toPersistentSet(), - exactNotType.toPersistentSet() -) +private fun TypeStatement.toPersistent(): PersistentTypeStatement = when (this) { + is PersistentTypeStatement -> this + else -> PersistentTypeStatement(variable, exactType.toPersistentSet(), exactNotType.toPersistentSet()) +} fun TypeStatement.asMutableStatement(): MutableTypeStatement = when (this) { is MutableTypeStatement -> this - is PersistentTypeStatement -> MutableTypeStatement(variable, exactType.toMutableSet(), exactNotType.toMutableSet()) - else -> throw IllegalArgumentException("Unknown TypeStatement type: ${this::class}") + else -> MutableTypeStatement(variable, exactType.toMutableSet(), exactNotType.toMutableSet()) } diff --git a/compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency.fir.kt b/compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency.fir.kt index 3e7b11953f5..0793b9ab038 100644 --- a/compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency.fir.kt +++ b/compiler/testData/diagnostics/tests/smartCasts/variables/reassignedDependency.fir.kt @@ -36,5 +36,5 @@ fun test3(p: Boolean) { c = C(null) } x.length // ok - c.x.length // bad + c.x.length // bad }