MIR borrowck doesn't accept the example of iterating and updating a mutable reference#56649
MIR borrowck doesn't accept the example of iterating and updating a mutable reference#56649bors merged 3 commits intorust-lang:masterfrom
Conversation
This commit adds the test for writing into a projection of a local to confirm there are no remaining borrows.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
b43129f to
17f2976
Compare
|
This PR now takes a more reasonable approach, I expect it'll require some changes but it should be good to review now. |
17f2976 to
a3021a7
Compare
a3021a7 to
fb6c7fc
Compare
There was a problem hiding this comment.
There is a NOTE in a comment below this line, and I always wonder whether the directive in this comment has been (or is being) followed...
The local assigned_place has presumably been alpha-renamed, probably to lhs here)... but in any case, do we need to make modifications to propagate_call_return to make the logic here match up with the logic there? Are the comments still relevant in the first place?
There was a problem hiding this comment.
Okay, after thinking the matter through, I think the PR as it stands is sound; it is just more conservative than it could be, because I think we could apply the same place-overwritten borrow-killing logic to propagate_call_return, since that represents an overwrite of the dest_place that is passed to that method.
It would be best to first come up with a separate example illustrating this, rather than blindly adding the code to the PR.
There was a problem hiding this comment.
Experimented with adding similar logic to the propagate_call_return function for a test case that might have been right. Not sure it is possible to get the MIR building to generate a call terminator that assigns into a destination like (_1.0) which would be required for this.
This commit extends previous work to kill borrows from a local after assignment into that local to kill borrows from a projection after assignment into a prefix of that place.
fb6c7fc to
db635fc
Compare
This avoids all sorts of confusing issues with using both `dest_place` and `self` in the `propagate_call_return` function in the `BitDenotation` implementation for `Borrows`.
|
@bors r+ |
|
📌 Commit 7b628e1 has been approved by |
MIR borrowck doesn't accept the example of iterating and updating a mutable reference Fixes #46589. r? @pnkfelix or @nikomatsakis
|
☀️ Test successful - status-appveyor, status-travis |
Fixes #46589.
r? @pnkfelix or @nikomatsakis