Skip to content

Commit 7da178c

Browse files
committed
Fix promote_op() and some corner-case operators on Number
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}. Also fix a few corner cases which did not work or were type-instable. Add systematic tests to catch this kind of bug.
1 parent cd39c6b commit 7da178c

File tree

11 files changed

+91
-27
lines changed

11 files changed

+91
-27
lines changed

base/bool.jl

-4
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,3 @@ fld(x::Bool, y::Bool) = div(x,y)
7272
cld(x::Bool, y::Bool) = div(x,y)
7373
rem(x::Bool, y::Bool) = y ? false : throw(DivideError())
7474
mod(x::Bool, y::Bool) = rem(x,y)
75-
76-
promote_op(op, ::Type{Bool}, ::Type{Bool}) = typeof(op(true, true))
77-
promote_op(::typeof(^), ::Type{Bool}, ::Type{Bool}) = Bool
78-
promote_op{T<:Integer}(::typeof(^), ::Type{Bool}, ::Type{T}) = Bool

base/complex.jl

+1-12
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,6 @@ promote_rule{T<:Real,S<:Real}(::Type{Complex{T}}, ::Type{S}) =
2626
promote_rule{T<:Real,S<:Real}(::Type{Complex{T}}, ::Type{Complex{S}}) =
2727
Complex{promote_type(T,S)}
2828

29-
promote_op{T<:Real,S<:Real}(op, ::Type{Complex{T}}, ::Type{Complex{S}}) =
30-
Complex{promote_op(op,T,S)}
31-
promote_op{T<:Real,S<:Real}(op, ::Type{Complex{T}}, ::Type{S}) =
32-
Complex{promote_op(op,T,S)}
33-
promote_op{T<:Real,S<:Real}(op, ::Type{T}, ::Type{Complex{S}}) =
34-
Complex{promote_op(op,T,S)}
35-
promote_op{T<:Integer,S<:Integer}(::typeof(^), ::Type{T}, ::Type{Complex{S}}) =
36-
Complex{Float64}
37-
promote_op{T<:Integer,S<:Integer}(::typeof(.^), ::Type{T}, ::Type{Complex{S}}) =
38-
Complex{Float64}
39-
4029
widen{T}(::Type{Complex{T}}) = Complex{widen(T)}
4130

4231
real(z::Complex) = z.re
@@ -461,7 +450,7 @@ function ^{T<:AbstractFloat}(z::Complex{T}, p::Complex{T})
461450
if p==2 #square
462451
zr, zi = reim(z)
463452
x = (zr-zi)*(zr+zi)
464-
y = 2zr*zi
453+
y = T(2)*zr*zi
465454
if isnan(x)
466455
if isinf(y)
467456
x = copysign(zero(T),zr)

base/gmp.jl

+1
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,7 @@ end
424424
^(x::BigInt , y::Bool ) = y ? x : one(x)
425425
^(x::BigInt , y::Integer) = bigint_pow(x, y)
426426
^(x::Integer, y::BigInt ) = bigint_pow(BigInt(x), y)
427+
^(x::Bool , y::BigInt ) = Base.power_by_squaring(x, y)
427428

428429
function powermod(x::BigInt, p::BigInt, m::BigInt)
429430
r = BigInt()

base/int.jl

-2
Original file line numberDiff line numberDiff line change
@@ -305,8 +305,6 @@ promote_rule{T<:BitSigned64}(::Type{UInt64}, ::Type{T}) = UInt64
305305
promote_rule{T<:Union{UInt32, UInt64}}(::Type{T}, ::Type{Int128}) = Int128
306306
promote_rule{T<:BitSigned}(::Type{UInt128}, ::Type{T}) = UInt128
307307

308-
promote_op{R<:Integer,S<:Integer}(op, ::Type{R}, ::Type{S}) = typeof(op(one(R), one(S)))
309-
310308
## traits ##
311309

312310
typemin(::Type{Int8 }) = Int8(-128)

base/intfuncs.jl

+2-2
Original file line numberDiff line numberDiff line change
@@ -186,7 +186,7 @@ ndigits0z(x::Integer) = ndigits0z(unsigned(abs(x)))
186186

187187
const ndigits_max_mul = Core.sizeof(Int) == 4 ? 69000000 : 290000000000000000
188188

189-
function ndigits0znb(n::Int, b::Int)
189+
function ndigits0znb(n::Integer, b::Integer)
190190
d = 0
191191
while n != 0
192192
n = cld(n,b)
@@ -198,7 +198,7 @@ end
198198
function ndigits0z(n::Unsigned, b::Int)
199199
d = 0
200200
if b < 0
201-
d = ndigits0znb(signed(n), b)
201+
d = ndigits0znb(n, b)
202202
else
203203
b == 2 && return (sizeof(n)<<3-leading_zeros(n))
204204
b == 8 && return div((sizeof(n)<<3)-leading_zeros(n)+2,3)

