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

Author: nikomatsakis

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

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) => {

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);
     }

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()

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);

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});
           }
         }

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() {

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() {

src/test/compile-fail/vec-res-add.rs

@@ -1,3 +1,4 @@
+// xfail-test #2587
 // error-pattern: copying a noncopyable value
 
 struct r {