From 1cf4b1d876a1888cfad2556ea98ffe7d7d43b5fd Mon Sep 17 00:00:00 2001 From: Andrey Vlasovskikh Date: Fri, 8 Jul 2011 22:20:08 +0400 Subject: [PATCH] Separated success and failure control flow for finally clause if no except clauses found (PY-4102) --- .../controlflow/PyControlFlowBuilder.java | 145 ++++++++++++------ .../PyUnboundLocalVariableInspection.java | 8 +- .../codeInsight/controlflow/doubletry.py | 10 +- .../codeInsight/controlflow/doubletry.txt | 19 ++- .../codeInsight/controlflow/trybreak.txt | 2 +- .../codeInsight/controlflow/tryfinally.txt | 19 ++- .../codeInsight/controlflow/trytry.py | 27 ++++ .../codeInsight/controlflow/trytry.txt | 64 ++++++++ .../python/PyControlFlowBuilderTest.java | 4 + 9 files changed, 233 insertions(+), 65 deletions(-) create mode 100644 python/testData/codeInsight/controlflow/trytry.py create mode 100644 python/testData/codeInsight/controlflow/trytry.txt diff --git a/python/src/com/jetbrains/python/codeInsight/controlflow/PyControlFlowBuilder.java b/python/src/com/jetbrains/python/codeInsight/controlflow/PyControlFlowBuilder.java index 9c2f1241ef96..f8fce0068ecb 100644 --- a/python/src/com/jetbrains/python/codeInsight/controlflow/PyControlFlowBuilder.java +++ b/python/src/com/jetbrains/python/codeInsight/controlflow/PyControlFlowBuilder.java @@ -27,6 +27,7 @@ public class PyControlFlowBuilder extends PyRecursiveElementVisitor { public static final TokenSet CALL_OR_REF_EXPR = TokenSet.create(PyElementTypes.CALL_EXPRESSION, PyElementTypes.REFERENCE_EXPRESSION); public static final String SELF_ASSERT_RAISES = "self.assertRaises"; private final ControlFlowBuilder myBuilder = new ControlFlowBuilder(); + private List> myPendindBackup = null; //////////////////////////////////////////////////////////////////////////////////////////////////////////////////////// //// Control flow builder staff @@ -431,14 +432,6 @@ public class PyControlFlowBuilder extends PyRecursiveElementVisitor { abruptFlow(node); } - private Instruction addBlockConnectedViaPending(final PsiElement node, final PsiElement scope) { - myBuilder.flowAbrupted(); - final Instruction result = myBuilder.startNode(node); - node.accept(this); - myBuilder.addPendingEdge(scope, myBuilder.prevInstruction); - return result; - } - @Override public void visitPyTryExceptStatement(final PyTryExceptStatement node) { myBuilder.startNode(node); @@ -456,75 +449,133 @@ public class PyControlFlowBuilder extends PyRecursiveElementVisitor { } myBuilder.addPendingEdge(node, myBuilder.prevInstruction); + // Process except parts final ArrayList exceptInstructions = new ArrayList(); - // Store pending and clear it - final List> pending = myBuilder.pending; - myBuilder.pending = new ArrayList>(); + storeAndClearPending(); for (PyExceptPart exceptPart : node.getExceptParts()) { - final Instruction exceptInstrcution = addBlockConnectedViaPending(exceptPart, node); + myBuilder.flowAbrupted(); + final Instruction exceptInstrcution = myBuilder.startNode(exceptPart); + exceptPart.accept(this); + myBuilder.addPendingEdge(node, myBuilder.prevInstruction); exceptInstructions.add(exceptInstrcution); } - // Restore pending - for (Pair pair : pending) { - myBuilder.addPendingEdge(pair.first, pair.second); - } - // Finally part handling + restorePending(); + + final List normalExits = new ArrayList(); final PyFinallyPart finallyPart = node.getFinallyPart(); - Instruction finallyInstruction = null; - Instruction lastFinallyInstruction = null; + final Instruction finallyFailInstruction; + + // Store pending normal exit instructions from try-except-else parts if (finallyPart != null) { - finallyInstruction = addBlockConnectedViaPending(finallyPart, node); - lastFinallyInstruction = myBuilder.prevInstruction; + myBuilder.processPending(new ControlFlowBuilder.PendingProcessor() { + public void process(final PsiElement pendingScope, final Instruction instruction) { + final PsiElement pendingElement = instruction.getElement(); + if (pendingElement != null) { + final boolean isPending = PsiTreeUtil.isAncestor(node, pendingElement, false) && + !PsiTreeUtil.isAncestor(finallyPart, pendingElement, false); + if (isPending && pendingScope != null) { + normalExits.add(instruction); + } + else { + myBuilder.addPendingEdge(pendingScope, instruction); + } + } + } + }); } + // Finally-fail part handling + if (finallyPart != null) { + myBuilder.flowAbrupted(); + finallyFailInstruction = myBuilder.startNode(finallyPart); + finallyPart.accept(this); + myBuilder.addPendingEdge(null, myBuilder.prevInstruction); + } else { + finallyFailInstruction = null; + } + + // Create exception edges for (Instruction instruction : myBuilder.instructions) { final PsiElement e = instruction.getElement(); if (e == null || !canRaiseExceptions(instruction)) { continue; } // All instructions inside the try part have edges to except and finally parts - if (PsiTreeUtil.isAncestor(tryPart, e, true)) { + if (PsiTreeUtil.getParentOfType(e, PyTryPart.class, true) == tryPart) { for (Instruction inst : exceptInstructions) { myBuilder.addEdge(instruction, inst); } if (finallyPart != null) { - myBuilder.addEdge(instruction, finallyInstruction); + myBuilder.addEdge(instruction, finallyFailInstruction); } } if (finallyPart != null) { // All instructions inside except parts have edges to the finally part for (PyExceptPart exceptPart : node.getExceptParts()) { if (PsiTreeUtil.isAncestor(exceptPart, e, true)) { - myBuilder.addEdge(instruction, finallyInstruction); + myBuilder.addEdge(instruction, finallyFailInstruction); } } // All instructions inside the else part have edges to the finally part if (PsiTreeUtil.isAncestor(elsePart, e, true)) { - myBuilder.addEdge(instruction, finallyInstruction); + myBuilder.addEdge(instruction, finallyFailInstruction); } } } - final Ref finallyRef = new Ref(finallyInstruction); - final Ref lastFinallyRef = new Ref(lastFinallyInstruction); + if (finallyPart != null) { + myBuilder.processPending(new ControlFlowBuilder.PendingProcessor() { + @Override + public void process(PsiElement pendingScope, Instruction instruction) { + final PsiElement e = instruction.getElement(); + if (e != null) { + // Change the scope of pending edges from finally-fail part to point to the last instruction + if (PsiTreeUtil.isAncestor(finallyPart, e, false)) { + myBuilder.addPendingEdge(null, instruction); + } + // Connect pending fail edges to the finally-fail part + else if (pendingScope == null && PsiTreeUtil.isAncestor(node, e, false)) { + myBuilder.addEdge(instruction, finallyFailInstruction); + } + else { + myBuilder.addPendingEdge(pendingScope, instruction); + } + } + } + }); - myBuilder.processPending(new ControlFlowBuilder.PendingProcessor() { - public void process(final PsiElement pendingScope, final Instruction instruction) { - final PsiElement pendingElement = instruction.getElement(); - if (pendingElement == null) { - return; - } - // Handle pending exit edges from try-except-else parts (in the node scope) if finally part exists - final boolean isPending = PsiTreeUtil.isAncestor(node, pendingElement, false) && - (finallyPart == null || !PsiTreeUtil.isAncestor(finallyPart, pendingElement, false)); - if (!finallyRef.isNull() && isPending) { - myBuilder.addEdge(instruction, finallyRef.get()); - myBuilder.addPendingEdge(null, lastFinallyRef.get()); - return; - } - myBuilder.addPendingEdge(pendingScope, instruction); + // Duplicate CFG for finally (-fail and -success) only if there are no except parts and there are some successfull exits from the + // try part. Otherwise a single CFG for finally provides the correct control flow + final Instruction finallyInstruction; + if (node.getExceptParts().length == 0 && !normalExits.isEmpty()) { + // Finally-success part handling + storeAndClearPending(); + myBuilder.flowAbrupted(); + Instruction finallySuccessInstruction = myBuilder.startNode(finallyPart); + finallyPart.accept(this); + restorePending(); + finallyInstruction = finallySuccessInstruction; } - }); + else { + finallyInstruction = finallyFailInstruction; + } + + // Connect normal exits from try and else parts to the finally part + for (Instruction instr : normalExits) { + myBuilder.addEdge(instr, finallyInstruction); + } + } + } + + private void storeAndClearPending() { + myPendindBackup = myBuilder.pending; + myBuilder.pending = new ArrayList>(); + } + + private void restorePending() { + for (Pair pair : myPendindBackup) { + myBuilder.addPendingEdge(pair.first, pair.second); + } } @Override @@ -677,9 +728,15 @@ public class PyControlFlowBuilder extends PyRecursiveElementVisitor { } return !PsiTreeUtil.instanceOf(instruction.getElement(), PyReturnStatement.class, + PyBreakStatement.class, + PyContinueStatement.class, PyAssignmentStatement.class, PyRaiseStatement.class, - PyStatementList.class); + PyStatementList.class, + PyTryExceptStatement.class, + PyTryPart.class, + PyExceptPart.class, + PyFinallyPart.class); } } diff --git a/python/src/com/jetbrains/python/inspections/PyUnboundLocalVariableInspection.java b/python/src/com/jetbrains/python/inspections/PyUnboundLocalVariableInspection.java index 718b044254c9..989983c44a13 100644 --- a/python/src/com/jetbrains/python/inspections/PyUnboundLocalVariableInspection.java +++ b/python/src/com/jetbrains/python/inspections/PyUnboundLocalVariableInspection.java @@ -157,7 +157,7 @@ public class PyUnboundLocalVariableInspection extends PyInspection { ControlFlowUtil.iteratePrev(number, instructions, new Function() { public ControlFlowUtil.Operation fun(final Instruction inst) { try { - if (inst.num() == number){ + if (inst.num() == number) { return ControlFlowUtil.Operation.NEXT; } if (inst instanceof ReadWriteInstruction) { @@ -190,15 +190,15 @@ public class PyUnboundLocalVariableInspection extends PyInspection { if (readAccessSeen.get()){ return; } - if (resolve2Scope){ + if (resolve2Scope) { if (owner instanceof PyFile){ registerProblem(node, PyBundle.message("INSP.unbound.name.not.defined", name)); } else { registerUnboundLocal(node); } - } else - if (owner instanceof PyFunction && PsiTreeUtil.getParentOfType(owner, PyClass.class, PyFile.class) instanceof PyFile){ + } + else if (owner instanceof PyFunction && PsiTreeUtil.getParentOfType(owner, PyClass.class, PyFile.class) instanceof PyFile){ registerUnboundLocal(node); } } diff --git a/python/testData/codeInsight/controlflow/doubletry.py b/python/testData/codeInsight/controlflow/doubletry.py index abd44d20ba89..9271efbbb6a3 100644 --- a/python/testData/codeInsight/controlflow/doubletry.py +++ b/python/testData/codeInsight/controlflow/doubletry.py @@ -1,9 +1,11 @@ try: - from mercurial import lsprof + from mercurial import lsprof except ImportError: - raise Error + raise Error p = 123 try: - return foo + return foo + x = 1 finally: - print(p) + print(p) +y = 2 diff --git a/python/testData/codeInsight/controlflow/doubletry.txt b/python/testData/codeInsight/controlflow/doubletry.txt index 9bb500bafedb..e86f9f829999 100644 --- a/python/testData/codeInsight/controlflow/doubletry.txt +++ b/python/testData/codeInsight/controlflow/doubletry.txt @@ -6,14 +6,21 @@ 5(6) element: PyExceptPart 6(7) READ ACCESS: ImportError 7(8) element: PyRaiseStatement -8(18) READ ACCESS: Error +8(25) READ ACCESS: Error 9(10) element: PyAssignmentStatement 10(11) WRITE ACCESS: p 11(12) element: PyTryExceptStatement 12(13) element: PyTryPart 13(14) element: PyReturnStatement -14(15) READ ACCESS: foo -15(16) element: PyFinallyPart -16(17) element: PyPrintStatement -17(18) READ ACCESS: p -18() element: null \ No newline at end of file +14(17) READ ACCESS: foo +15(16) element: PyAssignmentStatement +16(17,20) WRITE ACCESS: x +17(18) element: PyFinallyPart +18(19) element: PyPrintStatement +19(25) READ ACCESS: p +20(21) element: PyFinallyPart +21(22) element: PyPrintStatement +22(23) READ ACCESS: p +23(24) element: PyAssignmentStatement +24(25) WRITE ACCESS: y +25() element: null \ No newline at end of file diff --git a/python/testData/codeInsight/controlflow/trybreak.txt b/python/testData/codeInsight/controlflow/trybreak.txt index 8015ea851fa7..423d9c282d9c 100644 --- a/python/testData/codeInsight/controlflow/trybreak.txt +++ b/python/testData/codeInsight/controlflow/trybreak.txt @@ -5,7 +5,7 @@ 4(5,8,11) READ ACCESS: bar 5(6) element: PyStatementList 6(7,8) WRITE ACCESS: i -7(8,11) element: PyBreakStatement +7(11) element: PyBreakStatement 8(9) element: PyExceptPart 9(10) element: PyRaiseStatement 10(12) READ ACCESS: Exception diff --git a/python/testData/codeInsight/controlflow/tryfinally.txt b/python/testData/codeInsight/controlflow/tryfinally.txt index eb31ca6443f1..8300f506891f 100644 --- a/python/testData/codeInsight/controlflow/tryfinally.txt +++ b/python/testData/codeInsight/controlflow/tryfinally.txt @@ -6,14 +6,21 @@ 5(6) element: PyTryPart 6(7) element: PyAssignmentStatement 7(8,9) READ ACCESS: open -8(9) WRITE ACCESS: status +8(9,16) WRITE ACCESS: status 9(10) element: PyFinallyPart 10(11) element: PyIfStatement 11(12) READ ACCESS: status -12(13,16) READ ACCESS: None +12(13,25) READ ACCESS: None 13(14) element: PyStatementList. Condition: status is not None:true 14(15) element: PyPrintStatement -15(16,18) READ ACCESS: status -16(17) element: PyExpressionStatement -17(18) READ ACCESS: status -18() element: null \ No newline at end of file +15(25) READ ACCESS: status +16(17) element: PyFinallyPart +17(18) element: PyIfStatement +18(19) READ ACCESS: status +19(20,23) READ ACCESS: None +20(21) element: PyStatementList. Condition: status is not None:true +21(22) element: PyPrintStatement +22(23) READ ACCESS: status +23(24) element: PyExpressionStatement +24(25) READ ACCESS: status +25() element: null \ No newline at end of file diff --git a/python/testData/codeInsight/controlflow/trytry.py b/python/testData/codeInsight/controlflow/trytry.py new file mode 100644 index 000000000000..cc9efc411422 --- /dev/null +++ b/python/testData/codeInsight/controlflow/trytry.py @@ -0,0 +1,27 @@ +a = 1 +try: + b = 2 + for x in [1, 2, 3]: # loop:8 + try: + c = 3 + try: + d = 4 + if x == 0: + break + elif x == 1: + continue + elif x == 2: + raise Exception() + elif x == 3: + return 42 + e = 5 + finally: # f:37,s:40 + f = 6 + g = 7 + finally: # f:45,s:48 + h = 8 + i = 9 + j = 10 +finally: # f:55,s:58 + k = 11 +l = 12 diff --git a/python/testData/codeInsight/controlflow/trytry.txt b/python/testData/codeInsight/controlflow/trytry.txt new file mode 100644 index 000000000000..709d7f1a0c8b --- /dev/null +++ b/python/testData/codeInsight/controlflow/trytry.txt @@ -0,0 +1,64 @@ +0(1) element: null +1(2) element: PyAssignmentStatement +2(3) WRITE ACCESS: a +3(4) element: PyTryExceptStatement +4(5) element: PyTryPart +5(6) element: PyAssignmentStatement +6(7,55) WRITE ACCESS: b +7(8,53,55) element: PyForStatement +8(9) element: PyStatementList +9(10,55) WRITE ACCESS: x +10(11) element: PyTryExceptStatement +11(12) element: PyTryPart +12(13) element: PyAssignmentStatement +13(14,45) WRITE ACCESS: c +14(15) element: PyTryExceptStatement +15(16) element: PyTryPart +16(17) element: PyAssignmentStatement +17(18,37) WRITE ACCESS: d +18(19,37) element: PyIfStatement +19(20,22,37) READ ACCESS: x +20(21) element: PyStatementList. Condition: x == 0:true +21(40) element: PyBreakStatement +22(23,26,37) element: PyIfPartElif. Condition: x == 0:false +23(24,37) READ ACCESS: x +24(25,37) element: PyIfPartElif. Condition: x == 1:true +25(7) element: PyContinueStatement +26(27,31,37) element: PyIfPartElif. Condition: x == 1:false +27(28,37) READ ACCESS: x +28(29,37) element: PyIfPartElif. Condition: x == 2:true +29(30) element: PyRaiseStatement +30(37) READ ACCESS: Exception +31(32,35,37) element: PyIfPartElif. Condition: x == 2:false +32(33,37) READ ACCESS: x +33(34,37) element: PyIfPartElif. Condition: x == 3:true +34(37) element: PyReturnStatement +35(36) element: PyAssignmentStatement +36(37,40) WRITE ACCESS: e +37(38) element: PyFinallyPart +38(39) element: PyAssignmentStatement +39(45) WRITE ACCESS: f +40(41) element: PyFinallyPart +41(42) element: PyAssignmentStatement +42(43,45) WRITE ACCESS: f +43(44) element: PyAssignmentStatement +44(45,48) WRITE ACCESS: g +45(46) element: PyFinallyPart +46(47) element: PyAssignmentStatement +47(55) WRITE ACCESS: h +48(49) element: PyFinallyPart +49(50) element: PyAssignmentStatement +50(51,55) WRITE ACCESS: h +51(52) element: PyAssignmentStatement +52(8,53,55) WRITE ACCESS: i +53(54) element: PyAssignmentStatement +54(55,58) WRITE ACCESS: j +55(56) element: PyFinallyPart +56(57) element: PyAssignmentStatement +57(63) WRITE ACCESS: k +58(59) element: PyFinallyPart +59(60) element: PyAssignmentStatement +60(61) WRITE ACCESS: k +61(62) element: PyAssignmentStatement +62(63) WRITE ACCESS: l +63() element: null \ No newline at end of file diff --git a/python/testSrc/com/jetbrains/python/PyControlFlowBuilderTest.java b/python/testSrc/com/jetbrains/python/PyControlFlowBuilderTest.java index 1ae6a9917476..c07f6c603805 100644 --- a/python/testSrc/com/jetbrains/python/PyControlFlowBuilderTest.java +++ b/python/testSrc/com/jetbrains/python/PyControlFlowBuilderTest.java @@ -137,6 +137,10 @@ public class PyControlFlowBuilderTest extends LightMarkedTestCase { doTest(); } + public void testTryTry() { + doTest(); + } + public void testIsinstance() { doTest(); }