Add runtime checks for multiple inheritance

Cython/Compiler/Nodes.py

@@ -4736,14 +4736,11 @@ class CClassDefNode(ClassDefNode):
         if len(self.bases.args) > 1:
             if not has_body or self.in_pxd:
                 error(self.bases.args[1].pos, "Only declare first base in declaration.")
+            # At runtime, we check that the other bases are heap types
+            # and that a __dict__ is added if required.
             for other_base in self.bases.args[1:]:
                 if other_base.analyse_as_type(env):
-                    # TODO(robertwb): We may also want to enforce some checks
-                    # at runtime.
                     error(other_base.pos, "Only one extension type base class allowed.")
-            if not self.scope.lookup("__dict__"):
-                #TODO(robertwb): See if this can be safely removed.
-                error(self.pos, "Extension types with multiple bases must have a __dict__ attribute")
             self.entry.type.early_init = 0
             from . import ExprNodes
             self.type_init_args = ExprNodes.TupleNode(
@@ -4824,24 +4821,16 @@ class CClassDefNode(ClassDefNode):
             for slot in TypeSlots.slot_table:
                 slot.generate_dynamic_init_code(scope, code)
             if heap_type_bases:
-
-                # As of https://bugs.python.org/issue22079
-                # PyType_Ready enforces that all bases of a non-heap type
-                # are non-heap.  We know this is the case for the solid base,
-                # but other bases may be heap allocated and are kept alive
-                # though the bases reference.
-                # Other than this check, this flag is unused in this method.
-                code.putln("#if PY_VERSION_HEX >= 0x03050000")
-                code.putln("%s.tp_flags |= Py_TPFLAGS_HEAPTYPE;" % typeobj_cname)
-                code.putln("#endif")
+                code.globalstate.use_utility_code(
+                    UtilityCode.load_cached('PyType_Ready', 'ExtensionTypes.c'))
+                readyfunc = "__Pyx_PyType_Ready"
+            else:
+                readyfunc = "PyType_Ready"
             code.putln(
-                "if (PyType_Ready(&%s) < 0) %s" % (
+                "if (%s(&%s) < 0) %s" % (
+                    readyfunc,
                     typeobj_cname,
                     code.error_goto(entry.pos)))
-            if heap_type_bases:
-                code.putln("#if PY_VERSION_HEX >= 0x03050000")
-                code.putln("%s.tp_flags &= ~Py_TPFLAGS_HEAPTYPE;" % typeobj_cname)
-                code.putln("#endif")
             # Don't inherit tp_print from builtin types, restoring the
             # behavior of using tp_repr or tp_str instead.
             code.putln("%s.tp_print = 0;" % typeobj_cname)

Cython/Utility/ExtensionTypes.c

+/////////////// PyType_Ready.proto ///////////////
+
+static int __Pyx_PyType_Ready(PyTypeObject *t);
+
+/////////////// PyType_Ready ///////////////
+
+// Wrapper around PyType_Ready() with some runtime checks and fixes
+// to deal with multiple inheritance.
+static int __Pyx_PyType_Ready(PyTypeObject *t) {
+    // Loop over all bases (except the first) and check that those
+    // really are heap types. Otherwise, it would not be safe to
+    // subclass them.
+    //
+    // We also check tp_dictoffset: it is unsafe to inherit
+    // tp_dictoffset from a base class because the object structures
+    // would not be compatible. So, if our extension type doesn't set
+    // tp_dictoffset (i.e. there is no __dict__ attribute in the object
+    // structure), we need to check that none of the base classes sets
+    // it either.
+    PyObject *bases = t->tp_bases;
+    if (bases)
+    {
+        Py_ssize_t i, n = PyTuple_GET_SIZE(bases);
+        for (i = 1; i < n; i++)  /* Skip first base */
+        {
+            PyTypeObject *b = (PyTypeObject*)PyTuple_GET_ITEM(bases, i);
+            if (!PyType_HasFeature(b, Py_TPFLAGS_HEAPTYPE))
+            {
+                PyErr_Format(PyExc_TypeError, "base class '%.200s' is not a heap type",
+                             b->tp_name);
+                return -1;
+            }
+            if (t->tp_dictoffset == 0 && b->tp_dictoffset)
+            {
+                PyErr_Format(PyExc_TypeError,
+                    "extension type '%.200s' has no __dict__ slot, but base type '%.200s' has: "
+                    "either add 'cdef dict __dict__' to the extension type "
+                    "or add '__slots__ = [...]' to the base type",
+                    t->tp_name, b->tp_name);
+                return -1;
+            }
+        }
+    }
+
+#if PY_VERSION_HEX >= 0x03050000
+    // As of https://bugs.python.org/issue22079
+    // PyType_Ready enforces that all bases of a non-heap type are
+    // non-heap. We know that this is the case for the solid base but
+    // other bases are heap allocated and are kept alive through the
+    // tp_bases reference.
+    // Other than this check, the Py_TPFLAGS_HEAPTYPE flag is unused
+    // in PyType_Ready().
+    t->tp_flags |= Py_TPFLAGS_HEAPTYPE;
+#endif
+
+    int r = PyType_Ready(t);
+
+#if PY_VERSION_HEX >= 0x03050000
+    t->tp_flags &= ~Py_TPFLAGS_HEAPTYPE;
+#endif
+
+    return r;
+}
 
 /////////////// CallNextTpDealloc.proto ///////////////
 

tests/run/cdef_multiple_inheritance_errors.srctree

+PYTHON setup.py build_ext --inplace
+PYTHON -c "import runner"
+
+######## setup.py ########
+
+from Cython.Build.Dependencies import cythonize
+from distutils.core import setup
+
+setup(ext_modules=cythonize("*.pyx"))
+
+######## notheaptype.pyx ########
+
+cdef class Base:
+    pass
+
+Obj = type(object())
+
+cdef class Foo(Base, Obj):
+    pass
+
+######## wrongbase.pyx ########
+
+cdef class Base:
+    pass
+
+Str = type("")
+
+cdef class X(Base, Str):
+    pass
+
+######## badmro.pyx ########
+
+class Py(object):
+    pass
+
+cdef class X(object, Py):
+    pass
+
+######## nodict.pyx ########
+
+cdef class Base:
+    pass
+
+class Py(object):
+    pass
+
+cdef class X(Base, Py):
+    pass
+
+######## runner.py ########
+
+try:
+    import notheaptype
+    assert False
+except TypeError as msg:
+    assert str(msg) == "base class 'object' is not a heap type"
+
+try:
+    import wrongbase
+    assert False
+except TypeError as msg:
+    assert str(msg) == "best base 'str' must be equal to first base 'wrongbase.Base'"
+
+try:
+    import badmro
+    assert False
+except TypeError as msg:
+    assert str(msg).startswith("Cannot create a consistent method resolution")
+
+try:
+    import nodict
+    assert False
+except TypeError as msg:
+    assert str(msg) == "extension type 'nodict.X' has no __dict__ slot, but base type 'Py' has: either add 'cdef dict __dict__' to the extension type or add '__slots__ = [...]' to the base type"

tests/run/cdef_multiple_inheritance_nodict.pyx

+# Copied from cdef_multiple_inheritance.pyx
+# but with __slots__ and without __dict__
+
+cdef class CBase(object):
+    cdef int a
+    cdef c_method(self):
+        return "CBase"
+    cpdef cpdef_method(self):
+        return "CBase"
+
+class PyBase(object):
+    __slots__ = []
+    def py_method(self):
+        return "PyBase"
+
+cdef class Both(CBase, PyBase):
+    """
+    >>> b = Both()
+    >>> b.py_method()
+    'PyBase'
+    >>> b.cp_method()
+    'Both'
+    >>> b.call_c_method()
+    'Both'
+
+    >>> isinstance(b, CBase)
+    True
+    >>> isinstance(b, PyBase)
+    True
+    """
+    cdef c_method(self):
+        return "Both"
+    cpdef cp_method(self):
+        return "Both"
+    def call_c_method(self):
+        return self.c_method()
+
+cdef class BothSub(Both):
+    """
+    >>> b = BothSub()
+    >>> b.py_method()
+    'PyBase'
+    >>> b.cp_method()
+    'Both'
+    >>> b.call_c_method()
+    'Both'
+    """
+    pass