Skip to content

Commit

Permalink
promote_op() improvements
Browse files Browse the repository at this point in the history
Use common promote_op() based on one() for all Number types:
this fixes promote_op(+, ::Bool), which returned Int, and
promote_op(==, ::Complex, ::Complex), which returned Complex{Bool}.
Add fallback definitions returning Any so that broadcast() computes
the appropriate return type, instead of using promote_type() which
is often wrong.

This fixes broadcast() when the return type is different from the input
type. Add systematic tests for operators and their return types.
  • Loading branch information
nalimilan committed Jul 2, 2016
1 parent 732828c commit 17dddc0
Show file tree
Hide file tree
Showing 11 changed files with 90 additions and 26 deletions.
4 changes: 0 additions & 4 deletions base/bool.jl
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,3 @@ fld(x::Bool, y::Bool) = div(x,y)
cld(x::Bool, y::Bool) = div(x,y)
rem(x::Bool, y::Bool) = y ? false : throw(DivideError())
mod(x::Bool, y::Bool) = rem(x,y)

promote_op(op, ::Type{Bool}, ::Type{Bool}) = typeof(op(true, true))
promote_op(::typeof(^), ::Type{Bool}, ::Type{Bool}) = Bool
promote_op{T<:Integer}(::typeof(^), ::Type{Bool}, ::Type{T}) = Bool
11 changes: 0 additions & 11 deletions base/complex.jl
Original file line number Diff line number Diff line change
Expand Up @@ -26,17 +26,6 @@ promote_rule{T<:Real,S<:Real}(::Type{Complex{T}}, ::Type{S}) =
promote_rule{T<:Real,S<:Real}(::Type{Complex{T}}, ::Type{Complex{S}}) =
Complex{promote_type(T,S)}

promote_op{T<:Real,S<:Real}(op, ::Type{Complex{T}}, ::Type{Complex{S}}) =
Complex{promote_op(op,T,S)}
promote_op{T<:Real,S<:Real}(op, ::Type{Complex{T}}, ::Type{S}) =
Complex{promote_op(op,T,S)}
promote_op{T<:Real,S<:Real}(op, ::Type{T}, ::Type{Complex{S}}) =
Complex{promote_op(op,T,S)}
promote_op{T<:Integer,S<:Integer}(::typeof(^), ::Type{T}, ::Type{Complex{S}}) =
Complex{Float64}
promote_op{T<:Integer,S<:Integer}(::typeof(.^), ::Type{T}, ::Type{Complex{S}}) =
Complex{Float64}

widen{T}(::Type{Complex{T}}) = Complex{widen(T)}

real(z::Complex) = z.re
Expand Down
2 changes: 0 additions & 2 deletions base/int.jl
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,6 @@ promote_rule{T<:BitSigned64}(::Type{UInt64}, ::Type{T}) = UInt64
promote_rule{T<:Union{UInt32, UInt64}}(::Type{T}, ::Type{Int128}) = Int128
promote_rule{T<:BitSigned}(::Type{UInt128}, ::Type{T}) = UInt128

promote_op{R<:Integer,S<:Integer}(op, ::Type{R}, ::Type{S}) = typeof(op(one(R), one(S)))

## traits ##

typemin(::Type{Int8 }) = Int8(-128)
Expand Down
7 changes: 7 additions & 0 deletions base/irrationals.jl
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,13 @@ promote_rule{s}(::Type{Irrational{s}}, ::Type{Float32}) = Float32
promote_rule{s,t}(::Type{Irrational{s}}, ::Type{Irrational{t}}) = Float64
promote_rule{s,T<:Number}(::Type{Irrational{s}}, ::Type{T}) = promote_type(Float64,T)

promote_op{S<:Irrational,T<:Irrational}(op::Any, ::Type{S}, ::Type{T}) =
promote_op(op, Float64, Float64)
promote_op{S<:Irrational,T<:Number}(op::Any, ::Type{S}, ::Type{T}) =
promote_op(op, Float64, T)
promote_op{S<:Irrational,T<:Number}(op::Any, ::Type{T}, ::Type{S}) =
promote_op(op, T, Float64)

