Rollup merge of rust-lang#70015 - jonas-schievink:gen-needs-drop, r=m…

…atthewjasper

Make `needs_drop` less pessimistic on generators

Generators only have non-trivial drop logic when they may store (in upvars or across yields) a type that does.

This prevents generation of some unnecessary MIR in simple generators. There might be some impact on compile times, but this is probably limited in real-world applications.

~~This builds off of rust-lang#69814 since that contains some fixes that are made relevant by *this* PR (see rust-lang#69814 (comment) (this has been merged)

src/librustc/ty/util.rs

@@ -1047,10 +1047,7 @@ pub fn needs_drop_components(
         // Foreign types can never have destructors.
         ty::Foreign(..) => Ok(SmallVec::new()),
 
-        // Pessimistically assume that all generators will require destructors
-        // as we don't know if a destructor is a noop or not until after the MIR
-        // state transformation pass.
-        ty::Generator(..) | ty::Dynamic(..) | ty::Error => Err(AlwaysRequiresDrop),
+        ty::Dynamic(..) | ty::Error => Err(AlwaysRequiresDrop),
 
         ty::Slice(ty) => needs_drop_components(ty, target_layout),
         ty::Array(elem_ty, size) => {
@@ -1083,7 +1080,8 @@ pub fn needs_drop_components(
         | ty::Placeholder(..)
         | ty::Opaque(..)
         | ty::Infer(_)
-        | ty::Closure(..) => Ok(smallvec![ty]),
+        | ty::Closure(..)
+        | ty::Generator(..) => Ok(smallvec![ty]),
     }
 }
 

src/librustc_ty/needs_drop.rs

@@ -99,6 +99,23 @@ where
                         }
                     }
 
+                    ty::Generator(_, substs, _) => {
+                        let substs = substs.as_generator();
+                        for upvar_ty in substs.upvar_tys() {
+                            queue_type(self, upvar_ty);
+                        }
+
+                        let witness = substs.witness();
+                        let interior_tys = match &witness.kind {
+                            ty::GeneratorWitness(tys) => tcx.erase_late_bound_regions(tys),
+                            _ => bug!(),
+                        };
+
+                        for interior_ty in interior_tys {
+                            queue_type(self, interior_ty);
+                        }
+                    }
+
                     // Check for a `Drop` impl and whether this is a union or
                     // `ManuallyDrop`. If it's a struct or enum without a `Drop`
                     // impl then check whether the field types need `Drop`.

src/test/mir-opt/generator-drop-cleanup.rs

@@ -5,6 +5,7 @@
 
 fn main() {
     let gen = || {
+        let _s = String::new();
         yield;
     };
 }
@@ -13,35 +14,49 @@ fn main() {
 
 // START rustc.main-{{closure}}.generator_drop.0.mir
 // bb0: {
-//     _7 = discriminant((*_1));
-//     switchInt(move _7) -> [0u32: bb4, 3u32: bb7, otherwise: bb8];
+//     _9 = discriminant((*_1));
+//     switchInt(move _9) -> [0u32: bb7, 3u32: bb11, otherwise: bb12];
 // }
-// bb1: {
-//     StorageDead(_4);
-//     StorageDead(_3);
-//     goto -> bb5;
+// bb1 (cleanup): {
+//     resume;
 // }
-// bb2: {
-//     return;
+// bb2 (cleanup): {
+//     nop;
+//     goto -> bb8;
 // }
 // bb3: {
-//     return;
+//     StorageDead(_5);
+//     StorageDead(_4);
+//     drop((((*_1) as variant#3).0: std::string::String)) -> [return: bb4, unwind: bb2];
 // }
 // bb4: {
-//     goto -> bb6;
+//     nop;
+//     goto -> bb9;
 // }
 // bb5: {
-//     goto -> bb2;
+//     return;
 // }
 // bb6: {
-//     goto -> bb3;
+//     return;
 // }
 // bb7: {
-//     StorageLive(_3);
-//     StorageLive(_4);
+//     goto -> bb10;
+// }
+// bb8 (cleanup): {
 //     goto -> bb1;
 // }
-// bb8: {
+// bb9: {
+//     goto -> bb5;
+// }
+// bb10: {
+//     goto -> bb6;
+// }
+// bb11: {
+//     StorageLive(_4);
+//     StorageLive(_5);
+//     goto -> bb3;
+// }
+// bb12: {
 //     return;
 // }
 // END rustc.main-{{closure}}.generator_drop.0.mir

src/test/ui/generator/borrowing.stderr

@@ -1,19 +1,16 @@
 error[E0597]: `a` does not live long enough
   --> $DIR/borrowing.rs:9:33
    |
+LL |     let _b = {
+   |         -- borrow later stored here
+LL |         let a = 3;
 LL |         Pin::new(&mut || yield &a).resume(())
-   |                       ----------^
-   |                       |         |
-   |                       |         borrowed value does not live long enough
+   |                       --        ^ borrowed value does not live long enough
+   |                       |
    |                       value captured here by generator
-   |                       a temporary with access to the borrow is created here ...
 LL |
 LL |     };
-   |     -- ... and the borrow might be used here, when that temporary is dropped and runs the destructor for generator
-   |     |
-   |     `a` dropped here while still borrowed
-   |
-   = note: The temporary is part of an expression at the end of a block. Consider forcing this temporary to be dropped sooner, before the block's local variables are dropped. For example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block.
+   |     - `a` dropped here while still borrowed
 
 error[E0597]: `a` does not live long enough
   --> $DIR/borrowing.rs:16:20

src/test/ui/generator/retain-resume-ref.stderr

@@ -4,10 +4,9 @@ error[E0499]: cannot borrow `thing` as mutable more than once at a time
 LL |     gen.as_mut().resume(&mut thing);
    |                         ---------- first mutable borrow occurs here
 LL |     gen.as_mut().resume(&mut thing);
-   |                         ^^^^^^^^^^ second mutable borrow occurs here
-LL |
-LL | }
-   | - first borrow might be used here, when `gen` is dropped and runs the destructor for generator
+   |                  ------ ^^^^^^^^^^ second mutable borrow occurs here
+   |                  |
+   |                  first borrow later used by call
 
 error: aborting due to previous error