Skip to content

Commit 22fc74e

Browse files
committed
Added DestructionScope variant to CodeExtent, representing the area
immediately surrounding a node that is a terminating_scope (e.g. statements, looping forms) during which the destructors run (the destructors for temporaries from the execution of that node, that is). Introduced DestructionScopeData newtype wrapper around ast::NodeId, to preserve invariant that FreeRegion and ScopeChain::BlockScope carry destruction scopes (rather than arbitrary CodeExtents). Insert DestructionScope and block Remainder into enclosing CodeExtents hierarchy. Add more doc for DestructionScope, complete with ASCII art. Switch to constructing DestructionScope rather than Misc in a number of places, mostly related to `ty::ReFree` creation, and use destruction-scopes of node-ids at various calls to liberate_late_bound_regions. middle::resolve_lifetime: Map BlockScope to DestructionScope in `fn resolve_free_lifetime`. Add the InnermostDeclaringBlock and InnermostEnclosingExpr enums that are my attempt to clarify the region::Context structure, and that later commmts build upon. Improve the debug output for `CodeExtent` attached to `ty::Region::ReScope`. Loosened an assertion in `rustc_trans::trans::cleanup` to account for `DestructionScope`. (Perhaps this should just be switched entirely over to `DestructionScope`, rather than allowing for either `Misc` or `DestructionScope`.) ---- Even though the DestructionScope is new, this particular commit should not actually change the semantics of any current code.
1 parent bdd5cc7 commit 22fc74e

File tree

18 files changed

+253
-73
lines changed

18 files changed

+253
-73
lines changed

