rewrite except-as semantics using try-finally - seems to be the easiest way to...

rewrite except-as semantics using try-finally - seems to be the easiest way to get flow control analysis working for all cases

Cython/Compiler/ExprNodes.py

@@ -569,7 +569,7 @@ class ExprNode(Node):
         #  been reported earlier.
         pass
 
-    def generate_deletion_code(self, code):
+    def generate_deletion_code(self, code, ignore_nonexisting=False):
         #  Stub method for nodes that are not legal as
         #  the argument of a del statement. An error
         #  will have been reported earlier.
@@ -1853,30 +1853,46 @@ class NameNode(AtomicExprNode):
             code.putln("%s = 0;" % rhstmp)
             code.funcstate.release_temp(rhstmp)
 
-    def generate_deletion_code(self, code):
+    def generate_deletion_code(self, code, ignore_nonexisting=False):
         if self.entry is None:
             return # There was an error earlier
         elif self.entry.is_pyclass_attr:
             namespace = self.entry.scope.namespace_cname
             interned_cname = code.intern_identifier(self.entry.name)
-            code.put_error_if_neg(self.pos,
-                'PyObject_DelItem(%s, %s)' % (
-                    namespace,
-                    interned_cname))
+            del_code = 'PyObject_DelItem(%s, %s)' % (namespace, interned_cname)
+            if ignore_nonexisting:
+                code.putln('if (unlikely(%s < 0)) { if (likely(PyErr_ExceptionMatches(PyExc_KeyError))) PyErr_Clear(); else %s }' % (
+                    del_code,
+                    code.error_goto(self.pos)))
+            else:
+                code.put_error_if_neg(self.pos, del_code)
         elif self.entry.is_pyglobal:
-            code.put_error_if_neg(self.pos,
-                '__Pyx_DelAttrString(%s, "%s")' % (
-                    Naming.module_cname,
-                    self.entry.name))
+            py_name = code.get_py_string_const(
+                self.entry.name, is_str=True, identifier=True)
+            del_code = 'PyObject_DelAttr(%s, %s)' % (
+                Naming.module_cname, py_name)
+            if ignore_nonexisting:
+                code.putln('if (unlikely(%s < 0)) { if (likely(PyErr_ExceptionMatches(PyExc_AttributeError))) PyErr_Clear(); else %s }' % (
+                    del_code,
+                    code.error_goto(self.pos)))
+            else:
+                code.put_error_if_neg(self.pos, del_code)
         elif self.entry.type.is_pyobject or self.entry.type.is_memoryviewslice:
             if not self.cf_is_null:
-                if self.cf_maybe_null:
+                if self.cf_maybe_null and not ignore_nonexisting:
                     code.put_error_if_unbound(self.pos, self.entry)
 
                 if self.entry.type.is_pyobject:
                     if self.entry.in_closure:
