Rollup merge of rust-lang#89208 - wesleywiser:rfc_2229_droporder, r=nikomatsakis

[rfc 2229] Drop fully captured upvars in the same order as the regular drop code

Currently, with the new 2021 edition, if a closure captures all of the
fields of an upvar, we'll drop those fields in the order they are used
within the closure instead of the normal drop order (the definition
order of the fields in the type).

This changes that so we sort the captured fields by the definition order
which causes them to drop in that same order as well.

Fixes rust-lang/project-rfc-2229#42

r? `@nikomatsakis`

Author: Manishearth

compiler/rustc_typeck/src/check/upvar.rs

@@ -602,7 +602,78 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             }
         }
 
-        debug!("For closure={:?}, min_captures={:#?}", closure_def_id, root_var_min_capture_list);
+        debug!(
+            "For closure={:?}, min_captures before sorting={:?}",
+            closure_def_id, root_var_min_capture_list
+        );
+
+        // Now that we have the minimized list of captures, sort the captures by field id.
+        // This causes the closure to capture the upvars in the same order as the fields are
+        // declared which is also the drop order. Thus, in situations where we capture all the
+        // fields of some type, the obserable drop order will remain the same as it previously
+        // was even though we're dropping each capture individually.
+        // See https://github.com/rust-lang/project-rfc-2229/issues/42 and
+        // `src/test/ui/closures/2229_closure_analysis/preserve_field_drop_order.rs`.
+        for (_, captures) in &mut root_var_min_capture_list {
+            captures.sort_by(|capture1, capture2| {
+                for (p1, p2) in capture1.place.projections.iter().zip(&capture2.place.projections) {
+                    // We do not need to look at the `Projection.ty` fields here because at each
+                    // step of the iteration, the projections will either be the same and therefore
+                    // the types must be as well or the current projection will be different and
+                    // we will return the result of comparing the field indexes.
+                    match (p1.kind, p2.kind) {
+                        // Paths are the same, continue to next loop.
+                        (ProjectionKind::Deref, ProjectionKind::Deref) => {}
+                        (ProjectionKind::Field(i1, _), ProjectionKind::Field(i2, _))
+                            if i1 == i2 => {}
+
+                        // Fields are different, compare them.
+                        (ProjectionKind::Field(i1, _), ProjectionKind::Field(i2, _)) => {
+                            return i1.cmp(&i2);
+                        }
+
+                        // We should have either a pair of `Deref`s or a pair of `Field`s.
+                        // Anything else is a bug.
+                        (
+                            l @ (ProjectionKind::Deref | ProjectionKind::Field(..)),
+                            r @ (ProjectionKind::Deref | ProjectionKind::Field(..)),
+                        ) => bug!(
+                            "ProjectionKinds Deref and Field were mismatched: ({:?}, {:?})",
+                            l,
+                            r
+                        ),
+                        (
+                            l
+                            @
+                            (ProjectionKind::Index
+                            | ProjectionKind::Subslice
+                            | ProjectionKind::Deref
+                            | ProjectionKind::Field(..)),
+                            r
+                            @
+                            (ProjectionKind::Index
+                            | ProjectionKind::Subslice
+                            | ProjectionKind::Deref
+                            | ProjectionKind::Field(..)),
+                        ) => bug!(
+                            "ProjectionKinds Index or Subslice were unexpected: ({:?}, {:?})",
+                            l,
+                            r
+                        ),
+                    }
+                }
+
+                unreachable!(
+                    "we captured two identical projections: capture1 = {:?}, capture2 = {:?}",
+                    capture1, capture2
+                );
+            });
+        }
+
+        debug!(
+            "For closure={:?}, min_captures after sorting={:#?}",
+            closure_def_id, root_var_min_capture_list
+        );
         typeck_results.closure_min_captures.insert(closure_def_id, root_var_min_capture_list);
     }
 

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

