Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Evaluation order when using mutating operations with destructuring assignment #40574

Closed
simonbyrne opened this issue Apr 22, 2021 · 1 comment · Fixed by #40737
Closed

Evaluation order when using mutating operations with destructuring assignment #40574

simonbyrne opened this issue Apr 22, 2021 · 1 comment · Fixed by #40737
Labels
compiler:lowering Syntax lowering (compiler front end, 2nd stage)

Comments

@simonbyrne
Copy link
Contributor 

julia> x = [1,2]
2-element Vector{Int64}:
 1
 2

julia> x[2],x[1] = x
2-element Vector{Int64}:
 1
 1

This is caused by mutating the iterator as it is being iterated: i.e. it is being lowered to

julia> x = [1,2]
2-element Vector{Int64}:
 1
 2

julia> x[2], state = iterate(x)
(1, 2)

julia> x[1], state = iterate(x, state)
(1, 3)

julia> x
2-element Vector{Int64}:
 1
 1

From @simeonschaub on Slack:

For a non-trivial lhs, we should probably always assign the rhs to temporary variables first and only then call setindex! etc.

cc: @JeffBezanson @vtjnash
@JeffBezanson
Copy link
Member

I'm not 100% sure this is bad; iterating and assigning left-to-right is at least "obvious".

@simeonschaub simeonschaub added the compiler:lowering Syntax lowering (compiler front end, 2nd stage) label May 6, 2021
simeonschaub added a commit that referenced this issue May 6, 2021
@simeonschaub
RFC: don't mutate lhs while destructuring
4d07e2f 
@JeffBezanson was a bit sceptical about this in #40574, but I think this
is worth doing since it seems strictly more useful than the current
behavior and we already go out of our way in cases like `x, y = y, x` to
only assign to the lhs after the rhs has been evaluated, so I think
handling `x[2], x[1] = x` in a similar way would only be consisten. As a
general rule that assignment will always happen after destructuring does
not seem much less obvious than the current behavior to me. This might
technically be slightly breaking, but I would be surprised if people
relied on the current behavior here. Nevertheless, it would probably
good to run PkgEval if we decide to go with this change.

closes #40574
@simeonschaub simeonschaub mentioned this issue May 6, 2021
Merged
simeonschaub added a commit that referenced this issue May 6, 2021
@simeonschaub
RFC: don't mutate lhs while destructuring
bcccabf 
Jeff was a bit sceptical about this in #40574, but I think this
is worth doing since it seems strictly more useful than the current
behavior and we already go out of our way in cases like `x, y = y, x` to
only assign to the lhs after the rhs has been evaluated, so I think
handling `x[2], x[1] = x` in a similar way would only be consistent. As
a general rule that assignment will always happen after destructuring
does not seem much less obvious than the current behavior to me. This
might technically be slightly breaking, but I would be surprised if
people relied on the current behavior here. Nevertheless, it would
probably be good to run PkgEval if we decide to go with this change.

closes #40574
simeonschaub added a commit that referenced this issue May 27, 2021
@simeonschaub
RFC: don't mutate lhs while destructuring
823c6e0 
Jeff was a bit sceptical about this in #40574, but I think this
is worth doing since it seems strictly more useful than the current
behavior and we already go out of our way in cases like `x, y = y, x` to
only assign to the lhs after the rhs has been evaluated, so I think
handling `x[2], x[1] = x` in a similar way would only be consistent. As
a general rule that assignment will always happen after destructuring
does not seem much less obvious than the current behavior to me. This
might technically be slightly breaking, but I would be surprised if
people relied on the current behavior here. Nevertheless, it would
probably be good to run PkgEval if we decide to go with this change.

closes #40574
simeonschaub added a commit that referenced this issue May 27, 2021
@simeonschaub
don't mutate lhs while destructuring
030bf53 
Jeff was a bit sceptical about this in #40574, but I think this
is worth doing since it seems strictly more useful than the current
behavior and we already go out of our way in cases like `x, y = y, x` to
only assign to the lhs after the rhs has been evaluated, so I think
handling `x[2], x[1] = x` in a similar way would only be consistent. As
a general rule that assignment will always happen after destructuring
does not seem much less obvious than the current behavior to me. This
might technically be slightly breaking, but I would be surprised if
people relied on the current behavior here. Nevertheless, it would
probably be good to run PkgEval if we decide to go with this change.

closes #40574
simeonschaub added a commit that referenced this issue May 28, 2021
@simeonschaub
don't mutate lhs while destructuring (#40737)
d692b89 
Jeff was a bit sceptical about this in #40574, but I think this
is worth doing since it seems strictly more useful than the current
behavior and we already go out of our way in cases like `x, y = y, x` to
only assign to the lhs after the rhs has been evaluated, so I think
handling `x[2], x[1] = x` in a similar way would only be consistent. As
a general rule that assignment will always happen after destructuring
does not seem much less obvious than the current behavior to me. This
might technically be slightly breaking, but I would be surprised if
people relied on the current behavior here. Nevertheless, it would
probably be good to run PkgEval if we decide to go with this change.

closes #40574
shirodkara pushed a commit to shirodkara/julia that referenced this issue Jun 9, 2021
@simeonschaub
don't mutate lhs while destructuring (JuliaLang#40737)
b00020f 
Jeff was a bit sceptical about this in JuliaLang#40574, but I think this
is worth doing since it seems strictly more useful than the current
behavior and we already go out of our way in cases like `x, y = y, x` to
only assign to the lhs after the rhs has been evaluated, so I think
handling `x[2], x[1] = x` in a similar way would only be consistent. As
a general rule that assignment will always happen after destructuring
does not seem much less obvious than the current behavior to me. This
might technically be slightly breaking, but I would be surprised if
people relied on the current behavior here. Nevertheless, it would
probably be good to run PkgEval if we decide to go with this change.

closes JuliaLang#40574
johanmon pushed a commit to johanmon/julia that referenced this issue Jul 5, 2021
@simeonschaub @johanmon
don't mutate lhs while destructuring (JuliaLang#40737)
fdffd63 
Jeff was a bit sceptical about this in JuliaLang#40574, but I think this
is worth doing since it seems strictly more useful than the current
behavior and we already go out of our way in cases like `x, y = y, x` to
only assign to the lhs after the rhs has been evaluated, so I think
handling `x[2], x[1] = x` in a similar way would only be consistent. As
a general rule that assignment will always happen after destructuring
does not seem much less obvious than the current behavior to me. This
might technically be slightly breaking, but I would be surprised if
people relied on the current behavior here. Nevertheless, it would
probably be good to run PkgEval if we decide to go with this change.

closes JuliaLang#40574
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
compiler:lowering Syntax lowering (compiler front end, 2nd stage)
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

don't mutate lhs while destructuring JuliaLang/julia
3 participants
@JeffBezanson @simonbyrne @simeonschaub