From 5539ad8ce8d49efc36dc600e646953fda279c2d0 Mon Sep 17 00:00:00 2001 From: Denis Zharkov Date: Tue, 24 Mar 2020 19:33:52 +0300 Subject: [PATCH] FIR: Fix data-flow for jumps between members of local classes First of all, note that currently ControlFlowGraphBuilder instance is fully mutable and shared between all local classes and top-level classes in the same thread Before this change, previous node for member of local class was defined as lastNode And in the case of implicit types, lastNode might be in a middle of another local class member that is being resolved right now. See the test: - "a.length" expression in `bar` should be resolved because smart cast happens before the class declaration - "b.length" expression in `bar` should be unresolved because smart cast happens in a different function The latter case has been working incorrectly, the call was errorenously resolved because "lastNode" were pointed just before "bar()" call in "foo" --- .../resolve/cfg/localClassesWithImplicit.dot | 409 ++++++++++++++++++ .../resolve/cfg/localClassesWithImplicit.kt | 46 ++ .../resolve/cfg/localClassesWithImplicit.txt | 86 ++++ .../fir/FirDiagnosticsTestGenerated.java | 5 + ...DiagnosticsWithLightTreeTestGenerated.java | 5 + .../dfa/cfg/ControlFlowGraphBuilder.kt | 33 +- .../body/resolve/FirImplicitBodyResolve.kt | 8 +- 7 files changed, 590 insertions(+), 2 deletions(-) create mode 100644 compiler/fir/analysis-tests/testData/resolve/cfg/localClassesWithImplicit.dot create mode 100644 compiler/fir/analysis-tests/testData/resolve/cfg/localClassesWithImplicit.kt create mode 100644 compiler/fir/analysis-tests/testData/resolve/cfg/localClassesWithImplicit.txt diff --git a/compiler/fir/analysis-tests/testData/resolve/cfg/localClassesWithImplicit.dot b/compiler/fir/analysis-tests/testData/resolve/cfg/localClassesWithImplicit.dot new file mode 100644 index 00000000000..7d758354194 --- /dev/null +++ b/compiler/fir/analysis-tests/testData/resolve/cfg/localClassesWithImplicit.dot @@ -0,0 +1,409 @@ +digraph localClassesWithImplicit_kt { + graph [nodesep=3] + node [shape=box penwidth=2] + edge [penwidth=2] + + subgraph cluster_0 { + color=red + 0 [label="Enter function myRun" style="filled" fillcolor=red]; + 1 [label="Function call: R|/block|.R|FakeOverride|()"]; + 2 [label="Jump: ^myRun R|/block|.R|FakeOverride|()"]; + 3 [label="Stub" style="filled" fillcolor=gray]; + 4 [label="Exit function myRun" style="filled" fillcolor=red]; + } + + 0 -> {1}; + 1 -> {2}; + 2 -> {4}; + 2 -> {3} [style=dotted]; + 3 -> {4} [style=dotted]; + + subgraph cluster_1 { + color=red + 5 [label="Enter function test" style="filled" fillcolor=red]; + subgraph cluster_2 { + color=blue + 6 [label="Enter when"]; + subgraph cluster_3 { + color=blue + 7 [label="Enter when branch condition "]; + 8 [label="Access variable R|/a|"]; + 9 [label="Type operator: (R|/a| !is R|kotlin/String|)"]; + 10 [label="Exit when branch condition"]; + } + 11 [label="Synthetic else branch"]; + 12 [label="Enter when branch result"]; + subgraph cluster_4 { + color=blue + 13 [label="Enter block"]; + 14 [label="Jump: ^test Unit"]; + 15 [label="Stub" style="filled" fillcolor=gray]; + 16 [label="Exit block" style="filled" fillcolor=gray]; + } + 17 [label="Exit when branch result" style="filled" fillcolor=gray]; + 18 [label="Exit when"]; + } + 19 [label="Exit anonymous object"]; + 20 [label="Variable declaration: lval x: R|anonymous|"]; + 21 [label="Exit function test" style="filled" fillcolor=red]; + } + + 5 -> {6}; + 6 -> {7}; + 7 -> {8}; + 8 -> {9}; + 9 -> {10}; + 10 -> {12 11}; + 11 -> {18}; + 12 -> {13}; + 13 -> {14}; + 14 -> {21}; + 14 -> {15} [style=dotted]; + 15 -> {16} [style=dotted]; + 16 -> {17} [style=dotted]; + 17 -> {18} [style=dotted]; + 18 -> {19}; + 19 -> {20}; + 20 -> {21}; + + subgraph cluster_5 { + color=red + 22 [label="Enter function " style="filled" fillcolor=red]; + 23 [label="Delegated constructor call: super()"]; + 24 [label="Exit function " style="filled" fillcolor=red]; + } + + 22 -> {23}; + 23 -> {24}; + + subgraph cluster_6 { + color=red + 25 [label="Enter function foo" style="filled" fillcolor=red]; + 26 [label="Postponed enter to lambda"]; + subgraph cluster_7 { + color=blue + 27 [label="Enter function anonymousFunction"]; + 28 [label="Access variable R|/a|"]; + 29 [label="Access variable R|kotlin/String.length|"]; + subgraph cluster_8 { + color=blue + 30 [label="Enter when"]; + subgraph cluster_9 { + color=blue + 31 [label="Enter when branch condition "]; + 32 [label="Access variable R|/b|"]; + 33 [label="Type operator: (R|/b| is R|kotlin/String|)"]; + 34 [label="Exit when branch condition"]; + } + subgraph cluster_10 { + color=blue + 35 [label="Enter when branch condition else"]; + 36 [label="Exit when branch condition"]; + } + 37 [label="Enter when branch result"]; + subgraph cluster_11 { + color=blue + 38 [label="Enter block"]; + 39 [label="Const: Int(1)"]; + 40 [label="Exit block"]; + } + 41 [label="Exit when branch result"]; + 42 [label="Enter when branch result"]; + subgraph cluster_12 { + color=blue + 43 [label="Enter block"]; + 44 [label="Access variable R|/b|"]; + 45 [label="Access variable R|kotlin/String.length|"]; + 46 [label="Function call: this@R|/A|.R|/A.bar|()"]; + 47 [label="Exit block"]; + } + 48 [label="Exit when branch result"]; + 49 [label="Exit when"]; + } + 50 [label="Exit function anonymousFunction"]; + } + 51 [label="Postponed exit from lambda"]; + 52 [label="Function call: R|/myRun|(...)"]; + 53 [label="Jump: ^foo R|/myRun|( = myRun@fun (): R|kotlin/Int| { + R|/a|.R|kotlin/String.length| + ^ when () { + (R|/b| is R|kotlin/String|) -> { + R|/b|.R|kotlin/String.length| + this@R|/A|.R|/A.bar|() + } + else -> { + Int(1) + } + } + +} +)"]; + 54 [label="Stub" style="filled" fillcolor=gray]; + 55 [label="Exit function foo" style="filled" fillcolor=red]; + } + + 25 -> {26}; + 26 -> {27}; + 26 -> {51} [color=red]; + 27 -> {50 28}; + 28 -> {29}; + 29 -> {30}; + 30 -> {31}; + 31 -> {32}; + 32 -> {33}; + 33 -> {34}; + 34 -> {42 35}; + 35 -> {36}; + 36 -> {37}; + 37 -> {38}; + 38 -> {39}; + 39 -> {40}; + 40 -> {41}; + 41 -> {49}; + 42 -> {43}; + 43 -> {44}; + 44 -> {45}; + 45 -> {46}; + 46 -> {47}; + 47 -> {48}; + 48 -> {49}; + 49 -> {50}; + 50 -> {27}; + 50 -> {51} [color=green]; + 51 -> {52}; + 52 -> {53}; + 53 -> {55}; + 53 -> {54} [style=dotted]; + 54 -> {55} [style=dotted]; + + subgraph cluster_13 { + color=red + 56 [label="Enter function bar" style="filled" fillcolor=red]; + 57 [label="Postponed enter to lambda"]; + subgraph cluster_14 { + color=blue + 58 [label="Enter function anonymousFunction"]; + 59 [label="Access variable R|/b|"]; + 60 [label="Access variable #"]; + 61 [label="Access variable R|/a|"]; + 62 [label="Access variable R|kotlin/String.length|"]; + 63 [label="Function call: this@R|/A|.R|/A.baz|()"]; + 64 [label="Exit function anonymousFunction"]; + } + 65 [label="Postponed exit from lambda"]; + 66 [label="Function call: R|/myRun|(...)"]; + 67 [label="Jump: ^bar R|/myRun|( = myRun@fun (): R|kotlin/Int| { + R|/b|.# + R|/a|.R|kotlin/String.length| + ^ this@R|/A|.R|/A.baz|() +} +)"]; + 68 [label="Stub" style="filled" fillcolor=gray]; + 69 [label="Exit function bar" style="filled" fillcolor=red]; + } + + 56 -> {57}; + 57 -> {58}; + 57 -> {65} [color=red]; + 58 -> {64 59}; + 59 -> {60}; + 60 -> {61}; + 61 -> {62}; + 62 -> {63}; + 63 -> {64}; + 64 -> {58}; + 64 -> {65} [color=green]; + 65 -> {66}; + 66 -> {67}; + 67 -> {69}; + 67 -> {68} [style=dotted]; + 68 -> {69} [style=dotted]; + + subgraph cluster_15 { + color=red + 70 [label="Enter function baz" style="filled" fillcolor=red]; + 71 [label="Const: Int(1)"]; + 72 [label="Jump: ^baz Int(1)"]; + 73 [label="Stub" style="filled" fillcolor=gray]; + 74 [label="Exit function baz" style="filled" fillcolor=red]; + } + + 70 -> {71}; + 71 -> {72}; + 72 -> {74}; + 72 -> {73} [style=dotted]; + 73 -> {74} [style=dotted]; + + subgraph cluster_16 { + color=red + 75 [label="Enter function " style="filled" fillcolor=red]; + 76 [label="Delegated constructor call: super()"]; + 77 [label="Exit function " style="filled" fillcolor=red]; + } + + 75 -> {76}; + 76 -> {77}; + + subgraph cluster_17 { + color=red + 78 [label="Enter function foo" style="filled" fillcolor=red]; + 79 [label="Postponed enter to lambda"]; + subgraph cluster_18 { + color=blue + 80 [label="Enter function anonymousFunction"]; + 81 [label="Access variable R|/a|"]; + 82 [label="Access variable R|kotlin/String.length|"]; + subgraph cluster_19 { + color=blue + 83 [label="Enter when"]; + subgraph cluster_20 { + color=blue + 84 [label="Enter when branch condition "]; + 85 [label="Access variable R|/b|"]; + 86 [label="Type operator: (R|/b| is R|kotlin/String|)"]; + 87 [label="Exit when branch condition"]; + } + subgraph cluster_21 { + color=blue + 88 [label="Enter when branch condition else"]; + 89 [label="Exit when branch condition"]; + } + 90 [label="Enter when branch result"]; + subgraph cluster_22 { + color=blue + 91 [label="Enter block"]; + 92 [label="Const: Int(1)"]; + 93 [label="Exit block"]; + } + 94 [label="Exit when branch result"]; + 95 [label="Enter when branch result"]; + subgraph cluster_23 { + color=blue + 96 [label="Enter block"]; + 97 [label="Access variable R|/b|"]; + 98 [label="Access variable R|kotlin/String.length|"]; + 99 [label="Exit anonymous object"]; + 100 [label="Function call: this@R|/anonymous|.R|/anonymous.bar|()"]; + 101 [label="Exit block"]; + } + 102 [label="Exit when branch result"]; + 103 [label="Exit when"]; + } + 104 [label="Exit function anonymousFunction"]; + } + 105 [label="Postponed exit from lambda"]; + 106 [label="Function call: R|/myRun|(...)"]; + 107 [label="Jump: ^foo R|/myRun|( = myRun@fun (): R|kotlin/Int| { + R|/a|.R|kotlin/String.length| + ^ when () { + (R|/b| is R|kotlin/String|) -> { + R|/b|.R|kotlin/String.length| + this@R|/anonymous|.R|/anonymous.bar|() + } + else -> { + Int(1) + } + } + +} +)"]; + 108 [label="Stub" style="filled" fillcolor=gray]; + 109 [label="Exit function foo" style="filled" fillcolor=red]; + } + + 78 -> {79}; + 79 -> {80}; + 79 -> {105} [color=red]; + 80 -> {104 81}; + 81 -> {82}; + 82 -> {83}; + 83 -> {84}; + 84 -> {85}; + 85 -> {86}; + 86 -> {87}; + 87 -> {95 88}; + 88 -> {89}; + 89 -> {90}; + 90 -> {91}; + 91 -> {92}; + 92 -> {93}; + 93 -> {94}; + 94 -> {103}; + 95 -> {96}; + 96 -> {97}; + 97 -> {98}; + 98 -> {99}; + 99 -> {100}; + 100 -> {101}; + 101 -> {102}; + 102 -> {103}; + 103 -> {104}; + 104 -> {80}; + 104 -> {105} [color=green]; + 105 -> {106}; + 106 -> {107}; + 107 -> {109}; + 107 -> {108} [style=dotted]; + 108 -> {109} [style=dotted]; + + subgraph cluster_24 { + color=red + 110 [label="Enter function bar" style="filled" fillcolor=red]; + 111 [label="Postponed enter to lambda"]; + subgraph cluster_25 { + color=blue + 112 [label="Enter function anonymousFunction"]; + 113 [label="Access variable R|/a|"]; + 114 [label="Access variable R|kotlin/String.length|"]; + 115 [label="Access variable R|/b|"]; + 116 [label="Access variable #"]; + 117 [label="Exit anonymous object"]; + 118 [label="Function call: this@R|/anonymous|.R|/anonymous.baz|()"]; + 119 [label="Exit function anonymousFunction"]; + } + 120 [label="Postponed exit from lambda"]; + 121 [label="Function call: R|/myRun|(...)"]; + 122 [label="Jump: ^bar R|/myRun|( = myRun@fun (): R|kotlin/Int| { + R|/a|.R|kotlin/String.length| + R|/b|.# + ^ this@R|/anonymous|.R|/anonymous.baz|() +} +)"]; + 123 [label="Stub" style="filled" fillcolor=gray]; + 124 [label="Exit function bar" style="filled" fillcolor=red]; + } + + 110 -> {111}; + 111 -> {112}; + 111 -> {120} [color=red]; + 112 -> {119 113}; + 113 -> {114}; + 114 -> {115}; + 115 -> {116}; + 116 -> {117}; + 117 -> {118}; + 118 -> {119}; + 119 -> {112}; + 119 -> {120} [color=green]; + 120 -> {121}; + 121 -> {122}; + 122 -> {124}; + 122 -> {123} [style=dotted]; + 123 -> {124} [style=dotted]; + + subgraph cluster_26 { + color=red + 125 [label="Enter function baz" style="filled" fillcolor=red]; + 126 [label="Const: Int(1)"]; + 127 [label="Jump: ^baz Int(1)"]; + 128 [label="Stub" style="filled" fillcolor=gray]; + 129 [label="Exit function baz" style="filled" fillcolor=red]; + } + + 125 -> {126}; + 126 -> {127}; + 127 -> {129}; + 127 -> {128} [style=dotted]; + 128 -> {129} [style=dotted]; + +} diff --git a/compiler/fir/analysis-tests/testData/resolve/cfg/localClassesWithImplicit.kt b/compiler/fir/analysis-tests/testData/resolve/cfg/localClassesWithImplicit.kt new file mode 100644 index 00000000000..14f481b3112 --- /dev/null +++ b/compiler/fir/analysis-tests/testData/resolve/cfg/localClassesWithImplicit.kt @@ -0,0 +1,46 @@ +// !DUMP_CFG +inline fun myRun(block: () -> T) = block() + +fun test(a: Any, b: Any) { + if (a !is String) return + // (1) + class A { + fun foo() = myRun { + a.length + if (b is String) { + b.length + bar() + } else { + 1 + } + } + + fun bar() = myRun { + b.length + a.length + baz() + } + + fun baz() = 1 + } + + val x = object { + fun foo() = myRun { + a.length + if (b is String) { + b.length + bar() + } else { + 1 + } + } + + fun bar() = myRun { + a.length + b.length + baz() + } + + fun baz() = 1 + } +} diff --git a/compiler/fir/analysis-tests/testData/resolve/cfg/localClassesWithImplicit.txt b/compiler/fir/analysis-tests/testData/resolve/cfg/localClassesWithImplicit.txt new file mode 100644 index 00000000000..0984d0adba8 --- /dev/null +++ b/compiler/fir/analysis-tests/testData/resolve/cfg/localClassesWithImplicit.txt @@ -0,0 +1,86 @@ +FILE: localClassesWithImplicit.kt + public final inline fun myRun(block: R|() -> T|): R|T| { + ^myRun R|/block|.R|FakeOverride|() + } + public final fun test(a: R|kotlin/Any|, b: R|kotlin/Any|): R|kotlin/Unit| { + when () { + (R|/a| !is R|kotlin/String|) -> { + ^test Unit + } + } + + local final class A : R|kotlin/Any| { + public[local] constructor(): R|A| { + super() + } + + public[local] final fun foo(): R|kotlin/Int| { + ^foo R|/myRun|( = myRun@fun (): R|kotlin/Int| { + R|/a|.R|kotlin/String.length| + ^ when () { + (R|/b| is R|kotlin/String|) -> { + R|/b|.R|kotlin/String.length| + this@R|/A|.R|/A.bar|() + } + else -> { + Int(1) + } + } + + } + ) + } + + public[local] final fun bar(): R|kotlin/Int| { + ^bar R|/myRun|( = myRun@fun (): R|kotlin/Int| { + R|/b|.# + R|/a|.R|kotlin/String.length| + ^ this@R|/A|.R|/A.baz|() + } + ) + } + + public[local] final fun baz(): R|kotlin/Int| { + ^baz Int(1) + } + + } + + lval x: R|anonymous| = object : R|kotlin/Any| { + private constructor(): R|anonymous| { + super() + } + + public final fun foo(): R|kotlin/Int| { + ^foo R|/myRun|( = myRun@fun (): R|kotlin/Int| { + R|/a|.R|kotlin/String.length| + ^ when () { + (R|/b| is R|kotlin/String|) -> { + R|/b|.R|kotlin/String.length| + this@R|/anonymous|.R|/anonymous.bar|() + } + else -> { + Int(1) + } + } + + } + ) + } + + public final fun bar(): R|kotlin/Int| { + ^bar R|/myRun|( = myRun@fun (): R|kotlin/Int| { + R|/a|.R|kotlin/String.length| + R|/b|.# + ^ this@R|/anonymous|.R|/anonymous.baz|() + } + ) + } + + public final fun baz(): R|kotlin/Int| { + ^baz Int(1) + } + + } + + } diff --git a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java index 6217844f5a5..b29f2841409 100644 --- a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java +++ b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsTestGenerated.java @@ -639,6 +639,11 @@ public class FirDiagnosticsTestGenerated extends AbstractFirDiagnosticsTest { runTest("compiler/fir/analysis-tests/testData/resolve/cfg/lambdas.kt"); } + @TestMetadata("localClassesWithImplicit.kt") + public void testLocalClassesWithImplicit() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolve/cfg/localClassesWithImplicit.kt"); + } + @TestMetadata("loops.kt") public void testLoops() throws Exception { runTest("compiler/fir/analysis-tests/testData/resolve/cfg/loops.kt"); diff --git a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsWithLightTreeTestGenerated.java b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsWithLightTreeTestGenerated.java index ae55327956c..943218b55a5 100644 --- a/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsWithLightTreeTestGenerated.java +++ b/compiler/fir/analysis-tests/tests/org/jetbrains/kotlin/fir/FirDiagnosticsWithLightTreeTestGenerated.java @@ -639,6 +639,11 @@ public class FirDiagnosticsWithLightTreeTestGenerated extends AbstractFirDiagnos runTest("compiler/fir/analysis-tests/testData/resolve/cfg/lambdas.kt"); } + @TestMetadata("localClassesWithImplicit.kt") + public void testLocalClassesWithImplicit() throws Exception { + runTest("compiler/fir/analysis-tests/testData/resolve/cfg/localClassesWithImplicit.kt"); + } + @TestMetadata("loops.kt") public void testLoops() throws Exception { runTest("compiler/fir/analysis-tests/testData/resolve/cfg/loops.kt"); 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 2925a55b031..e7927cbb1c8 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 @@ -54,6 +54,8 @@ class ControlFlowGraphBuilder { private val exitsFromCompletedPostponedAnonymousFunctions: MutableList = mutableListOf() + private val enterToLocalClassesMembers: MutableMap, CFGNode<*>?> = mutableMapOf() + var levelCounter: Int = 0 private set @@ -66,6 +68,33 @@ class ControlFlowGraphBuilder { // ----------------------------------- Named function ----------------------------------- + fun prepareForLocalClassMembers(members: Collection>) { + members.forEachFunction { + enterToLocalClassesMembers[it.symbol] = lastNodes.topOrNull() + } + } + + fun cleanAfterForLocalClassMembers(members: Collection>) { + members.forEachFunction { + enterToLocalClassesMembers.remove(it.symbol) + } + } + + private inline fun Collection>.forEachFunction(block: (FirFunction<*>) -> Unit) { + for (member in this) { + for (function in member.unwrap()) { + block(function) + } + } + } + + private fun FirCallableMemberDeclaration<*>.unwrap(): List> = + when (this) { + is FirFunction<*> -> listOf(this) + is FirProperty -> listOfNotNull(this.getter, this.setter) + else -> emptyList() + } + /* * Second argument of pair is not null only if function is local and it is a * previous node before function declaration @@ -82,7 +111,9 @@ class ControlFlowGraphBuilder { val isInplace = invocationKind.isInplace() val previousNode = if (!isInplace && graphs.topOrNull()?.let { it.kind != ControlFlowGraph.Kind.TopLevel } == true) { - entersToPostponedAnonymousFunctions[function.symbol] ?: lastNodes.top() + entersToPostponedAnonymousFunctions[function.symbol] + ?: enterToLocalClassesMembers[function.symbol] + ?: lastNodes.top() } else { null } diff --git a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirImplicitBodyResolve.kt b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirImplicitBodyResolve.kt index 105e8f9154a..d1a0348b7e0 100644 --- a/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirImplicitBodyResolve.kt +++ b/compiler/fir/resolve/src/org/jetbrains/kotlin/fir/resolve/transformers/body/resolve/FirImplicitBodyResolve.kt @@ -82,7 +82,13 @@ fun > F.runBodiesResolutionForLocalClass( returnTypeCalculator ) - return this.transform(transformer, resolutionMode).single + val graphBuilder = components.context.dataFlowAnalyzerContext.graphBuilder + val members = designationMap.keys + graphBuilder.prepareForLocalClassMembers(members) + + return this.transform(transformer, resolutionMode).single.also { + graphBuilder.cleanAfterForLocalClassMembers(members) + } } fun createReturnTypeCalculatorForIDE(session: FirSession, scopeSession: ScopeSession): ReturnTypeCalculator =