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.

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

rustc_metadata: Switch module children decoding to an iterator #104730

Merged
merged 3 commits into from
Nov 26, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
99 changes: 38 additions & 61 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,16 @@ use rustc_session::cstore::{
CrateSource, ExternCrate, ForeignModule, LinkagePreference, NativeLib,
};
use rustc_session::Session;
use rustc_span::hygiene::{ExpnIndex, MacroKind};
use rustc_span::hygiene::ExpnIndex;
use rustc_span::source_map::{respan, Spanned};
use rustc_span::symbol::{kw, Ident, Symbol};
use rustc_span::{self, BytePos, ExpnId, Pos, Span, SyntaxContext, DUMMY_SP};

use proc_macro::bridge::client::ProcMacro;
use std::io;
use std::iter::TrustedLen;
use std::mem;
use std::num::NonZeroUsize;
use std::path::Path;
use std::{io, iter, mem};

pub(super) use cstore_impl::provide;
pub use cstore_impl::provide_extern;
Expand Down Expand Up @@ -990,64 +989,52 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
DiagnosticItems { id_to_name, name_to_id }
}

fn get_mod_child(self, id: DefIndex, sess: &Session) -> ModChild {
let ident = self.item_ident(id, sess);
let kind = self.def_kind(id);
let def_id = self.local_def_id(id);
let res = Res::Def(kind, def_id);
let vis = self.get_visibility(id);
let span = self.get_span(id, sess);
let macro_rules = match kind {
DefKind::Macro(..) => self.root.tables.macro_rules.get(self, id).is_some(),
_ => false,
};

ModChild { ident, res, vis, span, macro_rules }
}

