auto merge of #17378 : Gankro/rust/hashmap-entry, r=aturon

Deprecates the `find_or_*` family of "internal mutation" methods on `HashMap` in
favour of the "external mutation" Entry API as part of RFC 60. Part of #17320,
but this still needs to be done on the rest of the maps. However they don't have
any internal mutation methods defined, so they can be done without deprecating
or breaking anything. Work on `BTree` is part of the complete rewrite in #17334.

The implemented API deviates from the API described in the RFC in two key places:

* `VacantEntry.set` yields a mutable reference to the inserted element to avoid code
duplication where complex logic needs to be done *regardless* of whether the entry
was vacant or not.
* `OccupiedEntry.into_mut` was added so that it is possible to return a reference
into the map beyond the lifetime of the Entry itself, providing functional parity
to `VacantEntry.set`.

This allows the full find_or_insert functionality to be implemented using this API.
A PR will be submitted to the RFC to amend this.

[breaking-change]

Author: bors

src/librustc/driver/config.rs

@@ -31,6 +31,7 @@ use syntax::parse;
 use syntax::parse::token::InternedString;
 
 use std::collections::HashMap;
+use std::collections::hashmap::{Occupied, Vacant};
 use getopts::{optopt, optmulti, optflag, optflagopt};
 use getopts;
 use std::cell::{RefCell};
@@ -804,8 +805,11 @@ pub fn build_session_options(matches: &getopts::Matches) -> Options {
             Some(s) => s,
             None => early_error("--extern value must be of the format `foo=bar`"),
         };
-        let locs = externs.find_or_insert(name.to_string(), Vec::new());
-        locs.push(location.to_string());
+
+        match externs.entry(name.to_string()) {
+            Vacant(entry) => { entry.set(vec![location.to_string()]); },
+            Occupied(mut entry) => { entry.get_mut().push(location.to_string()); },
+        }
     }
 
     let crate_name = matches.opt_str("crate-name");

src/librustc/lint/builtin.rs

@@ -36,6 +36,7 @@ use lint::{Context, LintPass, LintArray};
 
 use std::cmp;
 use std::collections::HashMap;
+use std::collections::hashmap::{Occupied, Vacant};
 use std::slice;
 use std::{i8, i16, i32, i64, u8, u16, u32, u64, f32, f64};
 use syntax::abi;
