[FIR] Fix node ordering for delegating constructor graph

Inject delegated constructor and other in-place initializer sub-graphs
after the delegated constructor call node. This ensures property
initialization and use is calculated correctly when there are complex
calculations for the arguments to the delegated constructor.

#KT-59708 Fixed
#KT-59832 Fixed
This commit is contained in:
Brian Norman
2023-07-11 15:07:02 -05:00
committed by Space Team
parent 4797d30837
commit d2ad426350
11 changed files with 310 additions and 58 deletions
@@ -6258,6 +6258,12 @@ public class DiagnosticCompilerTestFE10TestdataTestGenerated extends AbstractDia
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturnInWhen.kt");
}
@Test
@TestMetadata("delegatedConstructorArguments.kt")
public void testDelegatedConstructorArguments() throws Exception {
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/delegatedConstructorArguments.kt");
}
@Test
@TestMetadata("delegatedMemberProperyWriteInInit.kt")
public void testDelegatedMemberProperyWriteInInit() throws Exception {
@@ -6258,6 +6258,12 @@ public class LLFirPreresolvedReversedDiagnosticCompilerFE10TestDataTestGenerated
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturnInWhen.kt");
}
@Test
@TestMetadata("delegatedConstructorArguments.kt")
public void testDelegatedConstructorArguments() throws Exception {
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/delegatedConstructorArguments.kt");
}
@Test
@TestMetadata("delegatedMemberProperyWriteInInit.kt")
public void testDelegatedMemberProperyWriteInInit() throws Exception {
@@ -8,43 +8,43 @@ digraph secondaryConstructorCfg_kt {
0 [label="Enter class B" style="filled" fillcolor=red];
subgraph cluster_1 {
color=blue
1 [label="Enter property" style="filled" fillcolor=red];
1 [label="Enter function <init>" style="filled" fillcolor=red];
2 [label="Access variable R|<local>/p0|"];
3 [label="Exit property" style="filled" fillcolor=red];
}
subgraph cluster_2 {
color=blue
4 [label="Enter property" style="filled" fillcolor=red];
5 [label="Access variable R|<local>/p0|"];
6 [label="Access variable R|kotlin/String.length|"];
7 [label="Exit property" style="filled" fillcolor=red];
}
subgraph cluster_3 {
color=blue
8 [label="Enter init block" style="filled" fillcolor=red];
3 [label="Delegated constructor call: this<R|B|>(...)" style="filled" fillcolor=yellow];
subgraph cluster_2 {
color=blue
4 [label="Enter property" style="filled" fillcolor=red];
5 [label="Access variable R|<local>/p0|"];
6 [label="Exit property" style="filled" fillcolor=red];
}
subgraph cluster_3 {
color=blue
7 [label="Enter property" style="filled" fillcolor=red];
8 [label="Access variable R|<local>/p0|"];
9 [label="Access variable R|kotlin/String.length|"];
10 [label="Exit property" style="filled" fillcolor=red];
}
subgraph cluster_4 {
color=blue
9 [label="Enter block"];
10 [label="Access variable R|<local>/p0|"];
11 [label="Access variable R|kotlin/String.length|"];
12 [label="Assignment: R|/B.p1|"];
13 [label="Const: String()"];
14 [label="Assignment: R|/B.p3|"];
15 [label="Exit block"];
11 [label="Enter init block" style="filled" fillcolor=red];
subgraph cluster_5 {
color=blue
12 [label="Enter block"];
13 [label="Access variable R|<local>/p0|"];
14 [label="Access variable R|kotlin/String.length|"];
15 [label="Assignment: R|/B.p1|"];
16 [label="Const: String()"];
17 [label="Assignment: R|/B.p3|"];
18 [label="Exit block"];
}
19 [label="Exit init block" style="filled" fillcolor=red];
}
subgraph cluster_6 {
color=blue
20 [label="Enter function <init>" style="filled" fillcolor=red];
21 [label="Delegated constructor call: super<R|kotlin/Any|>()" style="filled" fillcolor=yellow];
22 [label="Exit function <init>" style="filled" fillcolor=red];
}
16 [label="Exit init block" style="filled" fillcolor=red];
}
subgraph cluster_5 {
color=blue
17 [label="Enter function <init>" style="filled" fillcolor=red];
18 [label="Delegated constructor call: super<R|kotlin/Any|>()" style="filled" fillcolor=yellow];
19 [label="Exit function <init>" style="filled" fillcolor=red];
}
subgraph cluster_6 {
color=blue
20 [label="Enter function <init>" style="filled" fillcolor=red];
21 [label="Access variable R|<local>/p0|"];
22 [label="Delegated constructor call: this<R|B|>(...)" style="filled" fillcolor=yellow];
subgraph cluster_7 {
color=blue
23 [label="Enter block"];
@@ -56,31 +56,33 @@ digraph secondaryConstructorCfg_kt {
}
28 [label="Exit class B" style="filled" fillcolor=red];
}
0 -> {1} [color=green];
0 -> {1 4} [color=green];
0 -> {28} [style=dotted];
0 -> {1 4 8 17 20} [style=dashed];
0 -> {1 4 7 11 20} [style=dashed];
1 -> {2};
2 -> {3};
3 -> {4} [color=green];
3 -> {4} [color=green label="return@/B.B"];
3 -> {23} [color=red];
4 -> {5};
5 -> {6};
6 -> {7};
7 -> {8} [color=green];
6 -> {7} [color=green];
7 -> {8};
8 -> {9};
9 -> {10};
10 -> {11};
10 -> {11} [color=green];
11 -> {12};
12 -> {13};
13 -> {14};
14 -> {15};
15 -> {16};
16 -> {17} [color=green];
16 -> {17};
17 -> {18};
18 -> {19};
19 -> {20 28} [color=green];
19 -> {20} [color=green];
20 -> {21};
21 -> {22};
22 -> {23};
22 -> {23} [color=green label="return@/B.B"];
22 -> {28} [color=green];
23 -> {24};
24 -> {25};
25 -> {26};
@@ -6258,6 +6258,12 @@ public class FirLightTreeOldFrontendDiagnosticsTestGenerated extends AbstractFir
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturnInWhen.kt");
}
@Test
@TestMetadata("delegatedConstructorArguments.kt")
public void testDelegatedConstructorArguments() throws Exception {
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/delegatedConstructorArguments.kt");
}
@Test
@TestMetadata("delegatedMemberProperyWriteInInit.kt")
public void testDelegatedMemberProperyWriteInInit() throws Exception {
@@ -6264,6 +6264,12 @@ public class FirPsiOldFrontendDiagnosticsTestGenerated extends AbstractFirPsiDia
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturnInWhen.kt");
}
@Test
@TestMetadata("delegatedConstructorArguments.kt")
public void testDelegatedConstructorArguments() throws Exception {
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/delegatedConstructorArguments.kt");
}
@Test
@TestMetadata("delegatedMemberProperyWriteInInit.kt")
public void testDelegatedMemberProperyWriteInInit() throws Exception {
@@ -15,7 +15,7 @@ fun <I : ControlFlowInfo<I, *, *>> ControlFlowGraph.collectDataForNode(
direction: TraverseDirection,
visitor: PathAwareControlFlowGraphVisitor<I>,
): Map<CFGNode<*>, PathAwareControlFlowInfo<I>> {
val nodeMap = HashMap<CFGNode<*>, PathAwareControlFlowInfo<I>>()
val nodeMap = LinkedHashMap<CFGNode<*>, PathAwareControlFlowInfo<I>>()
var shouldContinue: Boolean
do {
shouldContinue = collectDataForNodeInternal(direction, visitor, nodeMap)
@@ -63,17 +63,16 @@ object UnusedChecker : AbstractFirPropertyInitializationChecker() {
if (node.fir.source == null) return
if (variableSymbol.isLoopIterator) return
val dataPerNode = data[node] ?: return
// TODO, KT-59832: merge values for labels, otherwise diagnostics are inconsistent
for (dataPerLabel in dataPerNode.values) {
val data = dataPerLabel[variableSymbol] ?: continue
val data = dataPerNode.values.mapNotNull { it[variableSymbol] }.reduceOrNull { acc, it -> acc.merge(it) }
if (data != null) {
variableSymbol.lazyResolveToPhase(FirResolvePhase.BODY_RESOLVE)
@OptIn(SymbolInternals::class)
val variable = variableSymbol.fir
val variableSource = variable.source
if (variableSource?.elementType == KtNodeTypes.DESTRUCTURING_DECLARATION) {
continue
return
}
when {
@@ -83,18 +82,15 @@ object UnusedChecker : AbstractFirPropertyInitializationChecker() {
node.fir.isCatchParameter == true -> {}
else -> {
reporter.reportOn(variableSource, FirErrors.UNUSED_VARIABLE, context)
break
}
}
}
data.isRedundantInit -> {
val source = variable.initializer?.source
reporter.reportOn(source, FirErrors.VARIABLE_INITIALIZER_IS_REDUNDANT, context)
break
}
data == VariableStatus.ONLY_WRITTEN_NEVER_READ -> {
reporter.reportOn(variableSource, FirErrors.VARIABLE_NEVER_READ, context)
break
}
else -> {
}
@@ -421,8 +421,11 @@ class ControlFlowGraphBuilder {
if (enterNode.previousNodes.isNotEmpty()) {
val firstInPlace = klass.firstInPlaceInitializedMember()
klass.forEachGraphOwner {
// For local classes, the first in-place initializer, all constructors when there are no in-place initializers, or any
// this-delegating constructors should have a forward edge from ClassEnterNode, while everything else should have a
// DFG-only forward edge.
val kind = if (firstInPlace == it ||
(firstInPlace == null && it is FirConstructor && it.delegatedConstructor?.isThis != true)
(it is FirConstructor && (firstInPlace == null || it.delegatedConstructor?.isThis == true))
) EdgeKind.Forward else EdgeKind.DfgForward
enterToLocalClassesMembers[(it as FirDeclaration).symbol] = enterNode to kind
}
@@ -492,11 +495,31 @@ class ControlFlowGraphBuilder {
}
for ((ctor, graph) in constructors) {
val delegatedConstructorExit = constructors[ctor.parentConstructor()]?.exitNode ?: lastInPlaceExit
// Similarly, if there are no in-place initializers, we already have control flow (+ data flow) edges
// from `enterNode` to all non-delegating constructors in local classes.
if (delegatedConstructorExit !== enterNode || delegatedConstructorExit.previousNodes.isEmpty()) {
addEdgeToSubGraph(delegatedConstructorExit, graph.enterNode)
val delegatedConstructorGraph = constructors[ctor.parentConstructor()]
if (delegatedConstructorGraph != null) {
// Inject delegated constructor and other in-place initializer sub-graphs after the delegated constructor call node. This
// ensures property initialization and use is calculated correctly when there are complex calculations for the arguments to
// the delegated constructor.
val delegatedConstructorEnter = calledInPlace.firstOrNull()?.enterNode ?: delegatedConstructorGraph.enterNode
val delegatedConstructorExit = delegatedConstructorGraph.exitNode
val delegatedConstructorCall = graph.nodes.single { it is DelegatedConstructorCallNode }
val edgeLabel = graph.exitNode as FunctionExitNode
val followingNodes = delegatedConstructorCall.followingNodes.toList()
CFGNode.removeAllOutgoingEdges(delegatedConstructorCall)
if (!isLocalClass) addEdgeToSubGraph(enterNode, graph.enterNode)
addEdgeToSubGraph(delegatedConstructorCall, delegatedConstructorEnter, label = edgeLabel)
for (node in followingNodes) {
addEdge(delegatedConstructorExit, node, preferredKind = EdgeKind.CfgForward, label = edgeLabel)
addEdge(delegatedConstructorCall, node, preferredKind = EdgeKind.DfgForward)
}
} else if (lastInPlaceExit !== enterNode || lastInPlaceExit.previousNodes.isEmpty()) {
// Similarly, if there are no in-place initializers, we already have control flow (+ data flow) edges
// from `enterNode` to all non-delegating constructors in local classes.
addEdgeToSubGraph(lastInPlaceExit, graph.enterNode)
}
addEdge(graph.exitNode, exitNode, preferredKind = if (exitNode.isUnion) EdgeKind.Forward else EdgeKind.CfgForward)
}
@@ -1322,11 +1345,11 @@ class ControlFlowGraphBuilder {
CFGNode.addEdge(from, to, kind, propagateDeadness, label)
}
private fun addEdgeToSubGraph(from: CFGNode<*>, to: CFGNode<*>) {
private fun addEdgeToSubGraph(from: CFGNode<*>, to: CFGNode<*>, label: EdgeLabel = NormalPath) {
val wasDead = to.isDead
val isDead = wasDead || from.isDead
// Can only add control flow since data flow for every node that follows `to` has already been computed.
CFGNode.addEdge(from, to, if (isDead) EdgeKind.DeadForward else EdgeKind.CfgForward, propagateDeadness = true)
CFGNode.addEdge(from, to, if (isDead) EdgeKind.DeadForward else EdgeKind.CfgForward, propagateDeadness = true, label)
if (isDead && !wasDead) {
propagateDeadnessForward(to)
}
@@ -0,0 +1,176 @@
digraph delegatedConstructorArguments_kt {
graph [nodesep=3]
node [shape=box penwidth=2]
edge [penwidth=2]
subgraph cluster_0 {
color=red
0 [label="Enter class Test" style="filled" fillcolor=red];
subgraph cluster_1 {
color=blue
1 [label="Enter function <init>" style="filled" fillcolor=red];
2 [label="Access variable R|<local>/set|"];
3 [label="Postponed enter to lambda"];
subgraph cluster_2 {
color=blue
4 [label="Enter function <anonymous>" style="filled" fillcolor=red];
subgraph cluster_3 {
color=blue
5 [label="Enter block"];
6 [label="Access variable R|<local>/it|"];
7 [label="Exit block"];
}
8 [label="Exit function <anonymous>" style="filled" fillcolor=red];
}
9 [label="Postponed exit from lambda"];
10 [label="Function call: R|<local>/set|.R|kotlin/collections/map|<R|kotlin/String|, R|kotlin/String|>(...)" style="filled" fillcolor=yellow];
11 [label="Delegated constructor call: this<R|Test|>(...)" style="filled" fillcolor=yellow];
subgraph cluster_4 {
color=blue
12 [label="Enter function <init>" style="filled" fillcolor=red];
13 [label="Access variable R|<local>/map|"];
14 [label="Access variable R|SubstitutionOverride<kotlin/collections/Map.values: R|kotlin/collections/Collection<kotlin/String>|>|"];
15 [label="Postponed enter to lambda"];
subgraph cluster_5 {
color=blue
16 [label="Enter function <anonymous>" style="filled" fillcolor=red];
subgraph cluster_6 {
color=blue
17 [label="Enter block"];
18 [label="Access variable R|<local>/it|"];
19 [label="Exit block"];
}
20 [label="Exit function <anonymous>" style="filled" fillcolor=red];
}
21 [label="Postponed exit from lambda"];
22 [label="Function call: R|<local>/map|.R|SubstitutionOverride<kotlin/collections/Map.values: R|kotlin/collections/Collection<kotlin/String>|>|.R|kotlin/collections/map|<R|kotlin/String|, R|kotlin/String|>(...)" style="filled" fillcolor=yellow];
23 [label="Delegated constructor call: this<R|Test|>(...)" style="filled" fillcolor=yellow];
subgraph cluster_7 {
color=blue
24 [label="Enter init block" style="filled" fillcolor=red];
subgraph cluster_8 {
color=blue
25 [label="Enter block"];
26 [label="Access variable this@R|/Test|"];
27 [label="Const: Int(0)"];
28 [label="Assignment: R|/Test.size|"];
29 [label="Exit block"];
}
30 [label="Exit init block" style="filled" fillcolor=red];
}
subgraph cluster_9 {
color=blue
31 [label="Enter function <init>" style="filled" fillcolor=red];
32 [label="Delegated constructor call: super<R|kotlin/Any|>()" style="filled" fillcolor=yellow];
subgraph cluster_10 {
color=blue
33 [label="Enter block"];
34 [label="Access variable this@R|/Test|"];
35 [label="Access variable R|<local>/list|"];
36 [label="Assignment: R|/Test.values|"];
37 [label="Exit block"];
}
38 [label="Exit function <init>" style="filled" fillcolor=red];
}
subgraph cluster_11 {
color=blue
39 [label="Enter block"];
40 [label="Access variable this@R|/Test|"];
41 [label="Access variable R|/Test.size|"];
42 [label="Access variable this@R|/Test|"];
43 [label="Access variable R|/Test.values|"];
44 [label="Access variable R|SubstitutionOverride<kotlin/collections/List.size: R|kotlin/Int|>|"];
45 [label="Function call: this@R|/Test|.R|/Test.size|.R|kotlin/Int.plus|(...)" style="filled" fillcolor=yellow];
46 [label="Assignment: R|/Test.size|"];
47 [label="Exit block"];
}
48 [label="Exit function <init>" style="filled" fillcolor=red];
}
subgraph cluster_12 {
color=blue
49 [label="Enter block"];
50 [label="Access variable this@R|/Test|"];
51 [label="Access variable R|/Test.size|"];
52 [label="Access variable this@R|/Test|"];
53 [label="Access variable R|/Test.values|"];
54 [label="Access variable R|SubstitutionOverride<kotlin/collections/List.size: R|kotlin/Int|>|"];
55 [label="Function call: this@R|/Test|.R|/Test.size|.R|kotlin/Int.plus|(...)" style="filled" fillcolor=yellow];
56 [label="Assignment: R|/Test.size|"];
57 [label="Exit block"];
}
58 [label="Exit function <init>" style="filled" fillcolor=red];
}
59 [label="Exit class Test" style="filled" fillcolor=red];
}
0 -> {1 12 24} [color=green];
0 -> {59} [style=dotted];
0 -> {1 12 24 31} [style=dashed];
1 -> {2};
2 -> {3};
3 -> {4 9 10};
3 -> {4} [style=dashed];
4 -> {5};
5 -> {6};
6 -> {7};
7 -> {8};
8 -> {9};
9 -> {3} [color=green style=dashed];
9 -> {10} [color=green];
9 -> {11} [color=red];
10 -> {11};
11 -> {24} [color=green label="return@/Test.Test"];
11 -> {39} [color=red];
12 -> {13};
13 -> {14};
14 -> {15};
15 -> {16 21 22};
15 -> {16} [style=dashed];
16 -> {17};
17 -> {18};
18 -> {19};
19 -> {20};
20 -> {21};
21 -> {15} [color=green style=dashed];
21 -> {22} [color=green];
21 -> {23} [color=red];
22 -> {23};
23 -> {24} [color=green label="return@/Test.Test"];
23 -> {49} [color=red];
24 -> {25};
25 -> {26};
26 -> {27};
27 -> {28};
28 -> {29};
29 -> {30};
30 -> {31} [color=green];
31 -> {32};
32 -> {33};
33 -> {34};
34 -> {35};
35 -> {36};
36 -> {37};
37 -> {38};
38 -> {39 49} [color=green label="return@/Test.Test"];
38 -> {59} [color=green];
39 -> {40};
40 -> {41};
41 -> {42};
42 -> {43};
43 -> {44};
44 -> {45};
45 -> {46};
46 -> {47};
47 -> {48};
48 -> {59} [color=green];
49 -> {50};
50 -> {51};
51 -> {52};
52 -> {53};
53 -> {54};
54 -> {55};
55 -> {56};
56 -> {57};
57 -> {58};
58 -> {59} [color=green];
}
@@ -0,0 +1,25 @@
// FIR_IDENTICAL
// WITH_STDLIB
// DUMP_CFG
// ISSUE: KT-53898
class Test {
private var size: Int
private val values: List<String>
init {
this.size = 0
}
constructor(map: Map<String, String>) : this(map.values.map { it }) {
this.size += this.values.size
}
constructor(set: Set<String>) : this(set.map { it }) {
this.size += this.values.size
}
private constructor(list: List<String>) {
this.values = list
}
}
@@ -6264,6 +6264,12 @@ public class DiagnosticTestGenerated extends AbstractDiagnosticTest {
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/definiteReturnInWhen.kt");
}
@Test
@TestMetadata("delegatedConstructorArguments.kt")
public void testDelegatedConstructorArguments() throws Exception {
runTest("compiler/testData/diagnostics/tests/controlFlowAnalysis/delegatedConstructorArguments.kt");
}
@Test
@TestMetadata("delegatedMemberProperyWriteInInit.kt")
public void testDelegatedMemberProperyWriteInInit() throws Exception {