From 5cab072b93bdb00c3b1ab1beef211801df7377c9 Mon Sep 17 00:00:00 2001 From: Dag Sverre Seljebotn Date: Tue, 30 Jun 2009 11:08:27 +0200 Subject: [PATCH] nogil check changes; fixes #338 and #329 --- Cython/Compiler/ExprNodes.py | 68 ++++++++++++-------------- Cython/Compiler/Nodes.py | 43 ++++++++-------- Cython/Compiler/ParseTreeTransforms.py | 22 +++++---- tests/errors/nogil.pyx | 51 +++++++++++-------- tests/run/nogil.pyx | 2 + 5 files changed, 98 insertions(+), 88 deletions(-) diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py index cc019e81..37becf48 100644 --- a/Cython/Compiler/ExprNodes.py +++ b/Cython/Compiler/ExprNodes.py @@ -330,12 +330,12 @@ class ExprNode(Node): def analyse_target_types(self, env): self.analyse_types(env) - def gil_check(self, env): + def nogil_check(self, env): # By default, any expression based on Python objects is # prevented in nogil environments. Subtypes must override # this if they can work without the GIL. if self.type.is_pyobject: - self._gil_check(env) + self.gil_error() def gil_assignment_check(self, env): if env.nogil and self.type.is_pyobject: @@ -599,7 +599,8 @@ class NoneNode(PyConstNode): value = "Py_None" constant_result = None - gil_check = None + + nogil_check = None def compile_time_value(self, denv): return None @@ -621,7 +622,7 @@ class ConstNode(AtomicExprNode): # value string C code fragment is_literal = 1 - gil_check = None + nogil_check = None def is_simple(self): return 1 @@ -1065,14 +1066,14 @@ class NameNode(AtomicExprNode): self.is_used_as_rvalue = 1 env.use_utility_code(get_name_interned_utility_code) - def gil_check(self, env): + def nogil_check(self, env): if self.is_used_as_rvalue: entry = self.entry if entry.is_builtin: # if not Options.cache_builtins: # cached builtins are ok - self._gil_check(env) + self.gil_error() elif entry.is_pyglobal: - self._gil_check(env) + self.gil_error() gil_message = "Accessing Python global or builtin" @@ -1671,24 +1672,17 @@ class IndexNode(ExprNode): error(self.pos, "Invalid index type '%s'" % self.index.type) - self.gil_check(env) - - def gil_check(self, env): - if not self.is_buffer_access: - if self.base.type.is_pyobject: - self._gil_check(env) - gil_message = "Indexing Python object" - def gil_check(self, env): - if self.is_buffer_access and env.nogil: + def nogil_check(self, env): + if self.is_buffer_access: if env.directives['boundscheck']: error(self.pos, "Cannot check buffer index bounds without gil; use boundscheck(False) directive") return elif self.type.is_pyobject: error(self.pos, "Cannot access buffer with object dtype without gil") return - super(IndexNode, self).gil_check(env) + super(IndexNode, self).nogil_check(env) def check_const_addr(self): @@ -1946,7 +1940,7 @@ class SliceIndexNode(ExprNode): self.stop = self.stop.coerce_to(c_int, env) self.is_temp = 1 - gil_check = ExprNode._gil_check + nogil_check = Node.gil_error gil_message = "Slicing Python object" def generate_result_code(self, code): @@ -2164,12 +2158,12 @@ class CallNode(ExprNode): self.coerce_to(type, env) return True - def gil_check(self, env): + def nogil_check(self, env): func_type = self.function_type() if func_type.is_pyobject: - self._gil_check(env) + self.gil_error() elif not getattr(func_type, 'nogil', False): - self._gil_check(env) + self.gil_error() gil_message = "Calling gil-requiring function" @@ -2466,7 +2460,7 @@ class GeneralCallNode(CallNode): subexprs = ['function', 'positional_args', 'keyword_args', 'starstar_arg'] - gil_check = CallNode._gil_check + nogil_check = Node.gil_error def compile_time_value(self, denv): function = self.function.compile_time_value(denv) @@ -2568,7 +2562,7 @@ class AsTupleNode(ExprNode): self.type = tuple_type self.is_temp = 1 - gil_check = ExprNode._gil_check + nogil_check = Node.gil_error gil_message = "Constructing Python tuple" def generate_result_code(self, code): @@ -2799,9 +2793,9 @@ class AttributeNode(ExprNode): "Object of type '%s' has no attribute '%s'" % (obj_type, self.attribute)) - def gil_check(self, env): + def nogil_check(self, env): if self.is_py_attr: - self._gil_check(env) + self.gil_error() gil_message = "Accessing Python attribute" @@ -3595,7 +3589,7 @@ class DictItemNode(ExprNode): # value ExprNode subexprs = ['key', 'value'] - gil_check = None # handled by DictNode + nogil_check = None # Parent DictNode takes care of it def calculate_constant_result(self): self.constant_result = ( @@ -3772,9 +3766,9 @@ class UnopNode(ExprNode): def is_py_operation(self): return self.operand.type.is_pyobject - def gil_check(self, env): + def nogil_check(self, env): if self.is_py_operation(): - self._gil_check(env) + self.gil_error() def coerce_operand_to_pyobject(self, env): self.operand = self.operand.coerce_to_pyobject(env) @@ -3979,9 +3973,9 @@ class TypecastNode(ExprNode): if self.typecheck and self.type.is_extension_type: self.operand = PyTypeTestNode(self.operand, self.type, env) - def gil_check(self, env): - if self.type.is_pyobject and self.is_temp: - self._gil_check(env) + def nogil_check(self, env): + if self.type and self.type.is_pyobject and self.is_temp: + self.gil_error() def check_const(self): self.operand.check_const() @@ -4186,9 +4180,9 @@ class BinopNode(ExprNode): return (self.operand1.type.is_pyobject or self.operand2.type.is_pyobject) - def gil_check(self, env): + def nogil_check(self, env): if self.is_py_operation(): - self._gil_check(env) + self.gil_error() def coerce_operands_to_pyobjects(self, env): self.operand1 = self.operand1.coerce_to_pyobject(env) @@ -5137,7 +5131,7 @@ class PyTypeTestNode(CoercionNode): self.type = dst_type self.result_ctype = arg.ctype() - gil_check = CoercionNode._gil_check + nogil_check = Node.gil_error gil_message = "Python type test" def analyse_types(self, env): @@ -5293,9 +5287,9 @@ class CoerceToBooleanNode(CoercionNode): if arg.type.is_pyobject: self.is_temp = 1 - def gil_check(self, env): + def nogil_check(self, env): if self.arg.type.is_pyobject: - self._gil_check(env) + self.gil_error() gil_message = "Truth-testing Python object" @@ -5381,7 +5375,7 @@ class CloneNode(CoercionNode): # node is responsible for doing those things. subexprs = [] # Arg is not considered a subexpr - gil_check = None + nogil_check = None def __init__(self, arg): CoercionNode.__init__(self, arg) diff --git a/Cython/Compiler/Nodes.py b/Cython/Compiler/Nodes.py index d4d8ee9e..17aa3f81 100644 --- a/Cython/Compiler/Nodes.py +++ b/Cython/Compiler/Nodes.py @@ -133,12 +133,9 @@ class Node(object): gil_message = "Operation" - gil_check = None - def _gil_check(self, env): - if env.nogil: - self.gil_error() + nogil_check = None - def gil_error(self): + def gil_error(self, env=None): error(self.pos, "%s not allowed without gil" % self.gil_message) def clone_node(self): @@ -1365,14 +1362,14 @@ class CFuncDefNode(FuncDefNode): def need_gil_acquisition(self, lenv): return self.type.with_gil - def gil_check(self, env): + def nogil_check(self, env): type = self.type with_gil = type.with_gil if type.nogil and not with_gil: if type.return_type.is_pyobject: error(self.pos, "Function with Python return type cannot be declared nogil") - for entry in env.var_entries: + for entry in self.local_scope.var_entries: if entry.type.is_pyobject: error(self.pos, "Function declared nogil has Python locals or temporaries") @@ -3190,7 +3187,7 @@ class PrintStatNode(StatNode): if len(self.arg_tuple.args) == 1 and self.append_newline: env.use_utility_code(printing_one_utility_code) - gil_check = StatNode._gil_check + nogil_check = Node.gil_error gil_message = "Python print statement" def generate_execution_code(self, code): @@ -3232,7 +3229,7 @@ class ExecStatNode(StatNode): self.args[i] = arg env.use_utility_code(Builtin.pyexec_utility_code) - gil_check = StatNode._gil_check + nogil_check = Node.gil_error gil_message = "Python exec statement" def generate_execution_code(self, code): @@ -3276,10 +3273,10 @@ class DelStatNode(StatNode): error(arg.pos, "Deletion of non-Python object") #arg.release_target_temp(env) - def gil_check(self, env): + def nogil_check(self, env): for arg in self.args: if arg.type.is_pyobject: - self._gil_check(env) + self.gil_error() gil_message = "Deleting Python object" @@ -3363,9 +3360,9 @@ class ReturnStatNode(StatNode): and not return_type.is_returncode): error(self.pos, "Return value required") - def gil_check(self, env): + def nogil_check(self, env): if self.return_type.is_pyobject: - self._gil_check(env) + self.gil_error() gil_message = "Returning Python object" @@ -3425,7 +3422,7 @@ class RaiseStatNode(StatNode): env.use_utility_code(raise_utility_code) env.use_utility_code(restore_exception_utility_code) - gil_check = StatNode._gil_check + nogil_check = Node.gil_error gil_message = "Raising exception" def generate_execution_code(self, code): @@ -3477,7 +3474,7 @@ class ReraiseStatNode(StatNode): env.use_utility_code(raise_utility_code) env.use_utility_code(restore_exception_utility_code) - gil_check = StatNode._gil_check + nogil_check = Node.gil_error gil_message = "Raising exception" def generate_execution_code(self, code): @@ -3503,7 +3500,7 @@ class AssertStatNode(StatNode): self.value.analyse_types(env) self.value = self.value.coerce_to_pyobject(env) - gil_check = StatNode._gil_check + nogil_check = Node.gil_error gil_message = "Raising exception" def generate_execution_code(self, code): @@ -3820,6 +3817,13 @@ class ForFromStatNode(LoopNode, StatNode): py_loopvar_node = None from_range = False + gil_message = "For-loop using object bounds or target" + + def nogil_check(self, env): + for x in (self.target, self.bound1, self.bound2): + if x.type.is_pyobject: + self.gil_error() + def analyse_declarations(self, env): self.target.analyse_target_declaration(env) self.body.analyse_declarations(env) @@ -4038,7 +4042,7 @@ class TryExceptStatNode(StatNode): if self.else_clause: self.else_clause.analyse_expressions(env) - gil_check = StatNode._gil_check + nogil_check = Node.gil_error gil_message = "Try-except statement" def generate_execution_code(self, code): @@ -4325,7 +4329,7 @@ class TryFinallyStatNode(StatNode): self.body.analyse_expressions(env) self.finally_clause.analyse_expressions(env) - gil_check = StatNode._gil_check + nogil_check = Node.gil_error gil_message = "Try-finally statement" def generate_execution_code(self, code): @@ -4489,8 +4493,7 @@ class GILStatNode(TryFinallyStatNode): TryFinallyStatNode.analyse_expressions(self, env) env.nogil = was_nogil - def gil_check(self, env): - pass + nogil_check = None def generate_execution_code(self, code): code.mark_pos(self.pos) diff --git a/Cython/Compiler/ParseTreeTransforms.py b/Cython/Compiler/ParseTreeTransforms.py index 1a977149..015fb952 100644 --- a/Cython/Compiler/ParseTreeTransforms.py +++ b/Cython/Compiler/ParseTreeTransforms.py @@ -866,30 +866,32 @@ class GilCheck(VisitorTransform): """ def __call__(self, root): self.env_stack = [root.scope] + self.nogil = False return super(GilCheck, self).__call__(root) def visit_FuncDefNode(self, node): self.env_stack.append(node.local_scope) - if node.gil_check is not None: - node.gil_check(self.env_stack[-1]) + was_nogil = self.nogil + self.nogil = node.local_scope.nogil + if self.nogil and node.nogil_check: + node.nogil_check(node.local_scope) self.visitchildren(node) self.env_stack.pop() + self.nogil = was_nogil return node def visit_GILStatNode(self, node): - # FIXME: should we do some kind of GIL checking here, too? - # if node.gil_check is not None: - # node.gil_check(self.env_stack[-1]) env = self.env_stack[-1] - was_nogil = env.nogil - env.nogil = node.state == 'nogil' + if self.nogil and node.nogil_check: node.nogil_check() + was_nogil = self.nogil + self.nogil = (node.state == 'nogil') self.visitchildren(node) - env.nogil = was_nogil + self.nogil = was_nogil return node def visit_Node(self, node): - if self.env_stack and node.gil_check is not None: - node.gil_check(self.env_stack[-1]) + if self.env_stack and self.nogil and node.nogil_check: + node.nogil_check(self.env_stack[-1]) self.visitchildren(node) return node diff --git a/tests/errors/nogil.pyx b/tests/errors/nogil.pyx index 841e5487..27edc945 100644 --- a/tests/errors/nogil.pyx +++ b/tests/errors/nogil.pyx @@ -76,18 +76,26 @@ cdef class C: cdef void t(C c) nogil: pass +def ticket_338(): + cdef object obj + with nogil: + for obj from 0 <= obj < 4: + pass + +# For m(), the important thing is that there are errors on all lines in the range 23-69 +# except these: 34, 44, 56, 58, 60, 62-64 _ERRORS = u""" -1: 5: Function with Python return type cannot be declared nogil -6: 6: Assignment of Python object not allowed without gil -4: 5: Function declared nogil has Python locals or temporaries -11: 5: Function with Python return type cannot be declared nogil -15: 5: Calling gil-requiring function not allowed without gil -24: 9: Calling gil-requiring function not allowed without gil +1:5: Function with Python return type cannot be declared nogil +4:5: Function declared nogil has Python locals or temporaries +6:6: Assignment of Python object not allowed without gil +11:5: Function with Python return type cannot be declared nogil +15:5: Calling gil-requiring function not allowed without gil +24:9: Calling gil-requiring function not allowed without gil 26:12: Assignment of Python object not allowed without gil 28:16: Constructing complex number not allowed without gil 29:12: Accessing Python global or builtin not allowed without gil -30: 8: Backquote expression not allowed without gil +30:8: Backquote expression not allowed without gil 31:15: Assignment of Python object not allowed without gil 31:15: Python import not allowed without gil 32:13: Python import not allowed without gil @@ -101,31 +109,32 @@ _ERRORS = u""" 37:15: Converting to Python object not allowed without gil 37:17: Converting to Python object not allowed without gil 38:11: Accessing Python attribute not allowed without gil -39: 9: Constructing Python tuple not allowed without gil -40: 8: Constructing Python list not allowed without gil -41: 8: Constructing Python dict not allowed without gil +39:9: Constructing Python tuple not allowed without gil +40:8: Constructing Python list not allowed without gil +41:8: Constructing Python dict not allowed without gil 42:12: Truth-testing Python object not allowed without gil 43:13: Python type test not allowed without gil -#44: 4: Converting to Python object not allowed without gil 45:10: Operation not allowed without gil -46: 8: Operation not allowed without gil +46:8: Operation not allowed without gil 47:10: Assignment of Python object not allowed without gil 47:14: Assignment of Python object not allowed without gil -48: 9: Assignment of Python object not allowed without gil +48:9: Assignment of Python object not allowed without gil 48:13: Assignment of Python object not allowed without gil 48:16: Creating temporary Python reference not allowed without gil 48:19: Creating temporary Python reference not allowed without gil -49:11: Indexing Python object not allowed without gil 49:11: Assignment of Python object not allowed without gil +49:11: Indexing Python object not allowed without gil 50:11: Accessing Python attribute not allowed without gil 50:11: Assignment of Python object not allowed without gil -51: 8: Constructing Python tuple not allowed without gil -51: 8: Python print statement not allowed without gil -52: 8: Deleting Python object not allowed without gil -53: 8: Returning Python object not allowed without gil -54: 8: Raising exception not allowed without gil +51:8: Constructing Python tuple not allowed without gil +51:8: Python print statement not allowed without gil +52:8: Deleting Python object not allowed without gil +53:8: Returning Python object not allowed without gil +54:8: Raising exception not allowed without gil 55:14: Truth-testing Python object not allowed without gil 57:17: Truth-testing Python object not allowed without gil -61: 8: Try-except statement not allowed without gil -65: 8: Try-finally statement not allowed without gil +59:8: For-loop using object bounds or target not allowed without gil +61:8: Try-except statement not allowed without gil +65:8: Try-finally statement not allowed without gil +82:8: For-loop using object bounds or target not allowed without gil """ diff --git a/tests/run/nogil.pyx b/tests/run/nogil.pyx index 0798b53c..06f93473 100644 --- a/tests/run/nogil.pyx +++ b/tests/run/nogil.pyx @@ -20,3 +20,5 @@ cdef int g(int x) nogil: cdef int y y = x + 42 return y + + -- 2.26.2