Repository: cython/cython
Pull request: Relax `nogil_check()` of `SliceIndexNode` to support array initialisation
Fixes #3160
Fixes #1662


Problem statement:
Cannot copy C arrays in nogil functions
This code:

```
cdef void foo() nogil:
    cdef double[16] x
    cdef double[16] y
    y[:] = x[:]
    y = x
```

throws the compile time error "Slicing Python object not allowed without gil" for both assignments. However, the generated code in both cases is a simple memcpy and doesn't require the GIL.

Initializing array with values requires GIL?
When trying to compile the following code with Cython, I get the error message below.

```cython
cdef void func() nogil: 
    cdef int[4] a = [0, 1, 2, 3]
```

```cython
/Users/jkirkham/miniconda/envs/cython/lib/python3.7/site-packages/Cython/Compiler/Main.py:369: FutureWarning: Cython directive 'language_level' not set, using 2 for now (Py2). This will change in a later release! File: /Users/jkirkham/test.pyx
  tree = Parsing.p_module(s, pxd, full_module_name)

Error compiling Cython file:
------------------------------------------------------------
...
cdef void func() nogil: 
    cdef int[4] a = [0, 1, 2, 3]
        ^
------------------------------------------------------------

test.pyx:2:9: Slicing Python object not allowed without gil
```

Dropping the `nogil` makes the error go away. However the C code generated looks something like this and as such doesn't appear to be using the GIL.

```c
static void __pyx_f_4test_func(void) {
  ...

  int __pyx_t_1[4];

  __pyx_t_1[0] = 0;
  __pyx_t_1[1] = 1;
  __pyx_t_1[2] = 2;
  __pyx_t_1[3] = 3;

  ...
}
```

Hints from issue discussion:
Also if I write the code like so, it appears I've run into issue ( https://github.com/cython/cython/issues/1662 ).

```cython
cdef void func() nogil: 
    cdef int[4] a
    a[:] = [0, 1, 2, 3]
```
Same issue I encounter right now.
Looks like the GIL check in `SliceIndexNode` is too broad and does not detect the "simple" cases that work without GIL.
PR welcome.
I had quick look at ExprNode.py but I wouldn't know where to start, Im not familiar at all with the Cython code base.
any fix to this? 
I suppose that the problem is that `SliceIndexNode` has disallowed GIL:

https://github.com/cython/cython/blob/d085015f5afcdfa246f8af61d0f77b9734ac69f6/Cython/Compiler/ExprNodes.py#L5420

`nogil_check()` method should be implemented for `SliceIndexNode` to decide when GIL is allowed and when not - e.g. as here:

https://github.com/cython/cython/blob/d085015f5afcdfa246f8af61d0f77b9734ac69f6/Cython/Compiler/ExprNodes.py#L731-L736

Commit message: Update tests/run/nogil.pyx

Co-authored-by: da-woods <dw-git@d-woods.co.uk>

Patch to review:
diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py
index 88185153541..df95b8615f7 100644
--- a/Cython/Compiler/ExprNodes.py
+++ b/Cython/Compiler/ExprNodes.py
@@ -5230,8 +5230,10 @@ class SliceIndexNode(ExprNode):
     #  start     ExprNode or None
     #  stop      ExprNode or None
     #  slice     ExprNode or None   constant slice object
+    #  nogil     bool               used internally
 
     subexprs = ['base', 'start', 'stop', 'slice']
+    nogil = False
 
     slice = None
 
@@ -5417,7 +5419,10 @@ def analyse_as_type(self, env):
                     base_type, MemoryView.get_axes_specs(env, [slice_node]))
         return None
 
