Initial implementation of nonecheck directive; some directive design changes

--HG--
rename : tests/run/noneattributeacc.pyx => tests/run/nonecheck.pyx

Cython/Compiler/Code.py

@@ -161,6 +161,12 @@ class GlobalState(object):
     # interned_nums
     # cached_builtins
 
+    # directives       set             Temporary variable used to track
+    #                                  the current set of directives in the code generation
+    #                                  process.
+
+    directives = {}
+
     def __init__(self, rootwriter):
         self.filename_table = {}
         self.filename_list = []
@@ -415,7 +421,7 @@ class CCodeWriter(object):
     #                                  generation (labels and temps state etc.)
     # globalstate      GlobalState     contains state global for a C file (input file info,
     #                                  utility code, declared constants etc.)
-   
+    
     def __init__(self, create_from=None, buffer=None, copy_formatting=False):
         if buffer is None: buffer = StringIOTree()
         self.buffer = buffer

Cython/Compiler/ExprNodes.py

@@ -830,12 +830,10 @@ class NameNode(AtomicExprNode):
     #
     #  entry           Entry     Symbol table entry
     #  interned_cname  string
-    #  possible_var_values object See Optimize.FindPossibleVariableValues
     
     is_name = 1
     skip_assignment_decref = False
     entry = None
-    possible_var_values = None
 
     def create_analysed_rvalue(pos, env, entry):
         node = NameNode(pos)
@@ -1597,13 +1595,12 @@ class IndexNode(ExprNode):
             code.putln("%s = %s;" % (temp, index.result_code))
         # Generate buffer access code using these temps
         import Buffer
-        assert self.options is not None
         # The above could happen because child_attrs is wrong somewhere so that
         # options are not propagated.
         return Buffer.put_buffer_lookup_code(entry=self.base.entry,
                                              index_signeds=[i.type.signed for i in self.indices],
                                              index_cnames=index_temps,
-                                             options=self.options,
+                                             options=code.globalstate.directives,
                                              pos=self.pos, code=code)
 
 class SliceIndexNode(ExprNode):
@@ -2076,6 +2073,7 @@ class AttributeNode(ExprNode):
     #  obj          ExprNode
     #  attribute    string
     #  needs_none_check boolean        Used if obj is an extension type.
+    #                                  If set to True, it is known that the type is not None.
     #
     #  Used internally:
     #
@@ -2324,12 +2322,10 @@ class AttributeNode(ExprNode):
         else:
             # result_code contains what is needed, but we may need to insert
             # a check and raise an exception
-            if self.obj.type.is_extension_type and self.needs_none_check:
-                code.globalstate.use_utility_code(raise_noneattr_error_utility_code)
-                code.putln("if (%s) {" % code.unlikely("%s == Py_None") % self.obj.result_as(PyrexTypes.py_object_type))
-                code.putln("__Pyx_RaiseNoneAttributeError(\"%s\");" % self.attribute.encode("UTF-8")) # todo: fix encoding
-                code.putln(code.error_goto(self.pos))
-                code.putln("}")
+            if (self.obj.type.is_extension_type
+                  and self.needs_none_check
+                  and code.globalstate.directives['nonecheck']):
+                self.put_nonecheck(code)
     
     def generate_assignment_code(self, rhs, code):
         self.obj.generate_evaluation_code(code)
@@ -2341,6 +2337,11 @@ class AttributeNode(ExprNode):
                     rhs.py_result()))
             rhs.generate_disposal_code(code)
         else:
+            if (self.obj.type.is_extension_type
+                  and self.needs_none_check
+                  and code.globalstate.directives['nonecheck']):
+                self.put_nonecheck(code)
+
             select_code = self.result_code
             if self.type.is_pyobject:
                 rhs.make_owned_reference(code)
@@ -2370,6 +2371,14 @@ class AttributeNode(ExprNode):
         else:
             code.annotate(self.pos, AnnotationItem('c_attr', 'c attribute', size=len(self.attribute)))
 
+    def put_nonecheck(self, code):
+        code.globalstate.use_utility_code(raise_noneattr_error_utility_code)
+        code.putln("if (%s) {" % code.unlikely("%s == Py_None") % self.obj.result_as(PyrexTypes.py_object_type))
+        code.putln("__Pyx_RaiseNoneAttributeError(\"%s\");" % self.attribute.encode("UTF-8")) # todo: fix encoding
+        code.putln(code.error_goto(self.pos))
+        code.putln("}")
+
+
 #-------------------------------------------------------------------
 #
 #  Constructor nodes
