Skip to content

Implementing a custom type caster that defers to the generic type caster #1176

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jbarlow83 opened this issue Nov 9, 2017 · 6 comments
Closed

Comments

@jbarlow83
Copy link
Contributor

Issue description

I'd like to implement a custom type caster that just says "use the generic caster for class objects".

Someone on Gitter kindly offered me a sample, which I edited to be compatible with C++11.

The initial reason I had for doing this was to debug a refcount problem – it let me intercept and log every occurrence of type conversion for the offending class. (I ended up solving my refcount problem by inspecting the outgoing object in cast to see if it has an implicit dependency on another C++ object.)

Another case where this could be useful to someone is a C++ object that sometimes, but not always, can be converted to a Python intrinsic – like a union of a pointer to a class and an integer.

While it works, this example has two problems, as annotated below. It embeds a second type_caster_base object and needs to copy that object's value into its own.

So I'd like to get the word on how to improve this example and do this properly, and then I'll add it to the documentation as another custom type caster example. Or perhaps the better way to go is to add 'callbacks' to the generic caster (i.e. a call to templated function that is a no-op in the generic case but can be specialized by class).

Reproducible example code

// This custom type caster just does the default casting behavior for the class.
namespace pybind11 { namespace detail {
    template <> struct type_caster<MyClass> {
    public:
        PYBIND11_TYPE_CASTER(MyClass, _("MyClass"));

        bool load(handle src, bool convert) {
            // Problem 1: we're allocating another case instead of transcluding it somehow
            static auto base_caster = type_caster_base<MyClass>();

            // Problem 2: we're copying the value out of the base caster, breaking unique_ptr<> etc
            if (base_caster.load(src, convert)) {
                value = *reinterpret_cast<MyClass *>(base_caster.value);
                /* Do any additional work here */
                return true;
            }
            return false;
        }

        static handle cast(MyClass src, return_value_policy policy, handle parent) {
            /* Do any additional work here */
            return type_caster_base<MyClass>::cast(src, policy, parent);
        }
    };
}}
@jagerman
Copy link
Member

jagerman commented Nov 9, 2017

How about just inheriting from the base caster instead of composition? Then your load and cast just intercept, do what they like, then call through to the parent class load/cast.

@jbarlow83
Copy link
Contributor Author

I was looking for a way to inherit it, yes.

I tried the following just now:

// Showing changes only compared to above
template <> struct type_caster<MyClass> : public type_caster_base<MyClass>
...
        bool load(handle src, bool convert) {
            return type_caster_base<MyClass>::load(src, convert);
        }

Maybe I'm missing something obvious but that looks a bit incestuous. Specializing and subclassing at the same time when type_caster is already a subclass of type_caster_base.... Sure enough it manages to compile but crashes and burns at runtime.

Perhaps the crash is because type_caster_base notes that it is for heap objects and the class in question can be heap or stack allocated?

@jagerman
Copy link
Member

jagerman commented Nov 9, 2017

Here's an example that seems to work for me:

#include <iostream>
#include <pybind11/pybind11.h>

namespace py = pybind11;

struct MyClass {
    MyClass(int i) : v{i} {}
    int v;
};

namespace pybind11 { namespace detail {

template <> struct type_caster<MyClass> : public type_caster_base<MyClass> {
    using base = type_caster_base<MyClass>;
public:
    bool load(handle src, bool convert) {
        if (base::load(src, convert)) {
            std::cerr << "loaded via base!\n";
            return true;
        }
        else if (py::isinstance<py::int_>(src)) {
            std::cerr << "loading from integer!\n";
            value = new MyClass(py::cast<int>(src));
            return true;
        }

        return false;
    }

    static handle cast(MyClass *src, return_value_policy policy, handle parent) {
        /* Do any additional work here */
        return base::cast(src, policy, parent);
    }
};

}}

PYBIND11_MODULE(i1176, m) {

    py::class_<MyClass>(m, "MyClass")
        .def(py::init<int>())
        .def_readwrite("v", &MyClass::v);
    m.def("f", [](MyClass &m) { return m.v; });

}

Python:

>>> from i1176 import *
>>> f(MyClass(3))
loaded via base!
3
>>> f(42)
loading from integer!
42
>>> 

A couple differences that may be contributing to the crash:

  • no PYBIND11_TYPE_CASTER macro; that isn't needed (or useful) for a type_caster_base<...>-derived class
  • cast takes the first argument by pointer (your original example took it by value). type_caster_base<...> has lvalue and rvalue-accepting cast functions that simply dispatch to the pointer-taking version, so you only need to override and wrap the latter.

@jbarlow83
Copy link
Contributor Author

jbarlow83 commented Nov 10, 2017

Thank you so much, that works very well is much cleaner than what I had before.

I did need to use cast(MyClass src...) because it so happens that I only return MyClass by value. It's a lightweight handle that can be freely copied.

As an example

PYBIND11_MODULE(i1176, m) {
    py::class_<MyClass>(m, "MyClass")
        .def(py::init<int>())
        .def_readwrite("v", &MyClass::v)
        .def_static("factory", [](){ return MyClass(666); } );  // added
    m.def("f", [](MyClass &m) { return m.v; });
}

produces the error (from clang):

error: no viable conversion from 'enable_if_t<!std::is_void<MyClass>::value, MyClass>' (aka 'MyClass') to 'MyClass *'
                std::move(args_converter).template call<Return, Guard>(cap->f), policy, call.parent);

This can be resolved using the signature static handle cast(MyClass src). This seems okay to me, and it passes my test suite, but when you look at this change do you see anything wrong with it?

@jagerman
Copy link
Member

You'll need to have all three cast functions that the base class has: revalue, lvalue, and pointer. The rvalue and lvalue ones can be very simple though: they just call the pointer version with &val.

(You might be able to have just two: value and pointer; but the value one would still be a simple dispatch to the pointer one)

@jagerman
Copy link
Member

jagerman commented Nov 10, 2017

I think that without the three argument version the internal cast call is falling through to the base class cast rather than invoking the custom one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants