Skip to content
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

Question about returning unique pointer in virtual function #673

Closed
psykokwak4 opened this issue Feb 15, 2017 · 5 comments
Closed

Question about returning unique pointer in virtual function #673

psykokwak4 opened this issue Feb 15, 2017 · 5 comments

Comments

@psykokwak4
Copy link

Sorry to bug you, I was trying to override a virtual function and return a unique pointer in that function, like a factory method:

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

using namespace pybind11;


class Animal {
public:
    virtual ~Animal() { }
    virtual void call()=0;
    virtual std::unique_ptr<Animal> reproduce() { return nullptr; };

public:
    std::string name;
};

class PyAnimal : public Animal {
public:
    using Animal::Animal;

    std::unique_ptr<Animal> reproduce() override {
        PYBIND11_OVERLOAD(
            std::unique_ptr<Animal>,
            Animal,
            reproduce,
        );
    }

    void call() override {
        PYBIND11_OVERLOAD_PURE(
            void,
            Animal,
            call,
        );
    }
};

std::unique_ptr<Animal> deliver(Animal *animal) {
    return animal->reproduce();
}


PYBIND11_PLUGIN(animal) {
    module m("animal", "pybind11 example plugin");

    class_<Animal, PyAnimal > animal(m, "Animal");
    animal
        .def(init<>())
        .def("call", &Animal::call)
        .def("reproduce", &Animal::reproduce)
        .def_readwrite("name", &Animal::name);

    m.def("deliver", &deliver);

    return m.ptr();
}

The code above failed to compile and the error message shows:

λ ~/pybind11_test/ g++ -std=c++11 animal.cpp -lpython2.7 -I/usr/local/include/python2.7 -I/usr/include/python2.7 -shared -o animal.so
In file included from animal.cpp:3:
In file included from /usr/local/include/python2.7/pybind11/pybind11.h:36:
In file included from /usr/local/include/python2.7/pybind11/attr.h:13:
/usr/local/include/python2.7/pybind11/cast.h:1065:12: error: no matching function for call to 'cast_op'
    return cast_op<T>(load_type<T>(handle));
           ^~~~~~~~~~
/usr/local/include/python2.7/pybind11/cast.h:1117:12: note: in instantiation of function template specialization
      'pybind11::cast<std::__1::unique_ptr<Animal, std::__1::default_delete<Animal> >, 0>' requested here
    return cast<T>(object);
           ^
/usr/local/include/python2.7/pybind11/cast.h:1147:22: note: in instantiation of function template specialization
      'pybind11::cast<std::__1::unique_ptr<Animal, std::__1::default_delete<Animal> > >' requested here
    return pybind11::cast<T>(std::move(o)); }
                     ^
