From e7e55695393a3da65648f7a5fecdbbd1c468be95 Mon Sep 17 00:00:00 2001 From: pyos Date: Mon, 28 Nov 2022 17:15:56 +0100 Subject: [PATCH] FIR CFA: do not remove provideDelegate subgraphs when rolling back The receiver of the provideDelegate call is the same FirExpression as the delegate itself, so there's only one copy of the nodes in the first place; trying to remove subgraphs completely detaches objects inside it from the parent graph, which is not great for checkers. Note that currently if provideDelegate is not selected, there will be a stray FunctionCallExit node in the control flow graph. This commit *does not change that*. It has been there for a while. Don't @ me. I'll try to fix that. No promises. --- .../delegates/delegateWithAnonymousObject.dot | 83 +++++++++---------- .../fir/resolve/dfa/FirDataFlowAnalyzer.kt | 4 - .../dfa/cfg/ControlFlowGraphBuilder.kt | 28 ------- .../FirDeclarationsResolveTransformer.kt | 5 +- .../kotlin/fir/resolve/dfa/cfg/CFGNode.kt | 9 -- .../fir/resolve/dfa/cfg/ControlFlowGraph.kt | 10 --- 6 files changed, 43 insertions(+), 96 deletions(-) diff --git a/compiler/fir/analysis-tests/testData/resolveWithStdlib/delegates/delegateWithAnonymousObject.dot b/compiler/fir/analysis-tests/testData/resolveWithStdlib/delegates/delegateWithAnonymousObject.dot index 63ee8e62f44..543a2064e3c 100644 --- a/compiler/fir/analysis-tests/testData/resolveWithStdlib/delegates/delegateWithAnonymousObject.dot +++ b/compiler/fir/analysis-tests/testData/resolveWithStdlib/delegates/delegateWithAnonymousObject.dot @@ -83,38 +83,6 @@ digraph delegateWithAnonymousObject_kt { 24 -> {25}; subgraph cluster_9 { - color=red - 26 [label="Enter function anonymousFunction" style="filled" fillcolor=red]; - subgraph cluster_10 { - color=blue - 27 [label="Enter block"]; - 28 [label="Enter anonymous object"]; - subgraph cluster_11 { - color=blue - 33 [label="Enter class " style="filled" fillcolor=red]; - 34 [label="Exit class " style="filled" fillcolor=red]; - } - 29 [label="Exit anonymous object"]; - 30 [label="Exit anonymous object expression"]; - 31 [label="Exit block"]; - } - 32 [label="Exit function anonymousFunction" style="filled" fillcolor=red]; - } - 26 -> {27}; - 27 -> {28}; - 27 -> {35 38 45} [color=red]; - 28 -> {33} [color=green]; - 28 -> {29} [color=red]; - 28 -> {33} [style=dashed]; - 29 -> {30}; - 29 -> {35 38 45} [color=green]; - 29 -> {35 38 45} [style=dashed]; - 30 -> {31}; - 31 -> {32}; - 33 -> {34} [color=green]; - 34 -> {29} [color=green]; - - subgraph cluster_12 { color=red 35 [label="Enter function " style="filled" fillcolor=red]; 36 [label="Delegated constructor call: super()" style="filled" fillcolor=yellow]; @@ -123,10 +91,10 @@ digraph delegateWithAnonymousObject_kt { 35 -> {36}; 36 -> {37}; - subgraph cluster_13 { + subgraph cluster_10 { color=red 38 [label="Enter function getValue" style="filled" fillcolor=red]; - subgraph cluster_14 { + subgraph cluster_11 { color=blue 39 [label="Enter block"]; 40 [label="Function call: R|/IssueListView.IssueListView|()" style="filled" fillcolor=yellow]; @@ -144,10 +112,10 @@ digraph delegateWithAnonymousObject_kt { 42 -> {43} [style=dotted]; 43 -> {44} [style=dotted]; - subgraph cluster_15 { + subgraph cluster_12 { color=red 45 [label="Enter function setValue" style="filled" fillcolor=red]; - subgraph cluster_16 { + subgraph cluster_13 { color=blue 46 [label="Enter block"]; 47 [label="Function call: R|/IssueListView.IssueListView|()" style="filled" fillcolor=yellow]; @@ -169,10 +137,10 @@ digraph delegateWithAnonymousObject_kt { 51 -> {52} [style=dotted]; 52 -> {53} [style=dotted]; - subgraph cluster_17 { + subgraph cluster_14 { color=red 54 [label="Enter function getter" style="filled" fillcolor=red]; - subgraph cluster_18 { + subgraph cluster_15 { color=blue 55 [label="Enter block"]; 56 [label="Access variable D|/IssuesListUserProfile.issueListView|"]; @@ -194,10 +162,10 @@ digraph delegateWithAnonymousObject_kt { 60 -> {61} [style=dotted]; 61 -> {62} [style=dotted]; - subgraph cluster_19 { + subgraph cluster_16 { color=red 63 [label="Enter function setter" style="filled" fillcolor=red]; - subgraph cluster_20 { + subgraph cluster_17 { color=blue 64 [label="Enter block"]; 65 [label="Access variable D|/IssuesListUserProfile.issueListView|"]; @@ -216,10 +184,28 @@ digraph delegateWithAnonymousObject_kt { 68 -> {69}; 69 -> {70}; - subgraph cluster_21 { + subgraph cluster_18 { color=red 71 [label="Enter property" style="filled" fillcolor=red]; 72 [label="Postponed enter to lambda"]; + subgraph cluster_19 { + color=blue + 26 [label="Enter function anonymousFunction" style="filled" fillcolor=red]; + subgraph cluster_20 { + color=blue + 27 [label="Enter block"]; + 28 [label="Enter anonymous object"]; + subgraph cluster_21 { + color=blue + 33 [label="Enter class " style="filled" fillcolor=red]; + 34 [label="Exit class " style="filled" fillcolor=red]; + } + 29 [label="Exit anonymous object"]; + 30 [label="Exit anonymous object expression"]; + 31 [label="Exit block"]; + } + 32 [label="Exit function anonymousFunction" style="filled" fillcolor=red]; + } 73 [label="Postponed exit from lambda"]; 74 [label="Function call: this@R|/IssuesListUserProfile|.R|/delegate|(...)" style="filled" fillcolor=yellow]; 75 [label="Access variable this@R|/IssuesListUserProfile|"]; @@ -228,7 +214,7 @@ digraph delegateWithAnonymousObject_kt { 78 [label="Exit property" style="filled" fillcolor=red]; } 71 -> {72}; - 72 -> {73 74}; + 72 -> {26 73 74}; 72 -> {26} [style=dashed]; 73 -> {74} [color=green]; 73 -> {77} [color=red]; @@ -237,6 +223,19 @@ digraph delegateWithAnonymousObject_kt { 76 -> {77}; 77 -> {78}; 78 -> {81} [color=green]; + 26 -> {27}; + 27 -> {28}; + 27 -> {35 38 45} [color=red]; + 28 -> {33} [color=green]; + 28 -> {29} [color=red]; + 28 -> {33} [style=dashed]; + 29 -> {30}; + 29 -> {35 38 45} [color=green]; + 29 -> {35 38 45} [style=dashed]; + 30 -> {31}; + 31 -> {32}; + 33 -> {34} [color=green]; + 34 -> {29} [color=green]; subgraph cluster_22 { color=red diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt index 225959cbea4..6edef348c20 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/dfa/FirDataFlowAnalyzer.kt @@ -143,10 +143,6 @@ abstract class FirDataFlowAnalyzer( returnExpressionsOfAnonymousFunctionOrNull(function) ?: error("anonymous function ${function.render()} not analyzed") - fun dropSubgraphFromCall(call: FirFunctionCall) { - graphBuilder.dropSubgraphFromCall(call) - } - // ----------------------------------- Named function ----------------------------------- fun enterFunction(function: FirFunction) { 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 ed0617a0e7d..3285061e404 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 @@ -1381,34 +1381,6 @@ class ControlFlowGraphBuilder { lastNodes.reset() } - fun dropSubgraphFromCall(call: FirFunctionCall) { - val graphs = mutableListOf() - - call.acceptChildren(object : FirDefaultVisitor() { - override fun visitElement(element: FirElement, data: Any?) { - element.acceptChildren(this, null) - } - - override fun visitAnonymousFunction(anonymousFunction: FirAnonymousFunction, data: Any?) { - anonymousFunction.controlFlowGraphReference?.accept(this, null) - } - - override fun visitAnonymousObject(anonymousObject: FirAnonymousObject, data: Any?) { - anonymousObject.controlFlowGraphReference?.accept(this, null) - } - - override fun visitControlFlowGraphReference(controlFlowGraphReference: FirControlFlowGraphReference, data: Any?) { - val graph = controlFlowGraphReference.controlFlowGraph ?: return - if (graph.owner == null) return - graphs += graph - } - }, null) - - for (graph in graphs) { - currentGraph.removeSubGraph(graph) - } - } - // ----------------------------------- Edge utils ----------------------------------- private fun addNewSimpleNode(node: CFGNode<*>, isDead: Boolean = false) { diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirDeclarationsResolveTransformer.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirDeclarationsResolveTransformer.kt index 4ddf9c2ff32..b676eb3f042 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirDeclarationsResolveTransformer.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirDeclarationsResolveTransformer.kt @@ -336,6 +336,8 @@ open class FirDeclarationsResolveTransformer(transformer: FirAbstractBodyResolve val provideDelegateCall = wrappedDelegateExpression.delegateProvider as FirFunctionCall // Resolve call for provideDelegate, without completion + // TODO: this generates some nodes in the control flow graph which we don't want if we + // end up not selecting this option. provideDelegateCall.transformSingle(this, ResolutionMode.ContextIndependent) // If we got successful candidate for provideDelegate, let's select it @@ -365,9 +367,6 @@ open class FirDeclarationsResolveTransformer(transformer: FirAbstractBodyResolve return provideDelegateCall } - // Otherwise, rollback - (provideDelegateCall as? FirFunctionCall)?.let { dataFlowAnalyzer.dropSubgraphFromCall(it) } - // Select delegate expression otherwise return delegateExpression } 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 77c4f80f201..b9c138b9876 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 @@ -55,15 +55,6 @@ sealed class CFGNode(val owner: ControlFlowGraph, val level: return true } - @CfgInternals - fun removeAllIncomingEdges(to: CFGNode<*>) { - for (from in to._previousNodes) { - from._followingNodes.remove(to) - } - to._incomingEdges = null - to._previousNodes.clear() - } - @CfgInternals fun removeAllOutgoingEdges(from: CFGNode<*>) { for (to in from._followingNodes) { diff --git a/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/ControlFlowGraph.kt b/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/ControlFlowGraph.kt index 98700464e42..873f209258c 100644 --- a/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/ControlFlowGraph.kt +++ b/compiler/fir/semantics/src/org/jetbrains/kotlin/fir/resolve/dfa/cfg/ControlFlowGraph.kt @@ -59,16 +59,6 @@ class ControlFlowGraph(val declaration: FirDeclaration?, val name: String, val k _subGraphs += graph } - @CfgInternals - fun removeSubGraph(graph: ControlFlowGraph) { - assert(graph.owner == this) - _subGraphs.remove(graph) - graph.owner = null - - CFGNode.removeAllIncomingEdges(graph.enterNode) - CFGNode.removeAllOutgoingEdges(graph.exitNode) - } - private fun assertState(state: State) { assert(this.state == state) { "This action can not be performed at $this state"