Skip to content

Commit

Permalink
Snapshot of pybind/pybind11#4762 applied to pywrapcc
Browse files Browse the repository at this point in the history
  • Loading branch information
Ralf W. Grosse-Kunstleve committed Jul 28, 2023
1 parent 4b612e4 commit 095d66b
Show file tree
Hide file tree
Showing 7 changed files with 130 additions and 8 deletions.
8 changes: 7 additions & 1 deletion include/pybind11/cast.h
Original file line number Diff line number Diff line change
Expand Up @@ -1618,7 +1618,13 @@ inline namespace literals {
/** \rst
String literal version of `arg`
\endrst */
constexpr detail::arg_base operator"" _a(const char *name, size_t) {
constexpr detail::arg_base
#if !defined(__clang__) && defined(__GNUC__) && __GNUC__ < 5
operator"" _a // gcc 4.8.5 insists on having a space (hard error).
#else
operator""_a // clang 17 generates a deprecation warning if there is a space.
#endif
(const char *name, size_t) {
return detail::arg_base(name);
}
} // namespace literals
Expand Down
23 changes: 18 additions & 5 deletions include/pybind11/detail/class.h
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,17 @@ extern "C" inline PyObject *pybind11_meta_getattro(PyObject *obj, PyObject *name
return PyType_Type.tp_getattro(obj, name);
}

// Band-aid workaround to fix a subtle but serious bug in a minimalistic fashion. See PR #4762.
inline bool is_redundant_value_and_holder(const std::vector<type_info *> &bases,
std::size_t ix_base) {
for (std::size_t i = 0; i < ix_base; i++) {
if (PyType_IsSubtype(bases[i]->type, bases[ix_base]->type) != 0) {
return true;
}
}
return false;
}

/// metaclass `__call__` function that is used to create all pybind11 objects.
extern "C" inline PyObject *pybind11_meta_call(PyObject *type, PyObject *args, PyObject *kwargs) {

Expand All @@ -189,18 +200,20 @@ extern "C" inline PyObject *pybind11_meta_call(PyObject *type, PyObject *args, P
return nullptr;
}

// This must be a pybind11 instance
auto *instance = reinterpret_cast<detail::instance *>(self);

// Ensure that the base __init__ function(s) were called
for (const auto &vh : values_and_holders(instance)) {
if (!vh.holder_constructed()) {
const auto &bases = all_type_info((PyTypeObject *) type);
values_and_holders vhs(reinterpret_cast<detail::instance *>(self));
assert(bases.size() == vhs.size());
std::size_t ix_base = 0;
for (const auto &vh : vhs) {
if (!vh.holder_constructed() && !is_redundant_value_and_holder(bases, ix_base)) {
PyErr_Format(PyExc_TypeError,
"%.200s.__init__() must be called when overriding __init__",
get_fully_qualified_tp_name(vh.type->type).c_str());
Py_DECREF(self);
return nullptr;
}
ix_base++;
}

return self;
Expand Down
16 changes: 15 additions & 1 deletion include/pybind11/detail/type_caster_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,22 @@ class loader_life_support {
inline std::pair<decltype(internals::registered_types_py)::iterator, bool>
all_type_info_get_cache(PyTypeObject *type);

// Band-aid workaround to fix a subtle but serious bug in a minimalistic fashion. See PR #4762.
inline void all_type_info_add_base_most_derived_first(std::vector<type_info *> &bases,
type_info *addl_base) {
for (auto it = bases.begin(); it != bases.end(); it++) {
type_info *existing_base = *it;
if (PyType_IsSubtype(addl_base->type, existing_base->type) != 0) {
bases.insert(it, addl_base);
return;
}
}
bases.push_back(addl_base);
}

// Populates a just-created cache entry.
PYBIND11_NOINLINE void all_type_info_populate(PyTypeObject *t, std::vector<type_info *> &bases) {
assert(bases.empty());
std::vector<PyTypeObject *> check;
for (handle parent : reinterpret_borrow<tuple>(t->tp_bases)) {
check.push_back((PyTypeObject *) parent.ptr());
Expand Down Expand Up @@ -136,7 +150,7 @@ PYBIND11_NOINLINE void all_type_info_populate(PyTypeObject *t, std::vector<type_
}
}
if (!found) {
bases.push_back(tinfo);
all_type_info_add_base_most_derived_first(bases, tinfo);
}
}
} else if (type->tp_bases) {
Expand Down
10 changes: 9 additions & 1 deletion include/pybind11/pytypes.h
Original file line number Diff line number Diff line change
Expand Up @@ -1620,7 +1620,15 @@ inline namespace literals {
/** \rst
String literal version of `str`
\endrst */
inline str operator"" _s(const char *s, size_t size) { return {s, size}; }
inline str
#if !defined(__clang__) && defined(__GNUC__) && __GNUC__ < 5
operator"" _s // gcc 4.8.5 insists on having a space (hard error).
#else
operator""_s // clang 17 generates a deprecation warning if there is a space.
#endif
(const char *s, size_t size) {
return {s, size};
}
} // namespace literals

/// \addtogroup pytypes
Expand Down
1 change: 1 addition & 0 deletions tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,7 @@ set(PYBIND11_TEST_FILES
test_opaque_types
test_operator_overloading
test_pickling
test_python_multiple_inheritance
test_pytypes
test_return_value_policy_override
test_return_value_policy_pack
Expand Down
45 changes: 45 additions & 0 deletions tests/test_python_multiple_inheritance.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#include "pybind11_tests.h"

namespace test_python_multiple_inheritance {

// Copied from:
// https://github.com/google/clif/blob/5718e4d0807fd3b6a8187dde140069120b81ecef/clif/testing/python_multiple_inheritance.h

struct CppBase {
explicit CppBase(int value) : base_value(value) {}
int get_base_value() const { return base_value; }
void reset_base_value(int new_value) { base_value = new_value; }

private:
int base_value;
};

struct CppDrvd : CppBase {
explicit CppDrvd(int value) : CppBase(value), drvd_value(value * 3) {}
int get_drvd_value() const { return drvd_value; }
void reset_drvd_value(int new_value) { drvd_value = new_value; }

int get_base_value_from_drvd() const { return get_base_value(); }
void reset_base_value_from_drvd(int new_value) { reset_base_value(new_value); }

private:
int drvd_value;
};

} // namespace test_python_multiple_inheritance

TEST_SUBMODULE(python_multiple_inheritance, m) {
using namespace test_python_multiple_inheritance;

py::class_<CppBase>(m, "CppBase")
.def(py::init<int>())
.def("get_base_value", &CppBase::get_base_value)
.def("reset_base_value", &CppBase::reset_base_value);

py::class_<CppDrvd, CppBase>(m, "CppDrvd")
.def(py::init<int>())
.def("get_drvd_value", &CppDrvd::get_drvd_value)
.def("reset_drvd_value", &CppDrvd::reset_drvd_value)
.def("get_base_value_from_drvd", &CppDrvd::get_base_value_from_drvd)
.def("reset_base_value_from_drvd", &CppDrvd::reset_base_value_from_drvd);
}
35 changes: 35 additions & 0 deletions tests/test_python_multiple_inheritance.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Adapted from:
# https://github.com/google/clif/blob/5718e4d0807fd3b6a8187dde140069120b81ecef/clif/testing/python/python_multiple_inheritance_test.py

from pybind11_tests import python_multiple_inheritance as m


class PC(m.CppBase):
pass


class PPCC(PC, m.CppDrvd):
pass


def test_PC():
d = PC(11)
assert d.get_base_value() == 11
d.reset_base_value(13)
assert d.get_base_value() == 13


def test_PPCC():
d = PPCC(11)
assert d.get_drvd_value() == 33
d.reset_drvd_value(55)
assert d.get_drvd_value() == 55

assert d.get_base_value() == 11
assert d.get_base_value_from_drvd() == 11
d.reset_base_value(20)
assert d.get_base_value() == 20
assert d.get_base_value_from_drvd() == 20
d.reset_base_value_from_drvd(30)
assert d.get_base_value() == 30
assert d.get_base_value_from_drvd() == 30

0 comments on commit 095d66b

Please sign in to comment.