Update to pymetabind 0.3

Author: oremanj

include/nanobind/nb_lib.h

@@ -286,7 +286,13 @@ NB_CORE PyObject *nb_type_new(const type_init_data *c) noexcept;
 NB_CORE bool nb_type_get(const std::type_info *t, PyObject *o, uint8_t flags,
                          cleanup_list *cleanup, void **out) noexcept;
 
-/// Cast a C++ type instance into a Python object
+/// Cast a C++ type instance into a Python object.
+///
+/// Note: If you call this as if casting a shared_ptr (non-null `is_new`, true
+/// `allow_foreign`, and `rv_policy::reference`) and you obtain a result with
+/// `*is_new == true`, you must call `keep_alive` on the new instance after
+/// you get it back. This allows for better interoperability with other
+/// frameworks that store a smart pointer inside the instance.
 NB_CORE PyObject *nb_type_put(const std::type_info *cpp_type, void *value,
                               rv_policy rvp, cleanup_list *cleanup,
                               bool *is_new = nullptr,

src/nb_foreign.cpp

@@ -80,8 +80,10 @@ static void *nb_foreign_from_python(pymb_binding *binding,
 static PyObject *nb_foreign_to_python(pymb_binding *binding,
                                       void *cobj,
                                       enum pymb_rv_policy rvp_,
-                                      PyObject *parent) noexcept {
-    cleanup_list cleanup{parent};
+                                      pymb_to_python_feedback *feedback) noexcept {
+    feedback->relocate = 0; // we don't support relocation
+    feedback->is_new = 0; // unless overridden below
+
     auto *td = (type_data *) binding->context;
     if (td->align == 0) { // enum
         int64_t key;
@@ -102,15 +104,25 @@ static PyObject *nb_foreign_to_python(pymb_binding *binding,
         return enum_from_cpp(td->type, key, td->size);
     }
 
-    rv_policy rvp = (rv_policy) rvp_;
-    if (rvp < rv_policy::take_ownership || rvp > rv_policy::none) {
-        // Future-proofing in case additional pymb_rv_policies are defined
-        // later: if we don't recognize this policy, then refuse the cast
-        // unless a pyobject wrapper already exists.
-        rvp = rv_policy::none;
+    rv_policy rvp = rv_policy::none; // conservative default if rvp unrecognized
+    switch (rvp_) {
+        case pymb_rv_policy_take_ownership:
+        case pymb_rv_policy_copy:
+        case pymb_rv_policy_move:
+        case pymb_rv_policy_reference:
+        case pymb_rv_policy_none:
+            // These have the same values and semantics as our own policies
+            rvp = (rv_policy) rvp_;
+            break;
+        case pymb_rv_policy_share_ownership:
+            // Shared ownership is always held by keep_alive in nanobind,
+            // so there's nothing special to do here.
+            rvp = rv_policy::reference;
+            break;
     }
-    return nb_type_put(td->type, cobj, rvp, &cleanup,
-                       /* is_new */ nullptr, /* allow_foreign */ false);
+    return nb_type_put(td->type, cobj, rvp, /* cleanup */ nullptr,
+                       /* is_new */ (bool *) &feedback->is_new,
+                       /* allow_foreign */ false);
 }
 
 static int nb_foreign_keep_alive(PyObject *nurse,
@@ -121,10 +133,10 @@ static int nb_foreign_keep_alive(PyObject *nurse,
             keep_alive(nurse, payload, (void (*)(void*) noexcept) cb);
         else
             keep_alive(nurse, (PyObject *) payload);
-        return 0;
+        return 1;
     } catch (const std::runtime_error& err) {
-        PyErr_SetString(PyExc_RuntimeError, err.what());
-        return -1;
+        PyErr_WriteUnraisable(nurse);
+        return 0;
     }
 }
 
@@ -388,14 +400,14 @@ void nb_type_import_impl(PyObject *pytype, const std::type_info *cpptype) {
         register_with_pymetabind(internals);
     pymb_framework* foreign_self = internals->foreign_self;
     pymb_binding* binding = pymb_get_binding(pytype);
-#if defined(Py_LIMITED_API)
-    str name_py = steal<str>(PyType_GetName((PyTypeObject *) pytype));
+    str name_py = steal<str>(nb_type_name(pytype));
     const char *name = name_py.c_str();
-#else
-    const char *name = ((PyTypeObject *) pytype)->tp_name;
-#endif
     if (!binding)
         raise("'%s' does not define a __pymetabind_binding__", name);
+    if (binding->pytype != (PyTypeObject *) pytype)
+        raise("The binding defined by '%s' is for a different type '%s', "
+              "likely a superclass; import that type instead",
+              name, steal<str>(nb_type_name((PyObject *) binding->pytype)).c_str());
     if (binding->framework == foreign_self)
         raise("'%s' is already bound by this nanobind domain", name);
     if (!cpptype) {

src/nb_type.cpp

@@ -1797,22 +1797,18 @@ void keep_alive(PyObject *nurse, PyObject *patient) {
         Py_INCREF(patient);
         ((nb_inst *) nurse)->clear_keep_alive = true;
     } else {
+#if !defined(NB_DISABLE_INTEROP)
+        if (pymb_binding *binding = pymb_get_binding(nurse);
+            binding && binding->framework->keep_alive(nurse, patient, nullptr))
+            return;
+#endif
         PyObject *callback =
             PyCFunction_New(&keep_alive_callback_def, patient);
 
         PyObject *weakref = PyWeakref_NewRef(nurse, callback);
         if (!weakref) {
             Py_DECREF(callback);
             PyErr_Clear();
-#if !defined(NB_DISABLE_INTEROP)
-            if (pymb_binding *binding = pymb_get_binding(nurse)) {
-                // Try a foreign framework's keep_alive as a last resort
-                if (binding->framework->keep_alive(nurse, patient,
-                                                   nullptr) == 0)
-                    return;
-                raise_python_error();
-            }
-#endif
             raise("nanobind::detail::keep_alive(): could not create a weak "
                   "reference! Likely, the 'nurse' argument you specified is not "
                   "a weak-referenceable type!");
@@ -1830,7 +1826,8 @@ void keep_alive(PyObject *nurse, void *payload,
                 void (*callback)(void *) noexcept) noexcept {
     check(nurse, "nanobind::detail::keep_alive(): 'nurse' is undefined!");
 
-    if (nb_type_check((PyObject *) Py_TYPE(nurse))) {
+    PyObject *nurse_ty = (PyObject *) Py_TYPE(nurse);
+    if (nb_type_check(nurse_ty)) {
 #if defined(NB_FREE_THREADED)
         nb_shard &shard = internals->shard(inst_ptr((nb_inst *) nurse));
         lock_shard guard(shard);
@@ -1850,6 +1847,11 @@ void keep_alive(PyObject *nurse, void *payload,
 
         ((nb_inst *) nurse)->clear_keep_alive = true;
     } else {
+#if !defined(NB_DISABLE_INTEROP)
+        if (pymb_binding *binding = pymb_get_binding(nurse_ty);
+            binding && binding->framework->keep_alive(nurse, payload, callback))
+            return;
+#endif
         PyObject *patient = capsule_new(payload, nullptr, callback);
         keep_alive(nurse, patient);
         Py_DECREF(patient);
@@ -1957,36 +1959,64 @@ PyObject *nb_type_put_foreign(nb_internals *internals_,
                               bool *is_new) noexcept {
     struct capture {
         void *value;
-        rv_policy rvp;
-        PyObject *parent;
-        bool check_new;
-        bool is_new = false;
-    } cap{value, rvp, cleanup ? cleanup->self() : nullptr, bool(is_new)};
+        pymb_rv_policy rvp = pymb_rv_policy_none; // conservative default
+        struct pymb_to_python_feedback feedback{};
+        struct pymb_framework *used_framework = nullptr;
+    } cap{value};
+
+    switch (rvp) {
+        case rv_policy::reference_internal:
+            if (!cleanup || !cleanup->self())
+                return nullptr;
+            cap.rvp = pymb_rv_policy_share_ownership;
+            break;
+        case rv_policy::reference:
+            if (is_new) {
+                cap.rvp = pymb_rv_policy_share_ownership;
+                break;
+            }
+            [[fallthrough]];
+        case rv_policy::take_ownership:
+        case rv_policy::copy:
+        case rv_policy::move:
+        case rv_policy::none:
+            cap.rvp = (pymb_rv_policy) rvp;
+            break;
+        case rv_policy::automatic:
+        case rv_policy::automatic_reference:
+            check(false,
+                  "nb_type_put_foreign(): automatic rvp should have been "
+                  "converted to a different one before reaching here!");
+            break;
+    }
 
     auto attempt = +[](void *closure, pymb_binding *binding) -> void* {
         capture &cap = *(capture *) closure;
-        if (cap.check_new || cap.rvp == rv_policy::none) {
-            PyObject* existing = binding->framework->to_python(
-                    binding, cap.value, pymb_rv_policy_none, nullptr);
-            if (existing || cap.rvp == rv_policy::none) {
-                cap.is_new = false;
-                return existing;
-            }
-            cap.is_new = true;
-        }
+        cap.used_framework = binding->framework;
         return binding->framework->to_python(
-                binding, cap.value, (pymb_rv_policy) (uint8_t) cap.rvp,
-                cap.parent);
+                binding, cap.value, cap.rvp, &cap.feedback);
     };
 
-    void *result = nullptr;
+    void *result_v = nullptr;
     if (cpp_type_p && cpp_type_p != cpp_type)
-        result = nb_type_try_foreign(internals_, cpp_type_p, attempt, &cap);
-    if (!result)
-        result = nb_type_try_foreign(internals_, cpp_type, attempt, &cap);
+        result_v = nb_type_try_foreign(internals_, cpp_type_p, attempt, &cap);
+    if (!result_v)
+        result_v = nb_type_try_foreign(internals_, cpp_type, attempt, &cap);
+
+    PyObject *result = (PyObject *) result_v;
     if (is_new)
-        *is_new = cap.is_new;
-    return (PyObject *) result;
+        *is_new = cap.feedback.is_new;
+    if (result && rvp == rv_policy::reference_internal && cap.feedback.is_new &&
+        !cap.used_framework->keep_alive(result, cleanup->self(), nullptr)) {
+        try {
+            keep_alive(result, cleanup->self());
+        } catch (python_error& exc) {
+            exc.restore();
+            Py_DECREF(result);
+            return nullptr;
+        }
+    }
+    return result;
 }
 #endif