Merge pull request #103 from Xanewok/further-tweaks

Further simplify and explain crate id mapping

Author: nrc

src/analysis.rs

@@ -187,7 +187,7 @@ impl Analysis {
                             if def.is_none() {
                                 info!("def not found for {}", id);
                             }
-                            def.map(|def| f(*id, &def))
+                            def.map(|def| f(*id, def))
                         })
                         .collect(),
                 );
@@ -215,7 +215,7 @@ impl Analysis {
         self.for_all_crates(|c| {
             c.def_names.get(name).map(|ids| {
                 ids.into_iter()
-                    .flat_map(|id| c.defs.get(id).map(|def| def.clone()))
+                    .flat_map(|id| c.defs.get(id).cloned())
                     .collect()
             })
         })

src/lib.rs

@@ -62,7 +62,7 @@ pub struct SymbolResult {
 impl SymbolResult {
     fn new(id: Id, def: &Def) -> SymbolResult {
         SymbolResult {
-            id: id,
+            id,
             name: def.name.clone(),
             span: def.span.clone(),
             kind: def.kind,
@@ -97,18 +97,18 @@ impl AnalysisHost<CargoAnalysisLoader> {
             master_crate_map: Mutex::new(HashMap::new()),
             loader: CargoAnalysisLoader {
                 path_prefix: Mutex::new(None),
-                target: target,
+                target,
             },
         }
     }
 }
 
 impl<L: AnalysisLoader> AnalysisHost<L> {
-    pub fn new_with_loader(l: L) -> AnalysisHost<L> {
-        AnalysisHost {
+    pub fn new_with_loader(loader: L) -> Self {
+        Self {
             analysis: Mutex::new(None),
             master_crate_map: Mutex::new(HashMap::new()),
-            loader: l,
+            loader,
         }
     }
 
@@ -167,12 +167,11 @@ impl<L: AnalysisLoader> AnalysisHost<L> {
 
         let raw_analysis = read_analysis_incremental(&self.loader, timestamps, blacklist);
 
-        let result = lowering::lower(raw_analysis, base_dir, self, |host, per_crate, path| {
+        lowering::lower(raw_analysis, base_dir, self, |host, per_crate, path| {
             let mut a = host.analysis.lock()?;
             a.as_mut().unwrap().update(per_crate, path);
             Ok(())
-        });
-        result
+        })
     }
 
     // Reloads the entire project's analysis data.
@@ -226,8 +225,8 @@ impl<L: AnalysisLoader> AnalysisHost<L> {
         Ok(())
     }
 
-    /// Note that self.has_def == true =/> self.goto_def.is_some(), since if the
-    /// def is in an api crate, there is no reasonable span to jump to.
+    /// Note that `self.has_def()` =/> `self.goto_def().is_ok()`, since if the
+    /// Def is in an api crate, there is no reasonable Span to jump to.
     pub fn has_def(&self, id: Id) -> bool {
         match self.analysis.lock() {
             Ok(a) => a.as_ref().unwrap().has_def(id),
@@ -416,7 +415,7 @@ impl<L: AnalysisLoader> AnalysisHost<L> {
 
     pub fn find_impls(&self, id: Id) -> AResult<Vec<Span>> {
         self.with_analysis(|a| {
-            Some(a.for_all_crates(|c| c.impls.get(&id).map(|v| v.clone())))
+            Some(a.for_all_crates(|c| c.impls.get(&id).cloned()))
         })
     }
 
@@ -490,7 +489,7 @@ impl<L: AnalysisLoader> AnalysisHost<L> {
                 analysis.with_defs(p, |parent| match def.kind {
                     DefKind::Field | DefKind::Method | DefKind::Tuple => {
                         let ns = name_space_for_def_kind(def.kind);
-                        let mut res = AnalysisHost::<L>::mk_doc_url(&parent, analysis)
+                        let mut res = AnalysisHost::<L>::mk_doc_url(parent, analysis)
                             .unwrap_or_else(|| "".into());
                         res.push_str(&format!("#{}.{}", def.name, ns));
                         res

src/loader.rs

@@ -98,7 +98,7 @@ fn extract_target_triple(sys_root_path: &Path) -> String {
         .expect("extracting toolchain failed");
     // Extracts x86_64-pc-windows-msvc from nightly-x86_64-pc-windows-pc
     let triple = toolchain
-        .splitn(2, "-")
+        .splitn(2, '-')
         .last()
         .map(String::from)
         .expect("extracting triple failed");

src/lowering.rs

@@ -85,6 +85,8 @@ fn lower_span(raw_span: &raw::SpanData, base_dir: &Path) -> Span {
 }
 
 struct CrateReader {
+    /// This is effectively a map from local crate id -> global crate id, where
+    /// local crate id are indices 0...external_crate_count.
     crate_map: Vec<u32>,
     base_dir: PathBuf,
     crate_name: String,
@@ -96,23 +98,27 @@ impl CrateReader {
         master_crate_map: &mut HashMap<String, u32>,
         base_dir: &Path,
     ) -> CrateReader {
-        // println!("building crate map for {}", crate_name);
-        let next = master_crate_map.len() as u32;
-        let mut crate_map = vec![
-            *master_crate_map
-                .entry(prelude.crate_name.clone())
-                .or_insert_with(|| next),
-        ];
-        // println!("  {} -> {}", crate_name, master_crate_map[&crate_name]);
+        fn fetch_crate_id(map: &mut HashMap<String, u32>, crate_name: &String) -> u32 {
+            let next = map.len() as u32;
+            *map.entry(crate_name.clone()).or_insert(next)
+        }
+        // When reading a local crate and its external crates, we need to:
+        // 1. Update a global crate id map if we encounter any new crate
+        // 2. Prepare a local crate id -> global crate id map, so we can easily
+        // map those when lowering symbols with local crate ids into global registry
+        // It's worth noting, that we assume that local crate id is 0, whereas
+        // the external crates will have num in 1..count contiguous range.
+        trace!("building crate map for {}", prelude.crate_name);
+        let id = fetch_crate_id(master_crate_map, &prelude.crate_name);
+        let mut crate_map = vec![id];
+        trace!("  {} -> {}", prelude.crate_name, master_crate_map[&prelude.crate_name]);
 
         prelude.external_crates.sort_by(|a, b| a.num.cmp(&b.num));
         for c in prelude.external_crates {
             assert!(c.num == crate_map.len() as u32);
-            let next = master_crate_map.len() as u32;
-            crate_map.push(*master_crate_map
-                .entry(c.name.clone())
-                .or_insert_with(|| next));
-            // println!("  {} -> {}", c.name, master_crate_map[&c.name]);
+            let id = fetch_crate_id(master_crate_map, &c.name);
+            crate_map.push(id);
+            trace!("  {} -> {}", c.name, master_crate_map[&c.name]);
         }
 
         CrateReader {
@@ -221,7 +227,7 @@ impl CrateReader {
                     docs: d.docs,
                     // sig: d.sig.map(|ref s| self.lower_sig(s, &self.base_dir)),
                 };
-                info!(
+                trace!(
                     "record def: {:?}/{:?} ({}): {:?}",
                     id,
                     d.id,
@@ -334,15 +340,15 @@ impl CrateReader {
     //     }
     // }
 
+    /// Recreates resulting crate-local (u32, u32) id from compiler to a global
+    /// `u64` `Id`, mapping from a local to global crate id.
     fn id_from_compiler_id(&self, id: &data::Id) -> Id {
         if id.krate == u32::MAX || id.index == u32::MAX {
             return NULL;
         }
-        // We build an id by looking up the local crate number into a global crate number and using
-        // that for the high order bits, then use the index for the least significant bits.
 
         let krate = self.crate_map[id.krate as usize] as u64;
-
+        // Use global crate number for high order bits, then index for least significant bits.
         Id((krate << 32) | (id.index as u64))
     }
 }