BUG: buffer: allow alignment characters also in the middle of buffer format strings (#630)

Small fix was needed to make the buffer string parsing handle this case
correctly.

Cython/Compiler/Buffer.py

@@ -747,7 +747,8 @@ typedef struct {
   int new_count, enc_count;
   int is_complex;
   char enc_type;
-  char packmode;
+  char new_packmode;
+  char enc_packmode;
 } __Pyx_BufFmt_Context;
 
 static void __Pyx_BufFmt_Init(__Pyx_BufFmt_Context* ctx,
@@ -762,7 +763,8 @@ static void __Pyx_BufFmt_Init(__Pyx_BufFmt_Context* ctx,
   ctx->head->field = &ctx->root;
   ctx->fmt_offset = 0;
   ctx->head->parent_offset = 0;
-  ctx->packmode = '@';
+  ctx->new_packmode = '@';
+  ctx->enc_packmode = '@';
   ctx->new_count = 1;
   ctx->enc_count = 0;
   ctx->enc_type = 0;
@@ -936,12 +938,12 @@ static int __Pyx_BufFmt_ProcessTypeChunk(__Pyx_BufFmt_Context* ctx) {
     __Pyx_StructField* field = ctx->head->field;
     __Pyx_TypeInfo* type = field->type;
 
-    if (ctx->packmode == '@' || ctx->packmode == '^') {
+    if (ctx->enc_packmode == '@' || ctx->enc_packmode == '^') {
       size = __Pyx_BufFmt_TypeCharToNativeSize(ctx->enc_type, ctx->is_complex);
     } else {
       size = __Pyx_BufFmt_TypeCharToStandardSize(ctx->enc_type, ctx->is_complex);
     }
-    if (ctx->packmode == '@') {
+    if (ctx->enc_packmode == '@') {
       int align_at = __Pyx_BufFmt_TypeCharToAlignment(ctx->enc_type, ctx->is_complex);
       int align_mod_offset;
       if (align_at == 0) return -1;
@@ -1008,14 +1010,6 @@ static int __Pyx_BufFmt_ProcessTypeChunk(__Pyx_BufFmt_Context* ctx) {
   return 0;
 }
 
-static int __Pyx_BufFmt_FirstPack(__Pyx_BufFmt_Context* ctx) {
-  if (ctx->enc_type != 0 || ctx->packmode != '@') {
-    PyErr_SetString(PyExc_ValueError, "Buffer packing mode currently only allowed at beginning of format string (this is a defect)");
-    return -1;
-  }
-  return 0;
-}
-
 static const char* __Pyx_BufFmt_CheckString(__Pyx_BufFmt_Context* ctx, const char* ts) {
   int got_Z = 0;
   while (1) {
@@ -1041,8 +1035,7 @@ static const char* __Pyx_BufFmt_CheckString(__Pyx_BufFmt_Context* ctx, const cha
           PyErr_SetString(PyExc_ValueError, "Little-endian buffer not supported on big-endian compiler");
           return NULL;
         }
-        if (__Pyx_BufFmt_FirstPack(ctx) == -1) return NULL;
-        ctx->packmode = '=';
+        ctx->new_packmode = '=';
         ++ts;
         break;
       case '>':
@@ -1051,15 +1044,13 @@ static const char* __Pyx_BufFmt_CheckString(__Pyx_BufFmt_Context* ctx, const cha
           PyErr_SetString(PyExc_ValueError, "Big-endian buffer not supported on little-endian compiler");
           return NULL;
         }
-        if (__Pyx_BufFmt_FirstPack(ctx) == -1) return NULL;
-        ctx->packmode = '=';
+        ctx->new_packmode = '=';
         ++ts;
         break;
       case '=':
       case '@':
       case '^':
-        if (__Pyx_BufFmt_FirstPack(ctx) == -1) return NULL;
-        ctx->packmode = *ts++;
+        ctx->new_packmode = *ts++;
         break;
       case 'T': /* substruct */
         {
@@ -1090,6 +1081,7 @@ static const char* __Pyx_BufFmt_CheckString(__Pyx_BufFmt_Context* ctx, const cha
         ctx->new_count = 1;
         ctx->enc_count = 0;
         ctx->enc_type = 0;
+        ctx->enc_packmode = ctx->new_packmode;
         ++ts;
         break;
       case 'Z':
@@ -1103,13 +1095,15 @@ static const char* __Pyx_BufFmt_CheckString(__Pyx_BufFmt_Context* ctx, const cha
       case 'l': case 'L': case 'q': case 'Q':
       case 'f': case 'd': case 'g':
       case 'O':
-        if (ctx->enc_type == *ts && got_Z == ctx->is_complex) {
+        if (ctx->enc_type == *ts && got_Z == ctx->is_complex &&
+            ctx->enc_packmode == ctx->new_packmode) {
           /* Continue pooling same type */
           ctx->enc_count += ctx->new_count;
         } else {
           /* New type */
           if (__Pyx_BufFmt_ProcessTypeChunk(ctx) == -1) return NULL;
           ctx->enc_count = ctx->new_count;
+          ctx->enc_packmode = ctx->new_packmode;
           ctx->enc_type = *ts;
           ctx->is_complex = got_Z;
         }

tests/run/buffmt.pyx

@@ -155,7 +155,7 @@ def char3int(fmt):
     >>> char3int("c3i")
     >>> char3int("ci2i")
 
-    #TODO > char3int("c@i@2i")
+    >>> char3int("c@i@2i")
 
     Extra pad bytes (assuming int size is 4 or more)
     >>> char3int("cxiii")
@@ -169,7 +169,7 @@ def char3int(fmt):
         ...
     ValueError: Buffer dtype mismatch; next field is at offset 1 but 4 expected
 
-    #TODO char3int("=cxxx@iii")
+    >>> char3int("=cxxx@iii")
 
     Error:
     >>> char3int("cii")
@@ -277,11 +277,25 @@ cdef packed struct PackedSubStruct:
     char x
     int y
 
+cdef struct UnpackedSubStruct:
+    char x
+    int y
+
 cdef packed struct PackedStruct:
     char a
     int b
     PackedSubStruct sub
 
+cdef struct PartiallyPackedStruct:
+    char a
+    int b
+    PackedSubStruct sub
+
+cdef packed struct PartiallyPackedStruct2:
+    char a
+    UnpackedSubStruct sub
+    char b
+    int c
 
 @testcase
 def packed_struct(fmt):
@@ -291,12 +305,13 @@ def packed_struct(fmt):
     >>> packed_struct("^cici")
     >>> packed_struct("=cibi")
 
+    However aligned access won't work:
+
     >>> packed_struct("^c@i^ci")
     Traceback (most recent call last):
         ...
-    ValueError: Buffer packing mode currently only allowed at beginning of format string (this is a defect)
+    ValueError: Buffer dtype mismatch; next field is at offset 4 but 1 expected
 
-    However aligned access won't work:
     >>> packed_struct("@cici")
     Traceback (most recent call last):
         ...
@@ -305,6 +320,63 @@ def packed_struct(fmt):
     """
     cdef object[PackedStruct] buf = MockBuffer(fmt, sizeof(PackedStruct))
 
+@testcase
+def partially_packed_struct(fmt):
+    """
+    Assuming int is four bytes:
+
+    >>> partially_packed_struct("^c@i^ci")
+    >>> partially_packed_struct("@ci^ci")
+    >>> partially_packed_struct("^c@i=ci")
+    >>> partially_packed_struct("@ci=ci")
+    >>> partially_packed_struct("ci^ci")
+    >>> partially_packed_struct("ci=ci")
+
+    Incorrectly aligned accesses won't work:
+
+    >>> partially_packed_struct("^cici")
+    Traceback (most recent call last):
+        ...
+    ValueError: Buffer dtype mismatch; next field is at offset 1 but 4 expected
+
+    >>> partially_packed_struct("=cibi")
+    Traceback (most recent call last):
+        ...
+    ValueError: Buffer dtype mismatch; next field is at offset 1 but 4 expected
+
+    """
+    cdef object[PartiallyPackedStruct] buf = MockBuffer(
+        fmt, sizeof(PartiallyPackedStruct))
+
+@testcase
+def partially_packed_struct_2(fmt):
+    """
+    Assuming int is four bytes:
+
+    >>> partially_packed_struct_2("^ccxxxici")
+    >>> partially_packed_struct_2("^ccxxxi^ci")
+    >>> partially_packed_struct_2("c=cxxxi^ci")
+    >>> partially_packed_struct_2("c^cxxxi^ci")
+    >>> partially_packed_struct_2("c^cxxxi=ci")
+    >>> partially_packed_struct_2("ccxxx^i@c^i")
+
+    Incorrectly aligned accesses won't work:
+
+    >>> partially_packed_struct_2("ccxxxici")
+    Traceback (most recent call last):
+        ...
+    ValueError: Buffer dtype mismatch; next field is at offset 8 but 5 expected
+    
+    >>> partially_packed_struct_2("ccici")
+    Traceback (most recent call last):
+        ...
+    ValueError: Buffer dtype mismatch; next field is at offset 4 but 5 expected
+
+    """
+    cdef object[PartiallyPackedStruct2] buf = MockBuffer(
+        fmt, sizeof(PartiallyPackedStruct2))
+
+
 # TODO: empty struct
 # TODO: Incomplete structs
 # TODO: mixed structs

tests/run/numpy_test.pyx

@@ -193,6 +193,27 @@ try:
     ValueError: ...
 
 
+    The following expose bugs in Numpy (versions prior to 2011-04-02):
+    
+    >>> print(test_partially_packed_align(np.zeros((1,), dtype=np.dtype([('a', 'b'), ('b', 'i'), ('sub', np.dtype('b,i')), ('c', 'i')], align=True))))
+    array([(22, 23, (24, 25), 26)], 
+          dtype=[('a', '|i1'), ('', '|V3'), ('b', '!i4'), ('sub', [('f0', '|i1'), ('f1', '!i4')]), ('', '|V3'), ('c', '!i4')])
+
+    >>> print(test_partially_packed_align_2(np.zeros((1,), dtype=np.dtype([('a', 'b'), ('b', 'i'), ('c', 'b'), ('sub', np.dtype('b,i', align=True))]))))
+    array([(22, 23, 24, (27, 28))], 
+          dtype=[('a', '|i1'), ('b', '!i4'), ('c', '|i1'), ('sub', [('f0', '|i1'), ('', '|V3'), ('f1', '!i4')])])
+
+    >>> print(test_partially_packed_align(np.zeros((1,), dtype=np.dtype([('a', 'b'), ('b', 'i'), ('sub', np.dtype('b,i')), ('c', 'i')], align=False)))) #doctest: +ELLIPSIS
+    Traceback (most recent call last):
+        ...
+    ValueError: ...
+
+    >>> print(test_partially_packed_align_2(np.zeros((1,), dtype=np.dtype([('a', 'b'), ('b', 'i'), ('c', 'b'), ('sub', np.dtype('b,i', align=False))])))) #doctest: +ELLIPSIS
+    Traceback (most recent call last):
+        ...
+    ValueError: ...
+
+
     >>> test_good_cast()
     True
     >>> test_bad_cast()
@@ -402,6 +423,18 @@ cdef struct UnpackedStruct:
     char a
     int b
 
+cdef struct PartiallyPackedStruct:
+    char a
+    int b
+    PackedStruct sub
+    int c
+
+cdef packed struct PartiallyPackedStruct2:
+    char a
+    int b
+    char c
+    UnpackedStruct sub
+
 def test_packed_align(np.ndarray[PackedStruct] arr):
     arr[0].a = 22
     arr[0].b = 23
@@ -412,6 +445,22 @@ def test_unpacked_align(np.ndarray[UnpackedStruct] arr):
     arr[0].b = 23    
     return repr(arr).replace('<', '!').replace('>', '!')
 
+def test_partially_packed_align(np.ndarray[PartiallyPackedStruct] arr):
+    arr[0].a = 22
+    arr[0].b = 23
+    arr[0].sub.a = 24
+    arr[0].sub.b = 25
+    arr[0].c = 26
+    return repr(arr).replace('<', '!').replace('>', '!')
+
+def test_partially_packed_align_2(np.ndarray[PartiallyPackedStruct2] arr):
+    arr[0].a = 22
+    arr[0].b = 23
+    arr[0].c = 24
+    arr[0].sub.a = 27
+    arr[0].sub.b = 28
+    return repr(arr).replace('<', '!').replace('>', '!')
+
 def test_complextypes():
     cdef np.complex64_t x64 = 1, y64 = 1j
     cdef np.complex128_t x128 = 1, y128 = 1j