Skip to content

Commit

Permalink
Improved errors when inheriting and forgetting to call __init__
Browse files Browse the repository at this point in the history
If the base class doesn't have a constructor, tell the user that it cannot be
inherited from.

Caveats:

* Doesn't work on PyPy
* Since default_object_init is inline, if the default instance base was no longer
  a PyHeapTypeObject it could crash.
* If pybind11_object_init changed in the future, and multiple pybind11 modules are
  present, get_internals might return an old version of the function
  • Loading branch information
virtuald committed Aug 24, 2020
1 parent c676036 commit d5bffa9
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 3 deletions.
13 changes: 10 additions & 3 deletions include/pybind11/detail/class.h
Original file line number Diff line number Diff line change
Expand Up @@ -171,8 +171,13 @@ extern "C" inline PyObject *pybind11_meta_call(PyObject *type, PyObject *args, P
// Ensure that the base __init__ function(s) were called
for (const auto &vh : values_and_holders(instance)) {
if (!vh.holder_constructed()) {
PyErr_Format(PyExc_TypeError, "%.200s.__init__() must be called when overriding __init__",
vh.type->type->tp_name);
auto message = "%.200s.__init__() must be called when overriding __init__";
#if !defined(PYPY_VERSION)
if (vh.type->type->tp_init == get_internals().default_object_init()) {
message = "%.200s has no __init__ and cannot be used as a base class from Python";
}
#endif
PyErr_Format(PyExc_TypeError, message, vh.type->type->tp_name);
Py_DECREF(self);
return nullptr;
}
Expand Down Expand Up @@ -302,6 +307,8 @@ extern "C" inline PyObject *pybind11_object_new(PyTypeObject *type, PyObject *,
/// An `__init__` function constructs the C++ object. Users should provide at least one
/// of these using `py::init` or directly with `.def(__init__, ...)`. Otherwise, the
/// following default function will be used which simply throws an exception.
///
/// Use `get_internals().default_object_init()` instead of using this function directly
extern "C" inline int pybind11_object_init(PyObject *self, PyObject *, PyObject *) {
PyTypeObject *type = Py_TYPE(self);
std::string msg;
Expand Down Expand Up @@ -620,7 +627,7 @@ inline PyObject* make_new_python_type(const type_record &rec) {
type->tp_bases = bases.release().ptr();

/* Don't inherit base __init__ */
type->tp_init = pybind11_object_init;
type->tp_init = internals.default_object_init();

/* Supported protocols */
type->tp_as_number = &heap_type->as_number;
Expand Down
5 changes: 5 additions & 0 deletions include/pybind11/detail/internals.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ struct internals {
PYBIND11_TLS_FREE(tstate);
}
#endif

inline initproc default_object_init() {
auto heap_type = (PyHeapTypeObject*)instance_base;
return heap_type->ht_type.tp_init;
}
};

/// Additional type information which does not fit into the PyTypeObject.
Expand Down
15 changes: 15 additions & 0 deletions tests/test_class.py
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,21 @@ def __init__(self):
expected = "m.class_.Hamster.__init__() must be called when overriding __init__"
assert msg(exc_info.value) == expected

# Base doesn't have __init__
class ChChimera(m.Chimera):
def __init__(self):
pass

with pytest.raises(TypeError) as exc_info:
ChChimera()
if env.PYPY:
# can't detect no __init__ in PyPy
expected = "Chimera.__init__() must be called when overriding __init__"
else:
expected = \
"m.class_.Chimera has no __init__ and cannot be used as a base class from Python"
assert msg(exc_info.value) == expected


def test_automatic_upcasting():
assert type(m.return_class_1()).__name__ == "DerivedClass1"
Expand Down

0 comments on commit d5bffa9

Please sign in to comment.