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 all decoder methods from vectors to iterators #92245

Merged
merged 1 commit into from
Jan 16, 2022
Merged

rustc_metadata: Switch all decoder methods from vectors to iterators #92245

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
100 changes: 33 additions & 67 deletions compiler/rustc_metadata/src/rmeta/decoder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1033,45 +1033,33 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {

/// Iterates over all the stability attributes in the given crate.
fn get_lib_features(self, tcx: TyCtxt<'tcx>) -> &'tcx [(Symbol, Option<Symbol>)] {
// FIXME: For a proc macro crate, not sure whether we should return the "host"
// features or an empty Vec. Both don't cause ICEs.
tcx.arena.alloc_from_iter(self.root.lib_features.decode(self))
}

/// Iterates over the language items in the given crate.
fn get_lang_items(self, tcx: TyCtxt<'tcx>) -> &'tcx [(DefId, usize)] {
if self.root.is_proc_macro_crate() {
// Proc macro crates do not export any lang-items to the target.
&[]
} else {
tcx.arena.alloc_from_iter(
self.root
.lang_items
.decode(self)
.map(|(def_index, index)| (self.local_def_id(def_index), index)),
)
}
tcx.arena.alloc_from_iter(
self.root
.lang_items
.decode(self)
.map(|(def_index, index)| (self.local_def_id(def_index), index)),
)
}

/// Iterates over the diagnostic items in the given crate.
fn get_diagnostic_items(self) -> DiagnosticItems {
if self.root.is_proc_macro_crate() {
// Proc macro crates do not export any diagnostic-items to the target.
Default::default()
} else {
let mut id_to_name = FxHashMap::default();
let name_to_id = self
.root
.diagnostic_items
.decode(self)
.map(|(name, def_index)| {
let id = self.local_def_id(def_index);
id_to_name.insert(id, name);
(name, id)
})
.collect();
DiagnosticItems { id_to_name, name_to_id }
}
let mut id_to_name = FxHashMap::default();
let name_to_id = self
.root
.diagnostic_items
.decode(self)
.map(|(name, def_index)| {
let id = self.local_def_id(def_index);
id_to_name.insert(id, name);
(name, id)
})
.collect();
DiagnosticItems { id_to_name, name_to_id }
}

/// Iterates over all named children of the given module,
Expand Down Expand Up @@ -1346,26 +1334,28 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
.decode((self, sess))
}

fn get_struct_field_names(self, id: DefIndex, sess: &Session) -> Vec<Spanned<Symbol>> {
fn get_struct_field_names(
self,
id: DefIndex,
sess: &'a Session,
) -> impl Iterator<Item = Spanned<Symbol>> + 'a {
self.root
.tables
.children
.get(self, id)
.unwrap_or_else(Lazy::empty)
.decode(self)
.map(|index| respan(self.get_span(index, sess), self.item_ident(index, sess).name))
.collect()
.map(move |index| respan(self.get_span(index, sess), self.item_ident(index, sess).name))
}

fn get_struct_field_visibilities(self, id: DefIndex) -> Vec<Visibility> {
fn get_struct_field_visibilities(self, id: DefIndex) -> impl Iterator<Item = Visibility> + 'a {
self.root
.tables
.children
.get(self, id)
.unwrap_or_else(Lazy::empty)
.decode(self)
.map(|field_index| self.get_visibility(field_index))
.collect()
.map(move |field_index| self.get_visibility(field_index))
}

fn get_inherent_implementations_for_type(
Expand Down Expand Up @@ -1401,8 +1391,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
tcx: TyCtxt<'tcx>,
trait_def_id: DefId,
) -> &'tcx [(DefId, Option<SimplifiedType>)] {
if self.root.is_proc_macro_crate() {
// proc-macro crates export no trait impls.
if self.trait_impls.is_empty() {
return &[];
}

Expand Down Expand Up @@ -1437,13 +1426,8 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
})
}

