workaround to assert introduced in #549

[francesco-ballarin]

Hi,
in dolfinx we are interfacing objects from a library called PETSc (written in C), its Cython wrapper petsc4py, and nanobind.

To do so we prepared our own caster at
https://github.com/FEniCS/dolfinx/blob/main/python/dolfinx/wrappers/caster_petsc.h
In there, the types Vec and Mat are part of the external library PETSc, and are pointers to private data structures _p_Vec and _p_Mat.

Since nanobind 2.0, we are hitting the assert at

nanobind/include/nanobind/nb_cast.h

Line 126 in c36584e

static_assert(!is_ref || std::is_same_v<T, const char*>,

when using nanobind-wrapped types in a std::array<..., 2> (but I guess it would be the same in other STL containers).

To my understanding, the assert is saying that in order to use STL containers with Vec (or Mat), we also need to wrap the corresponding non-pointer type _p_Vec (or _p_Mat). However, we definitely do NOT want to do that, because _p_Vec/_p_Mat are part of PETSc private interface, and we don't want to fiddle with that. We are not concerned about memory management for the pointed object, because that's done by petsc4py.

In this case how should we work around the assert introduced in #549?
I am aware that there is a description of the changes in the changelog at the top of https://github.com/wjakob/nanobind/pull/549/files , but I couldn't get from there what to do in this case.

Thanks,
Francesco

[wjakob]

It would be helpful if you can post a more minimal example that reproduces the regression (in case it is one), it would take too much time time to really understand the PETSc code.

Let me also CC @oremanj, who added this assertion check.

[oremanj]

My understanding: you have a type Vec, which is secretly a typedef for _p_Vec*, but it has value semantics: you want to treat Vec as an opaque type and not worry about the fact that it's implemented using a pointer. When you unwrap a Python object that stores one of these things, you get a Vec whose underlying pointer is stable, i.e., points into something owned by the Python object.

C++ metaprogramming can't see typedefs; whenever you write Vec, nanobind sees _p_Vec*. (If Vec were a structure wrapping a pointer, instead of a pointer typedef, you wouldn't have the problem you're having.) nanobind then treats the cast to _p_Vec* in the same way it would a cast to int*; it assumes the custom type caster is going to give you a value of underlying type, such as int, to which it can't safely store a pointer in a container, because the pointer would point to memory inside the type caster that will be deallocated before you have a chance to access that container.

It's a little tricky for nanobind to figure out what type of value the caster is actually storing. In the example you provided, the type you pass to NB_TYPE_CASTER is different than the template argument to type_caster, so the type caster is making a Vec (not a _p_Vec), so accessing that Vec should be safe in all cases - the pointer points to storage owned by the Python object you're casting from, not to storage owned by the type caster object (which would be destroyed before you can use the container you're casting to).

Here's a workaround that I think will solve your problem without getting too complicated. It's operating on the assumption that if the type caster can produce a U*, then the type caster probably stores a U. So in your case, if the caster can produce a Vec* (i.e. _p_Vec**), then the caster probably stores a Vec (indeed it does), and thus it's safe to use that Vec past the end of the lifetime of the type caster object.

diff --git a/include/nanobind/nb_cast.h b/include/nanobind/nb_cast.h
index 3f199bf..dfa7c5c 100644
--- a/include/nanobind/nb_cast.h
+++ b/include/nanobind/nb_cast.h
@@ -108,8 +108,9 @@ using precise_cast_t =
 /// where the template argument T is the type you plan to extract.
 template <typename T>
 NB_INLINE uint8_t flags_for_local_caster(uint8_t flags) noexcept {
+    using Caster = make_caster<T>;
     constexpr bool is_ref = std::is_pointer_v<T> || std::is_reference_v<T>;
-    if constexpr (is_base_caster_v<make_caster<T>>) {
+    if constexpr (is_base_caster_v<Caster>) {
         if constexpr (is_ref) {
             /* References/pointers to a type produced by implicit conversions
                refer to storage owned by the cleanup_list. In a nb::cast() call,
@@ -123,7 +124,9 @@ NB_INLINE uint8_t flags_for_local_caster(uint8_t flags) noexcept {
            into storage owned by the caster, which won't live long enough.
            Exception: the 'char' caster produces a result that points to
            storage owned by the incoming Python 'str' object, so it's OK. */
-        static_assert(!is_ref || std::is_same_v<T, const char*>,
+        static_assert(!is_ref || std::is_same_v<T, const char*> ||
+                      std::conjunction_v<std::is_pointer<T>,
+                                         std::is_constructible<T*, Caster>>,
                       "nanobind generally cannot produce objects that "
                       "contain interior pointers T* (or references T&) if "
                       "the pointee T is not handled by nanobind's regular "

I'm out of town the rest of this week, but hopefully that gives enough information to fix your use case.

Some final notes:

• nanobind pre-2.0 did have a check analogous to this, but it was implemented in the individual container casters, so it wasn't applied consistently. For example, casting to std::optional<Vec> would have failed to compile before 2.0.
• I don't think you need the operator TYPE() in your caster. It's equivalent to something that's already in the expansion of NB_TYPE_CASTER().

[francesco-ballarin]

Thanks @oremanj ! I can confirm your patch does fix my compilation error, and opened #608 on your behalf.