-    nogil_check = Node.gil_error
+    def nogil_check(self, env):
+        self.nogil = env.nogil
+        return super(SliceIndexNode, self).nogil_check(env)
+
     gil_message = "Slicing Python object"
 
     get_slice_utility_code = TempitaUtilityCode.load(
@@ -5685,11 +5690,15 @@ def generate_slice_guard_code(self, code, target_size):
 
         if runtime_check:
             code.putln("if (unlikely((%s) != (%s))) {" % (runtime_check, target_size))
+            if self.nogil:
+                code.put_ensure_gil()
             code.putln(
                 'PyErr_Format(PyExc_ValueError, "Assignment to slice of wrong length,'
                 ' expected %%" CYTHON_FORMAT_SSIZE_T "d, got %%" CYTHON_FORMAT_SSIZE_T "d",'
                 ' (Py_ssize_t)(%s), (Py_ssize_t)(%s));' % (
                     target_size, runtime_check))
+            if self.nogil:
+                code.put_release_ensured_gil()
             code.putln(code.error_goto(self.pos))
             code.putln("}")
 
diff --git a/tests/errors/nogil.pyx b/tests/errors/nogil.pyx
index dfdebeebd86..1fa323b69cb 100644
--- a/tests/errors/nogil.pyx
+++ b/tests/errors/nogil.pyx
@@ -97,6 +97,15 @@ cdef int fstrings(int x, object obj) except -1 nogil:
     f"{x}"
     f"{obj}"
 
+cdef void slice_array() nogil:
+    with gil:
+        b = [1, 2, 3, 4]
+    cdef int[4] a = b[:]
+
+cdef int[:] main() nogil:
+    cdef int[4] a = [1,2,3,4]
+    return a
+
 
 _ERRORS = u"""
 4:5: Function with Python return type cannot be declared nogil
@@ -169,4 +178,8 @@ _ERRORS = u"""
 97:6: String formatting not allowed without gil
 98:4: Discarding owned Python object not allowed without gil
 98:6: String formatting not allowed without gil
+
+103:21: Coercion from Python not allowed without the GIL
+103:21: Slicing Python object not allowed without gil
+107:11: Operation not allowed without gil
 """
diff --git a/tests/run/nogil.pyx b/tests/run/nogil.pyx
index 3560211498a..c8036ed2104 100644
--- a/tests/run/nogil.pyx
+++ b/tests/run/nogil.pyx
@@ -104,3 +104,76 @@ def test_unraisable():
     finally:
         sys.stderr = old_stderr
     return stderr.getvalue().strip()
+
+
+cdef int initialize_array() nogil:
+    cdef int[4] a = [1, 2, 3, 4]
+    return a[0] + a[1] + a[2] + a[3]
+
+cdef int copy_array() nogil:
+    cdef int[4] a
+    a[:] = [0, 1, 2, 3]
+    return a[0] + a[1] + a[2] + a[3]
+
+cdef double copy_array2() nogil:
+    cdef double[4] x = [1.0, 3.0, 5.0, 7.0]
+    cdef double[4] y
+    y[:] = x[:]
+    return y[0] + y[1] + y[2] + y[3]
+
+cdef double copy_array3() nogil:
+    cdef double[4] x = [2.0, 4.0, 6.0, 8.0]
+    cdef double[4] y
+    y = x
+    return y[0] + y[1] + y[2] + y[3]
+
+cdef void copy_array_exception(int n) nogil:
+    cdef double[5] a = [1,2,3,4,5]
+    cdef double[6] b
+    b[:n] = a
+
+def test_initalize_array():
+    """
+    >>> test_initalize_array()
+    10
+    """
+    return initialize_array()
+
+def test_copy_array():
+    """
+    >>> test_copy_array()
+    6
+    """
+    return copy_array()
+
+def test_copy_array2():
+    """
+    >>> test_copy_array2()
+    16.0
+    """
+    return copy_array2()
+
+def test_copy_array3():
+    """
+    >>> test_copy_array3()
+    20.0
+    """
+    return copy_array3()
+
+def test_copy_array_exception(n):
+    """
+    >>> test_copy_array_exception(20)
+    Traceback (most recent call last):
+        ...
+    ValueError: Assignment to slice of wrong length, expected 5, got 20
+    """
+    copy_array_exception(n)
+    
+def test_copy_array_exception_nogil(n): 
+    >>> test_copy_array_exception_nogil(20)
+    Traceback (most recent call last):
+        ...
+    ValueError: Assignment to slice of wrong length, expected 5, got 20
+    """
+    with nogil:
+        copy_array_exception(n)


Code review comments:
diff_hunk: @@ -104,3 +104,76 @@ def test_unraisable():
     finally:
         sys.stderr = old_stderr
     return stderr.getvalue().strip()
+
+
+cdef int initialize_array() nogil:
+    cdef int[4] a = [1, 2, 3, 4]
+    return a[0] + a[1] + a[2] + a[3]
+
+cdef int copy_array() nogil:
+    cdef int[4] a
+    a[:] = [0, 1, 2, 3]
+    return a[0] + a[1] + a[2] + a[3]
+
+cdef double copy_array2() nogil:
+    cdef double[4] x = [1.0, 3.0, 5.0, 7.0]
+    cdef double[4] y
+    y[:] = x[:]
+    return y[0] + y[1] + y[2] + y[3]
+
+cdef double copy_array3() nogil:
+    cdef double[4] x = [2.0, 4.0, 6.0, 8.0]
+    cdef double[4] y
+    y = x
+    return y[0] + y[1] + y[2] + y[3]
+
+cdef void copy_array_exception(int n) nogil:
+    cdef double[5] a = [1,2,3,4,5]
+    cdef double[6] b
+    b[:n] = a
+
+def test_initalize_array():
+    """
+    >>> test_initalize_array()
+    10
+    """
+    return initialize_array()
+
+def test_copy_array():
+    """
+    >>> test_copy_array()
+    6
+    """
+    return copy_array()
+
+def test_copy_array2():
+    """
+    >>> test_copy_array2()
+    16.0
+    """
+    return copy_array2()
+
+def test_copy_array3():
+    """
+    >>> test_copy_array3()
+    20.0
+    """
+    return copy_array3()
+
+def test_copy_array_exception(n):
+    """
+    >>> test_copy_array_exception(20)
+    Traceback (most recent call last):
+        ...
+    ValueError: Assignment to slice of wrong length, expected 5, got 20
+    """
+    copy_array_exception(n)
+    
+def test_copy_array_exception_nogil(n): 
+    >>> test_copy_array_exception_nogil(20)
path: tests/run/nogil.pyx
text: @user1:
```suggestion
def test_copy_array_exception_nogil(n): 
    """
    >>> test_copy_array_exception_nogil(20)
```

My mistake!