auto merge of #13686 : alexcrichton/rust/issue-12224, r=nikomatsakis

This alters the borrow checker's requirements on invoking closures from
requiring an immutable borrow to requiring a unique immutable borrow. This means 
that it is illegal to invoke a closure through a `&` pointer because there is no 
guarantee that is not aliased. This does not mean that a closure is required to
be in a mutable location, but rather a location which can be proven to be
unique (often through a mutable pointer).
                                                                                 
For example, the following code is unsound and is no longer allowed:             
                                                                                 
    type Fn<'a> = ||:'a;                                                         
                                                                                 
    fn call(f: |Fn|) {                                                           
        f(|| {                                                                   
            f(|| {})                                                             
        });                                                                      
    }                                                                            
                                                                                 
    fn main() {                                                                  
        call(|a| {                                                               
            a();                                                                 
        });                                                                      
    }                                                                            
                                                                                 
There is no replacement for this pattern. For all closures which are stored in
structures, it was previously allowed to invoke the closure through `&self` but
it now requires invocation through `&mut self`.

The standard library has a good number of violations of this new rule, but the
fixes will be separated into multiple breaking change commits.
                                                                                 
Closes #12224

Author: bors

src/libcollections/bitv.rs

@@ -632,7 +632,7 @@ impl<'a> RandomAccessIterator<bool> for Bits<'a> {
     }
 
     #[inline]
-    fn idx(&self, index: uint) -> Option<bool> {
+    fn idx(&mut self, index: uint) -> Option<bool> {
         if index >= self.indexable() {
             None
         } else {

src/libcollections/ringbuf.rs

@@ -272,7 +272,7 @@ impl<'a, T> RandomAccessIterator<&'a T> for Items<'a, T> {
     fn indexable(&self) -> uint { self.rindex - self.index }
 
     #[inline]
-    fn idx(&self, j: uint) -> Option<&'a T> {
+    fn idx(&mut self, j: uint) -> Option<&'a T> {
         if j >= self.indexable() {
             None
         } else {

src/libnum/bigint.rs

@@ -672,7 +672,7 @@ impl ToStrRadix for BigUint {
                 s.push_str("0".repeat(l - ss.len()));
                 s.push_str(ss);
             }
-            s.as_slice().trim_left_chars(&'0').to_owned()
+            s.as_slice().trim_left_chars('0').to_owned()
         }
     }
 }

src/librustc/front/config.rs

@@ -47,7 +47,7 @@ pub fn strip_items(krate: ast::Crate,
     ctxt.fold_crate(krate)
 }
 
-fn filter_view_item<'r>(cx: &Context, view_item: &'r ast::ViewItem)
+fn filter_view_item<'r>(cx: &mut Context, view_item: &'r ast::ViewItem)
                         -> Option<&'r ast::ViewItem> {
     if view_item_in_cfg(cx, view_item) {
         Some(view_item)
@@ -72,7 +72,7 @@ fn fold_mod(cx: &mut Context, m: &ast::Mod) -> ast::Mod {
     }
 }
 
