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

convert: add new RefArray type signature #390

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

benlorenz
Copy link
Contributor

@benlorenz benlorenz commented Oct 28, 2023

This seems to be needed for julia nightly, otherwise the CxxWrap tests error with:

basic_types: Error During Test at /home/pkgeval/.julia/packages/CxxWrap/66SRr/test/basic_types.jl:186
  Test threw exception
  Expression: BasicTypes.test_argv(Cint(length(argv)), argv) == join(argv)
  type RefArray has no field cpp_object
  Stacktrace:
   [1] getproperty
     @ Base ./Base.jl:37 [inlined]
   [2] unsafe_convert(to_type::Type{CxxPtr{CxxPtr{CxxChar}}}, x::Base.RefArray{Ptr{Int8}, Vector{Ptr{Int8}}, Vector{Any}})
     @ CxxWrap.CxxWrapCore ~/.julia/packages/CxxWrap/66SRr/src/CxxWrap.jl:271
   [3] test_argv(arg1::Int32, arg2::Vector{String})
     @ Main.BasicTypes ~/.julia/packages/CxxWrap/66SRr/src/CxxWrap.jl:624
   [4] macro expansion
     @ /opt/julia/share/julia/stdlib/v1.11/Test/src/Test.jl:676 [inlined]
   [5] macro expansion
     @ ~/.julia/packages/CxxWrap/66SRr/test/basic_types.jl:186 [inlined]
   [6] macro expansion
     @ /opt/julia/share/julia/stdlib/v1.11/Test/src/Test.jl:1598 [inlined]
   [7] top-level scope
     @ ~/.julia/packages/CxxWrap/66SRr/test/basic_types.jl:37

(from https://s3.amazonaws.com/julialang-reports/nanosoldier/pkgeval/by_date/2023-10/24/CxxWrap.primary.log, before libcxxwrap broke due to a different change, see JuliaInterop/libcxxwrap-julia#137)

This error seems to come from: https://github.com/JuliaLang/julia/pull/51764/files#diff-2331196f664261004e7d661fc57c6076578eeebc0bb8ef941e1c9f4215d72c90

Stepping through the code with two different julia version from before and after this change I got for before:

About to run: <(Base.unsafe_convert)(CxxPtr{CxxPtr{CxxChar}}, Base.RefArray{Ptr{Int8}, Vector{Ptr{Int8}}, Any}(Ptr{I...>

and after (note the Vector{Any} instead of Any as third type for RefArray):

About to run: <(Base.unsafe_convert)(CxxPtr{CxxPtr{CxxChar}}, Base.RefArray{Ptr{Int8}, Vector{Ptr{Int8}}, Vector{Any...>

And the new code then tried to call Base.unsafe_convert(to_type::Type{<:CxxBaseRef}, x) = to_type(x.cpp_object) instead of the one for RefArray.

Unfortunately the tests for this won't work for nightly due to the libcxxwrap error.
Edit: Locally this fixed the issue for julia nightly bde62adff7.

seems to be needed for julia nightly
@benlorenz benlorenz marked this pull request as ready for review October 29, 2023 11:46
@barche barche merged commit a80510a into JuliaInterop:main Nov 1, 2023
6 of 9 checks passed
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

Successfully merging this pull request may close these issues.

2 participants