ENH: add check_size option to ctypedef class for external classes

Cython/Compiler/ModuleNode.py

@@ -3059,10 +3059,21 @@ class ModuleNode(Nodes.Node, Nodes.BlockNode):
         else:
             code.put('sizeof(%s), ' % objstruct)
 
-        code.putln('%i); if (!%s) %s' % (
-            not type.is_external or type.is_subclassed,
-            type.typeptr_cname,
-            error_code))
+        # check_size
+        if not type.is_external or type.is_subclassed:
+            cs = '__PYX_CHECKSIZE_STRICT'
+        elif type.check_size == b'min':
+            cs = '__PYX_CHECKSIZE_MIN'
+        elif type.check_size is True:
+            cs = '__PYX_CHECKSIZE_STRICT'
+        elif type.check_size is False:
+            cs = '__PYX_CHECKSIZE_LOOSE'
+        else:
+            raise AttributeError("invalid value for check_size '%r' when compiling "
+                "%s.%s" % (type.check_size, module_name, type.name))
+        code.putln('%s);' % cs)
+
+        code.putln(' if (!%s) %s' % (type.typeptr_cname, error_code))
 
     def generate_type_ready_code(self, entry, code):
         Nodes.CClassDefNode.generate_type_ready_code(entry, code)

Cython/Compiler/Nodes.py

@@ -4629,6 +4629,7 @@ class CClassDefNode(ClassDefNode):
     #  bases              TupleNode         Base class(es)
     #  objstruct_name     string or None    Specified C name of object struct
     #  typeobj_name       string or None    Specified C name of type object
+    #  check_size         b'min' or boolean Issue warning if tp_basicsize does not match
     #  in_pxd             boolean           Is in a .pxd file
     #  decorators         [DecoratorNode]   list of decorators or None
     #  doc                string or None
@@ -4645,6 +4646,7 @@ class CClassDefNode(ClassDefNode):
     api = False
     objstruct_name = None
     typeobj_name = None
+    check_size = b'min'
     decorators = None
     shadow = False
 
@@ -4680,6 +4682,7 @@ class CClassDefNode(ClassDefNode):
             typeobj_cname=self.typeobj_name,
             visibility=self.visibility,
             typedef_flag=self.typedef_flag,
+            check_size = self.check_size,
             api=self.api,
             buffer_defaults=self.buffer_defaults(env),
             shadow=self.shadow)
@@ -4765,6 +4768,7 @@ class CClassDefNode(ClassDefNode):
             base_type=self.base_type,
             objstruct_cname=self.objstruct_name,
             typeobj_cname=self.typeobj_name,
+            check_size=self.check_size,
             visibility=self.visibility,
             typedef_flag=self.typedef_flag,
             api=self.api,

Cython/Compiler/Parsing.py

@@ -3471,6 +3471,7 @@ def p_c_class_definition(s, pos,  ctx):
     objstruct_name = None
     typeobj_name = None
     bases = None
+    check_size = b'min'
     if s.sy == '(':
         positional_args, keyword_args = p_call_parse_args(s, allow_genexp=False)
         if keyword_args:
@@ -3482,7 +3483,7 @@ def p_c_class_definition(s, pos,  ctx):
     if s.sy == '[':
         if ctx.visibility not in ('public', 'extern') and not ctx.api:
             error(s.position(), "Name options only allowed for 'public', 'api', or 'extern' C class")
-        objstruct_name, typeobj_name = p_c_class_options(s)
+        objstruct_name, typeobj_name, check_size = p_c_class_options(s)
     if s.sy == ':':
         if ctx.level == 'module_pxd':
             body_level = 'c_class_pxd'
@@ -3521,6 +3522,7 @@ def p_c_class_definition(s, pos,  ctx):
         bases = bases,
         objstruct_name = objstruct_name,
         typeobj_name = typeobj_name,
+        check_size = check_size,
         in_pxd = ctx.level == 'module_pxd',
         doc = doc,
         body = body)
@@ -3528,6 +3530,7 @@ def p_c_class_definition(s, pos,  ctx):
 def p_c_class_options(s):
     objstruct_name = None
     typeobj_name = None
+    check_size = b'min'
     s.expect('[')
     while 1:
         if s.sy != 'IDENT':
@@ -3538,11 +3541,14 @@ def p_c_class_options(s):
         elif s.systring == 'type':
             s.next()
             typeobj_name = p_ident(s)
+        elif s.systring == 'check_size':
+            s.next()
+            check_size = p_atom(s).value
         if s.sy != ',':
             break
         s.next()
     s.expect(']', "Expected 'object' or 'type'")
