Allow contig to follow indirect contig

Cython/Compiler/MemoryView.py

@@ -197,24 +197,22 @@ class MemoryViewSliceBufferEntry(Buffer.BufferEntry):
 
             flag = get_memoryview_flag(access, packing)
 
-            if flag == "generic":
+            if flag in ("generic", "generic_contiguous"):
+                # Note: we cannot do cast tricks to avoid stride multiplication
+                #       for generic_contiguous, as we may have to do (dtype *)
+                #       or (dtype **) arithmetic, we won't know which unless
+                #       we check suboffsets
                 code.globalstate.use_utility_code(memviewslice_index_helpers)
                 bufp = ('__pyx_memviewslice_index_full(%s, %s, %s, %s)' %
                                             (bufp, index, stride, suboffset))
 
-            elif flag == "generic_contiguous":
-                # We can skip stride multiplication with the cast
-                code.globalstate.use_utility_code(memviewslice_index_helpers)
-                bufp = '((char *) ((%s *) %s) + %s)' % (type_decl, bufp, index)
-                bufp = ('__pyx_memviewslice_index_full_contig(%s, %s)' %
-                                                            (bufp, suboffset))
-
             elif flag == "indirect":
-                bufp = ("(*((char **) %s + %s * %s) + %s)" %
-                                             (bufp, index, stride, suboffset))
+                bufp = "(%s + %s * %s)" % (bufp, index, stride)
+                bufp = ("(*((char **) %s) + %s)" % (bufp, suboffset))
 
             elif flag == "indirect_contiguous":
-                bufp = "(*((char **) %s) + %s)" % (bufp, suboffset)
+                # Note: we do char ** arithmetic
+                bufp = "(*((char **) %s + %s) + %s)" % (bufp, index, suboffset)
 
             elif flag == "strided":
                 bufp = "(%s + %s * %s)" % (bufp, index, stride)
@@ -621,7 +619,7 @@ def get_axes_specs(env, axes):
         else:
             raise CompileError(axis.step.pos, INVALID_ERR)
 
-    validate_axes_specs(axes[0].start.pos, axes_specs)
+    validate_axes_specs([axis.start.pos for axis in axes], axes_specs)
 
     return axes_specs
 
@@ -677,16 +675,16 @@ def get_access_packing(view_scope_constant):
     if view_scope_constant.name == 'generic':
         return 'full',
 
-def validate_axes_specs(pos, specs):
+def validate_axes_specs(positions, specs):
 
     packing_specs = ('contig', 'strided', 'follow')
     access_specs = ('direct', 'ptr', 'full')
 
     is_c_contig, is_f_contig = is_cf_contig(specs)
     
-    has_contig = has_follow = has_strided = False
+    has_contig = has_follow = has_strided = has_generic_contig = False
 
-    for access, packing in specs:
+    for pos, (access, packing) in zip(positions, specs):
 
         if not (access in access_specs and
                 packing in packing_specs):
@@ -696,8 +694,16 @@ def validate_axes_specs(pos, specs):
             has_strided = True
         elif packing == 'contig':
             if has_contig:
-                raise CompileError(pos, "Only one contiguous axis may be specified.")
-            has_contig = True
+                if access == 'ptr':
+                    raise CompileError(pos, "Indirect contiguous dimensions must precede direct contiguous")
+                elif has_generic_contig or access == 'full':
+                    raise CompileError(pos, "Generic contiguous cannot be combined with direct contiguous")
+                else:
+                    raise CompileError(pos, "Only one direct contiguous axis may be specified.")
+            # Note: We do NOT allow access == 'full' to act as
+            #       "additionally potentially contiguous"
+            has_contig = access != 'ptr'
+            has_generic_contig = has_generic_contig or access == 'full'
         elif packing == 'follow':
             if has_strided:
                 raise CompileError(pos, "A memoryview cannot have both follow and strided axis specifiers.")

Cython/Utility/MemoryView_C.c

@@ -42,6 +42,11 @@ static int __Pyx_init_memviewslice(
                 int ndim,
                 __Pyx_memviewslice *memviewslice);
 
+#if CYTHON_REFNANNY
+    /* disable inlining when running tests */
+    #define CYTHON_INLINE
+#endif
+
 #define __PYX_INC_MEMVIEW(slice, have_gil) __Pyx_INC_MEMVIEW(slice, have_gil, __LINE__)
 #define __PYX_XDEC_MEMVIEW(slice, have_gil) __Pyx_XDEC_MEMVIEW(slice, have_gil, __LINE__)
 static CYTHON_INLINE void __Pyx_INC_MEMVIEW({{memviewslice_name}} *, int, int);
