Skip to content

Commit

Permalink
fix promote_op function
Browse files Browse the repository at this point in the history
This function had some weird and broken extraneous code,
using Core.Inference directly is not recommended,
but where we are going to do it anyways, we should at least do it correctly.

The `cumsum` code had a test to assert that `Real + Real` was convertable to `Real`.
This was hacked into the code poorly. It is now hacked in directly.
We can separately debate if this is a good idea,
I am simply preserving the existing behavior of the code
but implementing it correctly (by changing the behavior of the actual function,
instead of by convincing inference to return the incorrect answer).
  • Loading branch information
vtjnash committed Oct 20, 2018
1 parent 2885b62 commit 67621cc
Show file tree
Hide file tree
Showing 11 changed files with 25 additions and 50 deletions.
4 changes: 2 additions & 2 deletions base/broadcast.jl
Expand Up @@ -610,7 +610,7 @@ broadcastable(x) = collect(x)
broadcastable(::Union{AbstractDict, NamedTuple}) = throw(ArgumentError("broadcasting over dictionaries and `NamedTuple`s is reserved")) broadcastable(::Union{AbstractDict, NamedTuple}) = throw(ArgumentError("broadcasting over dictionaries and `NamedTuple`s is reserved"))


## Computation of inferred result type, for empty and concretely inferred cases only ## Computation of inferred result type, for empty and concretely inferred cases only
_broadcast_getindex_eltype(bc::Broadcasted) = Base._return_type(bc.f, eltypes(bc.args)) _broadcast_getindex_eltype(bc::Broadcasted) = Base._unsafe_return_type(bc.f, eltypes(bc.args))
_broadcast_getindex_eltype(A) = eltype(A) # Tuple, Array, etc. _broadcast_getindex_eltype(A) = eltype(A) # Tuple, Array, etc.