base/irrationals.jl

+3
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,9 @@ promote_rule{s}(::Type{Irrational{s}}, ::Type{Float32}) = Float32
1010
promote_rule{s,t}(::Type{Irrational{s}}, ::Type{Irrational{t}}) = Float64
1111
promote_rule{s,T<:Number}(::Type{Irrational{s}}, ::Type{T}) = promote_type(Float64,T)
1212

13+
promote_op{S<:Irrational,T<:Number}(op::Any, ::Type{S}, ::Type{T}) = promote_op(op, Int, T)
14+
promote_op{S<:Irrational,T<:Number}(op::Any, ::Type{T}, ::Type{S}) = promote_op(op, Int, T)
15+
1316
convert(::Type{AbstractFloat}, x::Irrational) = Float64(x)
1417
convert(::Type{Float16}, x::Irrational) = Float16(Float32(x))
1518
convert{T<:Real}(::Type{Complex{T}}, x::Irrational) = convert(Complex{T}, convert(T,x))

base/number.jl

+17
Original file line numberDiff line numberDiff line change
@@ -63,4 +63,21 @@ zero{T<:Number}(::Type{T}) = convert(T,0)
6363
one(x::Number) = oftype(x,1)
6464
one{T<:Number}(::Type{T}) = convert(T,1)
6565

66+
promote_op{S<:Number,T}(op::Type{T}, ::Type{S}) = T # to fix ambiguities
67+
function promote_op{S<:Number}(op::Any, ::Type{S})
68+
R = typeof(op(one(S)))
69+
R <: S ? S : R # preserve the most general (abstract) type when possible
70+
end
71+
function promote_op{S<:Number,T<:Number}(op::Any, ::Type{S}, ::Type{T})
72+
R = typeof(op(one(S), one(T)))
73+
# preserve the most general (abstract) type when possible
74+
if T <: S && R <: S
75+
return S
76+
elseif S <: T && R <: T
77+
return T
78+
else
79+
return R
80+
end
81+
end
82+
6683
factorial(x::Number) = gamma(x + 1) # fallback for x not Integer

test/arrayops.jl

+4-5
Original file line numberDiff line numberDiff line change
@@ -1404,7 +1404,7 @@ b = rand(6,7)
14041404
# return type declarations (promote_op)
14051405
module RetTypeDecl
14061406
using Base.Test
1407-
import Base: +, *, .*, zero
1407+
import Base: +, *, .*, zero, one
14081408

14091409
immutable MeterUnits{T,P} <: Number
14101410
val::T
@@ -1419,10 +1419,9 @@ module RetTypeDecl
14191419
(*){T}(x::MeterUnits{T,1}, y::MeterUnits{T,1}) = MeterUnits{T,2}(x.val*y.val)
14201420
(.*){T}(x::MeterUnits{T,1}, y::MeterUnits{T,1}) = MeterUnits{T,2}(x.val*y.val)
14211421
zero{T,pow}(x::MeterUnits{T,pow}) = MeterUnits{T,pow}(zero(T))
1422-
1423-
Base.promote_op{R,S}(::typeof(+), ::Type{MeterUnits{R,1}}, ::Type{MeterUnits{S,1}}) = MeterUnits{promote_type(R,S),1}
1424-
Base.promote_op{R,S}(::typeof(*), ::Type{MeterUnits{R,1}}, ::Type{MeterUnits{S,1}}) = MeterUnits{promote_type(R,S),2}
1425-
Base.promote_op{R,S}(::typeof(.*), ::Type{MeterUnits{R,1}}, ::Type{MeterUnits{S,1}}) = MeterUnits{promote_type(R,S),2}
1422+
one{T,pow}(x::MeterUnits{T,pow}) = MeterUnits{T,pow}(one(T))
1423+
zero{T,pow}(x::Type{MeterUnits{T,pow}}) = MeterUnits{T,pow}(zero(T))
1424+
one{T,pow}(x::Type{MeterUnits{T,pow}}) = MeterUnits{T,pow}(one(T))
14261425

14271426
@test @inferred(m+[m,m]) == [m+m,m+m]
14281427
@test @inferred([m,m]+m) == [m+m,m+m]

test/broadcast.jl

