librustc: Require lifetimes within the types of values with destructors

to have strictly greater lifetime than the values themselves.

Additionally, remove `#[unsafe_destructor]` in favor of these new
restrictions.

This broke a fair amount of code that had to do with `RefCell`s. We will
probably need to do some work to improve the resulting ergonomics. Most
code that broke looked like:

    match foo.bar.borrow().baz { ... }
    use_foo(&mut foo);

Change this code to:

    {
        let bar = foo.bar.borrow();
        match bar.baz { ... }
    }
    use_foo(&mut foo);

This fixes an important memory safety hole relating to destructors,
represented by issue rust-lang#8861.

[breaking-change]

Author: pcwalton

src/jemalloc

@@ -1084,27 +1084,29 @@ impl<T: Ord> Set<T> for TreeSet<T> {
     }
 
     fn is_subset(&self, other: &TreeSet<T>) -> bool {
-        let mut x = self.iter();
-        let mut y = other.iter();
-        let mut a = x.next();
-        let mut b = y.next();
-        while a.is_some() {
-            if b.is_none() {
-                return false;
-            }
+        {
+            let mut x = self.iter();
+            let mut y = other.iter();
+            let mut a = x.next();
+            let mut b = y.next();
+            while a.is_some() {
+                if b.is_none() {
+                    return false;
+                }
 
-            let a1 = a.unwrap();
-            let b1 = b.unwrap();
+                let a1 = a.unwrap();
+                let b1 = b.unwrap();
 
-            match b1.cmp(a1) {
-                Less => (),
-                Greater => return false,
-                Equal => a = x.next(),
-            }
+                match b1.cmp(a1) {
+                    Less => (),
+                    Greater => return false,
+                    Equal => a = x.next(),
+                }
 
-            b = y.next();
+                b = y.next();
+            }
+            true
         }
-        true
     }
 }
 

src/libcollections/treemap.rs

@@ -310,8 +310,11 @@ impl<T> RefCell<T> {
 
 #[unstable = "waiting for `Clone` to become stable"]
 impl<T: Clone> Clone for RefCell<T> {
-    fn clone(&self) -> RefCell<T> {
-        RefCell::new(self.borrow().clone())
+    fn clone<'a>(&'a self) -> RefCell<T> {
+        {
+            let borrowed: Ref<'a,T> = self.borrow();
+            RefCell::new(borrowed.clone())
+        }
     }
 }
 

src/libcore/cell.rs

@@ -90,16 +90,22 @@ impl<T> Finally<T> for fn() -> T {
  *     })
  * ```
  */
-pub fn try_finally<T,U,R>(mutate: &mut T,
-                          drop: U,
-                          try_fn: |&mut T, U| -> R,
-                          finally_fn: |&mut T|)
-                          -> R {
-    let f = Finallyalizer {
-        mutate: mutate,
-        dtor: finally_fn,
-    };
-    try_fn(&mut *f.mutate, drop)
+pub fn try_finally<'a,
+                   T,
+                   U,
+                   R>(
+                   mutate: &'a mut T,
+                   drop: U,
+                   try_fn: |&mut T, U| -> R,
+                   finally_fn: |&mut T|:'a)
+                   -> R {
+    {
+        let f: Finallyalizer<'a,T> = Finallyalizer {
+            mutate: mutate,
+            dtor: finally_fn,
+        };
+        try_fn(&mut *f.mutate, drop)
+    }
 }
 
 struct Finallyalizer<'a,A:'a> {

src/libcore/finally.rs

@@ -61,10 +61,13 @@ impl BasicLoop {
 
     fn remote_work(&mut self) {
         let messages = unsafe {
-            mem::replace(&mut *self.messages.lock(), Vec::new())
+            let lock = &mut *self.messages.lock();
+            mem::replace(lock, Vec::new())
         };
-        for message in messages.move_iter() {
-            self.message(message);
+        {
+            for message in messages.move_iter() {
+                self.message(message);
+            }
         }
     }
 

src/libgreen/basic.rs

@@ -258,7 +258,10 @@ pub fn build_session_(sopts: config::Options,
         recursion_limit: Cell::new(64),
     };
 
-    sess.lint_store.borrow_mut().register_builtin(Some(&sess));
+    {
+        sess.lint_store.borrow_mut().register_builtin(Some(&sess));
+    }
+
     sess
 }
 

src/librustc/driver/session.rs

@@ -244,9 +244,8 @@ impl<'a, 'v> Visitor<'v> for Context<'a> {
                                        "unsafe_destructor") {
                     self.gate_feature("unsafe_destructor",
                                       i.span,
-                                      "`#[unsafe_destructor]` allows too \
-                                       many unsafe patterns and may be \
-                                       removed in the future");
+                                      "`#[unsafe_destructor]` does nothing \
+                                       anymore")
                 }
             }
 

src/librustc/front/feature_gate.rs