@@ -3984,7 +3993,6 @@ class CoercionNode(ExprNode):
     def __init__(self, arg):
         self.pos = arg.pos
         self.arg = arg
-        self.options = arg.options
         if debug_coercion:
             print("%s Coercing %s" % (self, self.arg))
             

Cython/Compiler/Main.py

@@ -79,9 +79,8 @@ class Context:
         from ParseTreeTransforms import WithTransform, NormalizeTree, PostParse, PxdPostParse
         from ParseTreeTransforms import AnalyseDeclarationsTransform, AnalyseExpressionsTransform
         from ParseTreeTransforms import CreateClosureClasses, MarkClosureVisitor, DecoratorTransform
-        from ParseTreeTransforms import ResolveOptions
+        from ParseTreeTransforms import InterpretCompilerDirectives
         from Optimize import FlattenInListTransform, SwitchTransform, OptimizeRefcounting
-        from Optimize import OptimizeNoneChecking, FindPossibleVariableValues
         from Buffer import IntroduceBufferAuxiliaryVars
         from ModuleNode import check_c_classes
 
@@ -96,7 +95,7 @@ class Context:
             NormalizeTree(self),
             PostParse(self),
             _specific_post_parse,
-            ResolveOptions(self, self.pragma_overrides),
+            InterpretCompilerDirectives(self, self.pragma_overrides),
             FlattenInListTransform(),
             WithTransform(self),
             DecoratorTransform(self),
@@ -105,9 +104,7 @@ class Context:
             _check_c_classes,
             AnalyseExpressionsTransform(self),
             SwitchTransform(),
-            FindPossibleVariableValues(self),
             OptimizeRefcounting(self),
-            OptimizeNoneChecking(self),
 #            SpecialFunctions(self),
             #        CreateClosureClasses(context),
             ]

Cython/Compiler/ModuleNode.py

@@ -41,6 +41,7 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode):
     #
     #  scope                The module scope.
     #  compilation_source   A CompilationSource (see Main)
+    #  directives           Top-level compiler directives
 
     child_attrs = ["body"]
     
@@ -52,6 +53,7 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode):
                 env.doc.encoding = self.doc.encoding
         else:
             env.doc = self.doc
+        env.directives = self.directives
         self.body.analyse_declarations(env)
     
     def process_implementation(self, options, result):
@@ -245,6 +247,7 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode):
         self.generate_module_preamble(env, modules, h_code)
 
         code.globalstate.module_pos = self.pos
+        code.globalstate.directives = self.directives
 
         code.putln("")
         code.putln("/* Implementation of %s */" % env.qualified_name)

Cython/Compiler/Nodes.py

@@ -71,12 +71,10 @@ class Node(object):
     #  pos         (string, int, int)   Source file position
     #  is_name     boolean              Is a NameNode
     #  is_literal  boolean              Is a ConstNode
-    #  options     dict                 Compiler directives in effect for this node
     
     is_name = 0
     is_literal = 0
     temps = None
-    options = None
 
     # All descandants should set child_attrs to a list of the attributes
     # containing nodes considered "children" in the tree. Each such attribute
@@ -204,6 +202,53 @@ class Node(object):
                 res += "%s  %s: %s\n" % (indent, key, dump_child(value, level + 1))
             res += "%s>" % indent
             return res
+
+class CompilerDirectivesNode(Node):
+    """
+    Sets compiler directives for the children nodes
+    """
+    #  directives     {string:value}  A dictionary holding the right value for
+    #                                 *all* possible directives.
+    #  body           Node
+    child_attrs = ["body"]
+
+    def analyse_control_flow(self, env):
+        old = env.directives
+        env.directives = self.directives
+        self.body.analyse_control_flow(env)
+        env.directives = old
+
+    def analyse_declarations(self, env):
+        old = env.directives
+        env.directives = self.directives
+        self.body.analyse_declarations(env)
+        env.directives = old
+    
+    def analyse_expressions(self, env):
+        old = env.directives
+        env.directives = self.directives
+        self.body.analyse_expressions(env)
+        env.directives = old
+
+    def generate_function_definitions(self, env, code):
+        env_old = env.directives
+        code_old = code.globalstate.directives
+        code.globalstate.directives = self.directives
+        self.body.generate_function_definitions(env, code)
+        env.directives = env_old
+        code.globalstate.directives = code_old
+            
+    def generate_execution_code(self, code):
+        old = code.globalstate.directives
+        code.globalstate.directives = self.directives
+        self.body.generate_execution_code(code)
+        code.globalstate.directives = old
+            
+    def annotate(self, code):
+        old = code.globalstate.directives
+        code.globalstate.directives = self.directives
+        self.body.annotate(code)
+        code.globalstate.directives = old
         
 class BlockNode:
     #  Mixin class for nodes representing a declaration block.
