RFC: Fix #31392, unaliasing of broadcast arguments against destinations with repeated indices

base/broadcast.jl

@@ -969,15 +969,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)
+
+Returns true if multiple locations in `A` reference the same memory
+"""
+potentially_self_aliased(::DenseArray) = false
+potentially_self_aliased(A::StridedArray) = any(==(0), strides(A))
+potentially_self_aliased(A::SubArray) = any(map(!allunique, A.indices))
+potentially_self_aliased(A::Union{Base.ReshapedArray,Base.ReinterpretArray}) = potentially_self_aliased(A.parent)
+potentially_self_aliased(::Any) = false
+
 # Preprocessing a `Broadcasted` does two things:
 # * unaliases any arguments from `dest`
 # * "extrudes" the arguments where it is advantageous to pre-compute the broadcasted indices

base/set.jl

@@ -416,8 +416,8 @@ function allunique(C)
 end
 
 allunique(::Union{AbstractSet,AbstractDict}) = true
-
 allunique(r::AbstractRange) = !iszero(step(r)) || length(r) <= 1
+allunique(::Number) = true
 
 filter!(f, s::Set) = unsafe_filter!(f, s)
 

test/broadcast.jl

@@ -800,6 +800,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)