/// Iterates over all named children of the given module,
/// including both proper items and reexports.
/// Module here is understood in name resolution sense - it can be a `mod` item,
/// or a crate root, or an enum, or a trait.
fn for_each_module_child(
fn get_module_children(
self,
id: DefIndex,
mut callback: impl FnMut(ModChild),
sess: &Session,
) {
if let Some(data) = &self.root.proc_macro_data {
// If we are loading as a proc macro, we want to return
// the view of this crate as a proc macro crate.
if id == CRATE_DEF_INDEX {
for def_index in data.macros.decode(self) {
let raw_macro = self.raw_proc_macro(def_index);
let res = Res::Def(
DefKind::Macro(macro_kind(raw_macro)),
self.local_def_id(def_index),
);
let ident = self.item_ident(def_index, sess);
callback(ModChild {
ident,
res,
vis: ty::Visibility::Public,
span: ident.span,
macro_rules: false,
});
sess: &'a Session,
) -> impl Iterator<Item = ModChild> + 'a {
iter::from_generator(move || {
if let Some(data) = &self.root.proc_macro_data {
// If we are loading as a proc macro, we want to return
// the view of this crate as a proc macro crate.
if id == CRATE_DEF_INDEX {
for child_index in data.macros.decode(self) {
yield self.get_mod_child(child_index, sess);
}
}
} else {
// Iterate over all children.
for child_index in self.root.tables.children.get(self, id).unwrap().decode(self) {
yield self.get_mod_child(child_index, sess);
}
}
return;
}

// Iterate over all children.
if let Some(children) = self.root.tables.children.get(self, id) {
for child_index in children.decode((self, sess)) {
let ident = self.item_ident(child_index, sess);
let kind = self.def_kind(child_index);
let def_id = self.local_def_id(child_index);
let res = Res::Def(kind, def_id);
let vis = self.get_visibility(child_index);
let span = self.get_span(child_index, sess);
let macro_rules = match kind {
DefKind::Macro(..) => {
self.root.tables.macro_rules.get(self, child_index).is_some()
if let Some(reexports) = self.root.tables.module_reexports.get(self, id) {
for reexport in reexports.decode((self, sess)) {
yield reexport;
}
_ => false,
};

callback(ModChild { ident, res, vis, span, macro_rules });
}
}

if let Some(exports) = self.root.tables.module_reexports.get(self, id) {
for exp in exports.decode((self, sess)) {
callback(exp);
}
}
}
})
}

fn is_ctfe_mir_available(self, id: DefIndex) -> bool {
Expand Down Expand Up @@ -1784,13 +1771,3 @@ impl CrateMetadata {
None
}
}

// Cannot be implemented on 'ProcMacro', as libproc_macro
// does not depend on librustc_ast
fn macro_kind(raw: &ProcMacro) -> MacroKind {
match raw {
ProcMacro::CustomDerive { .. } => MacroKind::Derive,
ProcMacro::Attr { .. } => MacroKind::Attr,
ProcMacro::Bang { .. } => MacroKind::Bang,
}
}
19 changes: 7 additions & 12 deletions compiler/rustc_metadata/src/rmeta/decoder/cstore_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ use rustc_span::source_map::{Span, Spanned};
use rustc_span::symbol::{kw, Symbol};

use rustc_data_structures::sync::Lrc;
use smallvec::SmallVec;
use std::any::Any;

use super::{Decodable, DecodeContext, DecodeIterator};
Expand Down Expand Up @@ -298,9 +297,7 @@ provide! { tcx, def_id, other, cdata,
r
}
module_children => {
let mut result = SmallVec::<[_; 8]>::new();
cdata.for_each_module_child(def_id.index, |child| result.push(child), tcx.sess);
tcx.arena.alloc_slice(&result)
tcx.arena.alloc_from_iter(cdata.get_module_children(def_id.index, tcx.sess))
}
defined_lib_features => { cdata.get_lib_features(tcx) }
stability_implications => {
Expand Down Expand Up @@ -503,14 +500,12 @@ impl CStore {
self.get_crate_data(def.krate).get_visibility(def.index)
}

pub fn module_children_untracked(&self, def_id: DefId, sess: &Session) -> Vec<ModChild> {
let mut result = vec![];
self.get_crate_data(def_id.krate).for_each_module_child(
def_id.index,
|child| result.push(child),
sess,
);
result
pub fn module_children_untracked<'a>(
&'a self,
def_id: DefId,
sess: &'a Session,
) -> impl Iterator<Item = ModChild> + 'a {
self.get_crate_data(def_id.krate).get_module_children(def_id.index, sess)
}

pub fn load_macro_untracked(&self, id: DefId, sess: &Session) -> LoadedMacro {
Expand Down
12 changes: 5 additions & 7 deletions compiler/rustc_metadata/src/rmeta/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1267,13 +1267,6 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
// the crate root for consistency with other crates (some of the resolver
// code uses it). However, we skip encoding anything relating to child
// items - we encode information about proc-macros later on.
let reexports = if !self.is_proc_macro {
tcx.module_reexports(local_def_id).unwrap_or(&[])
} else {
&[]
};

record_array!(self.tables.module_reexports[def_id] <- reexports);
if self.is_proc_macro {
// Encode this here because we don't do it in encode_def_ids.
record!(self.tables.expn_that_defined[def_id] <- tcx.expn_that_defined(local_def_id));
Expand Down Expand Up @@ -1305,6 +1298,11 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> {
}
}
}));

if let Some(reexports) = tcx.module_reexports(local_def_id) {
assert!(!reexports.is_empty());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
assert!(!reexports.is_empty());
debug_assert!(!reexports.is_empty());

?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assert is very cheap (both in absolute value, and relatively to everything else happening here), so I think we can always enable it.

record_array!(self.tables.module_reexports[def_id] <- reexports);
}
}
}

Expand Down
4 changes: 3 additions & 1 deletion compiler/rustc_resolve/src/build_reduced_graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,7 +204,9 @@ impl<'a> Resolver<'a> {
}

pub(crate) fn build_reduced_graph_external(&mut self, module: Module<'a>) {
for child in self.cstore().module_children_untracked(module.def_id(), self.session) {
for child in
Vec::from_iter(self.cstore().module_children_untracked(module.def_id(), self.session))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need the from_iter first?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Due to borrow checker, self is mutably borrowed inside the loop.

{
let parent_scope = ParentScope::module(module, self);
BuildReducedGraphVisitor { r: self, parent_scope }
.build_reduced_graph_for_external_crate_res(child);
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_resolve/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1920,7 +1920,7 @@ impl<'a> Resolver<'a> {
if let Some(def_id) = def_id.as_local() {
self.reexport_map.get(&def_id).cloned().unwrap_or_default()
} else {
self.cstore().module_children_untracked(def_id, self.session)
self.cstore().module_children_untracked(def_id, self.session).collect()
}
}

Expand Down