2229: Drop any deref in move closure

Author: arora-aman

compiler/rustc_typeck/src/check/upvar.rs

@@ -1896,31 +1896,20 @@ fn restrict_capture_precision<'tcx>(
     return (place, curr_mode);
 }
 
-/// Take ownership if data being accessed is owned by the variable used to access it
-/// (or if closure attempts to move data that it doesn’t own).
-/// Note: When taking ownership, only capture data found on the stack.
+/// Truncate deref of any reference.
 fn adjust_for_move_closure<'tcx>(
     mut place: Place<'tcx>,
     mut kind: ty::UpvarCapture<'tcx>,
 ) -> (Place<'tcx>, ty::UpvarCapture<'tcx>) {
-    let contains_deref_of_ref = place.deref_tys().any(|ty| ty.is_ref());
     let first_deref = place.projections.iter().position(|proj| proj.kind == ProjectionKind::Deref);
 
-    match kind {
-        ty::UpvarCapture::ByRef(..) if contains_deref_of_ref => (place, kind),
-
-        // If there's any Deref and the data needs to be moved into the closure body,
-        // or it's a Deref of a Box, truncate the path to the first deref
-        _ => {
-            if let Some(idx) = first_deref {
-                truncate_place_to_len_and_update_capture_kind(&mut place, &mut kind, idx);
-            }
-
-            // AMAN: I think we don't need the span inside the ByValue anymore
-            //       we have more detailed span in CaptureInfo
-            (place, ty::UpvarCapture::ByValue(None))
-        }
+    if let Some(idx) = first_deref {
+        truncate_place_to_len_and_update_capture_kind(&mut place, &mut kind, idx);
     }
+
+    // AMAN: I think we don't need the span inside the ByValue anymore
+    //       we have more detailed span in CaptureInfo
+    (place, ty::UpvarCapture::ByValue(None))
 }
 
 /// Adjust closure capture just that if taking ownership of data, only move data

src/test/ui/closures/2229_closure_analysis/move_closure.rs

@@ -35,7 +35,7 @@ fn simple_ref() {
     //~| ERROR: Min Capture analysis includes:
         *ref_s += 10;
         //~^ NOTE: Capturing ref_s[Deref] -> MutBorrow
-        //~| NOTE: Min Capture ref_s[Deref] -> MutBorrow
+        //~| NOTE: Min Capture ref_s[] -> ByValue
     };
     c();
 }
@@ -56,7 +56,7 @@ fn struct_contains_ref_to_another_struct_1() {
     //~| ERROR: Min Capture analysis includes:
         t.0.0 = "new s".into();
         //~^ NOTE: Capturing t[(0, 0),Deref,(0, 0)] -> MutBorrow
-        //~| NOTE: Min Capture t[(0, 0),Deref,(0, 0)] -> MutBorrow
+        //~| NOTE: Min Capture t[(0, 0)] -> ByValue
     };
 
     c();
@@ -79,7 +79,7 @@ fn struct_contains_ref_to_another_struct_2() {
     //~| ERROR: Min Capture analysis includes:
         let _t = t.0.0;
         //~^ NOTE: Capturing t[(0, 0),Deref,(0, 0)] -> ImmBorrow
-        //~| NOTE: Min Capture t[(0, 0),Deref] -> ImmBorrow
+        //~| NOTE: Min Capture t[(0, 0)] -> ByValue
     };
 
     c();
@@ -175,7 +175,7 @@ fn box_mut_1() {
     //~| First Pass analysis includes:
     //~| NOTE: Capturing box_p_foo[Deref,Deref,(0, 0)] -> MutBorrow
     //~| Min Capture analysis includes:
-    //~| NOTE: Min Capture box_p_foo[Deref,Deref,(0, 0)] -> MutBorrow
+    //~| NOTE: Min Capture box_p_foo[] -> ByValue
 }
 
 // Ensure that even in move closures, if the data is not owned by the root variable
@@ -192,7 +192,7 @@ fn box_mut_2() {
     //~| First Pass analysis includes:
     //~| NOTE: Capturing p_foo[Deref,Deref,(0, 0)] -> MutBorrow
     //~| Min Capture analysis includes:
-    //~| NOTE: Min Capture p_foo[Deref,Deref,(0, 0)] -> MutBorrow
+    //~| NOTE: Min Capture p_foo[] -> ByValue
 }
 
 // Test that move closures can take ownership of Copy type

src/test/ui/closures/2229_closure_analysis/move_closure.stderr

@@ -187,7 +187,7 @@ LL | |
 LL | |     };
    | |_____^
    |
-note: Min Capture ref_s[Deref] -> MutBorrow
+note: Min Capture ref_s[] -> ByValue
   --> $DIR/move_closure.rs:36:9
    |
 LL |         *ref_s += 10;
@@ -223,7 +223,7 @@ LL | |
 LL | |     };
    | |_____^
    |
