8000 Auto merge of #70015 - jonas-schievink:gen-needs-drop, r=matthewjasper · rust-lang-ci/rust@36b1a92 · GitHub
[go: up one dir, main page]
Skip to content
This repository was archived by the owner on May 28, 2025. It is now read-only.

Commit 36b1a92

Browse files
borsbors
committed
Auto merge of rust-lang#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 rust-lang#69814 since that contains some fixes that are made relevant by *this* PR (see rust-lang#69814 (comment) (this has been merged)
2 parents e7497a8 + ae53315 commit 36b1a92

File tree

6 files changed

+85
-44
lines changed
  • generator-drop-cleanup
  • ui/generator

    • 6 files changed

    +85
    -44
    lines changed

    ‎src/librustc_middle/ty/util.rs

    Lines changed: 3 additions & 5 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1047,10 +1047,7 @@ pub fn needs_drop_components(
    10471047
    // Foreign types can never have destructors.
    10481048
    ty::Foreign(..) => Ok(SmallVec::new()),
    10491049

    1050-
    // Pessimistically assume that all generators will require destructors
    1051-
    // as we don't know if a destructor is a noop or not until after the MIR
    1052-
    // state transformation pass.
    1053-
    ty::Generator(..) | ty::Dynamic(..) | ty::Error => Err(AlwaysRequiresDrop),
    1050+
    ty::Dynamic(..) | ty::Error => Err(AlwaysRequiresDrop),
    10541051

    10551052
    ty::Slice(ty) => needs_drop_components(ty, target_layout),
    10561053
    ty::Array(elem_ty, size) => {
    @@ -1083,7 +1080,8 @@ pub fn needs_drop_components(
    10831080
    | ty::Placeholder(..)
    10841081
    | ty::Opaque(..)
    10851082
    | ty::Infer(_)
    1086-
    | ty::Closure(..) => Ok(smallvec![ty]),
    1083+
    | ty::Closure(..)
    1084+
    | ty::Generator(..) => Ok(smallvec![ty]),
    10871085
    }
    10881086
    }
    10891087

    ‎src/librustc_ty/needs_drop.rs

    Lines changed: 17 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -99,6 +99,23 @@ where
    9999
    }
    100100
    }
    101101

    102+
    ty::Generator(_, substs, _) => {
    103+
    let substs = substs.as_generator();
    104+
    for upvar_ty in substs.upvar_tys() {
    105+
    queue_type(self, upvar_ty);
    106+
    }
    107+
    108+
    let witness = substs.witness();
    109+
    let interior_tys = match &witness.kind {
    110+
    ty::GeneratorWitness(tys) => tcx.erase_late_bound_regions(tys),
    111+
    _ => bug!(),
    112+
    };
    113+
    114+
    for interior_ty in interior_tys {
    115+
    queue_type(self, interior_ty);
    116+
    }
    117+
    }
    118+
    102119
    // Check for a `Drop` impl and whether this is a union or
    103120
    // `ManuallyDrop`. If it's a struct or enum without a `Drop`
    104121
    // impl then check whether the field types need `Drop`.

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

    Lines changed: 3 additions & 0 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1,11 +1,14 @@
    11
    #![feature(generators, generator_trait)]
    22

    3+
    // ignore-wasm32-bare compiled with panic=abort by default
    4+
    35
    // Regression test for #58892, generator drop shims should not have blocks
    46
    // spuriously marked as cleanup
    57

    68
    // EMIT_MIR rustc.main-{{closure}}.generator_drop.0.mir
    79
    fn main() {
    810
    let gen = || {
    11+
    let _s = String::new();
    912
    yield;
    1013
    };
    1114
    }

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

    Lines changed: 53 additions & 26 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1,53 +1,80 @@
    11
    // MIR for `main::{{closure}}#0` 0 generator_drop
    2-
    // generator_layout = GeneratorLayout { field_tys: [], variant_fields: [[], [], [], []], storage_conflicts: BitMatrix { num_rows: 0, num_columns: 0, words: [], marker: PhantomData } }
    2+
    // generator_layout = GeneratorLayout { field_tys: [std::string::String], variant_fields: [[], [], [], [_0]], storage_conflicts: BitMatrix { num_rows: 1, num_columns: 1, words: [1], marker: PhantomData } }
    33

    4-
    fn main::{{closure}}#0(_1: *mut [generator@$DIR/generator-drop-cleanup.rs:8:15: 10:6 {()}]) -> () {
    5-
    let mut _0: (); // return place in scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
    6-
    let mut _2: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
    7-
    let _3: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:9:9: 9:14
    8-
    let mut _4: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:9:9: 9:14
    9-
    let mut _5: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:8:18: 8:18
    10-
    let mut _6: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
    11-
    let mut _7: isize; // in scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
    4+
    fn main::{{closure}}#0(_1: *mut [generator@$DIR/generator-drop-cleanup.rs:10:15: 13:6 {std::string::String, ()}]) -> () {
    5+
    let mut _0: (); // return place in scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
    6+
    let mut _2: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
    7+
    let _3: std::string::String; // in scope 0 at $DIR/generator-drop-cleanup.rs:11:13: 11:15
    8+
    let _4: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:12:9: 12:14
    9+
    let mut _5: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:12:9: 12:14
    10+
    let mut _7: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:10:18: 10:18
    11+
    let mut _8: (); // in scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
    12+
    let mut _9: isize; // in scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
    13+
    scope 1 {
    14+
    debug _s => (((*_1) as variant#3).0: std::string::String); // in scope 1 at $DIR/generator-drop-cleanup.rs:11:13: 11:15
    15+
    }
    16+
    scope 2 {
    17+
    let mut _6: std::vec::Vec<u8>; // in scope 2 at $DIR/generator-drop-cleanup.rs:11:18: 11:31
    18+
    scope 3 {
    19+
    }
    20+
    }
    1221

    1322
    bb0: {
    14-
    _7 = discriminant((*_1)); // bb0[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
    15-
    switchInt(move _7) -> [0u32: bb4, 3u32: bb7, otherwise: bb8]; // bb0[1]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
    23+
    _9 = discriminant((*_1)); // bb0[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
    24+
    switchInt(move _9) -> [0u32: bb7, 3u32: bb11, otherwise: bb12]; // bb0[1]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
    1625
    }
    1726

    18-
    bb1: {
    19-
    StorageDead(_4); // bb1[0]: scope 0 at $DIR/generator-drop-cleanup.rs:9:13: 9:14
    20-
    StorageDead(_3); // bb1[1]: scope 0 at $DIR/generator-drop-cleanup.rs:9:14: 9:15
    21-
    goto -> bb5; // bb1[2]: scope 0 at $DIR/generator-drop-cleanup.rs:10:5: 10:6
    27+
    bb1 (cleanup): {
    28+
    resume; // bb1[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
    2229
    }
    2330

    24-
    bb2: {
    25-
    return; // bb2[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
    31+
    bb2 (cleanup): {
    32+
    nop; // bb2[0]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6
    33+
    goto -> bb8; // bb2[1]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6
    2634
    }
    2735

    2836
    bb3: {
    29-
    return; // bb3[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
    37+
    StorageDead(_5); // bb3[0]: scope 1 at $DIR/generator-drop-cleanup.rs:12:13: 12:14
    38+
    StorageDead(_4); // bb3[1]: scope 1 at $DIR/generator-drop-cleanup.rs:12:14: 12:15
    39+
    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
    3040
    }
    3141

    3242
    bb4: {
    33-
    goto -> bb6; // bb4[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
    43+
    nop; // bb4[0]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6
    44+
    goto -> bb9; // bb4[1]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6
    3445
    }
    3546

    3647
    bb5: {
    37-
    goto -> bb2; // bb5[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:5: 10:6
    48+
    return; // bb5[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
    3849
    }
    3950

    4051
    bb6: {
    41-
    goto -> bb3; // bb6[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
    52+
    return; // bb6[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
    4253
    }
    4354

    4455
    bb7: {
    45-
    StorageLive(_3); // bb7[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
    46-
    StorageLive(_4); // bb7[1]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
    47-
    goto -> bb1; // bb7[2]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
    56+
    goto -> bb10; // bb7[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
    57+
    }
    58+
    59+
    bb8 (cleanup): {
    60+
    goto -> bb1; // bb8[0]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6
    61+
    }
    62+
    63+
    bb9: {
    64+
    goto -> bb5; // bb9[0]: scope 0 at $DIR/generator-drop-cleanup.rs:13:5: 13:6
    65+
    }
    66+
    67+
    bb10: {
    68+
    goto -> bb6; // bb10[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
    69+
    }
    70+
    71+
    bb11: {
    72+
    StorageLive(_4); // bb11[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
    73+
    StorageLive(_5); // bb11[1]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
    74+
    goto -> bb3; // bb11[2]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
    4875
    }
    4976

    50-
    bb8: {
    51-
    return; // bb8[0]: scope 0 at $DIR/generator-drop-cleanup.rs:8:15: 10:6
    77+
    bb12: {
    78+
    return; // bb12[0]: scope 0 at $DIR/generator-drop-cleanup.rs:10:15: 13:6
    5279
    }
    5380
    }

    ‎src/test/ui/generator/borrowing.stderr

    Lines changed: 6 additions & 9 deletions
    Original file line numberDiff line numberDiff line change
    @@ -1,19 +1,16 @@
    11
    error[E0597]: `a` does not live long enough
    22
    --> $DIR/borrowing.rs:9:33
    33
    |
    4+
    LL | let _b = {
    5+
    | -- borrow later stored here
    6+
    LL | let a = 3;
    47
    LL | Pin::new(&mut || yield &a).resume(())
    5-
    | ----------^
    6-
    | | |
    7-
    | | borrowed value does not live long enough
    8+
    | -- ^ borrowed value does not live long enough
    9+
    | |
    810
    | value captured here by generator
    9-
    | a temporary with access to the borrow is created here ...
    1011
    LL |
    1112
    LL | };
    12-
    | -- ... and the borrow might be used here, when that temporary is dropped and runs the destructor for generator
    13-
    | |
    14-
    | `a` dropped here while still borrowed
    15-
    |
    16-
    = 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.
    13+
    | - `a` dropped here while still borrowed
    1714

    1815
    error[E0597]: `a` does not live long enough
    1916
    --> $DIR/borrowing.rs:16:20

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

    Lines changed: 3 additions & 4 deletions
    Original file line numberDiff line numberDiff line change
    @@ -4,10 +4,9 @@ error[E0499]: cannot borrow `thing` as mutable more than once at a time
    44
    LL | gen.as_mut().resume(&mut thing);
    55
    | ---------- first mutable borrow occurs here
    66
    LL | gen.as_mut().resume(&mut thing);
    7-
    | ^^^^^^^^^^ second mutable borrow occurs here
    8-
    LL |
    9-
    LL | }
    10-
    | - first borrow might be used here, when `gen` is dropped and runs the destructor for generator
    7+
    | ------ ^^^^^^^^^^ second mutable borrow occurs here
    8+
    | |
    9+
    | first borrow later used by call
    1110

    1211
    error: aborting due to previous error
    1312

    0 commit comments

    Comments
     (0)
    0