@@ -141,7 +146,13 @@ static int __Pyx_ValidateAndInit_memviewslice(
         spec = axes_specs[i];
 
         if (spec & __Pyx_MEMVIEW_CONTIG) {
-            if (buf->strides[i] != buf->itemsize) {
+            if (spec & (__Pyx_MEMVIEW_PTR|__Pyx_MEMVIEW_FULL)) {
+                if (buf->strides[i] != sizeof(void *)) {
+                    PyErr_Format(PyExc_ValueError,
+                        "Buffer is not indirectly contiguous in dimension %d.", i);
+                    goto fail;
+                }
+            } else if (buf->strides[i] != buf->itemsize) {
                 PyErr_SetString(PyExc_ValueError,
                     "Buffer and memoryview are not contiguous in the same dimension.");
                 goto fail;
@@ -156,26 +167,24 @@ static int __Pyx_ValidateAndInit_memviewslice(
             }
         }
 
+        /* Todo: without PyBUF_INDIRECT we may not have suboffset information, i.e., the
+           ptr may not be set to NULL but may be uninitialized? */
         if (spec & __Pyx_MEMVIEW_DIRECT) {
             if (buf->suboffsets && buf->suboffsets[i] >= 0) {
-                PyErr_SetString(PyExc_ValueError,
-                    "Buffer not compatible with direct access.");
+                PyErr_Format(PyExc_ValueError,
+                    "Buffer not compatible with direct access in dimension %d.", i);
                 goto fail;
             }
         }
 
-        if (spec & __Pyx_MEMVIEW_PTR) {
-            if (!buf->suboffsets) {
-                PyErr_SetString(PyExc_ValueError,
-                    "Buffer not able to be indirectly accessed.");
-                goto fail;
-            }
+        if (spec & (__Pyx_MEMVIEW_PTR|__Pyx_MEMVIEW_FULL) && !buf->suboffsets) {
+            memviewslice->suboffsets[i] = -1;
         }
 
         if (spec & __Pyx_MEMVIEW_PTR) {
-            if (buf->suboffsets[i] < 0) {
+            if (buf->suboffsets && buf->suboffsets[i] < 0) {
                 PyErr_Format(PyExc_ValueError,
-                    "Buffer not indirectly accessed in %d dimension, although memoryview is.", i);
+                    "Buffer is not indirectly accessisble in dimension %d.", i);
                 goto fail;
             }
         }

tests/errors/memview_declarations.pyx

@@ -27,6 +27,22 @@ cdef object[:, ::1] unconformable2 = unconformable1
 cdef int[::1, :] dtype_unconformable = object()
 unconformable1 = dtype_unconformable
 
+# These are INVALID
+cdef int[::view.contiguous, ::1] a1
+cdef int[::view.generic_contiguous, ::1] a2
+
+cdef int[::view.contiguous, ::view.generic_contiguous] a3
+cdef int[::view.generic_contiguous, ::view.generic_contiguous] a4
+
+cdef int[::view.contiguous, ::view.contiguous] a5
+cdef int[:, ::view.contiguous, ::view.indirect_contiguous] a6
+
+cdef int[::view.generic_contiguous, ::view.contiguous] a7
+cdef int[::view.contiguous, ::view.generic_contiguous] a8
+
+# These are VALID
+cdef int[::view.indirect_contiguous, ::view.contiguous] a9
+
 _ERRORS = u'''
 11:25: Cannot specify an array that is both C and Fortran contiguous.
 12:31: Cannot specify an array that is both C and Fortran contiguous.
@@ -42,4 +58,12 @@ _ERRORS = u'''
 22:22: no expressions allowed in axis spec, only names and literals.
 25:51: Memoryview 'object[::contiguous, :]' not conformable to memoryview 'object[:, ::contiguous]'.
 28:36: Different base types for memoryviews (int, Python object)
+31:15: Invalid axis specification for a C/Fortran contiguous array.
+32:15: Invalid axis specification for a C/Fortran contiguous array.
+34:9: Generic contiguous cannot be combined with direct contiguous
+35:9: Generic contiguous cannot be combined with direct contiguous
+37:9: Only one direct contiguous axis may be specified.
+38:9: Indirect contiguous dimensions must precede direct contiguous
+40:9: Generic contiguous cannot be combined with direct contiguous
+41:9: Generic contiguous cannot be combined with direct contiguous
 '''

tests/run/memslice.pyx

@@ -431,7 +431,7 @@ def wraparound_directive(int[:] buf, int pos_idx, int neg_idx):
 
 
 #
-# Test which flags are passed.
+# Test all kinds of indexing and flags
 #
 
 @testcase
@@ -512,6 +512,127 @@ def f_contig_2d(int[::1, :] buf):
     """
     return buf[3, 1]
 
+@testcase
+def generic(int[::view.generic, ::view.generic] buf1,
+            int[::view.generic, ::view.generic] buf2):
+    """
+    >>> A = IntMockBuffer("A", [[0,1,2], [3,4,5], [6,7,8]])
+    >>> B = IntMockBuffer("B", [[0,1,2], [3,4,5], [6,7,8]], shape=(3, 3), strides=(1, 3))
+    >>> generic(A, B)
+    acquired A
+    acquired B
+    4
+    4
+    10
+    11
+    released A
+    released B
+    >>> [str(x) for x in A.recieved_flags]
+    ['FORMAT', 'INDIRECT', 'ND', 'STRIDES', 'WRITABLE']
+    >>> [str(x) for x in B.recieved_flags]
+    ['FORMAT', 'INDIRECT', 'ND', 'STRIDES', 'WRITABLE']
+    """
+    print buf1[1, 1]
+    print buf2[1, 1]
+
+    buf1[2, -1] = 10
+    buf2[2, -1] = 11
+
+    print buf1[2, 2]
+    print buf2[2, 2]
+
+@testcase
+def generic_contig(int[::view.generic_contiguous, :] buf1,
+                   int[::view.generic_contiguous, :] buf2):
+    """
+    >>> A = IntMockBuffer("A", [[0,1,2], [3,4,5], [6,7,8]])
+    >>> B = IntMockBuffer("B", [[0,1,2], [3,4,5], [6,7,8]], shape=(3, 3), strides=(1, 3))
+    >>> generic_contig(A, B)
+    acquired A
+    acquired B
+    4
+    4
+    10
+    11
+    released A
+    released B
+    >>> [str(x) for x in A.recieved_flags]
+    ['FORMAT', 'INDIRECT', 'ND', 'STRIDES', 'WRITABLE']
+    >>> [str(x) for x in B.recieved_flags]
+    ['FORMAT', 'INDIRECT', 'ND', 'STRIDES', 'WRITABLE']
+    """
+    print buf1[1, 1]
+    print buf2[1, 1]
+
+    buf1[2, -1] = 10
+    buf2[2, -1] = 11
+
+    print buf1[2, 2]
+    print buf2[2, 2]
+
+@testcase
+def indirect_strided_and_contig(
+             int[::view.indirect, ::view.strided] buf1,
+             int[::view.indirect, ::view.contiguous] buf2):
+    """
+    >>> A = IntMockBuffer("A", [[0,1,2], [3,4,5], [6,7,8]])
+    >>> B = IntMockBuffer("B", [[0,1,2], [3,4,5], [6,7,8]], shape=(3, 3), strides=(1, 3))
+    >>> indirect_strided_and_contig(A, B)
+    acquired A
+    acquired B
+    4
+    4
+    10
+    11
+    released A
+    released B
+    >>> [str(x) for x in A.recieved_flags]
+    ['FORMAT', 'INDIRECT', 'ND', 'STRIDES', 'WRITABLE']
+    >>> [str(x) for x in B.recieved_flags]
+    ['FORMAT', 'INDIRECT', 'ND', 'STRIDES', 'WRITABLE']
+    """
+    print buf1[1, 1]
+    print buf2[1, 1]
+
+    buf1[2, -1] = 10
+    buf2[2, -1] = 11
+
+    print buf1[2, 2]
+    print buf2[2, 2]
+
+
+@testcase
+def indirect_contig(
+             int[::view.indirect_contiguous, ::view.contiguous] buf1,
+             int[::view.indirect_contiguous, ::view.generic] buf2):
+    """
+    >>> A = IntMockBuffer("A", [[0,1,2], [3,4,5], [6,7,8]])
+    >>> B = IntMockBuffer("B", [[0,1,2], [3,4,5], [6,7,8]], shape=(3, 3), strides=(1, 3))
+    >>> indirect_contig(A, B)
+    acquired A
+    acquired B
+    4
+    4
+    10
+    11
+    released A
+    released B
+    >>> [str(x) for x in A.recieved_flags]
+    ['FORMAT', 'INDIRECT', 'ND', 'STRIDES', 'WRITABLE']
+    >>> [str(x) for x in B.recieved_flags]
+    ['FORMAT', 'INDIRECT', 'ND', 'STRIDES', 'WRITABLE']
+    """
+    print buf1[1, 1]
+    print buf2[1, 1]
+
+    buf1[2, -1] = 10
+    buf2[2, -1] = 11
+
+    print buf1[2, 2]
+    print buf2[2, 2]
+
+
+
 #
 # Test compiler options for bounds checking. We create an array with a
 # safe "boundary" (memory

tests/run/mockbuffers.pxi

@@ -99,18 +99,22 @@ cdef class MockBuffer:
             it += self.itemsize
         return buf
 
-    cdef void* create_indirect_buffer(self, data, shape):
+    cdef void* create_indirect_buffer(self, data, shape) except NULL:
         cdef size_t n = 0
         cdef void** buf
-        assert shape[0] == len(data)
+        assert shape[0] == len(data), (shape[0], len(data))
         if len(shape) == 1:
             return self.create_buffer(data)
         else:
             shape = shape[1:]
             n = <size_t>len(data) * sizeof(void*)
             buf = <void**>stdlib.malloc(n)
+            if buf == NULL:
+                return NULL
+
             for idx, subdata in enumerate(data):
                 buf[idx] = self.create_indirect_buffer(subdata, shape)
+
             return buf
 
     cdef Py_ssize_t* list_to_sizebuf(self, l):