[FIR] Properties defined in a do-while may not always be initialized

Local properties defined within the body of a do-while loop can be used
in the condition of the loop. However, use of a `continue` may mean the
property isn't always initialized, even if it is initialized when it is
defined. So while a local property may be within scope and has an
initializer, this doesn't always mean that the property is initialized.

As such, properties that are defined within a do-while loop and also
used in the condition of the same do-while loop should be tracked. Then,
these properties should still be checked for proper initialization even
if they have an initializer.

^KT-64872 Fixed
This commit is contained in:
Brian Norman
2024-01-10 14:42:31 -06:00
committed by Space Team
parent 4af8b70f62
commit fced126c9f
11 changed files with 106 additions and 25 deletions
@@ -74,7 +74,9 @@ fun PropertyInitializationInfoData.checkPropertyAccesses(
) {
// If a property has an initializer (or does not need one), then any reads are OK while any writes are OK
// if it's a `var` and bad if it's a `val`. `FirReassignmentAndInvisibleSetterChecker` does this without a CFG.
val filtered = properties.filterTo(mutableSetOf()) { it.requiresInitialization(isForInitialization) }
val filtered = properties.filterTo(mutableSetOf()) {
it.requiresInitialization(isForInitialization) || it in conditionallyInitializedProperties
}
if (filtered.isEmpty()) return
checkPropertyAccesses(
@@ -18,6 +18,7 @@ import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol
class PropertyInitializationInfoData(
val properties: Set<FirPropertySymbol>,
val conditionallyInitializedProperties: Set<FirPropertySymbol>,
val receiver: FirBasedSymbol<*>?,
val graph: ControlFlowGraph,
) {
@@ -58,7 +58,7 @@ object FirMemberPropertiesChecker : FirClassChecker(MppCheckerKind.Common) {
}
if (memberPropertySymbols.isEmpty()) return null
// TODO, KT-59803: merge with `FirPropertyInitializationAnalyzer` for fewer passes.
val data = PropertyInitializationInfoData(memberPropertySymbols, symbol, graph)
val data = PropertyInitializationInfoData(memberPropertySymbols, conditionallyInitializedProperties = emptySet(), symbol, graph)
data.checkPropertyAccesses(isForInitialization = true, context, reporter)
return data.getValue(graph.exitNode)[NormalPath]
}
@@ -74,7 +74,7 @@ private fun FirDeclaration.collectionInitializationInfo(
if (propertySymbols.isEmpty()) return null
// TODO, KT-59803: merge with `FirPropertyInitializationAnalyzer` for fewer passes.
val data = PropertyInitializationInfoData(propertySymbols, receiver = null, graph)
val data = PropertyInitializationInfoData(propertySymbols, conditionallyInitializedProperties = emptySet(), receiver = null, graph)
data.checkPropertyAccesses(isForInitialization = true, context, reporter)
return data.getValue(graph.exitNode)[NormalPath]
}
@@ -13,9 +13,10 @@ import org.jetbrains.kotlin.fir.analysis.checkers.context.CheckerContext
import org.jetbrains.kotlin.fir.analysis.checkers.declaration.DeclarationCheckers
import org.jetbrains.kotlin.fir.analysis.checkersComponent
import org.jetbrains.kotlin.fir.declarations.*
import org.jetbrains.kotlin.fir.resolve.dfa.cfg.CFGNode
import org.jetbrains.kotlin.fir.resolve.dfa.cfg.ControlFlowGraphVisitorVoid
import org.jetbrains.kotlin.fir.resolve.dfa.cfg.VariableDeclarationNode
import org.jetbrains.kotlin.fir.expressions.FirDoWhileLoop
import org.jetbrains.kotlin.fir.expressions.FirLoop
import org.jetbrains.kotlin.fir.references.toResolvedPropertySymbol
import org.jetbrains.kotlin.fir.resolve.dfa.cfg.*
import org.jetbrains.kotlin.fir.resolve.dfa.controlFlowGraph
import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol
@@ -41,9 +42,10 @@ class ControlFlowAnalysisDiagnosticComponent(
if (graph.isSubGraph) return
cfaCheckers.forEach { it.analyze(graph, reporter, context) }
val properties = mutableSetOf<FirPropertySymbol>().apply { graph.traverse(LocalPropertyCollector(this)) }
val collector = LocalPropertyCollector().apply { graph.traverse(this) }
val properties = collector.properties
if (properties.isNotEmpty()) {
val data = PropertyInitializationInfoData(properties, receiver = null, graph)
val data = PropertyInitializationInfoData(properties, collector.conditionallyInitializedProperties, receiver = null, graph)
variableAssignmentCheckers.forEach { it.analyze(data, reporter, context) }
}
}
@@ -94,11 +96,60 @@ class ControlFlowAnalysisDiagnosticComponent(
analyze(constructor, data)
}
private class LocalPropertyCollector(private val result: MutableSet<FirPropertySymbol>) : ControlFlowGraphVisitorVoid() {
private class LocalPropertyCollector : ControlFlowGraphVisitorVoid() {
val properties = mutableSetOf<FirPropertySymbol>()
// Properties which may not be initialized when accessed, even if they have an initializer.
val conditionallyInitializedProperties = mutableSetOf<FirPropertySymbol>()
// Properties defined within do-while loops, and used within the condition of that same do-while loop, are considered conditionally
// initialized. It is possible they may not even be defined by the loop condition due to a `continue` in the do-while loop. Track
// do-while loop properties so those used in the condition can be recorded.
private val doWhileLoopProperties = ArrayDeque<Pair<FirLoop, MutableSet<FirPropertySymbol>>>()
private val insideDoWhileConditions = mutableSetOf<FirLoop>()
override fun visitNode(node: CFGNode<*>) {}
override fun visitVariableDeclarationNode(node: VariableDeclarationNode) {
result.add(node.fir.symbol)
val symbol = node.fir.symbol
properties.add(symbol)
doWhileLoopProperties.lastOrNull()?.second?.add(symbol)
}
override fun visitQualifiedAccessNode(node: QualifiedAccessNode) {
if (insideDoWhileConditions.isNotEmpty()) {
val symbol = node.fir.calleeReference.toResolvedPropertySymbol() ?: return
// It is possible to nest do-while loops within do-while loop conditions via in-place lambda functions. Make sure to check
// all properties for all loop conditions.
if (doWhileLoopProperties.any { it.first in insideDoWhileConditions && symbol in it.second }) {
conditionallyInitializedProperties.add(symbol)
}
}
}
override fun visitLoopEnterNode(node: LoopEnterNode) {
if (node.fir is FirDoWhileLoop) {
doWhileLoopProperties.addLast(node.fir to mutableSetOf())
}
}
override fun visitLoopExitNode(node: LoopExitNode) {
if (node.fir is FirDoWhileLoop) {
doWhileLoopProperties.removeLast()
}
}
override fun visitLoopConditionEnterNode(node: LoopConditionEnterNode) {
if (node.loop is FirDoWhileLoop) {
insideDoWhileConditions.add(node.loop)
}
}
override fun visitLoopConditionExitNode(node: LoopConditionExitNode) {
if (node.loop is FirDoWhileLoop) {
insideDoWhileConditions.remove(node.loop)
}
}
}
}
@@ -965,7 +965,7 @@ class ControlFlowGraphBuilder {
}
fun exitWhileLoopCondition(loop: FirLoop): Pair<LoopConditionExitNode, LoopBlockEnterNode> {
val conditionExitNode = createLoopConditionExitNode(loop.condition).also { addNewSimpleNode(it) }
val conditionExitNode = createLoopConditionExitNode(loop.condition, loop).also { addNewSimpleNode(it) }
val conditionConstBooleanValue = loop.condition.booleanLiteralValue
addEdge(conditionExitNode, loopExitNodes.getValue(loop), propagateDeadness = false, isDead = conditionConstBooleanValue == true)
val loopBlockEnterNode = createLoopBlockEnterNode(loop)
@@ -1007,7 +1007,7 @@ class ControlFlowGraphBuilder {
fun exitDoWhileLoop(loop: FirLoop): Pair<LoopConditionExitNode, LoopExitNode> {
loopConditionEnterNodes.remove(loop)
val conditionExitNode = createLoopConditionExitNode(loop.condition)
val conditionExitNode = createLoopConditionExitNode(loop.condition, loop)
val conditionBooleanValue = loop.condition.booleanLiteralValue
popAndAddEdge(conditionExitNode)
val blockEnterNode = lastNodes.pop()
@@ -106,8 +106,8 @@ fun ControlFlowGraphBuilder.createWhenSyntheticElseBranchNode(fir: FirWhenExpres
fun ControlFlowGraphBuilder.createWhenBranchResultEnterNode(fir: FirWhenBranch): WhenBranchResultEnterNode =
WhenBranchResultEnterNode(currentGraph, fir, levelCounter)
fun ControlFlowGraphBuilder.createLoopConditionExitNode(fir: FirExpression): LoopConditionExitNode =
LoopConditionExitNode(currentGraph, fir, levelCounter)
fun ControlFlowGraphBuilder.createLoopConditionExitNode(fir: FirExpression, loop: FirLoop): LoopConditionExitNode =
LoopConditionExitNode(currentGraph, fir, loop, levelCounter)
fun ControlFlowGraphBuilder.createLoopConditionEnterNode(fir: FirExpression, loop: FirLoop): LoopConditionEnterNode =
LoopConditionEnterNode(currentGraph, fir, loop, levelCounter)
@@ -520,7 +520,7 @@ class LoopConditionEnterNode(owner: ControlFlowGraph, override val fir: FirExpre
return visitor.visitLoopConditionEnterNode(this, data)
}
}
class LoopConditionExitNode(owner: ControlFlowGraph, override val fir: FirExpression, level: Int) : CFGNode<FirExpression>(owner, level),
class LoopConditionExitNode(owner: ControlFlowGraph, override val fir: FirExpression, val loop: FirLoop, level: Int) : CFGNode<FirExpression>(owner, level),
ExitNodeMarker {
override fun <R, D> accept(visitor: ControlFlowGraphVisitor<R, D>, data: D): R {
return visitor.visitLoopConditionExitNode(this, data)
@@ -1,6 +0,0 @@
fun test(cond1: Boolean) {
do {
if (cond1) continue
val cond2 = false
} while (cond2) // cond2 may be not defined here
}
@@ -1,6 +1,42 @@
fun test(cond1: Boolean) {
// FIR_IDENTICAL
// SKIP_TXT
fun test1(cond1: Boolean) {
do {
if (cond1) continue
val cond2 = false
} while (<!UNINITIALIZED_VARIABLE!>cond2<!>) // cond2 may be not defined here
}
fun test2(cond1: Boolean, cond3: Boolean) {
do {
if (cond1) continue
val cond2 = false
} while (
run {
do {
if (cond3) continue
val cond4 = false
} while (<!UNINITIALIZED_VARIABLE!>cond4<!>) // cond4 may be not defined here
<!UNINITIALIZED_VARIABLE!>cond2<!> // cond2 may be not defined here
}
)
}
fun test3(cond1: Boolean, cond3: Boolean) {
do {
if (cond1) continue
val cond2 = false
} while (
run {
do {
if (cond3) continue
val cond4 = false
} while (<!UNINITIALIZED_VARIABLE!>cond2<!> && <!UNINITIALIZED_VARIABLE!>cond4<!>) // cond2 and cond4 may be not defined here
cond3
}
)
}
@@ -1,3 +0,0 @@
package
public fun test(/*0*/ cond1: kotlin.Boolean): kotlin.Unit