From fced126c9fa05854c7fb6ac521cecc8c894d5d2b Mon Sep 17 00:00:00 2001 From: Brian Norman Date: Wed, 10 Jan 2024 14:42:31 -0600 Subject: [PATCH] [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 --- .../cfa/FirPropertyInitializationAnalyzer.kt | 4 +- .../PropertyInitializationInfoCollector.kt | 1 + .../declaration/FirMemberPropertiesChecker.kt | 2 +- .../FirTopLevelPropertiesChecker.kt | 2 +- .../ControlFlowAnalysisDiagnosticComponent.kt | 65 +++++++++++++++++-- .../dfa/cfg/ControlFlowGraphBuilder.kt | 4 +- .../dfa/cfg/ControlFlowGraphNodeBuilder.kt | 4 +- .../kotlin/fir/resolve/dfa/cfg/CFGNode.kt | 2 +- .../doWhileNotDefined.fir.kt | 6 -- .../controlFlowAnalysis/doWhileNotDefined.kt | 38 ++++++++++- .../controlFlowAnalysis/doWhileNotDefined.txt | 3 - 11 files changed, 106 insertions(+), 25 deletions(-) delete mode 100644 compiler/testData/diagnostics/tests/controlFlowAnalysis/doWhileNotDefined.fir.kt delete mode 100644 compiler/testData/diagnostics/tests/controlFlowAnalysis/doWhileNotDefined.txt 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 25dec889c3e..995d74f52a3 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 @@ -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( 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 259cdf6d6cb..e7e96989ef0 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 @@ -18,6 +18,7 @@ import org.jetbrains.kotlin.fir.symbols.impl.FirPropertySymbol class PropertyInitializationInfoData( val properties: Set, + val conditionallyInitializedProperties: Set, val receiver: FirBasedSymbol<*>?, val graph: ControlFlowGraph, ) { 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 7e654f07db7..8dbd35aee36 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 @@ -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] } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt index b38d445e402..59b79656f5d 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/checkers/declaration/FirTopLevelPropertiesChecker.kt @@ -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] } diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/collectors/components/ControlFlowAnalysisDiagnosticComponent.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/collectors/components/ControlFlowAnalysisDiagnosticComponent.kt index 37edcce6f90..1292602288f 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/collectors/components/ControlFlowAnalysisDiagnosticComponent.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/collectors/components/ControlFlowAnalysisDiagnosticComponent.kt @@ -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().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) : ControlFlowGraphVisitorVoid() { + private class LocalPropertyCollector : ControlFlowGraphVisitorVoid() { + val properties = mutableSetOf() + + // Properties which may not be initialized when accessed, even if they have an initializer. + val conditionallyInitializedProperties = mutableSetOf() + + // 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>>() + private val insideDoWhileConditions = mutableSetOf() + 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) + } } } } diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/ControlFlowGraphBuilder.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/ControlFlowGraphBuilder.kt index a773056f5e4..8db829a03b6 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/ControlFlowGraphBuilder.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/ControlFlowGraphBuilder.kt @@ -965,7 +965,7 @@ class ControlFlowGraphBuilder { } fun exitWhileLoopCondition(loop: FirLoop): Pair { - 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 { 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() diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/ControlFlowGraphNodeBuilder.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/ControlFlowGraphNodeBuilder.kt index 284c9b7f8b9..2cd47512bee 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/ControlFlowGraphNodeBuilder.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/ControlFlowGraphNodeBuilder.kt @@ -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) diff --git a/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/CFGNode.kt b/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/CFGNode.kt index 8e0ee6b132f..a766e58f171 100644 --- a/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/CFGNode.kt +++ b/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/CFGNode.kt @@ -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(owner, level), +class LoopConditionExitNode(owner: ControlFlowGraph, override val fir: FirExpression, val loop: FirLoop, level: Int) : CFGNode(owner, level), ExitNodeMarker { override fun accept(visitor: ControlFlowGraphVisitor, data: D): R { return visitor.visitLoopConditionExitNode(this, data) diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/doWhileNotDefined.fir.kt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/doWhileNotDefined.fir.kt deleted file mode 100644 index 92c4f436540..00000000000 --- a/compiler/testData/diagnostics/tests/controlFlowAnalysis/doWhileNotDefined.fir.kt +++ /dev/null @@ -1,6 +0,0 @@ -fun test(cond1: Boolean) { - do { - if (cond1) continue - val cond2 = false - } while (cond2) // cond2 may be not defined here -} diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/doWhileNotDefined.kt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/doWhileNotDefined.kt index e41bcd1a76e..960661c3edb 100644 --- a/compiler/testData/diagnostics/tests/controlFlowAnalysis/doWhileNotDefined.kt +++ b/compiler/testData/diagnostics/tests/controlFlowAnalysis/doWhileNotDefined.kt @@ -1,6 +1,42 @@ -fun test(cond1: Boolean) { +// FIR_IDENTICAL +// SKIP_TXT + +fun test1(cond1: Boolean) { do { if (cond1) continue val cond2 = false } while (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 (cond4) // cond4 may be not defined here + + 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 (cond2 && cond4) // cond2 and cond4 may be not defined here + + cond3 + } + ) +} diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/doWhileNotDefined.txt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/doWhileNotDefined.txt deleted file mode 100644 index 79431a5118b..00000000000 --- a/compiler/testData/diagnostics/tests/controlFlowAnalysis/doWhileNotDefined.txt +++ /dev/null @@ -1,3 +0,0 @@ -package - -public fun test(/*0*/ cond1: kotlin.Boolean): kotlin.Unit