@@ -2568,14 +2613,12 @@ class InPlaceAssignmentNode(AssignmentNode):
             target_lhs = ExprNodes.NameNode(self.dup.pos,
                                             name = self.dup.name,
                                             is_temp = self.dup.is_temp,
-                                            entry = self.dup.entry,
-                                            options = self.dup.options)
+                                            entry = self.dup.entry)
         elif isinstance(self.lhs, ExprNodes.AttributeNode):
             target_lhs = ExprNodes.AttributeNode(self.dup.pos,
                                                  obj = ExprNodes.CloneNode(self.lhs.obj),
                                                  attribute = self.dup.attribute,
-                                                 is_temp = self.dup.is_temp,
-                                                 options = self.dup.options)
+                                                 is_temp = self.dup.is_temp)
         elif isinstance(self.lhs, ExprNodes.IndexNode):
             if self.lhs.index:
                 index = ExprNodes.CloneNode(self.lhs.index)
@@ -2589,8 +2632,7 @@ class InPlaceAssignmentNode(AssignmentNode):
                                              base = ExprNodes.CloneNode(self.dup.base),
                                              index = index,
                                              indices = indices,
-                                             is_temp = self.dup.is_temp,
-                                             options = self.dup.options)
+                                             is_temp = self.dup.is_temp)
         self.lhs = target_lhs
         return self.dup
     

Cython/Compiler/Optimize.py

@@ -150,118 +150,3 @@ class OptimizeRefcounting(Visitor.CythonTransform):
                 lhs.skip_assignment_decref = True
         return node
 
-
-class ExtTypePossibleValues:
-    can_be_none = True
-    def copy_with(self, can_be_none=None):
-        result = ExtTypePossibleValues()
-        if can_be_none is not None:
-            result.can_be_none = can_be_none
-        return result
-    def new(self):
-        "Polymorphic constructor"
-        return ExtTypePossibleValues()
-
-class FindPossibleVariableValues(Visitor.CythonTransform):
-    """
-    Annotates NameNodes with information about the possible values
-    the variable referred to can take, *at that point* in the execution.
-
-    This is done on a best effort basis, so we can be as smart or dumb
-    as we want. A do-nothing-op should always be valid.
-
-    Each type of variable keeps a different type of "variable range"
-    information.
-
-    This information is invalid if the tree is reorganized (read:
-    keep this transform late in the pipeline).
-
-    Currently this is done:
-    - Extension types gets flagged 
-    """
-
-    #
-    # Manage info stack
-    #
-    def create_empty_knowledge(self, scope):
-        knowledge = {}
-        for entry in scope.entries.values():
-            if entry.type.is_extension_type:
-                knowledge[entry] = ExtTypePossibleValues()
-        return knowledge
-    
-    def visit_ModuleNode(self, node):
-        self.knowledge = self.create_empty_knowledge(node.scope)
-        self.visitchildren(node)
-        return node
-    
-    def visit_FuncDefNode(self, node):
-        oldknow = self.knowledge
-        self.knowledge = self.create_empty_knowledge(node.local_scope)
-        self.visitchildren(node)
-        self.knowledge = oldknow
-        return node
-
-    def visit_NameNode(self, node):
-        node.possible_var_values = self.knowledge.get(node.entry, None)
-        return node
-    
-    #
-    # Conditions which restrict possible variable values
-    #
-    def visit_IfClauseNode(self, clause):
-        def process():
-            self.visitchildren(clause)
-            return clause
-
-        # we're lazy and only check in one specific easy case: single comparison with None
-        # the code is a bit nasty but handling the proper cases will force through better code
-        # anyway
-        cond = clause.condition
-        if not isinstance(cond, ExprNodes.PrimaryCmpNode): return process()
-        if clause.condition.cascade is not None: return process()
-        if isinstance(cond.operand1, ExprNodes.NoneNode):
-            operand_checked = cond.operand2
-        elif isinstance(cond.operand2, ExprNodes.NoneNode):
-            operand_checked = cond.operand1
-        else:
-            return process()
-        if not isinstance(operand_checked, ExprNodes.NameNode):
-            return process()
-        entry = operand_checked.entry
-        if entry not in self.knowledge:
-            # Not tracking this variable
-            return process()
-        # Finally!
-        if cond.operator == 'is_not':
-            # Within this block we can assume the variable is not None
-            # (until it is reassigned)
-            self.visitchildren(clause, attrs=["condition"])
-            oldvalues = self.knowledge[entry]
-            self.knowledge[entry] = oldvalues.copy_with(can_be_none=False)
-            self.visitchildren(clause, attrs=["body"])
-            self.knowledge[entry] = oldvalues
-            return clause
-        else:
-            return process()
-            
-
-    # Assignments which reset possible variable values
-    def visit_SingleAssignmentNode(self, node):
-        if isinstance(node.lhs, ExprNodes.NameNode):
-            entry = node.lhs.entry
-            if entry in self.knowledge:
-                self.knowledge[entry] = self.knowledge[entry].new()
-        self.visitchildren(node)
-        return node
-
-class OptimizeNoneChecking(Visitor.CythonTransform):
-    def visit_AttributeNode(self, node):
-        if isinstance(node.obj, ExprNodes.NameNode):
-            obj = node.obj
-            if obj.type.is_extension_type and not obj.possible_var_values.can_be_none:
-                node.needs_none_check = False
-        self.visitchildren(node)
-        return node
-
-                