-    return objstruct_name, typeobj_name
+    return objstruct_name, typeobj_name, check_size
 
 
 def p_property_decl(s):

Cython/Compiler/PyrexTypes.py

@@ -1345,14 +1345,14 @@ class PyExtensionType(PyObjectType):
     #  vtable_cname     string           Name of C method table definition
     #  early_init       boolean          Whether to initialize early (as opposed to during module execution).
     #  defered_declarations [thunk]      Used to declare class hierarchies in order
-
+    #  check_size       b'min' or boolean should tp_basicsize match sizeof(obstruct_cname)
     is_extension_type = 1
     has_attributes = 1
     early_init = 1
 
     objtypedef_cname = None
 
-    def __init__(self, name, typedef_flag, base_type, is_external=0):
+    def __init__(self, name, typedef_flag, base_type, is_external=0, check_size=b'min'):
         self.name = name
         self.scope = None
         self.typedef_flag = typedef_flag
@@ -1368,6 +1368,7 @@ class PyExtensionType(PyObjectType):
         self.vtabptr_cname = None
         self.vtable_cname = None
         self.is_external = is_external
+        self.check_size = check_size
         self.defered_declarations = []
 
     def set_scope(self, scope):

Cython/Compiler/Symtab.py

@@ -1475,7 +1475,8 @@ class ModuleScope(Scope):
 
     def declare_c_class(self, name, pos, defining = 0, implementing = 0,
         module_name = None, base_type = None, objstruct_cname = None,
-        typeobj_cname = None, typeptr_cname = None, visibility = 'private', typedef_flag = 0, api = 0,
+        typeobj_cname = None, typeptr_cname = None, visibility = 'private',
+        typedef_flag = 0, api = 0, check_size=b'min',
         buffer_defaults = None, shadow = 0):
         # If this is a non-extern typedef class, expose the typedef, but use
         # the non-typedef struct internally to avoid needing forward
@@ -1508,7 +1509,8 @@ class ModuleScope(Scope):
         #  Make a new entry if needed
         #
         if not entry or shadow:
-            type = PyrexTypes.PyExtensionType(name, typedef_flag, base_type, visibility == 'extern')
+            type = PyrexTypes.PyExtensionType(name, typedef_flag, base_type,
+                                              visibility == 'extern', check_size=check_size)
             type.pos = pos
             type.buffer_defaults = buffer_defaults
             if objtypedef_cname is not None:

Cython/Utility/ImportExport.c

@@ -308,14 +308,21 @@ set_path:
 
 /////////////// TypeImport.proto ///////////////
 
