Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix unused 'mut' warning for capture's root variable
  • Loading branch information
arora-aman committed Jan 29, 2021
commit 604cbdcfddb959cd1d7de2f9afa14a199561a428
33 changes: 29 additions & 4 deletions compiler/rustc_mir/src/borrow_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1369,13 +1369,38 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> {

fn propagate_closure_used_mut_upvar(&mut self, operand: &Operand<'tcx>) {
let propagate_closure_used_mut_place = |this: &mut Self, place: Place<'tcx>| {
if !place.projection.is_empty() {
if let Some(field) = this.is_upvar_field_projection(place.as_ref()) {
// We have three possiblities here:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// We have three possiblities here:
// We have three possibilities here:

// a. We are modifying something through a mut-ref
// b. We are modifying something that is local to our parent
// c. Current body is a nested clsoure, and we are modifying path starting from
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// c. Current body is a nested clsoure, and we are modifying path starting from
// c. Current body is a nested closure, and we are modifying a path starting from

// a Place captured by our parent closure.

// Handle (c), the path being modified is exactly the path captured by our parent
if let Some(field) = this.is_upvar_field_projection(place.as_ref()) {
this.used_mut_upvars.push(field);
return;
}

for (place_ref, proj) in place.iter_projections().rev() {
// Handle (a)
if proj == ProjectionElem::Deref {
match place_ref.ty(this.body(), this.infcx.tcx).ty.kind() {
// We aren't modifying a variable directly
ty::Ref(_, _, hir::Mutability::Mut) => return,

_ => {}
}
}

// Handle (c)
if let Some(field) = this.is_upvar_field_projection(place_ref) {
this.used_mut_upvars.push(field);
return;
}
} else {
this.used_mut.insert(place.local);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To handle the deref of &mut` case, we could check here if

  • the projection is a deref
  • the type of the place_ref is an &mut


// Handle(b)
this.used_mut.insert(place.local);
};

// This relies on the current way that by-value
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,36 @@ fn struct_contains_ref_to_another_struct() {
c();
}

fn no_ref() {
struct S(String);
struct T(S);
#[derive(Debug)]
struct S(String);

let t = T(S("s".into()));
#[derive(Debug)]
struct T(S);

fn no_ref() {
let mut t = T(S("s".into()));
let mut c = move || {
t.0.0 = "new S".into();
};
c();
}

fn no_ref_nested() {
let mut t = T(S("s".into()));
let c = || {
println!("{:?}", t.0);
let mut c = move || {
t.0.0 = "new S".into();
println!("{:?}", t.0.0);
};
c();
};
c();
}

fn main() {
simple_ref();
struct_contains_ref_to_another_struct();
no_ref();
no_ref_nested();
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,5 @@ LL | #![feature(capture_disjoint_fields)]
= note: `#[warn(incomplete_features)]` on by default
= note: see issue #53488 <https://github.com/rust-lang/rust/issues/53488> for more information

warning: variable does not need to be mutable
--> $DIR/move_closure.rs:36:9
|
LL | let mut t = T(S("s".into()));
| ----^
| |
| help: remove this `mut`
|
= note: `#[warn(unused_mut)]` on by default

warning: 2 warnings emitted
warning: 1 warning emitted

Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@

// Test that we can mutate a place through a mut-borrow
// that is captured by the closure
//

// More specifically we test that the if the mutable reference isn't root variable of a capture
// but rather accessed while acessing the precise capture.

#![feature(capture_disjoint_fields)]
//~^ WARNING: the feature `capture_disjoint_fields` is incomplete

fn main() {
fn mut_tuple() {
let mut t = (10, 10);

let t1 = (&mut t, 10);
Expand All @@ -21,3 +21,25 @@ fn main() {

c();
}

fn mut_tuple_nested() {
let mut t = (10, 10);

let t1 = (&mut t, 10);

let mut c = || {
let mut c = || {
// Mutable because (*t.0) is mutable
t1.0.0 += 10;
};

c();
};

c();
}

fn main() {
mut_tuple();
mut_tuple_nested();
}