-                        code.put_gotref(self.result()) # generator
-                    code.put_decref(self.result(), self.ctype())
+                        # generator
+                        if ignore_nonexisting and self.cf_maybe_null:
+                            code.put_xgotref(self.result())
+                        else:
+                            code.put_gotref(self.result())
+                    if ignore_nonexisting and self.cf_maybe_null:
+                        code.put_xdecref(self.result(), self.ctype())
+                    else:
+                        code.put_decref(self.result(), self.ctype())
                     code.putln('%s = NULL;' % self.result())
                 else:
                     code.put_xdecref_memoryviewslice(self.entry.cname,
@@ -3221,7 +3237,7 @@ class IndexNode(ExprNode):
         rhs.generate_disposal_code(code)
         rhs.free_temps(code)
 
-    def generate_deletion_code(self, code):
+    def generate_deletion_code(self, code, ignore_nonexisting=False):
         self.generate_subexpr_evaluation_code(code)
         #if self.type.is_pyobject:
         if self.index.type.is_int:
@@ -3513,7 +3529,7 @@ class SliceIndexNode(ExprNode):
         rhs.generate_disposal_code(code)
         rhs.free_temps(code)
 
-    def generate_deletion_code(self, code):
+    def generate_deletion_code(self, code, ignore_nonexisting=False):
         if not self.base.type.is_pyobject:
             error(self.pos,
                   "Deleting slices is only supported for Python types, not '%s'." % self.type)
@@ -4846,7 +4862,7 @@ class AttributeNode(ExprNode):
         self.obj.generate_disposal_code(code)
         self.obj.free_temps(code)
 
-    def generate_deletion_code(self, code):
+    def generate_deletion_code(self, code, ignore_nonexisting=False):
         self.obj.generate_evaluation_code(code)
         if self.is_py_attr or (isinstance(self.entry.scope, Symtab.PropertyScope)
                                and u'__del__' in self.entry.scope.entries):

Cython/Compiler/FlowControl.py

@@ -1098,8 +1098,6 @@ class ControlFlowAnalysis(CythonTransform):
             if clause.target:
                 self.mark_assignment(clause.target)
             self.visit(clause.body)
-            if clause.is_except_as:
-                self.flow.mark_deletion(clause.target, clause.target.entry)
             if self.flow.block:
                 self.flow.block.add_child(next_block)
 

Cython/Compiler/Nodes.py

@@ -4774,6 +4774,7 @@ class DelStatNode(StatNode):
     #  args     [ExprNode]
 
     child_attrs = ["args"]
+    ignore_nonexisting = False
 
     def analyse_declarations(self, env):
         for arg in self.args:
@@ -4803,7 +4804,8 @@ class DelStatNode(StatNode):
     def generate_execution_code(self, code):
         for arg in self.args:
             if arg.type.is_pyobject or arg.type.is_memoryviewslice:
-                arg.generate_deletion_code(code)
+                arg.generate_deletion_code(
+                    code, ignore_nonexisting=self.ignore_nonexisting)
             elif arg.type.is_ptr and arg.type.base_type.is_cpp_class:
                 arg.generate_result_code(code)
                 code.putln("delete %s;" % arg.result())
@@ -6136,10 +6138,6 @@ class ExceptClauseNode(Node):
             self.excinfo_tuple.generate_disposal_code(code)
         for var in exc_vars:
             code.put_decref_clear(var, py_object_type)
-        if self.is_except_as and self.target:
-            # "except ... as x" deletes x after use
-            # target is known to be a NameNode
-            self.target.generate_deletion_code(code)
         code.put_goto(end_label)
 
         for new_label, old_label in [(code.break_label, old_break_label),
@@ -6150,8 +6148,6 @@ class ExceptClauseNode(Node):
                     self.excinfo_tuple.generate_disposal_code(code)
                 for var in exc_vars:
                     code.put_decref_clear(var, py_object_type)
-                if self.is_except_as and self.target:
-                    self.target.generate_deletion_code(code)
                 code.put_goto(old_label)
         code.break_label = old_break_label
         code.continue_label = old_continue_label

Cython/Compiler/ParseTreeTransforms.py

@@ -332,6 +332,25 @@ class PostParse(ScopeTrackingTransform):
         node.args = self._flatten_sequence(node, [])
         return node
 
+    def visit_ExceptClauseNode(self, node):
+        if node.is_except_as:
+            # except-as must delete NameNode target at the end
+            del_target = Nodes.DelStatNode(
+                node.pos,
+                args=[ExprNodes.NameNode(
+                    node.target.pos, name=node.target.name)],
+                ignore_nonexisting=True)
+            node.body = Nodes.StatListNode(
+                node.pos,
+                stats=[Nodes.TryFinallyStatNode(
+                    node.pos,
+                    body=node.body,
+                    finally_clause=Nodes.StatListNode(
+                        node.pos,
+                        stats=[del_target]))])
+        self.visitchildren(node)
+        return node
+
 
 def eliminate_rhs_duplicates(expr_list_list, ref_node_sequence):
     """Replace rhs items by LetRefNodes if they appear more than once.

tests/run/cython3.pyx

@@ -62,6 +62,19 @@ no_match_does_not_touch_target = (e == 123)
 
 ### more except-as tests
 
+def except_as_no_raise_does_not_touch_target(a):
+    """
+    >>> except_as_no_raise_does_not_touch_target(TypeError)
+    (1, 1)
+    """
+    b = 1
+    try:
+        i = 1
+    except a as b:
+        i = 2
+    return i, b
+
+
 def except_as_raise_deletes_target(x, a):
     """
     >>> except_as_raise_deletes_target(None, TypeError)
@@ -89,14 +102,79 @@ def except_as_raise_deletes_target(x, a):
     return i
 
 
+def except_as_raise_deletes_target_even_after_del(x, a):
+    """
+    >>> except_as_raise_deletes_target_even_after_del(None, TypeError)
+    1
+    1
+    >>> except_as_raise_deletes_target_even_after_del(TypeError('test'), TypeError)
+    2
+    >>> except_as_raise_deletes_target_even_after_del(ValueError('test'), TypeError)
+    Traceback (most recent call last):
+    ValueError: test
+    >>> except_as_raise_deletes_target_even_after_del(None, TypeError)
+    1
+    1
+    """
+    b = 1
+    try:
+        i = 1
+        if x:
+            raise x
+    except a as b:
+        i = 2
+        assert isinstance(b, a)
+        del b  # let's see if Cython can still 'del' it after this line!
+    try:
+        print(b)  # raises UnboundLocalError if except clause was executed
+    except UnboundLocalError:
+        pass
+    else:
+        if x:
+            print("UnboundLocalError not raised!")
+    return i
+
+
+def except_as_raise_deletes_target_on_error(x, a):
+    """
+    >>> except_as_raise_deletes_target_on_error(None, TypeError)
+    1
+    1
+    >>> except_as_raise_deletes_target_on_error(TypeError('test'), TypeError)
+    Traceback (most recent call last):
+    UnboundLocalError: local variable 'b' referenced before assignment
+    >>> except_as_raise_deletes_target_on_error(ValueError('test'), TypeError)
+    Traceback (most recent call last):
+    ValueError: test
+    >>> except_as_raise_deletes_target_on_error(None, TypeError)
+    1
+    1
+    """
+    b = 1
+    try:
+        try:
+            i = 1
+            if x:
+                raise x
+        except a as b:
+            i = 2
+            raise IndexError("TEST")
+    except IndexError as e:
+        assert 'TEST' in str(e), str(e)
+    print(b)  # raises UnboundLocalError if except clause was executed
+    return i
+
+
 def except_as_raise_with_empty_except(x, a):
     """
     >>> except_as_raise_with_empty_except(None, TypeError)
+    1
     >>> except_as_raise_with_empty_except(TypeError('test'), TypeError)
     >>> except_as_raise_with_empty_except(ValueError('test'), TypeError)
     Traceback (most recent call last):
     ValueError: test
     >>> except_as_raise_with_empty_except(None, TypeError)
+    1
     """
     try:
         if x:
@@ -104,6 +182,14 @@ def except_as_raise_with_empty_except(x, a):
         b = 1
     except a as b:  # previously raised UnboundLocalError
         pass
+    try:
+        print(b)  # raises UnboundLocalError if except clause was executed
+    except UnboundLocalError:
+        if not x:
+            print("unexpected UnboundLocalError raised!")
+    else:
+        if x:
+            print("expected UnboundLocalError not raised!")
 
 
 def except_as_deletes_target_in_gen(x, a):