Cython/Compiler/Options.py

@@ -56,11 +56,13 @@ c_line_in_traceback = 1
 
 # Declare pragmas
 option_types = {
-    'boundscheck' : bool
+    'boundscheck' : bool,
+    'nonecheck' : bool
 }
 
 option_defaults = {
-    'boundscheck' : True
+    'boundscheck' : True,
+    'nonecheck' : False
 }
 
 def parse_option_value(name, value):

Cython/Compiler/ParseTreeTransforms.py

@@ -238,7 +238,7 @@ class PxdPostParse(CythonTransform):
         else:
             return node
 
-class ResolveOptions(CythonTransform):
+class InterpretCompilerDirectives(CythonTransform):
     """
     After parsing, options can be stored in a number of places:
     - #cython-comments at the top of the file (stored in ModuleNode)
@@ -246,28 +246,38 @@ class ResolveOptions(CythonTransform):
     - @cython.optionname decorators
     - with cython.optionname: statements
 
-    This transform is responsible for annotating each node with an
-    "options" attribute linking it to a dict containing the exact
-    options that are in effect for that node. Any corresponding decorators
-    or with statements are removed in the process.
+    This transform is responsible for interpreting these various sources
+    and store the option in two ways:
+    - Set the directives attribute of the ModuleNode for global directives.
+    - Use a CompilerDirectivesNode to override directives for a subtree.
+
+    (The first one is primarily to not have to modify with the tree
+    structure, so that ModuleNode stay on top.)
+
+    The directives are stored in dictionaries from name to value in effect.
+    Each such dictionary is always filled in for all possible directives,
+    using default values where no value is given by the user.
+
+    The available directives are controlled in Options.py.
 
     Note that we have to run this prior to analysis, and so some minor
     duplication of functionality has to occur: We manually track cimports
-    to correctly intercept @cython... and with cython...
+    and which names the "cython" module may have been imported to.
     """
 
     def __init__(self, context, compilation_option_overrides):
-        super(ResolveOptions, self).__init__(context)
+        super(InterpretCompilerDirectives, self).__init__(context)
         self.compilation_option_overrides = compilation_option_overrides
         self.cython_module_names = set()
         self.option_names = {}
 
+    # Set up processing and handle the cython: comments.
     def visit_ModuleNode(self, node):
         options = copy.copy(Options.option_defaults)
         options.update(node.option_comments)
         options.update(self.compilation_option_overrides)
         self.options = options
-        node.options = options
+        node.directives = options
         self.visitchildren(node)
         return node
 
@@ -297,7 +307,6 @@ class ResolveOptions(CythonTransform):
         return node
 
     def visit_Node(self, node):
-        node.options = self.options
         self.visitchildren(node)
         return node
 
@@ -329,14 +338,17 @@ class ResolveOptions(CythonTransform):
 
         return None
 
-    def visit_with_options(self, node, options):
+    def visit_with_options(self, body, options):
         oldoptions = self.options
         newoptions = copy.copy(oldoptions)
         newoptions.update(options)
         self.options = newoptions
