From 604cbdcfddb959cd1d7de2f9afa14a199561a428 Mon Sep 17 00:00:00 2001
From: Aman Arora <me@aman-arora.com>
Date: Tue, 15 Dec 2020 23:00:19 -0500
Subject: [PATCH] Fix unused 'mut' warning for capture's root variable

---
 compiler/rustc_mir/src/borrow_check/mod.rs    | 37 ++++++++++++++++---
 .../run_pass/move_closure.rs                  | 25 +++++++++++--
 .../run_pass/move_closure.stderr              | 12 +-----
 .../run_pass/mut_ref_struct_mem.rs            | 26 ++++++++++++-
 4 files changed, 77 insertions(+), 23 deletions(-)

diff --git a/compiler/rustc_mir/src/borrow_check/mod.rs b/compiler/rustc_mir/src/borrow_check/mod.rs
index 1a771157e28..6e1e5c65aea 100644
--- a/compiler/rustc_mir/src/borrow_check/mod.rs
+++ b/compiler/rustc_mir/src/borrow_check/mod.rs
@@ -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()) {
-                    this.used_mut_upvars.push(field);
-                }
-            } else {
-                this.used_mut.insert(place.local);
+            // We have three possiblities 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
+            //    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;
+                }
+            }
+
+            // Handle(b)
+            this.used_mut.insert(place.local);
         };
 
         // This relies on the current way that by-value
diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs
index 83ed1c28462..4007a5a48aa 100644
--- a/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs
+++ b/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs
@@ -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();
 }
diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.stderr b/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.stderr
index 724b683bfbf..c1d8ba575d6 100644
--- a/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.stderr
+++ b/src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.stderr
@@ -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
 
diff --git a/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref_struct_mem.rs b/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref_struct_mem.rs
index 82e723cc825..2dba923647a 100644
--- a/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref_struct_mem.rs
+++ b/src/test/ui/closures/2229_closure_analysis/run_pass/mut_ref_struct_mem.rs
@@ -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);
@@ -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();
+}