__qualname__ and nested class naming fixes (#1171)

A few fixes related to how we set `__qualname__` and how we show the
type name in function signatures:

- `__qualname__` isn't supposed to have the module name at the
beginning, but we've been putting it there.  This removes it, while
keeping the `Nested.Class` name chaining.

- print `__module__.__qualname__` rather than `type->tp_name`; the
latter doesn't work properly for nested classes, so we would get
`module.B` rather than `module.A.B` for a class `B` with parent `A`.
This also unifies the Python 3 and PyPy code.  Fixes #1166.

- This now sets a `__qualname__` attribute on the type (as would happen
in Python 3.3+) for Python <3.3, including PyPy.  While not particularly
important to have in earlier Python versions, it's useful for us to be
able to extracted the nested name, which is why `__qualname__` was
invented in the first place.

- Added tests for the above.

Author: jagerman

include/pybind11/detail/class.h

@@ -14,6 +14,15 @@
 NAMESPACE_BEGIN(PYBIND11_NAMESPACE)
 NAMESPACE_BEGIN(detail)
 
+#if PY_VERSION_HEX >= 0x03030000
+#  define PYBIND11_BUILTIN_QUALNAME
+#  define PYBIND11_SET_OLDPY_QUALNAME(obj, nameobj)
+#else
+// In pre-3.3 Python, we still set __qualname__ so that we can produce reliable function type
+// signatures; in 3.3+ this macro expands to nothing:
+#  define PYBIND11_SET_OLDPY_QUALNAME(obj, nameobj) setattr((PyObject *) obj, "__qualname__", nameobj)
+#endif
+
 inline PyTypeObject *type_incref(PyTypeObject *type) {
     Py_INCREF(type);
     return type;
@@ -48,7 +57,7 @@ inline PyTypeObject *make_static_property_type() {
         pybind11_fail("make_static_property_type(): error allocating type!");
 
     heap_type->ht_name = name_obj.inc_ref().ptr();
-#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
+#ifdef PYBIND11_BUILTIN_QUALNAME
     heap_type->ht_qualname = name_obj.inc_ref().ptr();
 #endif
 
@@ -63,6 +72,7 @@ inline PyTypeObject *make_static_property_type() {
         pybind11_fail("make_static_property_type(): failure in PyType_Ready()!");
 
     setattr((PyObject *) type, "__module__", str("pybind11_builtins"));
+    PYBIND11_SET_OLDPY_QUALNAME(type, name_obj);
 
     return type;
 }
@@ -161,7 +171,7 @@ inline PyTypeObject* make_default_metaclass() {
         pybind11_fail("make_default_metaclass(): error allocating metaclass!");
 
     heap_type->ht_name = name_obj.inc_ref().ptr();
-#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
+#ifdef PYBIND11_BUILTIN_QUALNAME
     heap_type->ht_qualname = name_obj.inc_ref().ptr();
 #endif
 
@@ -179,6 +189,7 @@ inline PyTypeObject* make_default_metaclass() {
         pybind11_fail("make_default_metaclass(): failure in PyType_Ready()!");
 
     setattr((PyObject *) type, "__module__", str("pybind11_builtins"));
+    PYBIND11_SET_OLDPY_QUALNAME(type, name_obj);
 
     return type;
 }
@@ -363,7 +374,7 @@ inline PyObject *make_object_base_type(PyTypeObject *metaclass) {
         pybind11_fail("make_object_base_type(): error allocating type!");
 
     heap_type->ht_name = name_obj.inc_ref().ptr();
-#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
+#ifdef PYBIND11_BUILTIN_QUALNAME
     heap_type->ht_qualname = name_obj.inc_ref().ptr();
 #endif
 
@@ -384,6 +395,7 @@ inline PyObject *make_object_base_type(PyTypeObject *metaclass) {
         pybind11_fail("PyType_Ready failed in make_object_base_type():" + error_string());
 
     setattr((PyObject *) type, "__module__", str("pybind11_builtins"));
+    PYBIND11_SET_OLDPY_QUALNAME(type, name_obj);
 
     assert(!PyType_HasFeature(type, Py_TPFLAGS_HAVE_GC));
     return (PyObject *) heap_type;
@@ -504,13 +516,15 @@ inline void enable_buffer_protocol(PyHeapTypeObject *heap_type) {
 inline PyObject* make_new_python_type(const type_record &rec) {
     auto name = reinterpret_steal<object>(PYBIND11_FROM_STRING(rec.name));
 
-#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
-    auto ht_qualname = name;
-    if (rec.scope && hasattr(rec.scope, "__qualname__")) {
-        ht_qualname = reinterpret_steal<object>(
+    auto qualname = name;
+    if (rec.scope && !PyModule_Check(rec.scope.ptr()) && hasattr(rec.scope, "__qualname__")) {
+#if PY_MAJOR_VERSION >= 3
+        qualname = reinterpret_steal<object>(
             PyUnicode_FromFormat("%U.%U", rec.scope.attr("__qualname__").ptr(), name.ptr()));
-    }
+#else
+        qualname = str(rec.scope.attr("__qualname__").cast<std::string>() + "." + rec.name);
 #endif
+    }
 
     object module;
     if (rec.scope) {
@@ -552,8 +566,8 @@ inline PyObject* make_new_python_type(const type_record &rec) {
         pybind11_fail(std::string(rec.name) + ": Unable to create type object!");
 
     heap_type->ht_name = name.release().ptr();
-#if PY_MAJOR_VERSION >= 3 && PY_MINOR_VERSION >= 3
-    heap_type->ht_qualname = ht_qualname.release().ptr();
+#ifdef PYBIND11_BUILTIN_QUALNAME
+    heap_type->ht_qualname = qualname.inc_ref().ptr();
 #endif
 
     auto type = &heap_type->ht_type;
@@ -599,6 +613,8 @@ inline PyObject* make_new_python_type(const type_record &rec) {
     if (module) // Needed by pydoc
         setattr((PyObject *) type, "__module__", module);
 
+    PYBIND11_SET_OLDPY_QUALNAME(type, qualname);
+
     return (PyObject *) type;
 }
 

include/pybind11/pybind11.h

@@ -246,19 +246,16 @@ class cpp_function : public function {
                 if (!t)
                     pybind11_fail("Internal error while parsing type signature (1)");
                 if (auto tinfo = detail::get_type_info(*t)) {
-#if defined(PYPY_VERSION)
-                    signature += handle((PyObject *) tinfo->type)
-                                     .attr("__module__")
-                                     .cast<std::string>() + ".";
-#endif
-                    signature += tinfo->type->tp_name;
+                    handle th((PyObject *) tinfo->type);
+                    signature +=
+                        th.attr("__module__").cast<std::string>() + "." +
+                        th.attr("__qualname__").cast<std::string>(); // Python 3.3+, but we backport it to earlier versions
                 } else if (rec->is_new_style_constructor && arg_index == 0) {
                     // A new-style `__init__` takes `self` as `value_and_holder`.
                     // Rewrite it to the proper class type.
-#if defined(PYPY_VERSION)
-                    signature += rec->scope.attr("__module__").cast<std::string>() + ".";
-#endif
-                    signature += ((PyTypeObject *) rec->scope.ptr())->tp_name;
+                    signature +=
+                        rec->scope.attr("__module__").cast<std::string>() + "." +
+                        rec->scope.attr("__qualname__").cast<std::string>();
                 } else {
                     std::string tname(t->name());
                     detail::clean_type_id(tname);

tests/test_class.cpp

@@ -302,6 +302,21 @@ TEST_SUBMODULE(class_, m) {
         .def(py::init<const BogusImplicitConversion &>());
 
     py::implicitly_convertible<int, BogusImplicitConversion>();
+
+    // test_qualname
+    // #1166: nested class docstring doesn't show nested name
+    // Also related: tests that __qualname__ is set properly
+    struct NestBase {};
+    struct Nested {};
+    py::class_<NestBase> base(m, "NestBase");
+    base.def(py::init<>());
+    py::class_<Nested>(base, "Nested")
+        .def(py::init<>())
+        .def("fn", [](Nested &, int, NestBase &, Nested &) {})
+        .def("fa", [](Nested &, int, NestBase &, Nested &) {},
+                "a"_a, "b"_a, "c"_a);
+    base.def("g", [](NestBase &, Nested &) {});
+    base.def("h", []() { return NestBase(); });
 }
 
 template <int N> class BreaksBase { public: virtual ~BreaksBase() = default; };

tests/test_class.py

@@ -44,6 +44,31 @@ def test_docstrings(doc):
     """
 
 
+def test_qualname(doc):
+    """Tests that a properly qualified name is set in __qualname__ (even in pre-3.3, where we
+    backport the attribute) and that generated docstrings properly use it and the module name"""
+    assert m.NestBase.__qualname__ == "NestBase"
+    assert m.NestBase.Nested.__qualname__ == "NestBase.Nested"
+
+    assert doc(m.NestBase.__init__) == """
+        __init__(self: m.class_.NestBase) -> None
+    """
+    assert doc(m.NestBase.g) == """
+        g(self: m.class_.NestBase, arg0: m.class_.NestBase.Nested) -> None
+    """
+    assert doc(m.NestBase.Nested.__init__) == """
+        __init__(self: m.class_.NestBase.Nested) -> None
+    """
+    assert doc(m.NestBase.Nested.fn) == """
+        fn(self: m.class_.NestBase.Nested, arg0: int, arg1: m.class_.NestBase, arg2: m.class_.NestBase.Nested) -> None
+    """  # noqa: E501 line too long
+    assert doc(m.NestBase.Nested.fa) == """
+        fa(self: m.class_.NestBase.Nested, a: int, b: m.class_.NestBase, c: m.class_.NestBase.Nested) -> None
+    """  # noqa: E501 line too long
+    assert m.NestBase.__module__ == "pybind11_tests.class_"
+    assert m.NestBase.Nested.__module__ == "pybind11_tests.class_"
+
+
 def test_inheritance(msg):
     roger = m.Rabbit('Rabbit')
     assert roger.name() + " is a " + roger.species() == "Rabbit is a parrot"
@@ -229,7 +254,9 @@ def test_reentrant_implicit_conversion_failure(msg):
     # ensure that there is no runaway reentrant implicit conversion (#1035)
     with pytest.raises(TypeError) as excinfo:
         m.BogusImplicitConversion(0)
-    assert msg(excinfo.value) == '''__init__(): incompatible constructor arguments. The following argument types are supported:
-    1. m.class_.BogusImplicitConversion(arg0: m.class_.BogusImplicitConversion)
+    assert msg(excinfo.value) == '''
+        __init__(): incompatible constructor arguments. The following argument types are supported:
+            1. m.class_.BogusImplicitConversion(arg0: m.class_.BogusImplicitConversion)
 
-Invoked with: 0'''
+        Invoked with: 0
+    '''