JuliaInterop / CxxWrap.jl

Package to make C++ libraries available in Julia
Other
427 stars 66 forks source link

How to avoid deep type piracy? #458

Open peremato opened 1 week ago

peremato commented 1 week ago

While wrapping a C++ type that provides operator+ breaks the REPL in a very unexpected manner. For example using the ? to get the help of a entity, the wrapped operator is selected by the multiple-dispatch when trying to call the function +(arg1::Ptr{Nothing}, arg2::Int64). Here is the error:

help?> Hist
search:ERROR: MethodError: Cannot `convert` an object of type Ptr{Nothing} to an object of type Main.Types.Hist
The function `convert` exists, but no method is defined for this combination of argument types.

Closest candidates are:
  convert(::Type{Main.Types.Hist}, ::Main.Types.HistDereferenced)
   @ Main.Types ~/.julia/packages/CxxWrap/eWADG/src/CxxWrap.jl:751
  convert(::Type{Main.Types.Hist}, ::Main.Types.HistAllocated)
   @ Main.Types ~/.julia/packages/CxxWrap/eWADG/src/CxxWrap.jl:750
  convert(::Type{Main.Types.Hist}, ::T) where T<:Main.Types.Hist
   @ Main.Types ~/.julia/packages/CxxWrap/eWADG/src/CxxWrap.jl:746
  ...

Stacktrace:
  [1] cxxconvert(to_type::Type{ConstCxxPtr{Main.Types.Hist}}, x::Ptr{Nothing}, ::Type{CxxWrap.CxxWrapCore.IsCxxType})
    @ CxxWrap.CxxWrapCore ~/.julia/packages/CxxWrap/eWADG/src/CxxWrap.jl:619
  [2] cconvert(to_type::Type{ConstCxxPtr{Main.Types.Hist}}, x::Ptr{Nothing})
    @ CxxWrap.CxxWrapCore ~/.julia/packages/CxxWrap/eWADG/src/CxxWrap.jl:598
  [3] +(arg1::Ptr{Nothing}, arg2::Int64)
    @ Main.Types ~/.julia/packages/CxxWrap/eWADG/src/CxxWrap.jl:668
  [4] pointer
    @ ./abstractarray.jl:1232 [inlined]
  [5] unsafe_copyto!
    @ ./genericmemory.jl:139 [inlined]
  [6] unsafe_copyto!
    @ ./genericmemory.jl:124 [inlined]
  [7] _copyto_impl!
    @ ./array.jl:308 [inlined]
  [8] copyto!
    @ ./array.jl:294 [inlined]
  [9] setindex_widen_up_to
    @ ./array.jl:828 [inlined]
 [10] collect_to!(dest::Vector{Nothing}, itr::Base.Generator{Vector{…}, REPL.var"#42#43"}, offs::Int64, st::Int64)
    @ Base ./array.jl:845
 [11] collect_to_with_first!
    @ ./array.jl:816 [inlined]
 [12] _collect(c::Vector{…}, itr::Base.Generator{…}, ::Base.EltypeUnknown, isz::Base.HasShape{…})
    @ Base ./array.jl:810
 [13] collect_similar
    @ ./array.jl:709 [inlined]
 [14] map
    @ ./abstractarray.jl:3371 [inlined]
 [15] doc_completions(name::String, mod::Module)
    @ REPL ~/.julia/juliaup/julia-1.11.1+0.aarch64.apple.darwin14/share/julia/stdlib/v1.11/REPL/src/docview.jl:891
 [16] repl_search(io::Base.TTY, s::String, mod::Module)
    @ REPL ~/.julia/juliaup/julia-1.11.1+0.aarch64.apple.darwin14/share/julia/stdlib/v1.11/REPL/src/docview.jl:458
 [17] top-level scope
    @ ~/.julia/juliaup/julia-1.11.1+0.aarch64.apple.darwin14/share/julia/stdlib/v1.11/REPL/src/docview.jl:555
Some type information was truncated. Use `show(err)` to see complete types.

I am attaching a reproducer.

#include "jlcxx/jlcxx.hpp"
struct Hist
{
    Hist(double v): _value(v) {}
    Hist operator+(double f) const { return Hist(value() + f); }
    Hist operator+(const Hist& h) const { return Hist(value() + h.value()); }
    double value() const { return _value; }
private:
    double _value;
};

