sum!, prod!, any!, and all! may silently return incorrect results

[yurivish]

I noticed that sum! and prod! do not check for aliasing, leading to incorrect results when the same array is passed in both arguments. The incorrect behavior under aliasing is not documented and the result is a silent unexpected wrong result.

julia> a = [1 2 3];

julia> sum!(copy(a), a)
1×3 Matrix{Int64}:
 1  2  3

julia> sum!(a, a)
1×3 Matrix{Int64}:
 0  0  0

julia> a = [1 2 3];

julia> prod!(copy(a), a)
1×3 Matrix{Int64}:
 1  2  3

julia> prod!(a, a)
1×3 Matrix{Int64}:
 1  1  1

Not all mutating functions have this problem and cumsum! can be used this way without error:

julia> a = [1, 2, 3];

julia> cumsum!(a, a)
3-element Vector{Int64}:
 1
 3
 6

Other functions correctly flag the aliasing in the case of === arguments and throw an error:

julia> using LinearAlgebra

julia> a = rand(2, 2);

julia> mul!(a, a, a)
ERROR: ArgumentError: output matrix must not be aliased with input matrix

The examples above were generated on Julia 1.6.0-beta1 and I see the same behavior in version 1.5.3.

Edit: I noticed this issue also applies to any! and all!:

julia> let a = [true, false]
           any!(a, a)
       end
2-element Array{Bool,1}:
 0
 0

julia> let a = [true, false]
           any!(copy(a), a)
       end
2-element Array{Bool,1}:
 1
 0

julia> let a = [true, false]
           all!(a, a)
       end
2-element Array{Bool,1}:
 1
 1

julia> let a = [true, false]
           all!(copy(a), a)
       end
2-element Array{Bool,1}:
 1
 0

[barucden]

So what is the correct approach here? Add an alias check similar to that in LinearAlgebra.mul!? If so, it might be enough to add the check here https://github.com/JuliaLang/julia/blob/master/base/reducedim.jl#L908.

[mcabbott]

Maybe worth noting that

julia> sum!(a, a; init=false)
1×3 Matrix{Int64}:
 2  4  6

This is Base.mapreducedim!(identity, +, Base.initarray!(a, Base.add_sum, false, a), a), and the difference is whether initarray! does anything. (prod! and all! etc. come to the same place.)

[StefanKarpinski]

Can @mbauman's Base.mightalias help here?

[mbauman]

I mean, in all these cases it needs to collapse to a no-op. Can we avoid worrying about aliasing at all with a simple size(out) == size(in) && copyto!(out, in)?

[mcabbott]

I'm not sure what should happen, but a tricker example is:

julia> mat = [100 2 3; 4 5 6];

julia> sum!(view(mat,:,1), mat);

julia> mat
2×3 Matrix{Int64}:
  5  2  3
 11  5  6

The perhaps-comparable @views mul!(mat[:,[1,3]], mat[:,2:3], mat[:,1:2]) runs without complaint.

[nalimilan]

@mbauman You're right that calling size(out) == size(in) && copyto!(out, in) is probably a good idea anyway, but as @mcabbott showed this is not enough to catch all problematic cases. Do you think we should call Base.mightalias then? And should we error or make a copy of the input if it aliases the output?

At any rate, docstrings should mention this.

[adienes]

should this bug be release blocking at any point? I feel that it is

1. surprising
2. easy to encounter
3. uses only "basic" functions and objects

[oscardssmith]

at the very least, we could document that this is a case where weirdness can occur

[ufechner7]

Please, add this to the 1.10 milestone... This is a correctness issue, not so difficult to fix, and correctness should have priority for any function in Base or the standard library.