merge

Cython/Compiler/ExprNodes.py

@@ -1745,7 +1745,6 @@ class IndexNode(ExprNode):
                 else:
                     self.index = self.index.coerce_to_pyobject(env)
                 self.type = py_object_type
-                self.gil_check(env)
                 self.is_temp = 1
             else:
                 if self.base.type.is_ptr or self.base.type.is_array:
@@ -1762,9 +1761,21 @@ class IndexNode(ExprNode):
                     error(self.pos,
                         "Invalid index type '%s'" %
                             self.index.type)
+        self.gil_check(env)
 
     gil_message = "Indexing Python object"
 
+    def gil_check(self, env):
+        if self.is_buffer_access and env.nogil:
+            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)
+
+
     def check_const_addr(self):
         self.base.check_const_addr()
         self.index.check_const()
@@ -1856,14 +1867,13 @@ class IndexNode(ExprNode):
             index_code = self.index.py_result()
             if self.base.type is dict_type:
                 function = "PyDict_SetItem"
-            elif self.base.type is list_type:
-                function = "PyList_SetItem"
-            # don't use PyTuple_SetItem(), as we'd normally get a
-            # TypeError when changing a tuple, while PyTuple_SetItem()
-            # would allow updates
-            #
-            #elif self.base.type is tuple_type:
-            #    function = "PyTuple_SetItem"
+            # It would seem that we could specalized lists/tuples, but that
+            # shouldn't happen here. 
+            # Both PyList_SetItem PyTuple_SetItem and a Py_ssize_t as input, 
+            # not a PyObject*, and bad conversion here would give the wrong 
+            # exception. Also, tuples are supposed to be immutable, and raise 
+            # TypeErrors when trying to set their entries (PyTuple_SetItem 
+            # is for creating new tuples from). 
             else:
                 function = "PyObject_SetItem"
         code.putln(

tests/bugs.txt

@@ -14,4 +14,3 @@ large_consts_T237
 bad_c_struct_T252
 missing_baseclass_in_predecl_T262
 ifelseexpr_T267
-cdef_setitem_T284

tests/errors/e_bufaccess2.pyx

 cimport e_bufaccess_pxd # was needed to provoke a bug involving ErrorType
+import cython
 
 def f():
     cdef object[e_bufaccess_pxd.T] buf
 
+def withnogil_access_fail():
+    cdef object[int] buf = None
+    with nogil:
+        buf[2] = 2
+
+@cython.boundscheck(False)
+def withnogil_access_ok():
+    cdef object[int] buf = None
+    with nogil:
+        buf[2] = 2 # No error should be triggered here
+
+@cython.boundscheck(False)
+def withnogil_access_fail_2():
+    cdef object[object] buf = None
+    with nogil:
+        buf[2] = 2 # Not OK as dtype is object
+
+def withnogil_acquire(x):
+    cdef object[int] buf
+    with nogil:
+        buf = x
+
 _ERRORS = u"""
 3:9: 'nothing' is not a type identifier
+10:11: Cannot check buffer index bounds without gil; use boundscheck(False) directive
+22:11: Cannot access buffer with object dtype without gil
+22:11: Assignment of Python object not allowed without gil
+27:12: Assignment of Python object not allowed without gil
 """

tests/run/bufaccess.pyx

@@ -1449,3 +1449,18 @@ def complex_struct_inplace(object[LongComplex] buf):
     buf[0].imag += 2
     print buf[0].real, buf[0].imag
     
+#
+# Nogil
+#
+@testcase
+@cython.boundscheck(False)
+def buffer_nogil():
+    """
+    >>> buffer_nogil()
+    10
+    """
+    cdef object[int] buf = IntMockBuffer(None, [1,2,3])
+    with nogil:
+        buf[1] = 10
+    return buf[1]
+    

tests/run/cdef_setitem_T284.pyx

 __doc__ = u'''
 >>> no_cdef()
 >>> with_cdef()
+>>> test_list(range(11), -2, None)
+[0, 1, 2, 3, 4, 5, 6, 7, 8, None, 10]
+>>> test_list(range(11), "invalid index", None)
+Traceback (most recent call last):
+...
+TypeError: list indices must be integers
 '''
 def no_cdef():
     lst = range(11)
@@ -15,3 +21,7 @@ def with_cdef():
     lst[ob] = -10
     cdef dict dd = {}
     dd[ob] = -10
+
+def test_list(list L, object i, object a):
+    L[i] = a
+    return L