From 1c2e59cc6915d5afd52d8224b809c7f0bf14cea7 Mon Sep 17 00:00:00 2001 From: Petr Vana Date: Thu, 8 Jul 2021 14:20:03 +0200 Subject: [PATCH 1/5] Fix sum() and prod() for tuples --- base/abstractarray.jl | 8 ++++---- base/tuple.jl | 11 ++++------- test/tuple.jl | 15 +++++++++++++++ 3 files changed, 23 insertions(+), 11 deletions(-) diff --git a/base/abstractarray.jl b/base/abstractarray.jl index 96b1b834e7d43..fa192195f87ca 100644 --- a/base/abstractarray.jl +++ b/base/abstractarray.jl @@ -270,7 +270,7 @@ julia> length([1 2; 3 4]) 4 ``` """ -length(t::AbstractArray) = (@_inline_meta; prod(size(t))) +length(t::AbstractArray) = (@_inline_meta; _prod_simple(size(t))) # `eachindex` is mostly an optimization of `keys` eachindex(itrs...) = keys(itrs...) @@ -584,7 +584,7 @@ end # This version is type-stable even if inds is heterogeneous function trailingsize(inds::Indices) @_inline_meta - prod(map(length, inds)) + _prod_simple(map(length, inds)) end ## Bounds checking ## @@ -2164,7 +2164,7 @@ function hvncat_fill!(A::Array, row_first::Bool, xs::Tuple) if row_first nr, nc = size(A, 1), size(A, 2) nrc = nr * nc - na = prod(size(A)[3:end]) + na = _prod_simple(size(A)[3:end]) k = 1 for d ∈ 1:na dd = nrc * (d - 1) @@ -2296,7 +2296,7 @@ function _typed_hvncat(::Type{T}, dims::Tuple{Vararg{Int, N}}, row_first::Bool, for a ∈ as len += cat_length(a) end - outlen = prod(outdims) + outlen = _prod_simple(outdims) outlen == 0 && throw(ArgumentError("too few elements in arguments, unable to infer dimensions")) len == outlen || throw(ArgumentError("too many elements in arguments; expected $(outlen), got $(len)")) diff --git a/base/tuple.jl b/base/tuple.jl index cc4e52c930b80..64164ccdc11d6 100644 --- a/base/tuple.jl +++ b/base/tuple.jl @@ -488,17 +488,14 @@ reverse(t::Tuple) = revargs(t...) ## specialized reduction ## -# TODO: these definitions cannot yet be combined, since +(x...) -# where x might be any tuple matches too many methods. -# TODO: this is inconsistent with the regular sum in cases where the arguments -# require size promotion to system size. -sum(x::Tuple{Any, Vararg{Any}}) = +(x...) +prod(x::Tuple{}) = 1 # NOTE: should remove, but often used on array sizes # TODO: this is inconsistent with the regular prod in cases where the arguments # require size promotion to system size. -prod(x::Tuple{}) = 1 -prod(x::Tuple{Any, Vararg{Any}}) = *(x...) +_prod_simple(x) = prod(x) +_prod_simple(x::Tuple{}) = 1 +_prod_simple(x::Tuple{Any, Vararg{Any}}) = *(x...) all(x::Tuple{}) = true all(x::Tuple{Bool}) = x[1] diff --git a/test/tuple.jl b/test/tuple.jl index 9b44e421184d3..f73fdcf69a433 100644 --- a/test/tuple.jl +++ b/test/tuple.jl @@ -361,6 +361,21 @@ end @test prod(()) === 1 @test prod((1,2,3)) === 6 + # issue 39182 + @test sum((0xe1, 0x1f)) === sum([0xe1, 0x1f]) + @test sum((Int8(3),)) === Int(3) + @test sum((UInt8(3),)) === UInt(3) + @test sum((3,)) === Int(3) + @test sum((3.0,)) === 3.0 + @test sum(["a"]) == sum(("a",)) + + # issue 39183 + @test prod((Int8(100), Int8(100))) === 10000 + @test prod((Int8(3),)) === Int(3) + @test prod((UInt8(3),)) === UInt(3) + @test prod((3,)) === Int(3) + @test prod((3.0,)) === 3.0 + @testset "all" begin @test all(()) === true @test all((false,)) === false From 94765139d3010aa79fa547755b8a7489a23a4696 Mon Sep 17 00:00:00 2001 From: Petr Vana Date: Thu, 8 Jul 2021 20:16:20 +0200 Subject: [PATCH 2/5] Reduce number of _prod_simple() occurrences --- base/abstractarray.jl | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/base/abstractarray.jl b/base/abstractarray.jl index fa192195f87ca..6229f2b9caa0c 100644 --- a/base/abstractarray.jl +++ b/base/abstractarray.jl @@ -584,7 +584,7 @@ end # This version is type-stable even if inds is heterogeneous function trailingsize(inds::Indices) @_inline_meta - _prod_simple(map(length, inds)) + prod(map(length, inds)) end ## Bounds checking ## @@ -2296,7 +2296,7 @@ function _typed_hvncat(::Type{T}, dims::Tuple{Vararg{Int, N}}, row_first::Bool, for a ∈ as len += cat_length(a) end - outlen = _prod_simple(outdims) + outlen = prod(outdims) outlen == 0 && throw(ArgumentError("too few elements in arguments, unable to infer dimensions")) len == outlen || throw(ArgumentError("too many elements in arguments; expected $(outlen), got $(len)")) From 69ee20965e813ed176381616b40148e517fcf81f Mon Sep 17 00:00:00 2001 From: Petr Vana Date: Thu, 5 Aug 2021 22:27:39 +0200 Subject: [PATCH 3/5] Reduce number of _prod_simple() occurrences thanks to #41201 --- base/abstractarray.jl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/abstractarray.jl b/base/abstractarray.jl index 9420e3231cd95..aa9253424a182 100644 --- a/base/abstractarray.jl +++ b/base/abstractarray.jl @@ -2278,7 +2278,7 @@ function hvncat_fill!(A::Array, row_first::Bool, xs::Tuple) if row_first nr, nc = size(A, 1), size(A, 2) nrc = nr * nc - na = _prod_simple(size(A)[3:end]) + na = prod(size(A)[3:end]) k = 1 for d ∈ 1:na dd = nrc * (d - 1) From f834cdb4809cb48301346d9a2f10731f707bb655 Mon Sep 17 00:00:00 2001 From: Petr Vana Date: Sun, 15 Aug 2021 12:34:17 +0200 Subject: [PATCH 4/5] Remove unnecessary _prod_simple function --- base/abstractarray.jl | 2 +- base/tuple.jl | 10 +++------- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/base/abstractarray.jl b/base/abstractarray.jl index aa9253424a182..d5d47fe855bd5 100644 --- a/base/abstractarray.jl +++ b/base/abstractarray.jl @@ -270,7 +270,7 @@ julia> length([1 2; 3 4]) 4 ``` """ -length(t::AbstractArray) = (@_inline_meta; _prod_simple(size(t))) +length(t::AbstractArray) = (@_inline_meta; prod(size(t))) # `eachindex` is mostly an optimization of `keys` eachindex(itrs...) = keys(itrs...) diff --git a/base/tuple.jl b/base/tuple.jl index 214945168c678..37148d43bc8a0 100644 --- a/base/tuple.jl +++ b/base/tuple.jl @@ -489,13 +489,9 @@ reverse(t::Tuple) = revargs(t...) ## specialized reduction ## prod(x::Tuple{}) = 1 - -# NOTE: should remove, but often used on array sizes -# TODO: this is inconsistent with the regular prod in cases where the arguments -# require size promotion to system size. -_prod_simple(x) = prod(x) -_prod_simple(x::Tuple{}) = 1 -_prod_simple(x::Tuple{Any, Vararg{Any}}) = *(x...) +# This is consistent with the regular prod because there is no need for size promotion +# if all elements in the tuple are of system size. +prod(x::Tuple{Int, Vararg{Int}}) = *(x...) all(x::Tuple{}) = true all(x::Tuple{Bool}) = x[1] From 5f3727c5ab4af6232235c263b78410d0cdbaf1fa Mon Sep 17 00:00:00 2001 From: Petr Vana Date: Thu, 26 Aug 2021 23:08:01 +0200 Subject: [PATCH 5/5] Apply suggestions from code review Co-authored-by: Nathan Daly --- base/tuple.jl | 2 ++ test/tuple.jl | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/base/tuple.jl b/base/tuple.jl index 37148d43bc8a0..789c48deb0732 100644 --- a/base/tuple.jl +++ b/base/tuple.jl @@ -491,6 +491,8 @@ reverse(t::Tuple) = revargs(t...) prod(x::Tuple{}) = 1 # This is consistent with the regular prod because there is no need for size promotion # if all elements in the tuple are of system size. +# It is defined here separately in order to support bootstrap, because it's needed earlier +# than the general prod definition is available. prod(x::Tuple{Int, Vararg{Int}}) = *(x...) all(x::Tuple{}) = true diff --git a/test/tuple.jl b/test/tuple.jl index f73fdcf69a433..907cb3696c44e 100644 --- a/test/tuple.jl +++ b/test/tuple.jl @@ -367,7 +367,8 @@ end @test sum((UInt8(3),)) === UInt(3) @test sum((3,)) === Int(3) @test sum((3.0,)) === 3.0 - @test sum(["a"]) == sum(("a",)) + @test sum(("a",)) == sum(["a"]) + @test sum((0xe1, 0x1f), init=0x0) == sum([0xe1, 0x1f], init=0x0) # issue 39183 @test prod((Int8(100), Int8(100))) === 10000 @@ -375,6 +376,8 @@ end @test prod((UInt8(3),)) === UInt(3) @test prod((3,)) === Int(3) @test prod((3.0,)) === 3.0 + @test prod(("a",)) == prod(["a"]) + @test prod((0xe1, 0x1f), init=0x1) == prod([0xe1, 0x1f], init=0x1) @testset "all" begin @test all(()) === true