Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

rustc_metadata: Switch crate data iteration from a callback to iterator #92159

Merged
merged 1 commit into from
Dec 28, 2021
Merged

rustc_metadata: Switch crate data iteration from a callback to iterator #92159

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
161 changes: 67 additions & 94 deletions compiler/rustc_metadata/src/creader.rs
View file
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
Expand Down Expand Up @@ -440,10 +440,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