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

StdVector loses type information if given empty vector #367

Closed
glennmoy opened this issue Aug 23, 2023 · 1 comment · Fixed by #385
Closed

StdVector loses type information if given empty vector #367

glennmoy opened this issue Aug 23, 2023 · 1 comment · Fixed by #385

Comments

@glennmoy
Copy link

glennmoy commented Aug 23, 2023

I think the root issue is that broadcasting CxxRef over an empty vector loses the type info

MWE using StdString, but in practice it's any user-defined wrapped C++ type

julia> StdVector{StdString}()
0-element CxxWrap.StdLib.StdVectorAllocated{StdString}

julia> StdVector(StdString[])
0-element CxxWrap.StdLib.StdVectorAllocated{Any}

julia> CxxRef.(StdString[])
Any[]

It may be enough to just check for isempty(v) before calling CxxRef here

function StdVector(v::Vector{T}) where {T}
  if (CxxWrapCore.cpp_trait_type(T) == CxxWrapCore.IsCxxType)
    return StdVector(CxxRef.(v))
  end
  result = StdVector{T}()
  append(result, v)
  return result
end
barche added a commit that referenced this issue Aug 27, 2023
Fixes issue StdVector loses type information if given empty vector #367
barche added a commit that referenced this issue Sep 3, 2023
Fixes issue StdVector loses type information if given empty vector #367
@omus
Copy link
Contributor

omus commented Sep 11, 2023

Relatedly it would be good to also support StdVector{T}(::Vector):

julia> StdVector{StdString}()
0-element CxxWrap.StdLib.StdVectorAllocated{StdString}

julia> StdVector(StdString["hello"])
1-element CxxWrap.StdLib.StdVectorAllocated{StdString}:
 "hello"

julia> StdVector{StdString}(StdString["hello"])
ERROR: MethodError: no method matching StdVector{StdString}(::Vector{StdString})

Closest candidates are:
  StdVector{StdString}()
   @ CxxWrap ~/.julia/packages/CxxWrap/aXNBY/src/CxxWrap.jl:624

Stacktrace:
 [1] top-level scope
   @ REPL[18]:1

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 a pull request may close this issue.

2 participants