From e45c2114974a872674745a082828f07b34ea4721 Mon Sep 17 00:00:00 2001 From: Jason Rhinelander Date: Wed, 22 Feb 2017 21:36:09 -0500 Subject: [PATCH] Support multiple inheritance from python This commit allows multiple inheritance of pybind11 classes from Python, e.g. class MyType(Base1, Base2): def __init__(self): Base1.__init__(self) Base2.__init__(self) where Base1 and Base2 are pybind11-exported classes. This requires collapsing the various builtin base objects (pybind11_object_56, ...) introduced in 2.1 into a single pybind11_object of a fixed size; this fixed size object allocates enough space to contain either a simple object (one base class & small* holder instance), or a pointer to a new allocation that can contain an arbitrary number of base classes and holders, with holder size unrestricted. * "small" here means having a sizeof() of at most 2 pointers, which is enough to fit unique_ptr (sizeof is 1 ptr) and shared_ptr (sizeof is 2 ptrs). To minimize the performance impact, this repurposes `internals::registered_types_py` to store a vector of pybind-registered base types. For direct-use pybind types (e.g. the `PyA` for a C++ `A`) this is simply storing the same thing as before, but now in a vector; for Python-side inherited types, the map lets us avoid having to do a base class traversal as long as we've seen the class before. The change to vector is needed for multiple inheritance: Python types inheriting from multiple registered bases have one entry per base. --- docs/advanced/classes.rst | 30 +- include/pybind11/attr.h | 8 +- include/pybind11/cast.h | 571 ++++++++++++++++++++-------- include/pybind11/class_support.h | 162 ++++---- include/pybind11/common.h | 83 +++- include/pybind11/pybind11.h | 93 +++-- tests/constructor_stats.h | 2 +- tests/test_multiple_inheritance.cpp | 14 + tests/test_multiple_inheritance.py | 173 ++++++++- tests/test_smart_ptr.cpp | 27 ++ tests/test_smart_ptr.py | 10 + 11 files changed, 830 insertions(+), 343 deletions(-) diff --git a/docs/advanced/classes.rst b/docs/advanced/classes.rst index b6e0a2f4e..5617a4cb2 100644 --- a/docs/advanced/classes.rst +++ b/docs/advanced/classes.rst @@ -619,27 +619,19 @@ interspersed with alias types and holder types (discussed earlier in this document)---pybind11 will automatically find out which is which. The only requirement is that the first template argument is the type to be declared. -There are two caveats regarding the implementation of this feature: +It is also permitted to inherit multiply from exported C++ classes in Python, +as well as inheriting from multiple Python and/or pybind-exported classes. -1. When only one base type is specified for a C++ type that actually has - multiple bases, pybind11 will assume that it does not participate in - multiple inheritance, which can lead to undefined behavior. In such cases, - add the tag ``multiple_inheritance``: +There is one caveat regarding the implementation of this feature: - .. code-block:: cpp +When only one base type is specified for a C++ type that actually has multiple +bases, pybind11 will assume that it does not participate in multiple +inheritance, which can lead to undefined behavior. In such cases, add the tag +``multiple_inheritance`` to the class constructor: - py::class_(m, "MyType", py::multiple_inheritance()); +.. code-block:: cpp - The tag is redundant and does not need to be specified when multiple base - types are listed. + py::class_(m, "MyType", py::multiple_inheritance()); -2. As was previously discussed in the section on :ref:`overriding_virtuals`, it - is easy to create Python types that derive from C++ classes. It is even - possible to make use of multiple inheritance to declare a Python class which - has e.g. a C++ and a Python class as bases. However, any attempt to create a - type that has *two or more* C++ classes in its hierarchy of base types will - fail with a fatal error message: ``TypeError: multiple bases have instance - lay-out conflict``. Core Python types that are implemented in C (e.g. - ``dict``, ``list``, ``Exception``, etc.) also fall under this combination - and cannot be combined with C++ types bound using pybind11 via multiple - inheritance. +The tag is redundant and does not need to be specified when multiple base types +are listed. diff --git a/include/pybind11/attr.h b/include/pybind11/attr.h index 9f858d034..da81c0d88 100644 --- a/include/pybind11/attr.h +++ b/include/pybind11/attr.h @@ -210,17 +210,17 @@ struct type_record { /// How large is the underlying C++ type? size_t type_size = 0; - /// How large is pybind11::instance? - size_t instance_size = 0; + /// How large is the type's holder? + size_t holder_size = 0; /// The global operator new can be overridden with a class-specific variant void *(*operator_new)(size_t) = ::operator new; /// Function pointer to class_<..>::init_holder - void (*init_holder)(PyObject *, const void *) = nullptr; + void (*init_holder)(instance *, const void *) = nullptr; /// Function pointer to class_<..>::dealloc - void (*dealloc)(PyObject *) = nullptr; + void (*dealloc)(const detail::value_and_holder &) = nullptr; /// List of base classes of the newly created type list bases; diff --git a/include/pybind11/cast.h b/include/pybind11/cast.h index 9e7b4dda9..64d6d5300 100644 --- a/include/pybind11/cast.h +++ b/include/pybind11/cast.h @@ -16,20 +16,24 @@ #include #include #include +#include NAMESPACE_BEGIN(pybind11) NAMESPACE_BEGIN(detail) +// Forward declarations: inline PyTypeObject *make_static_property_type(); inline PyTypeObject *make_default_metaclass(); +inline PyObject *make_object_base_type(PyTypeObject *metaclass); +struct value_and_holder; /// Additional type information which does not fit into the PyTypeObject struct type_info { PyTypeObject *type; const std::type_info *cpptype; - size_t type_size; + size_t type_size, holder_size_in_ptrs; void *(*operator_new)(size_t); - void (*init_holder)(PyObject *, const void *); - void (*dealloc)(PyObject *); + void (*init_holder)(instance *, const void *); + void (*dealloc)(const value_and_holder &v_h); std::vector implicit_conversions; std::vector> implicit_casts; std::vector *direct_conversions; @@ -90,20 +94,93 @@ PYBIND11_NOINLINE inline internals &get_internals() { ); internals_ptr->static_property_type = make_static_property_type(); internals_ptr->default_metaclass = make_default_metaclass(); + internals_ptr->instance_base = make_object_base_type(internals_ptr->default_metaclass); } return *internals_ptr; } -PYBIND11_NOINLINE inline detail::type_info* get_type_info(PyTypeObject *type) { +// Gets the cache entry for the given type, creating it if necessary. The return value is the pair +// returned by emplace, i.e. an iterator for the entry and a bool set to `true` if the entry was +// just created. +inline std::pair all_type_info_get_cache(PyTypeObject *type); + +// Populates a just-created cache entry. +PYBIND11_NOINLINE inline void all_type_info_populate(PyTypeObject *t, std::vector &bases) { + std::vector check; + for (handle parent : reinterpret_borrow(t->tp_bases)) + check.push_back((PyTypeObject *) parent.ptr()); + auto const &type_dict = get_internals().registered_types_py; - do { + for (size_t i = 0; i < check.size(); i++) { + auto type = check[i]; + // Ignore Python2 old-style class super type: + if (!PyType_Check((PyObject *) type)) continue; + + // Check `type` in the current set of registered python types: auto it = type_dict.find(type); - if (it != type_dict.end()) - return (detail::type_info *) it->second; - type = type->tp_base; - if (!type) - return nullptr; - } while (true); + if (it != type_dict.end()) { + // We found a cache entry for it, so it's either pybind-registered or has pre-computed + // pybind bases, but we have to make sure we haven't already seen the type(s) before: we + // want to follow Python/virtual C++ rules that there should only be one instance of a + // common base. + for (auto *tinfo : it->second) { + // NB: Could use a second set here, rather than doing a linear search, but since + // having a large number of immediate pybind11-registered types seems fairly + // unlikely, that probably isn't worthwhile. + bool found = false; + for (auto *known : bases) { + if (known == tinfo) { found = true; break; } + } + if (!found) bases.push_back(tinfo); + } + } + else if (type->tp_bases) { + // It's some python type, so keep follow its bases classes to look for one or more + // registered types + if (i + 1 == check.size()) { + // When we're at the end, we can pop off the current element to avoid growing + // `check` when adding just one base (which is typical--.e. when there is no + // multiple inheritance) + check.pop_back(); + i--; + } + for (handle parent : reinterpret_borrow(type->tp_bases)) + check.push_back((PyTypeObject *) parent.ptr()); + } + } +} + +/** + * Extracts vector of type_info pointers of pybind-registered roots of the given Python type. Will + * be just 1 pybind type for the Python type of a pybind-registered class, or for any Python-side + * derived class that uses single inheritance. Will contain as many types as required for a Python + * class that uses multiple inheritance to inherit (directly or indirectly) from multiple + * pybind-registered classes. Will be empty if neither the type nor any base classes are + * pybind-registered. + * + * The value is cached for the lifetime of the Python type. + */ +inline const std::vector &all_type_info(PyTypeObject *type) { + auto ins = all_type_info_get_cache(type); + if (ins.second) + // New cache entry: populate it + all_type_info_populate(type, ins.first->second); + + return ins.first->second; +} + +/** + * Gets a single pybind11 type info for a python type. Returns nullptr if neither the type nor any + * ancestors are pybind11-registered. Throws an exception if there are multiple bases--use + * `all_type_info` instead if you want to support multiple bases. + */ +PYBIND11_NOINLINE inline detail::type_info* get_type_info(PyTypeObject *type) { + auto &bases = all_type_info(type); + if (bases.size() == 0) + return nullptr; + if (bases.size() > 1) + pybind11_fail("pybind11::detail::get_type_info: type has multiple pybind11-registered bases"); + return bases.front(); } PYBIND11_NOINLINE inline detail::type_info *get_type_info(const std::type_info &tp, @@ -126,6 +203,178 @@ PYBIND11_NOINLINE inline handle get_type_handle(const std::type_info &tp, bool t return handle(type_info ? ((PyObject *) type_info->type) : nullptr); } +struct value_and_holder { + instance *inst; + size_t index; + const detail::type_info *type; + void **vh; + + value_and_holder(instance *i, const detail::type_info *type, size_t vpos, size_t index) : + inst{i}, index{index}, type{type}, + vh{inst->simple_layout ? inst->simple_value_holder : &inst->nonsimple.values_and_holders[vpos]} + {} + + // Used for past-the-end iterator + value_and_holder(size_t index) : index{index} {} + + template V *&value_ptr() const { + return reinterpret_cast(vh[0]); + } + // True if this `value_and_holder` has a non-null value pointer + explicit operator bool() const { return value_ptr(); } + + template H &holder() const { + return reinterpret_cast(vh[1]); + } + bool holder_constructed() const { + return inst->simple_layout + ? inst->simple_holder_constructed + : inst->nonsimple.holder_constructed[index]; + } + void set_holder_constructed() { + if (inst->simple_layout) + inst->simple_holder_constructed = true; + else + inst->nonsimple.holder_constructed[index] = true; + } +}; + +// Container for accessing and iterating over an instance's values/holders +struct values_and_holders { +private: + instance *inst; + using type_vec = std::vector; + const type_vec &tinfo; + +public: + values_and_holders(instance *inst) : inst{inst}, tinfo(all_type_info(Py_TYPE(inst))) {} + + struct iterator { + private: + instance *inst; + using vec_iter = std::vector::const_iterator; + vec_iter typeit; + value_and_holder curr; + friend struct values_and_holders; + iterator(instance *inst, const type_vec &tinfo) + : inst{inst}, typeit{tinfo.begin()}, + curr(inst /* instance */, + tinfo.size() > 0 ? *typeit : nullptr /* type info */, + 0, /* vpos: (non-simple types only): the first vptr comes first */ + 0 /* index */) + {} + // Past-the-end iterator: + iterator(size_t end) : curr(end) {} + public: + bool operator==(const iterator &other) { return curr.index == other.curr.index; } + bool operator!=(const iterator &other) { return curr.index != other.curr.index; } + iterator &operator++() { + if (!inst->simple_layout) { + curr.vh += 1 + (*typeit)->holder_size_in_ptrs; + curr.type = *(++typeit); + } + ++curr.index; + return *this; + } + value_and_holder &operator*() { return curr; } + value_and_holder *operator->() { return &curr; } + }; + + iterator begin() { return iterator(inst, tinfo); } + iterator end() { return iterator(tinfo.size()); } + + iterator find(const type_info *find_type) { + auto it = begin(), endit = end(); + while (it != endit && it->type != find_type) ++it; + return it; + } + + size_t size() { return tinfo.size(); } +}; + +/** + * Extracts C++ value and holder pointer references from an instance (which may contain multiple + * values/holders for python-side multiple inheritance) that match the given type. Throws an error + * if the given type (or ValueType, if omitted) is not a pybind11 base of the given instance. If + * `find_type` is omitted (or explicitly specified as nullptr) the first value/holder are returned, + * regardless of type (and the resulting .type will be nullptr). + * + * The returned object should be short-lived: in particular, it must not outlive the called-upon + * instance. + */ +PYBIND11_NOINLINE inline value_and_holder instance::get_value_and_holder(const type_info *find_type /*= nullptr default in common.h*/) { + // Optimize common case: + if (!find_type || Py_TYPE(this) == find_type->type) + return value_and_holder(this, find_type, 0, 0); + + detail::values_and_holders vhs(this); + auto it = vhs.find(find_type); + if (it != vhs.end()) + return *it; + +#if defined(NDEBUG) + pybind11_fail("pybind11::detail::instance::get_value_and_holder: " + "type is not a pybind11 base of the given instance " + "(compile in debug mode for type details)"); +#else + pybind11_fail("pybind11::detail::instance::get_value_and_holder: `" + + std::string(find_type->type->tp_name) + "' is not a pybind11 base of the given `" + + std::string(Py_TYPE(this)->tp_name) + "' instance"); +#endif +} + +PYBIND11_NOINLINE inline void instance::allocate_layout() { + auto &tinfo = all_type_info(Py_TYPE(this)); + + const size_t n_types = tinfo.size(); + + if (n_types == 0) + pybind11_fail("instance allocation failed: new instance has no pybind11-registered base types"); + + simple_layout = + n_types == 1 && tinfo.front()->holder_size_in_ptrs <= instance_simple_holder_in_ptrs(); + + // Simple path: no python-side multiple inheritance, and a small-enough holder + if (simple_layout) { + simple_value_holder[0] = nullptr; + simple_holder_constructed = false; + } + else { // multiple base types or a too-large holder + // Allocate space to hold: [v1*][h1][v2*][h2]...[bb...] where [vN*] is a value pointer, + // [hN] is the (uninitialized) holder instance for value N, and [bb...] is a set of bool + // values that tracks whether each associated holder has been initialized. Each [block] is + // padded, if necessary, to an integer multiple of sizeof(void *). + size_t space = 0; + for (auto t : tinfo) { + space += 1; // value pointer + space += t->holder_size_in_ptrs; // holder instance + } + size_t flags_at = space; + space += size_in_ptrs(n_types * sizeof(bool)); // holder constructed flags + + // Allocate space for flags, values, and holders, and initialize it to 0 (flags and values, + // in particular, need to be 0). Use Python's memory allocation functions: in Python 3.6 + // they default to using pymalloc, which is designed to be efficient for small allocations + // like the one we're doing here; in earlier versions (and for larger allocations) they are + // just wrappers around malloc. +#if PY_VERSION_HEX >= 0x03050000 + nonsimple.values_and_holders = (void **) PyMem_Calloc(space, sizeof(void *)); + if (!nonsimple.values_and_holders) throw std::bad_alloc(); +#else + nonsimple.values_and_holders = (void **) PyMem_New(void *, space); + if (!nonsimple.values_and_holders) throw std::bad_alloc(); + std::memset(nonsimple.values_and_holders, 0, space * sizeof(void *)); +#endif + nonsimple.holder_constructed = reinterpret_cast(&nonsimple.values_and_holders[flags_at]); + } + owned = true; +} + +PYBIND11_NOINLINE inline void instance::deallocate_layout() { + if (!simple_layout) + PyMem_Free(nonsimple.values_and_holders); +} + PYBIND11_NOINLINE inline bool isinstance_generic(handle obj, const std::type_info &tp) { handle type = detail::get_type_handle(tp, false); if (!type) @@ -185,9 +434,10 @@ PYBIND11_NOINLINE inline handle get_object_handle(const void *ptr, const detail: auto &instances = get_internals().registered_instances; auto range = instances.equal_range(ptr); for (auto it = range.first; it != range.second; ++it) { - auto instance_type = detail::get_type_info(Py_TYPE(it->second)); - if (instance_type && instance_type == type) - return handle((PyObject *) it->second); + for (auto vh : values_and_holders(it->second)) { + if (vh.type == type) + return handle((PyObject *) it->second); + } } return handle(); } @@ -208,7 +458,7 @@ inline PyThreadState *get_thread_state_unchecked() { // Forward declarations inline void keep_alive_impl(handle nurse, handle patient); -inline void register_instance(void *self, const type_info *tinfo); +inline void register_instance(instance *self, void *valptr, const type_info *tinfo); inline PyObject *make_new_instance(PyTypeObject *type, bool allocate_value = true); class type_caster_generic { @@ -216,70 +466,8 @@ public: PYBIND11_NOINLINE type_caster_generic(const std::type_info &type_info) : typeinfo(get_type_info(type_info)) { } - PYBIND11_NOINLINE bool load(handle src, bool convert) { - if (!src) - return false; - return load(src, convert, Py_TYPE(src.ptr())); - } - - bool load(handle src, bool convert, PyTypeObject *tobj) { - if (!src || !typeinfo) - return false; - if (src.is_none()) { - // Defer accepting None to other overloads (if we aren't in convert mode): - if (!convert) return false; - value = nullptr; - return true; - } - - if (typeinfo->simple_type) { /* Case 1: no multiple inheritance etc. involved */ - /* Check if we can safely perform a reinterpret-style cast */ - if (PyType_IsSubtype(tobj, typeinfo->type)) { - value = reinterpret_cast *>(src.ptr())->value; - return true; - } - } else { /* Case 2: multiple inheritance */ - /* Check if we can safely perform a reinterpret-style cast */ - if (tobj == typeinfo->type) { - value = reinterpret_cast *>(src.ptr())->value; - return true; - } - - /* If this is a python class, also check the parents recursively */ - auto const &type_dict = get_internals().registered_types_py; - bool new_style_class = PyType_Check((PyObject *) tobj); - if (type_dict.find(tobj) == type_dict.end() && new_style_class && tobj->tp_bases) { - auto parents = reinterpret_borrow(tobj->tp_bases); - for (handle parent : parents) { - bool result = load(src, convert, (PyTypeObject *) parent.ptr()); - if (result) - return true; - } - } - - /* Try implicit casts */ - for (auto &cast : typeinfo->implicit_casts) { - type_caster_generic sub_caster(*cast.first); - if (sub_caster.load(src, convert)) { - value = cast.second(sub_caster.value); - return true; - } - } - } - - /* Perform an implicit conversion */ - if (convert) { - for (auto &converter : typeinfo->implicit_conversions) { - temp = reinterpret_steal(converter(src.ptr(), typeinfo->type)); - if (load(temp, false)) - return true; - } - for (auto &converter : *typeinfo->direct_conversions) { - if (converter(src.ptr(), value)) - return true; - } - } - return false; + bool load(handle src, bool convert) { + return load_impl(src, convert); } PYBIND11_NOINLINE static handle cast(const void *_src, return_value_policy policy, handle parent, @@ -296,34 +484,33 @@ public: auto it_instances = get_internals().registered_instances.equal_range(src); for (auto it_i = it_instances.first; it_i != it_instances.second; ++it_i) { - auto instance_type = detail::get_type_info(Py_TYPE(it_i->second)); - if (instance_type && instance_type == tinfo) - return handle((PyObject *) it_i->second).inc_ref(); + for (auto instance_type : detail::all_type_info(Py_TYPE(it_i->second))) { + if (instance_type && instance_type == tinfo) + return handle((PyObject *) it_i->second).inc_ref(); + } } auto inst = reinterpret_steal(make_new_instance(tinfo->type, false /* don't allocate value */)); - - auto wrapper = (instance *) inst.ptr(); - - wrapper->value = nullptr; + auto wrapper = reinterpret_cast(inst.ptr()); wrapper->owned = false; + void *&valueptr = values_and_holders(wrapper).begin()->value_ptr(); switch (policy) { case return_value_policy::automatic: case return_value_policy::take_ownership: - wrapper->value = src; + valueptr = src; wrapper->owned = true; break; case return_value_policy::automatic_reference: case return_value_policy::reference: - wrapper->value = src; + valueptr = src; wrapper->owned = false; break; case return_value_policy::copy: if (copy_constructor) - wrapper->value = copy_constructor(src); + valueptr = copy_constructor(src); else throw cast_error("return_value_policy = copy, but the " "object is non-copyable!"); @@ -332,9 +519,9 @@ public: case return_value_policy::move: if (move_constructor) - wrapper->value = move_constructor(src); + valueptr = move_constructor(src); else if (copy_constructor) - wrapper->value = copy_constructor(src); + valueptr = copy_constructor(src); else throw cast_error("return_value_policy = move, but the " "object is neither movable nor copyable!"); @@ -342,23 +529,119 @@ public: break; case return_value_policy::reference_internal: - wrapper->value = src; + valueptr = src; wrapper->owned = false; - detail::keep_alive_impl(inst, parent); + keep_alive_impl(inst, parent); break; default: throw cast_error("unhandled return_value_policy: should not happen!"); } - register_instance(wrapper, tinfo); - tinfo->init_holder(inst.ptr(), existing_holder); + register_instance(wrapper, valueptr, tinfo); + tinfo->init_holder(wrapper, existing_holder); return inst.release(); } protected: + // Base methods for generic caster; there are overridden in copyable_holder_caster + void load_value(const value_and_holder &v_h) { + value = v_h.value_ptr(); + } + bool try_implicit_casts(handle src, bool convert) { + for (auto &cast : typeinfo->implicit_casts) { + type_caster_generic sub_caster(*cast.first); + if (sub_caster.load(src, convert)) { + value = cast.second(sub_caster.value); + return true; + } + } + return false; + } + bool try_direct_conversions(handle src) { + for (auto &converter : *typeinfo->direct_conversions) { + if (converter(src.ptr(), value)) + return true; + } + return false; + } + void check_holder_compat() {} + + // Implementation of `load`; this takes the type of `this` so that it can dispatch the relevant + // bits of code between here and copyable_holder_caster where the two classes need different + // logic (without having to resort to virtual inheritance). + template + PYBIND11_NOINLINE bool load_impl(handle src, bool convert) { + if (!src || !typeinfo) + return false; + if (src.is_none()) { + // Defer accepting None to other overloads (if we aren't in convert mode): + if (!convert) return false; + value = nullptr; + return true; + } + + auto &this_ = static_cast(*this); + this_.check_holder_compat(); + + PyTypeObject *srctype = Py_TYPE(src.ptr()); + + // Case 1: If src is an exact type match for the target type then we can reinterpret_cast + // the instance's value pointer to the target type: + if (srctype == typeinfo->type) { + this_.load_value(reinterpret_cast(src.ptr())->get_value_and_holder()); + return true; + } + // Case 2: We have a derived class + else if (PyType_IsSubtype(srctype, typeinfo->type)) { + auto &bases = all_type_info(srctype); + bool no_cpp_mi = typeinfo->simple_type; + + // Case 2a: the python type is a Python-inherited derived class that inherits from just + // one simple (no MI) pybind11 class, or is an exact match, so the C++ instance is of + // the right type and we can use reinterpret_cast. + // (This is essentially the same as case 2b, but because not using multiple inheritance + // is extremely common, we handle it specially to avoid the loop iterator and type + // pointer lookup overhead) + if (bases.size() == 1 && (no_cpp_mi || bases.front()->type == typeinfo->type)) { + this_.load_value(reinterpret_cast(src.ptr())->get_value_and_holder()); + return true; + } + // Case 2b: the python type inherits from multiple C++ bases. Check the bases to see if + // we can find an exact match (or, for a simple C++ type, an inherited match); if so, we + // can safely reinterpret_cast to the relevant pointer. + else if (bases.size() > 1) { + for (auto base : bases) { + if (no_cpp_mi ? PyType_IsSubtype(base->type, typeinfo->type) : base->type == typeinfo->type) { + this_.load_value(reinterpret_cast(src.ptr())->get_value_and_holder(base)); + return true; + } + } + } + + // Case 2c: C++ multiple inheritance is involved and we couldn't find an exact type match + // in the registered bases, above, so try implicit casting (needed for proper C++ casting + // when MI is involved). + if (this_.try_implicit_casts(src, convert)) + return true; + } + + // Perform an implicit conversion + if (convert) { + for (auto &converter : typeinfo->implicit_conversions) { + temp = reinterpret_steal(converter(src.ptr(), typeinfo->type)); + if (load_impl(temp, false)) + return true; + } + if (this_.try_direct_conversions(src)) + return true; + } + return false; + } + + // Called to do type lookup and wrap the pointer and type in a pair when a dynamic_cast // isn't needed or can't be used. If the type is unknown, sets the error and returns a pair // with .second = nullptr. (p.first = nullptr is not an error: it becomes None). @@ -669,8 +952,9 @@ public: } /* Check if this is a C++ type */ - if (get_type_info((PyTypeObject *) h.get_type().ptr())) { - value = ((instance *) h.ptr())->value; + auto &bases = all_type_info((PyTypeObject *) h.get_type().ptr()); + if (bases.size() == 1) { // Only allowing loading from a single-value type + value = values_and_holders(reinterpret_cast(h.ptr())).begin()->value_ptr(); return true; } @@ -1016,64 +1300,38 @@ public: using base::value; using base::temp; - PYBIND11_NOINLINE bool load(handle src, bool convert) { - return load(src, convert, Py_TYPE(src.ptr())); + bool load(handle src, bool convert) { + return base::template load_impl>(src, convert); } - bool load(handle src, bool convert, PyTypeObject *tobj) { - if (!src || !typeinfo) - return false; - if (src.is_none()) { - // Defer accepting None to other overloads (if we aren't in convert mode): - if (!convert) return false; - value = nullptr; - return true; - } + explicit operator type*() { return this->value; } + explicit operator type&() { return *(this->value); } + explicit operator holder_type*() { return &holder; } + // Workaround for Intel compiler bug + // see pybind11 issue 94 + #if defined(__ICC) || defined(__INTEL_COMPILER) + operator holder_type&() { return holder; } + #else + explicit operator holder_type&() { return holder; } + #endif + + static handle cast(const holder_type &src, return_value_policy, handle) { + const auto *ptr = holder_helper::get(src); + return type_caster_base::cast_holder(ptr, &src); + } + +protected: + friend class type_caster_generic; + void check_holder_compat() { if (typeinfo->default_holder) throw cast_error("Unable to load a custom holder type from a default-holder instance"); - - if (typeinfo->simple_type) { /* Case 1: no multiple inheritance etc. involved */ - /* Check if we can safely perform a reinterpret-style cast */ - if (PyType_IsSubtype(tobj, typeinfo->type)) - return load_value_and_holder(src); - } else { /* Case 2: multiple inheritance */ - /* Check if we can safely perform a reinterpret-style cast */ - if (tobj == typeinfo->type) - return load_value_and_holder(src); - - /* If this is a python class, also check the parents recursively */ - auto const &type_dict = get_internals().registered_types_py; - bool new_style_class = PyType_Check((PyObject *) tobj); - if (type_dict.find(tobj) == type_dict.end() && new_style_class && tobj->tp_bases) { - auto parents = reinterpret_borrow(tobj->tp_bases); - for (handle parent : parents) { - bool result = load(src, convert, (PyTypeObject *) parent.ptr()); - if (result) - return true; - } - } - - if (try_implicit_casts(src, convert)) - return true; - } - - if (convert) { - for (auto &converter : typeinfo->implicit_conversions) { - temp = reinterpret_steal(converter(src.ptr(), typeinfo->type)); - if (load(temp, false)) - return true; - } - } - - return false; } - bool load_value_and_holder(handle src) { - auto inst = (instance *) src.ptr(); - value = (void *) inst->value; - if (inst->holder_constructed) { - holder = inst->holder; + bool load_value(const value_and_holder &v_h) { + if (v_h.holder_constructed()) { + value = v_h.value_ptr(); + holder = v_h.holder(); return true; } else { throw cast_error("Unable to cast from non-held to held instance (T& to Holder) " @@ -1101,24 +1359,9 @@ public: return false; } - explicit operator type*() { return this->value; } - explicit operator type&() { return *(this->value); } - explicit operator holder_type*() { return &holder; } + static constexpr bool try_direct_conversions(handle) { return false; } - // Workaround for Intel compiler bug - // see pybind11 issue 94 - #if defined(__ICC) || defined(__INTEL_COMPILER) - operator holder_type&() { return holder; } - #else - explicit operator holder_type&() { return holder; } - #endif - static handle cast(const holder_type &src, return_value_policy, handle) { - const auto *ptr = holder_helper::get(src); - return type_caster_base::cast_holder(ptr, &src); - } - -protected: holder_type holder; }; diff --git a/include/pybind11/class_support.h b/include/pybind11/class_support.h index fb73390ae..b6065bd58 100644 --- a/include/pybind11/class_support.h +++ b/include/pybind11/class_support.h @@ -87,34 +87,6 @@ inline PyTypeObject *make_static_property_type() { #endif // PYPY -/** Inheriting from multiple C++ types in Python is not supported -- set an error instead. - A Python definition (`class C(A, B): pass`) will call `tp_new` so we check for multiple - C++ bases here. On the other hand, C++ type definitions (`py::class_(m, "C")`) - don't not use `tp_new` and will not trigger this error. */ -extern "C" inline PyObject *pybind11_meta_new(PyTypeObject *metaclass, PyObject *args, - PyObject *kwargs) { - PyObject *bases = PyTuple_GetItem(args, 1); // arguments: (name, bases, dict) - if (!bases) - return nullptr; - - auto &internals = get_internals(); - auto num_cpp_bases = 0; - for (auto base : reinterpret_borrow(bases)) { - auto base_type = (PyTypeObject *) base.ptr(); - auto instance_size = static_cast(base_type->tp_basicsize); - if (PyObject_IsSubclass(base.ptr(), internals.get_base(instance_size))) - ++num_cpp_bases; - } - - if (num_cpp_bases > 1) { - PyErr_SetString(PyExc_TypeError, "Can't inherit from multiple C++ classes in Python." - "Use py::class_ to define the class in C++ instead."); - return nullptr; - } else { - return PyType_Type.tp_new(metaclass, args, kwargs); - } -} - /** Types with static properties need to handle `Type.static_prop = x` in a specific way. By default, Python replaces the `static_property` itself, but for wrapped C++ types we need to call `static_property.__set__()` in order to propagate the new value to @@ -193,7 +165,6 @@ inline PyTypeObject* make_default_metaclass() { type->tp_base = &PyType_Type; type->tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HEAPTYPE; - type->tp_new = pybind11_meta_new; type->tp_setattro = pybind11_meta_setattro; #if PY_MAJOR_VERSION >= 3 type->tp_getattro = pybind11_meta_getattro; @@ -210,8 +181,8 @@ inline PyTypeObject* make_default_metaclass() { /// For multiple inheritance types we need to recursively register/deregister base pointers for any /// base classes with pointers that are difference from the instance value pointer so that we can /// correctly recognize an offset base class pointer. This calls a function with any offset base ptrs. -inline void traverse_offset_bases(void *valueptr, const detail::type_info *tinfo, void *self, - bool (*f)(void * /*parentptr*/, void * /*self*/)) { +inline void traverse_offset_bases(void *valueptr, const detail::type_info *tinfo, instance *self, + bool (*f)(void * /*parentptr*/, instance * /*self*/)) { for (handle h : reinterpret_borrow(tinfo->type->tp_bases)) { if (auto parent_tinfo = get_type_info((PyTypeObject *) h.ptr())) { for (auto &c : parent_tinfo->implicit_casts) { @@ -227,11 +198,11 @@ inline void traverse_offset_bases(void *valueptr, const detail::type_info *tinfo } } -inline bool register_instance_impl(void *ptr, void *self) { +inline bool register_instance_impl(void *ptr, instance *self) { get_internals().registered_instances.emplace(ptr, self); return true; // unused, but gives the same signature as the deregister func } -inline bool deregister_instance_impl(void *ptr, void *self) { +inline bool deregister_instance_impl(void *ptr, instance *self) { auto ®istered_instances = get_internals().registered_instances; auto range = registered_instances.equal_range(ptr); for (auto it = range.first; it != range.second; ++it) { @@ -243,36 +214,48 @@ inline bool deregister_instance_impl(void *ptr, void *self) { return false; } -inline void register_instance(void *self, const type_info *tinfo) { - auto *inst = (instance_essentials *) self; - register_instance_impl(inst->value, self); +inline void register_instance(instance *self, void *valptr, const type_info *tinfo) { + register_instance_impl(valptr, self); if (!tinfo->simple_ancestors) - traverse_offset_bases(inst->value, tinfo, self, register_instance_impl); + traverse_offset_bases(valptr, tinfo, self, register_instance_impl); } -inline bool deregister_instance(void *self, const detail::type_info *tinfo) { - auto *inst = (instance_essentials *) self; - bool ret = deregister_instance_impl(inst->value, self); +inline bool deregister_instance(instance *self, void *valptr, const type_info *tinfo) { + bool ret = deregister_instance_impl(valptr, self); if (!tinfo->simple_ancestors) - traverse_offset_bases(inst->value, tinfo, self, deregister_instance_impl); + traverse_offset_bases(valptr, tinfo, self, deregister_instance_impl); return ret; } -/// Creates a new instance which, by default, includes allocation (but not construction of) the -/// wrapped C++ instance. If allocating value, the instance is registered; otherwise -/// register_instance will need to be called once the value has been assigned. +/// Instance creation function for all pybind11 types. It only allocates space for the C++ object +/// (or multiple objects, for Python-side inheritance from multiple pybind11 types), but doesn't +/// call the constructor -- an `__init__` function must do that. If allocating value, the instance +/// is registered; otherwise register_instance will need to be called once the value has been +/// assigned. inline PyObject *make_new_instance(PyTypeObject *type, bool allocate_value /*= true (in cast.h)*/) { - PyObject *self = type->tp_alloc(type, 0); - auto instance = (instance_essentials *) self; - auto tinfo = get_type_info(type); - instance->owned = true; - instance->holder_constructed = false; - if (allocate_value) { - instance->value = tinfo->operator_new(tinfo->type_size); - register_instance(self, tinfo); - } else { - instance->value = nullptr; +#if defined(PYPY_VERSION) + // PyPy gets tp_basicsize wrong (issue 2482) under multiple inheritance when the first inherited + // object is a a plain Python type (i.e. not derived from an extension type). Fix it. + ssize_t instance_size = static_cast(sizeof(instance)); + if (type->tp_basicsize < instance_size) { + type->tp_basicsize = instance_size; } +#endif + PyObject *self = type->tp_alloc(type, 0); + auto inst = reinterpret_cast(self); + // Allocate the value/holder internals: + inst->allocate_layout(); + + inst->owned = true; + // Allocate (if requested) the value pointers; otherwise leave them as nullptr + if (allocate_value) { + for (auto &v_h : values_and_holders(inst)) { + void *&vptr = v_h.value_ptr(); + vptr = v_h.type->operator_new(v_h.type->type_size); + register_instance(inst, vptr, v_h.type); + } + } + return self; } @@ -300,26 +283,27 @@ extern "C" inline int pybind11_object_init(PyObject *self, PyObject *, PyObject /// Clears all internal data from the instance and removes it from registered instances in /// preparation for deallocation. inline void clear_instance(PyObject *self) { - auto instance = (instance_essentials *) self; - bool has_value = instance->value; - type_info *tinfo = nullptr; - if (has_value || instance->holder_constructed) { - auto type = Py_TYPE(self); - tinfo = get_type_info(type); - tinfo->dealloc(self); - } - if (has_value) { - if (!tinfo) tinfo = get_type_info(Py_TYPE(self)); - if (!deregister_instance(self, tinfo)) - pybind11_fail("pybind11_object_dealloc(): Tried to deallocate unregistered instance!"); + auto instance = reinterpret_cast(self); - if (instance->weakrefs) - PyObject_ClearWeakRefs(self); + // Deallocate any values/holders, if present: + for (auto &v_h : values_and_holders(instance)) { + if (v_h) { + if (instance->owned || v_h.holder_constructed()) + v_h.type->dealloc(v_h); - PyObject **dict_ptr = _PyObject_GetDictPtr(self); - if (dict_ptr) - Py_CLEAR(*dict_ptr); + if (!deregister_instance(instance, v_h.value_ptr(), v_h.type)) + pybind11_fail("pybind11_object_dealloc(): Tried to deallocate unregistered instance!"); + } } + // Deallocate the value/holder layout internals: + instance->deallocate_layout(); + + if (instance->weakrefs) + PyObject_ClearWeakRefs(self); + + PyObject **dict_ptr = _PyObject_GetDictPtr(self); + if (dict_ptr) + Py_CLEAR(*dict_ptr); } /// Instance destructor function for all pybind11 types. It calls `type_info.dealloc` @@ -329,19 +313,17 @@ extern "C" inline void pybind11_object_dealloc(PyObject *self) { Py_TYPE(self)->tp_free(self); } -/** Create a type which can be used as a common base for all classes with the same - instance size, i.e. all classes with the same `sizeof(holder_type)`. This is +/** Create the type which can be used as a common base for all classes. This is needed in order to satisfy Python's requirements for multiple inheritance. Return value: New reference. */ -inline PyObject *make_object_base_type(size_t instance_size) { - auto name = "pybind11_object_" + std::to_string(instance_size); - auto name_obj = reinterpret_steal(PYBIND11_FROM_STRING(name.c_str())); +inline PyObject *make_object_base_type(PyTypeObject *metaclass) { + constexpr auto *name = "pybind11_object"; + auto name_obj = reinterpret_steal(PYBIND11_FROM_STRING(name)); /* Danger zone: from now (and until PyType_Ready), make sure to issue no Python C API calls which could potentially invoke the garbage collector (the GC will call type_traverse(), which will in turn find the newly constructed type in an invalid state) */ - auto metaclass = get_internals().default_metaclass; auto heap_type = (PyHeapTypeObject *) metaclass->tp_alloc(metaclass, 0); if (!heap_type) pybind11_fail("make_object_base_type(): error allocating type!"); @@ -352,9 +334,9 @@ inline PyObject *make_object_base_type(size_t instance_size) { #endif auto type = &heap_type->ht_type; - type->tp_name = strdup(name.c_str()); + type->tp_name = name; type->tp_base = &PyBaseObject_Type; - type->tp_basicsize = static_cast(instance_size); + type->tp_basicsize = static_cast(sizeof(instance)); type->tp_flags = Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE | Py_TPFLAGS_HEAPTYPE; type->tp_new = pybind11_object_new; @@ -362,7 +344,7 @@ inline PyObject *make_object_base_type(size_t instance_size) { type->tp_dealloc = pybind11_object_dealloc; /* Support weak references (needed for the keep_alive feature) */ - type->tp_weaklistoffset = offsetof(instance_essentials, weakrefs); + type->tp_weaklistoffset = offsetof(instance, weakrefs); if (PyType_Ready(type) < 0) pybind11_fail("PyType_Ready failed in make_object_base_type():" + error_string()); @@ -373,20 +355,6 @@ inline PyObject *make_object_base_type(size_t instance_size) { return (PyObject *) heap_type; } -/** Return the appropriate base type for the given instance size. The results are cached - in `internals.bases` so that only a single base is ever created for any size value. - Return value: Borrowed reference. */ -inline PyObject *internals::get_base(size_t instance_size) { - auto it = bases.find(instance_size); - if (it != bases.end()) { - return it->second; - } else { - auto base = make_object_base_type(instance_size); - bases[instance_size] = base; - return base; - } -} - /// dynamic_attr: Support for `d = instance.__dict__`. extern "C" inline PyObject *pybind11_get_dict(PyObject *self, void *) { PyObject *&dict = *_PyObject_GetDictPtr(self); @@ -460,7 +428,7 @@ extern "C" inline int pybind11_getbuffer(PyObject *obj, Py_buffer *view, int fla PyErr_SetString(PyExc_BufferError, "pybind11_getbuffer(): Internal error"); return -1; } - memset(view, 0, sizeof(Py_buffer)); + std::memset(view, 0, sizeof(Py_buffer)); buffer_info *info = tinfo->get_buffer(obj, tinfo->get_buffer_data); view->obj = obj; view->ndim = 1; @@ -536,7 +504,7 @@ inline PyObject* make_new_python_type(const type_record &rec) { auto &internals = get_internals(); auto bases = tuple(rec.bases); - auto base = (bases.size() == 0) ? internals.get_base(rec.instance_size) + auto base = (bases.size() == 0) ? internals.instance_base : bases[0].ptr(); /* Danger zone: from now (and until PyType_Ready), make sure to @@ -559,7 +527,7 @@ inline PyObject* make_new_python_type(const type_record &rec) { type->tp_name = strdup(full_name.c_str()); type->tp_doc = tp_doc; type->tp_base = (PyTypeObject *) handle(base).inc_ref().ptr(); - type->tp_basicsize = static_cast(rec.instance_size); + type->tp_basicsize = static_cast(sizeof(instance)); if (bases.size() > 0) type->tp_bases = bases.release().ptr(); diff --git a/include/pybind11/common.h b/include/pybind11/common.h index c1ce78760..b0d1bc22d 100644 --- a/include/pybind11/common.h +++ b/include/pybind11/common.h @@ -346,21 +346,77 @@ NAMESPACE_BEGIN(detail) inline static constexpr int log2(size_t n, int k = 0) { return (n <= 1) ? k : log2(n >> 1, k + 1); } +// Returns the size as a multiple of sizeof(void *), rounded up. +inline static constexpr size_t size_in_ptrs(size_t s) { return 1 + ((s - 1) >> log2(sizeof(void *))); } + inline std::string error_string(); -/// Core part of the 'instance' type which POD (needed to be able to use 'offsetof') -template struct instance_essentials { +/** + * The space to allocate for simple layout instance holders (see below) in multiple of the size of + * a pointer (e.g. 2 means 16 bytes on 64-bit architectures). The default is the minimum required + * to holder either a std::unique_ptr or std::shared_ptr (which is almost always + * sizeof(std::shared_ptr)). + */ +constexpr size_t instance_simple_holder_in_ptrs() { + static_assert(sizeof(std::shared_ptr) >= sizeof(std::unique_ptr), + "pybind assumes std::shared_ptrs are at least as big as std::unique_ptrs"); + return size_in_ptrs(sizeof(std::shared_ptr)); +} + +// Forward declarations +struct type_info; +struct value_and_holder; + +/// The 'instance' type which needs to be standard layout (need to be able to use 'offsetof') +struct instance { PyObject_HEAD - type *value; + /// Storage for pointers and holder; see simple_layout, below, for a description + union { + void *simple_value_holder[1 + instance_simple_holder_in_ptrs()]; + struct { + void **values_and_holders; + bool *holder_constructed; + } nonsimple; + }; + /// Weak references (needed for keep alive): PyObject *weakrefs; + /// If true, the pointer is owned which means we're free to manage it with a holder. bool owned : 1; - bool holder_constructed : 1; + /** + * An instance has two possible value/holder layouts. + * + * Simple layout (when this flag is true), means the `simple_value_holder` is set with a pointer + * and the holder object governing that pointer, i.e. [val1*][holder]. This layout is applied + * whenever there is no python-side multiple inheritance of bound C++ types *and* the type's + * holder will fit in the default space (which is large enough to hold either a std::unique_ptr + * or std::shared_ptr). + * + * Non-simple layout applies when using custom holders that require more space than `shared_ptr` + * (which is typically the size of two pointers), or when multiple inheritance is used on the + * python side. Non-simple layout allocates the required amount of memory to have multiple + * bound C++ classes as parents. Under this layout, `nonsimple.values_and_holders` is set to a + * pointer to allocated space of the required space to hold a a sequence of value pointers and + * holders followed by a set of holder-constructed flags (1 byte each), i.e. + * [val1*][holder1][val2*][holder2]...[bb...] where each [block] is rounded up to a multiple of + * `sizeof(void *)`. `nonsimple.holder_constructed` is, for convenience, a pointer to the + * beginning of the [bb...] block (but not independently allocated). + */ + bool simple_layout : 1; + /// For simple layout, tracks whether the holder has been constructed + bool simple_holder_constructed : 1; + + /// Initializes all of the above type/values/holders data + void allocate_layout(); + + /// Destroys/deallocates all of the above + void deallocate_layout(); + + /// Returns the value_and_holder wrapper for the given type (or the first, if `find_type` + /// omitted) + value_and_holder get_value_and_holder(const type_info *find_type = nullptr); }; -/// PyObject wrapper around generic types, includes a special holder type that is responsible for lifetime management -template > struct instance : instance_essentials { - holder_type holder; -}; +static_assert(std::is_standard_layout::value, "Internal error: `pybind11::detail::instance` is not standard layout!"); struct overload_hash { inline size_t operator()(const std::pair& v) const { @@ -372,23 +428,20 @@ struct overload_hash { /// Internal data structure used to track registered instances and types struct internals { - std::unordered_map registered_types_cpp; // std::type_index -> type_info - std::unordered_map registered_types_py; // PyTypeObject* -> type_info - std::unordered_multimap registered_instances; // void * -> PyObject* + std::unordered_map registered_types_cpp; // std::type_index -> type_info + std::unordered_map> registered_types_py; // PyTypeObject* -> base type_info(s) + std::unordered_multimap registered_instances; // void * -> instance* std::unordered_set, overload_hash> inactive_overload_cache; std::unordered_map> direct_conversions; std::forward_list registered_exception_translators; std::unordered_map shared_data; // Custom data to be shared across extensions PyTypeObject *static_property_type; PyTypeObject *default_metaclass; - std::unordered_map bases; // one base type per `instance_size` (very few) + PyObject *instance_base; #if defined(WITH_THREAD) decltype(PyThread_create_key()) tstate = 0; // Usually an int but a long on Cygwin64 with Python 3.x PyInterpreterState *istate = nullptr; #endif - - /// Return the appropriate base type for the given instance size - PyObject *get_base(size_t instance_size); }; /// Return a reference to the current 'internals' information diff --git a/include/pybind11/pybind11.h b/include/pybind11/pybind11.h index 6d25fa57d..2bfd81cf3 100644 --- a/include/pybind11/pybind11.h +++ b/include/pybind11/pybind11.h @@ -293,7 +293,7 @@ protected: if (!chain) { /* No existing overload was found, create a new function object */ rec->def = new PyMethodDef(); - memset(rec->def, 0, sizeof(PyMethodDef)); + std::memset(rec->def, 0, sizeof(PyMethodDef)); rec->def->ml_name = rec->name; rec->def->ml_meth = reinterpret_cast(*dispatcher); rec->def->ml_flags = METH_VARARGS | METH_KEYWORDS; @@ -707,10 +707,8 @@ protected: return nullptr; } else { if (overloads->is_constructor) { - /* When a constructor ran successfully, the corresponding - holder type (e.g. std::unique_ptr) must still be initialized. */ - auto tinfo = get_type_info(Py_TYPE(parent.ptr())); - tinfo->init_holder(parent.ptr(), nullptr); + auto tinfo = get_type_info((PyTypeObject *) overloads->scope.ptr()); + tinfo->init_holder(reinterpret_cast(parent.ptr()), nullptr); } return result.ptr(); } @@ -727,7 +725,7 @@ public: if (!options::show_user_defined_docstrings()) doc = nullptr; #if PY_MAJOR_VERSION >= 3 PyModuleDef *def = new PyModuleDef(); - memset(def, 0, sizeof(PyModuleDef)); + std::memset(def, 0, sizeof(PyModuleDef)); def->m_name = name; def->m_doc = doc; def->m_size = -1; @@ -830,6 +828,7 @@ protected: tinfo->cpptype = rec.type; tinfo->type_size = rec.type_size; tinfo->operator_new = rec.operator_new; + tinfo->holder_size_in_ptrs = size_in_ptrs(rec.holder_size); tinfo->init_holder = rec.init_holder; tinfo->dealloc = rec.dealloc; tinfo->simple_type = true; @@ -840,7 +839,7 @@ protected: tinfo->direct_conversions = &internals.direct_conversions[tindex]; tinfo->default_holder = rec.default_holder; internals.registered_types_cpp[tindex] = tinfo; - internals.registered_types_py[m_ptr] = tinfo; + internals.registered_types_py[(PyTypeObject *) m_ptr] = { tinfo }; if (rec.bases.size() > 1 || rec.multiple_inheritance) { mark_parents_nonsimple(tinfo->type); @@ -923,7 +922,6 @@ public: using type_alias = detail::exactly_one_t; constexpr static bool has_alias = !std::is_void::value; using holder_type = detail::exactly_one_t, options...>; - using instance_type = detail::instance; static_assert(detail::all_of...>::value, "Unknown/invalid class_ template parameters provided"); @@ -947,7 +945,7 @@ public: record.name = name; record.type = &typeid(type); record.type_size = sizeof(conditional_t); - record.instance_size = sizeof(instance_type); + record.holder_size = sizeof(holder_type); record.init_holder = init_holder; record.dealloc = dealloc; record.default_holder = std::is_same>::value; @@ -1139,53 +1137,57 @@ public: private: /// Initialize holder object, variant 1: object derives from enable_shared_from_this template - static void init_holder_helper(instance_type *inst, const holder_type * /* unused */, const std::enable_shared_from_this * /* dummy */) { + static void init_holder_helper(detail::instance *inst, detail::value_and_holder &v_h, + const holder_type * /* unused */, const std::enable_shared_from_this * /* dummy */) { try { - auto sh = std::dynamic_pointer_cast(inst->value->shared_from_this()); + auto sh = std::dynamic_pointer_cast( + v_h.value_ptr()->shared_from_this()); if (sh) { - new (&inst->holder) holder_type(std::move(sh)); - inst->holder_constructed = true; + new (&v_h.holder()) holder_type(std::move(sh)); + v_h.set_holder_constructed(); } } catch (const std::bad_weak_ptr &) {} - if (!inst->holder_constructed && inst->owned) { - new (&inst->holder) holder_type(inst->value); - inst->holder_constructed = true; + + if (!v_h.holder_constructed() && inst->owned) { + new (&v_h.holder()) holder_type(v_h.value_ptr()); + v_h.set_holder_constructed(); } } - static void init_holder_from_existing(instance_type *inst, const holder_type *holder_ptr, - std::true_type /*is_copy_constructible*/) { - new (&inst->holder) holder_type(*holder_ptr); + static void init_holder_from_existing(const detail::value_and_holder &v_h, + const holder_type *holder_ptr, std::true_type /*is_copy_constructible*/) { + new (&v_h.holder()) holder_type(*reinterpret_cast(holder_ptr)); } - static void init_holder_from_existing(instance_type *inst, const holder_type *holder_ptr, - std::false_type /*is_copy_constructible*/) { - new (&inst->holder) holder_type(std::move(*const_cast(holder_ptr))); + static void init_holder_from_existing(const detail::value_and_holder &v_h, + const holder_type *holder_ptr, std::false_type /*is_copy_constructible*/) { + new (&v_h.holder()) holder_type(std::move(*const_cast(holder_ptr))); } /// Initialize holder object, variant 2: try to construct from existing holder object, if possible - static void init_holder_helper(instance_type *inst, const holder_type *holder_ptr, const void * /* dummy */) { + static void init_holder_helper(detail::instance *inst, detail::value_and_holder &v_h, + const holder_type *holder_ptr, const void * /* dummy -- not enable_shared_from_this) */) { if (holder_ptr) { - init_holder_from_existing(inst, holder_ptr, std::is_copy_constructible()); - inst->holder_constructed = true; + init_holder_from_existing(v_h, holder_ptr, std::is_copy_constructible()); + v_h.set_holder_constructed(); } else if (inst->owned || detail::always_construct_holder::value) { - new (&inst->holder) holder_type(inst->value); - inst->holder_constructed = true; + new (&v_h.holder()) holder_type(v_h.value_ptr()); + v_h.set_holder_constructed(); } } /// Initialize holder object of an instance, possibly given a pointer to an existing holder - static void init_holder(PyObject *inst_, const void *holder_ptr) { - auto inst = (instance_type *) inst_; - init_holder_helper(inst, (const holder_type *) holder_ptr, inst->value); + static void init_holder(detail::instance *inst, const void *holder_ptr) { + auto v_h = inst->get_value_and_holder(detail::get_type_info(typeid(type))); + init_holder_helper(inst, v_h, (const holder_type *) holder_ptr, v_h.value_ptr()); } - static void dealloc(PyObject *inst_) { - instance_type *inst = (instance_type *) inst_; - if (inst->holder_constructed) - inst->holder.~holder_type(); - else if (inst->owned) - detail::call_operator_delete(inst->value); + /// Deallocates an instance; via holder, if constructed; otherwise via operator delete. + static void dealloc(const detail::value_and_holder &v_h) { + if (v_h.holder_constructed()) + v_h.holder().~holder_type(); + else + detail::call_operator_delete(v_h.value_ptr()); } static detail::function_record *get_function_record(handle h) { @@ -1349,6 +1351,25 @@ PYBIND11_NOINLINE inline void keep_alive_impl(size_t Nurse, size_t Patient, func ); } +inline std::pair all_type_info_get_cache(PyTypeObject *type) { + auto res = get_internals().registered_types_py +#ifdef z__cpp_lib_unordered_map_try_emplace + .try_emplace(type); +#else + .emplace(type, std::vector()); +#endif + if (res.second) { + // New cache entry created; set up a weak reference to automatically remove it if the type + // gets destroyed: + weakref((PyObject *) type, cpp_function([type](handle wr) { + get_internals().registered_types_py.erase(type); + wr.dec_ref(); + })).release(); + } + + return res; +} + template struct iterator_state { Iterator it; diff --git a/tests/constructor_stats.h b/tests/constructor_stats.h index f66ff71df..babded032 100644 --- a/tests/constructor_stats.h +++ b/tests/constructor_stats.h @@ -169,7 +169,7 @@ public: auto &internals = py::detail::get_internals(); const std::type_index *t1 = nullptr, *t2 = nullptr; try { - auto *type_info = internals.registered_types_py.at(class_.ptr()); + auto *type_info = internals.registered_types_py.at((PyTypeObject *) class_.ptr()).at(0); for (auto &p : internals.registered_types_cpp) { if (p.second == type_info) { if (t1) { diff --git a/tests/test_multiple_inheritance.cpp b/tests/test_multiple_inheritance.cpp index 41576db41..2fbe11278 100644 --- a/tests/test_multiple_inheritance.cpp +++ b/tests/test_multiple_inheritance.cpp @@ -23,6 +23,11 @@ struct Base2 { int i; }; +template struct BaseN { + BaseN(int i) : i(i) { } + int i; +}; + struct Base12 : Base1, Base2 { Base12(int i, int j) : Base1(i), Base2(j) { } }; @@ -45,6 +50,15 @@ test_initializer multiple_inheritance([](py::module &m) { py::class_(m, "MIType") .def(py::init()); + // Many bases for testing that multiple inheritance from many classes (i.e. requiring extra + // space for holder constructed flags) works. + #define PYBIND11_BASEN(N) py::class_>(m, "BaseN" #N).def(py::init()).def("f" #N, [](BaseN &b) { return b.i + N; }) + PYBIND11_BASEN( 1); PYBIND11_BASEN( 2); PYBIND11_BASEN( 3); PYBIND11_BASEN( 4); + PYBIND11_BASEN( 5); PYBIND11_BASEN( 6); PYBIND11_BASEN( 7); PYBIND11_BASEN( 8); + PYBIND11_BASEN( 9); PYBIND11_BASEN(10); PYBIND11_BASEN(11); PYBIND11_BASEN(12); + PYBIND11_BASEN(13); PYBIND11_BASEN(14); PYBIND11_BASEN(15); PYBIND11_BASEN(16); + PYBIND11_BASEN(17); + // Uncommenting this should result in a compile time failure (MI can only be specified via // template parameters because pybind has to know the types involved; see discussion in #742 for // details). diff --git a/tests/test_multiple_inheritance.py b/tests/test_multiple_inheritance.py index 2d40d2565..3ed47adcc 100644 --- a/tests/test_multiple_inheritance.py +++ b/tests/test_multiple_inheritance.py @@ -53,15 +53,174 @@ def test_multiple_inheritance_mix2(): assert mt.bar() == 4 -def test_multiple_inheritance_error(): - """Inheriting from multiple C++ bases in Python is not supported""" +def test_multiple_inheritance_python(): from pybind11_tests import Base1, Base2 - with pytest.raises(TypeError) as excinfo: - # noinspection PyUnusedLocal - class MI(Base1, Base2): - pass - assert "Can't inherit from multiple C++ classes in Python" in str(excinfo.value) + class MI1(Base1, Base2): + def __init__(self, i, j): + Base1.__init__(self, i) + Base2.__init__(self, j) + + class B1(object): + def v(self): + return 1 + + class MI2(B1, Base1, Base2): + def __init__(self, i, j): + B1.__init__(self) + Base1.__init__(self, i) + Base2.__init__(self, j) + + class MI3(MI2): + def __init__(self, i, j): + MI2.__init__(self, i, j) + + class MI4(MI3, Base2): + def __init__(self, i, j, k): + MI2.__init__(self, j, k) + Base2.__init__(self, i) + + class MI5(Base2, B1, Base1): + def __init__(self, i, j): + B1.__init__(self) + Base1.__init__(self, i) + Base2.__init__(self, j) + + class MI6(Base2, B1): + def __init__(self, i): + Base2.__init__(self, i) + B1.__init__(self) + + class B2(B1): + def v(self): + return 2 + + class B3(object): + def v(self): + return 3 + + class B4(B3, B2): + def v(self): + return 4 + + class MI7(B4, MI6): + def __init__(self, i): + B4.__init__(self) + MI6.__init__(self, i) + + class MI8(MI6, B3): + def __init__(self, i): + MI6.__init__(self, i) + B3.__init__(self) + + class MI8b(B3, MI6): + def __init__(self, i): + B3.__init__(self) + MI6.__init__(self, i) + + mi1 = MI1(1, 2) + assert mi1.foo() == 1 + assert mi1.bar() == 2 + + mi2 = MI2(3, 4) + assert mi2.v() == 1 + assert mi2.foo() == 3 + assert mi2.bar() == 4 + + mi3 = MI3(5, 6) + assert mi3.v() == 1 + assert mi3.foo() == 5 + assert mi3.bar() == 6 + + mi4 = MI4(7, 8, 9) + assert mi4.v() == 1 + assert mi4.foo() == 8 + assert mi4.bar() == 7 + + mi5 = MI5(10, 11) + assert mi5.v() == 1 + assert mi5.foo() == 10 + assert mi5.bar() == 11 + + mi6 = MI6(12) + assert mi6.v() == 1 + assert mi6.bar() == 12 + + mi7 = MI7(13) + assert mi7.v() == 4 + assert mi7.bar() == 13 + + mi8 = MI8(14) + assert mi8.v() == 1 + assert mi8.bar() == 14 + + mi8b = MI8b(15) + assert mi8b.v() == 3 + assert mi8b.bar() == 15 + + +def test_multiple_inheritance_python_many_bases(): + from pybind11_tests import (BaseN1, BaseN2, BaseN3, BaseN4, BaseN5, BaseN6, BaseN7, + BaseN8, BaseN9, BaseN10, BaseN11, BaseN12, BaseN13, BaseN14, + BaseN15, BaseN16, BaseN17) + + class MIMany14(BaseN1, BaseN2, BaseN3, BaseN4): + def __init__(self): + BaseN1.__init__(self, 1) + BaseN2.__init__(self, 2) + BaseN3.__init__(self, 3) + BaseN4.__init__(self, 4) + + class MIMany58(BaseN5, BaseN6, BaseN7, BaseN8): + def __init__(self): + BaseN5.__init__(self, 5) + BaseN6.__init__(self, 6) + BaseN7.__init__(self, 7) + BaseN8.__init__(self, 8) + + class MIMany916(BaseN9, BaseN10, BaseN11, BaseN12, BaseN13, BaseN14, BaseN15, BaseN16): + def __init__(self): + BaseN9.__init__(self, 9) + BaseN10.__init__(self, 10) + BaseN11.__init__(self, 11) + BaseN12.__init__(self, 12) + BaseN13.__init__(self, 13) + BaseN14.__init__(self, 14) + BaseN15.__init__(self, 15) + BaseN16.__init__(self, 16) + + class MIMany19(MIMany14, MIMany58, BaseN9): + def __init__(self): + MIMany14.__init__(self) + MIMany58.__init__(self) + BaseN9.__init__(self, 9) + + class MIMany117(MIMany14, MIMany58, MIMany916, BaseN17): + def __init__(self): + MIMany14.__init__(self) + MIMany58.__init__(self) + MIMany916.__init__(self) + BaseN17.__init__(self, 17) + + # Inherits from 4 registered C++ classes: can fit in one pointer on any modern arch: + a = MIMany14() + for i in range(1, 4): + assert getattr(a, "f" + str(i))() == 2 * i + + # Inherits from 8: requires 1/2 pointers worth of holder flags on 32/64-bit arch: + b = MIMany916() + for i in range(9, 16): + assert getattr(b, "f" + str(i))() == 2 * i + + # Inherits from 9: requires >= 2 pointers worth of holder flags + c = MIMany19() + for i in range(1, 9): + assert getattr(c, "f" + str(i))() == 2 * i + + # Inherits from 17: requires >= 3 pointers worth of holder flags + d = MIMany117() + for i in range(1, 17): + assert getattr(d, "f" + str(i))() == 2 * i def test_multiple_inheritance_virtbase(): diff --git a/tests/test_smart_ptr.cpp b/tests/test_smart_ptr.cpp index 14b49d554..1ece3d19d 100644 --- a/tests/test_smart_ptr.cpp +++ b/tests/test_smart_ptr.cpp @@ -81,6 +81,28 @@ private: } }; +/// This is just a wrapper around unique_ptr, but with extra fields to deliberately bloat up the +/// holder size to trigger the non-simple-layout internal instance layout for single inheritance with +/// large holder type. +template class huge_unique_ptr { + std::unique_ptr ptr; + uint64_t padding[10]; +public: + huge_unique_ptr(T *p) : ptr(p) {}; + T *get() { return ptr.get(); } +}; + +class MyObject5 { // managed by huge_unique_ptr +public: + MyObject5(int value) : value{value} { + print_created(this); + } + int value; + ~MyObject5() { + print_destroyed(this); + } +}; + /// Make pybind aware of the ref-counted wrapper type (s) // ref is a wrapper for 'Object' which uses intrusive reference counting @@ -89,6 +111,7 @@ private: PYBIND11_DECLARE_HOLDER_TYPE(T, ref, true); PYBIND11_DECLARE_HOLDER_TYPE(T, std::shared_ptr); // Not required any more for std::shared_ptr, // but it should compile without error +PYBIND11_DECLARE_HOLDER_TYPE(T, huge_unique_ptr); // Make pybind11 aware of the non-standard getter member function namespace pybind11 { namespace detail { @@ -184,6 +207,10 @@ test_initializer smart_ptr([](py::module &m) { .def(py::init()) .def_readwrite("value", &MyObject4::value); + py::class_>(m, "MyObject5") + .def(py::init()) + .def_readwrite("value", &MyObject5::value); + py::implicitly_convertible(); // Expose constructor stats for the ref type diff --git a/tests/test_smart_ptr.py b/tests/test_smart_ptr.py index 4b1e20ea4..7396c9f77 100644 --- a/tests/test_smart_ptr.py +++ b/tests/test_smart_ptr.py @@ -132,6 +132,16 @@ def test_unique_nodelete(): assert cstats.alive() == 1 # Leak, but that's intentional +def test_large_holder(): + from pybind11_tests import MyObject5 + o = MyObject5(5) + assert o.value == 5 + cstats = ConstructorStats.get(MyObject5) + assert cstats.alive() == 1 + del o + assert cstats.alive() == 0 + + def test_shared_ptr_and_references(): from pybind11_tests.smart_ptr import SharedPtrRef, A