Skip to content

Commit

Permalink
Remove unspecialized reduce_empty
Browse files Browse the repository at this point in the history
The fallback definitions of `reduce_empty` and `mapreduce_empty` are
big targets for invalidation, and the benefit they bring is
questionable. Instead of having a dedicated error path, instead we
print a custom `MethodError` hint that is more informative than the
current message.

This fixes ~500 invalidations from DiffEqBase
  • Loading branch information
timholy committed Aug 17, 2021
1 parent a03392a commit f2dcc44
Show file tree
Hide file tree
Showing 9 changed files with 73 additions and 42 deletions.
6 changes: 5 additions & 1 deletion base/errorshow.jl
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@ function showerror(io::IO, ex::MethodError)
return showerror_ambiguous(io, meth, f, arg_types)
end
arg_types_param::SimpleVector = arg_types.parameters
show_candidates = true
print(io, "MethodError: ")
ft = typeof(f)
name = ft.name.mt.name
Expand All @@ -242,6 +243,9 @@ function showerror(io::IO, ex::MethodError)
if f === Base.convert && length(arg_types_param) == 2 && !is_arg_types
f_is_function = true
show_convert_error(io, ex, arg_types_param)
elseif f === mapreduce_empty || f === reduce_empty
print(io, "reducing over an empty collection is not allowed; consider supplying `init` to the reducer")
show_candidates = false
elseif isempty(methods(f)) && isa(f, DataType) && isabstracttype(f)
print(io, "no constructors have been defined for ", f)
elseif isempty(methods(f)) && !isa(f, Function) && !isa(f, Type)
Expand Down Expand Up @@ -314,7 +318,7 @@ function showerror(io::IO, ex::MethodError)
end
end
Experimental.show_error_hints(io, ex, arg_types_param, kwargs)
try
show_candidates && try
show_method_candidates(io, ex, kwargs)
catch ex
@error "Error showing method candidates, aborted" exception=ex,catch_backtrace()
Expand Down
38 changes: 25 additions & 13 deletions base/reduce.jl
Original file line number Diff line number Diff line change
Expand Up @@ -299,30 +299,42 @@ pairwise_blocksize(::typeof(abs2), ::typeof(+)) = 4096

# handling empty arrays
_empty_reduce_error() = throw(ArgumentError("reducing over an empty collection is not allowed"))
_empty_reduce_error(@nospecialize(f), @nospecialize(T::Type)) = throw(ArgumentError("""
reducing with $f over an empty collection of element type $T is not allowed.
You may be able to prevent this error by supplying an `init` value to the reducer."""))

"""
Base.reduce_empty(op, T)
The value to be returned when calling [`reduce`](@ref), [`foldl`](@ref) or [`foldr`](@ref)
with reduction `op` over an empty array with element type of `T`.
If not defined, this will throw an `ArgumentError`.
This should only be defined in unambiguous cases; for example,
```julia
Base.reduce_empty(::typeof(+), ::Type{T}) where T = zero(T)
```
is justified (the sum of zero elements is zero), whereas
`reduce_empty(::typeof(max), ::Type{Any})` is not (the maximum value of an empty collection
is generally ambiguous, and especially so when the element type is unknown).
As an alternative, consider supplying an `init` value to the reducer.
"""
reduce_empty(op, ::Type{T}) where {T} = _empty_reduce_error()
reduce_empty(::typeof(+), ::Type{Union{}}) = _empty_reduce_error()
reduce_empty(::typeof(+), ::Type{Union{}}) = _empty_reduce_error(+, Union{})
reduce_empty(::typeof(+), ::Type{T}) where {T} = zero(T)
reduce_empty(::typeof(+), ::Type{Bool}) = zero(Int)
reduce_empty(::typeof(*), ::Type{Union{}}) = _empty_reduce_error()
reduce_empty(::typeof(*), ::Type{Union{}}) = _empty_reduce_error(*, Union{})
reduce_empty(::typeof(*), ::Type{T}) where {T} = one(T)
reduce_empty(::typeof(*), ::Type{<:AbstractChar}) = ""
reduce_empty(::typeof(&), ::Type{Bool}) = true
reduce_empty(::typeof(|), ::Type{Bool}) = false

