add value range check for C level assignments to bytearray indices in order to mimic...

diff --git a/Cython/Compiler/ExprNodes.py b/Cython/Compiler/ExprNodes.py
index f51629e..5c02a8c 100644 (file)
--- a/Cython/Compiler/ExprNodes.py
+++ b/Cython/Compiler/ExprNodes.py
@@ -3453,7 +3453,8 @@ class IndexNode(ExprNode):
         elif self.type.is_pyobject:
             self.generate_setitem_code(rhs.py_result(), code)
         elif self.base.type is bytearray_type:
-            self.generate_setitem_code(rhs.result(), code)
+            value_code = self._check_byte_value(code, rhs)
+            self.generate_setitem_code(value_code, code)
         else:
             code.putln(
                 "%s = %s;" % (
@@ -3468,6 +3469,42 @@ class IndexNode(ExprNode):
         rhs.generate_disposal_code(code)
         rhs.free_temps(code)
 
+    def _check_byte_value(self, code, rhs):
+        # TODO: should we do this generally on downcasts, or just here?
+        assert rhs.type.is_int, repr(rhs.type)
+        value_code = rhs.result()
+        if rhs.has_constant_result():
+            if 0 <= rhs.constant_result < 256:
+                return value_code
+            needs_cast = True  # make at least the C compiler happy
+            warning(rhs.pos,
+                    "value outside of range(0, 256)"
+                    " when assigning to byte: %s" % rhs.constant_result,
+                    level=1)
+        else:
+            needs_cast = rhs.type != PyrexTypes.c_uchar_type
+
+        if not self.nogil:
+            conditions = []
+            if rhs.is_literal or rhs.type.signed:
+                conditions.append('%s < 0' % value_code)
+            if (rhs.is_literal or not
+                    (rhs.is_temp and rhs.type in (
+                        PyrexTypes.c_uchar_type, PyrexTypes.c_char_type,
+                        PyrexTypes.c_schar_type))):
+                conditions.append('%s > 255' % value_code)
+            if conditions:
+                code.putln("if (unlikely(%s)) {" % ' || '.join(conditions))
+                code.putln(
+                    'PyErr_SetString(PyExc_ValueError,'
+                    ' "byte must be in range(0, 256)"); %s' %
+                    code.error_goto(self.pos))
+                code.putln("}")
+
+        if needs_cast:
+            value_code = '((unsigned char)%s)' % value_code
+        return value_code
+
     def generate_deletion_code(self, code, ignore_nonexisting=False):
         self.generate_subexpr_evaluation_code(code)
         #if self.type.is_pyobject:

diff --git a/tests/run/bytearray_coercion.pyx b/tests/run/bytearray_coercion.pyx
index 203f1aa..141b3fe 100644 (file)
--- a/tests/run/bytearray_coercion.pyx
+++ b/tests/run/bytearray_coercion.pyx
@@ -112,3 +112,48 @@ def assign_to_index(bytearray b, value):
         assert False, "IndexError not raised"
 
     return b
+
+
+def check_bounds(int cvalue):
+    """
+    >>> check_bounds(0)
+    0
+    >>> check_bounds(255)
+    255
+    >>> check_bounds(256)
+    Traceback (most recent call last):
+    ValueError: byte must be in range(0, 256)
+    >>> check_bounds(-1)
+    Traceback (most recent call last):
+    ValueError: byte must be in range(0, 256)
+    """
+    b = bytearray(b'x')
+
+    try:
+        b[0] = 256
+    except ValueError:
+        pass
+    else:
+        assert False, "ValueError not raised"
+
+    try:
+        b[0] = -1
+    except ValueError:
+        pass
+    else:
+        assert False, "ValueError not raised"
+
+    b[0] = cvalue
+    return b[0]
+
+
+def nogil_assignment(bytearray x, int value):
+    """
+    >>> b = bytearray(b'abc')
+    >>> nogil_assignment(b, ord('y'))
+    >>> b
+    bytearray(b'xyc')
+    """
+    with nogil:
+        x[0] = 'x'
+        x[1] = value