JLCXX_MODULE define_julia_module(jlcxx::Module& module)
{
    auto type = module.add_type<Hist>("Hist");
    type.constructor<double>();
    type.method("value", &Hist::value);
    module.set_override_module(jl_base_module);
    type.method("+", static_cast<Hist(Hist::*)(double)  const>(&Hist::operator+));
    type.method("+", static_cast<Hist(Hist::*)(const Hist&)  const>(&Hist::operator+));
    module.unset_override_module();
}

and the Julia code to trigger the error:

jlprefix = dirname(Sys.BINDIR)

Base.run(`c++ -O2 -shared -fPIC -std=c++17 -I$cxxprefix/include -I$jlprefix/include/julia 
          -Wl,-rpath,$cxxprefix/lib -L$cxxprefix/lib -lcxxwrap_julia 
          -Wl,-rpath,$jlprefix/lib -L$jlprefix/lib -ljulia 
          -o libhist.so $(@__DIR__)/operator+.cpp`)

# Load the module and generate the functions
module Types
    using CxxWrap
    @wrapmodule(() -> "./libhist.so")
    function __init__()
        @initcxx
    end
end

h = Types.Hist(66.)
h = h + 11.
println(Types.value(h))

# So far so good, but the following will fail
ptr = Ptr{Nothing}(0)
ptr + 10
benlorenz commented 1 week ago

This can be avoided by using a small lambda function instead of the member-function pointer, i.e.:

    type.method("+", [](const Hist& h, double f) { return h+f; });
    type.method("+", [](const Hist& h, const Hist& f) { return h+f; });

Then the generated functions will be more strict regarding the arguments (and only allow Union{Main.Types.Hist, ConstCxxRef{<:Main.Types.Hist}, CxxRef{<:Main.Types.Hist}} as first argument).

What happens in the original code is the following:

For a member function libcxxwrap-julia generates two methods, one for the Ptr type as a base object and one for the Ref type:

  /// Define a member function, const version
  template<typename R, typename CT, typename... ArgsT, typename... Extra>
  TypeWrapper<T>& method(const std::string& name, R(CT::*f)(ArgsT...) const, Extra... extra)
  {
    m_module.method(name, [f](const T& obj, ArgsT... args) -> R { return (obj.*f)(args...); }, extra... );
    m_module.method(name, [f](const T* obj, ArgsT... args) -> R { return ((*obj).*f)(args...); }, extra... );
    return *this;
  }

(https://github.com/JuliaInterop/libcxxwrap-julia/blob/d4192fe3ea20829bd399d812863abd8303f9bcab/include/jlcxx/module.hpp#L1128-L1129)

Which results in these CppFuntionInfo:

 CxxWrap.CxxWrapCore.CppFunctionInfo(:+, Any[ConstCxxRef{Main.Types.Hist}, Float64], Any, Main.Types.HistAllocated, Ptr{Nothing} @0x00007f6393220600, Ptr{Nothing} @0x000000002585c150, Base, "", Any[], Any[], 0)
 CxxWrap.CxxWrapCore.CppFunctionInfo(:+, Any[ConstCxxPtr{Main.Types.Hist}, Float64], Any, Main.Types.HistAllocated, Ptr{Nothing} @0x00007f639321c520, Ptr{Nothing} @0x0000000027d36930, Base, "", Any[], Any[], 0)

But CxxWrap will allow Ptr{Cvoid} when mapping ConstCxxPtr{T} to the arguments here: https://github.com/JuliaInterop/CxxWrap.jl/blob/ad11b163cc47c999b6edc36b137a182d78478467/src/CxxWrap.jl#L575 producing this signature:

:((Base).:+(arg1::Union{Ptr{Nothing}, ConstCxxPtr{<:Main.Types.Hist}, CxxPtr{<:Main.Types.Hist}}, arg2::Union{Float64, Int64, Irrational}; )::Main.Types.HistAllocated
peremato commented 1 week ago

Thanks very much for the explanation and way out. I have tested and indeed with lambdas I get the signature:

+(arg1::Union{Main.Types.Hist, ConstCxxRef{<:Main.Types.Hist}, CxxRef{<:Main.Types.Hist}}, arg2::Union{Float64, Int64, Irrational})

The problem I have now is that the wrapper code is genererated by WrapIt.jl and I need to find out why is using static_cast instead of lambdas.