WIP Add arg().disown() (superseding the consume call_policy)

[torokati44]

I finally found some time to work a little bit on #971. This PR supersedes it. (Should I close it?) I reworked it, taking some of @jagerman's advice. However, it is still not nearly complete.

• It is now done with py::arg, instead of as a call policy.
• The holder is now destructed, and the reference is set to be not owning.
• Releasing is done via a holder_helper static method.
• Moving owned to value_and_holder is not yet done.
• The default holder_helper::release does not give a meaningful error when there is no holder_type::release()
• holder_type is still assumed to be std::unique_ptr<void*>

For the last one: I have no idea how to get the proper holder_type for the args. I tried to look in the direction of class_<arg>::holder_type, but that would have to be forward declared, and I'm no even sure what to supply as the type template parameter.

Is this in the right place in dispatcher()?
Also, I saw that #1146 referenced #971, is this PR still relevant despite that one?

Any comments are still much appreciated.

[torokati44]

FWIW, even in its current wonky form, this has proven to be "working for me". At least for now.

[EricCousineau-TRI]

Also, I saw that #1146 referenced #971, is this PR still relevant despite that one?

I'd say yes, this PR is still quite relevant (and I'm actually still looking to have it land in some form, as it in it's own state has been useful to us :) ). It looks like you're providing a much more generic mechanism to transfer ownership, which is awesome!

For the last one: I have no idea how to get the proper holder_type for the args.

In (a slightly more refined version of) my WIP branch from #1146, I type-erase mechanisms that deal with holders, store those in detail::type_info (unfortunately needed, but meh), and then call them when needed:
https://github.com/RobotLocomotion/pybind11/blob/48999b6/include/pybind11/pybind11.h#L1277

I believe that ultimately, you'd need this sort of mechanism to properly mix holder types (which my implementation misses, since I don't store typeid, but @jagerman has in his #1161).

Feel free to steal bits from my PR if you're able to understand them and find them useful.
I'd be more than happy to help as well!

And naiive question: How does this pass compilation with shared_ptr and all that (things that don't have a release)? Is it because you've implicitly recast the unique_ptr<void*>?

[EricCousineau-TRI]

@torokati44 After seeing your stuff, and reworking my PR, I believe this function should be what you need for type-erased holder transfer:
https://github.com/EricCousineau-TRI/pybind11/blob/379afa73c7fedded57e4fedc9c3b077cb7292643/include/pybind11/cast.h#L482

You could use it to something like the tune of:

if (!reclaim_existing_if_needed(inst, tinfo, existing_holder)) {
  throw cast_error("Cannot release ownership of this object");  // If you have an incompatible holder type, like `shared_ptr`.
}

The caveat is, you'll need to figure out what the holder type is for the given argument. I believe that should be elsewhere in this area of code.
(Also, ensure that you get tinfo from the argument type and NOT the holder, as holder->type may be null!)

[torokati44]

I was actually asking if my PRs were relevant, seeing that yours' are progressing. :)
Thank you for your tips, I'll take a look at them once I have the time/capacity to work on this again.

[EricCousineau-TRI]

I was actually asking if my PRs were relevant, seeing that yours' are progressing. :)

Hup, sorry about that! (Was in technical mode)
Ideally, I think the ownership transfer should based on the holder: If you want to take an object from pybind and then pass it to something else in C++ (e.g. something that takes unique_ptr, or something that takes a raw pointer), then it'd be best to have a unique_ptr<> as an argument, rather than annotate the argument (which I believe would end up yielding the same functionality).

That being said, this is based on my usage of it, which looks like this:
https://github.com/RobotLocomotion/drake/blob/5dbb882/bindings/pydrake/systems/framework_py.cc#L186

Can I ask if you have a code snippet or open-source project that you could point to, using your code?

[torokati44]

Can I ask if you have a code snippet or open-source project that you could point to, using your code?

I'm trying to create Python bindings for the OMNeT++ API. (I don't think I can share it just yet.)

It has ownership transfer of objects between the simulation kernel and the user code, in both directions, passed as raw pointer function arguments.
See for example handleMessage and send.

If cSimpleModule is to be subclassed in Python, this would mean ownership transfer between C++ and Python, and vice versa. In one direction (C++ -> Python) it works well already, and I'm trying to make the other direction (for send()) work with this PR.

As the API above is pretty much set in stone, I can't just switch to unique_ptr. Well, I could put together a wrapper of some sorts, but I'd rather not, if it can be avoided, as that wouldn't be very elegant.

Ownership transfer (or the lack thereof) via return value (again, as raw pointers) also only works one way (AFAIK; C++ -> Python), but one issue at a time... :)

[EricCousineau-TRI]

Ah, I see. In that case, it sounds like you'd be interested @jagerman's proposal for adding a load_options-like option to py::arg():
#864 (comment)
#1241 (comment)

[torokati44]

I have been able to achieve (almost) what I wanted with current vanilla pybind11; by using a custom holder that works similar to std::shared_ptr, except .release() acts on all holder instances that refer to the same value: https://gist.github.com/torokati44/adc99fd7279449c9000803b128cc36c2

I'm sure it violates a bunch of programming principles, and the names could be better, etc., but it gets the job done (somehow). At least until a better solution (either a call policy, an arg() option, or whatever) is found to take ownership away from Python.

The only inconvenience is that wherever this kind of ownership transfer happens with a function argument or return value, I have to bind a tiny wrapper lambda instead of the real function, that calls holder.release() before/after the actual function call. (And obviously instances of subclasses defined in Python can't be kept alive after Python loses a reference to them.)

I had silently hoped that someone with more experience with this kind of template-heavy code will finish this instead of me, or provide a better alternative, but so far this hasn't happened. Oh well.

I don't think I'll ever finish this, rather just go ahead with my custom holder. Thanks for the help anyway, @EricCousineau-TRI!