@@ -1203,15 +1204,18 @@ impl UnusedMut {
     fn check_unused_mut_pat(&self, cx: &Context, pats: &[P<ast::Pat>]) {
         // collect all mutable pattern and group their NodeIDs by their Identifier to
         // avoid false warnings in match arms with multiple patterns
+
         let mut mutables = HashMap::new();
         for p in pats.iter() {
             pat_util::pat_bindings(&cx.tcx.def_map, &**p, |mode, id, _, path1| {
                 let ident = path1.node;
                 match mode {
                     ast::BindByValue(ast::MutMutable) => {
                         if !token::get_ident(ident).get().starts_with("_") {
-                            mutables.insert_or_update_with(ident.name.uint(),
-                                vec!(id), |_, old| { old.push(id); });
+                            match mutables.entry(ident.name.uint()) {
+                                Vacant(entry) => { entry.set(vec![id]); },
+                                Occupied(mut entry) => { entry.get_mut().push(id); },
+                            }
                         }
                     }
                     _ => {

src/librustc/metadata/creader.rs

@@ -24,6 +24,7 @@ use plugin::load::PluginMetadata;
 
 use std::rc::Rc;
 use std::collections::HashMap;
+use std::collections::hashmap::{Occupied, Vacant};
 use syntax::ast;
 use syntax::abi;
 use syntax::attr;
@@ -82,7 +83,10 @@ fn dump_crates(cstore: &CStore) {
 fn warn_if_multiple_versions(diag: &SpanHandler, cstore: &CStore) {
     let mut map = HashMap::new();
     cstore.iter_crate_data(|cnum, data| {
-        map.find_or_insert_with(data.name(), |_| Vec::new()).push(cnum);
+        match map.entry(data.name()) {
+            Vacant(entry) => { entry.set(vec![cnum]); },
+            Occupied(mut entry) => { entry.get_mut().push(cnum); },
+        }
     });
 
     for (name, dupes) in map.into_iter() {

src/librustc/metadata/loader.rs

@@ -237,6 +237,7 @@ use std::slice;
 use std::string;
 
 use std::collections::{HashMap, HashSet};
+use std::collections::hashmap::{Occupied, Vacant};
 use flate;
 use time;
 
@@ -428,15 +429,18 @@ impl<'a> Context<'a> {
                 return FileDoesntMatch
             };
             info!("lib candidate: {}", path.display());
-            let slot = candidates.find_or_insert_with(hash.to_string(), |_| {
-                (HashSet::new(), HashSet::new())
-            });
+
+            let slot = match candidates.entry(hash.to_string()) {
+                Occupied(entry) => entry.into_mut(),
+                Vacant(entry) => entry.set((HashSet::new(), HashSet::new())),
+            };
             let (ref mut rlibs, ref mut dylibs) = *slot;
             if rlib {
                 rlibs.insert(fs::realpath(path).unwrap());
             } else {
                 dylibs.insert(fs::realpath(path).unwrap());
             }
+
             FileMatches
         });
 

src/librustc/middle/const_eval.rs

@@ -28,6 +28,7 @@ use syntax::visit;
 use syntax::{ast, ast_map, ast_util};
 
 use std::rc::Rc;
+use std::collections::hashmap::Vacant;
 
 //
 // This pass classifies expressions by their constant-ness.
@@ -321,7 +322,10 @@ pub fn const_expr_to_pat(tcx: &ty::ctxt, expr: &Expr) -> P<Pat> {
 
         ExprCall(ref callee, ref args) => {
             let def = tcx.def_map.borrow().get_copy(&callee.id);
-            tcx.def_map.borrow_mut().find_or_insert(expr.id, def);
+            match tcx.def_map.borrow_mut().entry(expr.id) {
+              Vacant(entry) => { entry.set(def); }
+              _ => {}
+            };
             let path = match def {
                 def::DefStruct(def_id) => def_to_path(tcx, def_id),
                 def::DefVariant(_, variant_did, _) => def_to_path(tcx, variant_did),

src/librustc/middle/resolve.rs

@@ -59,6 +59,7 @@ use syntax::visit;
 use syntax::visit::Visitor;
 
 use std::collections::{HashMap, HashSet};
+use std::collections::hashmap::{Occupied, Vacant};
 use std::cell::{Cell, RefCell};
 use std::mem::replace;
 use std::rc::{Rc, Weak};
@@ -2815,10 +2816,13 @@ impl<'a> Resolver<'a> {
         let is_public = import_directive.is_public;
 
         let mut import_resolutions = module_.import_resolutions.borrow_mut();
-        let dest_import_resolution = import_resolutions.find_or_insert_with(name, |_| {
-            // Create a new import resolution from this child.
-            ImportResolution::new(id, is_public)
-        });
+        let dest_import_resolution = match import_resolutions.entry(name) {
+            Occupied(entry) => entry.into_mut(),
+            Vacant(entry) => {
+                // Create a new import resolution from this child.
+                entry.set(ImportResolution::new(id, is_public))
+            }
+        };
 
         debug!("(resolving glob import) writing resolution `{}` in `{}` \
                to `{}`",
@@ -6026,19 +6030,21 @@ impl<'a> Resolver<'a> {
         assert!(match lp {LastImport{..} => false, _ => true},
                 "Import should only be used for `use` directives");
         self.last_private.insert(node_id, lp);
-        self.def_map.borrow_mut().insert_or_update_with(node_id, def, |_, old_value| {
+
+        match self.def_map.borrow_mut().entry(node_id) {
             // Resolve appears to "resolve" the same ID multiple
             // times, so here is a sanity check it at least comes to
             // the same conclusion! - nmatsakis
-            if def != *old_value {
+            Occupied(entry) => if def != *entry.get() {
                 self.session
                     .bug(format!("node_id {:?} resolved first to {:?} and \
                                   then {:?}",
                                  node_id,
-                                 *old_value,
+                                 *entry.get(),
                                  def).as_slice());
-            }
-        });
+            },
+            Vacant(entry) => { entry.set(def); },
+        }
     }
 
     fn enforce_default_binding_mode(&mut self,

src/librustc/middle/ty.rs

@@ -48,6 +48,7 @@ use std::mem;
 use std::ops;
 use std::rc::Rc;
 use std::collections::{HashMap, HashSet};
+use std::collections::hashmap::{Occupied, Vacant};
 use arena::TypedArena;
 use syntax::abi;
 use syntax::ast::{CrateNum, DefId, FnStyle, Ident, ItemTrait, LOCAL_CRATE};
@@ -4566,9 +4567,10 @@ pub fn lookup_field_type(tcx: &ctxt,
         node_id_to_type(tcx, id.node)
     } else {
         let mut tcache = tcx.tcache.borrow_mut();
-        let pty = tcache.find_or_insert_with(id, |_| {
-            csearch::get_field_type(tcx, struct_id, id)
-        });
+        let pty = match tcache.entry(id) {
+            Occupied(entry) => entry.into_mut(),
+            Vacant(entry) => entry.set(csearch::get_field_type(tcx, struct_id, id)),
+        };
         pty.ty
     };
     t.subst(tcx, substs)

src/librustc/middle/typeck/check/mod.rs

@@ -121,6 +121,7 @@ use util::nodemap::{DefIdMap, FnvHashMap, NodeMap};
 
 use std::cell::{Cell, RefCell};
 use std::collections::HashMap;
+use std::collections::hashmap::{Occupied, Vacant};
 use std::mem::replace;
 use std::rc::Rc;
 use syntax::abi;
@@ -1991,11 +1992,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
          */
 
         let mut region_obligations = self.inh.region_obligations.borrow_mut();
-        let v = region_obligations.find_or_insert_with(self.body_id,
-                                                       |_| Vec::new());
-        v.push(RegionObligation { sub_region: r,
+        let region_obligation = RegionObligation { sub_region: r,
                                   sup_type: ty,
-                                  origin: origin });
+                                  origin: origin };
+
+        match region_obligations.entry(self.body_id) {
+            Vacant(entry) => { entry.set(vec![region_obligation]); },
+            Occupied(mut entry) => { entry.get_mut().push(region_obligation); },
+        }
     }
 
     pub fn add_obligations_for_parameters(&self,

src/librustc/middle/typeck/check/regionmanip.rs

@@ -18,6 +18,7 @@ use middle::ty_fold::TypeFolder;
 use syntax::ast;
 
 use std::collections::HashMap;
+use std::collections::hashmap::{Occupied, Vacant};
 use util::ppaux::Repr;
 
 // Helper functions related to manipulating region types.
@@ -35,7 +36,10 @@ pub fn replace_late_bound_regions_in_fn_sig(
             debug!("region r={}", r.to_string());
             match r {
                 ty::ReLateBound(s, br) if s == fn_sig.binder_id => {
-                    *map.find_or_insert_with(br, |_| mapf(br))
+                    * match map.entry(br) {
+                        Vacant(entry) => entry.set(mapf(br)),
+                        Occupied(entry) => entry.into_mut(),
+                    }
                 }
                 _ => r
             }

src/librustdoc/html/render.rs

@@ -34,6 +34,7 @@
 //! both occur before the crate is rendered.
 
 use std::collections::{HashMap, HashSet};
+use std::collections::hashmap::{Occupied, Vacant};
 use std::fmt;
 use std::io::fs::PathExtensions;
 use std::io::{fs, File, BufferedWriter, MemWriter, BufferedReader};
@@ -802,9 +803,10 @@ impl DocFolder for Cache {
             clean::ImplItem(ref i) => {
                 match i.trait_ {
                     Some(clean::ResolvedPath{ did, .. }) => {
-                        let v = self.implementors.find_or_insert_with(did, |_| {
-                            Vec::new()
-                        });
+                        let v = match self.implementors.entry(did) {
+                            Vacant(entry) => entry.set(Vec::with_capacity(1)),
+                            Occupied(entry) => entry.into_mut(),
+                        };
                         v.push(Implementor {
                             def_id: item.def_id,
                             generics: i.generics.clone(),
@@ -999,9 +1001,10 @@ impl DocFolder for Cache {
 
                         match did {
                             Some(did) => {
-                                let v = self.impls.find_or_insert_with(did, |_| {
-                                    Vec::new()
-                                });
+                                let v = match self.impls.entry(did) {
+                                    Vacant(entry) => entry.set(Vec::with_capacity(1)),
+                                    Occupied(entry) => entry.into_mut(),
+                                };
                                 v.push(Impl {
                                     impl_: i,
                                     dox: dox,
@@ -2143,7 +2146,10 @@ fn build_sidebar(m: &clean::Module) -> HashMap<String, Vec<String>> {
             None => continue,
             Some(ref s) => s.to_string(),
         };
-        let v = map.find_or_insert_with(short.to_string(), |_| Vec::new());
+        let v = match map.entry(short.to_string()) {
+            Vacant(entry) => entry.set(Vec::with_capacity(1)),
+            Occupied(entry) => entry.into_mut(),
+        };
         v.push(myname);
     }
 

src/librustdoc/lib.rs

@@ -31,6 +31,7 @@ extern crate time;
 use std::io;
 use std::io::{File, MemWriter};
 use std::collections::HashMap;
+use std::collections::hashmap::{Occupied, Vacant};
 use serialize::{json, Decodable, Encodable};
 use externalfiles::ExternalHtml;
 
@@ -340,7 +341,10 @@ fn parse_externs(matches: &getopts::Matches) -> Result<core::Externs, String> {
                 return Err("--extern value must be of the format `foo=bar`".to_string());
             }
         };
-        let locs = externs.find_or_insert(name.to_string(), Vec::new());
+        let locs = match externs.entry(name.to_string()) {
+            Vacant(entry) => entry.set(Vec::with_capacity(1)),
+            Occupied(entry) => entry.into_mut(),
+        };
         locs.push(location.to_string());
     }
     Ok(externs)