Skip to content

Commit

Permalink
Auto merge of #92159 - petrochenkov:decoditer, r=cjgillot
Browse files Browse the repository at this point in the history 
rustc_metadata: Switch crate data iteration from a callback to iterator

The iteration looks more conventional this way, and some allocations are avoided.
  • Loading branch information
@bors
bors committed Dec 28, 2021
2 parents cc65bf3 + 046a682 commit e91ad5f
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 99 deletions.
161 changes: 67 additions & 94 deletions compiler/rustc_metadata/src/creader.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
}

Expand Down Expand Up @@ -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<Item = (CrateNum, &CrateMetadata)> {
self.metas
.iter_enumerated()
.filter_map(|(cnum, data)| data.as_ref().map(|data| (cnum, &**data)))
}

fn push_dependencies_in_postorder(&self, deps: &mut Vec<CrateNum>, cnum: CrateNum) {
Expand All @@ -178,7 +169,9 @@ impl CStore {
crate fn crate_dependencies_in_postorder(&self, cnum: CrateNum) -> Vec<CrateNum> {
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);
}
Expand Down Expand Up @@ -263,21 +256,17 @@ impl<'a> CrateLoader<'a> {
}

fn existing_match(&self, name: Symbol, hash: Option<Svh>, kind: PathKind) -> Option<CrateNum> {
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 => {}
}
Expand All @@ -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
Expand All @@ -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> {
Expand All @@ -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(
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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;
}

Expand All @@ -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;
Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down
8 changes: 3 additions & 5 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,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
};
Expand Down Expand Up @@ -449,10 +449,8 @@ impl CStore {
self.get_crate_data(def.krate).def_kind(def.index)
}

pub fn crates_untracked(&self) -> Vec<CrateNum> {
let mut result = vec![];
self.iter_crate_data(|cnum, _| result.push(cnum));
result
pub fn crates_untracked(&self) -> impl Iterator<Item = CrateNum> + '_ {
self.iter_crate_data().map(|(cnum, _)| cnum)
}

pub fn item_generics_num_lifetimes(&self, def_id: DefId, sess: &Session) -> usize {
Expand Down

0 comments on commit e91ad5f

Please sign in to comment.