src/librustc/metadata/tydecode.rs

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,7 +349,7 @@ fn parse_region_<F>(st: &mut PState, conv: &mut F) -> ty::Region where
349349
}
350350
'f' => {
351351
assert_eq!(next(st), '[');
352-
let scope = parse_scope(st);
352+
let scope = parse_destruction_scope_data(st);
353353
assert_eq!(next(st), '|');
354354
let br = parse_bound_region_(st, conv);
355355
assert_eq!(next(st), ']');
@@ -377,6 +377,10 @@ fn parse_scope(st: &mut PState) -> region::CodeExtent {
377377
let node_id = parse_uint(st) as ast::NodeId;
378378
region::CodeExtent::Misc(node_id)
379379
}
380+
'D' => {
381+
let node_id = parse_uint(st) as ast::NodeId;
382+
region::CodeExtent::DestructionScope(node_id)
383+
}
380384
'B' => {
381385
let node_id = parse_uint(st) as ast::NodeId;
382386
let first_stmt_index = parse_uint(st);
@@ -389,6 +393,11 @@ fn parse_scope(st: &mut PState) -> region::CodeExtent {
389393
}
390394
}
391395

396+
fn parse_destruction_scope_data(st: &mut PState) -> region::DestructionScopeData {
397+
let node_id = parse_uint(st) as ast::NodeId;
398+
region::DestructionScopeData::new(node_id)
399+
}
400+
392401
fn parse_opt<'a, 'tcx, T, F>(st: &mut PState<'a, 'tcx>, f: F) -> Option<T> where
393402
F: FnOnce(&mut PState<'a, 'tcx>) -> T,
394403
{

src/librustc/metadata/tyencode.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ pub fn enc_region(w: &mut SeekableMemWriter, cx: &ctxt, r: ty::Region) {
251251
}
252252
ty::ReFree(ref fr) => {
253253
mywrite!(w, "f[");
254-
enc_scope(w, cx, fr.scope);
254+
enc_destruction_scope_data(w, fr.scope);
255255
mywrite!(w, "|");
256256
enc_bound_region(w, cx, fr.bound_region);
257257
mywrite!(w, "]");
@@ -279,9 +279,15 @@ fn enc_scope(w: &mut SeekableMemWriter, _cx: &ctxt, scope: region::CodeExtent) {
279279
region::CodeExtent::Misc(node_id) => mywrite!(w, "M{}", node_id),
280280
region::CodeExtent::Remainder(region::BlockRemainder {
281281
block: b, first_statement_index: i }) => mywrite!(w, "B{}{}", b, i),
282+
region::CodeExtent::DestructionScope(node_id) => mywrite!(w, "D{}", node_id),
282283
}
283284
}
284285

286+
fn enc_destruction_scope_data(w: &mut SeekableMemWriter,
287+
d: region::DestructionScopeData) {
288+
mywrite!(w, "{}", d.node_id);
289+
}
290+
285291
fn enc_bound_region(w: &mut SeekableMemWriter, cx: &ctxt, br: ty::BoundRegion) {
286292
match br {
287293
ty::BrAnon(idx) => {

src/librustc/middle/astencode.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,12 @@ impl tr for region::CodeExtent {
499499
}
500500
}
501501

502+
impl tr for region::DestructionScopeData {
503+
fn tr(&self, dcx: &DecodeContext) -> region::DestructionScopeData {
504+
region::DestructionScopeData { node_id: dcx.tr_id(self.node_id) }
505+
}
506+
}
507+
502508
impl tr for ty::BoundRegion {
503509
fn tr(&self, dcx: &DecodeContext) -> ty::BoundRegion {
504510
match *self {

src/librustc/middle/infer/error_reporting.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -304,7 +304,7 @@ impl<'a, 'tcx> ErrorReporting<'tcx> for InferCtxt<'a, 'tcx> {
304304
return None
305305
}
306306
assert!(fr1.scope == fr2.scope);
307-
(fr1.scope.node_id(), fr1, fr2)
307+
(fr1.scope.node_id, fr1, fr2)
308308
},
309309
_ => return None
310310
};

src/librustc/middle/infer/region_inference/mod.rs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -760,11 +760,12 @@ impl<'a, 'tcx> RegionVarBindings<'a, 'tcx> {
760760
// A "free" region can be interpreted as "some region
761761
// at least as big as the block fr.scope_id". So, we can
762762
// reasonably compare free regions and scopes:
763-
match self.tcx.region_maps.nearest_common_ancestor(fr.scope, s_id) {
763+
let fr_scope = fr.scope.to_code_extent();
764+
match self.tcx.region_maps.nearest_common_ancestor(fr_scope, s_id) {
764765
// if the free region's scope `fr.scope_id` is bigger than
765766
// the scope region `s_id`, then the LUB is the free
766767
// region itself:
767-
Some(r_id) if r_id == fr.scope => f,
768+
Some(r_id) if r_id == fr_scope => f,
768769

769770
// otherwise, we don't know what the free region is,
770771
// so we must conservatively say the LUB is static:
@@ -865,8 +866,9 @@ impl<'a, 'tcx> RegionVarBindings<'a, 'tcx> {
865866
// than the scope `s_id`, then we can say that the GLB
866867
// is the scope `s_id`. Otherwise, as we do not know
867868
// big the free region is precisely, the GLB is undefined.
868-
match self.tcx.region_maps.nearest_common_ancestor(fr.scope, s_id) {
869-
Some(r_id) if r_id == fr.scope => Ok(s),
869+
let fr_scope = fr.scope.to_code_extent();
870+
match self.tcx.region_maps.nearest_common_ancestor(fr_scope, s_id) {
871+
Some(r_id) if r_id == fr_scope => Ok(s),
870872
_ => Err(ty::terr_regions_no_overlap(b, a))
871873
}
872874
}
@@ -915,7 +917,8 @@ impl<'a, 'tcx> RegionVarBindings<'a, 'tcx> {
915917
Ok(ty::ReFree(*b))
916918
} else {
917919
this.intersect_scopes(ty::ReFree(*a), ty::ReFree(*b),
918-
a.scope, b.scope)
920+
a.scope.to_code_extent(),
921+
b.scope.to_code_extent())
919922
}
920923
}
921924
}

src/librustc/middle/liveness.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ use self::VarKind::*;
112112
use middle::def::*;
113113
use middle::mem_categorization::Typer;
114114
use middle::pat_util;
115-
use middle::region::CodeExtent;
115+
use middle::region;
116116
use middle::ty;
117117
use middle::ty::ClosureTyper;
118118
use lint;
@@ -1514,7 +1514,7 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> {
15141514
let fn_ret =
15151515
ty::liberate_late_bound_regions(
15161516
self.ir.tcx,
1517-
CodeExtent::from_node_id(body.id),
1517+
region::DestructionScopeData::new(body.id),
15181518
&self.fn_ret(id));
15191519

15201520
match fn_ret {

src/librustc/middle/mem_categorization.rs

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -760,7 +760,10 @@ impl<'t,'tcx,TYPER:Typer<'tcx>> MemCategorizationContext<'t,TYPER> {
760760

761761
// Region of environment pointer
762762
let env_region = ty::ReFree(ty::FreeRegion {
763-
scope: region::CodeExtent::from_node_id(fn_body_id),
763+
// The environment of a closure is guaranteed to
764+
// outlive any bindings introduced in the body of the
765+
// closure itself.
766+
scope: region::DestructionScopeData::new(fn_body_id),
764767
bound_region: ty::BrEnv
765768
});
766769

src/librustc/middle/region.rs

Lines changed: 123 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,66 @@ use syntax::{ast, visit};
2727
use syntax::ast::{Block, Item, FnDecl, NodeId, Arm, Pat, Stmt, Expr, Local};
2828
use syntax::ast_util::{stmt_id};
2929
use syntax::ast_map;
30+
use syntax::ptr::P;
3031
use syntax::visit::{Visitor, FnKind};
3132

3233
/// CodeExtent represents a statically-describable extent that can be
3334
/// used to bound the lifetime/region for values.
3435
///
36+
/// `Misc(node_id)`: Any AST node that has any extent at all has the
37+
/// `Misc(node_id)` extent. Other variants represent special cases not
38+
/// immediately derivable from the abstract syntax tree structure.
39+
///
40+
/// `DestructionScope(node_id)` represents the extent of destructors
41+
/// implicitly-attached to `node_id` that run immediately after the
42+
/// expression for `node_id` itself. Not every AST node carries a
43+
/// `DestructionScope`, but those that are `terminating_scopes` do;
44+
/// see discussion with `RegionMaps`.
45+
///
46+
/// `Remainder(BlockRemainder { block, statement_index })` represents
47+
/// the extent of user code running immediately after the initializer
48+
/// expression for the indexed statement, until the end of the block.
49+
///
50+
/// So: the following code can be broken down into the extents beneath:
51+
/// ```
52+
/// let a = f().g( 'b: { let x = d(); let y = d(); x.h(y) } ) ;
53+
/// ```
54+
///
55+
/// +-+ (D12.)
56+
/// +-+ (D11.)
57+
/// +---------+ (R10.)
58+
/// +-+ (D9.)
59+
/// +----------+ (M8.)
60+
/// +----------------------+ (R7.)
61+
/// +-+ (D6.)
62+
/// +----------+ (M5.)
63+
/// +-----------------------------------+ (M4.)
64+
/// +--------------------------------------------------+ (M3.)
65+
/// +--+ (M2.)
66+
/// +-----------------------------------------------------------+ (M1.)
67+
///
68+
/// (M1.): Misc extent of the whole `let a = ...;` statement.
69+
/// (M2.): Misc extent of the `f()` expression.
70+
/// (M3.): Misc extent of the `f().g(..)` expression.
71+
/// (M4.): Misc extent of the block labelled `'b:`.
72+
/// (M5.): Misc extent of the `let x = d();` statement
73+
/// (D6.): DestructionScope for temporaries created during M5.
74+
/// (R7.): Remainder extent for block `'b:`, stmt 0 (let x = ...).
75+
/// (M8.): Misc Extent of the `let y = d();` statement.
76+
/// (D9.): DestructionScope for temporaries created during M8.
77+
/// (R10.): Remainder extent for block `'b:`, stmt 1 (let y = ...).
78+
/// (D11.): DestructionScope for temporaries and bindings from block `'b:`.
79+
/// (D12.): DestructionScope for temporaries created during M1 (e.g. f()).
80+
///
81+
/// Note that while the above picture shows the destruction scopes
82+
/// as following their corresponding misc extents, in the internal
83+
/// data structures of the compiler the destruction scopes are
84+
/// represented as enclosing parents. This is sound because we use the
85+
/// enclosing parent relationship just to ensure that referenced
86+
/// values live long enough; phrased another way, the starting point
87+
/// of each range is not really the important thing in the above
88+
/// picture, but rather the ending point.
89+
///
3590
/// FIXME (pnkfelix): This currently derives `PartialOrd` and `Ord` to
3691
/// placate the same deriving in `ty::FreeRegion`, but we may want to
3792
/// actually attach a more meaningful ordering to scopes than the one
@@ -40,7 +95,24 @@ use syntax::visit::{Visitor, FnKind};
4095
RustcDecodable, Debug, Copy)]
4196
pub enum CodeExtent {
4297
Misc(ast::NodeId),
43-
Remainder(BlockRemainder),
98+
DestructionScope(ast::NodeId), // extent of destructors for temporaries of node-id
99+
Remainder(BlockRemainder)
100+
}
101+
102+
/// extent of destructors for temporaries of node-id
103+
#[derive(Clone, PartialEq, PartialOrd, Eq, Ord, Hash, RustcEncodable,
104+
RustcDecodable, Debug, Copy)]
105+
pub struct DestructionScopeData {
106+
pub node_id: ast::NodeId
107+
}
108+
109+
impl DestructionScopeData {
110+
pub fn new(node_id: ast::NodeId) -> DestructionScopeData {
111+
DestructionScopeData { node_id: node_id }
112+
}
113+
pub fn to_code_extent(&self) -> CodeExtent {
114+
CodeExtent::DestructionScope(self.node_id)
115+
}
44116
}
45117

46118
/// Represents a subscope of `block` for a binding that is introduced
@@ -82,6 +154,7 @@ impl CodeExtent {
82154
match *self {
83155
CodeExtent::Misc(node_id) => node_id,
84156
CodeExtent::Remainder(br) => br.block,
157+
CodeExtent::DestructionScope(node_id) => node_id,
85158
}
86159
}
87160

@@ -95,6 +168,8 @@ impl CodeExtent {
95168
CodeExtent::Remainder(br) =>
96169
CodeExtent::Remainder(BlockRemainder {
97170
block: f_id(br.block), first_statement_index: br.first_statement_index }),
171+
CodeExtent::DestructionScope(node_id) =>
172+
CodeExtent::DestructionScope(f_id(node_id)),
98173
}
99174
}
100175

@@ -105,7 +180,8 @@ impl CodeExtent {
105180
match ast_map.find(self.node_id()) {
106181
Some(ast_map::NodeBlock(ref blk)) => {
107182
match *self {
108-
CodeExtent::Misc(_) => Some(blk.span),
183+
CodeExtent::Misc(_) |
184+
CodeExtent::DestructionScope(_) => Some(blk.span),
109185

110186
CodeExtent::Remainder(r) => {
111187
assert_eq!(r.block, blk.id);
@@ -455,7 +531,7 @@ impl RegionMaps {
455531
}
456532

457533
(ty::ReScope(sub_scope), ty::ReFree(ref fr)) => {
458-
self.is_subscope_of(sub_scope, fr.scope)
534+
self.is_subscope_of(sub_scope, fr.scope.to_code_extent())
459535
}
460536

461537
(ty::ReFree(sub_fr), ty::ReFree(super_fr)) => {
@@ -567,7 +643,18 @@ fn resolve_block(visitor: &mut RegionResolutionVisitor, blk: &ast::Block) {
567643
let prev_cx = visitor.cx;
568644

569645
let blk_scope = CodeExtent::Misc(blk.id);
570-
record_superlifetime(visitor, blk_scope, blk.span);
646+
// If block was previously marked as a terminating scope during
647+
// the recursive visit of its parent node in the AST, then we need
648+
// to account for the destruction scope representing the extent of
649+
// the destructors that run immediately after the the block itself
650+
// completes.
651+
if visitor.region_maps.terminating_scopes.borrow().contains(&blk_scope) {
652+
let dtor_scope = CodeExtent::DestructionScope(blk.id);
653+
record_superlifetime(visitor, dtor_scope, blk.span);
654+
visitor.region_maps.record_encl_scope(blk_scope, dtor_scope);
655+
} else {
656+
record_superlifetime(visitor, blk_scope, blk.span);
657+
}
571658

572659
// We treat the tail expression in the block (if any) somewhat
573660
// differently from the statements. The issue has to do with
@@ -675,7 +762,9 @@ fn resolve_stmt(visitor: &mut RegionResolutionVisitor, stmt: &ast::Stmt) {
675762
// statement plus its destructors, and thus the extent for which
676763
// regions referenced by the destructors need to survive.
677764
visitor.region_maps.mark_as_terminating_scope(stmt_scope);
678-
record_superlifetime(visitor, stmt_scope, stmt.span);
765+
let dtor_scope = CodeExtent::DestructionScope(stmt_id);
766+
visitor.region_maps.record_encl_scope(stmt_scope, dtor_scope);
767+
record_superlifetime(visitor, dtor_scope, stmt.span);
679768

680769
let prev_parent = visitor.cx.parent;
681770
visitor.cx.parent = InnermostEnclosingExpr::Some(stmt_id);
@@ -687,15 +776,30 @@ fn resolve_expr(visitor: &mut RegionResolutionVisitor, expr: &ast::Expr) {
687776
debug!("resolve_expr(expr.id={:?})", expr.id);
688777

689778
let expr_scope = CodeExtent::Misc(expr.id);
690-
record_superlifetime(visitor, expr_scope, expr.span);
779+
// If expr was previously marked as a terminating scope during the
780+
// recursive visit of its parent node in the AST, then we need to
781+
// account for the destruction scope representing the extent of
782+
// the destructors that run immediately after the the expression
783+
// itself completes.
784+
if visitor.region_maps.terminating_scopes.borrow().contains(&expr_scope) {
785+
let dtor_scope = CodeExtent::DestructionScope(expr.id);
786+
record_superlifetime(visitor, dtor_scope, expr.span);
787+
visitor.region_maps.record_encl_scope(expr_scope, dtor_scope);
788+
} else {
789+
record_superlifetime(visitor, expr_scope, expr.span);
790+
}
691791

692792
let prev_cx = visitor.cx;
693793
visitor.cx.parent = InnermostEnclosingExpr::Some(expr.id);
694794

695795
{
696796
let region_maps = &mut visitor.region_maps;
697-
let terminating = |id| {
698-
let scope = CodeExtent::from_node_id(id);
797+
let terminating = |e: &P<ast::Expr>| {
798+
let scope = CodeExtent::from_node_id(e.id);
799+
region_maps.mark_as_terminating_scope(scope)
800+
};
801+
let terminating_block = |b: &P<ast::Block>| {
802+
let scope = CodeExtent::from_node_id(b.id);
699803
region_maps.mark_as_terminating_scope(scope)
700804
};
701805
match expr.node {
@@ -707,26 +811,26 @@ fn resolve_expr(visitor: &mut RegionResolutionVisitor, expr: &ast::Expr) {
707811
ast::ExprBinary(codemap::Spanned { node: ast::BiOr, .. }, _, ref r) => {
708812
// For shortcircuiting operators, mark the RHS as a terminating
709813
// scope since it only executes conditionally.
710-
terminating(r.id);
814+
terminating(r);
711815
}
712816

713817
ast::ExprIf(_, ref then, Some(ref otherwise)) => {
714-
terminating(then.id);
715-
terminating(otherwise.id);
818+
terminating_block(then);
819+
terminating(otherwise);
716820
}
717821

718822
ast::ExprIf(ref expr, ref then, None) => {
719-
terminating(expr.id);
720-
terminating(then.id);
823+
terminating(expr);
824+
terminating_block(then);
721825
}
722826

723827
ast::ExprLoop(ref body, _) => {
724-
terminating(body.id);
828+
terminating_block(body);
725829
}
726830

727831
ast::ExprWhile(ref expr, ref body, _) => {
728-
terminating(expr.id);
729-
terminating(body.id);
832+
terminating(expr);
833+
terminating_block(body);
730834
}
731835

732836
ast::ExprMatch(..) => {
@@ -1021,6 +1125,9 @@ fn resolve_fn(visitor: &mut RegionResolutionVisitor,
10211125

10221126
let body_scope = CodeExtent::from_node_id(body.id);
10231127
visitor.region_maps.mark_as_terminating_scope(body_scope);
1128+
let dtor_scope = CodeExtent::DestructionScope(body.id);
1129+
visitor.region_maps.record_encl_scope(body_scope, dtor_scope);
1130+
record_superlifetime(visitor, dtor_scope, body.span);
10241131

10251132
let outer_cx = visitor.cx;
10261133

0 commit comments

Comments
 (0)