From 3ec9599bc47d8a4f4591d391f2c29822f70d123b Mon Sep 17 00:00:00 2001 From: Andrey Zinovyev Date: Tue, 10 Aug 2021 13:59:54 +0300 Subject: [PATCH] [FIR][CFG] Partial support of postponed Nothing calls In try blocks, last call won't be completed when building node for it This is workaround to partially reconstruct nothing stub node for such calls. Should work for non-local returns in try only. #KT-48160 Fixed --- ...irOldFrontendDiagnosticsTestGenerated.java | 6 +++ ...DiagnosticsWithLightTreeTestGenerated.java | 6 +++ .../dfa/cfg/ControlFlowGraphBuilder.kt | 48 ++++++++++++++++++- .../lambdaInTryFalsePositive.kt | 24 ++++++++++ .../lambdaInTryFalsePositive.txt | 4 ++ .../test/runners/DiagnosticTestGenerated.java | 6 +++ ...CompilerTestFE10TestdataTestGenerated.java | 6 +++ 7 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturn/lambdaInTryFalsePositive.kt create mode 100644 compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturn/lambdaInTryFalsePositive.txt diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java index 009b4b4b2c3..7a7715cc596 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsTestGenerated.java @@ -5908,6 +5908,12 @@ public class FirOldFrontendDiagnosticsTestGenerated extends AbstractFirDiagnosti runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturn/kt4034.kt"); } + @Test + @TestMetadata("lambdaInTryFalsePositive.kt") + public void testLambdaInTryFalsePositive() throws Exception { + runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturn/lambdaInTryFalsePositive.kt"); + } + @Test @TestMetadata("ReturnFromFunctionInObject.kt") public void testReturnFromFunctionInObject() throws Exception { diff --git a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java index 578c2b12672..611d7b54ac0 100644 --- a/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests-gen/org/jetbrains/kotlin/test/runners/FirOldFrontendDiagnosticsWithLightTreeTestGenerated.java @@ -5908,6 +5908,12 @@ public class FirOldFrontendDiagnosticsWithLightTreeTestGenerated extends Abstrac runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturn/kt4034.kt"); } + @Test + @TestMetadata("lambdaInTryFalsePositive.kt") + public void testLambdaInTryFalsePositive() throws Exception { + runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturn/lambdaInTryFalsePositive.kt"); + } + @Test @TestMetadata("ReturnFromFunctionInObject.kt") public void testReturnFromFunctionInObject() throws Exception { 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 b0736220a63..41d8349952f 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 @@ -104,6 +104,8 @@ class ControlFlowGraphBuilder { private val exitElvisExpressionNodes: Stack = stackOf() private val elvisRhsEnterNodes: Stack = stackOf() + private val notCompletedFunctionCalls: Stack> = stackOf() + /* * ignoredFunctionCalls is needed for resolve of += operator: * we have two different calls for resolve, but we left only one of them, @@ -875,7 +877,7 @@ class ControlFlowGraphBuilder { finallyEnterNodes.push(finallyEnterNode) finallyExitNodes.push(createFinallyBlockExitNode(tryExpression)) } - + notCompletedFunctionCalls.push(mutableListOf()) return enterTryExpressionNode to enterTryNodeBlock } @@ -966,6 +968,9 @@ class ControlFlowGraphBuilder { catchNodeStorages.pop() val catchExitNodes = catchExitNodeStorages.pop() val tryMainExitNode = tryMainExitNodes.pop() + + notCompletedFunctionCalls.pop().forEach(::completeFunctionCall) + val node = tryExitNodes.pop() node.updateDeadStatus() lastNodes.push(node) @@ -985,6 +990,29 @@ class ControlFlowGraphBuilder { return node to unionNode } + //this is a workaround to make function call dead when call is completed _after_ building its node in the graph + //this happens when completing the last call in try/catch blocks + //todo this doesn't make fully 'right' Nothing node (doesn't support going to catch and pass through finally) + // because doing those afterwards is quite challenging + // it would be much easier if we could build calls after full completion only, at least for Nothing calls + private fun completeFunctionCall(node: FunctionCallNode) { + if (!node.fir.resultType.isNothing) return + val stub = withLevelOfNode(node) { createStubNode() } + val edges = node.followingNodes.map { it to node.outgoingEdges.getValue(it) } + CFGNode.removeAllOutgoingEdges(node) + addEdge(node, stub) + for ((to, edge) in edges) { + addEdge( + from = stub, + to = to, + isBack = edge.kind.isBack, + preferredKind = edge.kind, + label = edge.label + ) + } + stub.followingNodes.forEach { propagateDeadnessForward(it, deep = true) } + } + // ----------------------------------- Resolvable call ----------------------------------- fun exitQualifiedAccessExpression(qualifiedAccessExpression: FirQualifiedAccessExpression): QualifiedAccessNode { @@ -1026,6 +1054,9 @@ class ControlFlowGraphBuilder { } else { addNewSimpleNode(node, preferredKind = kind) } + if (!returnsNothing && !callCompleted) { + notCompletedFunctionCalls.topOrNull()?.add(node) + } return node to unionNode } @@ -1474,13 +1505,16 @@ class ControlFlowGraphBuilder { addEdge(from, to, propagateDeadness = false, isDead = isDead, isBack = true, preferredKind = EdgeKind.CfgBackward, label = label) } - private fun propagateDeadnessForward(node: CFGNode<*>) { + private fun propagateDeadnessForward(node: CFGNode<*>, deep: Boolean = false) { if (!node.isDead) return node.followingNodes .filter { node.outgoingEdges.getValue(it).kind == EdgeKind.Forward } .forEach { target -> CFGNode.addJustKindEdge(node, target, EdgeKind.DeadForward, false) target.updateDeadStatus() + if (deep) { + propagateDeadnessForward(target, true) + } } } @@ -1506,6 +1540,16 @@ class ControlFlowGraphBuilder { return addNewSimpleNode(newNode, isDead) } + private fun withLevelOfNode(node: CFGNode<*>, f: () -> R): R { + val last = levelCounter + levelCounter = node.level + try { + return f() + } finally { + levelCounter = last + } + } + } fun FirDeclaration?.isLocalClassOrAnonymousObject() = ((this as? FirRegularClass)?.isLocal == true) || this is FirAnonymousObject diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturn/lambdaInTryFalsePositive.kt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturn/lambdaInTryFalsePositive.kt new file mode 100644 index 00000000000..45a6214cc8b --- /dev/null +++ b/compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturn/lambdaInTryFalsePositive.kt @@ -0,0 +1,24 @@ +//KT-48160 +// FIR_IDENTICAL +// FULL_JDK +// WITH_STDLIB + +import java.io.File + +inline fun T.use(block: (T) -> R): R { + return block(this) +} + +fun foo(): Int? { + try { + File("123").bufferedWriter().use { + return 45 + } + } catch (e: Exception) { + if (e.message?.startsWith("Remote does not have ") == true) { + return null + } + + return null + } +} \ No newline at end of file diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturn/lambdaInTryFalsePositive.txt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturn/lambdaInTryFalsePositive.txt new file mode 100644 index 00000000000..3209ba53a74 --- /dev/null +++ b/compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturn/lambdaInTryFalsePositive.txt @@ -0,0 +1,4 @@ +package + +public fun foo(): kotlin.Int? +public inline fun T.use(/*0*/ block: (T) -> R): R 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 5ecb7f37953..d86485f1141 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 @@ -5914,6 +5914,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest { runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturn/kt4034.kt"); } + @Test + @TestMetadata("lambdaInTryFalsePositive.kt") + public void testLambdaInTryFalsePositive() throws Exception { + runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturn/lambdaInTryFalsePositive.kt"); + } + @Test @TestMetadata("ReturnFromFunctionInObject.kt") public void testReturnFromFunctionInObject() throws Exception { diff --git a/idea/idea-frontend-fir/idea-fir-low-level-api/tests/org/jetbrains/kotlin/idea/fir/low/level/api/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java b/idea/idea-frontend-fir/idea-fir-low-level-api/tests/org/jetbrains/kotlin/idea/fir/low/level/api/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java index db2918db3bd..8c0e3030ba6 100644 --- a/idea/idea-frontend-fir/idea-fir-low-level-api/tests/org/jetbrains/kotlin/idea/fir/low/level/api/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java +++ b/idea/idea-frontend-fir/idea-fir-low-level-api/tests/org/jetbrains/kotlin/idea/fir/low/level/api/diagnostic/compiler/based/DiagnosisCompilerTestFE10TestdataTestGenerated.java @@ -5908,6 +5908,12 @@ public class DiagnosisCompilerTestFE10TestdataTestGenerated extends AbstractDiag runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturn/kt4034.kt"); } + @Test + @TestMetadata("lambdaInTryFalsePositive.kt") + public void testLambdaInTryFalsePositive() throws Exception { + runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturn/lambdaInTryFalsePositive.kt"); + } + @Test @TestMetadata("ReturnFromFunctionInObject.kt") public void testReturnFromFunctionInObject() throws Exception {