Allow C/Fortran contiguity after the last indirect dimension + remove generic_contiguous

Cython/Compiler/Buffer.py

@@ -590,6 +590,8 @@ class GetAndReleaseBufferUtilityCode(object):
             for m in scope.cimported_modules:
                 find_buffer_types(m)
             for e in scope.type_entries:
+                if isinstance(e.utility_code_definition, CythonUtilityCode):
+                    continue
                 t = e.type
                 if t.is_extension_type:
                     if scope is cython_scope and not e.used:

Cython/Compiler/ExprNodes.py

@@ -2473,8 +2473,8 @@ class IndexNode(ExprNode):
                 if isinstance(index, SliceNode):
                     suboffsets_dim = i
                     self.memslice_slice = True
-                    if packing == 'contig' and index.step.is_none:
-                        axes.append((access, 'contig'))
+                    if index.step.is_none:
+                        axes.append((access, packing))
                     else:
                         axes.append((access, 'strided'))
 

Cython/Compiler/MemoryView.py

@@ -9,10 +9,9 @@ from PyrexTypes import py_object_type, cython_memoryview_ptr_type
 import Buffer
 import PyrexTypes
 
-START_ERR = "there must be nothing or the value 0 (zero) in the start slot."
-STOP_ERR = "Axis specification only allowed in the 'stop' slot."
-STEP_ERR = "Only the value 1 (one) or valid axis specification allowed in the step slot."
-ONE_ERR = "The value 1 (one) may appear in the first or last axis specification only."
+START_ERR = "Start must not be given."
+STOP_ERR = "Axis specification only allowed in the 'step' slot."
+STEP_ERR = "Step must be omitted, 1, or a valid specifier."
 BOTH_CF_ERR = "Cannot specify an array that is both C and Fortran contiguous."
 INVALID_ERR = "Invalid axis specification."
 EXPR_ERR = "no expressions allowed in axis spec, only names and literals."
@@ -122,8 +121,6 @@ def get_buf_flags(specs):
 
     access, packing = zip(*specs)
 
-    assert 'follow' not in packing
-
     if 'full' in access or 'ptr' in access:
         return memview_full_access
     else:
@@ -675,56 +672,27 @@ def get_axes_specs(env, axes):
     default_access, default_packing = 'direct', 'strided'
     cf_access, cf_packing = default_access, 'follow'
 
-    # set the is_{c,f}_contig flag.
-    for idx, axis in ((0,axes[0]), (-1,axes[-1])):
-        if isinstance(axis.step, IntNode):
-            if axis.step.compile_time_value(env) != 1:
-                raise CompileError(axis.step.pos, STEP_ERR)
-            if len(axes) > 1 and (is_c_contig or is_f_contig):
-                raise CompileError(axis.step.pos, BOTH_CF_ERR)
-            if not idx:
-                is_f_contig = True
-            else:
-                is_c_contig = True
-            if len(axes) == 1:
-                break
-
-    assert not (is_c_contig and is_f_contig)
-
     axes_specs = []
     # analyse all axes.
     for idx, axis in enumerate(axes):
-
-        # start slot can be either a literal '0' or None.
-        if isinstance(axis.start, IntNode):
-            if axis.start.compile_time_value(env):
-                raise CompileError(axis.start.pos,  START_ERR)
-        elif not isinstance(axis.start, NoneNode):
+        if not axis.start.is_none:
             raise CompileError(axis.start.pos,  START_ERR)
 
-        # stop slot must be None.
-        if not isinstance(axis.stop, NoneNode):
+        if not axis.stop.is_none:
             raise CompileError(axis.stop.pos, STOP_ERR)
 
-        # step slot can be None, the value 1,
-        # a single axis spec, or an IntBinopNode.
-        if isinstance(axis.step, NoneNode):
-            if is_c_contig or is_f_contig:
-                axes_specs.append((cf_access, cf_packing))
-            else:
+        if axis.step.is_none:
             axes_specs.append((default_access, default_packing))
 
         elif isinstance(axis.step, IntNode):
-            if idx not in (0, len(axes)-1):
-                raise CompileError(axis.step.pos, ONE_ERR)
             # the packing for the ::1 axis is contiguous,
             # all others are cf_packing.
-            axes_specs.append((cf_access, 'contig'))
+            if axis.step.compile_time_value(env) != 1:
+                raise CompileError(axis.step.pos, STEP_ERR)
 
-        elif isinstance(axis.step, (NameNode, AttributeNode)):
-            if is_c_contig or is_f_contig:
-                raise CompileError(axis.step.pos, CF_ERR)
+            axes_specs.append((cf_access, 'cfcontig'))
 
