From f4ec4bd5758890a66623e2a2fb3789c32e965297 Mon Sep 17 00:00:00 2001 From: Stefan Behnel Date: Tue, 26 Nov 2024 10:14:31 +0100 Subject: [PATCH] Refactor "with cython.nogil" handling into a separate method to keep the directives loop cleaner. Add tests to make sure multiple directives can be combined with gil/nogil in a single with-statement. --- Cython/Compiler/ParseTreeTransforms.py | 30 +++++++++++--------- tests/errors/pure_errors.py | 38 ++++++++++++++++++-------- tests/run/pure_py.py | 4 ++- 3 files changed, 47 insertions(+), 25 deletions(-) diff --git a/Cython/Compiler/ParseTreeTransforms.py b/Cython/Compiler/ParseTreeTransforms.py index bdf91c7a967..5e5f2a5d2e7 100644 --- a/Cython/Compiler/ParseTreeTransforms.py +++ b/Cython/Compiler/ParseTreeTransforms.py @@ -1414,24 +1414,28 @@ def visit_WithStatNode(self, node): name, value = directive if name in ('nogil', 'gil'): # special case: in pure mode, "with nogil" spells "with cython.nogil" - condition = None - if isinstance(node.manager, ExprNodes.SimpleCallNode) and len(node.manager.args) > 0: - if len(node.manager.args) == 1: - condition = node.manager.args[0] - else: - self.context.nonfatal_error( - PostParseError(node.pos, "Compiler directive %s accepts one positional argument." % name)) - elif isinstance(node.manager, ExprNodes.GeneralCallNode): - self.context.nonfatal_error( - PostParseError(node.pos, "Compiler directive %s accepts one positional argument." % name)) - node = Nodes.GILStatNode(node.pos, state=name, body=node.body, condition=condition) - return self.visit_Node(node) - if self.check_directive_scope(node.pos, name, 'with statement'): + return self._transform_with_gil(name, node) + elif self.check_directive_scope(node.pos, name, 'with statement'): directive_dict[name] = value if directive_dict: return self.visit_with_directives(node.body, directive_dict, contents_directives=None) return self.visit_Node(node) + def _transform_with_gil(self, state, node): + assert state in ('gil', 'nogil') + manager = node.manager + condition = None + if isinstance(manager, ExprNodes.SimpleCallNode) and manager.args: + if len(manager.args) > 1: + self.context.nonfatal_error( + PostParseError(node.pos, "Compiler directive %s accepts one positional argument." % state)) + condition = manager.args[0] + elif isinstance(manager, ExprNodes.GeneralCallNode): + self.context.nonfatal_error( + PostParseError(node.pos, "Compiler directive %s accepts one positional argument." % state)) + node = Nodes.GILStatNode(node.pos, state=state, body=node.body, condition=condition) + return self.visit_Node(node) + class ParallelRangeTransform(CythonTransform, SkipDeclarations): """ diff --git a/tests/errors/pure_errors.py b/tests/errors/pure_errors.py index e348abbaef2..aca17f1aeb0 100644 --- a/tests/errors/pure_errors.py +++ b/tests/errors/pure_errors.py @@ -1,5 +1,5 @@ # mode: error -# tag: warnings +# tag: warnings, werror import cython @@ -45,6 +45,21 @@ def test_cdef_nogil(x): cdef_nogil_false(x) # not ok +def test_nested_gil_nogil(x: cython.int): + arr: cython.int[5] = [1, 2, 3, 4, 5] + with cython.nogil: + with cython.nogil(False), cython.gil: + cdef_nogil(x) # ok + cdef_needs_gil(x) # ok + with cython.gil(True), cython.nogil: + cdef_nogil(x) # ok + cdef_needs_gil(x) # not ok + with cython.gil, cython.nogil, cython.wraparound(False): + cdef_nogil(x) # ok + cdef_needs_gil(x) # not ok + x += arr[-1] # not ok + + @cython.nogil def pyfunc(x): # invalid return x + 1 @@ -75,18 +90,19 @@ def add_one(x: cython.double) -> cython.double: _ERRORS = """ +30:0: Directive does not change previous value (nogil=False) 44:22: Calling gil-requiring function not allowed without gil 45:24: Calling gil-requiring function not allowed without gil -48:0: Python functions cannot be declared 'nogil' -53:0: Exception clause not allowed for function returning Python object -59:0: cfunc and ccall directives cannot be combined -65:0: cfunc and ccall directives cannot be combined -71:0: Cannot apply @cfunc to @ufunc, please reverse the decorators. -""" +56:26: Calling gil-requiring function not allowed without gil +59:26: Calling gil-requiring function not allowed without gil +60:22: the result of using negative indices inside of code sections marked as 'wraparound=False' is undefined +63:0: Python functions cannot be declared 'nogil' +68:0: Exception clause not allowed for function returning Python object +74:0: cfunc and ccall directives cannot be combined +80:0: cfunc and ccall directives cannot be combined +86:0: Cannot apply @cfunc to @ufunc, please reverse the decorators. -_WARNINGS = """ -30:0: Directive does not change previous value (nogil=False) # bugs: -59:0: 'test_contradicting_decorators1' redeclared -65:0: 'test_contradicting_decorators2' redeclared +74:0: 'test_contradicting_decorators1' redeclared +80:0: 'test_contradicting_decorators2' redeclared """ diff --git a/tests/run/pure_py.py b/tests/run/pure_py.py index 48b52af3c51..f2a143849b7 100644 --- a/tests/run/pure_py.py +++ b/tests/run/pure_py.py @@ -307,12 +307,14 @@ def cdef_nogil_false(x): def test_cdef_nogil(x): """ >>> test_cdef_nogil(5) - 18 + 24 """ with cython.nogil: result = cdef_nogil(x) with cython.nogil(True): result += cdef_nogil_true(x) + with cython.nogil(False), cython.nogil(True), cython.gil(False): + result += cdef_nogil(x) result += cdef_nogil_false(x) return result