Streamline implementation and avoid unsafe reinterpret_cast<instance *>() introduced with PR #2152

The `reinterpret_cast<instance *>(self)` is unsafe if `__new__` is mocked,
which was actually found in the wild: the mock returned `None` for `self`.
This was inconsequential because `inst` is currently cast straight back to
`PyObject *` to compute `all_type_info()`, which is empty if `self` is not
a pybind11 `instance`, and then `inst` is never dereferenced. However, the
unsafe detour through `instance *` is easily avoided and the updated
implementation is less prone to accidents while debugging or refactoring.

Author: rwgk

include/pybind11/detail/class.h

@@ -180,17 +180,6 @@ 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) {
 
@@ -201,19 +190,15 @@ extern "C" inline PyObject *pybind11_meta_call(PyObject *type, PyObject *args, P
     }
 
     // Ensure that the base __init__ function(s) were called
-    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;
+    values_and_holders vhs(self);
     for (const auto &vh : vhs) {
-        if (!vh.holder_constructed() && !is_redundant_value_and_holder(bases, ix_base)) {
+        if (!vh.holder_constructed() && !vhs.is_redundant_value_and_holder(vh)) {
             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;

include/pybind11/detail/type_caster_base.h

@@ -336,6 +336,13 @@ struct values_and_holders {
     explicit values_and_holders(instance *inst)
         : inst{inst}, tinfo(all_type_info(Py_TYPE(inst))) {}
 
+    explicit values_and_holders(PyObject *obj)
+        : inst{nullptr}, tinfo(all_type_info(Py_TYPE(obj))) {
+        if (!tinfo.empty()) {
+            inst = reinterpret_cast<instance *>(obj);
+        }
+    }
+
     struct iterator {
     private:
         instance *inst = nullptr;
@@ -378,6 +385,16 @@ struct values_and_holders {
     }
 
     size_t size() { return tinfo.size(); }
+
+    // Band-aid workaround to fix a subtle but serious bug in a minimalistic fashion. See PR #4762.
+    bool is_redundant_value_and_holder(const value_and_holder &vh) {
+        for (size_t i = 0; i < vh.index; i++) {
+            if (PyType_IsSubtype(tinfo[i]->type, tinfo[vh.index]->type) != 0) {
+                return true;
+            }
+        }
+        return false;
+    }
 };
 
 /**