Moving tp_class access, and consistent fully-qualified naming for PyPy, to detail::get_tp_name

Author: YannickJadoul

include/pybind11/cast.h

@@ -39,6 +39,9 @@
 PYBIND11_NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
 PYBIND11_NAMESPACE_BEGIN(detail)
 
+// Forward-declaration; see detail/class.h
+std::string get_tp_name(PyTypeObject*);
+
 /// A life support system for temporary objects created by `type_caster::load()`.
 /// Adding a patient will keep it alive up until the enclosing function returns.
 class loader_life_support {
@@ -342,8 +345,8 @@ PYBIND11_NOINLINE inline value_and_holder instance::get_value_and_holder(const t
             "(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");
+            get_tp_name(find_type->type) + "' is not a pybind11 base of the given `" +
+            get_tp_name(Py_TYPE(this)) + "' instance");
 #endif
 }
 
@@ -1162,7 +1165,7 @@ template <> class type_caster<bool> {
         if (!src) return false;
         else if (src.ptr() == Py_True) { value = true; return true; }
         else if (src.ptr() == Py_False) { value = false; return true; }
-        else if (convert || !strcmp("numpy.bool_", Py_TYPE(src.ptr())->tp_name)) {
+        else if (convert || "numpy.bool_" == get_tp_name(Py_TYPE(src.ptr()))) {
             // (allow non-implicit conversion for numpy booleans)
 
             Py_ssize_t res = -1;

include/pybind11/detail/class.h

@@ -24,6 +24,14 @@ PYBIND11_NAMESPACE_BEGIN(detail)
 #  define PYBIND11_SET_OLDPY_QUALNAME(obj, nameobj) setattr((PyObject *) obj, "__qualname__", nameobj)
 #endif
 
+inline std::string get_tp_name(PyTypeObject *type) {
+#if !defined(PYPY_VERSION)
+    return type->tp_name;
+#else
+    return handle((PyObject *) type).attr("__module__").cast<std::string>() + "." + type->tp_name;
+#endif
+}
+
 inline PyTypeObject *type_incref(PyTypeObject *type) {
     Py_INCREF(type);
     return type;
@@ -172,7 +180,7 @@ extern "C" inline PyObject *pybind11_meta_call(PyObject *type, PyObject *args, P
     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);
+                         get_tp_name(vh.type->type).c_str());
             Py_DECREF(self);
             return nullptr;
         }
@@ -304,12 +312,7 @@ extern "C" inline PyObject *pybind11_object_new(PyTypeObject *type, PyObject *,
 /// following default function will be used which simply throws an exception.
 extern "C" inline int pybind11_object_init(PyObject *self, PyObject *, PyObject *) {
     PyTypeObject *type = Py_TYPE(self);
-    std::string msg;
-#if defined(PYPY_VERSION)
-    msg += handle((PyObject *) type).attr("__module__").cast<std::string>() + ".";
-#endif
-    msg += type->tp_name;
-    msg += ": No constructor defined!";
+    std::string msg = get_tp_name(type) + ": No constructor defined!";
     PyErr_SetString(PyExc_TypeError, msg.c_str());
     return -1;
 }
@@ -448,7 +451,7 @@ extern "C" inline PyObject *pybind11_get_dict(PyObject *self, void *) {
 extern "C" inline int pybind11_set_dict(PyObject *self, PyObject *new_dict, void *) {
     if (!PyDict_Check(new_dict)) {
         PyErr_Format(PyExc_TypeError, "__dict__ must be set to a dictionary, not a '%.200s'",
-                     Py_TYPE(new_dict)->tp_name);
+                     get_tp_name(Py_TYPE(new_dict)).c_str());
         return -1;
     }
     PyObject *&dict = *_PyObject_GetDictPtr(self);
@@ -476,9 +479,8 @@ extern "C" inline int pybind11_clear(PyObject *self) {
 inline void enable_dynamic_attributes(PyHeapTypeObject *heap_type) {
     auto type = &heap_type->ht_type;
 #if defined(PYPY_VERSION) && (PYPY_VERSION_NUM < 0x06000000)
-    pybind11_fail(std::string(type->tp_name) + ": dynamic attributes are "
-                                               "currently not supported in "
-                                               "conjunction with PyPy!");
+    pybind11_fail(get_tp_type(type) + ": dynamic attributes are currently not "
+                                      "supported in conjunction with PyPy!");
 #endif
     type->tp_flags |= Py_TPFLAGS_HAVE_GC;
     type->tp_dictoffset = type->tp_basicsize; // place dict at the end

include/pybind11/pybind11.h

@@ -242,7 +242,7 @@ class cpp_function : public function {
 
 #if !defined(NDEBUG) && !defined(PYBIND11_DISABLE_NEW_STYLE_INIT_WARNING)
         if (rec->is_constructor && !rec->is_new_style_constructor) {
-            const auto class_name = std::string(((PyTypeObject *) rec->scope.ptr())->tp_name);
+            const auto class_name = detail::get_tp_name((PyTypeObject *) rec->scope.ptr());
             const auto func_name = std::string(rec->name);
             PyErr_WarnEx(
                 PyExc_FutureWarning,
@@ -1033,7 +1033,7 @@ class generic_type : public object {
         if (!type->ht_type.tp_as_buffer)
             pybind11_fail(
                 "To be able to register buffer protocol support for the type '" +
-                std::string(tinfo->type->tp_name) +
+                get_tp_name(tinfo->type) +
                 "' the associated class<>(..) invocation must "
                 "include the pybind11::buffer_protocol() annotation!");
 

tests/test_class.py

@@ -152,10 +152,8 @@ def __init__(self):
             pass
     with pytest.raises(TypeError) as exc_info:
         Python()
-    expected = ["m.class_.Pet.__init__() must be called when overriding __init__",
-                "Pet.__init__() must be called when overriding __init__"]  # PyPy?
-    # TODO: fix PyPy error message wrt. tp_name/__qualname__?
-    assert msg(exc_info.value) in expected
+    expected = "m.class_.Pet.__init__() must be called when overriding __init__"
+    assert msg(exc_info.value) == expected
 
     # Multiple bases
     class RabbitHamster(m.Rabbit, m.Hamster):
@@ -164,9 +162,8 @@ def __init__(self):
 
     with pytest.raises(TypeError) as exc_info:
         RabbitHamster()
-    expected = ["m.class_.Hamster.__init__() must be called when overriding __init__",
-                "Hamster.__init__() must be called when overriding __init__"]  # PyPy
-    assert msg(exc_info.value) in expected
+    expected = "m.class_.Hamster.__init__() must be called when overriding __init__"
+    assert msg(exc_info.value) == expected
 
 
 def test_automatic_upcasting():

tests/test_local_bindings.py

@@ -155,7 +155,6 @@ def test_internal_locals_differ():
     assert m.local_cpp_types_addr() != cm.local_cpp_types_addr()
 
 
-@pytest.mark.xfail("env.PYPY")
 def test_stl_caster_vs_stl_bind(msg):
     """One module uses a generic vector caster from `<pybind11/stl.h>` while the other
     exports `std::vector<int>` via `py:bind_vector` and `py::module_local`"""