From 99bebfa1838661527728769d27ba13926965ef8e Mon Sep 17 00:00:00 2001 From: pyos Date: Sun, 20 Nov 2022 19:45:22 +0100 Subject: [PATCH] FIR DFA: merge non-conflicting aliases from union flows E.g. after `f({ x = a }, { x })`, if `f` calls both lambdas in-place, `x` should be aliased to `a` even though only one path does that. --- .../resolve/cfg/flowFromTwoInplaceLambdas.dot | 64 +++++++------- .../cfg/flowFromTwoInplaceLambdas.fir.txt | 2 +- .../resolve/cfg/flowFromTwoInplaceLambdas.kt | 2 +- .../fir/resolve/dfa/PersistentLogicSystem.kt | 88 ++++++++++++------- 4 files changed, 91 insertions(+), 65 deletions(-) diff --git a/compiler/fir/analysis-tests/testData/resolve/cfg/flowFromTwoInplaceLambdas.dot b/compiler/fir/analysis-tests/testData/resolve/cfg/flowFromTwoInplaceLambdas.dot index bc990cbee27..5bfec9e0b9a 100644 --- a/compiler/fir/analysis-tests/testData/resolve/cfg/flowFromTwoInplaceLambdas.dot +++ b/compiler/fir/analysis-tests/testData/resolve/cfg/flowFromTwoInplaceLambdas.dot @@ -772,29 +772,29 @@ digraph flowFromTwoInplaceLambdas_kt { 281 [label="Postponed enter to lambda"]; subgraph cluster_56 { color=blue - 302 [label="Enter function anonymousFunction" style="filled" fillcolor=red]; + 303 [label="Enter function anonymousFunction" style="filled" fillcolor=red]; subgraph cluster_57 { color=blue - 303 [label="Enter block"]; - 304 [label="Access variable R|/x|"]; - 305 [label="Assignment: R|/y|"]; - 306 [label="Exit block"]; + 304 [label="Enter block"]; + 305 [label="Access variable R|/x|"]; + 306 [label="Assignment: R|/y|"]; + 307 [label="Exit block"]; } - 307 [label="Exit function anonymousFunction" style="filled" fillcolor=red]; + 308 [label="Exit function anonymousFunction" style="filled" fillcolor=red]; } 283 [label="Postponed exit from lambda"]; 284 [label="Postponed enter to lambda"]; subgraph cluster_58 { color=blue - 308 [label="Enter function anonymousFunction" style="filled" fillcolor=red]; + 309 [label="Enter function anonymousFunction" style="filled" fillcolor=red]; subgraph cluster_59 { color=blue - 309 [label="Enter block"]; - 310 [label="Jump: ^@run2 Unit"]; - 311 [label="Stub" style="filled" fillcolor=gray]; - 312 [label="Exit block" style="filled" fillcolor=gray]; + 310 [label="Enter block"]; + 311 [label="Jump: ^@run2 Unit"]; + 312 [label="Stub" style="filled" fillcolor=gray]; + 313 [label="Exit block" style="filled" fillcolor=gray]; } - 313 [label="Exit function anonymousFunction" style="filled" fillcolor=red]; + 314 [label="Exit function anonymousFunction" style="filled" fillcolor=red]; } 282 [label="Call arguments union" style="filled" fillcolor=yellow]; 285 [label="Postponed exit from lambda"]; @@ -815,29 +815,30 @@ digraph flowFromTwoInplaceLambdas_kt { color=blue 294 [label="Enter block"]; 295 [label="Access variable R|/x|"]; - 296 [label="Access variable #"]; - 297 [label="Exit block"]; + 296 [label="Smart cast: R|/x|"]; + 297 [label="Access variable R|kotlin/String.length|"]; + 298 [label="Exit block"]; } - 298 [label="Exit when branch result"]; - 299 [label="Exit when"]; + 299 [label="Exit when branch result"]; + 300 [label="Exit when"]; } - 300 [label="Exit block"]; + 301 [label="Exit block"]; } - 301 [label="Exit function test7" style="filled" fillcolor=red]; + 302 [label="Exit function test7" style="filled" fillcolor=red]; } 276 -> {277}; 277 -> {278}; 278 -> {279}; 279 -> {280}; 280 -> {281}; - 281 -> {302}; + 281 -> {303}; 281 -> {283} [color=red]; - 281 -> {302} [style=dashed]; + 281 -> {303} [style=dashed]; 282 -> {286} [color=red]; 283 -> {284}; - 284 -> {308}; + 284 -> {309}; 284 -> {285} [color=red]; - 284 -> {308} [style=dashed]; + 284 -> {309} [style=dashed]; 285 -> {286} [color=green]; 286 -> {287}; 287 -> {288}; @@ -845,7 +846,7 @@ digraph flowFromTwoInplaceLambdas_kt { 289 -> {290}; 290 -> {291}; 291 -> {293 292}; - 292 -> {299}; + 292 -> {300}; 293 -> {294}; 294 -> {295}; 295 -> {296}; @@ -854,20 +855,21 @@ digraph flowFromTwoInplaceLambdas_kt { 298 -> {299}; 299 -> {300}; 300 -> {301}; - 302 -> {303}; + 301 -> {302}; 303 -> {304}; 304 -> {305}; 305 -> {306}; 306 -> {307}; - 307 -> {282} [color=red]; - 307 -> {283} [color=green]; - 308 -> {309}; + 307 -> {308}; + 308 -> {282} [color=red]; + 308 -> {283} [color=green]; 309 -> {310}; - 310 -> {313}; - 310 -> {311} [style=dotted]; + 310 -> {311}; + 311 -> {314}; 311 -> {312} [style=dotted]; 312 -> {313} [style=dotted]; - 313 -> {282} [color=red]; - 313 -> {285} [color=green]; + 313 -> {314} [style=dotted]; + 314 -> {282} [color=red]; + 314 -> {285} [color=green]; } diff --git a/compiler/fir/analysis-tests/testData/resolve/cfg/flowFromTwoInplaceLambdas.fir.txt b/compiler/fir/analysis-tests/testData/resolve/cfg/flowFromTwoInplaceLambdas.fir.txt index a2e7e2fc301..940bd1f1f13 100644 --- a/compiler/fir/analysis-tests/testData/resolve/cfg/flowFromTwoInplaceLambdas.fir.txt +++ b/compiler/fir/analysis-tests/testData/resolve/cfg/flowFromTwoInplaceLambdas.fir.txt @@ -145,7 +145,7 @@ FILE: flowFromTwoInplaceLambdas.kt ) when () { (R|/y| is R|kotlin/String|) -> { - R|/x|.# + R|/x|.R|kotlin/String.length| } } diff --git a/compiler/fir/analysis-tests/testData/resolve/cfg/flowFromTwoInplaceLambdas.kt b/compiler/fir/analysis-tests/testData/resolve/cfg/flowFromTwoInplaceLambdas.kt index 6fd04d5ac4c..d89dea029fc 100644 --- a/compiler/fir/analysis-tests/testData/resolve/cfg/flowFromTwoInplaceLambdas.kt +++ b/compiler/fir/analysis-tests/testData/resolve/cfg/flowFromTwoInplaceLambdas.kt @@ -84,6 +84,6 @@ fun test7() { val y: Any? run2({ y = x }, { }) if (y is String) { - x.length // ok - aliased + x.length // ok - aliased } } 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 1ba5481d480..4bbfe1fac48 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 @@ -27,12 +27,23 @@ abstract class PersistentLogicSystem(context: ConeInferenceContext) : LogicSyste a.lowestCommonAncestor(b) ?: error("no common ancestor in $a, $b") } val result = commonFlow.fork() + result.mergeAssignments(flows) + if (union) { + result.copyNonConflictingAliases(flows, commonFlow) + } else { + result.copyCommonAliases(flows) + } + result.copyStatements(flows, commonFlow, union) + // TODO: compute common implications? + return result + } + private fun MutableFlow.mergeAssignments(flows: Collection) { // If a variable was reassigned in one branch, it was reassigned at the join point. val reassignedVariables = mutableMapOf() for (flow in flows) { for ((variable, index) in flow.assignmentIndex) { - if (result.assignmentIndex[variable] != index) { + if (assignmentIndex[variable] != index) { // Ideally we should generate an entirely new index here, but it doesn't really // matter; the important part is that it's different from `commonFlow.previousFlow`. reassignedVariables[variable] = max(index, reassignedVariables[variable] ?: 0) @@ -40,27 +51,49 @@ abstract class PersistentLogicSystem(context: ConeInferenceContext) : LogicSyste } } for ((variable, index) in reassignedVariables) { - recordNewAssignment(result, variable, index) + recordNewAssignment(this, variable, index) } + } - // TODO: if `union`, then all aliases from all flows are valid so long as other flows don't have a contradicting one + private fun MutableFlow.copyCommonAliases(flows: Collection) { for ((from, to) in flows.first().directAliasMap) { - // If `from -> to` is still in `result` (was not removed by the above code), then it is also in all `flows`, + // If `from -> to` is still in `this` (was not removed by the above code), then it is also in all `flows`, // as the only way to break aliasing is by reassignment. - if (result.directAliasMap[from] != to && flows.all { it.unwrapVariable(from) == to }) { + if (directAliasMap[from] != to && flows.all { it.unwrapVariable(from) == to }) { // if (p) { y = x } else { y = x } <-- after `if`, `y -> x` is in all `flows`, but not in `result` // (which was forked from the flow before the `if`) - addLocalVariableAlias(result, from, to) + addLocalVariableAlias(this, from, to) } } + } + private fun MutableFlow.copyNonConflictingAliases(flows: Collection, commonFlow: PersistentFlow) { + val candidates = mutableMapOf() + for (flow in flows) { + for ((from, to) in flow.directAliasMap) { + candidates[from] = when { + // f({ a = b }, { notReassigning() }) -> a = b + commonFlow.assignmentIndex[from] == flow.assignmentIndex[from] -> continue + // f({ a = b }, { a = c }) -> a = b or c; can't express that, so just don't + from in candidates && candidates[from] != to -> null + // f({ a = b }, { a = b }) -> a = b + else -> to + } + } + } + for ((from, to) in candidates) { + addLocalVariableAlias(this, from, to ?: continue) + } + } + + private fun MutableFlow.copyStatements(flows: Collection, commonFlow: PersistentFlow, union: Boolean) { flows.flatMapTo(mutableSetOf()) { it.knownVariables }.forEach computeStatement@{ variable -> - val statement = if (variable in result.directAliasMap) { + val statement = if (variable in directAliasMap) { return@computeStatement // statements about alias == statements about aliased variable } else if (!union) { // if (condition) { /* x: S1 */ } else { /* x: S2 */ } // -> x: S1 | S2 or(flows.mapTo(mutableSetOf()) { it.getTypeStatement(variable) ?: return@computeStatement }) - } else if (variable !in reassignedVariables) { + } else if (assignmentIndex[variable] == commonFlow.assignmentIndex[variable]) { // callAllInSomeOrder({ /* x: S1 */ }, { /* x: S2 */ }) // -> x: S1 & S2 and(flows.mapNotNullTo(mutableSetOf()) { it.getTypeStatement(variable) }) } else { @@ -78,19 +111,19 @@ abstract class PersistentLogicSystem(context: ConeInferenceContext) : LogicSyste or(byAssignment.values.mapTo(mutableSetOf()) { and(it.filterNotNull()) ?: return@computeStatement }) } if (statement?.isNotEmpty == true) { - result.approvedTypeStatements[variable] = statement.toPersistent() + approvedTypeStatements[variable] = statement.toPersistent() } } - // TODO: compute common implications? - return result } override fun addLocalVariableAlias(flow: MutableFlow, alias: RealVariable, underlyingVariable: RealVariable) { - flow.addAliases(persistentSetOf(alias), flow.unwrapVariable(underlyingVariable)) + if (underlyingVariable == alias) return // x = x + flow.directAliasMap[alias] = underlyingVariable + flow.backwardsAliasMap[underlyingVariable] = flow.backwardsAliasMap[underlyingVariable]?.add(alias) ?: persistentSetOf(alias) } private fun MutableFlow.replaceVariable(variable: RealVariable, replacement: RealVariable?) { - val original = directAliasMap[variable] + val original = directAliasMap.remove(variable) if (original != null) { // All statements should've been made about whatever variable this is an alias to. There is nothing to replace. if (AbstractTypeChecker.RUN_SLOW_ASSERTIONS) { @@ -100,11 +133,9 @@ abstract class PersistentLogicSystem(context: ConeInferenceContext) : LogicSyste } // `variable.dependentVariables` is not separated by flow, so it may be non-empty if aliasing of this variable // was broken in another flow. However, in *this* flow dependent variables should have no information attached to them. - - val siblings = backwardsAliasMap.getValue(original).remove(variable) - directAliasMap.remove(variable) - if (siblings.isNotEmpty()) { - backwardsAliasMap[original] = siblings + val siblings = backwardsAliasMap.getValue(original) + if (siblings.size > 1) { + backwardsAliasMap[original] = siblings.remove(variable) } else { backwardsAliasMap.remove(original) } @@ -112,7 +143,7 @@ abstract class PersistentLogicSystem(context: ConeInferenceContext) : LogicSyste addLocalVariableAlias(this, replacement, original) } } else { - val aliases = backwardsAliasMap[variable] + val aliases = backwardsAliasMap.remove(variable) // If asked to remove the variable but there are aliases, replace with a new representative for the alias group instead. val replacementOrNext = replacement ?: aliases?.first() for (dependent in variable.dependentVariables) { @@ -122,24 +153,17 @@ abstract class PersistentLogicSystem(context: ConeInferenceContext) : LogicSyste } logicStatements.replaceVariable(variable, replacementOrNext) approvedTypeStatements.replaceVariable(variable, replacementOrNext) - if (aliases != null) { - backwardsAliasMap -= variable - if (replacementOrNext != null) { - directAliasMap -= replacementOrNext - addAliases(aliases, replacementOrNext) + if (aliases != null && replacementOrNext != null) { + directAliasMap -= replacementOrNext + val withoutSelf = aliases - replacementOrNext + if (withoutSelf.isNotEmpty()) { + withoutSelf.associateWithTo(directAliasMap) { replacementOrNext } + backwardsAliasMap[replacementOrNext] = backwardsAliasMap[replacementOrNext]?.addAll(withoutSelf) ?: withoutSelf } } } } - private fun MutableFlow.addAliases(aliases: PersistentSet, target: RealVariable) { - val withoutSelf = aliases - target - if (withoutSelf.isNotEmpty()) { - withoutSelf.associateWithTo(directAliasMap) { target } - backwardsAliasMap[target] = backwardsAliasMap[target]?.addAll(withoutSelf) ?: withoutSelf - } - } - override fun addTypeStatement(flow: MutableFlow, statement: TypeStatement): TypeStatement? { if (statement.exactType.isEmpty()) return null val variable = statement.variable