-static PyTypeObject *__Pyx_ImportType(PyObject* module, const char *module_name, const char *class_name, size_t size, int strict);  /*proto*/
+typedef enum { /* What to do if tp_basicsize is different from size? */
+    __PYX_CHECKSIZE_STRICT, /* Error */
+    __PYX_CHECKSIZE_MIN,    /* Error if tp_basicsize is smaller, warn if larger */
+    __PYX_CHECKSIZE_LOOSE,  /* Error if tp_basicsize is smaller */
+} __pyx_CheckSizeState;
+
+
+static PyTypeObject *__Pyx_ImportType(PyObject* module, const char *module_name, const char *class_name, size_t size, __pyx_CheckSizeState check_size);  /*proto*/
 
 /////////////// TypeImport ///////////////
 
 #ifndef __PYX_HAVE_RT_ImportType
 #define __PYX_HAVE_RT_ImportType
 static PyTypeObject *__Pyx_ImportType(PyObject *module, const char *module_name, const char *class_name,
-    size_t size, int strict)
+    size_t size, __pyx_CheckSizeState check_size)
 {
     PyObject *result = 0;
     char warning[200];
@@ -345,18 +352,28 @@ static PyTypeObject *__Pyx_ImportType(PyObject *module, const char *module_name,
     if (basicsize == (Py_ssize_t)-1 && PyErr_Occurred())
         goto bad;
 #endif
-    if (!strict && (size_t)basicsize > size) {
-        PyOS_snprintf(warning, sizeof(warning),
-            "%s.%s size changed, may indicate binary incompatibility. Expected %zd, got %zd",
-            module_name, class_name, basicsize, size);
-        if (PyErr_WarnEx(NULL, warning, 0) < 0) goto bad;
+    if ((size_t)basicsize < size) {
+        PyErr_Format(PyExc_ValueError,
+            "%.200s.%.200s size changed, may indicate binary incompatibility. "
+            "Expected %zd from C header, got %zd from PyObject",
+            module_name, class_name, size, basicsize);
+        goto bad;
     }
-    else if ((size_t)basicsize != size) {
+    if (check_size == __PYX_CHECKSIZE_STRICT && (size_t)basicsize != size) {
         PyErr_Format(PyExc_ValueError,
-            "%.200s.%.200s has the wrong size, try recompiling. Expected %zd, got %zd",
-            module_name, class_name, basicsize, size);
+            "%.200s.%.200s size changed, may indicate binary incompatibility. "
+            "Expected %zd from C header, got %zd from PyObject",
+            module_name, class_name, size, basicsize);
         goto bad;
     }
+    else if (check_size == __PYX_CHECKSIZE_MIN && (size_t)basicsize > size) {
+        PyOS_snprintf(warning, sizeof(warning),
+            "%s.%s size changed, may indicate binary incompatibility. "
+            "Expected %zd from C header, got %zd from PyObject",
+            module_name, class_name, size, basicsize);
+        if (PyErr_WarnEx(NULL, warning, 0) < 0) goto bad;
+    }
+    /* __PYX_CHECKSIZE_LOOSE does not warn nor error */
     return (PyTypeObject *)result;
 bad:
     Py_XDECREF(result);

tests/run/check_size.srctree

@@ -11,6 +11,12 @@ setup(ext_modules= cythonize("check_size.pyx"))
 
 setup(ext_modules = cythonize("_check_size*.pyx"))
 
+try:
+    setup(ext_modules= cythonize("check_size6.pyx"))
+    assert False
+except AttributeError as e:
+    assert 'max' in str(e)
+
 ######## check_size_nominal.h ########
 
 #include <Python.h>
@@ -116,6 +122,58 @@ cdef extern from "check_size_smaller.h":
             int f9
 
 
+cpdef public int testme(Foo f) except -1:
+    return f.f9
+
+######## _check_size3.pyx ########
+
+cdef extern from "check_size_smaller.h":
+
+    # make sure missing check_size is equivalent to 'min'
+    ctypedef class check_size.Foo [object FooStructSmall, check_size 'min']:
+        cdef:
+            int f9
+
+
+cpdef public int testme(Foo f) except -1:
+    return f.f9
+
+######## _check_size4.pyx ########
+
+cdef extern from "check_size_smaller.h":
+
+    # Disable size check
+    ctypedef class check_size.Foo [object FooStructSmall, check_size False]:
+        cdef:
+            int f9
+
+
+cpdef public int testme(Foo f) except -1:
+    return f.f9
+
+######## _check_size5.pyx ########
+
+cdef extern from "check_size_smaller.h":
+
+    # Strict checking, will raise an error
+    ctypedef class check_size.Foo [object FooStructSmall, check_size True]:
+        cdef:
+            int f9
+
+
+cpdef public int testme(Foo f) except -1:
+    return f.f9
+
+######## check_size6.pyx ########
+
+cdef extern from "check_size_smaller.h":
+
+    # Raise AttributeError when using bad value
+    ctypedef class check_size.Foo [object FooStructSmall, check_size 'max']:
+        cdef:
+            int f9
+
+
 cpdef public int testme(Foo f) except -1:
     return f.f9
 
@@ -137,19 +195,36 @@ try:
     import _check_size1
     assert False
 except ValueError as e:
-    assert str(e).startswith('check_size.Foo has the wrong size, try recompiling')
+    assert str(e).startswith('check_size.Foo size changed')
 
 # Warining since check_size.Foo's tp_basicsize is larger than what is needed
 # for FooStructSmall. There is "spare", accessing FooStructSmall's fields will
 # never access invalid memory. This can happen, for instance, when using old
-# headers with a newer runtime, or when using an old _check_size2 with a newer
+# headers with a newer runtime, or when using an old _check_size{2,3} with a newer
 # check_size, where the developers of check_size are careful to be backward
 # compatible.
 
 with warnings.catch_warnings(record=True) as w:
+    warnings.simplefilter("always")
     import _check_size2
-    assert len(w) == 1, 'expected one warning, got %d' % len(w)
-    assert str(w[-1].message).startswith('check_size.Foo size changed')
+    import _check_size3
+    assert len(w) == 2, 'expected two warnings, got %d' % len(w)
+    assert str(w[0].message).startswith('check_size.Foo size changed')
+    assert str(w[1].message).startswith('check_size.Foo size changed')
 
 ret = _check_size2.testme(foo)
 assert ret == 23
+ret = _check_size3.testme(foo)
+assert ret == 23
+
+with warnings.catch_warnings(record=True) as w:
+    # No warning, runtime vendor must provide backward compatibility
+    import _check_size4
+    assert len(w) == 0
+
+try:
+    # Enforce strict checking
+    import _check_size5
+    assert False
+except ValueError as e:
+    assert str(e).startswith('check_size.Foo size changed')