avoid unnecessary None checks based on control flow analysis (ticket #743)

Cython/Compiler/ExprNodes.py

@@ -1481,6 +1481,16 @@ class NameNode(AtomicExprNode):
         #  If it's not a C variable, it'll be in a temp.
         return 1
 
+    def may_be_none(self):
+        if self.cf_state:
+            # evaluate control flow state to see if there were any
+            # potential None values assigned to the node so far
+            for assignment in self.cf_state:
+                if assignment.rhs.may_be_none():
+                    return True
+            return False
+        return super(NameNode, self).may_be_none()
+
     def nonlocally_immutable(self):
         if ExprNode.nonlocally_immutable(self):
             return True

Cython/Compiler/FlowControl.py

@@ -14,11 +14,16 @@ from Visitor import TreeVisitor, CythonTransform
 from Errors import error, warning, CompileError, InternalError
 
 class TypedExprNode(ExprNodes.ExprNode):
-    # Used for declaring assignments of a specified type whithout a known entry.
-    def __init__(self, type):
+    # Used for declaring assignments of a specified type without a known entry.
+    def __init__(self, type, may_be_none=None):
         self.type = type
+        self._may_be_none = may_be_none
 
-object_expr = TypedExprNode(py_object_type)
+    def may_be_none(self):
+        return self._may_be_none != False
+
+object_expr = TypedExprNode(py_object_type, may_be_none=True)
+object_expr_not_none = TypedExprNode(py_object_type, may_be_none=False)
 
 class ControlBlock(object):
     """Control flow graph node. Sequence of assignments and name references.
@@ -588,11 +593,13 @@ class CreateControlFlowGraph(CythonTransform):
 
         if node.star_arg:
             self.flow.mark_argument(node.star_arg,
-                                    TypedExprNode(Builtin.tuple_type),
+                                    TypedExprNode(Builtin.tuple_type,
+                                                  may_be_none=False),
                                     node.star_arg.entry)
         if node.starstar_arg:
             self.flow.mark_argument(node.starstar_arg,
-                                    TypedExprNode(Builtin.dict_type),
+                                    TypedExprNode(Builtin.dict_type,
+                                                  may_be_none=False),
                                     node.starstar_arg.entry)
         self.visitchildren(node)
         # Workaround for generators
@@ -621,7 +628,7 @@ class CreateControlFlowGraph(CythonTransform):
         if entry.is_anonymous:
             entry = self.env.lookup(node.name)
         if entry:
-            self.flow.mark_assignment(node, object_expr, entry)
+            self.flow.mark_assignment(node, object_expr_not_none, entry)
         return self.visit_FuncDefNode(node)
 
     def visit_GeneratorBodyDefNode(self, node):
@@ -717,7 +724,8 @@ class CreateControlFlowGraph(CythonTransform):
     def visit_CArgDeclNode(self, node):
         entry = self.env.lookup(node.name)
         if entry:
-            self.flow.mark_argument(node, TypedExprNode(entry.type), entry)
+            may_be_none = not node.not_none
+            self.flow.mark_argument(node, TypedExprNode(entry.type, may_be_none), entry)
         return node
 
     def visit_NameNode(self, node):
@@ -1091,7 +1099,7 @@ class CreateControlFlowGraph(CythonTransform):
 
     def visit_PyClassDefNode(self, node):
         self.flow.mark_assignment(node.target,
-                                  object_expr, self.env.lookup(node.name))
+                                  object_expr_not_none, self.env.lookup(node.name))
         # TODO: add negative attribute list to "visitchildren"?
         self.visitchildren(node, attrs=['dict', 'metaclass',
                                         'mkw', 'bases', 'classobj'])

Cython/Compiler/Optimize.py

@@ -3530,3 +3530,12 @@ class FinalOptimizePhase(Visitor.CythonTransform):
             if not node.arg.may_be_none():
                 node.notnone = True
         return node
+
+    def visit_NoneCheckNode(self, node):
+        """Remove None checks from expressions that definitely do not
+        carry a None value.
+        """
+        self.visitchildren(node)
+        if not node.arg.may_be_none():
+            return node.arg
+        return node

tests/run/cf_none.pyx

+
+cimport cython
+
+@cython.test_fail_if_path_exists('//NoneCheckNode')
+def none_checks(a):
+    """
+    >>> none_checks(1)
+    22
+    >>> none_checks(None)
+    True
+    """
+    c = None
+    d = {11:22}
+    if a is c:
+        return True
+    else:
+        return d.get(11)
+
+@cython.test_assert_path_exists('//NoneCheckNode')
+def dict_arg(dict a):
+    """
+    >>> dict_arg({})
+    >>> dict_arg({1:2})
+    2
+    """
+    return a.get(1)
+
+@cython.test_fail_if_path_exists('//NoneCheckNode')
+def dict_arg_not_none(dict a not None):
+    """
+    >>> dict_arg_not_none({})
+    >>> dict_arg_not_none({1:2})
+    2
+    """
+    return a.get(1)
+
+@cython.test_assert_path_exists('//NoneCheckNode')
+def reassignment(dict d):
+    """
+    >>> reassignment({})
+    (None, 2)
+    >>> reassignment({1:3})
+    (3, 2)
+    """
+    a = d.get(1)
+    d = {1:2}
+    b = d.get(1)
+    return a, b
+
+@cython.test_fail_if_path_exists('//NoneCheckNode')
+def conditional(a):
+    """
+    >>> conditional(True)
+    2
+    >>> conditional(False)
+    3
+    """
+    if a:
+        d = {1:2}
+    else:
+        d = {1:3}
+    return d.get(1)
+
+@cython.test_assert_path_exists('//NoneCheckNode')
+def conditional_arg(a, dict d):
+    """
+    >>> conditional_arg(True,  {1:2})
+    >>> conditional_arg(False, {1:2})
+    2
+    """
+    if a:
+        d = {}
+    return d.get(1)
+
+@cython.test_fail_if_path_exists('//NoneCheckNode')
+def conditional_not_none(a, dict d not None):
+    """
+    >>> conditional_not_none(True,  {1:2})
+    >>> conditional_not_none(False, {1:2})
+    2
+    """
+    if a:
+        d = {}
+    return d.get(1)

tests/run/list.pyx

@@ -57,7 +57,6 @@ def test_list_sort_reversed():
     l1.sort(reversed=True)
     return l1
 
-@cython.test_assert_path_exists("//SimpleCallNode//NoneCheckNode")
 def test_list_reverse():
     """
     >>> test_list_reverse()
@@ -68,7 +67,6 @@ def test_list_reverse():
     l1.reverse()
     return l1
 
-@cython.test_assert_path_exists("//SimpleCallNode//NoneCheckNode")
 def test_list_append():
     """
     >>> test_list_append()