2229: Handle MutBorrow/UniqueImmBorrow better #87676
Conversation
rust-highfive
added
the
S-waiting-on-review
We only want to use UniqueImmBorrow when the capture place is truncated and we drop Deref of a MutRef. r? @nikomatsakis
arora-aman
force-pushed
the
truncate_unique
branch
from
1778590
to
8e89971
Compare
joelpalmer
added
S-waiting-on-review
| @@ -1843,7 +1885,7 @@ fn adjust_for_move_closure<'tcx>( | |||
| _ if first_deref.is_some() => { | |||
| let place = match first_deref { | |||
| Some(idx) => { | |||
| place.projections.truncate(idx); | |||
| let (place, _) = truncate_place_to_len(place, kind, idx); | |||
There was a problem hiding this comment.
why don't we care about kind here?
There was a problem hiding this comment.
is this equivalent to just calling truncate(idx)?
There was a problem hiding this comment.
Yes can just do a regular truncate here because we instantly replace the kind with ByValue. I just used the function for consistency.
| /// contained `Deref` of `&mut`. | ||
| fn truncate_place_to_len( | ||
| mut place: Place<'tcx>, | ||
| curr_mode: ty::UpvarCapture<'tcx>, |
There was a problem hiding this comment.
I wonder if we want this to be
truncate_place_to_len(mut place: Place<'tcx>, curr_mode: &mut ty::UpvarCapture<'tcx>, len: usize)or even
truncate_place_to_len(place: &mut Place<'tcx>, curr_mode: &mut ty::UpvarCapture<'tcx>, len: usize)It seemed like a lot of the callers (almost all) were doing
let (..., mode) = truncate_place_to_len(place, curr_mode, ...);
curr_mode = mode;which is a bit silly.
There was a problem hiding this comment.
My thoughts:
- Looking at just the function all it's not directly evident that the
modehas changed, might need a better name. - Some codepaths don't really have a
&mut Place(eg: within Delegate -- we need to handle repr packed) so that might need adding a line to create an ownedPlacevariable.
Can we use unstable features? #71126 looks is near stabilization and the syntax is what I'd prefer, i.e. directly update the variables than destructure and then assign
There was a problem hiding this comment.
We are permitted to use unstable features, yes.
|
@bors r+ |
|
📌 Commit 8e89971 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
We only want to use UniqueImmBorrow when the capture place is truncated and we
drop Deref of a MutRef.
r? @nikomatsakis
Fixes: rust-lang/project-rfc-2229#56