convert(::Type{AbstractFloat}, x::Irrational) = Float64(x)
convert(::Type{Float16}, x::Irrational) = Float16(Float32(x))
convert{T<:Real}(::Type{Complex{T}}, x::Irrational) = convert(Complex{T}, convert(T,x))
Expand Down
12 changes: 12 additions & 0 deletions base/number.jl
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,16 @@ zero{T<:Number}(::Type{T}) = convert(T,0)
one(x::Number) = oftype(x,1)
one{T<:Number}(::Type{T}) = convert(T,1)

promote_op{R,S<:Number}(::Type{R}, ::Type{S}) = (@_pure_meta; R) # to fix ambiguities
function promote_op{T<:Number}(op, ::Type{T})
S = typeof(op(one(T)))
# preserve the most general (abstract) type when possible
return isleaftype(T) ? S : typejoin(S, T)
end
function promote_op{R<:Number,S<:Number}(op, ::Type{R}, ::Type{S})
T = typeof(op(one(R), one(S)))
# preserve the most general (abstract) type when possible
return isleaftype(R) && isleaftype(S) ? T : typejoin(R, S, T)
end

factorial(x::Number) = gamma(x + 1) # fallback for x not Integer
1 change: 1 addition & 0 deletions base/pkg/resolve/fieldvalue.jl
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ Base.typemin(::Type{FieldValue}) = (x=typemin(Int); y=typemin(VersionWeight); Fi

Base.:-(a::FieldValue, b::FieldValue) = FieldValue(a.l0-b.l0, a.l1-b.l1, a.l2-b.l2, a.l3-b.l3, a.l4-b.l4)
Base.:+(a::FieldValue, b::FieldValue) = FieldValue(a.l0+b.l0, a.l1+b.l1, a.l2+b.l2, a.l3+b.l3, a.l4+b.l4)
Base.promote_op(::Union{typeof(+), typeof(-)}, ::Type{FieldValue}, ::Type{FieldValue}) = FieldValue

function Base.isless(a::FieldValue, b::FieldValue)
a.l0 < b.l0 && return true
Expand Down
4 changes: 1 addition & 3 deletions base/promotion.jl
Original file line number Diff line number Diff line change
Expand Up @@ -222,10 +222,8 @@ minmax(x::Real, y::Real) = minmax(promote(x, y)...)
# for the multiplication of two types,
# promote_op{R<:MyType,S<:MyType}(::typeof(*), ::Type{R}, ::Type{S}) = MyType{multype(R,S)}
promote_op(::Any) = (@_pure_meta; Bottom)
promote_op(::Any, T) = (@_pure_meta; T)
promote_op(::Any, ::Any, ::Any...) = (@_pure_meta; Any)
promote_op{T}(::Type{T}, ::Any) = (@_pure_meta; T)
promote_op{R,S}(::Any, ::Type{R}, ::Type{S}) = (@_pure_meta; promote_type(R, S))
promote_op(op, T, S, U, V...) = (@_pure_meta; promote_op(op, T, promote_op(op, S, U, V...)))

## catch-alls to prevent infinite recursion when definitions are missing ##

Expand Down
7 changes: 2 additions & 5 deletions test/arrayops.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1408,7 +1408,7 @@ b = rand(6,7)
# return type declarations (promote_op)
module RetTypeDecl
using Base.Test
import Base: +, *, .*, zero
import Base: +, *, .*, convert

immutable MeterUnits{T,P} <: Number
val::T
Expand All @@ -1422,11 +1422,8 @@ module RetTypeDecl
(*){T,pow}(x::Int, y::MeterUnits{T,pow}) = MeterUnits{typeof(x*one(T)),pow}(x*y.val)
(*){T}(x::MeterUnits{T,1}, y::MeterUnits{T,1}) = MeterUnits{T,2}(x.val*y.val)
(.*){T}(x::MeterUnits{T,1}, y::MeterUnits{T,1}) = MeterUnits{T,2}(x.val*y.val)
zero{T,pow}(x::MeterUnits{T,pow}) = MeterUnits{T,pow}(zero(T))

Base.promote_op{R,S}(::typeof(+), ::Type{MeterUnits{R,1}}, ::Type{MeterUnits{S,1}}) = MeterUnits{promote_type(R,S),1}
convert{T,pow}(::Type{MeterUnits{T,pow}}, y::Real) = MeterUnits{T,pow}(convert(T,y))
Base.promote_op{R,S}(::typeof(*), ::Type{MeterUnits{R,1}}, ::Type{MeterUnits{S,1}}) = MeterUnits{promote_type(R,S),2}
Base.promote_op{R,S}(::typeof(.*), ::Type{MeterUnits{R,1}}, ::Type{MeterUnits{S,1}}) = MeterUnits{promote_type(R,S),2}

@test @inferred(m+[m,m]) == [m+m,m+m]
@test @inferred([m,m]+m) == [m+m,m+m]
Expand Down
15 changes: 14 additions & 1 deletion test/broadcast.jl
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ m = [1:2;]'
@test @inferred([0,1.2].+reshape([0,-2],1,1,2)) == reshape([0 -2; 1.2 -0.8],2,1,2)
rt = Base.return_types(.+, Tuple{Array{Float64, 3}, Array{Int, 1}})
@test length(rt) == 1 && rt[1] == Array{Float64, 3}
rt = Base.return_types(broadcast, Tuple{Function, Array{Float64, 3}, Array{Int, 1}})
rt = Base.return_types(broadcast, Tuple{typeof(.+), Array{Float64, 3}, Array{Int, 3}})
@test length(rt) == 1 && rt[1] == Array{Float64, 3}
rt = Base.return_types(broadcast!, Tuple{Function, Array{Float64, 3}, Array{Float64, 3}, Array{Int, 1}})
@test length(rt) == 1 && rt[1] == Array{Float64, 3}
Expand Down Expand Up @@ -200,3 +200,16 @@ end
# issue #4883
@test isa(broadcast(tuple, [1 2 3], ["a", "b", "c"]), Matrix{Tuple{Int,String}})
@test isa(broadcast((x,y)->(x==1?1.0:x,y), [1 2 3], ["a", "b", "c"]), Matrix{Tuple{Real,String}})
let a = length.(["foo", "bar"])
@test isa(a, Vector{Int})
@test a == [3, 3]
end
let a = sin.([1, 2])
@test isa(a, Vector{Float64})
@test a [0.8414709848078965, 0.9092974268256817]
end

# PR 16988
@test Base.promote_op(+, Bool) === Int
@test isa(broadcast(+, true), Array{Int,0})
@test Base.promote_op(Float64, Bool) === Float64
3 changes: 3 additions & 0 deletions test/linalg/dense.jl
Original file line number Diff line number Diff line change
Expand Up @@ -361,6 +361,9 @@ let
@test S*T == [z z; 0 0]
end

# similar issue for Array{Real}
@test Real[1 2] * Real[1.5; 2.0] == [5.5]

# Matrix exponential
for elty in (Float32, Float64, Complex64, Complex128)
A1 = convert(Matrix{elty}, [4 2 0; 1 4 1; 1 1 4])
Expand Down
50 changes: 50 additions & 0 deletions test/numbers.jl
Original file line number Diff line number Diff line change
Expand Up @@ -2762,3 +2762,53 @@ testmi(typemax(UInt32)-UInt32(1000):typemax(UInt32), map(UInt32, 1:100))

# issue #16282
@test_throws MethodError 3 // 4.5im

# PR #16995
let types = (Base.BitInteger_types..., BigInt, Bool,
Rational{Int}, Rational{BigInt},
Float16, Float32, Float64, BigFloat,
Complex{Int}, Complex{UInt}, Complex32, Complex64, Complex128)
for S in types
for op in (+, -)
T = @inferred Base.promote_op(op, S)
t = @inferred op(one(S))
@test T === typeof(t)
end
end

@test @inferred(Base.promote_op(!, Bool)) === Bool

for R in types, S in types
for op in (+, -, *, /, ^)
T = @inferred Base.promote_op(op, R, S)
t = @inferred op(one(R), one(S))
@test T === typeof(t)
end
end
end

let types = (Base.BitInteger_types..., BigInt, Bool,
Rational{Int}, Rational{BigInt},
Float16, Float32, Float64, BigFloat)
for S in types, T in types
for op in (<, >, <=, >=, (==))
@test @inferred(Base.promote_op(op, S, T)) === Bool
end
end
end

let types = (Base.BitInteger_types..., BigInt, Bool)
for S in types
T = @inferred Base.promote_op(~, S)
t = @inferred ~one(S)
@test T === typeof(t)
end

for S in types, T in types
for op in (&, |, <<, >>, (>>>), %, ÷)
T = @inferred Base.promote_op(op, S, T)
t = @inferred op(one(S), one(T))
@test T === typeof(t)
end
end
end

0 comments on commit 17dddc0

Please sign in to comment.