From 1d9a1e979a51a7fe74f7eb1f8811078ee2da9ae1 Mon Sep 17 00:00:00 2001 From: pyos Date: Mon, 23 Jan 2023 11:19:45 +0100 Subject: [PATCH] FIR CFA: skip member function graphs when looking for member assignments Shouldn't affect the result, since in member functions all properties are assumed to be initialized so the CFG is not needed for reporting VAL_REASSIGNMENT. --- .../fir/analysis/cfa/util/CfgTraverser.kt | 55 ++++++++----------- .../kotlin/fir/analysis/cfa/util/CfgUtils.kt | 5 -- .../util/PathAwareControlFlowGraphVisitor.kt | 3 + .../PropertyInitializationInfoCollector.kt | 5 ++ .../declaration/FirMemberPropertiesChecker.kt | 1 - 5 files changed, 32 insertions(+), 37 deletions(-) diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/util/CfgTraverser.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/util/CfgTraverser.kt index b6907b1eb48..b999b43221d 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/util/CfgTraverser.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/util/CfgTraverser.kt @@ -14,17 +14,12 @@ enum class TraverseDirection { fun > ControlFlowGraph.collectDataForNode( direction: TraverseDirection, visitor: PathAwareControlFlowGraphVisitor, - visitSubGraphs: Boolean = true ): Map, PathAwareControlFlowInfo> { - val nodeMap = LinkedHashMap, PathAwareControlFlowInfo>() - val startNode = getEnterNode(direction) - nodeMap[startNode] = visitor.emptyInfo - + val nodeMap = HashMap, PathAwareControlFlowInfo>() var shouldContinue: Boolean do { - shouldContinue = collectDataForNodeInternal(direction, visitor, nodeMap, visitSubGraphs) + shouldContinue = collectDataForNodeInternal(direction, visitor, nodeMap) } while (shouldContinue) - return nodeMap } @@ -32,38 +27,36 @@ private fun > ControlFlowGraph.collectDataForNodeIn direction: TraverseDirection, visitor: PathAwareControlFlowGraphVisitor, nodeMap: MutableMap, PathAwareControlFlowInfo>, - visitSubGraphs: Boolean = true ): Boolean { var changed = false - val nodes = getNodesInOrder(direction) - for (node in nodes) { - if (visitSubGraphs && direction == TraverseDirection.Backward && node is CFGNodeWithSubgraphs<*>) { - node.subGraphs.forEach { changed = changed or it.collectDataForNodeInternal(direction, visitor, nodeMap) } - } - val previousNodes = when (direction) { - TraverseDirection.Forward -> node.previousCfgNodes - TraverseDirection.Backward -> node.followingCfgNodes + for (node in getNodesInOrder(direction)) { + if (direction == TraverseDirection.Backward && node is CFGNodeWithSubgraphs<*>) { + node.subGraphs.forEach { + changed = changed or (visitor.visitSubGraph(node, it) && it.collectDataForNodeInternal(direction, visitor, nodeMap)) + } } // TODO: if data for previousNodes hasn't changed, then should be no need to recompute data for this one val union = node.isUnion - val previousData = - previousNodes.mapNotNull { - val k = when (direction) { - TraverseDirection.Forward -> node.edgeFrom(it) - TraverseDirection.Backward -> node.edgeTo(it) + val previousData = when (direction) { + TraverseDirection.Forward -> node.previousCfgNodes + TraverseDirection.Backward -> node.followingCfgNodes + }.mapNotNull { source -> + nodeMap[source]?.let { + val edge = when (direction) { + TraverseDirection.Forward -> node.edgeFrom(source) + TraverseDirection.Backward -> node.edgeTo(source) } - val v = nodeMap[it] ?: return@mapNotNull null - visitor.visitEdge(it, node, k, v) - }.reduceOrNull { a, b -> a.join(b, union) } - val data = nodeMap[node] + visitor.visitEdge(source, node, edge, it) + } + }.reduceOrNull { a, b -> a.join(b, union) } val newData = node.accept(visitor, previousData ?: visitor.emptyInfo) - val hasChanged = newData != data - changed = changed or hasChanged - if (hasChanged) { - nodeMap[node] = newData + if (newData != nodeMap.put(node, newData)) { + changed = true } - if (visitSubGraphs && direction == TraverseDirection.Forward && node is CFGNodeWithSubgraphs<*>) { - node.subGraphs.forEach { changed = changed or it.collectDataForNodeInternal(direction, visitor, nodeMap) } + if (direction == TraverseDirection.Forward && node is CFGNodeWithSubgraphs<*>) { + node.subGraphs.forEach { + changed = changed or (visitor.visitSubGraph(node, it) && it.collectDataForNodeInternal(direction, visitor, nodeMap)) + } } } return changed diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/util/CfgUtils.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/util/CfgUtils.kt index b94b140ad38..fdba368bb52 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/util/CfgUtils.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/util/CfgUtils.kt @@ -8,11 +8,6 @@ package org.jetbrains.kotlin.fir.analysis.cfa.util import org.jetbrains.kotlin.fir.resolve.dfa.cfg.CFGNode import org.jetbrains.kotlin.fir.resolve.dfa.cfg.ControlFlowGraph -fun ControlFlowGraph.getEnterNode(direction: TraverseDirection): CFGNode<*> = when (direction) { - TraverseDirection.Forward -> enterNode - TraverseDirection.Backward -> exitNode -} - fun ControlFlowGraph.getNodesInOrder(direction: TraverseDirection): List> = when (direction) { TraverseDirection.Forward -> nodes TraverseDirection.Backward -> nodes.asReversed() diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/util/PathAwareControlFlowGraphVisitor.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/util/PathAwareControlFlowGraphVisitor.kt index 01e7987a3d1..d8efc44c839 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/util/PathAwareControlFlowGraphVisitor.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/util/PathAwareControlFlowGraphVisitor.kt @@ -31,6 +31,9 @@ abstract class PathAwareControlFlowGraphVisitor> : abstract val emptyInfo: PathAwareControlFlowInfo + open fun visitSubGraph(node: CFGNodeWithSubgraphs<*>, graph: ControlFlowGraph): Boolean = + true // false to skip + open fun visitEdge(from: CFGNode<*>, to: CFGNode<*>, metadata: Edge, data: PathAwareControlFlowInfo): PathAwareControlFlowInfo { val label = metadata.label return when { diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/util/PropertyInitializationInfoCollector.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/util/PropertyInitializationInfoCollector.kt index 40a747e0dae..9f6edb3a34f 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/util/PropertyInitializationInfoCollector.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/util/PropertyInitializationInfoCollector.kt @@ -39,6 +39,11 @@ class PropertyInitializationInfoCollector( override val emptyInfo: PathAwarePropertyInitializationInfo get() = EMPTY_INFO + // When looking for initializations of member properties, skip subgraphs of member functions; + // all properties are assumed to be initialized there. + override fun visitSubGraph(node: CFGNodeWithSubgraphs<*>, graph: ControlFlowGraph): Boolean = + expectedReceiver == null || node !is ClassExitNode || node !== node.owner.exitNode + override fun visitVariableAssignmentNode( node: VariableAssignmentNode, data: PathAwarePropertyInitializationInfo diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertiesChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertiesChecker.kt index cbec8fe11a4..3333df1c309 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertiesChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirMemberPropertiesChecker.kt @@ -50,7 +50,6 @@ object FirMemberPropertiesChecker : FirClassChecker() { (it.symbol as? FirPropertySymbol)?.takeIf { symbol -> symbol.requiresInitialization } } if (memberPropertySymbols.isEmpty()) return null - // TODO: this also visits non-constructor member functions... // TODO: merge with `FirPropertyInitializationAnalyzer` for fewer passes. val data = PropertyInitializationInfoData(memberPropertySymbols, symbol, graph) data.checkPropertyAccesses(context, reporter)