diff options
| -rw-r--r-- | src/librustc/hir/intravisit.rs | 5 | ||||
| -rw-r--r-- | src/librustc/middle/region.rs | 66 | ||||
| -rw-r--r-- | src/librustc_mir/build/expr/as_rvalue.rs | 6 | ||||
| -rw-r--r-- | src/librustc_typeck/check/generator_interior.rs | 4 | ||||
| -rw-r--r-- | src/test/run-pass/generator/yield-in-args-rev.rs | 4 | ||||
| -rw-r--r-- | src/test/run-pass/generator/yield-in-box.rs | 26 |
6 files changed, 95 insertions, 16 deletions
diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs index 29147a2a50a..9d8075de2eb 100644 --- a/src/librustc/hir/intravisit.rs +++ b/src/librustc/hir/intravisit.rs @@ -411,10 +411,13 @@ pub fn walk_body<'v, V: Visitor<'v>>(visitor: &mut V, body: &'v Body) { } pub fn walk_local<'v, V: Visitor<'v>>(visitor: &mut V, local: &'v Local) { + // Intentionally visiting the expr first - the initialization expr + // dominates the local's definition. + walk_list!(visitor, visit_expr, &local.init); + visitor.visit_id(local.id); visitor.visit_pat(&local.pat); walk_list!(visitor, visit_ty, &local.ty); - walk_list!(visitor, visit_expr, &local.init); } pub fn walk_lifetime<'v, V: Visitor<'v>>(visitor: &mut V, lifetime: &'v Lifetime) { diff --git a/src/librustc/middle/region.rs b/src/librustc/middle/region.rs index d5b6e388bde..8b73bc5c840 100644 --- a/src/librustc/middle/region.rs +++ b/src/librustc/middle/region.rs @@ -259,8 +259,37 @@ pub struct ScopeTree { /// lower than theirs, and therefore don't need to be suspended /// at yield-points at these indexes. /// - /// Let's show that: let `D` be our binding/temporary and `U` be our - /// other HIR node, with `HIR-postorder(U) < HIR-postorder(D)`. + /// For an example, suppose we have some code such as: + /// ```rust,ignore (example) + /// foo(f(), yield y, bar(g())) + /// ``` + /// + /// With the HIR tree (calls numbered for expository purposes) + /// ``` + /// Call#0(foo, [Call#1(f), Yield(y), Call#2(bar, Call#3(g))]) + /// ``` + /// + /// Obviously, the result of `f()` was created before the yield + /// (and therefore needs to be kept valid over the yield) while + /// the result of `g()` occurs after the yield (and therefore + /// doesn't). If we want to infer that, we can look at the + /// postorder traversal: + /// ``` + /// `foo` `f` Call#1 `y` Yield `bar` `g` Call#3 Call#2 Call#0 + /// ``` + /// + /// In which we can easily see that `Call#1` occurs before the yield, + /// and `Call#3` after it. + /// + /// To see that this method works, consider: + /// + /// Let `D` be our binding/temporary and `U` be our other HIR node, with + /// `HIR-postorder(U) < HIR-postorder(D)` (in our example, U would be + /// the yield and D would be one of the calls). Let's show that + /// `D` is storage-dead at `U`. + /// + /// Remember that storage-live/storage-dead refers to the state of + /// the *storage*, and does not consider moves/drop flags. /// /// Then: /// 1. From the ordering guarantee of HIR visitors (see @@ -272,17 +301,26 @@ pub struct ScopeTree { /// or always storage-dead. This is what is being guaranteed /// by `terminating_scopes` including all blocks where the /// count of executions is not guaranteed. - /// 4. By `2.` and `3.`, `D` is *statically* dead at `U`, + /// 4. By `2.` and `3.`, `D` is *statically* storage-dead at `U`, /// QED. /// /// I don't think this property relies on `3.` in an essential way - it /// is probably still correct even if we have "unrestricted" terminating /// scopes. However, why use the complicated proof when a simple one /// works? + /// + /// A subtle thing: `box` expressions, such as `box (&x, yield 2, &y)`. It + /// might seem that a `box` expression creates a `Box<T>` temporary + /// when it *starts* executing, at `HIR-preorder(BOX-EXPR)`. That might + /// be true in the MIR desugaring, but it is not important in the semantics. + /// + /// The reason is that semantically, until the `box` expression returns, + /// the values are still owned by their containing expressions. So + /// we'll see that `&x`. yield_in_scope: FxHashMap<Scope, (Span, usize)>, - /// The number of visit_expr calls done in the body. - /// Used to sanity check visit_expr call count when + /// The number of visit_expr and visit_pat calls done in the body. + /// Used to sanity check visit_expr/visit_pat call count when /// calculating geneartor interiors. body_expr_count: FxHashMap<hir::BodyId, usize>, } @@ -307,8 +345,8 @@ pub struct Context { struct RegionResolutionVisitor<'a, 'tcx: 'a> { tcx: TyCtxt<'a, 'tcx, 'tcx>, - // The number of expressions visited in the current body - expr_count: usize, + // The number of expressions and patterns visited in the current body + expr_and_pat_count: usize, // Generated scope tree: scope_tree: ScopeTree, @@ -758,6 +796,8 @@ fn resolve_pat<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, pat: & } intravisit::walk_pat(visitor, pat); + + visitor.expr_and_pat_count += 1; } fn resolve_stmt<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, stmt: &'tcx hir::Stmt) { @@ -863,14 +903,14 @@ fn resolve_expr<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, expr: _ => intravisit::walk_expr(visitor, expr) } - visitor.expr_count += 1; + visitor.expr_and_pat_count += 1; if let hir::ExprYield(..) = expr.node { // Mark this expr's scope and all parent scopes as containing `yield`. let mut scope = Scope::Node(expr.hir_id.local_id); loop { visitor.scope_tree.yield_in_scope.insert(scope, - (expr.span, visitor.expr_count)); + (expr.span, visitor.expr_and_pat_count)); // Keep traversing up while we can. match visitor.scope_tree.parent_map.get(&scope) { @@ -1160,7 +1200,7 @@ impl<'a, 'tcx> Visitor<'tcx> for RegionResolutionVisitor<'a, 'tcx> { body_id, self.cx.parent); - let outer_ec = mem::replace(&mut self.expr_count, 0); + let outer_ec = mem::replace(&mut self.expr_and_pat_count, 0); let outer_cx = self.cx; let outer_ts = mem::replace(&mut self.terminating_scopes, FxHashSet()); self.terminating_scopes.insert(body.value.hir_id.local_id); @@ -1207,11 +1247,11 @@ impl<'a, 'tcx> Visitor<'tcx> for RegionResolutionVisitor<'a, 'tcx> { } if body.is_generator { - self.scope_tree.body_expr_count.insert(body_id, self.expr_count); + self.scope_tree.body_expr_count.insert(body_id, self.expr_and_pat_count); } // Restore context we had at the start. - self.expr_count = outer_ec; + self.expr_and_pat_count = outer_ec; self.cx = outer_cx; self.terminating_scopes = outer_ts; } @@ -1246,7 +1286,7 @@ fn region_scope_tree<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, def_id: DefId) let mut visitor = RegionResolutionVisitor { tcx, scope_tree: ScopeTree::default(), - expr_count: 0, + expr_and_pat_count: 0, cx: Context { root_id: None, parent: None, diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs index f0b6a4fcfd9..f2607b164de 100644 --- a/src/librustc_mir/build/expr/as_rvalue.rs +++ b/src/librustc_mir/build/expr/as_rvalue.rs @@ -96,7 +96,11 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { } ExprKind::Box { value } => { let value = this.hir.mirror(value); - let result = this.local_decls.push(LocalDecl::new_temp(expr.ty, expr_span)); + // The `Box<T>` temporary created here is not a part of the HIR, + // and therefore is not considered during generator OIBIT + // determination. See the comment about `box` at `yield_in_scope`. + let result = this.local_decls.push( + LocalDecl::new_internal(expr.ty, expr_span)); this.cfg.push(block, Statement { source_info, kind: StatementKind::StorageLive(result) diff --git a/src/librustc_typeck/check/generator_interior.rs b/src/librustc_typeck/check/generator_interior.rs index 168e0251caf..af1297697c2 100644 --- a/src/librustc_typeck/check/generator_interior.rs +++ b/src/librustc_typeck/check/generator_interior.rs @@ -41,7 +41,7 @@ impl<'a, 'gcx, 'tcx> InteriorVisitor<'a, 'gcx, 'tcx> { // be storage-live (and therefore live) at any of the yields. // // See the mega-comment at `yield_in_scope` for a proof. - if expr.is_none() || expr_count >= self.expr_count { + if expr_count >= self.expr_count { Some(span) } else { None @@ -115,6 +115,8 @@ impl<'a, 'gcx, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'gcx, 'tcx> { self.record(ty, Some(scope), None); } + self.expr_count += 1; + intravisit::walk_pat(self, pat); } diff --git a/src/test/run-pass/generator/yield-in-args-rev.rs b/src/test/run-pass/generator/yield-in-args-rev.rs index e22759291d1..df00329799e 100644 --- a/src/test/run-pass/generator/yield-in-args-rev.rs +++ b/src/test/run-pass/generator/yield-in-args-rev.rs @@ -8,6 +8,10 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +// Test that a borrow that occurs after a yield in the same +// argument list is not treated as live across the yield by +// type-checking. + #![feature(generators)] fn foo(_a: (), _b: &bool) {} diff --git a/src/test/run-pass/generator/yield-in-box.rs b/src/test/run-pass/generator/yield-in-box.rs new file mode 100644 index 00000000000..d68007be05c --- /dev/null +++ b/src/test/run-pass/generator/yield-in-box.rs @@ -0,0 +1,26 @@ +// Copyright 2017 The Rust Project Developers. See the COPYRIGHT +// file at the top-level directory of this distribution and at +// http://rust-lang.org/COPYRIGHT. +// +// Licensed under the Apache License, Version 2.0 <LICENSE-APACHE or +// http://www.apache.org/licenses/LICENSE-2.0> or the MIT license +// <LICENSE-MIT or http://opensource.org/licenses/MIT>, at your +// option. This file may not be copied, modified, or distributed +// except according to those terms. + +// Test that box-statements with yields in them work. + +#![feature(generators, box_syntax)] + +fn main() { + let x = 0i32; + || { + let y = 2u32; + { + let _t = box (&x, yield 0, &y); + } + match box (&x, yield 0, &y) { + _t => {} + } + }; +} |