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.
This commit is contained in:
pyos
2022-11-28 17:15:56 +01:00
committed by Dmitriy Novozhilov
parent ef2fa01a8d
commit e7e5569539
6 changed files with 43 additions and 96 deletions
@@ -83,38 +83,6 @@ digraph delegateWithAnonymousObject_kt {
24 -> {25}; 24 -> {25};
subgraph cluster_9 { 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 <anonymous object>" style="filled" fillcolor=red];
34 [label="Exit class <anonymous object>" 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 color=red
35 [label="Enter function <init>" style="filled" fillcolor=red]; 35 [label="Enter function <init>" style="filled" fillcolor=red];
36 [label="Delegated constructor call: super<R|kotlin/Any|>()" style="filled" fillcolor=yellow]; 36 [label="Delegated constructor call: super<R|kotlin/Any|>()" style="filled" fillcolor=yellow];
@@ -123,10 +91,10 @@ digraph delegateWithAnonymousObject_kt {
35 -> {36}; 35 -> {36};
36 -> {37}; 36 -> {37};
subgraph cluster_13 { subgraph cluster_10 {
color=red color=red
38 [label="Enter function getValue" style="filled" fillcolor=red]; 38 [label="Enter function getValue" style="filled" fillcolor=red];
subgraph cluster_14 { subgraph cluster_11 {
color=blue color=blue
39 [label="Enter block"]; 39 [label="Enter block"];
40 [label="Function call: R|/IssueListView.IssueListView|()" style="filled" fillcolor=yellow]; 40 [label="Function call: R|/IssueListView.IssueListView|()" style="filled" fillcolor=yellow];
@@ -144,10 +112,10 @@ digraph delegateWithAnonymousObject_kt {
42 -> {43} [style=dotted]; 42 -> {43} [style=dotted];
43 -> {44} [style=dotted]; 43 -> {44} [style=dotted];
subgraph cluster_15 { subgraph cluster_12 {
color=red color=red
45 [label="Enter function setValue" style="filled" fillcolor=red]; 45 [label="Enter function setValue" style="filled" fillcolor=red];
subgraph cluster_16 { subgraph cluster_13 {
color=blue color=blue
46 [label="Enter block"]; 46 [label="Enter block"];
47 [label="Function call: R|/IssueListView.IssueListView|()" style="filled" fillcolor=yellow]; 47 [label="Function call: R|/IssueListView.IssueListView|()" style="filled" fillcolor=yellow];
@@ -169,10 +137,10 @@ digraph delegateWithAnonymousObject_kt {
51 -> {52} [style=dotted]; 51 -> {52} [style=dotted];
52 -> {53} [style=dotted]; 52 -> {53} [style=dotted];
subgraph cluster_17 { subgraph cluster_14 {
color=red color=red
54 [label="Enter function getter" style="filled" fillcolor=red]; 54 [label="Enter function getter" style="filled" fillcolor=red];
subgraph cluster_18 { subgraph cluster_15 {
color=blue color=blue
55 [label="Enter block"]; 55 [label="Enter block"];
56 [label="Access variable D|/IssuesListUserProfile.issueListView|"]; 56 [label="Access variable D|/IssuesListUserProfile.issueListView|"];
@@ -194,10 +162,10 @@ digraph delegateWithAnonymousObject_kt {
60 -> {61} [style=dotted]; 60 -> {61} [style=dotted];
61 -> {62} [style=dotted]; 61 -> {62} [style=dotted];
subgraph cluster_19 { subgraph cluster_16 {
color=red color=red
63 [label="Enter function setter" style="filled" fillcolor=red]; 63 [label="Enter function setter" style="filled" fillcolor=red];
subgraph cluster_20 { subgraph cluster_17 {
color=blue color=blue
64 [label="Enter block"]; 64 [label="Enter block"];
65 [label="Access variable D|/IssuesListUserProfile.issueListView|"]; 65 [label="Access variable D|/IssuesListUserProfile.issueListView|"];
@@ -216,10 +184,28 @@ digraph delegateWithAnonymousObject_kt {
68 -> {69}; 68 -> {69};
69 -> {70}; 69 -> {70};
subgraph cluster_21 { subgraph cluster_18 {
color=red color=red
71 [label="Enter property" style="filled" fillcolor=red]; 71 [label="Enter property" style="filled" fillcolor=red];
72 [label="Postponed enter to lambda"]; 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 <anonymous object>" style="filled" fillcolor=red];
34 [label="Exit class <anonymous object>" 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"]; 73 [label="Postponed exit from lambda"];
74 [label="Function call: this@R|/IssuesListUserProfile|.R|/delegate|<R|IssuesListUserProfile|, R|IssuesListUserProfile|, R|IssueListView|>(...)" style="filled" fillcolor=yellow]; 74 [label="Function call: this@R|/IssuesListUserProfile|.R|/delegate|<R|IssuesListUserProfile|, R|IssuesListUserProfile|, R|IssueListView|>(...)" style="filled" fillcolor=yellow];
75 [label="Access variable this@R|/IssuesListUserProfile|"]; 75 [label="Access variable this@R|/IssuesListUserProfile|"];
@@ -228,7 +214,7 @@ digraph delegateWithAnonymousObject_kt {
78 [label="Exit property" style="filled" fillcolor=red]; 78 [label="Exit property" style="filled" fillcolor=red];
} }
71 -> {72}; 71 -> {72};
72 -> {73 74}; 72 -> {26 73 74};
72 -> {26} [style=dashed]; 72 -> {26} [style=dashed];
73 -> {74} [color=green]; 73 -> {74} [color=green];
73 -> {77} [color=red]; 73 -> {77} [color=red];
@@ -237,6 +223,19 @@ digraph delegateWithAnonymousObject_kt {
76 -> {77}; 76 -> {77};
77 -> {78}; 77 -> {78};
78 -> {81} [color=green]; 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 { subgraph cluster_22 {
color=red color=red
@@ -143,10 +143,6 @@ abstract class FirDataFlowAnalyzer(
returnExpressionsOfAnonymousFunctionOrNull(function) returnExpressionsOfAnonymousFunctionOrNull(function)
?: error("anonymous function ${function.render()} not analyzed") ?: error("anonymous function ${function.render()} not analyzed")
fun dropSubgraphFromCall(call: FirFunctionCall) {
graphBuilder.dropSubgraphFromCall(call)
}
// ----------------------------------- Named function ----------------------------------- // ----------------------------------- Named function -----------------------------------
fun enterFunction(function: FirFunction) { fun enterFunction(function: FirFunction) {
@@ -1381,34 +1381,6 @@ class ControlFlowGraphBuilder {
lastNodes.reset() lastNodes.reset()
} }
fun dropSubgraphFromCall(call: FirFunctionCall) {
val graphs = mutableListOf<ControlFlowGraph>()
call.acceptChildren(object : FirDefaultVisitor<Unit, Any?>() {
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 ----------------------------------- // ----------------------------------- Edge utils -----------------------------------
private fun addNewSimpleNode(node: CFGNode<*>, isDead: Boolean = false) { private fun addNewSimpleNode(node: CFGNode<*>, isDead: Boolean = false) {
@@ -336,6 +336,8 @@ open class FirDeclarationsResolveTransformer(transformer: FirAbstractBodyResolve
val provideDelegateCall = wrappedDelegateExpression.delegateProvider as FirFunctionCall val provideDelegateCall = wrappedDelegateExpression.delegateProvider as FirFunctionCall
// Resolve call for provideDelegate, without completion // 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) provideDelegateCall.transformSingle(this, ResolutionMode.ContextIndependent)
// If we got successful candidate for provideDelegate, let's select it // If we got successful candidate for provideDelegate, let's select it
@@ -365,9 +367,6 @@ open class FirDeclarationsResolveTransformer(transformer: FirAbstractBodyResolve
return provideDelegateCall return provideDelegateCall
} }
// Otherwise, rollback
(provideDelegateCall as? FirFunctionCall)?.let { dataFlowAnalyzer.dropSubgraphFromCall(it) }
// Select delegate expression otherwise // Select delegate expression otherwise
return delegateExpression return delegateExpression
} }
@@ -55,15 +55,6 @@ sealed class CFGNode<out E : FirElement>(val owner: ControlFlowGraph, val level:
return true return true
} }
@CfgInternals
fun removeAllIncomingEdges(to: CFGNode<*>) {
for (from in to._previousNodes) {
from._followingNodes.remove(to)
}
to._incomingEdges = null
to._previousNodes.clear()
}
@CfgInternals @CfgInternals
fun removeAllOutgoingEdges(from: CFGNode<*>) { fun removeAllOutgoingEdges(from: CFGNode<*>) {
for (to in from._followingNodes) { for (to in from._followingNodes) {
@@ -59,16 +59,6 @@ class ControlFlowGraph(val declaration: FirDeclaration?, val name: String, val k
_subGraphs += graph _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) { private fun assertState(state: State) {
assert(this.state == state) { assert(this.state == state) {
"This action can not be performed at $this state" "This action can not be performed at $this state"