+7-2
Original file line numberDiff line numberDiff line change
@@ -165,9 +165,9 @@ m = [1:2;]'
165165
@test @inferred([0,1.2].+reshape([0,-2],1,1,2)) == reshape([0 -2; 1.2 -0.8],2,1,2)
166166
rt = Base.return_types(.+, Tuple{Array{Float64, 3}, Array{Int, 1}})
167167
@test length(rt) == 1 && rt[1] == Array{Float64, 3}
168-
rt = Base.return_types(broadcast, Tuple{Function, Array{Float64, 3}, Array{Int, 1}})
168+
rt = Base.return_types(broadcast, Tuple{typeof(+), Array{Float64, 3}, Array{Int, 1}})
169169
@test length(rt) == 1 && rt[1] == Array{Float64, 3}
170-
rt = Base.return_types(broadcast!, Tuple{Function, Array{Float64, 3}, Array{Float64, 3}, Array{Int, 1}})
170+
rt = Base.return_types(broadcast!, Tuple{typeof(+), Array{Float64, 3}, Array{Float64, 3}, Array{Int, 1}})
171171
@test length(rt) == 1 && rt[1] == Array{Float64, 3}
172172

173173
# f.(args...) syntax (#15032)
@@ -196,3 +196,8 @@ end
196196
let a = broadcast(Float32, [3, 4, 5])
197197
@test eltype(a) == Float32
198198
end
199+
200+
# PR 16988
201+
@test Base.promote_op(+, Bool) === Int
202+
@test isa(broadcast(+, true), Array{Int,0})
203+
@test Base.promote_op(Float64, Bool) === Float64

test/linalg/dense.jl

+3
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,9 @@ let
361361
@test S*T == [z z; 0 0]
362362
end
363363

364+
# similar issue for Array{Real}
365+
@test Real[1 2] * Real[1.5; 2.0] == [5.5]
366+
364367
# Matrix exponential
365368
for elty in (Float32, Float64, Complex64, Complex128)
366369
A1 = convert(Matrix{elty}, [4 2 0; 1 4 1; 1 1 4])

test/numbers.jl

+53
Original file line numberDiff line numberDiff line change
@@ -2742,3 +2742,56 @@ testmi(typemin(Int)+1:typemin(Int)+1000, -100:100)
27422742
@test_throws ArgumentError Base.multiplicativeinverse(0)
27432743
testmi(map(UInt32, 0:1000), map(UInt32, 1:100))
27442744
testmi(typemax(UInt32)-UInt32(1000):typemax(UInt32), map(UInt32, 1:100))
2745+
2746+
# PR #16995
2747+
let types = (Base.BitInteger_types..., BigInt, Bool,
2748+
Rational{Int}, Rational{BigInt},
2749+
Float16, Float32, Float64, BigFloat,
2750+
Complex{Int}, Complex{UInt}, Complex32, Complex64, Complex128)
2751+
for S in types
2752+
for op in (+, -)
2753+
@test Base.promote_op(op, S) === typeof(op(one(S)))
2754+
@inferred Base.promote_op(op, S)
2755+
@inferred op(one(S))
2756+
end
2757+
end
2758+
2759+
@test Base.promote_op(!, Bool) === Bool
2760+
@inferred Base.promote_op(!, Bool)
2761+
2762+
for S in types, T in types
2763+
for op in (+, -, *, /, ^, (==))
2764+
@test Base.promote_op(op, S, T) === typeof(op(one(S), one(T)))
2765+
@inferred Base.promote_op(op, S, T)
2766+
@inferred op(one(S), one(T))
2767+
end
2768+
end
2769+
end
2770+
2771+
let types = (Base.BitInteger_types..., BigInt, Bool,
2772+
Rational{Int}, Rational{BigInt},
2773+
Float16, Float32, Float64, BigFloat)
2774+
for S in types, T in types
2775+
for op in (<, >, <=, >=)
2776+
@test Base.promote_op(op, S, T) === typeof(op(one(S), one(T)))
2777+
@inferred Base.promote_op(op, S, T)
2778+
@inferred op(one(S), one(T))
2779+
end
2780+
end
2781+
end
2782+
2783+
let types = (Base.BitInteger_types..., BigInt, Bool)
2784+
for S in types
2785+
@test Base.promote_op(~, S) === typeof(~one(S))
2786+
@inferred Base.promote_op(~, S)
2787+
@inferred ~one(S)
2788+
end
2789+
2790+
for S in types, T in types
2791+
for op in (&, |, <<, >>, (>>>), %, ÷)
2792+
@test Base.promote_op(op, S, T) === typeof(op(one(S), one(T)))
2793+
@inferred Base.promote_op(op, S, T)
2794+
@inferred op(one(S), one(T))
2795+
end
2796+
end
2797+
end

0 commit comments

Comments
 (0)