From a1167e12d5f15cc642169202589a5d49dfeb0f6b Mon Sep 17 00:00:00 2001 From: Brian Norman Date: Mon, 22 Jul 2024 07:48:24 -0500 Subject: [PATCH] [FIR] Propagate variables initialized in loops to parent statements When checking local variables initialization, there is a look-ahead pass that finds variables declared within repeatable statements. This is used to reset initialization information from certain control-flow graph paths when calculating if a variable is correctly initialized. Loops where not correctly propagating variables to outer repeatable statements, which caused problems for certain nested structures with jumps. ^KT-69494 Fixed (cherry picked from commit b450bd4ca687e3c50f3e0ea0a4cbd87a01e0bdc3) --- ...CompilerTestFE10TestdataTestGenerated.java | 6 ++ ...sticCompilerFE10TestDataTestGenerated.java | 6 ++ ...eeOldFrontendDiagnosticsTestGenerated.java | 6 ++ ...siOldFrontendDiagnosticsTestGenerated.java | 6 ++ .../VariableInitializationCheckProcessor.kt | 75 +++++++------------ .../PropertyInitializationInfoCollector.kt | 4 +- .../reassignmenGraphLoop.fir.kt | 17 +++++ .../reassignmenGraphLoop.kt | 17 +++++ .../test/runners/DiagnosticTestGenerated.java | 6 ++ 9 files changed, 95 insertions(+), 48 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/controlFlowAnalysis/reassignmenGraphLoop.fir.kt create mode 100644 compiler/testData/diagnostics/tests/controlFlowAnalysis/reassignmenGraphLoop.kt diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java index 36180a2c59eb6..697fc9adc2e1c 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/DiagnosticCompilerTestFE10TestdataTestGenerated.java @@ -8566,6 +8566,12 @@ public void testPropertiesOrderInPackage() { runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/propertiesOrderInPackage.kt"); } + @Test + @TestMetadata("reassignmenGraphLoop.kt") + public void testReassignmenGraphLoop() { + runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/reassignmenGraphLoop.kt"); + } + @Test @TestMetadata("reassignmentInCatch.kt") public void testReassignmentInCatch() { diff --git a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java index 48023ae043e87..34859952fe02c 100644 --- a/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java +++ b/analysis/low-level-api-fir/tests/org/jetbrains/kotlin/analysis/low/level/api/fir/diagnostic/compiler/based/LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated.java @@ -8566,6 +8566,12 @@ public void testPropertiesOrderInPackage() { runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/propertiesOrderInPackage.kt"); } + @Test + @TestMetadata("reassignmenGraphLoop.kt") + public void testReassignmenGraphLoop() { + runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/reassignmenGraphLoop.kt"); + } + @Test @TestMetadata("reassignmentInCatch.kt") public void testReassignmentInCatch() { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java index f61e37b4a7784..7a179ed95ac02 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirLightTreeOldFrontendDiagnosticsTestGenerated.java @@ -8560,6 +8560,12 @@ public void testPropertiesOrderInPackage() { runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/propertiesOrderInPackage.kt"); } + @Test + @TestMetadata("reassignmenGraphLoop.kt") + public void testReassignmenGraphLoop() { + runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/reassignmenGraphLoop.kt"); + } + @Test @TestMetadata("reassignmentInCatch.kt") public void testReassignmentInCatch() { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java index 720773de84923..c960699f7b842 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirPsiOldFrontendDiagnosticsTestGenerated.java @@ -8566,6 +8566,12 @@ public void testPropertiesOrderInPackage() { runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/propertiesOrderInPackage.kt"); } + @Test + @TestMetadata("reassignmenGraphLoop.kt") + public void testReassignmenGraphLoop() { + runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/reassignmenGraphLoop.kt"); + } + @Test @TestMetadata("reassignmentInCatch.kt") public void testReassignmentInCatch() { diff --git a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/VariableInitializationCheckProcessor.kt b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/VariableInitializationCheckProcessor.kt index 7458546f43986..7c7398b08d9b8 100644 --- a/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/VariableInitializationCheckProcessor.kt +++ b/compiler/fir/checkers/src/org/jetbrains/kotlin/fir/analysis/cfa/VariableInitializationCheckProcessor.kt @@ -103,45 +103,7 @@ abstract class VariableInitializationCheckProcessor { path: EdgeLabel, visited: MutableSet>, ) { - require(visited.add(this)) { - val problemNode = this@reportErrorsOnInitializationsInInputs - val containingDeclaration = problemNode.containingDeclaration() - buildString { - appendLine("Node has already been visited and could result in infinite recursion.") - appendLine() - - appendLine("CFG") - append("- Node: ").appendLine(problemNode.render()) - append("- Path: ").appendLine(path.label) - appendLine("- Visited:") - for (n in visited) { - append(" - ").appendLine(n.render()) - } - appendLine() - - appendLine("Context") - append("File Path: ").appendLine(context.containingFilePath) - appendLine("Declarations:") - for (d in context.containingDeclarations) { - append("- ").appendLine(d.symbol.fqName) - } - appendLine() - - appendLine("Variable") - append("- FQName: ").appendLine(symbol.fqName) - append("- ElementKind: ").appendLine(symbol.source?.kind?.let { it::class.simpleName }) - appendLine(symbol.source?.getElementTextInContextForDebug()) - appendLine() - - if (containingDeclaration != null) { - appendLine("Containing Declaration") - append("- FQName: ").appendLine(containingDeclaration.symbol.fqName) - append("- ElementKind: ").appendLine(containingDeclaration.source?.kind?.let { it::class.simpleName }) - appendLine(containingDeclaration.source?.getElementTextInContextForDebug()) - appendLine() - } - } - } + require(visited.add(this)) { buildRecursionErrorMessage(this, symbol, context) } for (previousNode in previousCfgNodes) { if (edgeFrom(previousNode).kind.isBack) continue @@ -348,20 +310,41 @@ private val FirVariableSymbol<*>.isLocal: Boolean else -> false } -private fun CFGNode<*>.containingDeclaration(): FirDeclaration? { +fun buildRecursionErrorMessage( + problemNode: CFGNode<*>, + symbol: FirVariableSymbol<*>, + context: CheckerContext, +): String { + return buildString { + appendLine("Node has already been visited and could result in infinite recursion.") + appendLine() + append("File Path: ").appendLine(context.containingFilePath) + append("Variable: ").appendLine(symbol.getDebugFqName()) + appendLine("Declarations:") + problemNode.firstGraphDeclaration()?.let { declaration -> + append("- ").append(declaration.symbol.getDebugFqName()).appendLine(" (graph declaration)") + } + for (declaration in context.containingDeclarations) { + append("- ").appendLine(declaration.symbol.getDebugFqName()) + } + } +} + +private fun CFGNode<*>.firstGraphDeclaration(): FirDeclaration? { owner.declaration?.let { return it } - return owner.enterNode.previousNodes.firstNotNullOfOrNull { it.containingDeclaration() } + return owner.enterNode.previousNodes.firstNotNullOfOrNull { it.firstGraphDeclaration() } } @OptIn(SymbolInternals::class) -private val FirBasedSymbol<*>.fqName: FqName - get() = when (val fir = this.fir) { +private fun FirBasedSymbol<*>.getDebugFqName(): FqName { + return when (val fir = this.fir) { is FirFile -> fir.packageFqName.child(Name.identifier(fir.name)) is FirScript -> fir.symbol.fqName is FirClassLikeDeclaration -> fir.symbol.classId.asSingleFqName() - is FirTypeParameter -> fir.containingDeclarationSymbol.fqName.child(fir.name) - is FirAnonymousInitializer -> (fir.containingDeclarationSymbol?.fqName ?: FqName.ROOT).child(Name.special("")) - is FirCallableDeclaration -> fir.symbol.callableId.asSingleFqName() + is FirTypeParameter -> fir.containingDeclarationSymbol.getDebugFqName().child(fir.name) + is FirAnonymousInitializer -> (fir.containingDeclarationSymbol?.getDebugFqName() ?: FqName.ROOT).child(Name.special("")) + is FirCallableDeclaration -> fir.symbol.callableId.asFqNameForDebugInfo() is FirCodeFragment -> FqName.topLevel(Name.special("")) is FirDanglingModifierList -> FqName.topLevel(Name.special("")) } +} 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 1f21351c0503d..013c42b11dc52 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 @@ -121,11 +121,11 @@ private class PropertyDeclarationCollector( } override fun visitWhileLoop(whileLoop: FirWhileLoop, data: FirStatement?) { - visitRepeatable(whileLoop, whileLoop) + visitRepeatable(whileLoop, data) } override fun visitDoWhileLoop(doWhileLoop: FirDoWhileLoop, data: FirStatement?) { - visitRepeatable(doWhileLoop, doWhileLoop) + visitRepeatable(doWhileLoop, data) } override fun visitAnonymousFunction(anonymousFunction: FirAnonymousFunction, data: FirStatement?) { diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/reassignmenGraphLoop.fir.kt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/reassignmenGraphLoop.fir.kt new file mode 100644 index 0000000000000..f29aa89291e8d --- /dev/null +++ b/compiler/testData/diagnostics/tests/controlFlowAnalysis/reassignmenGraphLoop.fir.kt @@ -0,0 +1,17 @@ +fun test(loop: Boolean) { + while (loop) { + try { + do { + run { + val a: String + if (loop) { + a = "" + } else { + a = "" + } + } + } while (loop) + } catch (e: Exception) { + } + } +} diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/reassignmenGraphLoop.kt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/reassignmenGraphLoop.kt new file mode 100644 index 0000000000000..bee6a4335e9ed --- /dev/null +++ b/compiler/testData/diagnostics/tests/controlFlowAnalysis/reassignmenGraphLoop.kt @@ -0,0 +1,17 @@ +fun test(loop: Boolean) { + while (loop) { + try { + do { + run { + val a: String + if (loop) { + a = "" + } else { + a = "" + } + } + } while (loop) + } catch (e: Exception) { + } + } +} diff --git a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java index ae2cf788cac84..3a2e452511d1f 100644 --- a/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java +++ b/compiler/tests-common-new/tests-gen/org/jetbrains/kotlin/test/runners/DiagnosticTestGenerated.java @@ -8566,6 +8566,12 @@ public void testPropertiesOrderInPackage() { runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/propertiesOrderInPackage.kt"); } + @Test + @TestMetadata("reassignmenGraphLoop.kt") + public void testReassignmenGraphLoop() { + runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/reassignmenGraphLoop.kt"); + } + @Test @TestMetadata("reassignmentInCatch.kt") public void testReassignmentInCatch() {