From 046a68289939d40f3452055cd7fe326de6081187 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Tue, 21 Dec 2021 19:40:11 +0800 Subject: [PATCH] rustc_metadata: Switch crate data iteration from a callback to iterator The iteration looks more conventional this way, and some allocations are avoided. --- compiler/rustc_metadata/src/creader.rs | 161 ++++++++---------- .../src/rmeta/decoder/cstore_impl.rs | 8 +- 2 files changed, 70 insertions(+), 99 deletions(-) diff --git a/compiler/rustc_metadata/src/creader.rs b/compiler/rustc_metadata/src/creader.rs index c0da386edfdf1..36a1798cd6a86 100644 --- a/compiler/rustc_metadata/src/creader.rs +++ b/compiler/rustc_metadata/src/creader.rs @@ -102,30 +102,23 @@ struct CrateDump<'a>(&'a CStore); impl<'a> std::fmt::Debug for CrateDump<'a> { fn fmt(&self, fmt: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { writeln!(fmt, "resolved crates:")?; - // `iter_crate_data` does not allow returning values. Thus we use a mutable variable here - // that aggregates the value (and any errors that could happen). - let mut res = Ok(()); - self.0.iter_crate_data(|cnum, data| { - res = res.and( - try { - writeln!(fmt, " name: {}", data.name())?; - writeln!(fmt, " cnum: {}", cnum)?; - writeln!(fmt, " hash: {}", data.hash())?; - writeln!(fmt, " reqd: {:?}", data.dep_kind())?; - let CrateSource { dylib, rlib, rmeta } = data.source(); - if let Some(dylib) = dylib { - writeln!(fmt, " dylib: {}", dylib.0.display())?; - } - if let Some(rlib) = rlib { - writeln!(fmt, " rlib: {}", rlib.0.display())?; - } - if let Some(rmeta) = rmeta { - writeln!(fmt, " rmeta: {}", rmeta.0.display())?; - } - }, - ); - }); - res + for (cnum, data) in self.0.iter_crate_data() { + writeln!(fmt, " name: {}", data.name())?; + writeln!(fmt, " cnum: {}", cnum)?; + writeln!(fmt, " hash: {}", data.hash())?; + writeln!(fmt, " reqd: {:?}", data.dep_kind())?; + let CrateSource { dylib, rlib, rmeta } = data.source(); + if let Some(dylib) = dylib { + writeln!(fmt, " dylib: {}", dylib.0.display())?; + } + if let Some(rlib) = rlib { + writeln!(fmt, " rlib: {}", rlib.0.display())?; + } + if let Some(rmeta) = rmeta { + writeln!(fmt, " rmeta: {}", rmeta.0.display())?; + } + } + Ok(()) } } @@ -154,12 +147,10 @@ impl CStore { self.metas[cnum] = Some(Lrc::new(data)); } - crate fn iter_crate_data(&self, mut f: impl FnMut(CrateNum, &CrateMetadata)) { - for (cnum, data) in self.metas.iter_enumerated() { - if let Some(data) = data { - f(cnum, data); - } - } + crate fn iter_crate_data(&self) -> impl Iterator { + self.metas + .iter_enumerated() + .filter_map(|(cnum, data)| data.as_ref().map(|data| (cnum, &**data))) } fn push_dependencies_in_postorder(&self, deps: &mut Vec, cnum: CrateNum) { @@ -178,7 +169,9 @@ impl CStore { crate fn crate_dependencies_in_postorder(&self, cnum: CrateNum) -> Vec { let mut deps = Vec::new(); if cnum == LOCAL_CRATE { - self.iter_crate_data(|cnum, _| self.push_dependencies_in_postorder(&mut deps, cnum)); + for (cnum, _) in self.iter_crate_data() { + self.push_dependencies_in_postorder(&mut deps, cnum); + } } else { self.push_dependencies_in_postorder(&mut deps, cnum); } @@ -263,21 +256,17 @@ impl<'a> CrateLoader<'a> { } fn existing_match(&self, name: Symbol, hash: Option, kind: PathKind) -> Option { - let mut ret = None; - self.cstore.iter_crate_data(|cnum, data| { + for (cnum, data) in self.cstore.iter_crate_data() { if data.name() != name { tracing::trace!("{} did not match {}", data.name(), name); - return; + continue; } match hash { - Some(hash) if hash == data.hash() => { - ret = Some(cnum); - return; - } + Some(hash) if hash == data.hash() => return Some(cnum), Some(hash) => { debug!("actual hash {} did not match expected {}", hash, data.hash()); - return; + continue; } None => {} } @@ -301,10 +290,10 @@ impl<'a> CrateLoader<'a> { || source.rlib.as_ref().map(|(p, _)| p) == Some(l) || source.rmeta.as_ref().map(|(p, _)| p) == Some(l) }) { - ret = Some(cnum); + return Some(cnum); } } - return; + continue; } // Alright, so we've gotten this far which means that `data` has the @@ -321,15 +310,16 @@ impl<'a> CrateLoader<'a> { .expect("No sources for crate") .1; if kind.matches(prev_kind) { - ret = Some(cnum); + return Some(cnum); } else { debug!( "failed to load existing crate {}; kind {:?} did not match prev_kind {:?}", name, kind, prev_kind ); } - }); - ret + } + + None } fn verify_no_symbol_conflicts(&self, root: &CrateRoot<'_>) -> Result<(), CrateError> { @@ -339,17 +329,14 @@ impl<'a> CrateLoader<'a> { } // Check for conflicts with any crate loaded so far - let mut res = Ok(()); - self.cstore.iter_crate_data(|_, other| { - if other.stable_crate_id() == root.stable_crate_id() && // same stable crate id - other.hash() != root.hash() - { - // but different SVH - res = Err(CrateError::SymbolConflictsOthers(root.name())); + for (_, other) in self.cstore.iter_crate_data() { + // Same stable crate id but different SVH + if other.stable_crate_id() == root.stable_crate_id() && other.hash() != root.hash() { + return Err(CrateError::SymbolConflictsOthers(root.name())); } - }); + } - res + Ok(()) } fn verify_no_stable_crate_id_hash_conflicts( @@ -607,13 +594,14 @@ impl<'a> CrateLoader<'a> { locator.triple == self.sess.opts.target_triple || locator.is_proc_macro; Ok(Some(if can_reuse_cratenum { let mut result = LoadResult::Loaded(library); - self.cstore.iter_crate_data(|cnum, data| { + for (cnum, data) in self.cstore.iter_crate_data() { if data.name() == root.name() && root.hash() == data.hash() { assert!(locator.hash.is_none()); info!("load success, going to previous cnum: {}", cnum); result = LoadResult::Previous(cnum); + break; } - }); + } result } else { LoadResult::Loaded(library) @@ -711,7 +699,7 @@ impl<'a> CrateLoader<'a> { let mut needs_panic_runtime = self.sess.contains_name(&krate.attrs, sym::needs_panic_runtime); - self.cstore.iter_crate_data(|cnum, data| { + for (cnum, data) in self.cstore.iter_crate_data() { needs_panic_runtime = needs_panic_runtime || data.needs_panic_runtime(); if data.is_panic_runtime() { // Inject a dependency from all #![needs_panic_runtime] to this @@ -721,7 +709,7 @@ impl<'a> CrateLoader<'a> { }); runtime_found = runtime_found || data.dep_kind() == CrateDepKind::Explicit; } - }); + } // If an explicitly linked and matching panic runtime was found, or if // we just don't need one at all, then we're done here and there's @@ -813,11 +801,9 @@ impl<'a> CrateLoader<'a> { // Check to see if we actually need an allocator. This desire comes // about through the `#![needs_allocator]` attribute and is typically // written down in liballoc. - let mut needs_allocator = self.sess.contains_name(&krate.attrs, sym::needs_allocator); - self.cstore.iter_crate_data(|_, data| { - needs_allocator = needs_allocator || data.needs_allocator(); - }); - if !needs_allocator { + if !self.sess.contains_name(&krate.attrs, sym::needs_allocator) + && !self.cstore.iter_crate_data().any(|(_, data)| data.needs_allocator()) + { return; } @@ -838,23 +824,19 @@ impl<'a> CrateLoader<'a> { // global allocator. let mut global_allocator = self.cstore.has_global_allocator.then(|| Symbol::intern("this crate")); - self.cstore.iter_crate_data(|_, data| { - if !data.has_global_allocator() { - return; - } - match global_allocator { - Some(other_crate) => { - self.sess.err(&format!( - "the `#[global_allocator]` in {} \ - conflicts with global \ - allocator in: {}", + for (_, data) in self.cstore.iter_crate_data() { + if data.has_global_allocator() { + match global_allocator { + Some(other_crate) => self.sess.err(&format!( + "the `#[global_allocator]` in {} conflicts with global allocator in: {}", other_crate, data.name() - )); + )), + None => global_allocator = Some(data.name()), } - None => global_allocator = Some(data.name()), } - }); + } + if global_allocator.is_some() { self.cstore.allocator_kind = Some(AllocatorKind::Global); return; @@ -864,19 +846,12 @@ impl<'a> CrateLoader<'a> { // allocator. At this point our allocator request is typically fulfilled // by the standard library, denoted by the `#![default_lib_allocator]` // attribute. - let mut has_default = self.sess.contains_name(&krate.attrs, sym::default_lib_allocator); - self.cstore.iter_crate_data(|_, data| { - if data.has_default_lib_allocator() { - has_default = true; - } - }); - - if !has_default { + if !self.sess.contains_name(&krate.attrs, sym::default_lib_allocator) + && !self.cstore.iter_crate_data().any(|(_, data)| data.has_default_lib_allocator()) + { self.sess.err( - "no global memory allocator found but one is \ - required; link to std or \ - add `#[global_allocator]` to a static item \ - that implements the GlobalAlloc trait", + "no global memory allocator found but one is required; link to std or add \ + `#[global_allocator]` to a static item that implements the GlobalAlloc trait", ); } self.cstore.allocator_kind = Some(AllocatorKind::Default); @@ -916,14 +891,12 @@ impl<'a> CrateLoader<'a> { // crate provided for this compile, but in order for this compilation to // be successfully linked we need to inject a dependency (to order the // crates on the command line correctly). - self.cstore.iter_crate_data(|cnum, data| { - if !needs_dep(data) { - return; + for (cnum, data) in self.cstore.iter_crate_data() { + if needs_dep(data) { + info!("injecting a dep from {} to {}", cnum, krate); + data.add_dependency(krate); } - - info!("injecting a dep from {} to {}", cnum, krate); - data.add_dependency(krate); - }); + } } fn report_unused_deps(&mut self, krate: &ast::Crate) { diff --git a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs index 5ba7efc37f8bd..8e7a525edb824 100644 --- a/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs +++ b/compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs @@ -357,7 +357,7 @@ pub fn provide(providers: &mut Providers) { tcx.arena .alloc_slice(&CStore::from_tcx(tcx).crate_dependencies_in_postorder(LOCAL_CRATE)) }, - crates: |tcx, ()| tcx.arena.alloc_slice(&CStore::from_tcx(tcx).crates_untracked()), + crates: |tcx, ()| tcx.arena.alloc_from_iter(CStore::from_tcx(tcx).crates_untracked()), ..*providers }; @@ -440,10 +440,8 @@ impl CStore { self.get_crate_data(def.krate).def_kind(def.index) } - pub fn crates_untracked(&self) -> Vec { - let mut result = vec![]; - self.iter_crate_data(|cnum, _| result.push(cnum)); - result + pub fn crates_untracked(&self) -> impl Iterator + '_ { + self.iter_crate_data().map(|(cnum, _)| cnum) } pub fn item_generics_num_lifetimes(&self, def_id: DefId, sess: &Session) -> usize {