From d24528f6bbb9137a0087e9862765597927af8c5e Mon Sep 17 00:00:00 2001 From: Mikhail Glukhikh Date: Tue, 19 Apr 2016 18:01:08 +0300 Subject: [PATCH] Control flow analysis: nested finally blocks now appear in correct order in CFG #KT-4764 Fixed --- .../ControlFlowInstructionsGenerator.kt | 2 +- .../cfg-variables/bugs/kt4764.instructions | 131 ++++++++++++++++++ .../testData/cfg-variables/bugs/kt4764.kt | 22 +++ .../testData/cfg-variables/bugs/kt4764.values | 57 ++++++++ .../controlFlowAnalysis/nestedTryFinally.kt | 24 ++++ .../controlFlowAnalysis/nestedTryFinally.txt | 4 + .../kotlin/cfg/DataFlowTestGenerated.java | 6 + .../kotlin/cfg/PseudoValueTestGenerated.java | 6 + .../checkers/DiagnosticsTestGenerated.java | 6 + 9 files changed, 257 insertions(+), 1 deletion(-) create mode 100644 compiler/testData/cfg-variables/bugs/kt4764.instructions create mode 100644 compiler/testData/cfg-variables/bugs/kt4764.kt create mode 100644 compiler/testData/cfg-variables/bugs/kt4764.values create mode 100644 compiler/testData/diagnostics/tests/controlFlowAnalysis/nestedTryFinally.kt create mode 100644 compiler/testData/diagnostics/tests/controlFlowAnalysis/nestedTryFinally.txt diff --git a/compiler/frontend/src/org/jetbrains/kotlin/cfg/pseudocode/ControlFlowInstructionsGenerator.kt b/compiler/frontend/src/org/jetbrains/kotlin/cfg/pseudocode/ControlFlowInstructionsGenerator.kt index 14ba99f8563..82bd26f235e 100644 --- a/compiler/frontend/src/org/jetbrains/kotlin/cfg/pseudocode/ControlFlowInstructionsGenerator.kt +++ b/compiler/frontend/src/org/jetbrains/kotlin/cfg/pseudocode/ControlFlowInstructionsGenerator.kt @@ -204,7 +204,7 @@ class ControlFlowInstructionsGenerator : ControlFlowBuilderAdapter() { val blockInfo = allBlocks[i] if (blockInfo is BreakableBlockInfo) { if (blockInfo.referablePoints.contains(jumpTarget) || jumpTarget === error) { - for (j in finallyBlocks.indices.reversed()) { + for (j in finallyBlocks.indices) { finallyBlocks[j].generateFinallyBlock() } break diff --git a/compiler/testData/cfg-variables/bugs/kt4764.instructions b/compiler/testData/cfg-variables/bugs/kt4764.instructions new file mode 100644 index 00000000000..74ab5e7d8ea --- /dev/null +++ b/compiler/testData/cfg-variables/bugs/kt4764.instructions @@ -0,0 +1,131 @@ +== sample == +fun sample(): String? { + try { + if (false) { + return "fail" + } else { + if (false) { + if (false) { + var foo: String? = null + try { + foo = "test" + } catch (e: Exception) { + return "fail" + } finally { + println(foo) // Variable 'foo' must be initialized + } + } + return "fail" + } + } + } finally {} + return null +} +--------------------- +L0: + 1 INIT: in: {} out: {} + 2 mark({ try { if (false) { return "fail" } else { if (false) { if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" } } } finally {} return null }) + mark(try { if (false) { return "fail" } else { if (false) { if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" } } } finally {}) + jmp?(L2) + 3 mark({ if (false) { return "fail" } else { if (false) { if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" } } }) + mark(if (false) { return "fail" } else { if (false) { if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" } }) + r(false) -> + jf(L3|) + 4 mark({ return "fail" }) + mark("fail") + r("fail") -> +L4 [start finally]: + 5 mark({}) + read (Unit) +L5 [finish finally]: + 4 ret(*|) L1 USE: in: {} out: {} +- 3 jmp(L6) +L3 [else branch]: + 4 mark({ if (false) { if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" } }) INIT: in: {} out: {} + mark(if (false) { if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" }) + r(false) -> + jf(L7|) + 5 mark({ if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" }) + mark(if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } }) + r(false) -> + jf(L8|) USE: in: {} out: {} + 6 mark({ var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } }) + v(var foo: String? = null) INIT: in: {} out: {foo=D} + r(null) -> INIT: in: {foo=D} out: {foo=D} + w(foo|) INIT: in: {foo=D} out: {foo=ID} + mark(try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized }) INIT: in: {foo=ID} out: {foo=ID} + jmp?(L9) + jmp?(L10) USE: in: {foo=READ} out: {foo=READ} + 7 mark({ foo = "test" }) + mark("test") + r("test") -> USE: in: {foo=WRITTEN_AFTER_READ} out: {foo=WRITTEN_AFTER_READ} + w(foo|) USE: in: {foo=READ} out: {foo=WRITTEN_AFTER_READ} + 6 jmp(L11) +L9 [onException]: + 7 v(e: Exception) INIT: in: {foo=ID} out: {e=D, foo=ID} + magic[FAKE_INITIALIZER](e: Exception) -> INIT: in: {e=D, foo=ID} out: {e=D, foo=ID} + w(e|) INIT: in: {e=D, foo=ID} out: {e=ID, foo=ID} + 8 mark({ return "fail" }) INIT: in: {e=ID, foo=ID} out: {e=ID, foo=ID} + mark("fail") + r("fail") -> +L12 [start finally]: + 9 mark({ println(foo) // Variable 'foo' must be initialized }) USE: in: {foo=READ} out: {foo=READ} + r(foo) -> USE: in: {} out: {foo=READ} + mark(println(foo)) + magic[UNRESOLVED_CALL](println(foo)|, !) -> +L13 [finish finally]: + 5 mark({}) INIT: in: {} out: {} + read (Unit) + 8 ret(*|) L1 USE: in: {} out: {} +- 7 jmp(L11) +L11 [afterCatches]: + 6 jmp(L14) INIT: in: {foo=ID} out: {foo=ID} +L10 [onExceptionToFinallyBlock]: + 9 mark({ println(foo) // Variable 'foo' must be initialized }) USE: in: {foo=READ} out: {foo=READ} + r(foo) -> USE: in: {} out: {foo=READ} + mark(println(foo)) + magic[UNRESOLVED_CALL](println(foo)|, !) -> + 5 mark({}) INIT: in: {} out: {} + read (Unit) + 6 jmp(error) USE: in: {} out: {} +L14 [skipFinallyToErrorBlock]: + 9 mark({ println(foo) // Variable 'foo' must be initialized }) INIT: in: {foo=ID} out: {foo=ID} USE: in: {foo=READ} out: {foo=READ} + r(foo) -> USE: in: {} out: {foo=READ} + mark(println(foo)) + magic[UNRESOLVED_CALL](println(foo)|, !) -> + 6 merge(try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized }|!, !) -> + 5 jmp(L15) INIT: in: {} out: {} +L8 [else branch]: + read (Unit) +L15 ['if' expression result]: + merge(if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } }|) -> + mark("fail") + r("fail") -> + mark({}) + read (Unit) + ret(*|) L1 USE: in: {} out: {} +- 4 jmp(L16) +L7 [else branch]: + read (Unit) INIT: in: {} out: {} +L16 ['if' expression result]: + merge(if (false) { if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" }|!) -> +L6 ['if' expression result]: + 3 merge(if (false) { return "fail" } else { if (false) { if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" } }|!, ) -> + 2 jmp(L17) +L2 [onExceptionToFinallyBlock]: + 5 mark({}) + read (Unit) + 2 jmp(error) +L17 [skipFinallyToErrorBlock]: + 5 mark({}) + read (Unit) + 2 merge(try { if (false) { return "fail" } else { if (false) { if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" } } } finally {}|) -> + r(null) -> + ret(*|) L1 +L1: + 1 +error: + +sink: + USE: in: {} out: {} +===================== diff --git a/compiler/testData/cfg-variables/bugs/kt4764.kt b/compiler/testData/cfg-variables/bugs/kt4764.kt new file mode 100644 index 00000000000..40acf7dc7db --- /dev/null +++ b/compiler/testData/cfg-variables/bugs/kt4764.kt @@ -0,0 +1,22 @@ +fun sample(): String? { + try { + if (false) { + return "fail" + } else { + if (false) { + if (false) { + var foo: String? = null + try { + foo = "test" + } catch (e: Exception) { + return "fail" + } finally { + println(foo) // Variable 'foo' must be initialized + } + } + return "fail" + } + } + } finally {} + return null +} diff --git a/compiler/testData/cfg-variables/bugs/kt4764.values b/compiler/testData/cfg-variables/bugs/kt4764.values new file mode 100644 index 00000000000..02b74a6e068 --- /dev/null +++ b/compiler/testData/cfg-variables/bugs/kt4764.values @@ -0,0 +1,57 @@ +== sample == +fun sample(): String? { + try { + if (false) { + return "fail" + } else { + if (false) { + if (false) { + var foo: String? = null + try { + foo = "test" + } catch (e: Exception) { + return "fail" + } finally { + println(foo) // Variable 'foo' must be initialized + } + } + return "fail" + } + } + } finally {} + return null +} +--------------------- + : {<: Exception} NEW: magic[FAKE_INITIALIZER](e: Exception) -> +false : Boolean NEW: r(false) -> +"fail" : {<: String?} NEW: r("fail") -> +return "fail" !: * +{ return "fail" } !: * COPY +false : Boolean NEW: r(false) -> +false : Boolean NEW: r(false) -> +null : {<: String?} NEW: r(null) -> +"test" : {<: String?} NEW: r("test") -> +foo = "test" !: * +{ foo = "test" } !: * COPY +"fail" : {<: String?} NEW: r("fail") -> +return "fail" !: * +{ return "fail" } !: * COPY +println !: * +foo : * NEW: r(foo) -> +println(foo) : * NEW: magic[UNRESOLVED_CALL](println(foo)|, !) -> +{ println(foo) // Variable 'foo' must be initialized } : * COPY +try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } : * NEW: merge(try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized }|!, !) -> +{ var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } : * COPY +if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } : * NEW: merge(if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } }|) -> +"fail" : {<: String?} NEW: r("fail") -> +return "fail" !: * +{ if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" } !: * COPY +if (false) { if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" } : * NEW: merge(if (false) { if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" }|!) -> +{ if (false) { if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" } } : * COPY +if (false) { return "fail" } else { if (false) { if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" } } : * NEW: merge(if (false) { return "fail" } else { if (false) { if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" } }|!, ) -> +{ if (false) { return "fail" } else { if (false) { if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" } } } : * COPY +try { if (false) { return "fail" } else { if (false) { if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" } } } finally {} : * NEW: merge(try { if (false) { return "fail" } else { if (false) { if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" } } } finally {}|) -> +null : {<: String?} NEW: r(null) -> +return null !: * +{ try { if (false) { return "fail" } else { if (false) { if (false) { var foo: String? = null try { foo = "test" } catch (e: Exception) { return "fail" } finally { println(foo) // Variable 'foo' must be initialized } } return "fail" } } } finally {} return null } !: * COPY +===================== diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/nestedTryFinally.kt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/nestedTryFinally.kt new file mode 100644 index 00000000000..eb493096c80 --- /dev/null +++ b/compiler/testData/diagnostics/tests/controlFlowAnalysis/nestedTryFinally.kt @@ -0,0 +1,24 @@ +fun use(arg: String?) = arg + +fun sample(): String? { + try { + if (false) { + return "fail" + } else { + if (false) { + if (false) { + var foo: String? = null + try { + foo = "test" + } catch (e: Exception) { + return "fail" + } finally { + use(foo) // 'foo' is initialized here + } + } + return "fail" + } + } + } finally {} + return null +} diff --git a/compiler/testData/diagnostics/tests/controlFlowAnalysis/nestedTryFinally.txt b/compiler/testData/diagnostics/tests/controlFlowAnalysis/nestedTryFinally.txt new file mode 100644 index 00000000000..12459476d27 --- /dev/null +++ b/compiler/testData/diagnostics/tests/controlFlowAnalysis/nestedTryFinally.txt @@ -0,0 +1,4 @@ +package + +public fun sample(): kotlin.String? +public fun use(/*0*/ arg: kotlin.String?): kotlin.String? diff --git a/compiler/tests/org/jetbrains/kotlin/cfg/DataFlowTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/cfg/DataFlowTestGenerated.java index 6aa25267655..647d400d834 100644 --- a/compiler/tests/org/jetbrains/kotlin/cfg/DataFlowTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/cfg/DataFlowTestGenerated.java @@ -106,6 +106,12 @@ public class DataFlowTestGenerated extends AbstractDataFlowTest { doTest(fileName); } + @TestMetadata("kt4764.kt") + public void testKt4764() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/cfg-variables/bugs/kt4764.kt"); + doTest(fileName); + } + @TestMetadata("referenceToPropertyInitializer.kt") public void testReferenceToPropertyInitializer() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/cfg-variables/bugs/referenceToPropertyInitializer.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/cfg/PseudoValueTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/cfg/PseudoValueTestGenerated.java index 6ed783a7fd8..0133af47937 100644 --- a/compiler/tests/org/jetbrains/kotlin/cfg/PseudoValueTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/cfg/PseudoValueTestGenerated.java @@ -864,6 +864,12 @@ public class PseudoValueTestGenerated extends AbstractPseudoValueTest { doTest(fileName); } + @TestMetadata("kt4764.kt") + public void testKt4764() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/cfg-variables/bugs/kt4764.kt"); + doTest(fileName); + } + @TestMetadata("referenceToPropertyInitializer.kt") public void testReferenceToPropertyInitializer() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/cfg-variables/bugs/referenceToPropertyInitializer.kt"); diff --git a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java index 15a93dc638e..9c222887e78 100644 --- a/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java +++ b/compiler/tests/org/jetbrains/kotlin/checkers/DiagnosticsTestGenerated.java @@ -3123,6 +3123,12 @@ public class DiagnosticsTestGenerated extends AbstractDiagnosticsTest { doTest(fileName); } + @TestMetadata("nestedTryFinally.kt") + public void testNestedTryFinally() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/controlFlowAnalysis/nestedTryFinally.kt"); + doTest(fileName); + } + @TestMetadata("nonLocalReturnUnreachable.kt") public void testNonLocalReturnUnreachable() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("compiler/testData/diagnostics/tests/controlFlowAnalysis/nonLocalReturnUnreachable.kt");