-note: Min Capture t[(0, 0),Deref,(0, 0)] -> MutBorrow
+note: Min Capture t[(0, 0)] -> ByValue
   --> $DIR/move_closure.rs:57:9
    |
 LL |         t.0.0 = "new s".into();
@@ -259,7 +259,7 @@ LL | |
 LL | |     };
    | |_____^
    |
-note: Min Capture t[(0, 0),Deref] -> ImmBorrow
+note: Min Capture t[(0, 0)] -> ByValue
   --> $DIR/move_closure.rs:80:18
    |
 LL |         let _t = t.0.0;
@@ -427,7 +427,7 @@ error: Min Capture analysis includes:
 LL |     let c = #[rustc_capture_analysis] move || box_p_foo.x += 10;
    |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^
    |
-note: Min Capture box_p_foo[Deref,Deref,(0, 0)] -> MutBorrow
+note: Min Capture box_p_foo[] -> ByValue
   --> $DIR/move_closure.rs:172:47
    |
 LL |     let c = #[rustc_capture_analysis] move || box_p_foo.x += 10;
@@ -451,7 +451,7 @@ error: Min Capture analysis includes:
 LL |     let c = #[rustc_capture_analysis] move || p_foo.x += 10;
    |                                       ^^^^^^^^^^^^^^^^^^^^^
    |
-note: Min Capture p_foo[Deref,Deref,(0, 0)] -> MutBorrow
+note: Min Capture p_foo[] -> ByValue
   --> $DIR/move_closure.rs:189:47
    |
 LL |     let c = #[rustc_capture_analysis] move || p_foo.x += 10;

src/test/ui/closures/2229_closure_analysis/run_pass/issue-88431.rs

@@ -0,0 +1,59 @@
+// edition:2021
+// check-pass
+
+use std::collections::HashMap;
+use std::future::Future;
+use std::pin::Pin;
+
+pub struct GameMode {}
+
+struct GameStateManager<'a> {
+    gamestate_stack: Vec<Box<dyn GameState<'a> + 'a>>,
+}
+
+pub trait GameState<'a> {}
+
+async fn construct_gamestate_replay<'a>(
+    _gamemode: &GameMode,
+    _factory: &mut GameStateManager<'a>,
+) -> Box<dyn GameState<'a> + 'a> {
+    unimplemented!()
+}
+
+type FutureGameState<'a, 'b> = Pin<Box<dyn Future<Output = Box<dyn GameState<'a> + 'a>> + 'b>>;
+
+struct MenuOption<'a> {
+    command: Box<dyn for<'b> Fn(&'b mut GameStateManager<'a>) -> FutureGameState<'a, 'b> + 'a>,
+}
+
+impl<'a> MenuOption<'a> {
+    fn new(
+        _command: impl for<'b> Fn(&'b mut GameStateManager<'a>) -> FutureGameState<'a, 'b> + 'a,
+    ) -> Self {
+        unimplemented!()
+    }
+}
+
+struct MenuState<'a> {
+    options: Vec<MenuOption<'a>>,
+}
+
+impl<'a> GameState<'a> for MenuState<'a> {}
+
+pub async fn get_replay_menu<'a>(
+    gamemodes: &'a HashMap<&str, GameMode>,
+) -> Box<dyn GameState<'a> + 'a> {
+    let recordings: Vec<String> = vec![];
+    let _ = recordings
+        .into_iter()
+        .map(|entry| {
+            MenuOption::new(move |f| {
+                Box::pin(construct_gamestate_replay(&gamemodes[entry.as_str()], f))
+            })
+        })
+        .collect::<Vec<_>>();
+
+    todo!()
+}
+
+fn main() {}

src/test/ui/closures/2229_closure_analysis/run_pass/move_closure.rs

@@ -56,28 +56,6 @@ fn no_ref_nested() {
     c();
 }
 
-struct A<'a>(&'a mut String,  &'a mut String);
-// Test that reborrowing works as expected for move closures
-// by attempting a disjoint capture through a reference.
-fn disjoint_via_ref() {
-    let mut x = String::new();
-    let mut y = String::new();
-
-    let mut a = A(&mut x, &mut y);
-    let a = &mut a;
-
-    let mut c1 = move || {
-        a.0.truncate(0);
-    };
-
-    let mut c2 = move || {
-        a.1.truncate(0);
-    };
-
-    c1();
-    c2();
-}
-
 // Test that even if a path is moved into the closure, the closure is not FnOnce
 // if the path is not moved by the closure call.
 fn data_moved_but_not_fn_once() {
@@ -109,7 +87,6 @@ fn main() {
     no_ref();
     no_ref_nested();
 
-    disjoint_via_ref();
     data_moved_but_not_fn_once();
 
     returned_closure_owns_copy_type_data();

src/test/ui/nll/closure-use-spans.stderr

@@ -26,7 +26,7 @@ LL |     let y = &x;
 LL |     x = 0;
    |     ^^^^^ assignment to borrowed `x` occurs here
 LL |     move || *y;
-   |              - borrow later captured here by closure
+   |             -- borrow later captured here by closure
 
 error: aborting due to 3 previous errors