+        elif isinstance(axis.step, (NameNode, AttributeNode)):
             entry = _get_resolved_spec(env, axis.step)
             if entry.name in view_constant_to_access_packing:
                 axes_specs.append(view_constant_to_access_packing[entry.name])
@@ -734,7 +702,66 @@ def get_axes_specs(env, axes):
         else:
             raise CompileError(axis.step.pos, INVALID_ERR)
 
-    validate_axes_specs([axis.start.pos for axis in axes], axes_specs)
+    # First, find out if we have a ::1 somewhere
+    contig_dim = 0
+    is_contig = False
+    for idx, (access, packing) in enumerate(axes_specs):
+        if packing == 'cfcontig':
+            if is_contig:
+                raise CompileError(axis.step.pos, BOTH_CF_ERR)
+
+            contig_dim = idx
+            axes_specs[idx] = (access, 'contig')
+            is_contig = True
+
+    if is_contig:
+        # We have a ::1 somewhere, see if we're C or Fortran contiguous
+        if contig_dim == len(axes) - 1:
+            is_c_contig = True
+        else:
+            is_f_contig = True
+
+            if contig_dim and not axes_specs[contig_dim - 1][0] in ('full', 'ptr'):
+                raise CompileError(axes[contig_dim].pos,
+                                   "Fortran contiguous specifier must follow an indirect dimension")
+
+        if is_c_contig:
+            # Contiguous in the last dimension, find the last indirect dimension
+            contig_dim = -1
+            for idx, (access, packing) in enumerate(reversed(axes_specs)):
+                if access in ('ptr', 'full'):
+                    contig_dim = len(axes) - idx - 1
+
+        # Replace 'strided' with 'follow' for any dimension following the last
+        # indirect dimension, the first dimension or the dimension following
+        # the ::1.
+        #               int[::indirect, ::1, :, :]
+        #                                    ^  ^
+        #               int[::indirect, :, :, ::1]
+        #                               ^  ^
+        start = contig_dim + 1
+        stop = len(axes) - is_c_contig
+        for idx, (access, packing) in enumerate(axes_specs[start:stop]):
+            idx = contig_dim + 1 + idx
+            if access != 'direct':
+                raise CompileError(axes[idx].pos,
+                                   "Indirect dimension may not follow "
+                                   "Fortran contiguous dimension")
+            if packing == 'contig':
+                raise CompileError(axes[idx].pos,
+                                   "Dimension may not be contiguous")
+            axes_specs[idx] = (access, cf_packing)
+
+        if is_c_contig:
+            # For C contiguity, we need to fix the 'contig' dimension
+            # after the loop
+            a, p = axes_specs[-1]
+            axes_specs[-1] = a, 'contig'
+
+    validate_axes_specs([axis.start.pos for axis in axes],
+                        axes_specs,
+                        is_c_contig,
+                        is_f_contig)
 
     return axes_specs
 
@@ -786,16 +813,21 @@ view_constant_to_access_packing = {
     'indirect_contiguous':  ('ptr',    'contig'),
 }
 
-def validate_axes_specs(positions, specs):
+def validate_axes_specs(positions, specs, is_c_contig, is_f_contig):
 
     packing_specs = ('contig', 'strided', 'follow')
     access_specs = ('direct', 'ptr', 'full')
 
-    is_c_contig, is_f_contig = is_cf_contig(specs)
+    # is_c_contig, is_f_contig = is_cf_contig(specs)
 
     has_contig = has_follow = has_strided = has_generic_contig = False
 
-    for pos, (access, packing) in zip(positions, specs):
+    last_indirect_dimension = -1
+    for idx, (access, packing) in enumerate(specs):
+        if access == 'ptr':
+            last_indirect_dimension = idx
+
+    for idx, pos, (access, packing) in zip(xrange(len(specs)), positions, specs):
 
         if not (access in access_specs and
                 packing in packing_specs):
@@ -805,22 +837,28 @@ def validate_axes_specs(positions, specs):
             has_strided = True
         elif packing == 'contig':
             if has_contig:
-                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")
+                raise CompileError(pos, "Only one direct contiguous "
+                                        "axis may be specified.")
+
+            valid_contig_dims = last_indirect_dimension + 1, len(specs) - 1
+            if idx not in valid_contig_dims and access != 'ptr':
+                if last_indirect_dimension + 1 != len(specs) - 1:
+                    dims = "dimensions %d and %d" % valid_contig_dims
                 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"
+                    dims = "dimension %d" % valid_contig_dims[0]
+
+                raise CompileError(pos, "Only %s may be contiguous and direct" % dims)
+
             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.")
             if not (is_c_contig or is_f_contig):
                 raise CompileError(pos, "Invalid use of the follow specifier.")
 
