Fix #31392, unaliasing of broadcast arguments against destinations wi…

…th repeated indices

When introducing the new broadcasting implementation (which came alongside greater
semantic guarantees with regards to aliasing source and destination), I found that
we needed a performance optimization for the common case where the destination was
`===` to an argument. This is quite common due to the `.op=` syntax and indeed is
safe in most cases as the iteration order between the destination and the source
is matched. This performance optimization is not safe, however, in the case that
the destination has multiple locations that refer to the same memory.

base/broadcast.jl

@@ -877,15 +877,34 @@ end
     end
 end
 
-# For broadcasted assignments like `broadcast!(f, A, ..., A, ...)`, where `A`
-# appears on both the LHS and the RHS of the `.=`, then we know we're only
-# going to make one pass through the array, and even though `A` is aliasing
-# against itself, the mutations won't affect the result as the indices on the
-# LHS and RHS will always match. This is not true in general, but with the `.op=`
-# syntax it's fairly common for an argument to be `===` a source.
-broadcast_unalias(dest, src) = dest === src ? src : unalias(dest, src)
+"""
+    broadcast_unalias(dest, src)
+
+`broadcast_unalias` is a simple extension of `Base.unalias` to enable an important
+performance optimization specific to the semantics of broadcasting.
+
+For broadcasted assignments like `broadcast!(f, A, ..., A, ...)`, where `A`
+appears on both the LHS and the RHS of the `.=`, then we know we're only
+going to make one pass through the array, and even though `A` is aliasing
+against itself, the mutations won't affect the result as the indices on the
+LHS and RHS will always match. As long as the indices do not repeat, the
+RHS does not need to be unaliased. This is particuclarly valuable with the `.op=`
+syntax since that makes it fairly common for an argument to be `===` a source.
+"""
+broadcast_unalias(dest, src) = dest === src && !potentially_self_aliased(dest) ? src : unalias(dest, src)
 broadcast_unalias(::Nothing, src) = src
 
+"""
+    potentially_self_aliased(A)
+
+Conservatively returns true if multiple locations in `A` might reference the same memory
+"""
+potentially_self_aliased(::DenseArray) = false
+potentially_self_aliased(A::StridedArray) = any(==(0), strides(A))
+potentially_self_aliased(A::SubArray) = any(!allunique, A.indices)
+potentially_self_aliased(A::Union{Base.ReshapedArray,Base.ReinterpretArray}) = potentially_self_aliased(A.parent)
+potentially_self_aliased(::Any) = true
+
 # Preprocessing a `Broadcasted` does two things:
 # * unaliases any arguments from `dest`
 # * "extrudes" the arguments where it is advantageous to pre-compute the broadcasted indices

test/broadcast.jl

@@ -791,6 +791,39 @@ end
     @test iszero.((big(0):big(2)) .- 1) == [false, true, false]
 end
 
+@testset "Issue #31392: In-place broadcasting with repeated indices" begin
+    x = [3]
+    @views x[[1,1]] .= x[[1,1]] .+ x[[1,1]]
+    @test x[] == 6
+    x[[1,1]] .+= 1
+    @test x[] == 7
+
+    x = [3]
+    @views x[[1,1]] .+= x[[1,1]]
+    @test x[] == 6
+    x[[1,1]] .+= 1
+    @test x[] == 7
+
+    for I in ([1,1], StepRangeLen(1, 0, 2))
+        x = [3]
+        @views x[I] .= x[I] .+ x[I]
+        @test x[] == 6
+
+        x = [3]
+        @views x[I] .+= x[I]
+        @test x[] == 6
+
+        for v in (view([3], I), reshape(view([3], I), 1, 2))
+            v .= v .+ v
+            @test v[1] == 6
+            v .+= v
+            @test v[1] == 12
+            v .+= 1
+            @test v[1] == 13
+        end
+    end
+end
+
 @testset "Issue #27775: Broadcast!ing over nested scalar operations" begin
     a = zeros(2)
     a .= 1 ./ (1 + 2)