Skip to content

Commit edf4328

Browse files
committed
leak the affects of closures on the free-region-map, like we used to
This restores the behavior of regionck with respect to the free-region-map: that is, it collects all the relations from the fn and its closures. This feels a bit fishy but it's the behavior we've had for some time, and it will go away with NLL, so seems best to just keep it.
1 parent 68eb102 commit edf4328

File tree

3 files changed

+92
-2
lines changed

3 files changed

+92
-2
lines changed

src/librustc/infer/outlives/env.rs

+46
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,52 @@ impl<'a, 'gcx: 'tcx, 'tcx: 'a> OutlivesEnvironment<'tcx> {
8989
self.free_region_map
9090
}
9191

92+
/// This is a hack to support the old-skool regionck, which
93+
/// processes region constraints from the main function and the
94+
/// closure together. In that context, when we enter a closure, we
95+
/// want to be able to "save" the state of the surrounding a
96+
/// function. We can then add implied bounds and the like from the
97+
/// closure arguments into the environment -- these should only
98+
/// apply in the closure body, so once we exit, we invoke
99+
/// `pop_snapshot_post_closure` to remove them.
100+
///
101+
/// Example:
102+
///
103+
/// ```
104+
/// fn foo<T>() {
105+
/// callback(for<'a> |x: &'a T| {
106+
/// // ^^^^^^^ not legal syntax, but probably should be
107+
/// // within this closure body, `T: 'a` holds
108+
/// })
109+
/// }
110+
/// ```
111+
///
112+
/// This "containment" of closure's effects only works so well. In
113+
/// particular, we (intentionally) leak relationships between free
114+
/// regions that are created by the closure's bounds. The case
115+
/// where this is useful is when you have (e.g.) a closure with a
116+
/// signature like `for<'a, 'b> fn(x: &'a &'b u32)` -- in this
117+
/// case, we want to keep the relationship `'b: 'a` in the
118+
/// free-region-map, so that later if we have to take `LUB('b,
119+
/// 'a)` we can get the result `'b`.
120+
///
121+
/// I have opted to keep **all modifications** to the
122+
/// free-region-map, however, and not just those that concern free
123+
/// variables bound in the closure. The latter seems more correct,
124+
/// but it is not the existing behavior, and I could not find a
125+
/// case where the existing behavior went wrong. In any case, it
126+
/// seems like it'd be readily fixed if we wanted. There are
127+
/// similar leaks around givens that seem equally suspicious, to
128+
/// be honest. --nmatsakis
129+
pub fn push_snapshot_pre_closure(&self) -> usize {
130+
self.region_bound_pairs.len()
131+
}
132+
133+
/// See `push_snapshot_pre_closure`.
134+
pub fn pop_snapshot_post_closure(&mut self, len: usize) {
135+
self.region_bound_pairs.truncate(len);
136+
}
137+
92138
/// This method adds "implied bounds" into the outlives environment.
93139
/// Implied bounds are outlives relationships that we can deduce
94140
/// on the basis that certain types must be well-formed -- these are

src/librustc_typeck/check/regionck.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -428,17 +428,17 @@ impl<'a, 'gcx, 'tcx> Visitor<'gcx> for RegionCtxt<'a, 'gcx, 'tcx> {
428428

429429
// Save state of current function before invoking
430430
// `visit_fn_body`. We will restore afterwards.
431-
let outlives_environment = self.outlives_environment.clone();
432431
let old_body_id = self.body_id;
433432
let old_call_site_scope = self.call_site_scope;
433+
let env_snapshot = self.outlives_environment.push_snapshot_pre_closure();
434434

435435
let body = self.tcx.hir.body(body_id);
436436
self.visit_fn_body(id, body, span);
437437

438438
// Restore state from previous function.
439+
self.outlives_environment.pop_snapshot_post_closure(env_snapshot);
439440
self.call_site_scope = old_call_site_scope;
440441
self.body_id = old_body_id;
441-
self.outlives_environment = outlives_environment;
442442
}
443443

444444
//visit_pat: visit_pat, // (..) see above
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
// Copyright 2016 The Rust Project Developers. See the COPYRIGHT
2+
// file at the top-level directory of this distribution and at
3+
// http://rust-lang.org/COPYRIGHT.
4+
//
5+
// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or
6+
// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license
7+
// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your
8+
// option. This file may not be copied, modified, or distributed
9+
// except according to those terms.
10+
11+
// Test that we are able to handle the relationships between free
12+
// regions bound in a closure callback.
13+
14+
#[derive(Copy, Clone)]
15+
struct MyCx<'short, 'long: 'short> {
16+
short: &'short u32,
17+
long: &'long u32,
18+
}
19+
20+
impl<'short, 'long> MyCx<'short, 'long> {
21+
fn short(self) -> &'short u32 { self.short }
22+
fn long(self) -> &'long u32 { self.long }
23+
fn set_short(&mut self, v: &'short u32) { self.short = v; }
24+
}
25+
26+
fn with<F, R>(op: F) -> R
27+
where
28+
F: for<'short, 'long> FnOnce(MyCx<'short, 'long>) -> R,
29+
{
30+
op(MyCx {
31+
short: &22,
32+
long: &22,
33+
})
34+
}
35+
36+
fn main() {
37+
with(|mut cx| {
38+
// For this to type-check, we need to be able to deduce that
39+
// the lifetime of `l` can be `'short`, even though it has
40+
// input from `'long`.
41+
let l = if true { cx.long() } else { cx.short() };
42+
cx.set_short(l);
43+
});
44+
}

0 commit comments

Comments
 (0)