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

WIP: Allow convert and reinterpret for Arrays of unions with missing/nothing #36186

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions base/missing.jl
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,33 @@ convert(::Type{T}, x::T) where {T>:Union{Missing, Nothing}} = x
convert(::Type{T}, x) where {T>:Missing} = convert(nonmissingtype_checked(T), x)
convert(::Type{T}, x) where {T>:Union{Missing, Nothing}} = convert(nonmissingtype_checked(nonnothingtype_checked(T)), x)

for Cs in ((:Missing,), (:Nothing,), (:Missing, :Nothing))
@eval function Base.reinterpret(
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@eval function Base.reinterpret(
@eval function reinterpret(

::Type{<:T}, # Note this is effectively same as Type{T} since will error if T is abstract
xs::Array{Union{T, $(Cs...)},N}
) where {T,N}
isbitstype(T) || throw(ArgumentError("cannot reinterpret `$(eltype(xs))`, type `$(T)` is not a bits type"))
return unsafe_wrap(Array{T,N}, Ptr{T}(pointer(xs)), length(xs))
Copy link
Sponsor Member

@KristofferC KristofferC Jun 8, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when xs get's garbage collected? Doesn't this mess with TBAA?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

idk, I didn't even know we did aliasing detection.

end

#==
@eval function Base.convert(
::Type{Array{Q,N}},
xs::Array{Union{T, $(Cs...)},N}
) where {N} where {T<:Q} where Q

all(x->x isa T, xs) || throw(ArgumentError("cannot convert $(eltype(xs)) to $T"))
if isbitstype(T) && T===Q
reinterpret(T, xs)
else
Q[x for x in xs]
end
end
==#
end
# Resolve ambiguity
#Base.convert(::Type{Array{Any,1}}, xs::Array{Any,1}) = xs
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why this is ambiguous, and the answer would probably go a long way to explaining why this breaks so much



# Comparison operators
==(::Missing, ::Missing) = missing
Expand Down
42 changes: 42 additions & 0 deletions test/missing.jl
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,48 @@ end
@test_throws MethodError convert(Union{Int, Missing}, "a")
end

mutable struct NotABitsType a end
==(n1::NotABitsType,n2::NotABitsType) = n1.a == n2.a

@testset "convert/reinterpret Arrays of Unions" begin
unions(T) = (Union{T, Missing}, Union{T, Nothing}, Union{T, Missing, Nothing})
corrupters(::Type{T}) where T = (x for x in (missing, nothing) if x isa T)

@testset "bits type ($U)" for U in unions(Int)
xo = U[1, 2, 3]
xr = reinterpret(Int, xo)
#xc = convert(Vector{Int}, xo)
#@test xc isa Vector{Int}
@test xr isa Vector{Int}
@test xr == xo
#@test xc == xo
# should not have allocated new memory
#@test pointer(xc) == pointer(xo)
@test pointer(xr) == pointer(xo)

@test_throws ArgumentError reinterpret(Integer, xo)

@testset "With non-target type values" begin
yo = U[1, 2, 3, corrupters(U)...]
#@test_throws ArgumentError convert(Vector{Int}, yo)
yr = reinterpret(Int, yo)
@test yr isa Vector{Int}
# No comment on what happens for the nonInt values, unspecified behavour
@test yr[1:3] == yo[1:3]
@test length(yo) == length(yr)
end
end
@testset "non-bits type ($U)" for U in unions(NotABitsType)
xo = U[NotABitsType(1), NotABitsType(2), NotABitsType(3)]
@test_throws ArgumentError reinterpret(Int, xo)
#xc = convert(Vector{NotABitsType}, x)
#@test xc isa Vector{Int}
#@test xc == xo
# should have allocated new memory
#@test pointer(xc) != pointer(xo)
end
end

@testset "promote rules" begin
@test promote_type(Missing, Missing) == Missing
@test promote_type(Missing, Int) == Union{Missing, Int}
Expand Down