@@ -318,6 +318,9 @@ fn parse_region(st: &mut PState, conv: conv_did) -> ty::Region {
       't' => {
         ty::ReStatic
       }
+      'n' => {
+        ty::ReFunction
+      }
       'e' => {
         ty::ReStatic
       }

src/librustc/metadata/tydecode.rs

@@ -149,6 +149,9 @@ pub fn enc_region(w: &mut SeekableMemWriter, cx: &ctxt, r: ty::Region) {
         ty::ReScope(nid) => {
             mywrite!(w, "s{}|", nid);
         }
+        ty::ReFunction => {
+            mywrite!(w, "n");
+        }
         ty::ReStatic => {
             mywrite!(w, "t");
         }

src/librustc/metadata/tyencode.rs

@@ -490,7 +490,7 @@ impl tr for ty::Region {
             ty::ReScope(id) => {
                 ty::ReScope(dcx.tr_id(id))
             }
-            ty::ReEmpty | ty::ReStatic | ty::ReInfer(..) => {
+            ty::ReEmpty | ty::ReStatic | ty::ReInfer(..) | ty::ReFunction => {
                 *self
             }
             ty::ReFree(ref fr) => {

src/librustc/middle/astencode.rs

@@ -163,36 +163,7 @@ impl<'a, 'tcx> GuaranteeLifetimeContext<'a, 'tcx> {
 
         // See the SCOPE(LV) function in doc.rs
 
-        match cmt.cat {
-            mc::cat_rvalue(temp_scope) => {
-                temp_scope
-            }
-            mc::cat_upvar(..) |
-            mc::cat_copied_upvar(_) => {
-                ty::ReScope(self.item_scope_id)
-            }
-            mc::cat_static_item => {
-                ty::ReStatic
-            }
-            mc::cat_local(local_id) |
-            mc::cat_arg(local_id) => {
-                ty::ReScope(self.bccx.tcx.region_maps.var_scope(local_id))
-            }
-            mc::cat_deref(_, _, mc::UnsafePtr(..)) => {
-                ty::ReStatic
-            }
-            mc::cat_deref(_, _, mc::BorrowedPtr(_, r)) |
-            mc::cat_deref(_, _, mc::Implicit(_, r)) => {
-                r
-            }
-            mc::cat_downcast(ref cmt) |
-            mc::cat_deref(ref cmt, _, mc::OwnedPtr) |
-            mc::cat_deref(ref cmt, _, mc::GcPtr) |
-            mc::cat_interior(ref cmt, _) |
-            mc::cat_discr(ref cmt, _) => {
-                self.scope(cmt)
-            }
-        }
+        mc::scope(self.bccx.tcx, cmt, self.item_scope_id)
     }
 
     fn report_error(&self, code: bckerr_code) {

src/librustc/middle/borrowck/gather_loans/lifetime.rs

@@ -275,6 +275,12 @@ impl<'a, 'tcx> GatherLoanCtxt<'a, 'tcx> {
                     ty::ReScope(id) => id,
                     ty::ReFree(ref fr) => fr.scope_id,
 
+                    ty::ReFunction => {
+                        // For purposes of the borrow check, we can consider
+                        // this a loan for the outer function block.
+                        self.item_ub
+                    }
+
                     ty::ReStatic => {
                         // If we get here, an error must have been
                         // reported in

src/librustc/middle/borrowck/gather_loans/mod.rs

@@ -511,37 +511,41 @@ impl<'d,'t,'tcx,TYPER:mc::Typer<'tcx>> ExprUseVisitor<'d,'t,TYPER> {
                 }
             }
             _ => {
-                let overloaded_call_type =
-                    match self.tcx()
-                              .method_map
-                              .borrow()
-                              .find(&MethodCall::expr(call.id)) {
-                    Some(ref method_callee) => {
-                        OverloadedCallType::from_method_origin(
-                            self.tcx(),
-                            &method_callee.origin)
-                    }
-                    None => {
-                        self.tcx().sess.span_bug(
-                            callee.span,
-                            format!("unexpected callee type {}",
-                                    callee_ty.repr(self.tcx())).as_slice())
-                    }
-                };
-                match overloaded_call_type {
-                    FnMutOverloadedCall => {
-                        self.borrow_expr(callee,
-                                         ty::ReScope(call.id),
-                                         ty::MutBorrow,
-                                         ClosureInvocation);
+                let overloaded_call_type;
+                {
+                    let method_map = self.tcx().method_map.borrow();
+                    overloaded_call_type = match method_map.find(
+                            &MethodCall::expr(call.id)) {
+                        Some(ref method_callee) => {
+                            OverloadedCallType::from_method_origin(
+                                self.tcx(),
+                                &method_callee.origin)
+                        }
+                        None => {
+                            self.tcx().sess.span_bug(
+                                callee.span,
+                                format!("unexpected callee type {}",
+                                        callee_ty.repr(self.tcx()))
+                                                 .as_slice())
+                        }
                     }
-                    FnOverloadedCall => {
-                        self.borrow_expr(callee,
-                                         ty::ReScope(call.id),
-                                         ty::ImmBorrow,
-                                         ClosureInvocation);
+                }
+                {
+                    match overloaded_call_type {
+                        FnMutOverloadedCall => {
+                            self.borrow_expr(callee,
+                                             ty::ReScope(call.id),
+                                             ty::MutBorrow,
+                                             ClosureInvocation);
+                        }
+                        FnOverloadedCall => {
+                            self.borrow_expr(callee,
+                                             ty::ReScope(call.id),
+                                             ty::ImmBorrow,
+                                             ClosureInvocation);
+                        }
+                        FnOnceOverloadedCall => self.consume_expr(callee),
                     }
-                    FnOnceOverloadedCall => self.consume_expr(callee),
                 }
             }
         }
@@ -933,8 +937,10 @@ impl<'d,'t,'tcx,TYPER:mc::Typer<'tcx>> ExprUseVisitor<'d,'t,TYPER> {
             // inferred by regionbk
             let upvar_id = ty::UpvarId { var_id: id_var,
                                          closure_expr_id: closure_expr.id };
-            let upvar_borrow = self.tcx().upvar_borrow_map.borrow()
-                                   .get_copy(&upvar_id);
+            let upvar_borrow = {
+                let upvar_borrow_map = self.tcx().upvar_borrow_map.borrow();
+                upvar_borrow_map.get_copy(&upvar_id)
+            };
 
             self.delegate.borrow(closure_expr.id,
                                  closure_expr.span,

src/librustc/middle/expr_use_visitor.rs

@@ -67,76 +67,7 @@ pub fn check_crate(tcx: &ty::ctxt) {
     tcx.sess.abort_if_errors();
 }
 
-struct EmptySubstsFolder<'a, 'tcx: 'a> {
-    tcx: &'a ty::ctxt<'tcx>
-}
-impl<'a, 'tcx> ty_fold::TypeFolder<'tcx> for EmptySubstsFolder<'a, 'tcx> {
-    fn tcx<'a>(&'a self) -> &'a ty::ctxt<'tcx> {
-        self.tcx
-    }
-    fn fold_substs(&mut self, _: &subst::Substs) -> subst::Substs {
-        subst::Substs::empty()
-    }
-}
-
-fn check_struct_safe_for_destructor(cx: &mut Context,
-                                    span: Span,
-                                    struct_did: DefId) {
-    let struct_tpt = ty::lookup_item_type(cx.tcx, struct_did);
-    if !struct_tpt.generics.has_type_params(subst::TypeSpace)
-      && !struct_tpt.generics.has_region_params(subst::TypeSpace) {
-        let mut folder = EmptySubstsFolder { tcx: cx.tcx };
-        if !ty::type_is_sendable(cx.tcx, struct_tpt.ty.fold_with(&mut folder)) {
-            span_err!(cx.tcx.sess, span, E0125,
-                      "cannot implement a destructor on a \
-                       structure or enumeration that does not satisfy Send");
-            span_note!(cx.tcx.sess, span,
-                       "use \"#[unsafe_destructor]\" on the implementation \
-                        to force the compiler to allow this");
-        }
-    } else {
-        span_err!(cx.tcx.sess, span, E0141,
-                  "cannot implement a destructor on a structure \
-                   with type parameters");
-        span_note!(cx.tcx.sess, span,
-                   "use \"#[unsafe_destructor]\" on the implementation \
-                    to force the compiler to allow this");
-    }
-}
-
-fn check_impl_of_trait(cx: &mut Context, it: &Item, trait_ref: &TraitRef, self_type: &Ty) {
-    let ast_trait_def = *cx.tcx.def_map.borrow()
-                              .find(&trait_ref.ref_id)
-                              .expect("trait ref not in def map!");
-    let trait_def_id = ast_trait_def.def_id();
-
-    // If this is a destructor, check kinds.
-    if cx.tcx.lang_items.drop_trait() == Some(trait_def_id) &&
-        !attr::contains_name(it.attrs.as_slice(), "unsafe_destructor")
-    {
-        match self_type.node {
-            TyPath(_, ref bounds, path_node_id) => {
-                assert!(bounds.is_none());
-                let struct_def = cx.tcx.def_map.borrow().get_copy(&path_node_id);
-                let struct_did = struct_def.def_id();
-                check_struct_safe_for_destructor(cx, self_type.span, struct_did);
-            }
-            _ => {
-                cx.tcx.sess.span_bug(self_type.span,
-                    "the self type for the Drop trait impl is not a path");
-            }
-        }
-    }
-}
-
 fn check_item(cx: &mut Context, item: &Item) {
-    match item.node {
-        ItemImpl(_, Some(ref trait_ref), ref self_type, _) => {
-            check_impl_of_trait(cx, item, trait_ref, &**self_type);
-        }
-        _ => {}
-    }
-
     visit::walk_item(cx, item)
 }