Skip to content

Commit

Permalink
don't allow promoting Some{T} and Some{S} (#27065)
Browse files Browse the repository at this point in the history
add promotion rule for `Union{Nothing, Missing, T}`

fixes the bug part of #26927
  • Loading branch information
JeffBezanson authored May 13, 2018
1 parent 2a21849 commit 0b1e166
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 2 deletions.
2 changes: 2 additions & 0 deletions base/missing.jl
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ end
promote_rule(::Type{Union{Nothing, Missing}}, ::Type{Any}) = Any
promote_rule(::Type{Union{Nothing, Missing}}, ::Type{T}) where {T} =
Union{Nothing, Missing, T}
promote_rule(::Type{Union{Nothing, Missing, S}}, ::Type{T}) where {T,S} =
Union{Nothing, Missing, promote_type(T, S)}

convert(::Type{Union{T, Missing}}, x) where {T} = convert(T, x)
# To fix ambiguities
Expand Down
2 changes: 1 addition & 1 deletion base/some.jl
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ struct Some{T}
value::T
end

promote_rule(::Type{Some{S}}, ::Type{Some{T}}) where {S,T} = Some{promote_type(S, T)}
promote_rule(::Type{Some{T}}, ::Type{Some{S}}) where {T, S<:T} = Some{T}
promote_rule(::Type{Some{T}}, ::Type{Nothing}) where {T} = Union{Some{T}, Nothing}

convert(::Type{Some{T}}, x::Some) where {T} = Some{T}(convert(T, x.value))
Expand Down
1 change: 1 addition & 0 deletions test/ambiguous.jl
Original file line number Diff line number Diff line change
Expand Up @@ -286,6 +286,7 @@ end
pop!(need_to_handle_undef_sparam, which(Base.convert, Tuple{Type{Union{Missing, T}} where T, Any}))
pop!(need_to_handle_undef_sparam, which(Base.promote_rule, Tuple{Type{Union{Nothing, S}} where S, Type{T} where T}))
pop!(need_to_handle_undef_sparam, which(Base.promote_rule, Tuple{Type{Union{Missing, S}} where S, Type{T} where T}))
pop!(need_to_handle_undef_sparam, which(Base.promote_rule, Tuple{Type{Union{Missing, Nothing, S}} where S, Type{T} where T}))
pop!(need_to_handle_undef_sparam, which(Base.zero, Tuple{Type{Union{Missing, T}} where T}))
pop!(need_to_handle_undef_sparam, which(Base.one, Tuple{Type{Union{Missing, T}} where T}))
pop!(need_to_handle_undef_sparam, which(Base.oneunit, Tuple{Type{Union{Missing, T}} where T}))
Expand Down
13 changes: 12 additions & 1 deletion test/some.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@

## promote()

@test promote_type(Some{Int}, Some{Float64}) === Some{Float64}
@test promote_type(Some{Int}, Some{Float64}) == Some
@test promote_type(Some{Int}, Some{Real}) == Some{Real}
@test promote_type(Some{Int}, Nothing) == Union{Some{Int},Nothing}

## convert()

Expand Down Expand Up @@ -75,6 +77,15 @@ for v in (nothing, missing)
@test coalesce(v, nothing) === nothing
@test coalesce(v, missing, v) === v
@test coalesce(v, nothing, v) === v

# issue #26927
a = [missing, nothing, Some(nothing), Some(missing)]
@test a isa Vector{Union{Missing, Nothing, Some}}
@test a[1] === missing && a[2] === nothing && a[3] === Some(nothing) && a[4] === Some(missing)
b = [ "replacement", "replacement", nothing, missing ]
@test b isa Vector{Union{Missing, Nothing, String}}
# the original operation from the issue, though it was not the source of the problem
@test all(coalesce.(a, "replacement") .=== b)
end

# notnothing()
Expand Down

2 comments on commit 0b1e166

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Executing the daily benchmark build, I will reply here when finished:

@nanosoldier runbenchmarks(ALL, isdaily = true)

@nanosoldier
Copy link
Collaborator

Choose a reason for hiding this comment

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

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @ararslan

Please sign in to comment.