JuliaInterop / CxxWrap.jl

Package to make C++ libraries available in Julia
Other
421 stars 68 forks source link

Idiomatic smartpointer generation #141

Open kyonifer opened 5 years ago

kyonifer commented 5 years ago

Suppose I want to wrap

struct A {};
void foo(std::shared_ptr<A> p){ }

If I wrap with

mod.add_type<A>("A").constructor();
mod.method("foo", &foo);

Then trying to generate an A in Julia and call the function fails with

julia> a=A()
AAllocated(Ptr{Nothing} @0x00007fafde6f2150)

julia> foo(a)
ERROR: MethodError: no method matching foo(::AAllocated)
Closest candidates are:
  foo(::CxxWrap.SmartPointer{T2} where T2<:A) at none:0
Stacktrace:
 [1] top-level scope at none:0

One workaround I found is

a_ptr=CxxWrap.SmartPointerWithDeref{A, :NSt__110shared_ptrIiEE}(a.cpp_object)
foo(a_ptr)

which, if I'm reading the convert chains correctly, will do some work to convert a pointer into a shared_ptr if the quoted mangled name is something with a known conversion. However it's not clear where to harvest the mangled symbol names from other than the compiled library, which is ugly. Of course, one could also write shims in C++ that return smart pointers.

In pybind11 they allow specification of a holder type which allows python to wrangle python-owned references into a selected smart pointer type. However, add_type<A, std::shared_ptr<A>> doesn't seem to work. I'm wondering if there is anything equivalent in CxxWrap, or maybe a convenience method somewhere that I didn't find?

barche commented 5 years ago

If A is consistently passed as a shared_ptr, it is best to immediately make its Julia constructor return one:

namespace jlcxx
{
template<> struct DefaultConstructible<A> : std::false_type {};
}

mod.add_type<A>("A");
mod.method("A", [] () { return std::make_shared<A>(); });
mod.method("foo", &foo);

The DefaultConstructible specialization is needed because otherwise CxxWrap will automatically generate the constructor (the .constructor() in your code was redundant). Objects returned by these "automatic" CxxWrap constructors automatically have a finalizer that deletes the allocated object, so they can't safely be placed in a shared_ptr because a double free would occur.

kyonifer commented 5 years ago

That approach does indeed work for my toy example. Unfortunately the library I'm trying to write bindings for passes objects around quite a lot. Objects can be returned by-value or as a shared_ptr depending on if shared ownership semantics are required or not, and it's rather heterogeneous.

I'd prefer not to cripple the julia bindings by locking them into a particular pointer type, because I would prefer to mostly work with this library in Julia.

One answer is to ask the user to manually wrap things into/out of smart pointers themselves. For example, if the user needs to take the result of A findFoo() and pass it back into void foo(std::shared_ptr<A> p){ } I could expose a function make_shared to Julia such that the user could call foo(makeShared(findFoo())), but that is an awkward solution as we'd have to intercept all by-value returns that get sent to Julia to make sure they don't end up in the automatic container with a finalizer that nukes them when julia loses all references (thus putting them into a shared_ptr would be unsafe, as you noted).

barche commented 5 years ago

It should always be OK to return a shared pointer from the constructor, conversion from shared pointer to value is done automatically for functions that take values or references as argument.

For the findFoo example, the make_shared approach can work, but make_shared should return a shared pointer to a copy of the input (e.g. std::make_shared<A>(a)). Alternatively you can wrap findFoo in a lambda that directly returns a shared pointer, it really depends on which use case happens most often. Even in pure C++, having a value and needing to convert it to a shared pointer is a pain, so I would guess this should be the rarest use case.