FIR CFA: remove CFGNode.outgoingEdges

If it's symmetric with incomingEdges, then what's the point?..
2022-11-27 22:51:22 +01:00
parent 7feeb7cd4e
commit fa0ea1504e
10 changed files with 35 additions and 78 deletions
@@ -72,8 +72,8 @@ private fun <I> ControlFlowGraph.collectDataForNodeInternal(
        val previousData =
            previousNodes.mapNotNull {
                val k = when (direction) {
-                    TraverseDirection.Forward -> node.incomingEdges[it]?.label ?: NormalPath
-                    TraverseDirection.Backward -> node.outgoingEdges[it]?.label ?: NormalPath
+                    TraverseDirection.Forward -> node.edgeFrom(it).label
+                    TraverseDirection.Backward -> node.edgeTo(it).label
                }
                val v = nodeMap[it] ?: return@mapNotNull null
                k to v
@@ -26,7 +26,7 @@ fun CFGNode<*>.isEnterNode(direction: TraverseDirection): Boolean = when (direct

 val CFGNode<*>.previousCfgNodes: List<CFGNode<*>>
    get() = previousNodes.filter {
-        val kind = incomingEdges.getValue(it).kind
+        val kind = edgeFrom(it).kind
        if (this.isDead) {
            kind.usedInCfa
        } else {
@@ -39,7 +39,7 @@ val CFGNode<*>.followingCfgNodes: List<CFGNode<*>>
        val nodes = mutableListOf<CFGNode<*>>()

        followingNodes.filterTo(nodes) {
-            val kind = outgoingEdges.getValue(it).kind
+            val kind = edgeTo(it).kind
            kind.usedInCfa && !kind.isDead
        }
        (this as? CFGNodeWithSubgraphs<*>)?.subGraphs?.mapTo(nodes) { it.enterNode }
@@ -232,7 +232,7 @@ object FirMemberPropertiesChecker : FirClassChecker() {
            else -> null
        }
        val cfg = controlFlowGraphReference?.controlFlowGraph ?: return emptySet()
-        return cfg.exitNode.incomingEdges.keys
+        return cfg.exitNode.previousNodes
            .map { it.fir }
            .filter { it.isDeadEnd() }
            .toSet()
@@ -246,10 +246,6 @@ object FirMemberPropertiesChecker : FirClassChecker() {
     */
    private fun FirElement.isDeadEnd(): Boolean {
        val cfg = (this as? FirControlFlowGraphOwner)?.controlFlowGraphReference?.controlFlowGraph ?: return false
-        cfg.exitNode.incomingEdges.keys.find { it is BlockExitNode }?.let {
-            return it.isDead
-        }
-
-        return cfg.exitNode.incomingEdges.keys.any { it.isDead }
+        return cfg.exitNode.previousNodes.find { it is BlockExitNode }?.isDead ?: cfg.exitNode.previousNodes.any { it.isDead }
    }
 }
@@ -100,7 +100,7 @@ object FirTailrecFunctionChecker : FirSimpleFunctionChecker() {

    private fun CFGNode<*>.hasMoreFollowingInstructions(tailrecFunction: FirSimpleFunction): Boolean {
        for (next in followingNodes) {
-            val edge = outgoingEdges.getValue(next)
+            val edge = edgeTo(next)
            if (!edge.kind.usedInCfa || edge.kind.isDead) continue
            if (edge.kind.isBack) return true
            val hasMore = when (next) {
@@ -1128,7 +1128,7 @@ abstract class FirDataFlowAnalyzer(
    // In that case `mergeIncomingFlow` will automatically ensure consistency once called on that node.
    private fun CFGNode<*>.buildIncomingFlow(): MutableFlow {
        val previousFlows = previousNodes.mapNotNull {
-            val incomingEdgeKind = incomingEdges.getValue(it).kind
+            val incomingEdgeKind = edgeFrom(it).kind
            if (if (isDead) !incomingEdgeKind.usedInDeadDfa else !incomingEdgeKind.usedInDfa) return@mapNotNull null
            // `MergePostponedLambdaExitsNode` nodes form a parallel data flow graph. We never compute
            // data flow for any of them until reaching a completed call.
@@ -114,7 +114,7 @@ class ControlFlowGraphBuilder {
        }

        val returnValues = exitNode.previousNodes.mapNotNullTo(mutableSetOf()) {
-            val edge = exitNode.incomingEdges.getValue(it)
+            val edge = exitNode.edgeFrom(it)
            // * NormalPath: last expression = return value
            // * UncaughtExceptionPath: last expression = whatever threw, *not* a return value
            // * ReturnPath(this lambda): these go from `finally` blocks, so that's not the return value;
@@ -1009,7 +1009,7 @@ class ControlFlowGraphBuilder {
        val exitNode = createFinallyBlockExitNode(enterNode.fir)
        popAndAddEdge(exitNode)
        val allNormalInputsAreDead = enterNode.previousNodes.all {
-            val edge = enterNode.incomingEdges.getValue(it)
+            val edge = enterNode.edgeFrom(it)
            edge.kind.isDead || edge.label != NormalPath
        }
        addEdge(exitNode, tryExitNode, isDead = allNormalInputsAreDead)
@@ -1087,7 +1087,7 @@ class ControlFlowGraphBuilder {
    private fun completeFunctionCall(node: FunctionCallNode) {
        if (!node.fir.resultType.isNothing) return
        val stub = withLevelOfNode(node) { createStubNode() }
-        val edges = node.followingNodes.map { it to node.outgoingEdges.getValue(it) }
+        val edges = node.followingNodes.map { it to node.edgeTo(it) }
        CFGNode.removeAllOutgoingEdges(node)
        CFGNode.addEdge(node, stub, EdgeKind.DeadForward, propagateDeadness = false)
        for ((to, edge) in edges) {
@@ -1448,7 +1448,7 @@ class ControlFlowGraphBuilder {
    private fun propagateDeadnessForward(node: CFGNode<*>) {
        if (!node.isDead) return
        for (next in node.followingNodes) {
-            val kind = node.outgoingEdges.getValue(next).kind
+            val kind = node.edgeTo(next).kind
            if (CFGNode.killEdge(node, next, propagateDeadness = false) && !kind.isBack && kind.usedInCfa) {
                next.updateDeadStatus()
                propagateDeadnessForward(next)
@@ -122,7 +122,7 @@ class FirControlFlowGraphRenderVisitor(

    private fun Printer.renderEdges(graph: ControlFlowGraph) {
        for (node in graph.nodes) {
-            for ((style, group) in node.followingNodes.groupBy { node.outgoingEdges.getValue(it).style }.entries.sortedBy { it.key }) {
+            for ((style, group) in node.followingNodes.groupBy { node.edgeTo(it).style }.entries.sortedBy { it.key }) {
                val mappedGroup = group.map { indices.getValue(it) }.sorted()
                print(indices.getValue(node), EDGE, mappedGroup.joinToString(prefix = "{", postfix = "}", separator = " "))
                style?.let { printWithNoIndent(" $it") }
@@ -36,9 +36,7 @@ sealed class CFGNode<out E : FirElement>(val owner: ControlFlowGraph, val level:
            from._followingNodes += to
            to._previousNodes += from
            if (kind != EdgeKind.Forward || label != NormalPath) {
-                val edge = Edge.create(label, kind)
-                from.insertOutgoingEdge(to, edge)
-                to.insertIncomingEdge(from, edge)
+                to.insertIncomingEdge(from, Edge.create(label, kind))
            }
            if (propagateDeadness && kind == EdgeKind.DeadForward) {
                to.isDead = true
@@ -47,11 +45,9 @@ sealed class CFGNode<out E : FirElement>(val owner: ControlFlowGraph, val level:

        @CfgInternals
        fun killEdge(from: CFGNode<*>, to: CFGNode<*>, propagateDeadness: Boolean): Boolean {
-            val oldEdge = to.incomingEdges.getValue(from)
+            val oldEdge = to.edgeFrom(from)
            if (oldEdge.kind.isDead) return false
-
            val newEdge = Edge.create(oldEdge.label, if (oldEdge.kind.isBack) EdgeKind.DeadBackward else EdgeKind.DeadForward)
-            from.insertOutgoingEdge(to, newEdge)
            to.insertIncomingEdge(from, newEdge)
            if (propagateDeadness) {
                to.isDead = true
@@ -63,7 +59,6 @@ sealed class CFGNode<out E : FirElement>(val owner: ControlFlowGraph, val level:
        fun removeAllIncomingEdges(to: CFGNode<*>) {
            for (from in to._previousNodes) {
                from._followingNodes.remove(to)
-                from._outgoingEdges?.remove(to)
            }
            to._incomingEdges = null
            to._previousNodes.clear()
@@ -75,12 +70,8 @@ sealed class CFGNode<out E : FirElement>(val owner: ControlFlowGraph, val level:
                to._previousNodes.remove(from)
                to._incomingEdges?.remove(from)
            }
-            from._outgoingEdges = null
            from._followingNodes.clear()
        }
-
-        private fun createEmptyEdgeMapWithDefault(): MutableMap<CFGNode<*>, Edge> =
-            mutableMapOf<CFGNode<*>, Edge>().withDefault { Edge.Normal_Forward }
    }

    init {
@@ -95,33 +86,18 @@ sealed class CFGNode<out E : FirElement>(val owner: ControlFlowGraph, val level:
    val followingNodes: List<CFGNode<*>> get() = _followingNodes

    private var _incomingEdges: MutableMap<CFGNode<*>, Edge>? = null
-    private var _outgoingEdges: MutableMap<CFGNode<*>, Edge>? = null

-    private fun insertIncomingEdge(to: CFGNode<*>, edge: Edge) {
-        if (_incomingEdges == null) {
-            _incomingEdges = createEmptyEdgeMapWithDefault()
+    private fun insertIncomingEdge(from: CFGNode<*>, edge: Edge) {
+        val map = _incomingEdges
+        if (map != null) {
+            map[from] = edge
+        } else {
+            _incomingEdges = mutableMapOf(from to edge)
        }
-        _incomingEdges?.put(to, edge)
    }

-    private fun insertOutgoingEdge(to: CFGNode<*>, edge: Edge) {
-        if (_outgoingEdges == null) {
-            _outgoingEdges = createEmptyEdgeMapWithDefault()
-        }
-        _outgoingEdges?.put(to, edge)
-    }
-
-    // Note: may be it should be made MapWithImplicitDefault and override getOrImplicitDefault and not get
-    // However, it works even in the current form
-    private object EmptyEdgesMap : AbstractMap<CFGNode<*>, Edge>() {
-        override val entries: Set<Map.Entry<CFGNode<*>, Edge>>
-            get() = emptySet()
-
-        override fun get(key: CFGNode<*>): Edge = Edge.Normal_Forward
-    }
-
-    val incomingEdges: Map<CFGNode<*>, Edge> get() = _incomingEdges ?: EmptyEdgesMap
-    val outgoingEdges: Map<CFGNode<*>, Edge> get() = _outgoingEdges ?: EmptyEdgesMap
+    fun edgeFrom(other: CFGNode<*>) = _incomingEdges?.get(other) ?: Edge.Normal_Forward
+    fun edgeTo(other: CFGNode<*>) = other.edgeFrom(this)

    abstract val fir: E
    var isDead: Boolean = false
@@ -139,13 +115,9 @@ sealed class CFGNode<out E : FirElement>(val owner: ControlFlowGraph, val level:
    @CfgInternals
    fun updateDeadStatus() {
        isDead = if (this is UnionNodeMarker)
-            incomingEdges.values.any {
-                it.kind == EdgeKind.DeadForward
-            }
+            _incomingEdges?.let { map -> map.values.any { it.kind.isDead } } == true
        else
-            incomingEdges.size == previousNodes.size && incomingEdges.values.all {
-                it.kind == EdgeKind.DeadForward || !it.kind.usedInCfa
-            }
+            _incomingEdges?.let { map -> map.size == previousNodes.size && map.values.all { it.kind.isDead || !it.kind.usedInCfa } } == true
    }

    abstract fun <R, D> accept(visitor: ControlFlowGraphVisitor<R, D>, data: D): R
@@ -176,7 +176,7 @@ private fun ControlFlowGraph.orderNodes(): LinkedHashSet<CFGNode<*>> {
    while (stack.isNotEmpty()) {
        val node = stack.removeFirst()
        val previousNodes = node.previousNodes
-        if (previousNodes.any { it !in visitedNodes && it.owner == this && !node.incomingEdges.getValue(it).kind.isBack }) {
+        if (previousNodes.any { it !in visitedNodes && it.owner == this && !node.edgeFrom(it).kind.isBack }) {
            stack.addLast(node)
            continue
        }
@@ -9,7 +9,6 @@ import org.jetbrains.kotlin.fir.references.FirControlFlowGraphReference
 import org.jetbrains.kotlin.fir.resolve.dfa.FirControlFlowGraphReferenceImpl
 import org.jetbrains.kotlin.fir.resolve.dfa.cfg.CFGNode
 import org.jetbrains.kotlin.fir.resolve.dfa.cfg.ControlFlowGraph
-import org.jetbrains.kotlin.fir.resolve.dfa.cfg.EdgeKind
 import org.jetbrains.kotlin.fir.visitors.FirVisitorVoid
 import org.jetbrains.kotlin.test.Assertions

@@ -27,28 +26,18 @@ class FirCfgConsistencyChecker(private val assertions: Assertions) : FirVisitorV

    private fun checkConsistency(graph: ControlFlowGraph) {
        for (node in graph.nodes) {
+            assertions.assertEquals(node.followingNodes.size, node.followingNodes.toSet().size) { "followingNodes has repeats: $node" }
+            assertions.assertEquals(node.previousNodes.size, node.previousNodes.toSet().size) { "previousNodes has repeats: $node" }
            for (to in node.followingNodes) {
-                checkEdge(node, to)
+                assertions.assertContainsElements(to.previousNodes, node)
+                assertions.assertFalse(node.isDead && to.isDead && to.edgeFrom(node).kind.usedInDfa) {
+                    "data flow between dead nodes: $node -> $to"
+                }
            }
            for (from in node.previousNodes) {
-                checkEdge(from, node)
+                assertions.assertContainsElements(from.followingNodes, node)
            }
-            if (node.followingNodes.isEmpty() && node.previousNodes.isEmpty()) {
-                throw AssertionError("Unconnected CFG node: $node")
-            }
-        }
-    }
-
-    private val cfgKinds = listOf(EdgeKind.DeadForward, EdgeKind.CfgForward, EdgeKind.DeadBackward, EdgeKind.CfgBackward)
-
-    private fun checkEdge(from: CFGNode<*>, to: CFGNode<*>) {
-        assertions.assertContainsElements(from.followingNodes, to)
-        assertions.assertContainsElements(to.previousNodes, from)
-        val fromKind = from.outgoingEdges.getValue(to).kind
-        val toKind = to.incomingEdges.getValue(from).kind
-        assertions.assertEquals(fromKind, toKind)
-        if (from.isDead && to.isDead) {
-            assertions.assertContainsElements(cfgKinds, toKind)
+            assertions.assertFalse(node.followingNodes.isEmpty() && node.previousNodes.isEmpty()) { "Unconnected CFG node: $node" }
        }
    }

@@ -57,7 +46,7 @@ class FirCfgConsistencyChecker(private val assertions: Assertions) : FirVisitorV
        for (node in graph.nodes) {
            for (previousNode in node.previousNodes) {
                if (previousNode.owner != graph) continue
-                if (!node.incomingEdges.getValue(previousNode).kind.isBack) {
+                if (!node.edgeFrom(previousNode).kind.isBack) {
                    assertions.assertTrue(previousNode in visited)
                }
            }