animal.cpp:23:9: note: in instantiation of function template specialization
      'pybind11::detail::cast_safe<std::__1::unique_ptr<Animal, std::__1::default_delete<Animal> > >' requested here
        PYBIND11_OVERLOAD(
        ^
/usr/local/include/python2.7/pybind11/pybind11.h:1824:5: note: expanded from macro 'PYBIND11_OVERLOAD'
    PYBIND11_OVERLOAD_NAME(ret_type, cname, #fn, fn, __VA_ARGS__)
    ^
/usr/local/include/python2.7/pybind11/pybind11.h:1816:5: note: expanded from macro 'PYBIND11_OVERLOAD_NAME'
    PYBIND11_OVERLOAD_INT(ret_type, cname, name, __VA_ARGS__) \
    ^
/usr/local/include/python2.7/pybind11/pybind11.h:1811:43: note: expanded from macro 'PYBIND11_OVERLOAD_INT'
            else return pybind11::detail::cast_safe<ret_type>(std::move(o)); \
                                          ^
/usr/local/include/python2.7/pybind11/cast.h:436:73: note: candidate template ignored: substitution failure [with T =
      std::__1::unique_ptr<Animal, std::__1::default_delete<Animal> >]: 'cast_op_type' following the 'template'
      keyword does not refer to a template
template <typename T> typename make_caster<T>::template cast_op_type<T> cast_op(make_caster<T> &caster) {
                      ~~~~~~~~                                          ^
/usr/local/include/python2.7/pybind11/cast.h:439:73: note: candidate template ignored: substitution failure [with T =
      std::__1::unique_ptr<Animal, std::__1::default_delete<Animal> >]: 'cast_op_type' following the 'template'
      keyword does not refer to a template
template <typename T> typename make_caster<T>::template cast_op_type<T> cast_op(make_caster<T> &&caster) {
                      ~~~~~~~~                                          ^
/usr/local/include/python2.7/pybind11/cast.h:1040:15: error: no member named 'load' in
      'pybind11::detail::type_caster<std::__1::unique_ptr<Animal, std::__1::default_delete<Animal> >, void>'
    if (!conv.load(handle, true)) {
         ~~~~ ^
/usr/local/include/python2.7/pybind11/cast.h:1053:5: note: in instantiation of function template specialization
      'pybind11::detail::load_type<std::__1::unique_ptr<Animal, std::__1::default_delete<Animal> >, void>' requested
      here
    load_type(conv, handle);
    ^
/usr/local/include/python2.7/pybind11/cast.h:1065:23: note: in instantiation of function template specialization
      'pybind11::detail::load_type<std::__1::unique_ptr<Animal, std::__1::default_delete<Animal> > >' requested here
    return cast_op<T>(load_type<T>(handle));
                      ^
/usr/local/include/python2.7/pybind11/cast.h:1117:12: note: in instantiation of function template specialization
      'pybind11::cast<std::__1::unique_ptr<Animal, std::__1::default_delete<Animal> >, 0>' requested here
    return cast<T>(object);
           ^
/usr/local/include/python2.7/pybind11/cast.h:1147:22: note: in instantiation of function template specialization
      'pybind11::cast<std::__1::unique_ptr<Animal, std::__1::default_delete<Animal> > >' requested here
    return pybind11::cast<T>(std::move(o)); }
                     ^
animal.cpp:23:9: note: in instantiation of function template specialization
      'pybind11::detail::cast_safe<std::__1::unique_ptr<Animal, std::__1::default_delete<Animal> > >' requested here
        PYBIND11_OVERLOAD(
        ^
/usr/local/include/python2.7/pybind11/pybind11.h:1824:5: note: expanded from macro 'PYBIND11_OVERLOAD'
    PYBIND11_OVERLOAD_NAME(ret_type, cname, #fn, fn, __VA_ARGS__)
    ^
/usr/local/include/python2.7/pybind11/pybind11.h:1816:5: note: expanded from macro 'PYBIND11_OVERLOAD_NAME'
    PYBIND11_OVERLOAD_INT(ret_type, cname, name, __VA_ARGS__) \
    ^
/usr/local/include/python2.7/pybind11/pybind11.h:1811:43: note: expanded from macro 'PYBIND11_OVERLOAD_INT'
            else return pybind11::detail::cast_safe<ret_type>(std::move(o)); \
                                          ^
2 errors generated.

And the same if I substitute std::unique_ptr<Animal> to typedef.

Is the right way to use pybind11? Did I miss something?

Thanks so much for your help.

@ghost
Copy link

ghost commented Feb 17, 2017

If we are returning a unique_ptr to the base class in a pure virtual member function, pybind11 gives the above compiler error. Just wondering if this is something pybind11 supports.

However, if the member function is just a regular function (not a virtual function), everything works as expected.

//Everything works great with the below!
class Animal {
public:
    Animal(){}
    ~Animal() { }
    std::unique_ptr<Animal> create()
    {
        return std::unique_ptr<Animal>(new Animal());
    }
};

class Dog : public Animal {
    public:
        Dog(){}
        ~Dog(){}
    std::unique_ptr<Animal> create() override
    {
        return std::unique_ptr<Animal>(new Dog());
    }
};

PYBIND11_PLUGIN(animal) {
    module m("animal", "pybind11 example plugin");
    class_<Animal> animal(m, "Animal");
    animal
        .def(init<>())
        .def("create", &Animal::create);

    class_<Dog> dog(m, "Dog");
    dog
        .def(init<>())
        .def("create", &Dog::create);

    return m.ptr();
}
//Compiler error!
class Animal {
public:
    virtual ~Animal() { }
   virtual  std::unique_ptr<Animal> create()=0;
};

class PyAnimal : public Animal {
public:
    using Animal::Animal;

    std::unique_ptr<Animal>  create() override {
        PYBIND11_OVERLOAD_PURE(
            std::unique_ptr<Animal> ,
            Animal,
            create,
        );
    }
};
class Dog : public Animal {
    public:
        Dog(){}
        ~Dog(){}
    std::unique_ptr<Animal> create() override
    {
        return std::unique_ptr<Animal>(new Dog());
    }
};

PYBIND11_PLUGIN(animal) {
    module m("animal", "pybind11 example plugin");
    class_<Animal, PyAnimal> animal(m, "Animal");
    animal
        .def(init<>())
        .def("create", &Animal::create);

    class_<Dog> dog(m, "Dog");
    dog
        .def(init<>())
        .def("create", &Dog::create);

    return m.ptr();
}

@jagerman
Copy link
Member

The fundamental problem is that there isn't any way to get a unique_ptr to an instance returned by Python: the unique_ptr is an internal pybind11 holder created when the returned object was created in python (i.e. before the value gets returned). Pybind can't give up the unique_ptr because its tied to the Python instance. So while returning a std::unique_ptr from a C function is fine, it isn't going to work for a Python override.

The easiest solution is to provide a custom wrapper around the parent method that returns a raw pointer instead, which will work for both overloaded and non-overloaded cases:

class Animal {
public:
    virtual std::unique_ptr<Whatever> f() { /* ... */ }
    virtual std::unique_ptr<Whatever> pure() = 0;
};
class PyAnimal : public Animal {
public:
    Whatever * f_wrapper() {
        PYBIND11_OVERLOAD_INT(Whatever *, Animal, "f", /* extra args here, if any */);
        return Animal::f().release();
    }
    Whatever * pure_wrapper() {
        PYBIND11_OVERLOAD_PURE(Whatever *, Animal, pure);
    }
};

// ... later, in binding code:

    // virtual function that releases the returned pointer (if not overloaded):
    animal.def("f", &PyAnimal::f_wrapper);
    animal.def("pure", &PyAnimal::pure_wrapper);

This works because, when pybind gets the raw pointer, the first thing it's going to do is a lookup to see if it's already stored internally; if it is, it'll use the already-known instance, and if it isn't it'll construct a new holder around the pointer. In essence what that means is that if there is no overload, pybind is going to get a pointer that it doesn't know about, and so construct a new unique_ptr around it; if it comes from Python, it's going to get a pointer it already knows about, and will thus reference that internal object.

@psykokwak4
Copy link
Author

psykokwak4 commented Feb 18, 2017

Thanks so much for your time and reply. The solution does not apply to our implementation(we have to return a unique pointer to the same type Animal). However if I change the the return type from unique_ptr to shared_ptr. it compiles, but I got the error about "tried to call pure virtual function". And the bug disappears if I try to use a debugger and run it step by step:

animal.cpp

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

using namespace pybind11;


class Animal {
public:
    virtual ~Animal() { }
    virtual void call()=0;
    virtual std::shared_ptr<Animal> create() { return nullptr; };

public:
    std::string name;
};

class PyAnimal : public Animal {
public:
    using Animal::Animal;

    std::shared_ptr<Animal> create() override {
        PYBIND11_OVERLOAD(
            std::shared_ptr<Animal>,
            Animal,
            create,
        );
    }

    void call() override {
        PYBIND11_OVERLOAD_PURE(
            void,
            Animal,
            call,
        );
    }
};

void testing(Animal & animal) {
    auto t = animal.create();
    t->call();
}


PYBIND11_PLUGIN(animal) {
    module m("animal", "pybind11 example plugin");

    class_<Animal, PyAnimal, std::shared_ptr<Animal> > animal(m, "Animal");
    animal
        .def(init<>())
        .def("call", &Animal::call)
        .def("create", &Animal::create)
        .def_readwrite("name", &Animal::name);

    m.def("testing", &testing);

    return m.ptr();
}

test.py

from animal import Animal, testing

class A(Animal):
    def __init__(self):
        super(A, self).__init__()
        self.b = A.B()
    def call(self):
        print('a')
    def create(self):
        return A.B()

    class B(Animal):
        def __init__(self):
            super(A.B, self).__init__()
        def call(self):
            print('b')
        def create(self):
            return A()


testing(A())

error:

Traceback (most recent call last):
  File "test.py", line 21, in <module>
    testing(A())
RuntimeError: Tried to call pure virtual function "Animal::call"

Is it has something to do with holder type for Animal? (std::shared_ptr<Animal>)

And the strange thing is if I set a breaking point just before testing(A()); like this:

from ipdb import set_trace; set_trace()

and step into this testing function(for some depth, after I step into the return statement for A's create method in this case), it prints expected output.

Do you know what might cause this problem? Why step into the function help to remove the error? And what is the correct way to understand the error message?

Thanks so much for your time and help.


From my understanding the Whatever should be a type different from Animal. But I substitute Whatever with Animal anyway, and get segfault for the same test.py:

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

using namespace pybind11;


class Animal {
public:
    virtual ~Animal() { }
    virtual void call()=0;
    virtual std::unique_ptr<Animal> create() { return nullptr; };

public:
    std::string name;
};

class PyAnimal : public Animal {
public:
    using Animal::Animal;

    Animal* create_wrapper() {
        PYBIND11_OVERLOAD_INT(Animal *, Animal, "create", /* extra args here, if any */);
        return Animal::create().release();
    }

    void call() override {
        PYBIND11_OVERLOAD_PURE(
            void,
            Animal,
            call,
        );
    }
};

void testing(Animal & animal) {
    auto t = animal.create();
    t->call();
}


PYBIND11_PLUGIN(animal) {
    module m("animal", "pybind11 example plugin");

    class_<Animal, PyAnimal, std::shared_ptr<Animal> > animal(m, "Animal");
    animal
        .def(init<>())
        .def("call", &Animal::call)
        .def("create", &PyAnimal::create_wrapper)
        .def_readwrite("name", &Animal::name);

    m.def("testing", &testing);

    return m.ptr();
}

trace back:

0   animal.so                     	0x00000001080649cd testing(Animal&) + 93
1   animal.so                     	0x0000000108097178 void pybind11::detail::argument_loader<Animal&>::call_impl<void, void (*&)(Animal&), 0ul>(void (*&&&)(Animal&), pybind11::detail::index_sequence<0ul>) + 72
2   animal.so                     	0x0000000108097125 std::__1::enable_if<std::is_void<void>::value, pybind11::detail::void_type>::type pybind11::detail::argument_loader<Animal&>::call<void, void (*&)(Animal&)>(void (*&&&)(Animal&)) + 37
3   animal.so                     	0x0000000108097061 void pybind11::cpp_function::initialize<void (*&)(Animal&), void, Animal&, pybind11::name, pybind11::scope, pybind11::sibling>(void (*&&&)(Animal&), void (*)(Animal&), pybind11::name const&, pybind11::scope const&, pybind11::sibling const&)::'lambda'(pybind11::detail::function_record*, pybind11::handle, pybind11::handle, pybind11::handle)::operator()(pybind11::detail::function_record*, pybind11::handle, pybind11::handle, pybind11::handle) const + 225
4   animal.so                     	0x0000000108096f60 void pybind11::cpp_function::initialize<void (*&)(Animal&), void, Animal&, pybind11::name, pybind11::scope, pybind11::sibling>(void (*&&&)(Animal&), void (*)(Animal&), pybind11::name const&, pybind11::scope const&, pybind11::sibling const&)::'lambda'(pybind11::detail::function_record*, pybind11::handle, pybind11::handle, pybind11::handle)::__invoke(pybind11::detail::function_record*, pybind11::handle, pybind11::handle, pybind11::handle) + 48
5   animal.so                     	0x000000010808c10e pybind11::cpp_function::dispatcher(_object*, _object*, _object*) + 2014

@jagerman
Copy link
Member

jagerman commented Feb 18, 2017

My mistake: I forgot that, of course, you'll also need an actual override of the create() method. But now that I think about it a bit more, I don't think it's actually possible to do this with unique_ptr: pybind11 needs the unique_ptr because it is the heart of a living Python object; you can't have it referenced both there and in a returned value (but the override needs to be an actual returned unique_ptr).

You were on the right track with shared_ptr. The reason you get the error about calling a virtual call() is that the Python object doesn't exist anymore at the time you're making the call. You still have the C++ instance, in the shared_ptr, but the Python instance (which was returned by A.create()) isn't around anymore because nothing references it once the call is over: pybind11 itself only holds on to it long enough to convert the value to a C++ type (std::shared_ptr<Animal> in this case), which means that in your testing function, your t will be a shared pointer to a PyAnimal, but when pybind11 tries to look up the python instance associated with t to find the overridden call function there is no Python instance anymore.

(You can see this working if you return a reference instead of a new instance, e.g. returning self.b instead of the new A.B() instance).

If returning references to existing objects won't work for you, your other option is to deal with storing the python objects as long as you need to. Basically, you'll have to hold the py::object yourself, something along the lines of:

{
    pybind11::gil_scoped_acquire gil; // Only necessary if this could be called from gil-released code
    pybind11::function overload = pybind11::get_overload(static_cast<const Animal *>(this), "create");
    if (overload) {
        object o = overload();
        auto shptr = pybind11::cast<std::shared_ptr<Animal>>(o);
        // Virtual calls on shptr will work here (as long as `o` persists)
    }
}

@psykokwak4
Copy link
Author

psykokwak4 commented Feb 18, 2017

Thanks so much for your help. Sorry for my poor understanding, would you mind elaborate on "hold the py::object"? Is it means I need to manually expand the PYBIND11_OVERLOAD macro? Or it means I need to store the object o as a member of class PyAnimal?
I've tried PyAnimal like this, it complies, however I got segfault after running the same test

class PyAnimal : public Animal {
public:
    using Animal::Animal;

    std::shared_ptr<Animal> create() override {
        pybind11::gil_scoped_acquire gil;
        pybind11::function overload = pybind11::get_overload(static_cast<const Animal *>(this), "create");
            object o = overload();
            auto shptr =  pybind11::cast<std::shared_ptr<Animal>>(o);
            return shptr->create();
    }

...
0   animal.so                     	0x000000010b155562 pybind11::detail::type_caster_generic::load(pybind11::handle, bool, _typeobject*) + 66
1   animal.so                     	0x000000010b155500 pybind11::detail::type_caster_generic::load(pybind11::handle, bool) + 112
2   animal.so                     	0x000000010b1553ef bool pybind11::detail::argument_loader<std::__1::vector<std::__1::shared_ptr<Animal>, std::__1::allocator<std::__1::shared_ptr<Animal> > >&>::load_impl_sequence<0ul>(pybind11::handle, pybind11::detail::index_sequence<0ul>) + 95
3   animal.so                     	0x000000010b155384 pybind11::detail::argument_loader<std::__1::vector<std::__1::shared_ptr<Animal>, std::__1::allocator<std::__1::shared_ptr<Animal> > >&>::load_impl(pybind11::handle, pybind11::handle, ...) + 36
4   animal.so                     	0x000000010b154fd7 pybind11::detail::argument_loader<std::__1::vector<std::__1::shared_ptr<Animal>, std::__1::allocator<std::__1::shared_ptr<Animal> > >&>::load_args(pybind11::handle, pybind11::handle) + 55
5   animal.so                     	0x000000010b181645 void pybind11::cpp_function::initialize<void pybind11::detail::init_alias<>::execute<pybind11::class_<Animal, PyAnimal, std::__1::shared_ptr<Animal> >, 0>(pybind11::class_<Animal, PyAnimal, std::__1::shared_ptr<Animal> >&)::'lambda'(PyAnimal*), void, PyAnimal*, pybind11::name, pybind11::is_method, pybind11::sibling>(pybind11::class_<Animal, PyAnimal, std::__1::shared_ptr<Animal> >&&, void (*)(PyAnimal*), pybind11::name const&, pybind11::is_method const&, pybind11::sibling const&)::'lambda'(pybind11::detail::function_record*, pybind11::handle, pybind11::handle, pybind11::handle)::operator()(pybind11::detail::function_record*, pybind11::handle, pybind11::handle, pybind11::handle) const + 85
6   animal.so                     	0x000000010b1815d0 void pybind11::cpp_function::initialize<void pybind11::detail::init_alias<>::execute<pybind11::class_<Animal, PyAnimal, std::__1::shared_ptr<Animal> >, 0>(pybind11::class_<Animal, PyAnimal, std::__1::shared_ptr<Animal> >&)::'lambda'(PyAnimal*), void, PyAnimal*, pybind11::name, pybind11::is_method, pybind11::sibling>(pybind11::class_<Animal, PyAnimal, std::__1::shared_ptr<Animal> >&&, void (*)(PyAnimal*), pybind11::name const&, pybind11::is_method const&, pybind11::sibling const&)::'lambda'(pybind11::detail::function_record*, pybind11::handle, pybind11::handle, pybind11::handle)::__invoke(pybind11::detail::function_record*, pybind11::handle, pybind11::handle, pybind11::handle) + 48

The reference solution works like magic, however I need to use the second solution because the my primary use case is to append object to a local list:

Sorry, I found a workaround for the local reference solution, it works just like magic!

Thanks so much for you time!

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