Skip to content

Commit a151d37

Browse files
committed
fix generator drop caching
Fixes #45328.
1 parent abe7c87 commit a151d37

File tree

2 files changed

+50
-9
lines changed

2 files changed

+50
-9
lines changed

src/librustc_mir/build/scope.rs

+27-8
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,11 @@ struct CachedBlock {
154154
unwind: Option<BasicBlock>,
155155

156156
/// The cached block for unwinds during cleanups-on-generator-drop path
157+
///
158+
/// This is split from the standard unwind path here to prevent drop
159+
/// elaboration from creating drop flags that would have to be captured
160+
/// by the generator. I'm not sure how important this optimization is,
161+
/// but it is here.
157162
generator_drop: Option<BasicBlock>,
158163
}
159164

@@ -217,13 +222,26 @@ impl<'tcx> Scope<'tcx> {
217222
/// Should always be run for all inner scopes when a drop is pushed into some scope enclosing a
218223
/// larger extent of code.
219224
///
220-
/// `unwind` controls whether caches for the unwind branch are also invalidated.
221-
fn invalidate_cache(&mut self, unwind: bool) {
225+
/// `storage_only` controls whether to invalidate only drop paths run `StorageDead`.
226+
/// `this_scope_only` controls whether to invalidate only drop paths that refer to the current
227+
/// top-of-scope (as opposed to dependent scopes).
228+
fn invalidate_cache(&mut self, storage_only: bool, this_scope_only: bool) {
229+
// FIXME: maybe do shared caching of `cached_exits` etc. to handle functions
230+
// with lots of `try!`?
231+
232+
// cached exits drop storage and refer to the top-of-scope
222233
self.cached_exits.clear();
223-
if !unwind { return; }
224-
for dropdata in &mut self.drops {
225-
if let DropKind::Value { ref mut cached_block } = dropdata.kind {
226-
cached_block.invalidate();
234+
235+
if !storage_only {
236+
// the current generator drop ignores storage but refers to top-of-scope
237+
self.cached_generator_drop = None;
238+
}
239+
240+
if !storage_only && !this_scope_only {
241+
for dropdata in &mut self.drops {
242+
if let DropKind::Value { ref mut cached_block } = dropdata.kind {
243+
cached_block.invalidate();
244+
}
227245
}
228246
}
229247
}
@@ -672,8 +690,7 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> {
672690
// invalidating caches of each scope visited. This way bare minimum of the
673691
// caches gets invalidated. i.e. if a new drop is added into the middle scope, the
674692
// cache of outer scpoe stays intact.
675-
let invalidate_unwind = needs_drop && !this_scope;
676-
scope.invalidate_cache(invalidate_unwind);
693+
scope.invalidate_cache(!needs_drop, this_scope);
677694
if this_scope {
678695
if let DropKind::Value { .. } = drop_kind {
679696
scope.needs_cleanup = true;
@@ -942,5 +959,7 @@ fn build_diverge_scope<'a, 'gcx, 'tcx>(tcx: TyCtxt<'a, 'gcx, 'tcx>,
942959
target = block
943960
}
944961

962+
debug!("build_diverge_scope({:?}, {:?}) = {:?}", scope, span, target);
963+
945964
target
946965
}

src/test/run-pass/dynamic-drop.rs

+23-1
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@
88
// option. This file may not be copied, modified, or distributed
99
// except according to those terms.
1010

11-
#![feature(untagged_unions)]
11+
#![feature(generators, generator_trait, untagged_unions)]
1212

1313
use std::cell::{Cell, RefCell};
14+
use std::ops::Generator;
1415
use std::panic;
1516
use std::usize;
1617

@@ -161,6 +162,21 @@ fn vec_simple(a: &Allocator) {
161162
let _x = vec![a.alloc(), a.alloc(), a.alloc(), a.alloc()];
162163
}
163164

165+
fn generator(a: &Allocator, run_count: usize) {
166+
assert!(run_count < 4);
167+
168+
let mut gen = || {
169+
(a.alloc(),
170+
yield a.alloc(),
171+
a.alloc(),
172+
yield a.alloc()
173+
);
174+
};
175+
for _ in 0..run_count {
176+
gen.resume();
177+
}
178+
}
179+
164180
#[allow(unreachable_code)]
165181
fn vec_unreachable(a: &Allocator) {
166182
let _x = vec![a.alloc(), a.alloc(), a.alloc(), return];
@@ -228,5 +244,11 @@ fn main() {
228244
run_test(|a| field_assignment(a, false));
229245
run_test(|a| field_assignment(a, true));
230246

247+
// FIXME: fix leaks on panics
248+
run_test_nopanic(|a| generator(a, 0));
249+
run_test_nopanic(|a| generator(a, 1));
250+
run_test_nopanic(|a| generator(a, 2));
251+
run_test_nopanic(|a| generator(a, 3));
252+
231253
run_test_nopanic(|a| union1(a));
232254
}

0 commit comments

Comments
 (0)