+        if access in ('ptr', 'full'):
+            has_strided = False
+
 def _get_resolved_spec(env, spec):
     # spec must be a NameNode or an AttributeNode
     if isinstance(spec, NameNode):
@@ -859,7 +897,11 @@ def _resolve_AttributeNode(env, node):
                     node.pos, "undeclared name not builtin: %s" % modname)
         scope = mod.as_module
 
-    return scope.lookup(path[-1])
+    entry = scope.lookup(path[-1])
+    if not entry:
+        raise CompileError(node.pos, "No such attribute '%s'" % path[-1])
+
+    return entry
 
 def load_memview_cy_utility(util_code_name, context=None, **kwargs):
     return CythonUtilityCode.load(util_code_name, "MemoryView.pyx",

Cython/Utility/MemoryView.pyx

@@ -206,10 +206,27 @@ cdef class Enum(object):
     def __repr__(self):
         return self.name
 
+# Disable generic_contiguous, as it makes trouble verifying contiguity:
+#   - 'contiguous' or '::1' means the dimension is contiguous with dtype
+#   - 'indirect_contiguous' means a contiguous list of pointers
+#   - dtype contiguous must be contiguous in the first or last dimension
+#     from the start, or from the dimension following the last indirect dimension
+#
+#   e.g.
+#           int[::indirect_contiguous, ::contiguous, :]
+#
+#   is valid (list of pointers to 2d fortran-contiguous array), but
+#
+#           int[::generic_contiguous, ::contiguous, :]
+#
+#   would mean you'd have assert dimension 0 to be indirect (and pointer contiguous) at runtime.
+#   So it doesn't bring any performance benefit, and it's only confusing.
+
 cdef generic = Enum("<strided and direct or indirect>")
 cdef strided = Enum("<strided and direct>") # default
 cdef indirect = Enum("<strided and indirect>")
-cdef generic_contiguous = Enum("<contiguous and direct or indirect>")
+# Disable generic_contiguous, as it is a troublemaker
+#cdef generic_contiguous = Enum("<contiguous and direct or indirect>")
 cdef contiguous = Enum("<contiguous and direct>")
 cdef indirect_contiguous = Enum("<contiguous and indirect>")
 
@@ -677,10 +694,6 @@ cdef class _memoryviewslice(memoryview):
         def __get__(self):
             return self.from_object
 
-@cname('__pyx_memoryviewslice_check')
-cdef int __pyx_memoryviewslice_check(obj):
-    return isinstance(obj, _memoryviewslice)
-
 @cname('__pyx_memoryview_fromslice')
 cdef memoryview_fromslice({{memviewslice_name}} *memviewslice,
                           int ndim,

tests/compile/memview_declaration.pyx

@@ -4,7 +4,7 @@ cimport cython
 # from cython.view cimport contig as foo, full as bar #, follow
 from cython cimport view
 from cython.view cimport (generic, strided, indirect,
-                          generic_contiguous, contiguous, indirect_contiguous)
+                          contiguous, indirect_contiguous)
 
 cdef char[:] one_dim
 cdef char[:,:,:] three_dim
@@ -13,8 +13,7 @@ cdef unsigned int[:, ::1] view2
 cdef long long[::1, :, :, :] fort_contig
 cdef unsigned long[:, :, :, ::1] c_contig
 cdef unsigned short int[::1] c_and_fort
-cdef long long[0x0::0x1, 00:, -0   :,0 :] fort_contig0
-cdef unsigned long[0:, 0:, 0:, 0::0x0001] c_contig0
+cdef unsigned long[:, :, :, ::0x0001] c_contig0
 
 cdef int[::generic, ::generic] a1
 cdef int[::strided, ::generic] a2
@@ -26,9 +25,6 @@ cdef int[::generic, ::indirect] a7
 cdef int[::strided, ::indirect] a8
 cdef int[::indirect, ::indirect] a9
 
-cdef int[::generic, ::generic_contiguous] a10
-cdef int[::strided, ::generic_contiguous] a11
-cdef int[::indirect, ::generic_contiguous] a12
 cdef int[::generic, ::contiguous] a13
 cdef int[::strided, ::contiguous] a14
 cdef int[::indirect, ::contiguous] a15
@@ -39,7 +35,28 @@ cdef int[::indirect, ::indirect_contiguous] a18
 cdef int[::generic, ::] a19
 cdef int[::strided, :] a20
 cdef int[::indirect, :] a21
-cdef int[::generic_contiguous, :] a22
 cdef int[::contiguous, :] a23
 cdef int[::indirect_contiguous, :] a24
 
+cdef int[::indirect_contiguous, ::1] a25
+cdef int[::indirect_contiguous, ::1, :] a26
+cdef int[::indirect_contiguous, :, ::1] a27
+cdef int[::indirect_contiguous, ::1, :] a28
+cdef int[::indirect_contiguous, ::view.contiguous, :] a29
+cdef int[::indirect_contiguous, :, ::view.contiguous] a30
+
+cdef int[::indirect, ::1] a31
+cdef int[::indirect, ::1, :] a32 = object()
+cdef int[::indirect, :, ::1] a33 = object()
+cdef int[::indirect, ::1, :] a34
+cdef int[::indirect, ::view.contiguous, :] a35
+cdef int[::indirect, :, ::view.contiguous] a36
+
+cdef int[::1, :] my_f_contig = a32[0]
+cdef int[:, ::1] my_c_contig = a33[0]
+
+my_f_contig = a32[0, :, :]
+my_c_contig = a33[0, :, :]
+
+my_f_contig = a32[0, ...]
+my_c_contig = a33[0, ...]

tests/errors/memview_declarations.pyx

@@ -29,16 +29,16 @@ unconformable1 = dtype_unconformable
 
 # These are INVALID
 cdef int[::view.contiguous, ::1] a1
-cdef int[::view.generic_contiguous, ::1] a2
+#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.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
+#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
@@ -46,24 +46,19 @@ 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.
-13:19: Only the value 1 (one) or valid axis specification allowed in the step slot.
-14:20: Only the value 1 (one) or valid axis specification allowed in the step slot.
-15:20: Only the value 1 (one) or valid axis specification allowed in the step slot.
-16:17: there must be nothing or the value 0 (zero) in the start slot.
-17:18: there must be nothing or the value 0 (zero) in the start slot.
-18:22: Axis specification only allowed in the 'stop' slot.
-19:23: The value 1 (one) may appear in the first or last axis specification only.
+13:19: Step must be omitted, 1, or a valid specifier.
+14:20: Step must be omitted, 1, or a valid specifier.
+15:20: Step must be omitted, 1, or a valid specifier.
+16:17: Start must not be given.
+17:18: Start must not be given.
+18:22: Axis specification only allowed in the 'step' slot.
+19:19: Fortran contiguous specifier must follow an indirect dimension
 20:22: Invalid axis specification.
 21:25: Invalid axis specification.
 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
+31:9: Dimension may not be 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
+38:9:Only dimensions 3 and 2 may be contiguous and direct
 '''

tests/run/cythonscope.pyx

@@ -154,13 +154,12 @@ def viewobjs():
     <strided and direct or indirect>
     <strided and direct>
     <strided and indirect>
-    <contiguous and direct or indirect>
     <contiguous and direct>
     <contiguous and indirect>
     """
     print cython.view.generic
     print cython.view.strided
     print cython.view.indirect
-    print cython.view.generic_contiguous
+    #print cython.view.generic_contiguous
     print cython.view.contiguous
     print cython.view.indirect_contiguous

tests/run/memoryview.pyx

@@ -400,31 +400,31 @@ def generic(int[::view.generic, ::view.generic] mslice1,
     print buf1[2, 2]
     print buf2[2, 2]
 
-def generic_contig(int[::view.generic_contiguous, :] mslice1,
-                   int[::view.generic_contiguous, :] mslice2):
-    """
-    >>> 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
-    """
-    buf1, buf2 = mslice1, mslice2
-
-    print buf1[1, 1]
-    print buf2[1, 1]
-
-    buf1[2, -1] = 10
-    buf2[2, -1] = 11
-
-    print buf1[2, 2]
-    print buf2[2, 2]
+#def generic_contig(int[::view.generic_contiguous, :] mslice1,
+#                   int[::view.generic_contiguous, :] mslice2):
+#    """
+#    >>> 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
+#    """
+#    buf1, buf2 = mslice1, mslice2
+#
+#    print buf1[1, 1]
+#    print buf2[1, 1]
+#
+#    buf1[2, -1] = 10
+#    buf2[2, -1] = 11
+#
+#    print buf1[2, 2]
+#    print buf2[2, 2]
 
 ctypedef int td_cy_int
 cdef extern from "bufaccess.h":

tests/run/memslice.pyx

@@ -543,34 +543,36 @@ def generic(int[::view.generic, ::view.generic] buf1,
     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]
+# Note: disabled. generic_contiguous isn't very useful (you have to check suboffsets,
+#                                                       might as well multiply with strides)
+# @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(