RFC: fix #31295, allow e.g. view(a,:) .+= view(b,:)

[JeffBezanson]

I'm not 100% sure what to do here, but basically for any LHS expression except a getindex this will be lowered to

temp = LHS
temp .= temp .+ RHS

So there's no fusion between the LHS and RHS, but that seems tricky.

[mbauman]

I think this lowering makes the most amount of sense here, but you're right that a dot-expression on the LHS does indeed throw a wrinkle into things.

Assuming A is a vector for simplicity, consider the expression:

f.(A) .+= g.(A)

I have two possible mental models for the above expression. I could imagine it being the equivalent of the for loop over the appropriate is such that it mimics the scalar expression (e.g., without the @. macro):

for i in eachindex(A)
    f(A[i]) += g(A[i])
end

or I can think of it as broadcast!ing into the expression on the LHS:

broadcast!((x,y)->f(x)+g(y), f.(A), A, A)

Neither of these models suggest a very useful semantic for the lowering here — the first is a syntax error and the second broadcasts into a temporary array that negates the whole raison d'être of .=.

So, I think I approve of your interpretation here — we'll throw up our hands and not worry about the potential of fusing the LHS on either side of the expression.

[mbauman]

I'm not a lisp reviewer, but I approve of what this PR says on the tin. :)

[stevengj]

Arguably we should do the same transformation for ref expressions a[...] except set temp = dotview(a, ...)

[bkamins]

The practical case when it is important is for instance (I give the simplest possible scenario):

julia> struct A
       a
       end

julia> x = A([1,2,3])
A([1, 2, 3])

julia> x.a .+= 1
3-element Array{Int64,1}:
 2
 3
 4

julia> x
A([2, 3, 4])

julia> getproperty(x, :a) .+= 1
ERROR: syntax: invalid assignment location "getproperty(x, :a)"

while this works:

julia> getproperty(x, :a) .= getproperty(x, :a) .+ 1
3-element Array{Int64,1}:
 3
 4
 5

julia> x
A([3, 4, 5])

The reason is that quite often you dynamically determine the symbol to point to a field (this happens often when you work with tabular data structures with special methods for getproperty).

As I am not a Lisp expert either I just want to make sure that we have it covered (I guess we have, but just to make sure).

[bkamins]

I think this lowering makes the most amount of sense here, but you're right that a dot-expression on the LHS does indeed throw a wrinkle into things.

Maybe it would be best to explicitly write in the documentation in what cases what kind of lowering happens so at least we are explicit?

[JeffBezanson]

Arguably we should do the same transformation for ref expressions a[...] except set temp = dotview(a, ...)

I added this change. It should be more efficient, since we can avoid an extra getindex call. Are we sure it's safe? No aliasing concerns?

[mbauman]

Are we sure it's safe? No aliasing concerns?

Yes, this should be safe and completely nonbreaking:

• The previous implementation required both getindex and view to be implemented for the array being assigned into.
• In the default broadcast implementation, we ensure that aliasing is not a problem.
• Even in other broadcast implementations that don't do explicit aliasing checks, this will still be safe (and indeed the default broadcast implementation won't make a preventative copy) because the iteration order on the LHS matches that on the RHS.

I approve!

[mbauman]

Well I had a blind spot in both my broadcasting alias analysis and the above reasoning; this PR does indeed change behaviors if there are repeated indices.

I do think this could be considered a bug given how the behaviors are different with and without @views.

Julia 1.1:

julia> x = [1]
       x[[1,1]] .+= 1
2-element view(::Array{Int64,1}, [1, 1]) with eltype Int64:
 2
 2

julia> x = [1]
       @views x[[1,1]] .+= 1
2-element view(::Array{Int64,1}, [1, 1]) with eltype Int64:
 3
 3

This PR makes the answer in both cases [3, 3]. Now: is this the behavior we want? Or is the true bug in Broadcast.broadcast_unalias, which assumes that if the destination is === to the source then our iteration order is well-defined? (Thanks to @willtebbutt for bringing this up on the Slack.)

[iamed2]

I do believe it should be [3, 3]. It makes sense to me that

x[[1,1]] .+= 1

is equivalent to

x[1] += 1
x[1] += 1

[mbauman]

I'm inclined to agree, but that mental model doesn't extend to our other unaliasing behaviors:

julia> x = [3,4]
       x .+= view(x, [2,1]) # This view gets unaliased and copied
       x
2-element Array{Int64,1}:
 7
 7

julia> x = [3,4]
       x[1] += x[2]# So it's not the same as the unrolled version
       x[2] += x[1]
       x
2-element Array{Int64,1}:
  7
 11

That's the simplification, but the same thing happens in @views x[1:2] .+= x[[1,1]] where the two feel even closer together.

[iamed2]

Ah so the alternate view is something like:

x[1] = xorig[1] + 1
x[1] = xorig[1] + 1

Hmm.

[JeffBezanson]

Looks like I should put the old behavior back pending a full decision on this.

[mbauman]

Thanks Jeff. We can move the larger design questions to #31392.

[bkamins]

Is this change going to be backported? (CC @nalimilan)