eltypes(::Tuple{}) = Tuple{} eltypes(::Tuple{}) = Tuple{}
Expand All @@ -619,7 +619,7 @@ eltypes(t::Tuple{Any,Any}) = Tuple{_broadcast_getindex_eltype(t[1]), _broadcast_
eltypes(t::Tuple) = Tuple{_broadcast_getindex_eltype(t[1]), eltypes(tail(t)).types...} eltypes(t::Tuple) = Tuple{_broadcast_getindex_eltype(t[1]), eltypes(tail(t)).types...}


# Inferred eltype of result of broadcast(f, args...) # Inferred eltype of result of broadcast(f, args...)
combine_eltypes(f, args::Tuple) = Base._return_type(f, eltypes(args)) combine_eltypes(f, args::Tuple) = Base._unsafe_return_type(f, eltypes(args))


## Broadcasting core ## Broadcasting core


Expand Down
4 changes: 0 additions & 4 deletions base/complex.jl
Expand Up @@ -991,7 +991,3 @@ function complex(A::AbstractArray{T}) where T
end end
convert(AbstractArray{typeof(complex(zero(T)))}, A) convert(AbstractArray{typeof(complex(zero(T)))}, A)
end end

## promotion to complex ##

_default_type(T::Type{Complex}) = Complex{Int}
2 changes: 0 additions & 2 deletions base/float.jl
Expand Up @@ -381,8 +381,6 @@ promote_rule(::Type{Float64}, ::Type{Float32}) = Float64
widen(::Type{Float16}) = Float32 widen(::Type{Float16}) = Float32
widen(::Type{Float32}) = Float64 widen(::Type{Float32}) = Float64


_default_type(T::Union{Type{Real},Type{AbstractFloat}}) = Float64

## floating point arithmetic ## ## floating point arithmetic ##
-(x::Float64) = neg_float(x) -(x::Float64) = neg_float(x)
-(x::Float32) = neg_float(x) -(x::Float32) = neg_float(x)
Expand Down
3 changes: 0 additions & 3 deletions base/int.jl
Expand Up @@ -632,9 +632,6 @@ promote_rule(::Type{UInt32}, ::Type{Int32} ) = UInt32
promote_rule(::Type{UInt64}, ::Type{Int64} ) = UInt64 promote_rule(::Type{UInt64}, ::Type{Int64} ) = UInt64
promote_rule(::Type{UInt128}, ::Type{Int128}) = UInt128 promote_rule(::Type{UInt128}, ::Type{Int128}) = UInt128


_default_type(::Type{Unsigned}) = UInt
_default_type(::Union{Type{Integer},Type{Signed}}) = Int

## traits ## ## traits ##


""" """
Expand Down
2 changes: 1 addition & 1 deletion base/intfuncs.jl
Expand Up @@ -169,7 +169,7 @@ end
invmod(n::Integer, m::Integer) = invmod(promote(n,m)...) invmod(n::Integer, m::Integer) = invmod(promote(n,m)...)


# ^ for any x supporting * # ^ for any x supporting *
to_power_type(x) = convert(promote_op(*, typeof(x), typeof(x)), x) to_power_type(x) = convert(Base._unsafe_return_type(*, Tuple{typeof(x), typeof(x)}), x)
@noinline throw_domerr_powbysq(::Any, p) = throw(DomainError(p, @noinline throw_domerr_powbysq(::Any, p) = throw(DomainError(p,
string("Cannot raise an integer x to a negative power ", p, '.', string("Cannot raise an integer x to a negative power ", p, '.',
"\nConvert input to float."))) "\nConvert input to float.")))
Expand Down
4 changes: 2 additions & 2 deletions base/iterators.jl
Expand Up @@ -1054,7 +1054,7 @@ else
# Try to find an appropriate type for the (value, state tuple), # Try to find an appropriate type for the (value, state tuple),
# by doing a recursive unrolling of the iteration protocol up to # by doing a recursive unrolling of the iteration protocol up to
# fixpoint. # fixpoint.
approx_iter_type(itrT::Type) = _approx_iter_type(itrT, Base._return_type(iterate, Tuple{itrT})) approx_iter_type(itrT::Type) = _approx_iter_type(itrT, Base._unsafe_return_type(iterate, Tuple{itrT}))
# Not actually called, just passed to return type to avoid # Not actually called, just passed to return type to avoid
# having to typesubtract # having to typesubtract
function doiterate(itr, valstate::Union{Nothing, Tuple{Any, Any}}) function doiterate(itr, valstate::Union{Nothing, Tuple{Any, Any}})
Expand All @@ -1065,7 +1065,7 @@ else
function _approx_iter_type(itrT::Type, vstate::Type) function _approx_iter_type(itrT::Type, vstate::Type)
vstate <: Union{Nothing, Tuple{Any, Any}} || return Any vstate <: Union{Nothing, Tuple{Any, Any}} || return Any
vstate <: Union{} && return Union{} vstate <: Union{} && return Union{}
nextvstate = Base._return_type(doiterate, Tuple{itrT, vstate}) nextvstate = Base._unsafe_return_type(doiterate, Tuple{itrT, vstate})
return (nextvstate <: vstate ? vstate : Any) return (nextvstate <: vstate ? vstate : Any)
end end
end end
Expand Down
2 changes: 0 additions & 2 deletions base/number.jl
Expand Up @@ -320,8 +320,6 @@ julia> import Dates; oneunit(Dates.Day)
oneunit(x::T) where {T} = T(one(x)) oneunit(x::T) where {T} = T(one(x))
oneunit(::Type{T}) where {T} = T(one(T)) oneunit(::Type{T}) where {T} = T(one(T))


_default_type(::Type{Number}) = Int

""" """
big(T::Type) big(T::Type)
Expand Down
26 changes: 5 additions & 21 deletions base/promotion.jl
Expand Up @@ -364,15 +364,10 @@ max(x::Real, y::Real) = max(promote(x,y)...)
min(x::Real, y::Real) = min(promote(x,y)...) min(x::Real, y::Real) = min(promote(x,y)...)
minmax(x::Real, y::Real) = minmax(promote(x, y)...) minmax(x::Real, y::Real) = minmax(promote(x, y)...)


# "Promotion" that takes a function into account and tries to preserve
# non-concrete types. These are meant to be used mainly by elementwise
# operations, so it is advised against overriding them
_default_type(T::Type) = T

if isdefined(Core, :Compiler) if isdefined(Core, :Compiler)
const _return_type = Core.Compiler.return_type const _unsafe_return_type = Core.Compiler.return_type
else else
_return_type(@nospecialize(f), @nospecialize(t)) = Any _unsafe_return_type(@nospecialize(f), @nospecialize(t)) = Any
end end


""" """
Expand All @@ -381,28 +376,17 @@ end
Guess what an appropriate container eltype would be for storing results of Guess what an appropriate container eltype would be for storing results of
`f(::argtypes...)`. The guess is in part based on type inference, so can change any time. `f(::argtypes...)`. The guess is in part based on type inference, so can change any time.
!!! warning
In pathological cases, the type returned by `promote_op(f, argtypes...)` may not even
be a supertype of the return value of `f(::argtypes...)`. Therefore, `promote_op`
should _not_ be used e.g. in the preallocation of an output array.
!!! warning !!! warning
Due to its fragility, use of `promote_op` should be avoided. It is preferable to base Due to its fragility, use of `promote_op` should be avoided. It is preferable to base
the container eltype on the type of the actual elements. Only in the absence of any the container eltype on the type of the actual elements. Only in the absence of any
elements (for an empty result container), it may be unavoidable to call `promote_op`. elements (for an empty result container), it may be unavoidable to call `promote_op`.
""" """
promote_op(::Any...) = Any promote_op(::Any...) = Any
function promote_op(f, ::Type{S}) where S function promote_op(f, ::Type{S}) where S
TT = Tuple{_default_type(S)} return _unsafe_return_type(f, Tuple{S})
T = _return_type(f, TT)
isdispatchtuple(Tuple{S}) && return isdispatchtuple(Tuple{T}) ? T : Any
return typejoin(S, T)
end end
function promote_op(f, ::Type{R}, ::Type{S}) where {R,S} function promote_op(f, ::Type{R}, ::Type{S}) where {R, S}
TT = Tuple{_default_type(R), _default_type(S)} return _unsafe_return_type(f, Tuple{R, S})
T = _return_type(f, TT)
isdispatchtuple(Tuple{R}) && isdispatchtuple(Tuple{S}) && return isdispatchtuple(Tuple{T}) ? T : Any
return typejoin(R, S, T)
end end


## catch-alls to prevent infinite recursion when definitions are missing ## ## catch-alls to prevent infinite recursion when definitions are missing ##
Expand Down
20 changes: 11 additions & 9 deletions base/reduce.jl
Expand Up @@ -13,24 +13,26 @@ else
end end


""" """
Base.add_sum(x,y) Base.add_sum(x, y)
The reduction operator used in `sum`. The main difference from [`+`](@ref) is that small The reduction operator used in `sum`. The main difference from [`+`](@ref) is that small
integers are promoted to `Int`/`UInt`. integers are promoted to `Int`/`UInt`.
""" """
add_sum(x,y) = x + y add_sum(x, y) = x + y
add_sum(x::SmallSigned,y::SmallSigned) = Int(x) + Int(y) add_sum(x::SmallSigned, y::SmallSigned) = Int(x) + Int(y)
add_sum(x::SmallUnsigned,y::SmallUnsigned) = UInt(x) + UInt(y) add_sum(x::SmallUnsigned, y::SmallUnsigned) = UInt(x) + UInt(y)
add_sum(x::Real, y::Real)::Real = x + y


""" """
Base.mul_prod(x,y) Base.mul_prod(x, y)
The reduction operator used in `prod`. The main difference from [`*`](@ref) is that small The reduction operator used in `prod`. The main difference from [`*`](@ref) is that small
integers are promoted to `Int`/`UInt`. integers are promoted to `Int`/`UInt`.
""" """
mul_prod(x,y) = x * y mul_prod(x, y) = x * y
mul_prod(x::SmallSigned,y::SmallSigned) = Int(x) * Int(y) mul_prod(x::SmallSigned, y::SmallSigned) = Int(x) * Int(y)
mul_prod(x::SmallUnsigned,y::SmallUnsigned) = UInt(x) * UInt(y) mul_prod(x::SmallUnsigned, y::SmallUnsigned) = UInt(x) * UInt(y)
mul_prod(x::Real, y::Real)::Real = x * y


## foldl && mapfoldl ## foldl && mapfoldl


Expand All @@ -56,7 +58,7 @@ function mapfoldl_impl(f, op, nt::NamedTuple{()}, itr)
end end
(x, i) = y (x, i) = y
init = mapreduce_first(f, op, x) init = mapreduce_first(f, op, x)
mapfoldl_impl(f, op, (init=init,), itr, i) return mapfoldl_impl(f, op, (init=init,), itr, i)
end end




Expand Down
2 changes: 1 addition & 1 deletion stdlib/LinearAlgebra/src/adjtrans.jl
@@ -1,6 +1,6 @@
# This file is a part of Julia. License is MIT: https://julialang.org/license # This file is a part of Julia. License is MIT: https://julialang.org/license


using Base: @propagate_inbounds, _return_type, _default_type, @_inline_meta using Base: @propagate_inbounds, @_inline_meta
import Base: length, size, axes, IndexStyle, getindex, setindex!, parent, vec, convert, similar import Base: length, size, axes, IndexStyle, getindex, setindex!, parent, vec, convert, similar


### basic definitions (types, aliases, constructors, abstractarray interface, sundry similar) ### basic definitions (types, aliases, constructors, abstractarray interface, sundry similar)
Expand Down
6 changes: 3 additions & 3 deletions stdlib/LinearAlgebra/src/uniformscaling.jl
Expand Up @@ -103,7 +103,7 @@ for (t1, t2) in ((:UnitUpperTriangular, :UpperTriangular),
(:UnitLowerTriangular, :LowerTriangular)) (:UnitLowerTriangular, :LowerTriangular))
@eval begin @eval begin
function (+)(UL::$t1, J::UniformScaling) function (+)(UL::$t1, J::UniformScaling)
ULnew = copy_oftype(UL.data, Base._return_type(+, Tuple{eltype(UL), typeof(J)})) ULnew = copy_oftype(UL.data, Base._unsafe_return_type(+, Tuple{eltype(UL), typeof(J)}))
for i in axes(ULnew, 1) for i in axes(ULnew, 1)
ULnew[i,i] = one(ULnew[i,i]) + J ULnew[i,i] = one(ULnew[i,i]) + J
end end
Expand All @@ -114,7 +114,7 @@ end


function (+)(A::AbstractMatrix, J::UniformScaling) function (+)(A::AbstractMatrix, J::UniformScaling)
checksquare(A) checksquare(A)
B = copy_oftype(A, Base._return_type(+, Tuple{eltype(A), typeof(J)})) B = copy_oftype(A, Base._unsafe_return_type(+, Tuple{eltype(A), typeof(J)}))
@inbounds for i in axes(A, 1) @inbounds for i in axes(A, 1)
B[i,i] += J B[i,i] += J
end end
Expand All @@ -123,7 +123,7 @@ end


function (-)(J::UniformScaling, A::AbstractMatrix) function (-)(J::UniformScaling, A::AbstractMatrix)
checksquare(A) checksquare(A)
B = convert(AbstractMatrix{Base._return_type(+, Tuple{eltype(A), typeof(J)})}, -A) B = convert(AbstractMatrix{Base._unsafe_return_type(+, Tuple{eltype(A), typeof(J)})}, -A)
@inbounds for i in axes(A, 1) @inbounds for i in axes(A, 1)
B[i,i] += J B[i,i] += J
end end
Expand Down

0 comments on commit 67621cc

Please sign in to comment.