pybind11 icon indicating copy to clipboard operation
pybind11 copied to clipboard

Proposal: introduce a call guard factory

Open darwin opened this issue 5 years ago • 1 comments

Hi. Working on some glue code between Python and V8. I just ported quite non-trivial boost::python project under pybind11. And my mission was successful. I'm still amazed how much effort was put into this library. Kudos!

The problem

I have pretty heavy logging going on in the dev mode. I wanted to automatically track all calls crossing Python->C++ boundary. To keep the task simple, let's say I want to log the invoked python method name with each call. (Later I want to do more sophisticated pre-call/post-call logic)

I didn't really understand internal pre/post-call code. I wanted to use py::call_guard which is documented but I found it rather limiting. The problem is that it has to be stateless to be default-constructible and I don't have an easy way how to hook all "def-like" places. Of course I could wrap every method used in .def* with a lambda or do something there but how to best do this in an automated central way?

I ended up with a working solution, but I don't like it: https://github.com/darwin/naga/blob/8d0fee2f1207481f78cd355dc94796f01553d911/src/PybindNagaClass.h Here is an example where I use the wrapper class instead of py::class_: https://github.com/darwin/naga/blob/8d0fee2f1207481f78cd355dc94796f01553d911/src/PythonExpose.cpp#L111

The code looks the same like using vanilla py::class_, which was the goal.

More flexible solution

I guess that the limitation of call_guard comes from the support for composition of multiple guards. I don't care much about the composition, because I can easily compose more complex guards myself. What I need is some captured state.

Ideally I would like to pass a lambda (with captures) or just some callable object (with state) which would act as a factory for guards.

The call_impl code here could be something like:

    template <typename Return, typename Func, size_t... Is, typename GuardFactory>
    Return call_impl(Func &&f, index_sequence<Is...>, GuardFactory&& gf) && {
        auto guard = gf();
        return std::forward<Func>(f)(cast_op<Args>(std::move(std::get<Is>(argcasters)))...);
    }

Thank you for any suggestions and considering this. I could also attempt a proof-of-concept implementation with some guidance.

darwin avatar Apr 25 '20 18:04 darwin


#include <pybind11/pybind11.h>
#include <pybind11/stl.h>
#include <pybind11/functional.h>
#include <memory>
#include <vector>
#include <string>

namespace py = pybind11;

class Pet {
public:
    Pet() : id(makePetId()) {
        printf("Pet::Pet\n");
    }
    virtual ~Pet() {
        printf("Pet::~Pet\n");
    }
    Pet(const Pet&) = delete;
    Pet(Pet&&) = delete;

    virtual std::string name() const {
        return "pet";
    }

    static int makePetId() {
        static int id = 0;
        return id++;
    }

    int id = -1;
};

class PetRoom {
public:
    PetRoom() = default;

    void echoPetName(std::shared_ptr<Pet> pet) {
        printf("pet name: %s\n", pet->name().c_str());
    }

    void addPet(std::shared_ptr<Pet> pet) {
        printf("add pet: %s\n", pet->name().c_str());
        pets.push_back(pet);
    }

    void addPetCustom(py::object petClass) {
        // Instantiate the Python class
        py::object petInstance = petClass();
        // Convert to std::shared_ptr<Pet>
        std::shared_ptr<Pet> pet = petInstance.cast<std::shared_ptr<Pet>>();
        printf("add pet custom: %s\n", pet->name().c_str());
        pets.push_back(pet);
    }

    std::vector<std::shared_ptr<Pet>> pets;
};

PYBIND11_MODULE(libpybind, m) {
    py::class_<Pet, std::shared_ptr<Pet>>(m, "Pet")
        .def(py::init<>())
        .def("name", &Pet::name)
        .def_readonly("id", &Pet::id);

    py::class_<PetRoom>(m, "PetRoom")
        .def(py::init<>())
        .def("echoPetName", &PetRoom::echoPetName)
        .def("addPet", &PetRoom::addPet)
        .def("addPetCustom", &PetRoom::addPetCustom);
}

ljluestc avatar Nov 23 '24 20:11 ljluestc