Auto merge of #70015 - jonas-schievink:gen-needs-drop, r=matthewjasper

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 #69814 since that contains some fixes that are made relevant by *this* PR (see #69814 (comment) (this has been merged)

src/librustc_middle/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

@@ -1,11 +1,14 @@
 #![feature(generators, generator_trait)]
 
+// ignore-wasm32-bare compiled with panic=abort by default
+
 // Regression test for #58892, generator drop shims should not have blocks
 // spuriously marked as cleanup
 
 // EMIT_MIR rustc.main-{{closure}}.generator_drop.0.mir
 fn main() {
     let gen = || {
+        let _s = String::new();
         yield;
     };
 }

src/test/mir-opt/generator-drop-cleanup/rustc.main-{{closure}}.generator_drop.0.mir

@@ -1,53 +1,80 @@
 // MIR for `main::{{closure}}#0` 0 generator_drop
-// generator_layout = GeneratorLayout { field_tys: [], variant_fields: [[], [], [], []], storage_conflicts: BitMatrix { num_rows: 0, num_columns: 0, words: [], marker: PhantomData } }
+// generator_layout = GeneratorLayout { field_tys: [std::string::String], variant_fields: [[], [], [], [_0]], storage_conflicts: BitMatrix { num_rows: 1, num_columns: 1, words: [1], marker: PhantomData } }
 
-fn main::{{closure}}#0(_1: *mut [generator@$DIR/generator-drop-cleanup.rs:8:15: 10:6 {()}]) -> () {
-    let mut _0: ();                      // return place in scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
-    let mut _2: ();                      // in scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
-    let _3: ();                          // in scope 0 at $DIR/generator-drop-cleanup.rs:9:9: 9:14
-    let mut _4: ();                      // in scope 0 at $DIR/generator-drop-cleanup.rs:9:9: 9:14
-    let mut _5: ();                      // in scope 0 at $DIR/generator-drop-cleanup.rs:8:18: 8:18
-    let mut _6: ();                      // in scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
-    let mut _7: isize;                   // in scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
+fn main::{{closure}}#0(_1: *mut [generator@$DIR/generator-drop-cleanup.rs:10:15: 13:6 {std::string::String, ()}]) -> () {
+    let mut _0: ();                      // return place in scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
+    let mut _2: ();                      // in scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
+    let _3: std::string::String;         // in scope 0 at $DIR/generator-drop-cleanup.rs:11:13: 11:15
+    let _4: ();                          // in scope 0 at $DIR/generator-drop-cleanup.rs:12:9: 12:14
+    let mut _5: ();                      // in scope 0 at $DIR/generator-drop-cleanup.rs:12:9: 12:14
+    let mut _7: ();                      // in scope 0 at $DIR/generator-drop-cleanup.rs:10:18: 10:18
+    let mut _8: ();                      // in scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
+    let mut _9: isize;                   // in scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
+    scope 1 {
+        debug _s => (((*_1) as variant#3).0: std::string::String); // in scope 1 at $DIR/generator-drop-cleanup.rs:11:13: 11:15
+    }
+    scope 2 {
+        let mut _6: std::vec::Vec<u8>;   // in scope 2 at $DIR/generator-drop-cleanup.rs:11:18: 11:31
+        scope 3 {
+        }
+    }
 
     bb0: {
-        _7 = discriminant((*_1));        // bb0[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
-        switchInt(move _7) -> [0u32: bb4, 3u32: bb7, otherwise: bb8]; // bb0[1]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
+        _9 = discriminant((*_1));        // bb0[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
+        switchInt(move _9) -> [0u32: bb7, 3u32: bb11, otherwise: bb12]; // bb0[1]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
     }
 
-    bb1: {
-        StorageDead(_4);                 // bb1[0]: scope 0 at $DIR/generator-drop-cleanup.rs:9:13: 9:14
-        StorageDead(_3);                 // bb1[1]: scope 0 at $DIR/generator-drop-cleanup.rs:9:14: 9:15
-        goto -> bb5;                     // bb1[2]: scope 0 at $DIR/generator-drop-cleanup.rs:10:5: 10:6
+    bb1 (cleanup): {
+        resume;                          // bb1[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
     }
 
-    bb2: {
-        return;                          // bb2[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
+    bb2 (cleanup): {
+        nop;                             // bb2[0]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6
+        goto -> bb8;                     // bb2[1]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6
     }
 
     bb3: {
-        return;                          // bb3[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
+        StorageDead(_5);                 // bb3[0]: scope 1 at $DIR/generator-drop-cleanup.rs:12:13: 12:14
+        StorageDead(_4);                 // bb3[1]: scope 1 at $DIR/generator-drop-cleanup.rs:12:14: 12:15
+        drop((((*_1) as variant#3).0: std::string::String)) -> [return: bb4, unwind: bb2]; // bb3[2]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6
     }
 
     bb4: {
-        goto -> bb6;                     // bb4[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
+        nop;                             // bb4[0]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6
+        goto -> bb9;                     // bb4[1]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6
     }
 
     bb5: {
-        goto -> bb2;                     // bb5[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:5: 10:6
+        return;                          // bb5[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
     }
 
     bb6: {
-        goto -> bb3;                     // bb6[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
+        return;                          // bb6[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
     }
 
     bb7: {
-        StorageLive(_3);                 // bb7[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
-        StorageLive(_4);                 // bb7[1]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
-        goto -> bb1;                     // bb7[2]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
+        goto -> bb10;                    // bb7[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
+    }
+
+    bb8 (cleanup): {
+        goto -> bb1;                     // bb8[0]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6
+    }
+
+    bb9: {
+        goto -> bb5;                     // bb9[0]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6
+    }
+
+    bb10: {
+        goto -> bb6;                     // bb10[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
+    }
+
+    bb11: {
+        StorageLive(_4);                 // bb11[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
+        StorageLive(_5);                 // bb11[1]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
+        goto -> bb3;                     // bb11[2]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
     }
 
-    bb8: {
-        return;                          // bb8[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
+    bb12: {
+        return;                          // bb12[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
     }
 }

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