diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/AbstractFirPropertyInitializationChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/AbstractFirPropertyInitializationChecker.kt index e4683484ef7..85ee137300e 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/AbstractFirPropertyInitializationChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/AbstractFirPropertyInitializationChecker.kt @@ -14,7 +14,7 @@ abstract class AbstractFirPropertyInitializationChecker { abstract fun analyze( graph: ControlFlowGraph, reporter: DiagnosticReporter, - data: Map, PropertyInitializationInfo>, + data: Map, PathAwarePropertyInitializationInfo>, properties: Set ) } \ No newline at end of file diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/CfaUtils.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/CfaUtils.kt index 00180f9efd6..ed8f129be1b 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/CfaUtils.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/CfaUtils.kt @@ -11,7 +11,7 @@ import org.jetbrains.kotlin.contracts.description.EventOccurrencesRange import org.jetbrains.kotlin.fir.references.FirResolvedNamedReference import org.jetbrains.kotlin.fir.resolve.dfa.cfg.* import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol - +import java.lang.IllegalStateException class PropertyInitializationInfo( map: PersistentMap = persistentMapOf() @@ -52,17 +52,97 @@ class LocalPropertyCollector private constructor() : ControlFlowGraphVisitorVoid } } +class PathAwarePropertyInitializationInfo( + map: PersistentMap = persistentMapOf() +) : ControlFlowInfo(map) { + companion object { + val EMPTY = PathAwarePropertyInitializationInfo(persistentMapOf(NormalPath to PropertyInitializationInfo.EMPTY)) + } + + override val constructor: (PersistentMap) -> PathAwarePropertyInitializationInfo = + ::PathAwarePropertyInitializationInfo + + val infoAtNormalPath: PropertyInitializationInfo + get() = map[NormalPath] ?: PropertyInitializationInfo.EMPTY + + val hasNormalPath: Boolean + get() = map.containsKey(NormalPath) + + fun applyLabel(node: CFGNode<*>, label: EdgeLabel): PathAwarePropertyInitializationInfo { + if (label.isNormal) { + // Special case: when we exit the try expression, null label means a normal path. + // Filter out any info bound to non-null label + // One day, if we allow multiple edges between nodes with different labels, e.g., labeling all paths in try/catch/finally, + // instead of this kind of special handling, proxy enter/exit nodes per label are preferred. + if (node is TryExpressionExitNode) { + return if (hasNormalPath) { + constructor(persistentMapOf(NormalPath to infoAtNormalPath)) + } else { + /* This means no info for normal path. */ + EMPTY + } + } + // In general, null label means no additional path info, hence return `this` as-is. + return this + } + + val hasAbnormalLabels = map.keys.any { !it.isNormal } + return if (hasAbnormalLabels) { + // { |-> ... l1 |-> I1, l2 |-> I2, ... } + // | l1 // path exit: if the given info has non-null labels, this acts like a filtering + // { |-> I1 } // NB: remove the path info + if (map.keys.contains(label)) { + constructor(persistentMapOf(NormalPath to map[label]!!)) + } else { + /* This means no info for the specific label. */ + EMPTY + } + } else { + // { |-> ... } // empty path info + // | l1 // path entry + // { l1 -> ... } // now, every info bound to the label + constructor(persistentMapOf(label to infoAtNormalPath)) + } + } + + fun merge(other: PathAwarePropertyInitializationInfo): PathAwarePropertyInitializationInfo { + var resultMap = persistentMapOf() + for (label in keys.union(other.keys)) { + // disjoint merging to preserve paths. i.e., merge the property initialization info if and only if both have the key. + // merge({ |-> I1 }, { |-> I2, l1 |-> I3 } + // == { |-> merge(I1, I2), l1 |-> I3 } + val i1 = this[label] + val i2 = other[label] + resultMap = when { + i1 != null && i2 != null -> + resultMap.put(label, i1.merge(i2)) + i1 != null -> + resultMap.put(label, i1) + i2 != null -> + resultMap.put(label, i2) + else -> + throw IllegalStateException() + } + } + return constructor(resultMap) + } +} + class PropertyInitializationInfoCollector(private val localProperties: Set) : - ControlFlowGraphVisitor>() { - override fun visitNode(node: CFGNode<*>, data: Collection): PropertyInitializationInfo { - if (data.isEmpty()) return PropertyInitializationInfo.EMPTY - return data.reduce(PropertyInitializationInfo::merge) + ControlFlowGraphVisitor>>() { + override fun visitNode( + node: CFGNode<*>, + data: Collection> + ): PathAwarePropertyInitializationInfo { + if (data.isEmpty()) return PathAwarePropertyInitializationInfo.EMPTY + return data.map { (label, info) -> info.applyLabel(node, label) } + .reduce(PathAwarePropertyInitializationInfo::merge) } override fun visitVariableAssignmentNode( node: VariableAssignmentNode, - data: Collection - ): PropertyInitializationInfo { + data: Collection> + ): PathAwarePropertyInitializationInfo { val dataForNode = visitNode(node, data) val reference = node.fir.lValue as? FirResolvedNamedReference ?: return dataForNode val symbol = reference.resolvedSymbol as? FirPropertySymbol ?: return dataForNode @@ -75,8 +155,8 @@ class PropertyInitializationInfoCollector(private val localProperties: Set - ): PropertyInitializationInfo { + data: Collection> + ): PathAwarePropertyInitializationInfo { val dataForNode = visitNode(node, data) return if (node.fir.initializer == null && node.fir.delegate == null) { dataForNode @@ -86,18 +166,27 @@ class PropertyInitializationInfoCollector(private val localProperties: Set() + // before: { |-> { p1 |-> PI1 }, l1 |-> { p2 |-> PI2 } + for (label in dataForNode.keys) { + val dataPerLabel = dataForNode[label]!! + val existingKind = dataPerLabel[symbol] ?: EventOccurrencesRange.ZERO + val kind = existingKind + EventOccurrencesRange.EXACTLY_ONCE + resultMap = resultMap.put(label, dataPerLabel.put(symbol, kind)) + } + // after (if symbol is p1): + // { |-> { p1 |-> PI1 + 1 }, l1 |-> { p1 |-> [1, 1], p2 |-> PI2 } + return PathAwarePropertyInitializationInfo(resultMap) } } \ No newline at end of file diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/CfgTraverser.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/CfgTraverser.kt index 4bd1a2afb57..c2cd181f4d9 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/CfgTraverser.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/CfgTraverser.kt @@ -7,6 +7,8 @@ package org.jetbrains.kotlin.fir.analysis.cfa import org.jetbrains.kotlin.fir.resolve.dfa.cfg.* +// ------------------------------ Graph Traversal ------------------------------ + enum class TraverseDirection { Forward, Backward } @@ -29,6 +31,10 @@ fun ControlFlowGraph.traverse( traverse(direction, visitor, null) } + +// --------------------- Path-insensitive data collection ---------------------- + +// TODO: Deprecate and make all existing checkers path-sensitive fun , K : Any, V : Any> ControlFlowGraph.collectDataForNode( direction: TraverseDirection, initialInfo: I, @@ -75,3 +81,62 @@ private fun , K : Any, V : Any> ControlFlowGraph.co } } } + +// ---------------------- Path-sensitive data collection ----------------------- + +// TODO: Once migration is done, get rid of "PathAware" from util names +fun , K : Any, V : Any> ControlFlowGraph.collectPathAwareDataForNode( + direction: TraverseDirection, + initialInfo: I, + visitor: ControlFlowGraphVisitor>> +): Map, I> { + val nodeMap = LinkedHashMap, I>() + val startNode = getEnterNode(direction) + nodeMap[startNode] = initialInfo + + val changed = mutableMapOf, Boolean>() + do { + collectPathAwareDataForNodeInternal(direction, initialInfo, visitor, nodeMap, changed) + } while (changed.any { it.value }) + + return nodeMap +} + +private fun , K : Any, V : Any> ControlFlowGraph.collectPathAwareDataForNodeInternal( + direction: TraverseDirection, + initialInfo: I, + visitor: ControlFlowGraphVisitor>>, + nodeMap: MutableMap, I>, + changed: MutableMap, Boolean> +) { + val nodes = getNodesInOrder(direction) + for (node in nodes) { + if (direction == TraverseDirection.Backward && node is CFGNodeWithCfgOwner<*>) { + node.subGraphs.forEach { it.collectPathAwareDataForNodeInternal(direction, initialInfo, visitor, nodeMap, changed) } + } + val previousNodes = when (direction) { + TraverseDirection.Forward -> node.previousCfgNodes + TraverseDirection.Backward -> node.followingCfgNodes + } + // One noticeable different against the path-unaware version is, here, we pair the control-flow info with the label. + val previousData = + previousNodes.mapNotNull { + val k = when (direction) { + TraverseDirection.Forward -> node.incomingEdges[it]?.label ?: NormalPath + TraverseDirection.Backward -> node.outgoingEdges[it]?.label ?: NormalPath + } + val v = nodeMap[it] ?: return@mapNotNull null + k to v + } + val data = nodeMap[node] + val newData = node.accept(visitor, previousData) + val hasChanged = newData != data + changed[node] = hasChanged + if (hasChanged) { + nodeMap[node] = newData + } + if (direction == TraverseDirection.Forward && node is CFGNodeWithCfgOwner<*>) { + node.subGraphs.forEach { it.collectPathAwareDataForNodeInternal(direction, initialInfo, visitor, nodeMap, changed) } + } + } +} diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/FirPropertyInitializationAnalyzer.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/FirPropertyInitializationAnalyzer.kt index c05c94b6ec1..f42b1564e7f 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/FirPropertyInitializationAnalyzer.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/FirPropertyInitializationAnalyzer.kt @@ -22,7 +22,7 @@ object FirPropertyInitializationAnalyzer : AbstractFirPropertyInitializationChec override fun analyze( graph: ControlFlowGraph, reporter: DiagnosticReporter, - data: Map, PropertyInitializationInfo>, + data: Map, PathAwarePropertyInitializationInfo>, properties: Set ) { val localData = data.filter { @@ -37,7 +37,7 @@ object FirPropertyInitializationAnalyzer : AbstractFirPropertyInitializationChec } private class UninitializedPropertyReporter( - val data: Map, PropertyInitializationInfo>, + val data: Map, PathAwarePropertyInitializationInfo>, val localProperties: Set, val reporter: DiagnosticReporter ) : ControlFlowGraphVisitorVoid() { @@ -48,12 +48,24 @@ object FirPropertyInitializationAnalyzer : AbstractFirPropertyInitializationChec val symbol = reference.resolvedSymbol as? FirPropertySymbol ?: return if (symbol !in localProperties) return if (symbol.fir.isLateInit) return - val kind = data.getValue(node)[symbol] ?: EventOccurrencesRange.ZERO - if (!kind.isDefinitelyVisited()) { - node.fir.source?.let { - reporter.report(FirErrors.UNINITIALIZED_VARIABLE.on(it, symbol)) + val pathAwareInfo = data.getValue(node) + for (label in pathAwareInfo.keys) { + if (investigate(pathAwareInfo[label]!!, symbol, node)) { + // To avoid duplicate reports, stop investigating remaining paths if the property is not initialized at any path. + break } } } + + private fun investigate(info: PropertyInitializationInfo, symbol: FirPropertySymbol, node: QualifiedAccessNode): Boolean { + val kind = info[symbol] ?: EventOccurrencesRange.ZERO + if (!kind.isDefinitelyVisited()) { + node.fir.source?.let { + reporter.report(FirErrors.UNINITIALIZED_VARIABLE.on(it, symbol)) + return true + } + } + return false + } } } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/CanBeValChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/CanBeValChecker.kt index b0f7a6f3b77..76db48e44bd 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/CanBeValChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/extended/CanBeValChecker.kt @@ -5,16 +5,12 @@ package org.jetbrains.kotlin.fir.analysis.checkers.extended - import com.intellij.lang.LighterASTNode import com.intellij.openapi.util.Ref import org.jetbrains.kotlin.KtNodeTypes import org.jetbrains.kotlin.contracts.description.EventOccurrencesRange import org.jetbrains.kotlin.fir.* -import org.jetbrains.kotlin.fir.analysis.cfa.AbstractFirPropertyInitializationChecker -import org.jetbrains.kotlin.fir.analysis.cfa.PropertyInitializationInfo -import org.jetbrains.kotlin.fir.analysis.cfa.TraverseDirection -import org.jetbrains.kotlin.fir.analysis.cfa.traverse +import org.jetbrains.kotlin.fir.analysis.cfa.* import org.jetbrains.kotlin.fir.analysis.diagnostics.DiagnosticReporter import org.jetbrains.kotlin.fir.analysis.diagnostics.FirErrors import org.jetbrains.kotlin.fir.analysis.getChild @@ -23,12 +19,11 @@ import org.jetbrains.kotlin.fir.resolve.dfa.cfg.* import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol import org.jetbrains.kotlin.lexer.KtTokens - object CanBeValChecker : AbstractFirPropertyInitializationChecker() { override fun analyze( graph: ControlFlowGraph, reporter: DiagnosticReporter, - data: Map, PropertyInitializationInfo>, + data: Map, PathAwarePropertyInitializationInfo>, properties: Set ) { val unprocessedProperties = mutableSetOf() @@ -75,7 +70,7 @@ object CanBeValChecker : AbstractFirPropertyInitializationChecker() { value in canBeValOccurrenceRanges && symbol.fir.isVar private class UninitializedPropertyReporter( - val data: Map, PropertyInitializationInfo>, + val data: Map, PathAwarePropertyInitializationInfo>, val localProperties: Set, val unprocessedProperties: MutableSet, val propertiesCharacteristics: MutableMap @@ -89,7 +84,8 @@ object CanBeValChecker : AbstractFirPropertyInitializationChecker() { unprocessedProperties.remove(symbol) val currentCharacteristic = propertiesCharacteristics.getOrDefault(symbol, EventOccurrencesRange.ZERO) - propertiesCharacteristics[symbol] = currentCharacteristic.or(data.getValue(node)[symbol] ?: EventOccurrencesRange.ZERO) + val info = data.getValue(node) + propertiesCharacteristics[symbol] = currentCharacteristic.or(info.infoAtNormalPath[symbol] ?: EventOccurrencesRange.ZERO) } override fun visitVariableDeclarationNode(node: VariableDeclarationNode) { diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInTryWithCatch.fir.kt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInTryWithCatch.fir.kt index 09c54d7085a..7bc3cbb4aae 100644 --- a/compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInTryWithCatch.fir.kt +++ b/compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInTryWithCatch.fir.kt @@ -27,7 +27,7 @@ fun assignedInTryAndCatch() { a = 41 } finally { } - a.hashCode() + a.hashCode() } fun sideEffectBeforeAssignedInTryAndCatch(s: Any) { @@ -40,7 +40,7 @@ fun sideEffectBeforeAssignedInTryAndCatch(s: Any) { a = 41 } finally { } - a.hashCode() + a.hashCode() } fun assignedAtAll() { diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInTryWithoutCatch.fir.kt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInTryWithoutCatch.fir.kt index f4e05210985..3e92f0d54c8 100644 --- a/compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInTryWithoutCatch.fir.kt +++ b/compiler/testData/diagnostics/tests/controlFlowAnalysis/assignedInTryWithoutCatch.fir.kt @@ -4,7 +4,7 @@ fun assignedInTry() { a = 42 } finally { } - a.hashCode() + a.hashCode() } fun sideEffectBeforeAssignmentInTry(s: Any) { @@ -14,7 +14,7 @@ fun sideEffectBeforeAssignmentInTry(s: Any) { a = 42 } finally { } - a.hashCode() + a.hashCode() } fun assignedInTryAndFinally() { diff --git a/compiler/tests-spec/testData/diagnostics/notLinked/contracts/analysis/controlFlow/initialization/neg/3.fir.kt b/compiler/tests-spec/testData/diagnostics/notLinked/contracts/analysis/controlFlow/initialization/neg/3.fir.kt index b9e7dd10574..bb192f928b3 100644 --- a/compiler/tests-spec/testData/diagnostics/notLinked/contracts/analysis/controlFlow/initialization/neg/3.fir.kt +++ b/compiler/tests-spec/testData/diagnostics/notLinked/contracts/analysis/controlFlow/initialization/neg/3.fir.kt @@ -95,7 +95,7 @@ fun case_6() { println(value_2.inc()) } - value_2++ + value_2++ } // TESTCASE NUMBER: 7