Skip to content

Commit c8fb14e

Browse files
authored
Rollup 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 32b6b1e + 1df7641 commit c8fb14e

File tree

5 files changed

+59
-33
lines changed

5 files changed

+59
-33
lines changed

src/librustc/ty/util.rs

+3-5
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

+17
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

+30-15
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
fn main() {
77
let gen = || {
8+
let _s = String::new();
89
yield;
910
};
1011
}
@@ -13,35 +14,49 @@ fn main() {
1314

1415
// START rustc.main-{{closure}}.generator_drop.0.mir
1516
// bb0: {
16-
// _7 = discriminant((*_1));
17-
// switchInt(move _7) -> [0u32: bb4, 3u32: bb7, otherwise: bb8];
17+
// _9 = discriminant((*_1));
18+
// switchInt(move _9) -> [0u32: bb7, 3u32: bb11, otherwise: bb12];
1819
// }
19-
// bb1: {
20-
// StorageDead(_4);
21-
// StorageDead(_3);
22-
// goto -> bb5;
20+
// bb1 (cleanup): {
21+
// resume;
2322
// }
24-
// bb2: {
25-
// return;
23+
// bb2 (cleanup): {
24+
// nop;
25+
// goto -> bb8;
2626
// }
2727
// bb3: {
28-
// return;
28+
// StorageDead(_5);
29+
// StorageDead(_4);
30+
// drop((((*_1) as variant#3).0: std::string::String)) -> [return: bb4, unwind: bb2];
2931
// }
3032
// bb4: {
31-
// goto -> bb6;
33+
// nop;
34+
// goto -> bb9;
3235
// }
3336
// bb5: {
34-
// goto -> bb2;
37+
// return;
3538
// }
3639
// bb6: {
37-
// goto -> bb3;
40+
// return;
3841
// }
3942
// bb7: {
40-
// StorageLive(_3);
41-
// StorageLive(_4);
43+
// goto -> bb10;
44+
// }
45+
// bb8 (cleanup): {
4246
// goto -> bb1;
4347
// }
44-
// bb8: {
48+
// bb9: {
49+
// goto -> bb5;
50+
// }
51+
// bb10: {
52+
// goto -> bb6;
53+
// }
54+
// bb11: {
55+
// StorageLive(_4);
56+
// StorageLive(_5);
57+
// goto -> bb3;
58+
// }
59+
// bb12: {
4560
// return;
4661
// }
4762
// END rustc.main-{{closure}}.generator_drop.0.mir

src/test/ui/generator/borrowing.stderr

+6-9
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

+3-4
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)