Make borrowck's notion of scopes consistent with trans's notion of scopes

This eliminates an ICE in trans where the scope for a particular
borrow was a statement ID, but the code in trans that does cleanups
wasn't finding the block with that scope. As per #3860

preserve looks at a node ID to see if it's for a statement -- if it
is, it uses the enclosing scope instead when updating the map that
trans looks at later.

I added a comment noting that this is not the best fix (since it may
cause boxes to be frozen for longer than necessary) and referring
to #3511.

r=nmatsakis

Author: catamorphism

Diff for: src/librustc/middle/borrowck/gather_loans.rs

@@ -84,6 +84,7 @@ fn gather_loans(bccx: borrowck_ctxt, crate: @ast::crate) -> req_maps {
                                   mut ignore_adjustments: LinearMap()});
     let v = visit::mk_vt(@visit::Visitor {visit_expr: req_loans_in_expr,
                                           visit_fn: req_loans_in_fn,
+                                          visit_stmt: add_stmt_to_map,
                                           .. *visit::default_visitor()});
     visit::visit_crate(*crate, glcx, v);
     return glcx.req_maps;
@@ -573,3 +574,16 @@ impl gather_loan_ctxt {
     }
 }
 
+// Setting up info that preserve needs.
+// This is just the most convenient place to do it.
+fn add_stmt_to_map(stmt: @ast::stmt,
+                   &&self: gather_loan_ctxt,
+                   vt: visit::vt<gather_loan_ctxt>) {
+    match stmt.node {
+        ast::stmt_expr(_, id) | ast::stmt_semi(_, id) => {
+            self.bccx.stmt_map.insert(id, ());
+        }
+        _ => ()
+    }
+    visit::visit_stmt(stmt, self, vt);
+}

Diff for: src/librustc/middle/borrowck/mod.rs

@@ -232,7 +232,7 @@ use middle::liveness;
 use middle::mem_categorization::*;
 use middle::region;
 use middle::ty;
-use util::common::indenter;
+use util::common::{indenter, stmt_set};
 use util::ppaux::{expr_repr, note_and_explain_region};
 use util::ppaux::{ty_to_str, region_to_str, explain_region};
 
@@ -272,6 +272,7 @@ fn check_crate(tcx: ty::ctxt,
                                 root_map: root_map(),
                                 mutbl_map: HashMap(),
                                 write_guard_map: HashMap(),
+                                stmt_map: HashMap(),
                                 mut loaned_paths_same: 0,
                                 mut loaned_paths_imm: 0,
                                 mut stable_paths: 0,
@@ -313,6 +314,7 @@ type borrowck_ctxt_ = {tcx: ty::ctxt,
                        root_map: root_map,
                        mutbl_map: mutbl_map,
                        write_guard_map: write_guard_map,
+                       stmt_map: stmt_set,
 
                        // Statistics:
                        mut loaned_paths_same: uint,

Diff for: src/librustc/middle/borrowck/preserve.rs

@@ -357,10 +357,30 @@ priv impl &preserve_ctxt {
             if self.bccx.is_subregion_of(self.scope_region, root_region) {
                 debug!("Elected to root");
                 let rk = {id: base.id, derefs: derefs};
+                // This code could potentially lead cause boxes to be frozen
+                // for longer than necessarily at runtime. It prevents an ICE
+                // in trans; the fundamental problem is that it's hard to make
+                // sure trans and borrowck have the same notion of scope. The
+                // real fix is to clean up how trans handles cleanups, but
+                // that's hard. If this becomes an issue, it's an option to just
+                // change this to `let scope_to_use = scope_id;`. Though that
+                // would potentially re-introduce the ICE. See #3511 for more
+                // details.
+                let scope_to_use = if
+                    self.bccx.stmt_map.contains_key(scope_id) {
+                    // Root it in its parent scope, b/c
+                    // trans won't introduce a new scope for the
+                    // stmt
+                    self.root_ub
+                }
+                else {
+                    // Use the more precise scope
+                    scope_id
+                };
                 // We freeze if and only if this is a *mutable* @ box that
                 // we're borrowing into a pointer.
                 self.bccx.root_map.insert(rk, RootInfo {
-                    scope: scope_id,
+                    scope: scope_to_use,
                     freezes: cmt.cat.derefs_through_mutable_box()
                 });
                 return Ok(pc_ok);

Diff for: src/librustc/middle/region.rs

@@ -25,6 +25,7 @@ use middle::resolve;
 use middle::ty::{region_variance, rv_covariant, rv_invariant};
 use middle::ty::{rv_contravariant};
 use middle::ty;
+use util::common::stmt_set;
 
 use core::cmp;
 use core::dvec::DVec;
@@ -254,6 +255,7 @@ fn resolve_stmt(stmt: @ast::stmt, cx: ctxt, visitor: visit::vt<ctxt>) {
       ast::stmt_decl(*) => {
         visit::visit_stmt(stmt, cx, visitor);
       }
+      // This code has to be kept consistent with trans::base::trans_stmt
       ast::stmt_expr(_, stmt_id) |
       ast::stmt_semi(_, stmt_id) => {
         record_parent(cx, stmt_id);

Diff for: src/librustc/util/common.rs

@@ -112,6 +112,9 @@ fn pluralize(n: uint, +s: ~str) -> ~str {
     else { str::concat([s, ~"s"]) }
 }
 
+// A set of node IDs (used to keep track of which node IDs are for statements)
+type stmt_set = HashMap<ast::node_id, ()>;
+
 //
 // Local Variables:
 // mode: rust

Diff for: src/test/run-pass/issue-3860.rs

@@ -8,7 +8,6 @@
 // option. This file may not be copied, modified, or distributed
 // except according to those terms.
 
-// xfail-test
 struct Foo { x: int }
 
 impl Foo {
@@ -19,11 +18,7 @@ impl Foo {
 
 fn main() {
     let mut x = @mut Foo { x: 3 };
-    x.stuff(); // error: internal compiler error: no enclosing scope with id 49
-    // storing the result removes the error, so replacing the above
-    // with the following, works:
-    // let _y = x.stuff()
-
-    // also making 'stuff()' not return anything fixes it
-    // I guess the "dangling &ptr" cuases issues?
-}
+    // Neither of the next two lines should cause an error
+    let _ = x.stuff(); 
+    x.stuff();
+}