Should intrusive ref type caster be in its own header file?

[d0t2]

There's this section in include/nanobind/intrusive/ref.h:

// Registar a type caster for ``ref<T>`` if nanobind was previously #included
#if defined(NB_VERSION_MAJOR)
NAMESPACE_BEGIN(detail)
template <typename T> struct type_caster<nanobind::ref<T>> {
    using Caster = make_caster<T>;
    static constexpr bool IsClass = true;
    NB_TYPE_CASTER(ref<T>, Caster::Name)

    bool from_python(handle src, uint8_t flags,
                     cleanup_list *cleanup) noexcept {
        Caster caster;
        if (!caster.from_python(src, flags, cleanup))
            return false;

        value = Value(caster.operator T *());
        return true;
    }

    static handle from_cpp(const ref<T> &value, rv_policy policy,
                           cleanup_list *cleanup) noexcept {
        if constexpr (std::is_base_of_v<intrusive_base, T>)
            if (policy != rv_policy::copy && policy != rv_policy::move && value.get())
                if (PyObject* obj = value->self_py())
                    return handle(obj).inc_ref();

        return Caster::from_cpp(value.get(), policy, cleanup);
    }
};
NAMESPACE_END(detail)
#endif

For my project, I only include counter.h, counter.inl, and ref.h in the main C++ library which I'd like to keep largely independent from nanobind and python, while I have a separate python binding component that includes nanobind.h, etc. My impression is that the intrusive ref counter is implemented specifically to allow this independence.

I'm not an expert on the C++ build process, but I think with this setup you won't be able to include the type_caster code even if you re-include ref.h followed by nanobind.h in the binding code include nanobind.h then include ref.h in the binding code. So currently, my workaround is to simply copy paste the type_caster code in my binding code. Alternatively, I guess I could also use an #ifdef guard to include nanobind.h in my main C++ library when building the python extension.

However, it seems more reasonable to me to have the ref.h type_caster code in a separate header file that can be included with your binding code. That way you also wouldn't have to worry about the order of your #includes.

[wjakob]

My impression is that the intrusive ref counter is implemented specifically to allow this independence.
Correct.

I'm not an expert on the C++ build process, but I think with this setup you won't be able to include the type_caster code even if you re-include ref.h
Can't you include nanobind.h and then ref.h first in your python bindings? (before any of your project code).

This header is tricky for me to change because it is a break under SemVer. (i.e. must be postponed to next major version).

[d0t2]

Can't you include nanobind.h and then ref.h first in your python bindings? (before any of your project code).

Sorry that was a typo, I meant include nanobind.h then include ref.h in the binding code. For some reason this isn't working.

I've created a minimal example of my current setup:

PyMyLib.h (included in all binding code)

#pragma once

#include "nanobind/nanobind.h"
#include "nanobind/intrusive/ref.h"

namespace nb = nanobind;
using namespace nb::literals;

template <typename T>
auto intrusivePtr() {
  return nb::intrusive_ptr<T>([](T *o, PyObject *po) noexcept { o->set_self_py(po); });
}

void bindMyClass(nb::module_ &);

PyMyLib.cpp (main binding declaration)

#include "PyMyLib.h"
#include "nanobind/intrusive/counter.h"

namespace {

void IntrusiveIncRefPy(PyObject* o) noexcept {
  nb::gil_scoped_acquire guard;
  Py_INCREF(o);
}

void IntrusiveDecRefPy(PyObject* o) noexcept {
  nb::gil_scoped_acquire guard;
  Py_DECREF(o);
}

} // namespace

NB_MODULE(mylib, m) {
  nb::intrusive_init(&IntrusiveIncRefPy, &IntrusiveDecRefPy);
  bindMyClass(m);
}

PyMyClass.cpp (each class gets a PyXXX.cpp file for binding code)

#include "PyMyLib.h"
#include "MyClass.h"

using namespace mylib;

void bindMyClass(nb::module_ &m) {
    nb::class_<MyClass>(m, "MyClass", intrusivePtr<MyClass>())
        .def(nb::init<int>())
        .def("get_ref_counted_self", &MyClass::getRefCountedSelf);
}

RefCountedObj.h (included in almost all C++ library class header files)

#pragma once

#include "nanobind/intrusive/counter.h"
#include "nanobind/intrusive/ref.h"

namespace mylib {

template <typename T> using Ref = nanobind::ref<T>;

class RefCountedObj : public nanobind::intrusive_base {};

} // namespace mylib

RefCountedObj.cpp

#include "RefCountedObj.h"
#include "nanobind/intrusive/counter.inl"

MyClass.h

#include "RefCountedObj.h"

namespace mylib {

class MyClass : public RefCountedObj {
    int data;
public:
    MyClass(int data);
    Ref<MyClass> getRefCountedSelf();
};

} // namespace mylib

MyClass.cpp

#include "MyClass.h"

using namespace mylib;

MyClass::MyClass(int data) : data(data) {}

Ref<MyClass> MyClass::getRefCountedSelf() {
    return Ref<MyClass>(this);
}

CMakeLists.txt

cmake_minimum_required(VERSION 4.0.1)
project(nanobind_experiments)
set(CMAKE_EXPORT_COMPILE_COMMANDS ON)

find_package(Python 3.12 COMPONENTS Interpreter Development.Module REQUIRED)
add_subdirectory(ext/nanobind)

nanobind_add_module(mylib PyMyLib.cpp PyMyClass.cpp MyClass.cpp RefCountedObj.cpp)
install(TARGETS mylib DESTINATION ${CMAKE_CURRENT_SOURCE_DIR})

Testing in python:

import mylib

x = mylib.MyClass(1)
print(x.get_ref_counted_self())

we get:

... nanobind-experiments % python3.13 test.py
Traceback (most recent call last):
  File ".../nanobind-experiments/test.py", line 14, in <module>
    print(x.get_ref_counted_self())
          ~~~~~~~~~~~~~~~~~~~~~~^^
TypeError: Unable to convert function return value to a Python type! The signature was
    get_ref_counted_self(self) -> ref<mylib::MyClass>

[d0t2]

By the way, just wanted to say a big thank you for creating nanobind!

I work in a ML compiler team where we have a component that uses pybind11, after a month of very tedious and pretty painful work (and more months of convincing people this is a good idea before that), I'm near the finish line of porting the entire component to nanobind. And I've shown results of improving the compile time of the middle end of our compiler by 25% (the largest broad-base improvement that I know about)! And yes, our middle end is in python... Many other things that might seem bizarre about this, and I can assure there are no good reasons for them...

Working in a terrible code base really makes you appreciate when people have the taste and self respect to take the effort to write beautiful, elegant code. And nanobind is definitely an example of that! I'm really amazed by and grateful for the fact that we get so much free and high quality code from the open source community.

[wjakob]

Could I ask you to make a tiny GitHub repo with CMakeLists.txt that I can clone to look at this?

[d0t2]

Oops, it tried reproducing it myself and found it was working... Ironically, the issue was because I had made the modification I suggested in ext/nanobind and forgot about it (and I hadn't included ref_caster.h).

In the production repo, the issue was because we use llvm's clang-format presets (which I hate with a passion) which reorders includes alphabetically... There were a couple files that were reordered which I hadn't noticed. I've fixed it by enforcing the equivalent of PyMyLib.h is always included first (by inserting a comment) and the same with nanobind.h in PyMyLib.h.

Sorry about the confusion! I'll close the discussion thread.