fn get_native_libraries(self, sess: &Session) -> Vec<NativeLib> {
if self.root.is_proc_macro_crate() {
// Proc macro crates do not have any *target* native libraries.
vec![]
} else {
self.root.native_libraries.decode((self, sess)).collect()
}
fn get_native_libraries(self, sess: &'a Session) -> impl Iterator<Item = NativeLib> + 'a {
self.root.native_libraries.decode((self, sess))
Copy link
Member

Choose a reason for hiding this comment

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

Hm, it isn't entirely obvious to me that native_libraries will always decode to an empty list for is_proc_macro_crate() == true, though I'll admit that I didn't spend a long time spelunking the relevant code trying to figure out if there's any code elsewhere that ensures this property holds.

Is there any way it would make sense to add an assertion or something along the lines here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For proc macro crates encoder now encodes only a specific set of metadata fields related to proc macros, everything else is empty.
It wasn't always like that, so decoder still has many is_proc_macro conditions.
We should generally cleanup it and remove them. But I guess they can be replaced with asserts instead, that would be more reliable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like we are not checking the is_proc_macro property in majority of cases.
This is how it looks if the asserts are added - petrochenkov@b026841
For Lazy<[T]> it looks like we cannot even check whether it's empty without decoding, that's why some checks are commented out.

So I think we should just drop the is_proc_macro checks without adding asserts, and assume that encoder encodes everything correctly.

Copy link
Member

Choose a reason for hiding this comment

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

For Lazy<[T]> it looks like we cannot even check whether it's empty without decoding, that's why some checks are commented out.

I think you can check the .meta field on a Lazy<[T]>.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see.
IMO, it's still simpler to assume that we are just decoding whatever the encoder put there, and make the decoder "fast and dumb", I've updated the PR description.

Copy link
Member

Choose a reason for hiding this comment

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

I'm fine with not adding any of the asserts too, if we are sufficiently confident lack of them will not really cause any issues. Just wanted to make sure we at least considered it.

}

fn get_proc_macro_quoted_span(self, index: usize, sess: &Session) -> Span {
Expand All @@ -1455,15 +1439,8 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
.decode((self, sess))
}

fn get_foreign_modules(self, tcx: TyCtxt<'tcx>) -> Lrc<FxHashMap<DefId, ForeignModule>> {
if self.root.is_proc_macro_crate() {
// Proc macro crates do not have any *target* foreign modules.
Lrc::new(FxHashMap::default())
} else {
let modules: FxHashMap<DefId, ForeignModule> =
self.root.foreign_modules.decode((self, tcx.sess)).map(|m| (m.def_id, m)).collect();
Lrc::new(modules)
}
fn get_foreign_modules(self, sess: &'a Session) -> impl Iterator<Item = ForeignModule> + '_ {
self.root.foreign_modules.decode((self, sess))
}

fn get_dylib_dependency_formats(
Expand All @@ -1479,12 +1456,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
}

fn get_missing_lang_items(self, tcx: TyCtxt<'tcx>) -> &'tcx [lang_items::LangItem] {
if self.root.is_proc_macro_crate() {
// Proc macro crates do not depend on any target weak lang-items.
&[]
} else {
tcx.arena.alloc_from_iter(self.root.lang_items_missing.decode(self))
}
tcx.arena.alloc_from_iter(self.root.lang_items_missing.decode(self))
}

fn get_fn_param_names(self, tcx: TyCtxt<'tcx>, id: DefIndex) -> &'tcx [Ident] {
Expand All @@ -1500,13 +1472,7 @@ impl<'a, 'tcx> CrateMetadataRef<'a> {
self,
tcx: TyCtxt<'tcx>,
) -> &'tcx [(ExportedSymbol<'tcx>, SymbolExportLevel)] {
if self.root.is_proc_macro_crate() {
// If this crate is a custom derive crate, then we're not even going to
// link those in so we skip those crates.
&[]
} else {
tcx.arena.alloc_from_iter(self.root.exported_symbols.decode((self, tcx)))
}
tcx.arena.alloc_from_iter(self.root.exported_symbols.decode((self, tcx)))
}

fn get_rendered_const(self, id: DefIndex) -> String {
Expand Down
33 changes: 23 additions & 10 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 @@ -179,8 +179,10 @@ provide! { <'tcx> tcx, def_id, other, cdata,

reachable_non_generics
}
native_libraries => { Lrc::new(cdata.get_native_libraries(tcx.sess)) }
foreign_modules => { cdata.get_foreign_modules(tcx) }
native_libraries => { Lrc::new(cdata.get_native_libraries(tcx.sess).collect()) }
foreign_modules => {
Lrc::new(cdata.get_foreign_modules(tcx.sess).map(|m| (m.def_id, m)).collect())
}
crate_hash => { cdata.root.hash }
crate_host_hash => { cdata.host_hash }
crate_name => { cdata.root.name }
Expand Down Expand Up @@ -371,11 +373,18 @@ pub(in crate::rmeta) fn provide(providers: &mut Providers) {
}

impl CStore {
pub fn struct_field_names_untracked(&self, def: DefId, sess: &Session) -> Vec<Spanned<Symbol>> {
pub fn struct_field_names_untracked<'a>(
&'a self,
def: DefId,
sess: &'a Session,
) -> impl Iterator<Item = Spanned<Symbol>> + 'a {
self.get_crate_data(def.krate).get_struct_field_names(def.index, sess)
}

pub fn struct_field_visibilities_untracked(&self, def: DefId) -> Vec<Visibility> {
pub fn struct_field_visibilities_untracked(
&self,
def: DefId,
) -> impl Iterator<Item = Visibility> + '_ {
self.get_crate_data(def.krate).get_struct_field_visibilities(def.index)
}

Expand Down Expand Up @@ -460,8 +469,12 @@ impl CStore {
self.get_crate_data(cnum).num_def_ids()
}

pub fn item_attrs_untracked(&self, def_id: DefId, sess: &Session) -> Vec<ast::Attribute> {
self.get_crate_data(def_id.krate).get_item_attrs(def_id.index, sess).collect()
pub fn item_attrs_untracked<'a>(
&'a self,
def_id: DefId,
sess: &'a Session,
) -> impl Iterator<Item = ast::Attribute> + 'a {
self.get_crate_data(def_id.krate).get_item_attrs(def_id.index, sess)
}

pub fn get_proc_macro_quoted_span_untracked(
Expand All @@ -473,15 +486,15 @@ impl CStore {
self.get_crate_data(cnum).get_proc_macro_quoted_span(id, sess)
}

pub fn traits_in_crate_untracked(&self, cnum: CrateNum) -> Vec<DefId> {
self.get_crate_data(cnum).get_traits().collect()
pub fn traits_in_crate_untracked(&self, cnum: CrateNum) -> impl Iterator<Item = DefId> + '_ {
self.get_crate_data(cnum).get_traits()
}

pub fn trait_impls_in_crate_untracked(
&self,
cnum: CrateNum,
) -> Vec<(DefId, Option<SimplifiedType>)> {
self.get_crate_data(cnum).get_trait_impls().collect()
) -> impl Iterator<Item = (DefId, Option<SimplifiedType>)> + '_ {
self.get_crate_data(cnum).get_trait_impls()
}
}

Expand Down
9 changes: 6 additions & 3 deletions compiler/rustc_resolve/src/build_reduced_graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -999,20 +999,23 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> {
let cstore = self.r.cstore();
match res {
Res::Def(DefKind::Struct, def_id) => {
let field_names = cstore.struct_field_names_untracked(def_id, self.r.session);
let field_names =
cstore.struct_field_names_untracked(def_id, self.r.session).collect();
let ctor = cstore.ctor_def_id_and_kind_untracked(def_id);
if let Some((ctor_def_id, ctor_kind)) = ctor {
let ctor_res = Res::Def(DefKind::Ctor(CtorOf::Struct, ctor_kind), ctor_def_id);
let ctor_vis = cstore.visibility_untracked(ctor_def_id);
let field_visibilities = cstore.struct_field_visibilities_untracked(def_id);
let field_visibilities =
cstore.struct_field_visibilities_untracked(def_id).collect();
self.r
.struct_constructors
.insert(def_id, (ctor_res, ctor_vis, field_visibilities));
}
self.insert_field_names(def_id, field_names);
}
Res::Def(DefKind::Union, def_id) => {
let field_names = cstore.struct_field_names_untracked(def_id, self.r.session);
let field_names =
cstore.struct_field_names_untracked(def_id, self.r.session).collect();
self.insert_field_names(def_id, field_names);
}
Res::Def(DefKind::AssocFn, def_id) => {
Expand Down
10 changes: 4 additions & 6 deletions compiler/rustc_resolve/src/diagnostics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -914,19 +914,17 @@ impl<'a> Resolver<'a> {
// a note about editions
let note = if let Some(did) = did {
let requires_note = !did.is_local()
&& this
.cstore()
.item_attrs_untracked(did, this.session)
.iter()
.any(|attr| {
&& this.cstore().item_attrs_untracked(did, this.session).any(
|attr| {
if attr.has_name(sym::rustc_diagnostic_item) {
[sym::TryInto, sym::TryFrom, sym::FromIterator]
.map(|x| Some(x))
.contains(&attr.value_str())
} else {
false
}
});
},
);

requires_note.then(|| {
format!(
Expand Down
1 change: 0 additions & 1 deletion compiler/rustc_resolve/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3442,7 +3442,6 @@ impl<'a> Resolver<'a> {
let attr = self
.cstore()
.item_attrs_untracked(def_id, self.session)
.into_iter()
.find(|a| a.has_name(sym::rustc_legacy_const_generics))?;
let mut ret = Vec::new();
for meta in attr.meta_item_list()? {
Expand Down