-        node = self.visit_Node(node)
+        assert isinstance(body, StatListNode), body
+        retbody = self.visit_Node(body)
+        directive = CompilerDirectivesNode(pos=retbody.pos, body=retbody,
+                                           directives=options)
         self.options = oldoptions
-        return node  
+        return directive
  
     # Handle decorators
     def visit_DefNode(self, node):
@@ -359,7 +371,8 @@ class ResolveOptions(CythonTransform):
             for option in options:
                 name, value = option
                 optdict[name] = value
-            return self.visit_with_options(node, optdict)
+            body = StatListNode(node.pos, stats=[node])
+            return self.visit_with_options(body, optdict)
         else:
             return self.visit_Node(node)
                                    
@@ -370,8 +383,7 @@ class ResolveOptions(CythonTransform):
             if node.target is not None:
                 raise PostParseError(node.pos, "Compiler option with statements cannot contain 'as'")
             name, value = option
-            self.visit_with_options(node.body, {name:value})
-            return node.body.stats
+            return self.visit_with_options(node.body, {name:value})
         else:
             return self.visit_Node(node)
 
@@ -427,7 +439,7 @@ class WithTransform(CythonTransform):
                 u'BODY' : node.body,
                 u'TARGET' : node.target,
                 u'EXCINFO' : excinfo_namenode
-                }, pos = node.pos)
+                }, pos=node.pos)
             # Set except excinfo target to EXCINFO
             result.stats[4].body.stats[0].except_clauses[0].excinfo_target = excinfo_target
         else:
@@ -435,7 +447,7 @@ class WithTransform(CythonTransform):
                 u'EXPR' : node.manager,
                 u'BODY' : node.body,
                 u'EXCINFO' : excinfo_namenode
-                }, pos = node.pos)
+                }, pos=node.pos)
             # Set except excinfo target to EXCINFO
             result.stats[4].body.stats[0].except_clauses[0].excinfo_target = excinfo_target
         

Cython/Compiler/Symtab.py

@@ -178,6 +178,8 @@ class Scope:
     #                                        Python strings in this scope
     # control_flow     ControlFlow  Used for keeping track of environment state
     # nogil             boolean            In a nogil section
+    # directives       dict                Helper variable for the recursive
+    #                                      analysis, contains directive values.
 
     is_py_class_scope = 0
     is_c_class_scope = 0
@@ -185,6 +187,7 @@ class Scope:
     scope_prefix = ""
     in_cinclude = 0
     nogil = 0
+    directives = {}
     
     temp_prefix = Naming.pyrex_prefix
     

Cython/Compiler/Visitor.py

@@ -111,6 +111,7 @@ class TreeVisitor(BasicVisitor):
                     childretval = [self.visitchild(x, parent, attr, idx) for idx, x in enumerate(child)]
                 else:
                     childretval = self.visitchild(child, parent, attr, None)
+                    assert not isinstance(childretval, list), 'Cannot insert list here: %s in %r' % (attr, parent)
                 result[attr] = childretval
         return result
 

tests/run/noneattributeacc.pyx → tests/run/nonecheck.pyx

@@ -3,13 +3,24 @@ Tests accessing attributes of extension type variables
 set to None
 
 >>> obj = MyClass(2, 3)
->>> func(obj)
+>>> getattr_(obj)
 2
->>> func(None)
+>>> getattr_(None)
 Traceback (most recent call last):
    ...
 AttributeError: 'NoneType' object has no attribute 'a'
 
+>>> setattr_(obj)
+>>> getattr_(obj)
+10
+>>> setattr_(None)
+Traceback (most recent call last):
+   ...
+AttributeError: 'NoneType' object has no attribute 'a'
+
+
+
+>>> obj = MyClass(2, 3)
 >>> checking(obj)
 2
 2
@@ -23,18 +34,26 @@ AttributeError: 'NoneType' object has no attribute 'a'
 
 """
 
+cimport cython
+
 cdef class MyClass:
     cdef int a, b
     def __init__(self, a, b):
         self.a = a
         self.b = b
 
-def func(MyClass var):
+@cython.nonecheck(True)
+def getattr_(MyClass var):
     print var.a
 
+@cython.nonecheck(True)
+def setattr_(MyClass var):
+    var.a = 10
+
 def some():
     return MyClass(4, 5)
 
+@cython.nonecheck(True)
 def checking(MyClass var):
     state = (var is None)
     if not state:
@@ -44,6 +63,7 @@ def checking(MyClass var):
     else:
         print "var is None"
 
+@cython.nonecheck(True)
 def check_and_assign(MyClass var):
     if var is not None:
         print var.a