Encode more precise scoping rules for function params

src/librustc/metadata/tydecode.rs

@@ -373,6 +373,16 @@ fn parse_region_<F>(st: &mut PState, conv: &mut F) -> ty::Region where
 
 fn parse_scope(st: &mut PState) -> region::CodeExtent {
     match next(st) {
+        'P' => {
+            assert_eq!(next(st), '[');
+            let fn_id = parse_uint(st) as ast::NodeId;
+            assert_eq!(next(st), '|');
+            let body_id = parse_uint(st) as ast::NodeId;
+            assert_eq!(next(st), ']');
+            region::CodeExtent::ParameterScope {
+                fn_id: fn_id, body_id: body_id
+            }
+        }
         'M' => {
             let node_id = parse_uint(st) as ast::NodeId;
             region::CodeExtent::Misc(node_id)
@@ -382,8 +392,11 @@ fn parse_scope(st: &mut PState) -> region::CodeExtent {
             region::CodeExtent::DestructionScope(node_id)
         }
         'B' => {
+            assert_eq!(next(st), '[');
             let node_id = parse_uint(st) as ast::NodeId;
+            assert_eq!(next(st), '|');
             let first_stmt_index = parse_uint(st);
+            assert_eq!(next(st), ']');
             let block_remainder = region::BlockRemainder {
                 block: node_id, first_statement_index: first_stmt_index,
             };

src/librustc/metadata/tyencode.rs

@@ -275,9 +275,11 @@ pub fn enc_region(w: &mut Encoder, cx: &ctxt, r: ty::Region) {
 
 fn enc_scope(w: &mut Encoder, _cx: &ctxt, scope: region::CodeExtent) {
     match scope {
+        region::CodeExtent::ParameterScope {
+            fn_id, body_id } => mywrite!(w, "P[{}|{}]", fn_id, body_id),
         region::CodeExtent::Misc(node_id) => mywrite!(w, "M{}", node_id),
         region::CodeExtent::Remainder(region::BlockRemainder {
-            block: b, first_statement_index: i }) => mywrite!(w, "B{}{}", b, i),
+            block: b, first_statement_index: i }) => mywrite!(w, "B[{}|{}]", b, i),
         region::CodeExtent::DestructionScope(node_id) => mywrite!(w, "D{}", node_id),
     }
 }

src/librustc/middle/region.rs

@@ -95,7 +95,15 @@ use syntax::visit::{Visitor, FnKind};
            RustcDecodable, Debug, Copy)]
 pub enum CodeExtent {
     Misc(ast::NodeId),
-    DestructionScope(ast::NodeId), // extent of destructors for temporaries of node-id
+
+    // extent of parameters passed to a function or closure (they
+    // outlive its body)
+    ParameterScope { fn_id: ast::NodeId, body_id: ast::NodeId },
+
+    // extent of destructors for temporaries of node-id
+    DestructionScope(ast::NodeId),
+
+    // extent of code following a `let id = expr;` binding in a block
     Remainder(BlockRemainder)
 }
 
@@ -153,15 +161,19 @@ impl CodeExtent {
     pub fn node_id(&self) -> ast::NodeId {
         match *self {
             CodeExtent::Misc(node_id) => node_id,
+
+            // These cases all return rough approximations to the
+            // precise extent denoted by `self`.
             CodeExtent::Remainder(br) => br.block,
             CodeExtent::DestructionScope(node_id) => node_id,
+            CodeExtent::ParameterScope { fn_id: _, body_id } => body_id,
         }
     }
 
     /// Maps this scope to a potentially new one according to the
     /// NodeId transformer `f_id`.
-    pub fn map_id<F>(&self, f_id: F) -> CodeExtent where
-        F: FnOnce(ast::NodeId) -> ast::NodeId,
+    pub fn map_id<F>(&self, mut f_id: F) -> CodeExtent where
+        F: FnMut(ast::NodeId) -> ast::NodeId,
     {
         match *self {
             CodeExtent::Misc(node_id) => CodeExtent::Misc(f_id(node_id)),
@@ -170,6 +182,8 @@ impl CodeExtent {
                     block: f_id(br.block), first_statement_index: br.first_statement_index }),
             CodeExtent::DestructionScope(node_id) =>
                 CodeExtent::DestructionScope(f_id(node_id)),
+            CodeExtent::ParameterScope { fn_id, body_id } =>
+                CodeExtent::ParameterScope { fn_id: f_id(fn_id), body_id: f_id(body_id) },
         }
     }
 
@@ -180,6 +194,7 @@ impl CodeExtent {
         match ast_map.find(self.node_id()) {
             Some(ast_map::NodeBlock(ref blk)) => {
                 match *self {
+                    CodeExtent::ParameterScope { .. } |
                     CodeExtent::Misc(_) |
                     CodeExtent::DestructionScope(_) => Some(blk.span),
 
@@ -277,6 +292,7 @@ enum InnermostDeclaringBlock {
     Block(ast::NodeId),
     Statement(DeclaringStatementContext),
     Match(ast::NodeId),
+    FnDecl { fn_id: ast::NodeId, body_id: ast::NodeId },
 }
 
 impl InnermostDeclaringBlock {
@@ -285,6 +301,8 @@ impl InnermostDeclaringBlock {
             InnermostDeclaringBlock::None => {
                 return Option::None;
             }
+            InnermostDeclaringBlock::FnDecl { fn_id, body_id } =>
+                CodeExtent::ParameterScope { fn_id: fn_id, body_id: body_id },
             InnermostDeclaringBlock::Block(id) |
             InnermostDeclaringBlock::Match(id) => CodeExtent::from_node_id(id),
             InnermostDeclaringBlock::Statement(s) =>  s.to_code_extent(),
@@ -1198,25 +1216,34 @@ fn resolve_fn(visitor: &mut RegionResolutionVisitor,
            body.id,
            visitor.cx.parent);
 
+    // This scope covers the function body, which includes the
+    // bindings introduced by let statements as well as temporaries
+    // created by the fn's tail expression (if any). It does *not*
+    // include the fn parameters (see below).
     let body_scope = CodeExtent::from_node_id(body.id);
     visitor.region_maps.mark_as_terminating_scope(body_scope);
 
     let dtor_scope = CodeExtent::DestructionScope(body.id);
     visitor.region_maps.record_encl_scope(body_scope, dtor_scope);
 
-    record_superlifetime(visitor, dtor_scope, body.span);
+    let fn_decl_scope = CodeExtent::ParameterScope { fn_id: id, body_id: body.id };
+    visitor.region_maps.record_encl_scope(dtor_scope, fn_decl_scope);
+
+    record_superlifetime(visitor, fn_decl_scope, body.span);
 
     if let Some(root_id) = visitor.cx.root_id {
         visitor.region_maps.record_fn_parent(body.id, root_id);
     }
 
     let outer_cx = visitor.cx;
 
-    // The arguments and `self` are parented to the body of the fn.
+    // The arguments and `self` are parented to the fn.
     visitor.cx = Context {
         root_id: Some(body.id),
-        parent: InnermostEnclosingExpr::Some(body.id),
-        var_parent: InnermostDeclaringBlock::Block(body.id)
+        parent: InnermostEnclosingExpr::None,
+        var_parent: InnermostDeclaringBlock::FnDecl {
+            fn_id: id, body_id: body.id
+        },
     };
     visit::walk_fn_decl(visitor, decl);
 

src/librustc/util/ppaux.rs

@@ -113,6 +113,9 @@ pub fn explain_region_and_span(cx: &ctxt, region: ty::Region)
         };
         let scope_decorated_tag = match scope {
             region::CodeExtent::Misc(_) => tag,
+            region::CodeExtent::ParameterScope { .. } => {
+                "scope of parameters for function"
+            }
             region::CodeExtent::DestructionScope(_) => {
                 new_string = format!("destruction scope surrounding {}", tag);
                 &*new_string
@@ -952,6 +955,8 @@ impl<'tcx> Repr<'tcx> for ty::FreeRegion {
 impl<'tcx> Repr<'tcx> for region::CodeExtent {
     fn repr(&self, _tcx: &ctxt) -> String {
         match *self {
+            region::CodeExtent::ParameterScope { fn_id, body_id } =>
+                format!("ParameterScope({}, {})", fn_id, body_id),
             region::CodeExtent::Misc(node_id) =>
                 format!("Misc({})", node_id),
             region::CodeExtent::DestructionScope(node_id) =>

src/test/compile-fail/issue-23338-locals-die-before-temps-of-body.rs

@@ -0,0 +1,36 @@
+// Copyright 2015 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.
+
+// This is just checking that we still reject code where temp values
+// are borrowing values for longer than they will be around.
+//
+// Compare to run-pass/issue-23338-params-outlive-temps-of-body.rs
+
+use std::cell::RefCell;
+
+fn foo(x: RefCell<String>) -> String {
+    let y = x;
+    y.borrow().clone() //~ ERROR `y` does not live long enough
+}
+
+fn foo2(x: RefCell<String>) -> String {
+    let ret = {
+        let y = x;
+        y.borrow().clone() //~ ERROR `y` does not live long enough
+    };
+    ret
+}
+
+fn main() {
+    let r = RefCell::new(format!("data"));
+    assert_eq!(foo(r), "data");
+    let r = RefCell::new(format!("data"));
+    assert_eq!(foo2(r), "data");
+}

src/test/run-pass/issue-23338-ensure-param-drop-order.rs

@@ -0,0 +1,171 @@
+// Copyright 2015 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.
+
+// ignore-pretty : (#23623) problems when  ending with // comments
+
+// This test is ensuring that parameters are indeed dropped after
+// temporaries in a fn body.
+
+use std::cell::RefCell;
+
+use self::d::D;
+
+pub fn main() {
+    let log = RefCell::new(vec![]);
+    d::println(&format!("created empty log"));
+    test(&log);
+
+    assert_eq!(&log.borrow()[..],
+               [
+                   //                                    created empty log
+                   //    +-- Make D(da_0, 0)
+                   //    | +-- Make D(de_1, 1)
+                   //    | |                             calling foo
+                   //    | |                             entered foo
+                   //    | | +-- Make D(de_2, 2)
+                   //    | | | +-- Make D(da_1, 3)
+                   //    | | | | +-- Make D(de_3, 4)
+                   //    | | | | | +-- Make D(de_4, 5)
+                   3, // | | | +-- Drop D(da_1, 3)
+                   //    | | |   | |
+                   4, // | | |   +-- Drop D(de_3, 4)
+                   //    | | |     |
+                   //    | | |     |                     eval tail of foo
+                   //    | | | +-- Make D(de_5, 6)
+                   //    | | | | +-- Make D(de_6, 7)
+                   6, // | | | +-- Drop D(de_5, 6)
+                   //    | | |   | |
+                   5, // | | |   | +-- Drop D(de_4, 5)
+                   //    | | |   |
+                   2, // | | +-- Drop D(de_2, 2)
+                   //    | |     |
+                   1, // | +-- Drop D(de_1, 1)
+                   //    |       |
+                   0, // +-- Drop D(da_0, 0)
+                   //            |
+                   //            |                       result D(de_6, 7)
+                   7 //          +-- Drop D(de_6, 7)
+
+                       ]);
+}
+
+fn test<'a>(log: d::Log<'a>) {
+    let da = D::new("da", 0, log);
+    let de = D::new("de", 1, log);
+    d::println(&format!("calling foo"));
+    let result = foo(da, de);
+    d::println(&format!("result {}", result));
+}
+
+fn foo<'a>(da0: D<'a>, de1: D<'a>) -> D<'a> {
+    d::println(&format!("entered foo"));
+    let de2 = de1.incr();      // creates D(de_2, 2)
+    let de4 = {
+        let _da1 = da0.incr(); // creates D(da_1, 3)
+        de2.incr().incr()      // creates D(de_3, 4) and D(de_4, 5)
+    };
+    d::println(&format!("eval tail of foo"));
+    de4.incr().incr()          // creates D(de_5, 6) and D(de_6, 7)
+}
+
+// This module provides simultaneous printouts of the dynamic extents
+// of all of the D values, in addition to logging the order that each
+// is dropped.
+
+const PREF_INDENT: u32 = 16;
+
+pub mod d {
+    #![allow(unused_parens)]
+    use std::fmt;
+    use std::mem;
+    use std::cell::RefCell;
+
+    static mut counter: u32 = 0;
+    static mut trails: u64 = 0;
+
+    pub type Log<'a> = &'a RefCell<Vec<u32>>;
+
+    pub fn current_width() -> u32 {
+        unsafe { max_width() - trails.leading_zeros() }
+    }
+
+    pub fn max_width() -> u32 {
+        unsafe {
+            (mem::size_of_val(&trails)*8) as u32
+        }
+    }
+
+    pub fn indent_println(my_trails: u32, s: &str) {
+        let mut indent: String = String::new();
+        for i in 0..my_trails {
+            unsafe {
+                if trails & (1 << i) != 0 {
+                    indent = indent + "| ";
+                } else {
+                    indent = indent + "  ";
+                }
+            }
+        }
+        println!("{}{}", indent, s);
+    }
+
+    pub fn println(s: &str) {
+        indent_println(super::PREF_INDENT, s);
+    }
+
+    fn first_avail() -> u32 {
+        unsafe {
+            for i in 0..64 {
+                if trails & (1 << i) == 0 {
+                    return i;
+                }
+            }
+        }
+        panic!("exhausted trails");
+    }
+
+    pub struct D<'a> {
+        name: &'static str, i: u32, uid: u32, trail: u32, log: Log<'a>
+    }
+
+    impl<'a> fmt::Display for D<'a> {
+        fn fmt(&self, w: &mut fmt::Formatter) -> fmt::Result {
+            write!(w, "D({}_{}, {})", self.name, self.i, self.uid)
+        }
+    }
+
+    impl<'a> D<'a> {
+        pub fn new(name: &'static str, i: u32, log: Log<'a>) -> D<'a> {
+            unsafe {
+                let trail = first_avail();
+                let ctr = counter;
+                counter += 1;
+                trails |= (1 << trail);
+                let ret = D {
+                    name: name, i: i, log: log, uid: ctr, trail: trail
+                };
+                indent_println(trail, &format!("+-- Make {}", ret));
+                ret
+            }
+        }
+        pub fn incr(&self) -> D<'a> {
+            D::new(self.name, self.i + 1, self.log)
+        }
+    }
+
+    impl<'a> Drop for D<'a> {
+        fn drop(&mut self) {
+            unsafe { trails &= !(1 << self.trail); };
+            self.log.borrow_mut().push(self.uid);
+            indent_println(self.trail, &format!("+-- Drop {}", self));
+            indent_println(::PREF_INDENT, "");
+        }
+    }
+}