reduce_empty(::typeof(add_sum), ::Type{Union{}}) = _empty_reduce_error()
reduce_empty(::typeof(add_sum), ::Type{Union{}}) = _empty_reduce_error(add_sum, Union{})
reduce_empty(::typeof(add_sum), ::Type{T}) where {T} = reduce_empty(+, T)
reduce_empty(::typeof(add_sum), ::Type{T}) where {T<:SmallSigned} = zero(Int)
reduce_empty(::typeof(add_sum), ::Type{T}) where {T<:SmallUnsigned} = zero(UInt)
reduce_empty(::typeof(mul_prod), ::Type{Union{}}) = _empty_reduce_error()
reduce_empty(::typeof(mul_prod), ::Type{Union{}}) = _empty_reduce_error(mul_prod, Union{})
reduce_empty(::typeof(mul_prod), ::Type{T}) where {T} = reduce_empty(*, T)
reduce_empty(::typeof(mul_prod), ::Type{T}) where {T<:SmallSigned} = one(Int)
reduce_empty(::typeof(mul_prod), ::Type{T}) where {T<:SmallUnsigned} = one(UInt)
Expand All @@ -337,11 +349,8 @@ reduce_empty(op::FlipArgs, ::Type{T}) where {T} = reduce_empty(op.f, T)
The value to be returned when calling [`mapreduce`](@ref), [`mapfoldl`](@ref`) or
[`mapfoldr`](@ref) with map `f` and reduction `op` over an empty array with element type
of `T`.
If not defined, this will throw an `ArgumentError`.
of `T`. See [`Base.reduce_empty`](@ref) for more information.
"""
mapreduce_empty(f, op, T) = _empty_reduce_error()
mapreduce_empty(::typeof(identity), op, T) = reduce_empty(op, T)
mapreduce_empty(::typeof(abs), op, T) = abs(reduce_empty(op, T))
mapreduce_empty(::typeof(abs2), op, T) = abs2(reduce_empty(op, T))
Expand All @@ -355,7 +364,10 @@ mapreduce_empty_iter(f, op, itr, ItrEltype) =

@inline reduce_empty_iter(op, itr) = reduce_empty_iter(op, itr, IteratorEltype(itr))
@inline reduce_empty_iter(op, itr, ::HasEltype) = reduce_empty(op, eltype(itr))
reduce_empty_iter(op, itr, ::EltypeUnknown) = _empty_reduce_error()
reduce_empty_iter(op, itr, ::EltypeUnknown) = throw(ArgumentError("""
reducing over an empty collection of unknown element type is not allowed.
You may be able to prevent this error by supplying an `init` value to the reducer."""))


# handling of single-element iterators
"""
Expand Down Expand Up @@ -726,7 +738,7 @@ julia> maximum([1,2,3])
3
julia> maximum(())
ERROR: ArgumentError: reducing over an empty collection is not allowed
ERROR: MethodError: reducing over an empty collection is not allowed; consider supplying `init` to the reducer
Stacktrace:
[...]
Expand Down Expand Up @@ -758,7 +770,7 @@ julia> minimum([1,2,3])
1
julia> minimum([])
ERROR: ArgumentError: reducing over an empty collection is not allowed
ERROR: MethodError: reducing over an empty collection is not allowed; consider supplying `init` to the reducer
Stacktrace:
[...]
Expand Down
6 changes: 4 additions & 2 deletions stdlib/SparseArrays/test/sparse.jl
Original file line number Diff line number Diff line change
Expand Up @@ -786,6 +786,8 @@ end
end

@testset "empty cases" begin
errchecker(str) = occursin("reducing over an empty collection is not allowed", str) ||
occursin("collection slices must be non-empty", str)
@test sum(sparse(Int[])) === 0
@test prod(sparse(Int[])) === 1
@test_throws ArgumentError minimum(sparse(Int[]))
Expand All @@ -798,9 +800,9 @@ end
@test isequal(f(spzeros(0, 1), dims=3), f(Matrix{Int}(I, 0, 1), dims=3))
end
for f in (minimum, maximum, findmin, findmax)
@test_throws ArgumentError f(spzeros(0, 1), dims=1)
@test_throws errchecker f(spzeros(0, 1), dims=1)
@test isequal(f(spzeros(0, 1), dims=2), f(Matrix{Int}(I, 0, 1), dims=2))
@test_throws ArgumentError f(spzeros(0, 1), dims=(1, 2))
@test_throws errchecker f(spzeros(0, 1), dims=(1, 2))
@test isequal(f(spzeros(0, 1), dims=3), f(Matrix{Int}(I, 0, 1), dims=3))
end
end
Expand Down
3 changes: 2 additions & 1 deletion test/abstractarray.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1433,7 +1433,8 @@ using Base: typed_hvncat
v1 == v2 == 1 && continue
for v3 ((), (1,), ([1],), (1, [1]), ([1], 1), ([1], [1]))
@test_throws ArgumentError hvncat((v1, v2), true, v3...)
@test_throws ArgumentError hvncat(((v1,), (v2,)), true, v3...)
@test_throws str->(occursin("`shape` argument must consist of positive integers", str) ||
occursin("reducing over an empty collection is not allowed", str)) hvncat(((v1,), (v2,)), true, v3...)
end
end
end
Expand Down
12 changes: 6 additions & 6 deletions test/missing.jl
Original file line number Diff line number Diff line change
Expand Up @@ -465,10 +465,10 @@ end
@test_throws BoundsError x[3, 1]
@test findfirst(==(2), x) === nothing
@test isempty(findall(==(2), x))
@test_throws ArgumentError argmin(x)
@test_throws ArgumentError findmin(x)
@test_throws ArgumentError argmax(x)
@test_throws ArgumentError findmax(x)
@test_throws "reducing over an empty collection is not allowed" argmin(x)
@test_throws "reducing over an empty collection is not allowed" findmin(x)
@test_throws "reducing over an empty collection is not allowed" argmax(x)
@test_throws "reducing over an empty collection is not allowed" findmax(x)
end
end

Expand Down Expand Up @@ -525,8 +525,8 @@ end
for n in 0:3
itr = skipmissing(Vector{Union{Int,Missing}}(fill(missing, n)))
@test sum(itr) == reduce(+, itr) == mapreduce(identity, +, itr) === 0
@test_throws ArgumentError reduce(x -> x/2, itr)
@test_throws ArgumentError mapreduce(x -> x/2, +, itr)
@test_throws "reducing over an empty collection is not allowed" reduce(x -> x/2, itr)
@test_throws "reducing over an empty collection is not allowed" mapreduce(x -> x/2, +, itr)
end

# issue #35504
Expand Down
33 changes: 22 additions & 11 deletions test/reduce.jl
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ end
@test reduce(max, [8 6 7 5 3 0 9]) == 9
@test reduce(+, 1:5; init=1000) == (1000 + 1 + 2 + 3 + 4 + 5)
@test reduce(+, 1) == 1
@test_throws ArgumentError reduce(*, ())
@test_throws ArgumentError reduce(*, Union{}[])
@test_throws "reducing with * over an empty collection of element type Union{} is not allowed" reduce(*, ())
@test_throws "reducing with * over an empty collection of element type Union{} is not allowed" reduce(*, Union{}[])

# mapreduce
@test mapreduce(-, +, [-10 -9 -3]) == ((10 + 9) + 3)
Expand Down Expand Up @@ -87,8 +87,10 @@ end
@test mapreduce(abs2, *, Float64[]) === 1.0
@test mapreduce(abs2, max, Float64[]) === 0.0
@test mapreduce(abs, max, Float64[]) === 0.0
@test_throws ArgumentError mapreduce(abs2, &, Float64[])
@test_throws ArgumentError mapreduce(abs2, |, Float64[])
@test_throws ["reducing over an empty collection is not allowed",
"consider supplying `init`"] mapreduce(abs2, &, Float64[])
@test_throws str -> !occursin("Closest candidates are", str) mapreduce(abs2, &, Float64[])
@test_throws "reducing over an empty collection is not allowed" mapreduce(abs2, |, Float64[])

# mapreduce() type stability
@test typeof(mapreduce(*, +, Int8[10])) ===
Expand Down Expand Up @@ -138,8 +140,9 @@ fz = float(z)
@test sum(z) === 136
@test sum(fz) === 136.0

@test_throws ArgumentError sum(Union{}[])
@test_throws ArgumentError sum(sin, Int[])
@test_throws "reducing with add_sum over an empty collection of element type Union{} is not allowed" sum(Union{}[])
@test_throws ["reducing over an empty collection is not allowed",
"consider supplying `init`"] sum(sin, Int[])
@test sum(sin, 3) == sin(3.0)
@test sum(sin, [3]) == sin(3.0)
a = sum(sin, z)
Expand Down Expand Up @@ -170,7 +173,7 @@ for f in (sum2, sum5, sum6, sum9, sum10)
end
for f in (sum3, sum4, sum7, sum8)
@test sum(z) == f(z)
@test_throws ArgumentError f(Int[])
@test_throws "reducing over an empty" f(Int[])
@test sum(Int[7]) == f(Int[7]) == 7
end
@test typeof(sum(Int8[])) == typeof(sum(Int8[1])) == typeof(sum(Int8[1 7]))
Expand Down Expand Up @@ -239,8 +242,8 @@ prod2(itr) = invoke(prod, Tuple{Any}, itr)

# maximum & minimum & extrema

@test_throws ArgumentError maximum(Int[])
@test_throws ArgumentError minimum(Int[])
@test_throws "reducing over an empty" maximum(Int[])
@test_throws "reducing over an empty" minimum(Int[])

@test maximum(Int[]; init=-1) == -1
@test minimum(Int[]; init=-1) == -1
Expand Down Expand Up @@ -594,14 +597,22 @@ end
# issue #18695
test18695(r) = sum( t^2 for t in r )
@test @inferred(test18695([1.0,2.0,3.0,4.0])) == 30.0
@test_throws ArgumentError test18695(Any[])
@test_throws str -> ( occursin("reducing over an empty", str) &&
occursin("consider supplying `init`", str) &&
!occursin("or defining", str)) test18695(Any[])

# For Core.IntrinsicFunction
@test_throws str -> ( occursin("reducing over an empty", str) &&
occursin("consider supplying `init`", str) &&
!occursin("or defining", str)) reduce(Base.xor_int, Int[])

# issue #21107
@test foldr(-,2:2) == 2

# test neutral element not picked incorrectly for &, |
@test @inferred(foldl(&, Int[1])) === 1
@test_throws ArgumentError foldl(&, Int[])
@test_throws ["reducing over an empty",
"consider supplying `init`"] foldl(&, Int[])

# prod on Chars
@test prod(Char[]) == ""
Expand Down
11 changes: 6 additions & 5 deletions test/reducedim.jl
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ end

# Combining dims and init
A = Array{Int}(undef, 0, 3)
@test_throws ArgumentError maximum(A; dims=1)
@test_throws "reducing over an empty collection is not allowed" maximum(A; dims=1)
@test maximum(A; dims=1, init=-1) == reshape([-1,-1,-1], 1, 3)

# Test reduction along first dimension; this is special-cased for
Expand Down Expand Up @@ -169,8 +169,9 @@ end
A = Matrix{Int}(undef, 0,1)
@test sum(A) === 0
@test prod(A) === 1
@test_throws ArgumentError minimum(A)
@test_throws ArgumentError maximum(A)
@test_throws ["reducing over an empty",
"consider supplying `init`"] minimum(A)
@test_throws "consider supplying `init`" maximum(A)

@test isequal(sum(A, dims=1), zeros(Int, 1, 1))
@test isequal(sum(A, dims=2), zeros(Int, 0, 1))
Expand All @@ -182,9 +183,9 @@ end
@test isequal(prod(A, dims=3), fill(1, 0, 1))

for f in (minimum, maximum)
@test_throws ArgumentError f(A, dims=1)
@test_throws "reducing over an empty collection is not allowed" f(A, dims=1)
@test isequal(f(A, dims=2), zeros(Int, 0, 1))
@test_throws ArgumentError f(A, dims=(1, 2))
@test_throws "reducing over an empty collection is not allowed" f(A, dims=(1, 2))
@test isequal(f(A, dims=3), zeros(Int, 0, 1))
end
for f in (findmin, findmax)
Expand Down
4 changes: 2 additions & 2 deletions test/spawn.jl
Original file line number Diff line number Diff line change
Expand Up @@ -579,8 +579,8 @@ end
@test_throws ArgumentError run(Base.AndCmds(`$truecmd`, ``))

# tests for reducing over collection of Cmd
@test_throws ArgumentError reduce(&, Base.AbstractCmd[])
@test_throws ArgumentError reduce(&, Base.Cmd[])
@test_throws "reducing over an empty collection is not allowed" reduce(&, Base.AbstractCmd[])
@test_throws "reducing over an empty collection is not allowed" reduce(&, Base.Cmd[])
@test reduce(&, [`$echocmd abc`, `$echocmd def`, `$echocmd hij`]) == `$echocmd abc` & `$echocmd def` & `$echocmd hij`

# readlines(::Cmd), accidentally broken in #20203
Expand Down
2 changes: 1 addition & 1 deletion test/tuple.jl
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ end
@test mapfoldl(abs, =>, (-1,-2,-3,-4), init=-10) == ((((-10=>1)=>2)=>3)=>4)
@test mapfoldl(abs, =>, (), init=-10) == -10
@test mapfoldl(abs, Pair{Any,Any}, (-30:-1...,)) == mapfoldl(abs, Pair{Any,Any}, [-30:-1...,])
@test_throws ArgumentError mapfoldl(abs, =>, ())
@test_throws "reducing over an empty collection" mapfoldl(abs, =>, ())
end

@testset "filter" begin
Expand Down

0 comments on commit f2dcc44

Please sign in to comment.