-fn filter_foreign_item(cx: &Context, item: @ast::ForeignItem)
+fn filter_foreign_item(cx: &mut Context, item: @ast::ForeignItem)
                        -> Option<@ast::ForeignItem> {
     if foreign_item_in_cfg(cx, item) {
         Some(item)
@@ -144,7 +144,7 @@ fn fold_item_underscore(cx: &mut Context, item: &ast::Item_) -> ast::Item_ {
     fold::noop_fold_item_underscore(&item, cx)
 }
 
-fn fold_struct(cx: &Context, def: &ast::StructDef) -> @ast::StructDef {
+fn fold_struct(cx: &mut Context, def: &ast::StructDef) -> @ast::StructDef {
     let mut fields = def.fields.iter().map(|c| c.clone()).filter(|m| {
         (cx.in_cfg)(m.node.attrs.as_slice())
     });
@@ -156,7 +156,7 @@ fn fold_struct(cx: &Context, def: &ast::StructDef) -> @ast::StructDef {
     }
 }
 
-fn retain_stmt(cx: &Context, stmt: @ast::Stmt) -> bool {
+fn retain_stmt(cx: &mut Context, stmt: @ast::Stmt) -> bool {
     match stmt.node {
       ast::StmtDecl(decl, _) => {
         match decl.node {
@@ -189,23 +189,23 @@ fn fold_block(cx: &mut Context, b: ast::P<ast::Block>) -> ast::P<ast::Block> {
     })
 }
 
-fn item_in_cfg(cx: &Context, item: &ast::Item) -> bool {
+fn item_in_cfg(cx: &mut Context, item: &ast::Item) -> bool {
     return (cx.in_cfg)(item.attrs.as_slice());
 }
 
-fn foreign_item_in_cfg(cx: &Context, item: &ast::ForeignItem) -> bool {
+fn foreign_item_in_cfg(cx: &mut Context, item: &ast::ForeignItem) -> bool {
     return (cx.in_cfg)(item.attrs.as_slice());
 }
 
-fn view_item_in_cfg(cx: &Context, item: &ast::ViewItem) -> bool {
+fn view_item_in_cfg(cx: &mut Context, item: &ast::ViewItem) -> bool {
     return (cx.in_cfg)(item.attrs.as_slice());
 }
 
-fn method_in_cfg(cx: &Context, meth: &ast::Method) -> bool {
+fn method_in_cfg(cx: &mut Context, meth: &ast::Method) -> bool {
     return (cx.in_cfg)(meth.attrs.as_slice());
 }
 
-fn trait_method_in_cfg(cx: &Context, meth: &ast::TraitMethod) -> bool {
+fn trait_method_in_cfg(cx: &mut Context, meth: &ast::TraitMethod) -> bool {
     match *meth {
         ast::Required(ref meth) => (cx.in_cfg)(meth.attrs.as_slice()),
         ast::Provided(meth) => (cx.in_cfg)(meth.attrs.as_slice())

src/librustc/metadata/encoder.rs

@@ -84,7 +84,7 @@ pub struct EncodeContext<'a> {
     pub non_inlineable_statics: &'a RefCell<NodeSet>,
     pub link_meta: &'a LinkMeta,
     pub cstore: &'a cstore::CStore,
-    pub encode_inlined_item: EncodeInlinedItem<'a>,
+    pub encode_inlined_item: RefCell<EncodeInlinedItem<'a>>,
     pub type_abbrevs: tyencode::abbrev_map,
 }
 
@@ -765,8 +765,8 @@ fn encode_info_for_method(ecx: &EncodeContext,
         if num_params > 0u ||
                 is_default_impl ||
                 should_inline(ast_method.attrs.as_slice()) {
-            (ecx.encode_inlined_item)(
-                ecx, ebml_w, IIMethodRef(local_def(parent_id), false, ast_method));
+            encode_inlined_item(ecx, ebml_w,
+                                IIMethodRef(local_def(parent_id), false, ast_method));
         } else {
             encode_symbol(ecx, ebml_w, m.def_id.node);
         }
@@ -775,6 +775,14 @@ fn encode_info_for_method(ecx: &EncodeContext,
     ebml_w.end_tag();
 }
 
+fn encode_inlined_item(ecx: &EncodeContext,
+                       ebml_w: &mut Encoder,
+                       ii: InlinedItemRef) {
+    let mut eii = ecx.encode_inlined_item.borrow_mut();
+    let eii: &mut EncodeInlinedItem = &mut *eii;
+    (*eii)(ecx, ebml_w, ii)
+}
+
 fn style_fn_family(s: FnStyle) -> char {
     match s {
         UnsafeFn => 'u',
@@ -880,7 +888,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
         let inlineable = !ecx.non_inlineable_statics.borrow().contains(&item.id);
 
         if inlineable {
-            (ecx.encode_inlined_item)(ecx, ebml_w, IIItemRef(item));
+            encode_inlined_item(ecx, ebml_w, IIItemRef(item));
         }
         encode_visibility(ebml_w, vis);
         ebml_w.end_tag();
@@ -896,7 +904,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
         encode_path(ebml_w, path);
         encode_attributes(ebml_w, item.attrs.as_slice());
         if tps_len > 0u || should_inline(item.attrs.as_slice()) {
-            (ecx.encode_inlined_item)(ecx, ebml_w, IIItemRef(item));
+            encode_inlined_item(ecx, ebml_w, IIItemRef(item));
         } else {
             encode_symbol(ecx, ebml_w, item.id);
         }
@@ -954,7 +962,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
         for v in (*enum_definition).variants.iter() {
             encode_variant_id(ebml_w, local_def(v.node.id));
         }
-        (ecx.encode_inlined_item)(ecx, ebml_w, IIItemRef(item));
+        encode_inlined_item(ecx, ebml_w, IIItemRef(item));
         encode_path(ebml_w, path);
 
         // Encode inherent implementations for this enumeration.
@@ -1002,7 +1010,7 @@ fn encode_info_for_item(ecx: &EncodeContext,
         needs to know*/
         encode_struct_fields(ebml_w, fields.as_slice(), def_id);
 
-        (ecx.encode_inlined_item)(ecx, ebml_w, IIItemRef(item));
+        encode_inlined_item(ecx, ebml_w, IIItemRef(item));
 
         // Encode inherent implementations for this structure.
         encode_inherent_implementations(ecx, ebml_w, def_id);
@@ -1175,8 +1183,8 @@ fn encode_info_for_item(ecx: &EncodeContext,
                         encode_bounds_and_type(ebml_w, ecx, &tpt);
                     }
                     encode_method_sort(ebml_w, 'p');
-                    (ecx.encode_inlined_item)(
-                        ecx, ebml_w, IIMethodRef(def_id, true, m));
+                    encode_inlined_item(ecx, ebml_w,
+                                        IIMethodRef(def_id, true, m));
                 }
             }
 
@@ -1212,7 +1220,7 @@ fn encode_info_for_foreign_item(ecx: &EncodeContext,
                                &lookup_item_type(ecx.tcx,local_def(nitem.id)));
         encode_name(ebml_w, nitem.ident.name);
         if abi == abi::RustIntrinsic {
-            (ecx.encode_inlined_item)(ecx, ebml_w, IIForeignRef(nitem));
+            encode_inlined_item(ecx, ebml_w, IIForeignRef(nitem));
         } else {
             encode_symbol(ecx, ebml_w, nitem.id);
         }
@@ -1544,12 +1552,12 @@ fn encode_macro_registrar_fn(ecx: &EncodeContext, ebml_w: &mut Encoder) {
     }
 }
 
-struct MacroDefVisitor<'a, 'b> {
-    ecx: &'a EncodeContext<'a>,
-    ebml_w: &'a mut Encoder<'b>
+struct MacroDefVisitor<'a, 'b, 'c> {
+    ecx: &'a EncodeContext<'b>,
+    ebml_w: &'a mut Encoder<'c>
 }
 
-impl<'a, 'b> Visitor<()> for MacroDefVisitor<'a, 'b> {
+impl<'a, 'b, 'c> Visitor<()> for MacroDefVisitor<'a, 'b, 'c> {
     fn visit_item(&mut self, item: &Item, _: ()) {
         match item.node {
             ItemMac(..) => {
@@ -1565,9 +1573,9 @@ impl<'a, 'b> Visitor<()> for MacroDefVisitor<'a, 'b> {
     }
 }
 
-fn encode_macro_defs(ecx: &EncodeContext,
-                     krate: &Crate,
-                     ebml_w: &mut Encoder) {
+fn encode_macro_defs<'a>(ecx: &'a EncodeContext,
+                         krate: &Crate,
+                         ebml_w: &'a mut Encoder) {
     ebml_w.start_tag(tag_exported_macros);
     {
         let mut visitor = MacroDefVisitor {
@@ -1579,12 +1587,12 @@ fn encode_macro_defs(ecx: &EncodeContext,
     ebml_w.end_tag();
 }
 
-struct ImplVisitor<'a,'b> {
-    ecx: &'a EncodeContext<'a>,
-    ebml_w: &'a mut Encoder<'b>,
+struct ImplVisitor<'a,'b,'c> {
+    ecx: &'a EncodeContext<'b>,
+    ebml_w: &'a mut Encoder<'c>,
 }
 
-impl<'a,'b> Visitor<()> for ImplVisitor<'a,'b> {
+impl<'a,'b,'c> Visitor<()> for ImplVisitor<'a,'b,'c> {
     fn visit_item(&mut self, item: &Item, _: ()) {
         match item.node {
             ItemImpl(_, Some(ref trait_ref), _, _) => {
@@ -1617,9 +1625,9 @@ impl<'a,'b> Visitor<()> for ImplVisitor<'a,'b> {
 /// * Destructors (implementations of the Drop trait).
 ///
 /// * Implementations of traits not defined in this crate.
-fn encode_impls(ecx: &EncodeContext,
-                krate: &Crate,
-                ebml_w: &mut Encoder) {
+fn encode_impls<'a>(ecx: &'a EncodeContext,
+                    krate: &Crate,
+                    ebml_w: &'a mut Encoder) {
     ebml_w.start_tag(tag_impls);
 
     {
@@ -1744,7 +1752,7 @@ fn encode_metadata_inner(wr: &mut MemWriter, parms: EncodeParams, krate: &Crate)
         non_inlineable_statics: non_inlineable_statics,
         link_meta: link_meta,
         cstore: cstore,
-        encode_inlined_item: encode_inlined_item,
+        encode_inlined_item: RefCell::new(encode_inlined_item),
         type_abbrevs: RefCell::new(HashMap::new()),
      };
 

src/librustc/metadata/filesearch.rs

@@ -92,9 +92,9 @@ impl<'a> FileSearch<'a> {
             match fs::readdir(lib_search_path) {
                 Ok(files) => {
                     let mut rslt = FileDoesntMatch;
-                    let is_rlib = |p: & &Path| {
+                    fn is_rlib(p: & &Path) -> bool {
                         p.extension_str() == Some("rlib")
-                    };
+                    }
                     // Reading metadata out of rlibs is faster, and if we find both
                     // an rlib and a dylib we only read one of the files of
                     // metadata, so in the name of speed, bring all rlib files to

src/librustc/middle/borrowck/check_loans.rs

@@ -327,7 +327,7 @@ impl<'a> CheckLoanCtxt<'a> {
                             self.bccx.loan_path_to_str(&*old_loan.loan_path))
                 }
 
-                AddrOf | AutoRef | RefBinding => {
+                AddrOf | AutoRef | RefBinding | ClosureInvocation => {
                     format!("previous borrow of `{}` occurs here",
                             self.bccx.loan_path_to_str(&*old_loan.loan_path))
                 }

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

@@ -292,6 +292,26 @@ fn gather_loans_in_expr(this: &mut GatherLoanCtxt,
           visit::walk_expr(this, ex, ());
       }
 
+      ast::ExprCall(f, _) => {
+          let expr_ty = ty::expr_ty_adjusted(tcx, f);
+          match ty::get(expr_ty).sty {
+              ty::ty_closure(~ty::ClosureTy {
+                  store: ty::RegionTraitStore(..), ..
+              }) => {
+                  let scope_r = ty::ReScope(ex.id);
+                  let base_cmt = this.bccx.cat_expr(f);
+                  this.guarantee_valid_kind(f.id,
+                                            f.span,
+                                            base_cmt,
+                                            ty::UniqueImmBorrow,
+                                            scope_r,
+                                            ClosureInvocation);
+              }
+              _ => {}
+          }
+          visit::walk_expr(this, ex, ());
+      }
+
       _ => {
           visit::walk_expr(this, ex, ());
       }

src/librustc/middle/borrowck/mod.rs

@@ -202,6 +202,7 @@ pub enum LoanCause {
     AddrOf,
     AutoRef,
     RefBinding,
+    ClosureInvocation,
 }
 
 #[deriving(Eq, TotalEq, Hash)]
@@ -629,6 +630,10 @@ impl<'a> BorrowckCtxt<'a> {
                     AddrOf | RefBinding | AutoRef => {
                         format!("cannot borrow {} as mutable", descr)
                     }
+                    ClosureInvocation => {
+                        self.tcx.sess.span_bug(err.span,
+                            "err_mutbl with a closure invocation");
+                    }
                 }
             }
             err_out_of_root_scope(..) => {
@@ -677,6 +682,10 @@ impl<'a> BorrowckCtxt<'a> {
             BorrowViolation(RefBinding) => {
                 "cannot borrow data mutably"
             }
+
+            BorrowViolation(ClosureInvocation) => {
+                "closure invocation"
+            }
         };
 
         match cause {

src/librustc/middle/lint.rs

@@ -1164,7 +1164,7 @@ fn check_item_non_camel_case_types(cx: &Context, it: &ast::Item) {
     fn is_camel_case(ident: ast::Ident) -> bool {
         let ident = token::get_ident(ident);
         assert!(!ident.get().is_empty());
-        let ident = ident.get().trim_chars(&'_');
+        let ident = ident.get().trim_chars('_');
 
         // start with a non-lowercase letter rather than non-uppercase
         // ones (some scripts don't have a concept of upper/lowercase)

src/librustc/middle/privacy.rs

@@ -1104,34 +1104,34 @@ impl<'a> SanePrivacyVisitor<'a> {
     /// control over anything so this forbids any mention of any visibility
     fn check_all_inherited(&self, item: &ast::Item) {
         let tcx = self.tcx;
-        let check_inherited = |sp: Span, vis: ast::Visibility| {
+        fn check_inherited(tcx: &ty::ctxt, sp: Span, vis: ast::Visibility) {
             if vis != ast::Inherited {
                 tcx.sess.span_err(sp, "visibility has no effect inside functions");
             }
-        };
+        }
         let check_struct = |def: &@ast::StructDef| {
             for f in def.fields.iter() {
                match f.node.kind {
-                    ast::NamedField(_, p) => check_inherited(f.span, p),
+                    ast::NamedField(_, p) => check_inherited(tcx, f.span, p),
                     ast::UnnamedField(..) => {}
                 }
             }
         };
-        check_inherited(item.span, item.vis);
+        check_inherited(tcx, item.span, item.vis);
         match item.node {
             ast::ItemImpl(_, _, _, ref methods) => {
                 for m in methods.iter() {
-                    check_inherited(m.span, m.vis);
+                    check_inherited(tcx, m.span, m.vis);
                 }
             }
             ast::ItemForeignMod(ref fm) => {
                 for i in fm.items.iter() {
-                    check_inherited(i.span, i.vis);
+                    check_inherited(tcx, i.span, i.vis);
                 }
             }
             ast::ItemEnum(ref def, _) => {
                 for v in def.variants.iter() {
-                    check_inherited(v.span, v.node.vis);
+                    check_inherited(tcx, v.span, v.node.vis);
 
                     match v.node.kind {
                         ast::StructVariantKind(ref s) => check_struct(s),
@@ -1146,7 +1146,8 @@ impl<'a> SanePrivacyVisitor<'a> {
                 for m in methods.iter() {
                     match *m {
                         ast::Required(..) => {}
-                        ast::Provided(ref m) => check_inherited(m.span, m.vis),
+                        ast::Provided(ref m) => check_inherited(tcx, m.span,
+                                                                m.vis),
                     }
                 }
             }