Merge pull request #2627 from mattip/check_size2

 ENH: add check_size option to ctypedef class for external classes 

CHANGES.rst

@@ -85,6 +85,10 @@ Features added
 
 * Constants in ``libc.math`` are now declared as ``const`` to simplify their handling.
 
+* An additional ``check_size`` clause was added to the ``ctypedef class`` name
+  specification to allow suppressing warnings when importing modules with
+  backward-compatible `PyTypeObject`` size changes.
+
 Bugs fixed
 ----------
 

Cython/Compiler/ModuleNode.py

@@ -3059,10 +3059,24 @@ 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:
+            if type.check_size != 'min':
+                raise AttributeError("unexpected check_size value '%s' when "
+                "compiling %s.%s" % (type.check_size, module_name, type.name))
+            cs = 0
+        elif type.check_size == 'min':
+            cs = 1
+        elif type.check_size == True:
+            cs = 0
+        elif type.check_size == False:
+            cs = 2
+        else:
+            raise AttributeError("invalid value for check_size '%s' when compiling "
+                "%s.%s" % (type.check_size, module_name, type.name))
+        code.putln('%d);' % 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         'min' or boolean  What to do 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 = '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 = '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 = 'min'
     s.expect('[')
     while 1:
         if s.sy != 'IDENT':
@@ -3538,11 +3541,22 @@ 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_ident(s)
+            if check_size == 'False':
+                check_size = False
+            elif check_size == 'True':
+                check_size = True
+            elif check_size == 'min':
+                pass
+            else:
+                s.error('Expected False, True, or min, not %r' % check_size)
         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,15 @@ 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       'min' or boolean What to do if tp_basicsize does not match
     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='min'):
         self.name = name
         self.scope = None
         self.typedef_flag = typedef_flag
@@ -1368,6 +1369,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

@@ -1481,7 +1481,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='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
@@ -1514,7 +1515,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,15 +308,22 @@ set_path:
 
 /////////////// TypeImport.proto ///////////////
 
-static PyTypeObject *__Pyx_ImportType(PyObject* module, const char *module_name, const char *class_name, size_t size, int strict);  /*proto*/
+
+static PyTypeObject *__Pyx_ImportType(PyObject* module, const char *module_name, const char *class_name, size_t size, int 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, int check_size)
 {
+    /*
+     * check_size tells what to do if tp_basicsize is different from size:
+     * 0 - Error (originates in check_size=True)
+     * 1 - Error if tp_basicsize is smaller, warn if larger (originates in check_size='min')
+     * 2 - Error if tp_basicsize is smaller (originates in check_size=False)
+    */ 
     PyObject *result = 0;
     char warning[200];
     Py_ssize_t basicsize;
@@ -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 == 0 && (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 == 1 && (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;
+    }
+    /* check_size == 2 does not warn nor error */
     return (PyTypeObject *)result;
 bad:
     Py_XDECREF(result);

docs/src/userguide/extension_types.rst

@@ -736,6 +736,14 @@ built-in complex object.::
             ...
         } PyComplexObject;
 
+       At runtime, a check will be performed when importing the Cython
+       c-extension module that ``__builtin__.complex``'s ``tp_basicsize``
+       matches ``sizeof(`PyComplexObject)``. This check can fail if the Cython
+       c-extension module was compiled with one version of the
+       ``complexobject.h`` header but imported into a Python with a changed
+       header. This check can be tweaked by using ``check_size`` in the name
+       specification clause.
+
     2. As well as the name of the extension type, the module in which its type
        object can be found is also specified. See the implicit importing section
        below.
@@ -756,11 +764,23 @@ The part of the class declaration in square brackets is a special feature only
 available for extern or public extension types. The full form of this clause
 is::
 
-    [object object_struct_name, type type_object_name ]
+    [object object_struct_name, type type_object_name, check_size cs_option]
+
+Where:
+
+- ``object_struct_name`` is the name to assume for the type's C struct.
+- ``type_object_name`` is the name to assume for the type's statically
+  declared type object.
+- ``cs_option`` is ``min`` (the default), ``True``, or ``False`` and is only
+  used for external extension types. If ``True``, ``sizeof(object_struct)`` must
+  match the type's ``tp_basicsize``. If ``False``, or ``min``, the
+  ``object_struct`` may be smaller than the type's ``tp_basicsize``, which
+  indicates the type allocated at runtime may be part of an updated module, and
+  that the external module's developers extended the object in a
+  backward-compatible fashion (only adding new fields to the end of the
+  object). If ``min``, a warning will be emitted.
 
-where ``object_struct_name`` is the name to assume for the type's C struct,
-and type_object_name is the name to assume for the type's statically declared
-type object. (The object and type clauses can be written in either order.)
+The clauses can be written in any order.
 
 If the extension type declaration is inside a :keyword:`cdef` extern from
 block, the object clause is required, because Cython must be able to generate

tests/run/check_size.srctree

@@ -4,6 +4,7 @@ PYTHON -c "import runner"
 ######## setup.py ########
 
 from Cython.Build.Dependencies import cythonize
+from Cython.Compiler.Errors import CompileError
 from distutils.core import setup
 
 # force the build order
@@ -11,6 +12,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 CompileError as e:
+    pass
+
 ######## check_size_nominal.h ########
 
 #include <Python.h>
@@ -116,6 +123,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 Compilerror 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 +196,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')