JuliaInterop / libcxxwrap-julia

C++ library for backing CxxWrap.jl
Other
85 stars 43 forks source link

Unable to use smart pointers for added types #65

Closed rgcv closed 4 years ago

rgcv commented 4 years ago

Despite smart pointers having factories to be converted into julia types, being mapped to the SmartPointer abstract type in CxxWrap, when used as function arguments, libcxxwrap-julia adequately complains that there is no factory for .

This would be useful since I'd very much like to avoid creating smart pointers from function arguments. This can become a mess since I can have multiple smart pointers managing the same pointer.

As an example, take the following excerpt:

#include <jlcxx/module.hpp>

class MyType {
public:
  int v;
  MyType() : v(42) {}
};

namespace jlcxx {
  template<> struct IsMirroredType<MyType> : std::false_type {};
}

JLCXX_MODULE define_julia_module(jlcxx::Module& mod) {
  mod.add_type<MyType>("MyType");
  mod.method("do_something", [](const std::shared_ptr<MyType>& t) {
    return t->v;
  });
}

This can be compiled with g++ -std=c++17 -isystem /path/to/include/julia -fPIC -shared -o libtype.so type.cpp. Opening a julia REPL and defining the following module

julia> module M
       using CxxWrap
       @wrapmodule "./libtype.so"
       __init__() = @initcxx
       end

will yield the following exception/error

C++ exception while wrapping module M: No appropriate factory for type St10shared_ptrI4TypeE
ERROR: No appropriate factory for type St10shared_ptrI4TypeE

This is expected, this is just how libcxxwrap works. Arguably, there's already a large facility in place to map smart pointers to julia. It would be great if one could go a step further and map functions, methods, or what have you, that use smart pointers as parameter types.

At time of writing, there's a pretty nasty hack, IMHO, to work around this debilitation. One could just add the smart pointer type in question as a type...

JLCXX_MODULE define_julia_module(jlcxx::Module& mod) {
  mod.add_type<std::shared_ptr<MyType>>("MyTypeSharedPointer");
  mod.add_type<MyType>("MyType");
  mod.method("do_something", [](const std::shared_ptr<MyType>& t) {
    return t->v;
  });
  mod.method("shared_type", []() -> std::shared_ptr<MyType> {
    return std::make_shared<MyType>();
  });
}

This compiles perfectly fine, and the following succeeds

julia> module M
       using CxxWrap
       @wrapmodule "./libtype.so"
       __init__ = @initcxx
       end
Main.M

julia> M.shared_type()
Main.M.MyTypeSharedPointerAllocated(Ptr{Nothing} @0x000000000513b6cd)

julia> M.do_something(ans)
42

This is, however, a terrible approach. One can already tell from seeing MyTypeSharedPointerALLOCATED, which is to be expected, since it's now masked as a "regular" type.

rgcv commented 4 years ago

...Scratch that completely, this is what I get for trying to include only the headers I think I need or I think I am directly using... #include <jlcxx/smart_pointers.hpp> solves the issue, of course.

I'm so very sorry for cluttering the issues. I should read more, for a change. And sleep, that helps too.

barche commented 4 years ago

OK, thanks for the update, indeed the smart_pointers include is needed to get the types to be generated. Wether that is good design is open for discussion, I did it on a hunch that including all that template stuff could slow things down, but it may just be premature optimization.

rgcv commented 4 years ago

I'd argue it's a good design choice, no problem there. The least the compiler has to unnecessarily churn, the better. The real issue was just lack of attention on my end, took me way too long to even remember something as simple as to include the header. I'm usually weary about this, but this one slipped.