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

How to avoid deep type piracy? #458

Open
peremato opened this issue Nov 18, 2024 · 2 comments
Open

How to avoid deep type piracy? #458

peremato opened this issue Nov 18, 2024 · 2 comments

Comments

@peremato
Copy link

peremato commented Nov 18, 2024

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
Copy link
Contributor

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:

map_julia_arg_type(t::Type{ConstCxxPtr{T}}, ::Type{IsNormalType}) where {T} = Union{ConstPtrTypes{T},Ptr{Cvoid}}

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
Copy link
Author

peremato commented Nov 18, 2024

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.

grasph added a commit to grasph/wrapit that referenced this issue Nov 18, 2024
Lambda function are now systemtically used in Module::module() calls.
peremato referenced this issue in JuliaHEP/PYTHIA8.jl Nov 21, 2024
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