make borrowck more conservative around rvalues.

this will require more temporaries, but is probably less magical.
also, it means that borrowck matches trans better, so fewer crashes.
bonus.

Finally, stop warning about implicit copies when we are actually borrowing.

Also, one test (vec-res-add) stopped failing due to #2587, and hence I
added an xfail-test.

Fixes #3217, #2977, #3067

9 files changed, 89 insertions, 49 deletions

diff --git a/src/libsyntax/ast_map.rs b/src/libsyntax/ast_map.rs
index c9b6b879427..70aaa5e8be6 100644
--- a/src/libsyntax/ast_map.rs
+++ b/src/libsyntax/ast_map.rs
@@ -2,7 +2,7 @@ import std::map;
 import std::map::hashmap;
 import ast::*;
 import print::pprust;
-import ast_util::path_to_ident;
+import ast_util::{path_to_ident, stmt_id};
 import diagnostic::span_handler;
 
 enum path_elt { path_mod(ident), path_name(ident) }
@@ -39,6 +39,7 @@ enum ast_node {
     node_method(@method, def_id /* impl did */, @path /* path to the impl */),
     node_variant(variant, @item, @path),
     node_expr(@expr),
+    node_stmt(@stmt),
     node_export(@view_path, @path),
     // Locals are numbered, because the alias analysis needs to know in which
     // order they are introduced.
@@ -65,6 +66,7 @@ fn mk_ast_map_visitor() -> vt {
     return visit::mk_vt(@{
         visit_item: map_item,
         visit_expr: map_expr,
+        visit_stmt: map_stmt,
         visit_fn: map_fn,
         visit_local: map_local,
         visit_arm: map_arm,
@@ -284,6 +286,11 @@ fn map_expr(ex: @expr, cx: ctx, v: vt) {
     visit::visit_expr(ex, cx, v);
 }
 
+fn map_stmt(stmt: @stmt, cx: ctx, v: vt) {
+    cx.map.insert(stmt_id(*stmt), node_stmt(stmt));
+    visit::visit_stmt(stmt, cx, v);
+}
+
 fn node_id_to_str(map: map, id: node_id) -> ~str {
     match map.find(id) {
       none => {
@@ -313,6 +320,10 @@ fn node_id_to_str(map: map, id: node_id) -> ~str {
         fmt!{"expr %s (id=%?)",
              pprust::expr_to_str(expr), id}
       }
+      some(node_stmt(stmt)) => {
+        fmt!{"stmt %s (id=%?)",
+             pprust::stmt_to_str(*stmt), id}
+      }
       // FIXMEs are as per #2410
       some(node_export(_, path)) => {
         fmt!{"export %s (id=%?)", // add more info here
diff --git a/src/rustc/middle/borrowck/preserve.rs b/src/rustc/middle/borrowck/preserve.rs
index 964a54bcad4..7b3b20db269 100644
--- a/src/rustc/middle/borrowck/preserve.rs
+++ b/src/rustc/middle/borrowck/preserve.rs
@@ -79,10 +79,9 @@ priv impl &preserve_ctxt {
             let scope_region = if self.root_ub == 0 {
                 ty::re_static
             } else {
-                ty::re_scope(self.root_ub)
+                ty::re_scope(self.tcx().region_map.get(cmt.id))
             };
 
-            // FIXME(#2977)--need to update trans!
             self.compare_scope(cmt, scope_region)
           }
           cat_stack_upvar(cmt) => {
diff --git a/src/rustc/middle/kind.rs b/src/rustc/middle/kind.rs
index e960e21c752..c41758f7052 100644
--- a/src/rustc/middle/kind.rs
+++ b/src/rustc/middle/kind.rs
@@ -389,8 +389,17 @@ fn is_nullary_variant(cx: ctx, ex: @expr) -> bool {
 
 fn check_copy_ex(cx: ctx, ex: @expr, implicit_copy: bool) {
     if ty::expr_is_lval(cx.method_map, ex) &&
-       !cx.last_use_map.contains_key(ex.id) &&
-       !is_nullary_variant(cx, ex) {
+
+        // this is a move
+        !cx.last_use_map.contains_key(ex.id) &&
+
+        // a reference to a constant like `none`... no need to warn
+        // about *this* even if the type is option<~int>
+        !is_nullary_variant(cx, ex) &&
+
+        // borrowed unique value isn't really a copy
+        !cx.tcx.borrowings.contains_key(ex.id)
+    {
         let ty = ty::expr_ty(cx.tcx, ex);
         check_copy(cx, ex.id, ty, ex.span, implicit_copy);
     }
diff --git a/src/rustc/middle/region.rs b/src/rustc/middle/region.rs
index e226e0ef9c5..4071800d4cf 100644
--- a/src/rustc/middle/region.rs
+++ b/src/rustc/middle/region.rs
@@ -28,16 +28,24 @@ type binding = {node_id: ast::node_id,
                 name: ~str,
                 br: ty::bound_region};
 
-/// Mapping from a block/expr/binding to the innermost scope that
-/// bounds its lifetime.  For a block/expression, this is the lifetime
-/// in which it will be evaluated.  For a binding, this is the lifetime
-/// in which is in scope.
+/**
+Encodes the bounding lifetime for a given AST node:
+
+- Expressions are mapped to the expression or block encoding the maximum
+  (static) lifetime of a value produced by that expression.  This is
+  generally the innermost call, statement, match, or block.
+
+- Variables and bindings are mapped to the block in which they are declared.
+
+*/
 type region_map = hashmap<ast::node_id, ast::node_id>;
 
-type ctxt = {
-    sess: session,
-    def_map: resolve3::DefMap,
-    region_map: region_map,
+struct ctxt {
+    sess: session;
+    def_map: resolve3::DefMap;
+
+    // Generated maps:
+    region_map: region_map;
 
     // Generally speaking, expressions are parented to their innermost
     // enclosing block. But some kinds of expressions serve as
@@ -46,9 +54,9 @@ type ctxt = {
     // the condition in a while loop is always a parent.  In those
     // cases, we add the node id of such an expression to this set so
     // that when we visit it we can view it as a parent.
-    root_exprs: hashmap<ast::node_id, ()>,
+    root_exprs: hashmap<ast::node_id, ()>;
 
-    // The parent scope is the innermost block, call, or alt
+    // The parent scope is the innermost block, statement, call, or alt
     // expression during the execution of which the current expression
     // will be evaluated.  Generally speaking, the innermost parent
     // scope is also the closest suitable ancestor in the AST tree.
@@ -79,8 +87,8 @@ type ctxt = {
     // Here, the first argument `&**x` will be a borrow of the `~int`,
     // but the second argument overwrites that very value! Bad.
     // (This test is borrowck-pure-scope-in-call.rs, btw)
-    parent: parent
-};
+    parent: parent;
+}
 
 /// Returns true if `subscope` is equal to or is lexically nested inside
 /// `superscope` and false otherwise.
@@ -186,12 +194,9 @@ fn parent_id(cx: ctxt, span: span) -> ast::node_id {
 
 /// Records the current parent (if any) as the parent of `child_id`.
 fn record_parent(cx: ctxt, child_id: ast::node_id) {
-    match cx.parent {
-      none => { /* no-op */ }
-      some(parent_id) => {
+    for cx.parent.each |parent_id| {
         debug!{"parent of node %d is node %d", child_id, parent_id};
         cx.region_map.insert(child_id, parent_id);
-      }
     }
 }
 
@@ -200,7 +205,7 @@ fn resolve_block(blk: ast::blk, cx: ctxt, visitor: visit::vt<ctxt>) {
     record_parent(cx, blk.node.id);
 
     // Descend.
-    let new_cx: ctxt = {parent: some(blk.node.id) with cx};
+    let new_cx: ctxt = ctxt {parent: some(blk.node.id) with cx};
     visit::visit_block(blk, new_cx, visitor);
 }
 
@@ -228,6 +233,21 @@ fn resolve_pat(pat: @ast::pat, cx: ctxt, visitor: visit::vt<ctxt>) {
     visit::visit_pat(pat, cx, visitor);
 }
 
+fn resolve_stmt(stmt: @ast::stmt, cx: ctxt, visitor: visit::vt<ctxt>) {
+    match stmt.node {
+      ast::stmt_decl(*) => {
+        visit::visit_stmt(stmt, cx, visitor);
+      }
+      ast::stmt_expr(expr, stmt_id) |
+      ast::stmt_semi(expr, stmt_id) => {
+        record_parent(cx, stmt_id);
+        let mut expr_cx = cx;
+        expr_cx.parent = some(stmt_id);
+        visit::visit_stmt(stmt, expr_cx, visitor);
+      }
+    }
+}
+
 fn resolve_expr(expr: @ast::expr, cx: ctxt, visitor: visit::vt<ctxt>) {
     record_parent(cx, expr.id);
 
@@ -270,7 +290,7 @@ fn resolve_local(local: @ast::local, cx: ctxt, visitor: visit::vt<ctxt>) {
 
 fn resolve_item(item: @ast::item, cx: ctxt, visitor: visit::vt<ctxt>) {
     // Items create a new outer block scope as far as we're concerned.
-    let new_cx: ctxt = {parent: none with cx};
+    let new_cx: ctxt = ctxt {parent: none with cx};
     visit::visit_item(item, new_cx, visitor);
 }
 
@@ -282,7 +302,7 @@ fn resolve_fn(fk: visit::fn_kind, decl: ast::fn_decl, body: ast::blk,
       visit::fk_item_fn(*) | visit::fk_method(*) |
       visit::fk_ctor(*) | visit::fk_dtor(*) => {
         // Top-level functions are a root scope.
-        {parent: some(id) with cx}
+        ctxt {parent: some(id) with cx}
       }
 
       visit::fk_anon(*) | visit::fk_fn_block(*) => {
@@ -304,17 +324,18 @@ fn resolve_fn(fk: visit::fn_kind, decl: ast::fn_decl, body: ast::blk,
 
 fn resolve_crate(sess: session, def_map: resolve3::DefMap,
                  crate: @ast::crate) -> region_map {
-    let cx: ctxt = {sess: sess,
-                    def_map: def_map,
-                    region_map: int_hash(),
-                    root_exprs: int_hash(),
-                    parent: none};
+    let cx: ctxt = ctxt {sess: sess,
+                         def_map: def_map,
+                         region_map: int_hash(),
+                         root_exprs: int_hash(),
+                         parent: none};
     let visitor = visit::mk_vt(@{
         visit_block: resolve_block,
         visit_item: resolve_item,
         visit_fn: resolve_fn,
         visit_arm: resolve_arm,
         visit_pat: resolve_pat,
+        visit_stmt: resolve_stmt,
         visit_expr: resolve_expr,
         visit_local: resolve_local
         with *visit::default_visitor()
diff --git a/src/rustc/middle/trans/base.rs b/src/rustc/middle/trans/base.rs
index 6e1f2e79805..d266a5b59d1 100644
--- a/src/rustc/middle/trans/base.rs
+++ b/src/rustc/middle/trans/base.rs
@@ -2171,6 +2171,9 @@ fn monomorphic_fn(ccx: @crate_ctxt, fn_id: ast::def_id,
       ast_map::node_expr(*) => {
         ccx.tcx.sess.bug(~"Can't monomorphize an expr")
       }
+      ast_map::node_stmt(*) => {
+        ccx.tcx.sess.bug(~"Can't monomorphize a stmt")
+      }
       ast_map::node_export(*) => {
           ccx.tcx.sess.bug(~"Can't monomorphize an export")
       }
@@ -2270,21 +2273,14 @@ fn monomorphic_fn(ccx: @crate_ctxt, fn_id: ast::def_id,
           dtor.node.id, psubsts, some(hash_id), parent_id)
       }
       // Ugh -- but this ensures any new variants won't be forgotten
-      ast_map::node_expr(*) => {
-        ccx.tcx.sess.bug(~"Can't monomorphize an expr")
-      }
-      ast_map::node_trait_method(*) => {
-        ccx.tcx.sess.bug(~"Can't monomorphize a trait method")
-      }
-      ast_map::node_export(*) => {
-          ccx.tcx.sess.bug(~"Can't monomorphize an export")
-      }
-      ast_map::node_arg(*) => ccx.tcx.sess.bug(~"Can't monomorphize an arg"),
-      ast_map::node_block(*) => {
-          ccx.tcx.sess.bug(~"Can't monomorphize a block")
-      }
+      ast_map::node_expr(*) |
+      ast_map::node_stmt(*) |
+      ast_map::node_trait_method(*) |
+      ast_map::node_export(*) |
+      ast_map::node_arg(*) |
+      ast_map::node_block(*) |
       ast_map::node_local(*) => {
-          ccx.tcx.sess.bug(~"Can't monomorphize a local")
+        ccx.tcx.sess.bug(fmt!("Can't monomorphize a %?", map_node))
       }
     };
     ccx.monomorphizing.insert(fn_id, depth);
diff --git a/src/rustc/middle/ty.rs b/src/rustc/middle/ty.rs
index 8936e203e0d..e01fdeee60a 100644
--- a/src/rustc/middle/ty.rs
+++ b/src/rustc/middle/ty.rs
@@ -2861,10 +2861,9 @@ fn item_path(cx: ctxt, id: ast::def_id) -> ast_map::path {
             vec::append_one(*path, ast_map::path_name(@~"dtor"))
           }
 
-
-          ast_map::node_expr(_) | ast_map::node_arg(_, _) |
-          ast_map::node_local(_) | ast_map::node_export(_, _) |
-          ast_map::node_block(_) => {
+          ast_map::node_stmt(*) | ast_map::node_expr(*) |
+          ast_map::node_arg(*) | ast_map::node_local(*) |
+          ast_map::node_export(*) | ast_map::node_block(*) => {
             cx.sess.bug(fmt!{"cannot find item_path for node %?", node});
           }
         }
diff --git a/src/test/compile-fail/borrowck-loan-rcvr-overloaded-op.rs b/src/test/compile-fail/borrowck-loan-rcvr-overloaded-op.rs
index 00ce2485b68..ecaad792fb9 100644
--- a/src/test/compile-fail/borrowck-loan-rcvr-overloaded-op.rs
+++ b/src/test/compile-fail/borrowck-loan-rcvr-overloaded-op.rs
@@ -28,11 +28,13 @@ fn b() {
 
     // Here I create an outstanding loan and check that we get conflicts:
 
-    &mut p; //~ NOTE prior loan as mutable granted here
+    let q = &mut p; //~ NOTE prior loan as mutable granted here
     //~^ NOTE prior loan as mutable granted here
 
     p + 3; //~ ERROR loan of mutable local variable as immutable conflicts with prior loan
     p.times(3); //~ ERROR loan of mutable local variable as immutable conflicts with prior loan
+
+    q.x += 1;
 }
 
 fn c() {
diff --git a/src/test/compile-fail/borrowck-loan-rcvr.rs b/src/test/compile-fail/borrowck-loan-rcvr.rs
index 6972905a8cc..39550e6ecd4 100644
--- a/src/test/compile-fail/borrowck-loan-rcvr.rs
+++ b/src/test/compile-fail/borrowck-loan-rcvr.rs
@@ -35,11 +35,13 @@ fn b() {
 
     // Here I create an outstanding loan and check that we get conflicts:
 
-    &mut p; //~ NOTE prior loan as mutable granted here
+    let l = &mut p; //~ NOTE prior loan as mutable granted here
     //~^ NOTE prior loan as mutable granted here
 
     p.purem(); //~ ERROR loan of mutable local variable as immutable conflicts with prior loan
     p.impurem(); //~ ERROR loan of mutable local variable as immutable conflicts with prior loan
+
+    l.x += 1;
 }
 
 fn c() {
diff --git a/src/test/compile-fail/vec-res-add.rs b/src/test/compile-fail/vec-res-add.rs
index 7fbcc67da17..4b558d7fe8f 100644
--- a/src/test/compile-fail/vec-res-add.rs
+++ b/src/test/compile-fail/vec-res-add.rs
@@ -1,3 +1,4 @@
+// xfail-test #2587
 // error-pattern: copying a noncopyable value
 
 struct r {