@@ -0,0 +1,98 @@
+// edition:2021
+
+// Tests that in cases where we individually capture all the fields of a type,
+// we still drop them in the order they would have been dropped in the 2018 edition.
+
+#![feature(rustc_attrs)]
+
+#[derive(Debug)]
+struct HasDrop;
+impl Drop for HasDrop {
+    fn drop(&mut self) {
+        println!("dropped");
+    }
+}
+
+fn test_one() {
+    let a = (HasDrop, HasDrop);
+    let b = (HasDrop, HasDrop);
+
+    let c = #[rustc_capture_analysis]
+    //~^ ERROR: attributes on expressions are experimental
+    //~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
+    || {
+        //~^ ERROR: First Pass analysis includes:
+        //~| ERROR: Min Capture analysis includes:
+        println!("{:?}", a.0);
+        //~^ NOTE: Capturing a[(0, 0)] -> ImmBorrow
+        //~| NOTE: Min Capture a[(0, 0)] -> ImmBorrow
+        println!("{:?}", a.1);
+        //~^ NOTE: Capturing a[(1, 0)] -> ImmBorrow
+        //~| NOTE: Min Capture a[(1, 0)] -> ImmBorrow
+
+        println!("{:?}", b.0);
+        //~^ NOTE: Capturing b[(0, 0)] -> ImmBorrow
+        //~| NOTE: Min Capture b[(0, 0)] -> ImmBorrow
+        println!("{:?}", b.1);
+        //~^ NOTE: Capturing b[(1, 0)] -> ImmBorrow
+        //~| NOTE: Min Capture b[(1, 0)] -> ImmBorrow
+    };
+}
+
+fn test_two() {
+    let a = (HasDrop, HasDrop);
+    let b = (HasDrop, HasDrop);
+
+    let c = #[rustc_capture_analysis]
+    //~^ ERROR: attributes on expressions are experimental
+    //~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
+    || {
+        //~^ ERROR: First Pass analysis includes:
+        //~| ERROR: Min Capture analysis includes:
+        println!("{:?}", a.1);
+        //~^ NOTE: Capturing a[(1, 0)] -> ImmBorrow
+        //~| NOTE: Min Capture a[(1, 0)] -> ImmBorrow
+        println!("{:?}", a.0);
+        //~^ NOTE: Capturing a[(0, 0)] -> ImmBorrow
+        //~| NOTE: Min Capture a[(0, 0)] -> ImmBorrow
+
+        println!("{:?}", b.1);
+        //~^ NOTE: Capturing b[(1, 0)] -> ImmBorrow
+        //~| NOTE: Min Capture b[(1, 0)] -> ImmBorrow
+        println!("{:?}", b.0);
+        //~^ NOTE: Capturing b[(0, 0)] -> ImmBorrow
+        //~| NOTE: Min Capture b[(0, 0)] -> ImmBorrow
+    };
+}
+
+fn test_three() {
+    let a = (HasDrop, HasDrop);
+    let b = (HasDrop, HasDrop);
+
+    let c = #[rustc_capture_analysis]
+    //~^ ERROR: attributes on expressions are experimental
+    //~| NOTE: see issue #15701 <https://github.com/rust-lang/rust/issues/15701>
+    || {
+        //~^ ERROR: First Pass analysis includes:
+        //~| ERROR: Min Capture analysis includes:
+        println!("{:?}", b.1);
+        //~^ NOTE: Capturing b[(1, 0)] -> ImmBorrow
+        //~| NOTE: Min Capture b[(1, 0)] -> ImmBorrow
+        println!("{:?}", a.1);
+        //~^ NOTE: Capturing a[(1, 0)] -> ImmBorrow
+        //~| NOTE: Min Capture a[(1, 0)] -> ImmBorrow
+        println!("{:?}", a.0);
+        //~^ NOTE: Capturing a[(0, 0)] -> ImmBorrow
+        //~| NOTE: Min Capture a[(0, 0)] -> ImmBorrow
+
+        println!("{:?}", b.0);
+        //~^ NOTE: Capturing b[(0, 0)] -> ImmBorrow
+        //~| NOTE: Min Capture b[(0, 0)] -> ImmBorrow
+    };
+}
+
+fn main() {
+    test_one();
+    test_two();
+    test_three();
+}