From bd2b210c59eb9e7e245060f08826351d355ce634 Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Tue, 3 May 2022 13:27:13 -0400 Subject: [PATCH 01/30] Remove CheckConstTraitVisitor Signed-off-by: Miguel Guarniz --- compiler/rustc_passes/src/check_const.rs | 122 ++++++++++------------- 1 file changed, 54 insertions(+), 68 deletions(-) diff --git a/compiler/rustc_passes/src/check_const.rs b/compiler/rustc_passes/src/check_const.rs index 15e24299075b9..e0922d41937b3 100644 --- a/compiler/rustc_passes/src/check_const.rs +++ b/compiler/rustc_passes/src/check_const.rs @@ -10,6 +10,7 @@ use rustc_attr as attr; use rustc_errors::struct_span_err; use rustc_hir as hir; +use rustc_hir::def::DefKind; use rustc_hir::def_id::LocalDefId; use rustc_hir::intravisit::{self, Visitor}; use rustc_middle::hir::nested_filter; @@ -58,88 +59,73 @@ impl NonConstExpr { fn check_mod_const_bodies(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { let mut vis = CheckConstVisitor::new(tcx); tcx.hir().visit_item_likes_in_module(module_def_id, &mut vis.as_deep_visitor()); - tcx.hir().visit_item_likes_in_module(module_def_id, &mut CheckConstTraitVisitor::new(tcx)); + for id in tcx.hir_module_items(module_def_id).items() { + check_item(tcx, id); + } } pub(crate) fn provide(providers: &mut Providers) { *providers = Providers { check_mod_const_bodies, ..*providers }; } -struct CheckConstTraitVisitor<'tcx> { - tcx: TyCtxt<'tcx>, -} +fn check_item<'tcx>(tcx: TyCtxt<'tcx>, id: hir::ItemId) { + let _: Option<_> = try { + if !matches!(tcx.hir().def_kind(id.def_id), DefKind::Impl) { + None? + } -impl<'tcx> CheckConstTraitVisitor<'tcx> { - fn new(tcx: TyCtxt<'tcx>) -> Self { - CheckConstTraitVisitor { tcx } - } -} + let item = tcx.hir().item(id); + if let hir::ItemKind::Impl(ref imp) = item.kind && let hir::Constness::Const = imp.constness { + let trait_def_id = imp.of_trait.as_ref()?.trait_def_id()?; + let ancestors = tcx + .trait_def(trait_def_id) + .ancestors(tcx, item.def_id.to_def_id()) + .ok()?; + let mut to_implement = Vec::new(); -impl<'tcx> hir::itemlikevisit::ItemLikeVisitor<'tcx> for CheckConstTraitVisitor<'tcx> { - /// check for const trait impls, and errors if the impl uses provided/default functions - /// of the trait being implemented; as those provided functions can be non-const. - fn visit_item<'hir>(&mut self, item: &'hir hir::Item<'hir>) { - let _: Option<_> = try { - if let hir::ItemKind::Impl(ref imp) = item.kind && let hir::Constness::Const = imp.constness { - let trait_def_id = imp.of_trait.as_ref()?.trait_def_id()?; - let ancestors = self - .tcx - .trait_def(trait_def_id) - .ancestors(self.tcx, item.def_id.to_def_id()) - .ok()?; - let mut to_implement = Vec::new(); - - for trait_item in self.tcx.associated_items(trait_def_id).in_definition_order() + for trait_item in tcx.associated_items(trait_def_id).in_definition_order() + { + if let ty::AssocItem { + kind: ty::AssocKind::Fn, + defaultness, + def_id: trait_item_id, + .. + } = *trait_item + { + // we can ignore functions that do not have default bodies: + // if those are unimplemented it will be caught by typeck. + if !defaultness.has_value() + || tcx + .has_attr(trait_item_id, sym::default_method_body_is_const) { - if let ty::AssocItem { - kind: ty::AssocKind::Fn, - defaultness, - def_id: trait_item_id, - .. - } = *trait_item - { - // we can ignore functions that do not have default bodies: - // if those are unimplemented it will be caught by typeck. - if !defaultness.has_value() - || self - .tcx - .has_attr(trait_item_id, sym::default_method_body_is_const) - { - continue; - } - - let is_implemented = ancestors - .leaf_def(self.tcx, trait_item_id) - .map(|node_item| !node_item.defining_node.is_from_trait()) - .unwrap_or(false); - - if !is_implemented { - to_implement.push(self.tcx.item_name(trait_item_id).to_string()); - } - } + continue; } - // all nonconst trait functions (not marked with #[default_method_body_is_const]) - // must be implemented - if !to_implement.is_empty() { - self.tcx - .sess - .struct_span_err( - item.span, - "const trait implementations may not use non-const default functions", - ) - .note(&format!("`{}` not implemented", to_implement.join("`, `"))) - .emit(); + let is_implemented = ancestors + .leaf_def(tcx, trait_item_id) + .map(|node_item| !node_item.defining_node.is_from_trait()) + .unwrap_or(false); + + if !is_implemented { + to_implement.push(tcx.item_name(trait_item_id).to_string()); } + } } - }; - } - fn visit_trait_item<'hir>(&mut self, _: &'hir hir::TraitItem<'hir>) {} - - fn visit_impl_item<'hir>(&mut self, _: &'hir hir::ImplItem<'hir>) {} - - fn visit_foreign_item<'hir>(&mut self, _: &'hir hir::ForeignItem<'hir>) {} + // all nonconst trait functions (not marked with #[default_method_body_is_const]) + // must be implemented + if !to_implement.is_empty() { + tcx + .sess + .struct_span_err( + item.span, + "const trait implementations may not use non-const default functions", + ) + .note(&format!("`{}` not implemented", to_implement.join("`, `"))) + .emit(); + } + } + }; } #[derive(Copy, Clone)] From 00260347e344bbf3af708b7a70b7aaecfc684c03 Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Tue, 3 May 2022 15:46:44 -0400 Subject: [PATCH 02/30] replace usage of visit_item_likes_in_modules with hir_module_items query Signed-off-by: Miguel Guarniz --- compiler/rustc_passes/src/check_const.rs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_passes/src/check_const.rs b/compiler/rustc_passes/src/check_const.rs index e0922d41937b3..7567fb075cced 100644 --- a/compiler/rustc_passes/src/check_const.rs +++ b/compiler/rustc_passes/src/check_const.rs @@ -58,10 +58,28 @@ impl NonConstExpr { fn check_mod_const_bodies(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { let mut vis = CheckConstVisitor::new(tcx); - tcx.hir().visit_item_likes_in_module(module_def_id, &mut vis.as_deep_visitor()); - for id in tcx.hir_module_items(module_def_id).items() { + let module = tcx.hir_module_items(module_def_id); + + for id in module.items() { + vis.visit_item(tcx.hir().item(id)); check_item(tcx, id); } + + for id in module.trait_items() { + vis.visit_trait_item(tcx.hir().trait_item(id)); + } + + for id in module.impl_items() { + vis.visit_impl_item(tcx.hir().impl_item(id)); + } + + for id in module.foreign_items() { + vis.visit_foreign_item(tcx.hir().foreign_item(id)); + } + + // for id in tcx.hir_module_items(module_def_id).items() { + // check_item(tcx, id); + // } } pub(crate) fn provide(providers: &mut Providers) { From 52f833a254ddfa786f34944bc31c205b51e28ecb Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Tue, 3 May 2022 16:19:59 -0400 Subject: [PATCH 03/30] remove LifeSeeder Signed-off-by: Miguel Guarniz --- compiler/rustc_passes/src/dead.rs | 121 +++++++++++++++++------------- 1 file changed, 69 insertions(+), 52 deletions(-) diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index df28ea4d444ba..e136bb000919e 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -8,7 +8,6 @@ use rustc_hir as hir; use rustc_hir::def::{CtorOf, DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::intravisit::{self, Visitor}; -use rustc_hir::itemlikevisit::ItemLikeVisitor; use rustc_hir::{Node, PatKind, TyKind}; use rustc_middle::hir::nested_filter; use rustc_middle::middle::codegen_fn_attrs::CodegenFnAttrFlags; @@ -468,7 +467,7 @@ fn has_allow_dead_code_or_lang_attr(tcx: TyCtxt<'_>, id: hir::HirId) -> bool { tcx.lint_level_at_node(lint::builtin::DEAD_CODE, id).0 == lint::Allow } -// This visitor seeds items that +// These check_* functions seeds items that // 1) We want to explicitly consider as live: // * Item annotated with #[allow(dead_code)] // - This is done so that if we want to suppress warnings for a @@ -481,82 +480,89 @@ fn has_allow_dead_code_or_lang_attr(tcx: TyCtxt<'_>, id: hir::HirId) -> bool { // or // 2) We are not sure to be live or not // * Implementations of traits and trait methods -struct LifeSeeder<'tcx> { - worklist: Vec, +fn check_item<'tcx>( tcx: TyCtxt<'tcx>, - // see `MarkSymbolVisitor::struct_constructors` - struct_constructors: FxHashMap, -} - -impl<'v, 'tcx> ItemLikeVisitor<'v> for LifeSeeder<'tcx> { - fn visit_item(&mut self, item: &hir::Item<'_>) { - let allow_dead_code = has_allow_dead_code_or_lang_attr(self.tcx, item.hir_id()); - if allow_dead_code { - self.worklist.push(item.def_id); - } - match item.kind { - hir::ItemKind::Enum(ref enum_def, _) => { - let hir = self.tcx.hir(); + worklist: &mut Vec, + struct_constructors: &mut FxHashMap, + id: hir::ItemId, +) { + let allow_dead_code = has_allow_dead_code_or_lang_attr(tcx, id.hir_id()); + if allow_dead_code { + worklist.push(id.def_id); + } + + match tcx.hir().def_kind(id.def_id) { + DefKind::Enum => { + let item = tcx.hir().item(id); + if let hir::ItemKind::Enum(ref enum_def, _) = item.kind { + let hir = tcx.hir(); if allow_dead_code { - self.worklist.extend( + worklist.extend( enum_def.variants.iter().map(|variant| hir.local_def_id(variant.id)), ); } for variant in enum_def.variants { if let Some(ctor_hir_id) = variant.data.ctor_hir_id() { - self.struct_constructors + struct_constructors .insert(hir.local_def_id(ctor_hir_id), hir.local_def_id(variant.id)); } } } - hir::ItemKind::Impl(hir::Impl { ref of_trait, items, .. }) => { + } + DefKind::Impl => { + let item = tcx.hir().item(id); + if let hir::ItemKind::Impl(hir::Impl { ref of_trait, items, .. }) = item.kind { if of_trait.is_some() { - self.worklist.push(item.def_id); + worklist.push(item.def_id); } for impl_item_ref in *items { - let impl_item = self.tcx.hir().impl_item(impl_item_ref.id); + let impl_item = tcx.hir().impl_item(impl_item_ref.id); if of_trait.is_some() - || has_allow_dead_code_or_lang_attr(self.tcx, impl_item.hir_id()) + || has_allow_dead_code_or_lang_attr(tcx, impl_item.hir_id()) { - self.worklist.push(impl_item_ref.id.def_id); + worklist.push(impl_item_ref.id.def_id); } } } - hir::ItemKind::Struct(ref variant_data, _) => { + } + DefKind::Struct => { + let item = tcx.hir().item(id); + if let hir::ItemKind::Struct(ref variant_data, _) = item.kind { if let Some(ctor_hir_id) = variant_data.ctor_hir_id() { - self.struct_constructors - .insert(self.tcx.hir().local_def_id(ctor_hir_id), item.def_id); + struct_constructors.insert(tcx.hir().local_def_id(ctor_hir_id), item.def_id); } } - hir::ItemKind::GlobalAsm(_) => { - // global_asm! is always live. - self.worklist.push(item.def_id); - } - _ => (), } + DefKind::GlobalAsm => { + // global_asm! is always live. + worklist.push(id.def_id); + } + _ => {} } +} - fn visit_trait_item(&mut self, trait_item: &hir::TraitItem<'_>) { - use hir::TraitItemKind::{Const, Fn}; +fn check_trait_item<'tcx>(tcx: TyCtxt<'tcx>, worklist: &mut Vec, id: hir::TraitItemId) { + use hir::TraitItemKind::{Const, Fn}; + if matches!(tcx.hir().def_kind(id.def_id), DefKind::AssocConst | DefKind::AssocFn) { + let trait_item = tcx.hir().trait_item(id); if matches!(trait_item.kind, Const(_, Some(_)) | Fn(_, hir::TraitFn::Provided(_))) - && has_allow_dead_code_or_lang_attr(self.tcx, trait_item.hir_id()) + && has_allow_dead_code_or_lang_attr(tcx, trait_item.hir_id()) { - self.worklist.push(trait_item.def_id); + worklist.push(trait_item.def_id); } } +} - fn visit_impl_item(&mut self, _item: &hir::ImplItem<'_>) { - // ignore: we are handling this in `visit_item` above - } - - fn visit_foreign_item(&mut self, foreign_item: &hir::ForeignItem<'_>) { - use hir::ForeignItemKind::{Fn, Static}; - if matches!(foreign_item.kind, Static(..) | Fn(..)) - && has_allow_dead_code_or_lang_attr(self.tcx, foreign_item.hir_id()) - { - self.worklist.push(foreign_item.def_id); - } +fn check_foreign_item<'tcx>( + tcx: TyCtxt<'tcx>, + worklist: &mut Vec, + id: hir::ForeignItemId, +) { + if matches!(tcx.hir().def_kind(id.def_id), DefKind::Static(_) | DefKind::Fn) + && has_allow_dead_code_or_lang_attr(tcx, id.hir_id()) + { + worklist.push(id.def_id); } } @@ -564,7 +570,9 @@ fn create_and_seed_worklist<'tcx>( tcx: TyCtxt<'tcx>, ) -> (Vec, FxHashMap) { let access_levels = &tcx.privacy_access_levels(()); - let worklist = access_levels + // see `MarkSymbolVisitor::struct_constructors` + let mut struct_constructors = Default::default(); + let mut worklist = access_levels .map .iter() .filter_map( @@ -576,11 +584,20 @@ fn create_and_seed_worklist<'tcx>( .chain(tcx.entry_fn(()).and_then(|(def_id, _)| def_id.as_local())) .collect::>(); - // Seed implemented trait items - let mut life_seeder = LifeSeeder { worklist, tcx, struct_constructors: Default::default() }; - tcx.hir().visit_all_item_likes(&mut life_seeder); + let crate_items = tcx.hir_crate_items(()); + for id in crate_items.items() { + check_item(tcx, &mut worklist, &mut struct_constructors, id); + } + + for id in crate_items.trait_items() { + check_trait_item(tcx, &mut worklist, id); + } + + for id in crate_items.foreign_items() { + check_foreign_item(tcx, &mut worklist, id); + } - (life_seeder.worklist, life_seeder.struct_constructors) + (worklist, struct_constructors) } fn live_symbols_and_ignored_derived_traits<'tcx>( From dab0e75911b44618c594cda02141ff2102041772 Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Tue, 3 May 2022 17:05:19 -0400 Subject: [PATCH 04/30] remove DiagnosticItemCollector Signed-off-by: Miguel Guarniz --- compiler/rustc_passes/src/diagnostic_items.rs | 67 ++++++++----------- 1 file changed, 28 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_passes/src/diagnostic_items.rs b/compiler/rustc_passes/src/diagnostic_items.rs index ed694eb0e327a..e6b69d8986cf8 100644 --- a/compiler/rustc_passes/src/diagnostic_items.rs +++ b/compiler/rustc_passes/src/diagnostic_items.rs @@ -10,49 +10,22 @@ //! * Compiler internal types like `Ty` and `TyCtxt` use rustc_ast as ast; -use rustc_hir as hir; use rustc_hir::diagnostic_items::DiagnosticItems; -use rustc_hir::itemlikevisit::ItemLikeVisitor; use rustc_middle::ty::query::Providers; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::{CrateNum, DefId, LocalDefId, LOCAL_CRATE}; use rustc_span::symbol::{sym, Symbol}; -struct DiagnosticItemCollector<'tcx> { +fn observe_item<'tcx>( tcx: TyCtxt<'tcx>, - diagnostic_items: DiagnosticItems, -} - -impl<'v, 'tcx> ItemLikeVisitor<'v> for DiagnosticItemCollector<'tcx> { - fn visit_item(&mut self, item: &hir::Item<'_>) { - self.observe_item(item.def_id); - } - - fn visit_trait_item(&mut self, trait_item: &hir::TraitItem<'_>) { - self.observe_item(trait_item.def_id); - } - - fn visit_impl_item(&mut self, impl_item: &hir::ImplItem<'_>) { - self.observe_item(impl_item.def_id); - } - - fn visit_foreign_item(&mut self, foreign_item: &hir::ForeignItem<'_>) { - self.observe_item(foreign_item.def_id); - } -} - -impl<'tcx> DiagnosticItemCollector<'tcx> { - fn new(tcx: TyCtxt<'tcx>) -> DiagnosticItemCollector<'tcx> { - DiagnosticItemCollector { tcx, diagnostic_items: DiagnosticItems::default() } - } - - fn observe_item(&mut self, def_id: LocalDefId) { - let hir_id = self.tcx.hir().local_def_id_to_hir_id(def_id); - let attrs = self.tcx.hir().attrs(hir_id); - if let Some(name) = extract(attrs) { - // insert into our table - collect_item(self.tcx, &mut self.diagnostic_items, name, def_id.to_def_id()); - } + diagnostic_items: &mut DiagnosticItems, + def_id: LocalDefId, +) { + let hir_id = tcx.hir().local_def_id_to_hir_id(def_id); + let attrs = tcx.hir().attrs(hir_id); + if let Some(name) = extract(attrs) { + // insert into our table + collect_item(tcx, diagnostic_items, name, def_id.to_def_id()); } } @@ -95,12 +68,28 @@ fn diagnostic_items<'tcx>(tcx: TyCtxt<'tcx>, cnum: CrateNum) -> DiagnosticItems assert_eq!(cnum, LOCAL_CRATE); // Initialize the collector. - let mut collector = DiagnosticItemCollector::new(tcx); + let mut diagnostic_items = DiagnosticItems::default(); // Collect diagnostic items in this crate. - tcx.hir().visit_all_item_likes(&mut collector); + let crate_items = tcx.hir_crate_items(()); + + for id in crate_items.items() { + observe_item(tcx, &mut diagnostic_items, id.def_id); + } + + for id in crate_items.trait_items() { + observe_item(tcx, &mut diagnostic_items, id.def_id); + } + + for id in crate_items.impl_items() { + observe_item(tcx, &mut diagnostic_items, id.def_id); + } + + for id in crate_items.foreign_items() { + observe_item(tcx, &mut diagnostic_items, id.def_id); + } - collector.diagnostic_items + diagnostic_items } /// Traverse and collect all the diagnostic items in all crates. From fb73ae2c8a91cf1de599cc2f55f59b58d1a349ec Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Tue, 3 May 2022 17:25:42 -0400 Subject: [PATCH 05/30] remove ItemLikeVisitor impl for EntryContext Signed-off-by: Miguel Guarniz --- compiler/rustc_passes/src/entry.rs | 31 ++++++++---------------------- 1 file changed, 8 insertions(+), 23 deletions(-) diff --git a/compiler/rustc_passes/src/entry.rs b/compiler/rustc_passes/src/entry.rs index f84b848e08d1e..7c543c48824ad 100644 --- a/compiler/rustc_passes/src/entry.rs +++ b/compiler/rustc_passes/src/entry.rs @@ -1,8 +1,7 @@ use rustc_ast::entry::EntryPointType; use rustc_errors::struct_span_err; use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID, LOCAL_CRATE}; -use rustc_hir::itemlikevisit::ItemLikeVisitor; -use rustc_hir::{ForeignItem, ImplItem, Item, ItemKind, Node, TraitItem, CRATE_HIR_ID}; +use rustc_hir::{Item, ItemKind, Node, CRATE_HIR_ID}; use rustc_middle::ty::query::Providers; use rustc_middle::ty::{DefIdTree, TyCtxt}; use rustc_session::config::{CrateType, EntryFnType}; @@ -25,25 +24,6 @@ struct EntryContext<'tcx> { non_main_fns: Vec, } -impl<'tcx> ItemLikeVisitor<'tcx> for EntryContext<'tcx> { - fn visit_item(&mut self, item: &'tcx Item<'tcx>) { - let at_root = self.tcx.opt_local_parent(item.def_id) == Some(CRATE_DEF_ID); - find_item(item, self, at_root); - } - - fn visit_trait_item(&mut self, _trait_item: &'tcx TraitItem<'tcx>) { - // Entry fn is never a trait item. - } - - fn visit_impl_item(&mut self, _impl_item: &'tcx ImplItem<'tcx>) { - // Entry fn is never a trait item. - } - - fn visit_foreign_item(&mut self, _: &'tcx ForeignItem<'tcx>) { - // Entry fn is never a foreign item. - } -} - fn entry_fn(tcx: TyCtxt<'_>, (): ()) -> Option<(DefId, EntryFnType)> { let any_exe = tcx.sess.crate_types().iter().any(|ty| *ty == CrateType::Executable); if !any_exe { @@ -59,7 +39,10 @@ fn entry_fn(tcx: TyCtxt<'_>, (): ()) -> Option<(DefId, EntryFnType)> { let mut ctxt = EntryContext { tcx, attr_main_fn: None, start_fn: None, non_main_fns: Vec::new() }; - tcx.hir().visit_all_item_likes(&mut ctxt); + for id in tcx.hir().items() { + let item = tcx.hir().item(id); + find_item(item, &mut ctxt); + } configure_main(tcx, &ctxt) } @@ -89,7 +72,9 @@ fn throw_attr_err(sess: &Session, span: Span, attr: &str) { .emit(); } -fn find_item(item: &Item<'_>, ctxt: &mut EntryContext<'_>, at_root: bool) { +fn find_item(item: &Item<'_>, ctxt: &mut EntryContext<'_>) { + let at_root = ctxt.tcx.opt_local_parent(item.def_id) == Some(CRATE_DEF_ID); + match entry_point_type(ctxt, item, at_root) { EntryPointType::None => (), _ if !matches!(item.kind, ItemKind::Fn(..)) => { From b1f0209cd147e7b3d45059f6fcef050a29e03f6d Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Tue, 3 May 2022 17:58:19 -0400 Subject: [PATCH 06/30] optimize find_item to fetch Item only when needed Signed-off-by: Miguel Guarniz --- compiler/rustc_passes/src/entry.rs | 65 +++++++++++++++----------- src/test/ui/error-codes/E0138.stderr | 4 +- src/test/ui/main-wrong-location.stderr | 2 +- 3 files changed, 42 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_passes/src/entry.rs b/compiler/rustc_passes/src/entry.rs index 7c543c48824ad..b494a0fe2edf9 100644 --- a/compiler/rustc_passes/src/entry.rs +++ b/compiler/rustc_passes/src/entry.rs @@ -1,7 +1,8 @@ use rustc_ast::entry::EntryPointType; use rustc_errors::struct_span_err; +use rustc_hir::def::DefKind; use rustc_hir::def_id::{DefId, LocalDefId, CRATE_DEF_ID, LOCAL_CRATE}; -use rustc_hir::{Item, ItemKind, Node, CRATE_HIR_ID}; +use rustc_hir::{ItemId, Node, CRATE_HIR_ID}; use rustc_middle::ty::query::Providers; use rustc_middle::ty::{DefIdTree, TyCtxt}; use rustc_session::config::{CrateType, EntryFnType}; @@ -40,8 +41,7 @@ fn entry_fn(tcx: TyCtxt<'_>, (): ()) -> Option<(DefId, EntryFnType)> { EntryContext { tcx, attr_main_fn: None, start_fn: None, non_main_fns: Vec::new() }; for id in tcx.hir().items() { - let item = tcx.hir().item(id); - find_item(item, &mut ctxt); + find_item(id, &mut ctxt); } configure_main(tcx, &ctxt) @@ -49,21 +49,26 @@ fn entry_fn(tcx: TyCtxt<'_>, (): ()) -> Option<(DefId, EntryFnType)> { // Beware, this is duplicated in `librustc_builtin_macros/test_harness.rs` // (with `ast::Item`), so make sure to keep them in sync. -fn entry_point_type(ctxt: &EntryContext<'_>, item: &Item<'_>, at_root: bool) -> EntryPointType { - let attrs = ctxt.tcx.hir().attrs(item.hir_id()); +// A small optimization was added so that hir::Item is fetched only when needed. +// An equivalent optimization was not applied to the duplicated code in test_harness.rs. +fn entry_point_type(ctxt: &EntryContext<'_>, id: ItemId, at_root: bool) -> EntryPointType { + let attrs = ctxt.tcx.hir().attrs(id.hir_id()); if ctxt.tcx.sess.contains_name(attrs, sym::start) { EntryPointType::Start } else if ctxt.tcx.sess.contains_name(attrs, sym::rustc_main) { EntryPointType::MainAttr - } else if item.ident.name == sym::main { - if at_root { - // This is a top-level function so can be `main`. - EntryPointType::MainNamed + } else { + let item = ctxt.tcx.hir().item(id); + if item.ident.name == sym::main { + if at_root { + // This is a top-level function so can be `main`. + EntryPointType::MainNamed + } else { + EntryPointType::OtherMain + } } else { - EntryPointType::OtherMain + EntryPointType::None } - } else { - EntryPointType::None } } @@ -72,13 +77,13 @@ fn throw_attr_err(sess: &Session, span: Span, attr: &str) { .emit(); } -fn find_item(item: &Item<'_>, ctxt: &mut EntryContext<'_>) { - let at_root = ctxt.tcx.opt_local_parent(item.def_id) == Some(CRATE_DEF_ID); +fn find_item(id: ItemId, ctxt: &mut EntryContext<'_>) { + let at_root = ctxt.tcx.opt_local_parent(id.def_id) == Some(CRATE_DEF_ID); - match entry_point_type(ctxt, item, at_root) { + match entry_point_type(ctxt, id, at_root) { EntryPointType::None => (), - _ if !matches!(item.kind, ItemKind::Fn(..)) => { - let attrs = ctxt.tcx.hir().attrs(item.hir_id()); + _ if !matches!(ctxt.tcx.hir().def_kind(id.def_id), DefKind::Fn) => { + let attrs = ctxt.tcx.hir().attrs(id.hir_id()); if let Some(attr) = ctxt.tcx.sess.find_by_name(attrs, sym::start) { throw_attr_err(&ctxt.tcx.sess, attr.span, "start"); } @@ -88,31 +93,39 @@ fn find_item(item: &Item<'_>, ctxt: &mut EntryContext<'_>) { } EntryPointType::MainNamed => (), EntryPointType::OtherMain => { - ctxt.non_main_fns.push(item.span); + ctxt.non_main_fns.push(ctxt.tcx.def_span(id.def_id.to_def_id())); } EntryPointType::MainAttr => { if ctxt.attr_main_fn.is_none() { - ctxt.attr_main_fn = Some((item.def_id, item.span)); + ctxt.attr_main_fn = Some((id.def_id, ctxt.tcx.def_span(id.def_id.to_def_id()))); } else { struct_span_err!( ctxt.tcx.sess, - item.span, + ctxt.tcx.def_span(id.def_id.to_def_id()), E0137, "multiple functions with a `#[main]` attribute" ) - .span_label(item.span, "additional `#[main]` function") + .span_label( + ctxt.tcx.def_span(id.def_id.to_def_id()), + "additional `#[main]` function", + ) .span_label(ctxt.attr_main_fn.unwrap().1, "first `#[main]` function") .emit(); } } EntryPointType::Start => { if ctxt.start_fn.is_none() { - ctxt.start_fn = Some((item.def_id, item.span)); + ctxt.start_fn = Some((id.def_id, ctxt.tcx.def_span(id.def_id.to_def_id()))); } else { - struct_span_err!(ctxt.tcx.sess, item.span, E0138, "multiple `start` functions") - .span_label(ctxt.start_fn.unwrap().1, "previous `#[start]` function here") - .span_label(item.span, "multiple `start` functions") - .emit(); + struct_span_err!( + ctxt.tcx.sess, + ctxt.tcx.def_span(id.def_id.to_def_id()), + E0138, + "multiple `start` functions" + ) + .span_label(ctxt.start_fn.unwrap().1, "previous `#[start]` function here") + .span_label(ctxt.tcx.def_span(id.def_id.to_def_id()), "multiple `start` functions") + .emit(); } } } diff --git a/src/test/ui/error-codes/E0138.stderr b/src/test/ui/error-codes/E0138.stderr index 2dc6976fe0e9c..fa8c39427323c 100644 --- a/src/test/ui/error-codes/E0138.stderr +++ b/src/test/ui/error-codes/E0138.stderr @@ -2,10 +2,10 @@ error[E0138]: multiple `start` functions --> $DIR/E0138.rs:7:1 | LL | fn foo(argc: isize, argv: *const *const u8) -> isize { 0 } - | ---------------------------------------------------------- previous `#[start]` function here + | ---------------------------------------------------- previous `#[start]` function here ... LL | fn f(argc: isize, argv: *const *const u8) -> isize { 0 } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ multiple `start` functions + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ multiple `start` functions error: aborting due to previous error diff --git a/src/test/ui/main-wrong-location.stderr b/src/test/ui/main-wrong-location.stderr index 0058af9b79ebf..3d64b0a67a1df 100644 --- a/src/test/ui/main-wrong-location.stderr +++ b/src/test/ui/main-wrong-location.stderr @@ -8,7 +8,7 @@ note: here is a function named `main` --> $DIR/main-wrong-location.rs:4:5 | LL | fn main() { } - | ^^^^^^^^^^^^^ + | ^^^^^^^^^ = note: you have one or more functions named `main` not defined at the crate level = help: consider moving the `main` function definitions From 0ef16feb72a53694ce4d5bfa25894e51425eb35d Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Wed, 4 May 2022 13:16:33 -0400 Subject: [PATCH 07/30] remove OuterVisitor Signed-off-by: Miguel Guarniz --- compiler/rustc_passes/src/hir_id_validator.rs | 91 +++++++++++-------- 1 file changed, 52 insertions(+), 39 deletions(-) diff --git a/compiler/rustc_passes/src/hir_id_validator.rs b/compiler/rustc_passes/src/hir_id_validator.rs index 379a6827c8aac..b91249badd4e4 100644 --- a/compiler/rustc_passes/src/hir_id_validator.rs +++ b/compiler/rustc_passes/src/hir_id_validator.rs @@ -3,7 +3,7 @@ use rustc_data_structures::sync::Lock; use rustc_hir as hir; use rustc_hir::def_id::{LocalDefId, CRATE_DEF_ID}; use rustc_hir::intravisit; -use rustc_hir::itemlikevisit::ItemLikeVisitor; +use rustc_hir::intravisit::Visitor; use rustc_hir::{HirId, ItemLocalId}; use rustc_middle::hir::map::Map; use rustc_middle::hir::nested_filter; @@ -20,8 +20,30 @@ pub fn check_crate(tcx: TyCtxt<'_>) { let hir_map = tcx.hir(); hir_map.par_for_each_module(|module_id| { - hir_map - .visit_item_likes_in_module(module_id, &mut OuterVisitor { hir_map, errors: &errors }) + let mut v = HirIdValidator { + hir_map, + owner: None, + hir_ids_seen: Default::default(), + errors: &errors, + }; + + let module = tcx.hir_module_items(module_id); + + for id in module.items() { + v.visit_item(tcx.hir().item(id)) + } + + for id in module.trait_items() { + v.visit_trait_item(tcx.hir().trait_item(id)) + } + + for id in module.impl_items() { + v.visit_impl_item(tcx.hir().impl_item(id)) + } + + for id in module.foreign_items() { + v.visit_foreign_item(tcx.hir().foreign_item(id)) + } }); let errors = errors.into_inner(); @@ -39,13 +61,8 @@ struct HirIdValidator<'a, 'hir> { errors: &'a Lock>, } -struct OuterVisitor<'a, 'hir> { - hir_map: Map<'hir>, - errors: &'a Lock>, -} - -impl<'a, 'hir> OuterVisitor<'a, 'hir> { - fn new_inner_visitor(&self, hir_map: Map<'hir>) -> HirIdValidator<'a, 'hir> { +impl<'a, 'hir> HirIdValidator<'a, 'hir> { + fn new_visitor(&self, hir_map: Map<'hir>) -> HirIdValidator<'a, 'hir> { HirIdValidator { hir_map, owner: None, @@ -53,31 +70,7 @@ impl<'a, 'hir> OuterVisitor<'a, 'hir> { errors: self.errors, } } -} - -impl<'a, 'hir> ItemLikeVisitor<'hir> for OuterVisitor<'a, 'hir> { - fn visit_item(&mut self, i: &'hir hir::Item<'hir>) { - let mut inner_visitor = self.new_inner_visitor(self.hir_map); - inner_visitor.check(i.def_id, |this| intravisit::walk_item(this, i)); - } - - fn visit_trait_item(&mut self, i: &'hir hir::TraitItem<'hir>) { - let mut inner_visitor = self.new_inner_visitor(self.hir_map); - inner_visitor.check(i.def_id, |this| intravisit::walk_trait_item(this, i)); - } - - fn visit_impl_item(&mut self, i: &'hir hir::ImplItem<'hir>) { - let mut inner_visitor = self.new_inner_visitor(self.hir_map); - inner_visitor.check(i.def_id, |this| intravisit::walk_impl_item(this, i)); - } - - fn visit_foreign_item(&mut self, i: &'hir hir::ForeignItem<'hir>) { - let mut inner_visitor = self.new_inner_visitor(self.hir_map); - inner_visitor.check(i.def_id, |this| intravisit::walk_foreign_item(this, i)); - } -} -impl<'a, 'hir> HirIdValidator<'a, 'hir> { #[cold] #[inline(never)] fn error(&self, f: impl FnOnce() -> String) { @@ -146,6 +139,11 @@ impl<'a, 'hir> intravisit::Visitor<'hir> for HirIdValidator<'a, 'hir> { self.hir_map } + fn visit_item(&mut self, i: &'hir hir::Item<'hir>) { + let mut inner_visitor = self.new_visitor(self.hir_map); + inner_visitor.check(i.def_id, |this| intravisit::walk_item(this, i)); + } + fn visit_id(&mut self, hir_id: HirId) { let owner = self.owner.expect("no owner"); @@ -163,11 +161,19 @@ impl<'a, 'hir> intravisit::Visitor<'hir> for HirIdValidator<'a, 'hir> { self.hir_ids_seen.insert(hir_id.local_id); } - fn visit_impl_item_ref(&mut self, _: &'hir hir::ImplItemRef) { - // Explicitly do nothing here. ImplItemRefs contain hir::Visibility - // values that actually belong to an ImplItem instead of the ItemKind::Impl - // we are currently in. So for those it's correct that they have a - // different owner. + fn visit_foreign_item(&mut self, i: &'hir hir::ForeignItem<'hir>) { + let mut inner_visitor = self.new_visitor(self.hir_map); + inner_visitor.check(i.def_id, |this| intravisit::walk_foreign_item(this, i)); + } + + fn visit_trait_item(&mut self, i: &'hir hir::TraitItem<'hir>) { + let mut inner_visitor = self.new_visitor(self.hir_map); + inner_visitor.check(i.def_id, |this| intravisit::walk_trait_item(this, i)); + } + + fn visit_impl_item(&mut self, i: &'hir hir::ImplItem<'hir>) { + let mut inner_visitor = self.new_visitor(self.hir_map); + inner_visitor.check(i.def_id, |this| intravisit::walk_impl_item(this, i)); } fn visit_foreign_item_ref(&mut self, _: &'hir hir::ForeignItemRef) { @@ -176,4 +182,11 @@ impl<'a, 'hir> intravisit::Visitor<'hir> for HirIdValidator<'a, 'hir> { // we are currently in. So for those it's correct that they have a // different owner. } + + fn visit_impl_item_ref(&mut self, _: &'hir hir::ImplItemRef) { + // Explicitly do nothing here. ImplItemRefs contain hir::Visibility + // values that actually belong to an ImplItem instead of the ItemKind::Impl + // we are currently in. So for those it's correct that they have a + // different owner. + } } From 45c37da0f757283d6d50a03a6915eec787179aa4 Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Wed, 4 May 2022 14:02:30 -0400 Subject: [PATCH 08/30] remove LayoutTest Signed-off-by: Miguel Guarniz --- compiler/rustc_passes/src/layout_test.rs | 146 +++++++++++------------ 1 file changed, 67 insertions(+), 79 deletions(-) diff --git a/compiler/rustc_passes/src/layout_test.rs b/compiler/rustc_passes/src/layout_test.rs index 728aaab6137ab..fd03f657111d9 100644 --- a/compiler/rustc_passes/src/layout_test.rs +++ b/compiler/rustc_passes/src/layout_test.rs @@ -1,8 +1,6 @@ use rustc_ast::Attribute; -use rustc_hir as hir; +use rustc_hir::def::DefKind; use rustc_hir::def_id::LocalDefId; -use rustc_hir::itemlikevisit::ItemLikeVisitor; -use rustc_hir::ItemKind; use rustc_middle::ty::layout::{HasParamEnv, HasTyCtxt, LayoutError, LayoutOfHelpers, TyAndLayout}; use rustc_middle::ty::{ParamEnv, Ty, TyCtxt}; use rustc_span::symbol::sym; @@ -12,97 +10,87 @@ use rustc_target::abi::{HasDataLayout, TargetDataLayout}; pub fn test_layout(tcx: TyCtxt<'_>) { if tcx.features().rustc_attrs { // if the `rustc_attrs` feature is not enabled, don't bother testing layout - tcx.hir().visit_all_item_likes(&mut LayoutTest { tcx }); - } -} - -struct LayoutTest<'tcx> { - tcx: TyCtxt<'tcx>, -} - -impl<'tcx> ItemLikeVisitor<'tcx> for LayoutTest<'tcx> { - fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) { - match item.kind { - ItemKind::TyAlias(..) - | ItemKind::Enum(..) - | ItemKind::Struct(..) - | ItemKind::Union(..) => { - for attr in self.tcx.get_attrs(item.def_id.to_def_id(), sym::rustc_layout) { - self.dump_layout_of(item.def_id, item, attr); + for id in tcx.hir().items() { + if matches!( + tcx.def_kind(id.def_id), + DefKind::TyAlias | DefKind::Enum | DefKind::Struct | DefKind::Union + ) { + for attr in tcx.get_attrs(id.def_id.to_def_id(), sym::rustc_layout) { + dump_layout_of(tcx, id.def_id, attr); } } - _ => {} } } - - fn visit_trait_item(&mut self, _: &'tcx hir::TraitItem<'tcx>) {} - fn visit_impl_item(&mut self, _: &'tcx hir::ImplItem<'tcx>) {} - fn visit_foreign_item(&mut self, _: &'tcx hir::ForeignItem<'tcx>) {} } -impl<'tcx> LayoutTest<'tcx> { - fn dump_layout_of(&self, item_def_id: LocalDefId, item: &hir::Item<'tcx>, attr: &Attribute) { - let tcx = self.tcx; - let param_env = self.tcx.param_env(item_def_id); - let ty = self.tcx.type_of(item_def_id); - match self.tcx.layout_of(param_env.and(ty)) { - Ok(ty_layout) => { - // Check out the `#[rustc_layout(..)]` attribute to tell what to dump. - // The `..` are the names of fields to dump. - let meta_items = attr.meta_item_list().unwrap_or_default(); - for meta_item in meta_items { - match meta_item.name_or_empty() { - sym::abi => { - self.tcx.sess.span_err(item.span, &format!("abi: {:?}", ty_layout.abi)); - } +fn dump_layout_of<'tcx>(tcx: TyCtxt<'tcx>, item_def_id: LocalDefId, attr: &Attribute) { + let tcx = tcx; + let param_env = tcx.param_env(item_def_id); + let ty = tcx.type_of(item_def_id); + match tcx.layout_of(param_env.and(ty)) { + Ok(ty_layout) => { + // Check out the `#[rustc_layout(..)]` attribute to tell what to dump. + // The `..` are the names of fields to dump. + let meta_items = attr.meta_item_list().unwrap_or_default(); + for meta_item in meta_items { + match meta_item.name_or_empty() { + sym::abi => { + tcx.sess.span_err( + tcx.def_span(item_def_id.to_def_id()), + &format!("abi: {:?}", ty_layout.abi), + ); + } - sym::align => { - self.tcx - .sess - .span_err(item.span, &format!("align: {:?}", ty_layout.align)); - } + sym::align => { + tcx.sess.span_err( + tcx.def_span(item_def_id.to_def_id()), + &format!("align: {:?}", ty_layout.align), + ); + } - sym::size => { - self.tcx - .sess - .span_err(item.span, &format!("size: {:?}", ty_layout.size)); - } + sym::size => { + tcx.sess.span_err( + tcx.def_span(item_def_id.to_def_id()), + &format!("size: {:?}", ty_layout.size), + ); + } - sym::homogeneous_aggregate => { - self.tcx.sess.span_err( - item.span, - &format!( - "homogeneous_aggregate: {:?}", - ty_layout - .homogeneous_aggregate(&UnwrapLayoutCx { tcx, param_env }), - ), - ); - } + sym::homogeneous_aggregate => { + tcx.sess.span_err( + tcx.def_span(item_def_id.to_def_id()), + &format!( + "homogeneous_aggregate: {:?}", + ty_layout.homogeneous_aggregate(&UnwrapLayoutCx { tcx, param_env }), + ), + ); + } - sym::debug => { - let normalized_ty = self.tcx.normalize_erasing_regions( - param_env.with_reveal_all_normalized(self.tcx), - ty, - ); - self.tcx.sess.span_err( - item.span, - &format!("layout_of({:?}) = {:#?}", normalized_ty, *ty_layout), - ); - } + sym::debug => { + let normalized_ty = tcx.normalize_erasing_regions( + param_env.with_reveal_all_normalized(tcx), + ty, + ); + tcx.sess.span_err( + tcx.def_span(item_def_id.to_def_id()), + &format!("layout_of({:?}) = {:#?}", normalized_ty, *ty_layout), + ); + } - name => { - self.tcx.sess.span_err( - meta_item.span(), - &format!("unrecognized field name `{}`", name), - ); - } + name => { + tcx.sess.span_err( + meta_item.span(), + &format!("unrecognized field name `{}`", name), + ); } } } + } - Err(layout_error) => { - self.tcx.sess.span_err(item.span, &format!("layout error: {:?}", layout_error)); - } + Err(layout_error) => { + tcx.sess.span_err( + tcx.def_span(item_def_id.to_def_id()), + &format!("layout error: {:?}", layout_error), + ); } } } From 0a029e2ed09b8f5dedf7265038ebdf363acb661b Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Wed, 4 May 2022 16:19:34 -0400 Subject: [PATCH 09/30] remove CollectPrivateImplItemsVisitor Signed-off-by: Miguel Guarniz --- compiler/rustc_passes/src/reachable.rs | 127 +++++++++++-------------- 1 file changed, 57 insertions(+), 70 deletions(-) diff --git a/compiler/rustc_passes/src/reachable.rs b/compiler/rustc_passes/src/reachable.rs index a5133bfd9459c..88ca48eae3e78 100644 --- a/compiler/rustc_passes/src/reachable.rs +++ b/compiler/rustc_passes/src/reachable.rs @@ -10,7 +10,6 @@ use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId}; use rustc_hir::intravisit::{self, Visitor}; -use rustc_hir::itemlikevisit::ItemLikeVisitor; use rustc_hir::Node; use rustc_middle::middle::codegen_fn_attrs::{CodegenFnAttrFlags, CodegenFnAttrs}; use rustc_middle::middle::privacy; @@ -314,79 +313,56 @@ impl<'tcx> ReachableContext<'tcx> { } } -// Some methods from non-exported (completely private) trait impls still have to be -// reachable if they are called from inlinable code. Generally, it's not known until -// monomorphization if a specific trait impl item can be reachable or not. So, we -// conservatively mark all of them as reachable. -// FIXME: One possible strategy for pruning the reachable set is to avoid marking impl -// items of non-exported traits (or maybe all local traits?) unless their respective -// trait items are used from inlinable code through method call syntax or UFCS, or their -// trait is a lang item. -struct CollectPrivateImplItemsVisitor<'a, 'tcx> { +fn check_item<'tcx>( tcx: TyCtxt<'tcx>, - access_levels: &'a privacy::AccessLevels, - worklist: &'a mut Vec, -} + item: &hir::Item<'_>, + worklist: &mut Vec, + access_levels: &privacy::AccessLevels +) { + push_to_worklist_if_has_custom_linkage(tcx, worklist, item.def_id); + + // We need only trait impls here, not inherent impls, and only non-exported ones + if let hir::ItemKind::Impl(hir::Impl { of_trait: Some(ref trait_ref), ref items, .. }) = + item.kind + { + if !access_levels.is_reachable(item.def_id) { + // FIXME(#53488) remove `let` + let tcx = tcx; + worklist.extend(items.iter().map(|ii_ref| ii_ref.id.def_id)); -impl CollectPrivateImplItemsVisitor<'_, '_> { - fn push_to_worklist_if_has_custom_linkage(&mut self, def_id: LocalDefId) { - // Anything which has custom linkage gets thrown on the worklist no - // matter where it is in the crate, along with "special std symbols" - // which are currently akin to allocator symbols. - if self.tcx.def_kind(def_id).has_codegen_attrs() { - let codegen_attrs = self.tcx.codegen_fn_attrs(def_id); - if codegen_attrs.contains_extern_indicator() - || codegen_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL) - // FIXME(nbdd0121): `#[used]` are marked as reachable here so it's picked up by - // `linked_symbols` in cg_ssa. They won't be exported in binary or cdylib due to their - // `SymbolExportLevel::Rust` export level but may end up being exported in dylibs. - || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED) - || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER) - { - self.worklist.push(def_id); + let Res::Def(DefKind::Trait, trait_def_id) = trait_ref.path.res else { + unreachable!(); + }; + + if !trait_def_id.is_local() { + return; } + + worklist.extend( + tcx.provided_trait_methods(trait_def_id) + .map(|assoc| assoc.def_id.expect_local()), + ); } } } -impl<'a, 'tcx> ItemLikeVisitor<'tcx> for CollectPrivateImplItemsVisitor<'a, 'tcx> { - fn visit_item(&mut self, item: &hir::Item<'_>) { - self.push_to_worklist_if_has_custom_linkage(item.def_id); - - // We need only trait impls here, not inherent impls, and only non-exported ones - if let hir::ItemKind::Impl(hir::Impl { of_trait: Some(ref trait_ref), ref items, .. }) = - item.kind +fn push_to_worklist_if_has_custom_linkage<'tcx>(tcx: TyCtxt<'tcx>, worklist: &mut Vec, def_id: LocalDefId) { + // Anything which has custom linkage gets thrown on the worklist no + // matter where it is in the crate, along with "special std symbols" + // which are currently akin to allocator symbols. + if tcx.def_kind(def_id).has_codegen_attrs() { + let codegen_attrs = tcx.codegen_fn_attrs(def_id); + if codegen_attrs.contains_extern_indicator() + || codegen_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL) + // FIXME(nbdd0121): `#[used]` are marked as reachable here so it's picked up by + // `linked_symbols` in cg_ssa. They won't be exported in binary or cdylib due to their + // `SymbolExportLevel::Rust` export level but may end up being exported in dylibs. + || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED) + || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER) { - if !self.access_levels.is_reachable(item.def_id) { - // FIXME(#53488) remove `let` - let tcx = self.tcx; - self.worklist.extend(items.iter().map(|ii_ref| ii_ref.id.def_id)); - - let Res::Def(DefKind::Trait, trait_def_id) = trait_ref.path.res else { - unreachable!(); - }; - - if !trait_def_id.is_local() { - return; - } - - self.worklist.extend( - tcx.provided_trait_methods(trait_def_id) - .map(|assoc| assoc.def_id.expect_local()), - ); - } + worklist.push(def_id); } } - - fn visit_trait_item(&mut self, _trait_item: &hir::TraitItem<'_>) {} - - fn visit_impl_item(&mut self, impl_item: &hir::ImplItem<'_>) { - self.push_to_worklist_if_has_custom_linkage(impl_item.def_id); - } - - fn visit_foreign_item(&mut self, _foreign_item: &hir::ForeignItem<'_>) { - // We never export foreign functions as they have no body to export. - } } fn reachable_set<'tcx>(tcx: TyCtxt<'tcx>, (): ()) -> FxHashSet { @@ -418,12 +394,23 @@ fn reachable_set<'tcx>(tcx: TyCtxt<'tcx>, (): ()) -> FxHashSet { } } { - let mut collect_private_impl_items = CollectPrivateImplItemsVisitor { - tcx, - access_levels, - worklist: &mut reachable_context.worklist, - }; - tcx.hir().visit_all_item_likes(&mut collect_private_impl_items); + // Some methods from non-exported (completely private) trait impls still have to be + // reachable if they are called from inlinable code. Generally, it's not known until + // monomorphization if a specific trait impl item can be reachable or not. So, we + // conservatively mark all of them as reachable. + // FIXME: One possible strategy for pruning the reachable set is to avoid marking impl + // items of non-exported traits (or maybe all local traits?) unless their respective + // trait items are used from inlinable code through method call syntax or UFCS, or their + // trait is a lang item. + let crate_items = tcx.hir_crate_items(()); + + for id in crate_items.items() { + check_item(tcx, tcx.hir().item(id), &mut reachable_context.worklist, access_levels); + } + + for id in crate_items.impl_items() { + push_to_worklist_if_has_custom_linkage(tcx, &mut reachable_context.worklist, id.def_id) + } } // Step 2: Mark all symbols that the symbols on the worklist touch. From 90685c633357054dac6a91ecd0c14cbfc7a726a7 Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Wed, 4 May 2022 16:34:24 -0400 Subject: [PATCH 10/30] check def_kind before fetching item Signed-off-by: Miguel Guarniz --- compiler/rustc_passes/src/reachable.rs | 48 +++++++++++++++----------- 1 file changed, 27 insertions(+), 21 deletions(-) diff --git a/compiler/rustc_passes/src/reachable.rs b/compiler/rustc_passes/src/reachable.rs index 88ca48eae3e78..0ded6a421f57f 100644 --- a/compiler/rustc_passes/src/reachable.rs +++ b/compiler/rustc_passes/src/reachable.rs @@ -315,15 +315,22 @@ impl<'tcx> ReachableContext<'tcx> { fn check_item<'tcx>( tcx: TyCtxt<'tcx>, - item: &hir::Item<'_>, + id: hir::ItemId, worklist: &mut Vec, - access_levels: &privacy::AccessLevels + access_levels: &privacy::AccessLevels, ) { - push_to_worklist_if_has_custom_linkage(tcx, worklist, item.def_id); + if has_custom_linkage(tcx, id.def_id) { + worklist.push(id.def_id); + } + + if !matches!(tcx.def_kind(id.def_id), DefKind::Impl) { + return; + } // We need only trait impls here, not inherent impls, and only non-exported ones + let item = tcx.hir().item(id); if let hir::ItemKind::Impl(hir::Impl { of_trait: Some(ref trait_ref), ref items, .. }) = - item.kind + item.kind { if !access_levels.is_reachable(item.def_id) { // FIXME(#53488) remove `let` @@ -339,30 +346,27 @@ fn check_item<'tcx>( } worklist.extend( - tcx.provided_trait_methods(trait_def_id) - .map(|assoc| assoc.def_id.expect_local()), + tcx.provided_trait_methods(trait_def_id).map(|assoc| assoc.def_id.expect_local()), ); } } } -fn push_to_worklist_if_has_custom_linkage<'tcx>(tcx: TyCtxt<'tcx>, worklist: &mut Vec, def_id: LocalDefId) { +fn has_custom_linkage<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> bool { // Anything which has custom linkage gets thrown on the worklist no // matter where it is in the crate, along with "special std symbols" // which are currently akin to allocator symbols. - if tcx.def_kind(def_id).has_codegen_attrs() { - let codegen_attrs = tcx.codegen_fn_attrs(def_id); - if codegen_attrs.contains_extern_indicator() - || codegen_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL) - // FIXME(nbdd0121): `#[used]` are marked as reachable here so it's picked up by - // `linked_symbols` in cg_ssa. They won't be exported in binary or cdylib due to their - // `SymbolExportLevel::Rust` export level but may end up being exported in dylibs. - || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED) - || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER) - { - worklist.push(def_id); - } + if !tcx.def_kind(def_id).has_codegen_attrs() { + return false; } + let codegen_attrs = tcx.codegen_fn_attrs(def_id); + codegen_attrs.contains_extern_indicator() + || codegen_attrs.flags.contains(CodegenFnAttrFlags::RUSTC_STD_INTERNAL_SYMBOL) + // FIXME(nbdd0121): `#[used]` are marked as reachable here so it's picked up by + // `linked_symbols` in cg_ssa. They won't be exported in binary or cdylib due to their + // `SymbolExportLevel::Rust` export level but may end up being exported in dylibs. + || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED) + || codegen_attrs.flags.contains(CodegenFnAttrFlags::USED_LINKER) } fn reachable_set<'tcx>(tcx: TyCtxt<'tcx>, (): ()) -> FxHashSet { @@ -405,11 +409,13 @@ fn reachable_set<'tcx>(tcx: TyCtxt<'tcx>, (): ()) -> FxHashSet { let crate_items = tcx.hir_crate_items(()); for id in crate_items.items() { - check_item(tcx, tcx.hir().item(id), &mut reachable_context.worklist, access_levels); + check_item(tcx, id, &mut reachable_context.worklist, access_levels); } for id in crate_items.impl_items() { - push_to_worklist_if_has_custom_linkage(tcx, &mut reachable_context.worklist, id.def_id) + if has_custom_linkage(tcx, id.def_id) { + reachable_context.worklist.push(id.def_id); + } } } From eea16de9f70738e1ee19d6d73f0c74bc746a9633 Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Wed, 4 May 2022 16:43:36 -0400 Subject: [PATCH 11/30] replace hir().def_kind for def_kind query in rustc_passes Signed-off-by: Miguel Guarniz --- compiler/rustc_passes/src/check_const.rs | 2 +- compiler/rustc_passes/src/dead.rs | 6 +++--- compiler/rustc_passes/src/entry.rs | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_passes/src/check_const.rs b/compiler/rustc_passes/src/check_const.rs index 7567fb075cced..0e4df831f3fe7 100644 --- a/compiler/rustc_passes/src/check_const.rs +++ b/compiler/rustc_passes/src/check_const.rs @@ -88,7 +88,7 @@ pub(crate) fn provide(providers: &mut Providers) { fn check_item<'tcx>(tcx: TyCtxt<'tcx>, id: hir::ItemId) { let _: Option<_> = try { - if !matches!(tcx.hir().def_kind(id.def_id), DefKind::Impl) { + if !matches!(tcx.def_kind(id.def_id), DefKind::Impl) { None? } diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index e136bb000919e..55a67070a9674 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -491,7 +491,7 @@ fn check_item<'tcx>( worklist.push(id.def_id); } - match tcx.hir().def_kind(id.def_id) { + match tcx.def_kind(id.def_id) { DefKind::Enum => { let item = tcx.hir().item(id); if let hir::ItemKind::Enum(ref enum_def, _) = item.kind { @@ -544,7 +544,7 @@ fn check_item<'tcx>( fn check_trait_item<'tcx>(tcx: TyCtxt<'tcx>, worklist: &mut Vec, id: hir::TraitItemId) { use hir::TraitItemKind::{Const, Fn}; - if matches!(tcx.hir().def_kind(id.def_id), DefKind::AssocConst | DefKind::AssocFn) { + if matches!(tcx.def_kind(id.def_id), DefKind::AssocConst | DefKind::AssocFn) { let trait_item = tcx.hir().trait_item(id); if matches!(trait_item.kind, Const(_, Some(_)) | Fn(_, hir::TraitFn::Provided(_))) && has_allow_dead_code_or_lang_attr(tcx, trait_item.hir_id()) @@ -559,7 +559,7 @@ fn check_foreign_item<'tcx>( worklist: &mut Vec, id: hir::ForeignItemId, ) { - if matches!(tcx.hir().def_kind(id.def_id), DefKind::Static(_) | DefKind::Fn) + if matches!(tcx.def_kind(id.def_id), DefKind::Static(_) | DefKind::Fn) && has_allow_dead_code_or_lang_attr(tcx, id.hir_id()) { worklist.push(id.def_id); diff --git a/compiler/rustc_passes/src/entry.rs b/compiler/rustc_passes/src/entry.rs index b494a0fe2edf9..266e1315ce26d 100644 --- a/compiler/rustc_passes/src/entry.rs +++ b/compiler/rustc_passes/src/entry.rs @@ -82,7 +82,7 @@ fn find_item(id: ItemId, ctxt: &mut EntryContext<'_>) { match entry_point_type(ctxt, id, at_root) { EntryPointType::None => (), - _ if !matches!(ctxt.tcx.hir().def_kind(id.def_id), DefKind::Fn) => { + _ if !matches!(ctxt.tcx.def_kind(id.def_id), DefKind::Fn) => { let attrs = ctxt.tcx.hir().attrs(id.hir_id()); if let Some(attr) = ctxt.tcx.sess.find_by_name(attrs, sym::start) { throw_attr_err(&ctxt.tcx.sess, attr.span, "start"); From e8ef5bf464f142948b7cc061ca2bb4304df088ff Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Wed, 4 May 2022 17:03:47 -0400 Subject: [PATCH 12/30] remove TraitVisitor Signed-off-by: Miguel Guarniz --- compiler/rustc_metadata/src/rmeta/encoder.rs | 23 +++++--------------- 1 file changed, 6 insertions(+), 17 deletions(-) diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index 1de7dae3c25cb..fe06f879775ee 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -14,7 +14,6 @@ use rustc_hir::def_id::{ }; use rustc_hir::definitions::DefPathData; use rustc_hir::intravisit::{self, Visitor}; -use rustc_hir::itemlikevisit::ItemLikeVisitor; use rustc_hir::lang_items; use rustc_hir::{AnonConst, GenericParamKind}; use rustc_index::bit_set::GrowableBitSet; @@ -2243,26 +2242,16 @@ pub fn provide(providers: &mut Providers) { traits_in_crate: |tcx, cnum| { assert_eq!(cnum, LOCAL_CRATE); - #[derive(Default)] - struct TraitsVisitor { - traits: Vec, - } - impl ItemLikeVisitor<'_> for TraitsVisitor { - fn visit_item(&mut self, item: &hir::Item<'_>) { - if let hir::ItemKind::Trait(..) | hir::ItemKind::TraitAlias(..) = item.kind { - self.traits.push(item.def_id.to_def_id()); - } + let mut traits = Vec::new(); + for id in tcx.hir().items() { + if matches!(tcx.def_kind(id.def_id), DefKind::Trait | DefKind::TraitAlias) { + traits.push(id.def_id.to_def_id()) } - fn visit_trait_item(&mut self, _trait_item: &hir::TraitItem<'_>) {} - fn visit_impl_item(&mut self, _impl_item: &hir::ImplItem<'_>) {} - fn visit_foreign_item(&mut self, _foreign_item: &hir::ForeignItem<'_>) {} } - let mut visitor = TraitsVisitor::default(); - tcx.hir().visit_all_item_likes(&mut visitor); // Bring everything into deterministic order. - visitor.traits.sort_by_cached_key(|&def_id| tcx.def_path_hash(def_id)); - tcx.arena.alloc_slice(&visitor.traits) + traits.sort_by_cached_key(|&def_id| tcx.def_path_hash(def_id)); + tcx.arena.alloc_slice(&traits) }, ..*providers From e166409f0d1560a7172cac6ee457391a1f3a4e77 Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Thu, 5 May 2022 13:34:27 -0400 Subject: [PATCH 13/30] remove Visitor impl for PrivateItemsInPublicInterfacesChecker Signed-off-by: Miguel Guarniz --- compiler/rustc_privacy/src/lib.rs | 176 ++++++++++++++++-------------- 1 file changed, 95 insertions(+), 81 deletions(-) diff --git a/compiler/rustc_privacy/src/lib.rs b/compiler/rustc_privacy/src/lib.rs index ee459d9c129d3..0bcb0719f9ba7 100644 --- a/compiler/rustc_privacy/src/lib.rs +++ b/compiler/rustc_privacy/src/lib.rs @@ -14,8 +14,8 @@ use rustc_errors::struct_span_err; use rustc_hir as hir; use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{DefId, LocalDefId, LocalDefIdSet, CRATE_DEF_ID}; -use rustc_hir::intravisit::{self, DeepVisitor, Visitor}; -use rustc_hir::{AssocItemKind, HirIdSet, Node, PatKind}; +use rustc_hir::intravisit::{self, Visitor}; +use rustc_hir::{AssocItemKind, HirIdSet, ItemId, Node, PatKind}; use rustc_middle::bug; use rustc_middle::hir::nested_filter; use rustc_middle::middle::privacy::{AccessLevel, AccessLevels}; @@ -1802,12 +1802,12 @@ impl<'tcx> DefIdVisitor<'tcx> for SearchInterfaceForPrivateItemsVisitor<'tcx> { } } -struct PrivateItemsInPublicInterfacesVisitor<'tcx> { +struct PrivateItemsInPublicInterfacesChecker<'tcx> { tcx: TyCtxt<'tcx>, old_error_set_ancestry: LocalDefIdSet, } -impl<'tcx> PrivateItemsInPublicInterfacesVisitor<'tcx> { +impl<'tcx> PrivateItemsInPublicInterfacesChecker<'tcx> { fn check( &self, def_id: LocalDefId, @@ -1841,110 +1841,121 @@ impl<'tcx> PrivateItemsInPublicInterfacesVisitor<'tcx> { check.ty(); } } -} - -impl<'tcx> Visitor<'tcx> for PrivateItemsInPublicInterfacesVisitor<'tcx> { - type NestedFilter = nested_filter::OnlyBodies; - fn nested_visit_map(&mut self) -> Self::Map { - self.tcx.hir() - } - - fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) { + pub fn check_item(&mut self, id: ItemId) { let tcx = self.tcx; - let item_visibility = tcx.visibility(item.def_id); + let item_visibility = tcx.visibility(id.def_id); + let def_kind = tcx.def_kind(id.def_id); + + if matches!( + def_kind, + DefKind::ExternCrate + | DefKind::Mod + | DefKind::Use + | DefKind::Macro(_) + | DefKind::GlobalAsm + ) { + return; + } - match item.kind { - // Crates are always public. - hir::ItemKind::ExternCrate(..) => {} - // All nested items are checked by `visit_item`. - hir::ItemKind::Mod(..) => {} - // Checked in resolve. - hir::ItemKind::Use(..) => {} - // No subitems. - hir::ItemKind::Macro(..) | hir::ItemKind::GlobalAsm(..) => {} - // Subitems of these items have inherited publicity. - hir::ItemKind::Const(..) - | hir::ItemKind::Static(..) - | hir::ItemKind::Fn(..) - | hir::ItemKind::TyAlias(..) => { - self.check(item.def_id, item_visibility).generics().predicates().ty(); + match def_kind { + DefKind::Const | DefKind::Static(_) | DefKind::Fn | DefKind::TyAlias => { + self.check(id.def_id, item_visibility).generics().predicates().ty(); } - hir::ItemKind::OpaqueTy(..) => { + DefKind::OpaqueTy => { // `ty()` for opaque types is the underlying type, // it's not a part of interface, so we skip it. - self.check(item.def_id, item_visibility).generics().bounds(); + self.check(id.def_id, item_visibility).generics().bounds(); } - hir::ItemKind::Trait(.., trait_item_refs) => { - self.check(item.def_id, item_visibility).generics().predicates(); + DefKind::Trait => { + let item = tcx.hir().item(id); + if let hir::ItemKind::Trait(.., trait_item_refs) = item.kind { + self.check(item.def_id, item_visibility).generics().predicates(); - for trait_item_ref in trait_item_refs { - self.check_assoc_item( - trait_item_ref.id.def_id, - trait_item_ref.kind, - trait_item_ref.defaultness, - item_visibility, - ); - - if let AssocItemKind::Type = trait_item_ref.kind { - self.check(trait_item_ref.id.def_id, item_visibility).bounds(); + for trait_item_ref in trait_item_refs { + self.check_assoc_item( + trait_item_ref.id.def_id, + trait_item_ref.kind, + trait_item_ref.defaultness, + item_visibility, + ); + + if let AssocItemKind::Type = trait_item_ref.kind { + self.check(trait_item_ref.id.def_id, item_visibility).bounds(); + } } } } - hir::ItemKind::TraitAlias(..) => { - self.check(item.def_id, item_visibility).generics().predicates(); + DefKind::TraitAlias => { + self.check(id.def_id, item_visibility).generics().predicates(); } - hir::ItemKind::Enum(ref def, _) => { - self.check(item.def_id, item_visibility).generics().predicates(); + DefKind::Enum => { + let item = tcx.hir().item(id); + if let hir::ItemKind::Enum(ref def, _) = item.kind { + self.check(item.def_id, item_visibility).generics().predicates(); - for variant in def.variants { - for field in variant.data.fields() { - self.check(self.tcx.hir().local_def_id(field.hir_id), item_visibility).ty(); + for variant in def.variants { + for field in variant.data.fields() { + self.check(self.tcx.hir().local_def_id(field.hir_id), item_visibility) + .ty(); + } } } } // Subitems of foreign modules have their own publicity. - hir::ItemKind::ForeignMod { items, .. } => { - for foreign_item in items { - let vis = tcx.visibility(foreign_item.id.def_id); - self.check(foreign_item.id.def_id, vis).generics().predicates().ty(); + DefKind::ForeignMod => { + let item = tcx.hir().item(id); + if let hir::ItemKind::ForeignMod { items, .. } = item.kind { + for foreign_item in items { + let vis = tcx.visibility(foreign_item.id.def_id); + self.check(foreign_item.id.def_id, vis).generics().predicates().ty(); + } } } // Subitems of structs and unions have their own publicity. - hir::ItemKind::Struct(ref struct_def, _) | hir::ItemKind::Union(ref struct_def, _) => { - self.check(item.def_id, item_visibility).generics().predicates(); + DefKind::Struct | DefKind::Union => { + let item = tcx.hir().item(id); + if let hir::ItemKind::Struct(ref struct_def, _) + | hir::ItemKind::Union(ref struct_def, _) = item.kind + { + self.check(item.def_id, item_visibility).generics().predicates(); - for field in struct_def.fields() { - let def_id = tcx.hir().local_def_id(field.hir_id); - let field_visibility = tcx.visibility(def_id); - self.check(def_id, min(item_visibility, field_visibility, tcx)).ty(); + for field in struct_def.fields() { + let def_id = tcx.hir().local_def_id(field.hir_id); + let field_visibility = tcx.visibility(def_id); + self.check(def_id, min(item_visibility, field_visibility, tcx)).ty(); + } } } // An inherent impl is public when its type is public // Subitems of inherent impls have their own publicity. // A trait impl is public when both its type and its trait are public // Subitems of trait impls have inherited publicity. - hir::ItemKind::Impl(ref impl_) => { - let impl_vis = ty::Visibility::of_impl(item.def_id, tcx, &Default::default()); - // check that private components do not appear in the generics or predicates of inherent impls - // this check is intentionally NOT performed for impls of traits, per #90586 - if impl_.of_trait.is_none() { - self.check(item.def_id, impl_vis).generics().predicates(); - } - for impl_item_ref in impl_.items { - let impl_item_vis = if impl_.of_trait.is_none() { - min(tcx.visibility(impl_item_ref.id.def_id), impl_vis, tcx) - } else { - impl_vis - }; - self.check_assoc_item( - impl_item_ref.id.def_id, - impl_item_ref.kind, - impl_item_ref.defaultness, - impl_item_vis, - ); + DefKind::Impl => { + let item = tcx.hir().item(id); + if let hir::ItemKind::Impl(ref impl_) = item.kind { + let impl_vis = ty::Visibility::of_impl(item.def_id, tcx, &Default::default()); + // check that private components do not appear in the generics or predicates of inherent impls + // this check is intentionally NOT performed for impls of traits, per #90586 + if impl_.of_trait.is_none() { + self.check(item.def_id, impl_vis).generics().predicates(); + } + for impl_item_ref in impl_.items { + let impl_item_vis = if impl_.of_trait.is_none() { + min(tcx.visibility(impl_item_ref.id.def_id), impl_vis, tcx) + } else { + impl_vis + }; + self.check_assoc_item( + impl_item_ref.id.def_id, + impl_item_ref.kind, + impl_item_ref.defaultness, + impl_item_vis, + ); + } } } + _ => {} } } } @@ -2069,7 +2080,7 @@ fn check_private_in_public(tcx: TyCtxt<'_>, (): ()) { } // Check for private types and traits in public interfaces. - let mut visitor = PrivateItemsInPublicInterfacesVisitor { + let mut checker = PrivateItemsInPublicInterfacesChecker { tcx, // Only definition IDs are ever searched in `old_error_set_ancestry`, // so we can filter away all non-definition IDs at this point. @@ -2078,5 +2089,8 @@ fn check_private_in_public(tcx: TyCtxt<'_>, (): ()) { .filter_map(|hir_id| tcx.hir().opt_local_def_id(hir_id)) .collect(), }; - tcx.hir().visit_all_item_likes(&mut DeepVisitor::new(&mut visitor)); + + for id in tcx.hir().items() { + checker.check_item(id); + } } From cdba1dcef65545b2634e3bf53b358de789f23675 Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Sat, 7 May 2022 14:45:30 -0400 Subject: [PATCH 14/30] add module_items Signed-off-by: Miguel Guarniz --- compiler/rustc_middle/src/hir/map/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs index 20c8b0bb70f8b..b1de4b8289a63 100644 --- a/compiler/rustc_middle/src/hir/map/mod.rs +++ b/compiler/rustc_middle/src/hir/map/mod.rs @@ -161,6 +161,10 @@ impl<'hir> Map<'hir> { self.tcx.hir_crate_items(()).items.iter().copied() } + pub fn module_items(self, module: LocalDefId) -> impl Iterator + 'hir { + self.tcx.hir_module_items(module).items() + } + pub fn par_for_each_item(self, f: impl Fn(ItemId) + Sync + Send) { par_for_each_in(&self.tcx.hir_crate_items(()).items[..], |id| f(*id)); } From 52d721c3fa87b828b9b311e2c2fe69bf3209cd54 Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Sat, 7 May 2022 14:51:05 -0400 Subject: [PATCH 15/30] remove TestItemNamesVisitor Signed-off-by: Miguel Guarniz --- src/tools/clippy/clippy_utils/src/lib.rs | 58 ++++++++++-------------- 1 file changed, 23 insertions(+), 35 deletions(-) diff --git a/src/tools/clippy/clippy_utils/src/lib.rs b/src/tools/clippy/clippy_utils/src/lib.rs index 98a073d122e31..6db7f247a9925 100644 --- a/src/tools/clippy/clippy_utils/src/lib.rs +++ b/src/tools/clippy/clippy_utils/src/lib.rs @@ -74,11 +74,10 @@ use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, CRATE_DEF_ID}; use rustc_hir::hir_id::{HirIdMap, HirIdSet}; use rustc_hir::intravisit::{walk_expr, FnKind, Visitor}; -use rustc_hir::itemlikevisit::ItemLikeVisitor; use rustc_hir::LangItem::{OptionNone, ResultErr, ResultOk}; use rustc_hir::{ def, Arm, ArrayLen, BindingAnnotation, Block, BlockCheckMode, Body, Constness, Destination, Expr, ExprKind, FnDecl, - ForeignItem, HirId, Impl, ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, MatchSource, + HirId, Impl, ImplItem, ImplItemKind, IsAsync, Item, ItemKind, LangItem, Local, MatchSource, Mutability, Node, Param, Pat, PatKind, Path, PathSegment, PrimTy, QPath, Stmt, StmtKind, TraitItem, TraitItemKind, TraitRef, TyKind, UnOp, }; @@ -2068,35 +2067,6 @@ pub fn is_hir_ty_cfg_dependant(cx: &LateContext<'_>, ty: &hir::Ty<'_>) -> bool { false } -struct TestItemNamesVisitor<'tcx> { - tcx: TyCtxt<'tcx>, - names: Vec, -} - -impl<'hir> ItemLikeVisitor<'hir> for TestItemNamesVisitor<'hir> { - fn visit_item(&mut self, item: &Item<'_>) { - if let ItemKind::Const(ty, _body) = item.kind { - if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind { - // We could also check for the type name `test::TestDescAndFn` - if let Res::Def(DefKind::Struct, _) = path.res { - let has_test_marker = self - .tcx - .hir() - .attrs(item.hir_id()) - .iter() - .any(|a| a.has_name(sym::rustc_test_marker)); - if has_test_marker { - self.names.push(item.ident.name); - } - } - } - } - } - fn visit_trait_item(&mut self, _: &TraitItem<'_>) {} - fn visit_impl_item(&mut self, _: &ImplItem<'_>) {} - fn visit_foreign_item(&mut self, _: &ForeignItem<'_>) {} -} - static TEST_ITEM_NAMES_CACHE: SyncOnceCell>>> = SyncOnceCell::new(); fn with_test_item_names<'tcx>(tcx: TyCtxt<'tcx>, module: LocalDefId, f: impl Fn(&[Symbol]) -> bool) -> bool { @@ -2105,10 +2075,28 @@ fn with_test_item_names<'tcx>(tcx: TyCtxt<'tcx>, module: LocalDefId, f: impl Fn( match map.entry(module) { Entry::Occupied(entry) => f(entry.get()), Entry::Vacant(entry) => { - let mut visitor = TestItemNamesVisitor { tcx, names: Vec::new() }; - tcx.hir().visit_item_likes_in_module(module, &mut visitor); - visitor.names.sort_unstable(); - f(&*entry.insert(visitor.names)) + let mut names = Vec::new(); + for id in tcx.hir().module_items(module) { + if matches!(tcx.def_kind(id.def_id), DefKind::Const) + && let item = tcx.hir().item(id) + && let ItemKind::Const(ty, _body) = item.kind { + if let TyKind::Path(QPath::Resolved(_, path)) = ty.kind { + // We could also check for the type name `test::TestDescAndFn` + if let Res::Def(DefKind::Struct, _) = path.res { + let has_test_marker = tcx + .hir() + .attrs(item.hir_id()) + .iter() + .any(|a| a.has_name(sym::rustc_test_marker)); + if has_test_marker { + names.push(item.ident.name); + } + } + } + } + } + names.sort_unstable(); + f(&*entry.insert(names)) }, } } From 885b90bb349d0e37e0fe26cdb7afadbc1e8c57f0 Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Sat, 7 May 2022 15:03:04 -0400 Subject: [PATCH 16/30] remove DebuggerVisualizerCollector Signed-off-by: Miguel Guarniz --- .../rustc_passes/src/debugger_visualizer.rs | 158 ++++++++---------- 1 file changed, 68 insertions(+), 90 deletions(-) diff --git a/compiler/rustc_passes/src/debugger_visualizer.rs b/compiler/rustc_passes/src/debugger_visualizer.rs index f89092c57a37a..e8a508da52d03 100644 --- a/compiler/rustc_passes/src/debugger_visualizer.rs +++ b/compiler/rustc_passes/src/debugger_visualizer.rs @@ -5,7 +5,6 @@ use rustc_data_structures::fx::FxHashSet; use rustc_expand::base::resolve_path; use rustc_hir as hir; use rustc_hir::def_id::CrateNum; -use rustc_hir::itemlikevisit::ItemLikeVisitor; use rustc_hir::{HirId, Target}; use rustc_middle::ty::query::Providers; use rustc_middle::ty::TyCtxt; @@ -14,96 +13,71 @@ use rustc_span::{sym, DebuggerVisualizerFile, DebuggerVisualizerType}; use std::sync::Arc; -struct DebuggerVisualizerCollector<'tcx> { - debugger_visualizers: FxHashSet, +fn check_for_debugger_visualizer<'tcx>( tcx: TyCtxt<'tcx>, -} - -impl<'v, 'tcx> ItemLikeVisitor<'v> for DebuggerVisualizerCollector<'tcx> { - fn visit_item(&mut self, item: &hir::Item<'_>) { - let target = Target::from_item(item); - match target { - Target::Mod => { - self.check_for_debugger_visualizer(item.hir_id()); - } - _ => {} - } - } - - fn visit_trait_item(&mut self, _: &hir::TraitItem<'_>) {} - - fn visit_impl_item(&mut self, _: &hir::ImplItem<'_>) {} - - fn visit_foreign_item(&mut self, _: &hir::ForeignItem<'_>) {} -} - -impl<'tcx> DebuggerVisualizerCollector<'tcx> { - fn new(tcx: TyCtxt<'tcx>) -> DebuggerVisualizerCollector<'tcx> { - DebuggerVisualizerCollector { tcx, debugger_visualizers: FxHashSet::default() } - } - - fn check_for_debugger_visualizer(&mut self, hir_id: HirId) { - let attrs = self.tcx.hir().attrs(hir_id); - for attr in attrs { - if attr.has_name(sym::debugger_visualizer) { - let list = match attr.meta_item_list() { - Some(list) => list, - _ => continue, - }; - - let meta_item = match list.len() { - 1 => match list[0].meta_item() { - Some(meta_item) => meta_item, - _ => continue, - }, + hir_id: HirId, + debugger_visualizers: &mut FxHashSet +) { + let attrs = tcx.hir().attrs(hir_id); + for attr in attrs { + if attr.has_name(sym::debugger_visualizer) { + let list = match attr.meta_item_list() { + Some(list) => list, + _ => continue, + }; + + let meta_item = match list.len() { + 1 => match list[0].meta_item() { + Some(meta_item) => meta_item, _ => continue, - }; - - let file = match (meta_item.name_or_empty(), meta_item.value_str()) { - (sym::natvis_file, Some(value)) => { - match resolve_path(&self.tcx.sess.parse_sess, value.as_str(), attr.span) { - Ok(file) => file, - Err(mut err) => { - err.emit(); - continue; - } + }, + _ => continue, + }; + + let file = match (meta_item.name_or_empty(), meta_item.value_str()) { + (sym::natvis_file, Some(value)) => { + match resolve_path(&tcx.sess.parse_sess, value.as_str(), attr.span) { + Ok(file) => file, + Err(mut err) => { + err.emit(); + continue; } } - (_, _) => continue, + } + (_, _) => continue, + }; + + if file.is_file() { + let contents = match std::fs::read(&file) { + Ok(contents) => contents, + Err(err) => { + tcx + .sess + .struct_span_err( + attr.span, + &format!( + "Unable to read contents of file `{}`. {}", + file.display(), + err + ), + ) + .emit(); + continue; + } }; - if file.is_file() { - let contents = match std::fs::read(&file) { - Ok(contents) => contents, - Err(err) => { - self.tcx - .sess - .struct_span_err( - attr.span, - &format!( - "Unable to read contents of file `{}`. {}", - file.display(), - err - ), - ) - .emit(); - continue; - } - }; - - self.debugger_visualizers.insert(DebuggerVisualizerFile::new( - Arc::from(contents), - DebuggerVisualizerType::Natvis, - )); - } else { - self.tcx - .sess - .struct_span_err( - attr.span, - &format!("{} is not a valid file", file.display()), - ) - .emit(); - } + debugger_visualizers.insert(DebuggerVisualizerFile::new( + Arc::from(contents), + DebuggerVisualizerType::Natvis, + )); + } else { + tcx + .sess + .struct_span_err( + attr.span, + &format!("{} is not a valid file", file.display()), + ) + .emit(); } } } @@ -114,17 +88,21 @@ fn debugger_visualizers<'tcx>(tcx: TyCtxt<'tcx>, cnum: CrateNum) -> Vec>(); // Sort the visualizers so we always get a deterministic query result. From 0b7dd95475d0150719cd859dbd6e2fe7092d83bd Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Sat, 7 May 2022 15:27:50 -0400 Subject: [PATCH 17/30] remove HirVisitor Signed-off-by: Miguel Guarniz --- .../rustc_passes/src/debugger_visualizer.rs | 14 ++--- .../obtain-borrowck/driver.rs | 60 +++++++++---------- 2 files changed, 31 insertions(+), 43 deletions(-) diff --git a/compiler/rustc_passes/src/debugger_visualizer.rs b/compiler/rustc_passes/src/debugger_visualizer.rs index e8a508da52d03..dc63a2d8666bd 100644 --- a/compiler/rustc_passes/src/debugger_visualizer.rs +++ b/compiler/rustc_passes/src/debugger_visualizer.rs @@ -16,7 +16,7 @@ use std::sync::Arc; fn check_for_debugger_visualizer<'tcx>( tcx: TyCtxt<'tcx>, hir_id: HirId, - debugger_visualizers: &mut FxHashSet + debugger_visualizers: &mut FxHashSet, ) { let attrs = tcx.hir().attrs(hir_id); for attr in attrs { @@ -51,8 +51,7 @@ fn check_for_debugger_visualizer<'tcx>( let contents = match std::fs::read(&file) { Ok(contents) => contents, Err(err) => { - tcx - .sess + tcx.sess .struct_span_err( attr.span, &format!( @@ -71,12 +70,8 @@ fn check_for_debugger_visualizer<'tcx>( DebuggerVisualizerType::Natvis, )); } else { - tcx - .sess - .struct_span_err( - attr.span, - &format!("{} is not a valid file", file.display()), - ) + tcx.sess + .struct_span_err(attr.span, &format!("{} is not a valid file", file.display())) .emit(); } } @@ -101,7 +96,6 @@ fn debugger_visualizers<'tcx>(tcx: TyCtxt<'tcx>, cnum: CrateNum) -> Vec>(); diff --git a/src/test/run-make-fulldeps/obtain-borrowck/driver.rs b/src/test/run-make-fulldeps/obtain-borrowck/driver.rs index 86e6d9e756c51..c3b82aa853c78 100644 --- a/src/test/run-make-fulldeps/obtain-borrowck/driver.rs +++ b/src/test/run-make-fulldeps/obtain-borrowck/driver.rs @@ -21,7 +21,7 @@ extern crate rustc_session; use rustc_borrowck::consumers::BodyWithBorrowckFacts; use rustc_driver::Compilation; use rustc_hir::def_id::LocalDefId; -use rustc_hir::itemlikevisit::ItemLikeVisitor; +use rustc_hir::def::DefKind; use rustc_interface::interface::Compiler; use rustc_interface::{Config, Queries}; use rustc_middle::ty::query::query_values::mir_borrowck; @@ -65,11 +65,34 @@ impl rustc_driver::Callbacks for CompilerCalls { queries.global_ctxt().unwrap().peek_mut().enter(|tcx| { // Collect definition ids of MIR bodies. let hir = tcx.hir(); - let mut visitor = HirVisitor { bodies: Vec::new() }; - hir.visit_all_item_likes(&mut visitor); + let mut bodies = Vec::new(); + + let crate_items = tcx.hir_crate_items(()); + for id in crate_items.items() { + if matches!(tcx.def_kind(id.def_id), DefKind::Fn) { + bodies.push(id.def_id); + } + } + + for id in crate_items.trait_items() { + if matches!(tcx.def_kind(id.def_id), DefKind::AssocFn) { + let trait_item = hir.trait_item(id); + if let rustc_hir::TraitItemKind::Fn(_, trait_fn) = &trait_item.kind { + if let rustc_hir::TraitFn::Provided(_) = trait_fn { + bodies.push(trait_item.def_id); + } + } + } + } + + for id in crate_items.impl_items() { + if matches!(tcx.def_kind(id.def_id), DefKind::AssocFn) { + bodies.push(id.def_id); + } + } // Trigger borrow checking of all bodies. - for def_id in visitor.bodies { + for def_id in bodies { let _ = tcx.optimized_mir(def_id); } @@ -121,35 +144,6 @@ fn mir_borrowck<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId) -> mir_borrowck<'tc original_mir_borrowck(tcx, def_id) } -/// Visitor that collects all body definition ids mentioned in the program. -struct HirVisitor { - bodies: Vec, -} - -impl<'tcx> ItemLikeVisitor<'tcx> for HirVisitor { - fn visit_item(&mut self, item: &rustc_hir::Item) { - if let rustc_hir::ItemKind::Fn(..) = item.kind { - self.bodies.push(item.def_id); - } - } - - fn visit_trait_item(&mut self, trait_item: &rustc_hir::TraitItem) { - if let rustc_hir::TraitItemKind::Fn(_, trait_fn) = &trait_item.kind { - if let rustc_hir::TraitFn::Provided(_) = trait_fn { - self.bodies.push(trait_item.def_id); - } - } - } - - fn visit_impl_item(&mut self, impl_item: &rustc_hir::ImplItem) { - if let rustc_hir::ImplItemKind::Fn(..) = impl_item.kind { - self.bodies.push(impl_item.def_id); - } - } - - fn visit_foreign_item(&mut self, _foreign_item: &rustc_hir::ForeignItem) {} -} - /// Pull MIR bodies stored in the thread-local. fn get_bodies<'tcx>(tcx: TyCtxt<'tcx>) -> Vec<(String, BodyWithBorrowckFacts<'tcx>)> { MIR_BODIES.with(|state| { From 93616dd5396ecd0fe0f48badc60157f74840d19c Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Sat, 7 May 2022 15:43:10 -0400 Subject: [PATCH 18/30] remove ItemLikeVisitor and DeepVisitor Signed-off-by: Miguel Guarniz --- compiler/rustc_hir/src/intravisit.rs | 43 +--------------- compiler/rustc_hir/src/itemlikevisit.rs | 50 ------------------- .../rustc_incremental/src/assert_dep_graph.rs | 2 +- compiler/rustc_metadata/src/rmeta/encoder.rs | 2 +- compiler/rustc_middle/src/hir/map/mod.rs | 5 +- compiler/rustc_mir_transform/src/lib.rs | 2 +- compiler/rustc_passes/src/check_attr.rs | 2 +- compiler/rustc_passes/src/intrinsicck.rs | 2 +- compiler/rustc_passes/src/liveness.rs | 2 +- compiler/rustc_passes/src/loops.rs | 2 +- compiler/rustc_passes/src/naked_functions.rs | 5 +- compiler/rustc_passes/src/stability.rs | 4 +- compiler/rustc_typeck/src/collect.rs | 5 +- src/librustdoc/scrape_examples.rs | 2 +- 14 files changed, 15 insertions(+), 113 deletions(-) diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index 977c0eb3cd2bc..e6d8ad1c375cf 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -32,43 +32,12 @@ //! example generator inference, and possibly also HIR borrowck. use crate::hir::*; -use crate::itemlikevisit::{ItemLikeVisitor, ParItemLikeVisitor}; +use crate::itemlikevisit::ParItemLikeVisitor; use rustc_ast::walk_list; use rustc_ast::{Attribute, Label}; use rustc_span::symbol::{Ident, Symbol}; use rustc_span::Span; -pub struct DeepVisitor<'v, V> { - visitor: &'v mut V, -} - -impl<'v, V> DeepVisitor<'v, V> { - pub fn new(base: &'v mut V) -> Self { - DeepVisitor { visitor: base } - } -} - -impl<'v, 'hir, V> ItemLikeVisitor<'hir> for DeepVisitor<'v, V> -where - V: Visitor<'hir>, -{ - fn visit_item(&mut self, item: &'hir Item<'hir>) { - self.visitor.visit_item(item); - } - - fn visit_trait_item(&mut self, trait_item: &'hir TraitItem<'hir>) { - self.visitor.visit_trait_item(trait_item); - } - - fn visit_impl_item(&mut self, impl_item: &'hir ImplItem<'hir>) { - self.visitor.visit_impl_item(impl_item); - } - - fn visit_foreign_item(&mut self, foreign_item: &'hir ForeignItem<'hir>) { - self.visitor.visit_foreign_item(foreign_item); - } -} - pub trait IntoVisitor<'hir> { type Visitor: Visitor<'hir>; fn into_visitor(&self) -> Self::Visitor; @@ -315,16 +284,6 @@ pub trait Visitor<'v>: Sized { walk_body(self, b); } - /// When invoking `visit_all_item_likes()`, you need to supply an - /// item-like visitor. This method converts an "intra-visit" - /// visitor into an item-like visitor that walks the entire tree. - /// If you use this, you probably don't want to process the - /// contents of nested item-like things, since the outer loop will - /// visit them as well. - fn as_deep_visitor(&mut self) -> DeepVisitor<'_, Self> { - DeepVisitor::new(self) - } - /////////////////////////////////////////////////////////////////////////// fn visit_id(&mut self, _hir_id: HirId) { diff --git a/compiler/rustc_hir/src/itemlikevisit.rs b/compiler/rustc_hir/src/itemlikevisit.rs index b2c6ca1354f10..a490268dc9f94 100644 --- a/compiler/rustc_hir/src/itemlikevisit.rs +++ b/compiler/rustc_hir/src/itemlikevisit.rs @@ -1,55 +1,5 @@ use super::{ForeignItem, ImplItem, Item, TraitItem}; -/// The "item-like visitor" defines only the top-level methods -/// that can be invoked by `Crate::visit_all_item_likes()`. Whether -/// this trait is the right one to implement will depend on the -/// overall pattern you need. Here are the three available patterns, -/// in roughly the order of desirability: -/// -/// 1. **Shallow visit**: Get a simple callback for every item (or item-like thing) in the HIR. -/// - Example: find all items with a `#[foo]` attribute on them. -/// - How: Implement `ItemLikeVisitor` and call `tcx.hir().visit_all_item_likes()`. -/// - Pro: Efficient; just walks the lists of item-like things, not the nodes themselves. -/// - Con: Don't get information about nesting -/// - Con: Don't have methods for specific bits of HIR, like "on -/// every expr, do this". -/// 2. **Deep visit**: Want to scan for specific kinds of HIR nodes within -/// an item, but don't care about how item-like things are nested -/// within one another. -/// - Example: Examine each expression to look for its type and do some check or other. -/// - How: Implement `intravisit::Visitor` and override the `NestedFilter` type to -/// `nested_filter::OnlyBodies` (and implement `nested_visit_map`), and use -/// `tcx.hir().visit_all_item_likes(&mut visitor.as_deep_visitor())`. Within your -/// `intravisit::Visitor` impl, implement methods like `visit_expr()` (don't forget to invoke -/// `intravisit::walk_expr()` to keep walking the subparts). -/// - Pro: Visitor methods for any kind of HIR node, not just item-like things. -/// - Pro: Integrates well into dependency tracking. -/// - Con: Don't get information about nesting between items -/// 3. **Nested visit**: Want to visit the whole HIR and you care about the nesting between -/// item-like things. -/// - Example: Lifetime resolution, which wants to bring lifetimes declared on the -/// impl into scope while visiting the impl-items, and then back out again. -/// - How: Implement `intravisit::Visitor` and override the `NestedFilter` type to -/// `nested_filter::All` (and implement `nested_visit_map`). Walk your crate with -/// `tcx.hir().walk_toplevel_module(visitor)` invoked on `tcx.hir().krate()`. -/// - Pro: Visitor methods for any kind of HIR node, not just item-like things. -/// - Pro: Preserves nesting information -/// - Con: Does not integrate well into dependency tracking. -/// -/// Note: the methods of `ItemLikeVisitor` intentionally have no -/// defaults, so that as we expand the list of item-like things, we -/// revisit the various visitors to see if they need to change. This -/// is harder to do with `intravisit::Visitor`, so when you add a new -/// `visit_nested_foo()` method, it is recommended that you search for -/// existing `fn visit_nested` methods to see where changes are -/// needed. -pub trait ItemLikeVisitor<'hir> { - fn visit_item(&mut self, item: &'hir Item<'hir>); - fn visit_trait_item(&mut self, trait_item: &'hir TraitItem<'hir>); - fn visit_impl_item(&mut self, impl_item: &'hir ImplItem<'hir>); - fn visit_foreign_item(&mut self, foreign_item: &'hir ForeignItem<'hir>); -} - /// A parallel variant of `ItemLikeVisitor`. pub trait ParItemLikeVisitor<'hir> { fn visit_item(&self, item: &'hir Item<'hir>); diff --git a/compiler/rustc_incremental/src/assert_dep_graph.rs b/compiler/rustc_incremental/src/assert_dep_graph.rs index f69ae8ebe410f..7cddd10203e2e 100644 --- a/compiler/rustc_incremental/src/assert_dep_graph.rs +++ b/compiler/rustc_incremental/src/assert_dep_graph.rs @@ -75,7 +75,7 @@ pub fn assert_dep_graph(tcx: TyCtxt<'_>) { let mut visitor = IfThisChanged { tcx, if_this_changed: vec![], then_this_would_need: vec![] }; visitor.process_attrs(hir::CRATE_HIR_ID); - tcx.hir().visit_all_item_likes(&mut visitor.as_deep_visitor()); + tcx.hir().visit_all_item_likes(&mut visitor); (visitor.if_this_changed, visitor.then_this_would_need) }; diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index fe06f879775ee..fb408f35398fa 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -452,7 +452,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { return; } - self.tcx.hir().visit_all_item_likes(&mut self.as_deep_visitor()); + self.tcx.hir().visit_all_item_likes(self); } fn encode_def_path_table(&mut self) { diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs index b1de4b8289a63..7b444c7c84068 100644 --- a/compiler/rustc_middle/src/hir/map/mod.rs +++ b/compiler/rustc_middle/src/hir/map/mod.rs @@ -9,7 +9,6 @@ use rustc_hir::def::{DefKind, Res}; use rustc_hir::def_id::{CrateNum, DefId, LocalDefId, CRATE_DEF_ID, LOCAL_CRATE}; use rustc_hir::definitions::{DefKey, DefPath, DefPathHash}; use rustc_hir::intravisit::{self, Visitor}; -use rustc_hir::itemlikevisit::ItemLikeVisitor; use rustc_hir::*; use rustc_index::vec::Idx; use rustc_middle::hir::nested_filter; @@ -616,7 +615,7 @@ impl<'hir> Map<'hir> { /// visitor and then call `intravisit::walk_crate` instead. pub fn visit_all_item_likes(self, visitor: &mut V) where - V: itemlikevisit::ItemLikeVisitor<'hir>, + V: Visitor<'hir>, { let krate = self.krate(); for owner in krate.owners.iter().filter_map(|i| i.as_owner()) { @@ -649,7 +648,7 @@ impl<'hir> Map<'hir> { pub fn visit_item_likes_in_module(self, module: LocalDefId, visitor: &mut V) where - V: ItemLikeVisitor<'hir>, + V: Visitor<'hir>, { let module = self.tcx.hir_module_items(module); diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 40cc6dafe6177..40dbdbc687a62 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -170,7 +170,7 @@ fn mir_keys(tcx: TyCtxt<'_>, (): ()) -> FxIndexSet { intravisit::walk_struct_def(self, v) } } - tcx.hir().visit_all_item_likes(&mut GatherCtors { tcx, set: &mut set }.as_deep_visitor()); + tcx.hir().visit_all_item_likes(&mut GatherCtors { tcx, set: &mut set }); set } diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index 8d207e4e1a924..0aaaf3df18530 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -2384,7 +2384,7 @@ fn check_non_exported_macro_for_invalid_attrs(tcx: TyCtxt<'_>, item: &Item<'_>) fn check_mod_attrs(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { let check_attr_visitor = &mut CheckAttrVisitor { tcx }; - tcx.hir().visit_item_likes_in_module(module_def_id, &mut check_attr_visitor.as_deep_visitor()); + tcx.hir().visit_item_likes_in_module(module_def_id, check_attr_visitor); if module_def_id.is_top_level_module() { check_attr_visitor.check_attributes(CRATE_HIR_ID, DUMMY_SP, Target::Mod, None); check_invalid_crate_level_attr(tcx, tcx.hir().krate_attrs()); diff --git a/compiler/rustc_passes/src/intrinsicck.rs b/compiler/rustc_passes/src/intrinsicck.rs index 7028fc4412648..8bb6b08e94091 100644 --- a/compiler/rustc_passes/src/intrinsicck.rs +++ b/compiler/rustc_passes/src/intrinsicck.rs @@ -17,7 +17,7 @@ use rustc_target::asm::{InlineAsmRegOrRegClass, InlineAsmType}; use rustc_target::spec::abi::Abi::RustIntrinsic; fn check_mod_intrinsics(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { - tcx.hir().visit_item_likes_in_module(module_def_id, &mut ItemVisitor { tcx }.as_deep_visitor()); + tcx.hir().visit_item_likes_in_module(module_def_id, &mut ItemVisitor { tcx }); } pub fn provide(providers: &mut Providers) { diff --git a/compiler/rustc_passes/src/liveness.rs b/compiler/rustc_passes/src/liveness.rs index 9eba7fb0811c6..bd7bccf6ef154 100644 --- a/compiler/rustc_passes/src/liveness.rs +++ b/compiler/rustc_passes/src/liveness.rs @@ -140,7 +140,7 @@ fn live_node_kind_to_string(lnk: LiveNodeKind, tcx: TyCtxt<'_>) -> String { } fn check_mod_liveness(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { - tcx.hir().visit_item_likes_in_module(module_def_id, &mut IrMaps::new(tcx).as_deep_visitor()); + tcx.hir().visit_item_likes_in_module(module_def_id, &mut IrMaps::new(tcx)); } pub fn provide(providers: &mut Providers) { diff --git a/compiler/rustc_passes/src/loops.rs b/compiler/rustc_passes/src/loops.rs index 02b09daf0a41e..4f904dd087489 100644 --- a/compiler/rustc_passes/src/loops.rs +++ b/compiler/rustc_passes/src/loops.rs @@ -33,7 +33,7 @@ struct CheckLoopVisitor<'a, 'hir> { fn check_mod_loops(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { tcx.hir().visit_item_likes_in_module( module_def_id, - &mut CheckLoopVisitor { sess: &tcx.sess, hir_map: tcx.hir(), cx: Normal }.as_deep_visitor(), + &mut CheckLoopVisitor { sess: &tcx.sess, hir_map: tcx.hir(), cx: Normal }, ); } diff --git a/compiler/rustc_passes/src/naked_functions.rs b/compiler/rustc_passes/src/naked_functions.rs index e85720952da7a..a8f7d3df2dedf 100644 --- a/compiler/rustc_passes/src/naked_functions.rs +++ b/compiler/rustc_passes/src/naked_functions.rs @@ -14,10 +14,7 @@ use rustc_span::Span; use rustc_target::spec::abi::Abi; fn check_mod_naked_functions(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { - tcx.hir().visit_item_likes_in_module( - module_def_id, - &mut CheckNakedFunctions { tcx }.as_deep_visitor(), - ); + tcx.hir().visit_item_likes_in_module(module_def_id, &mut CheckNakedFunctions { tcx }); } crate fn provide(providers: &mut Providers) { diff --git a/compiler/rustc_passes/src/stability.rs b/compiler/rustc_passes/src/stability.rs index e1bc248971ae2..4cd5cf0cc23a7 100644 --- a/compiler/rustc_passes/src/stability.rs +++ b/compiler/rustc_passes/src/stability.rs @@ -661,7 +661,7 @@ fn stability_index(tcx: TyCtxt<'_>, (): ()) -> Index { /// Cross-references the feature names of unstable APIs with enabled /// features and possibly prints errors. fn check_mod_unstable_api_usage(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { - tcx.hir().visit_item_likes_in_module(module_def_id, &mut Checker { tcx }.as_deep_visitor()); + tcx.hir().visit_item_likes_in_module(module_def_id, &mut Checker { tcx }); } pub(crate) fn provide(providers: &mut Providers) { @@ -837,7 +837,7 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) { let mut missing = MissingStabilityAnnotations { tcx, access_levels }; missing.check_missing_stability(CRATE_DEF_ID, tcx.hir().span(CRATE_HIR_ID)); tcx.hir().walk_toplevel_module(&mut missing); - tcx.hir().visit_all_item_likes(&mut missing.as_deep_visitor()); + tcx.hir().visit_all_item_likes(&mut missing); } let declared_lang_features = &tcx.features().declared_lang_features; diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index cda817dee1e1e..81a4f6e547deb 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -59,10 +59,7 @@ struct OnlySelfBounds(bool); // Main entry point fn collect_mod_item_types(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { - tcx.hir().visit_item_likes_in_module( - module_def_id, - &mut CollectItemTypesVisitor { tcx }.as_deep_visitor(), - ); + tcx.hir().visit_item_likes_in_module(module_def_id, &mut CollectItemTypesVisitor { tcx }); } pub fn provide(providers: &mut Providers) { diff --git a/src/librustdoc/scrape_examples.rs b/src/librustdoc/scrape_examples.rs index 0da490f3cd6c8..fc07f49f150ba 100644 --- a/src/librustdoc/scrape_examples.rs +++ b/src/librustdoc/scrape_examples.rs @@ -303,7 +303,7 @@ crate fn run( // Run call-finder on all items let mut calls = FxHashMap::default(); let mut finder = FindCalls { calls: &mut calls, tcx, map: tcx.hir(), cx, target_crates }; - tcx.hir().visit_all_item_likes(&mut finder.as_deep_visitor()); + tcx.hir().visit_all_item_likes(&mut finder); // Sort call locations within a given file in document order for fn_calls in calls.values_mut() { From 7e44078e9a12179b17a357da7e06c353036f68b9 Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Sat, 7 May 2022 16:22:52 -0400 Subject: [PATCH 19/30] update comments about visitor strategy Signed-off-by: Miguel Guarniz --- compiler/rustc_hir/src/intravisit.rs | 37 ++++++++++++++++++++++++++-- 1 file changed, 35 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index e6d8ad1c375cf..338b654a391a2 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -1,7 +1,40 @@ //! HIR walker for walking the contents of nodes. //! -//! **For an overview of the visitor strategy, see the docs on the -//! `super::itemlikevisit::ItemLikeVisitor` trait.** +//! Here are the three available patterns for the visitor strategy, +//! in roughly the order of desirability: +//! +//! 1. **Shallow visit**: Get a simple callback for every item (or item-like thing) in the HIR. +//! - Example: find all items with a `#[foo]` attribute on them. +//! - How: Use the `hir_crate_items` or `hir_module_items` query to traverse over item-like ids +//! (ItemId, TraitItemId, etc.) and use tcx.def_kind and `tcx.hir().item*(id)` to filter and +//! access actual item-like thing, respectively. +//! - Pro: Efficient; just walks the lists of item ids and gives users control whether to access +//! the hir_owners themselves or not. +//! - Con: Don't get information about nesting +//! - Con: Don't have methods for specific bits of HIR, like "on +//! every expr, do this". +//! 2. **Deep visit**: Want to scan for specific kinds of HIR nodes within +//! an item, but don't care about how item-like things are nested +//! within one another. +//! - Example: Examine each expression to look for its type and do some check or other. +//! - How: Implement `intravisit::Visitor` and override the `NestedFilter` type to +//! `nested_filter::OnlyBodies` (and implement `nested_visit_map`), and use +//! `tcx.hir().visit_all_item_likes(&mut visitor)`. Within your +//! `intravisit::Visitor` impl, implement methods like `visit_expr()` (don't forget to invoke +//! `intravisit::walk_expr()` to keep walking the subparts). +//! - Pro: Visitor methods for any kind of HIR node, not just item-like things. +//! - Pro: Integrates well into dependency tracking. +//! - Con: Don't get information about nesting between items +//! 3. **Nested visit**: Want to visit the whole HIR and you care about the nesting between +//! item-like things. +//! - Example: Lifetime resolution, which wants to bring lifetimes declared on the +//! impl into scope while visiting the impl-items, and then back out again. +//! - How: Implement `intravisit::Visitor` and override the `NestedFilter` type to +//! `nested_filter::All` (and implement `nested_visit_map`). Walk your crate with +//! `tcx.hir().walk_toplevel_module(visitor)` invoked on `tcx.hir().krate()`. +//! - Pro: Visitor methods for any kind of HIR node, not just item-like things. +//! - Pro: Preserves nesting information +//! - Con: Does not integrate well into dependency tracking. //! //! If you have decided to use this visitor, here are some general //! notes on how to do so: From f975d0511690c1dabe2e9df0dde189b90bec587f Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Sat, 7 May 2022 16:35:38 -0400 Subject: [PATCH 20/30] rename visit item-like methods Signed-off-by: Miguel Guarniz --- compiler/rustc_hir/src/intravisit.rs | 2 +- .../rustc_incremental/src/assert_dep_graph.rs | 2 +- compiler/rustc_metadata/src/rmeta/encoder.rs | 2 +- compiler/rustc_middle/src/hir/map/mod.rs | 19 +++++++++++-------- compiler/rustc_mir_transform/src/lib.rs | 2 +- compiler/rustc_passes/src/check_attr.rs | 2 +- compiler/rustc_passes/src/intrinsicck.rs | 2 +- compiler/rustc_passes/src/liveness.rs | 2 +- compiler/rustc_passes/src/loops.rs | 2 +- compiler/rustc_passes/src/naked_functions.rs | 2 +- compiler/rustc_passes/src/stability.rs | 4 ++-- compiler/rustc_typeck/src/collect.rs | 2 +- 12 files changed, 23 insertions(+), 20 deletions(-) diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index 338b654a391a2..1254d3a161856 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -19,7 +19,7 @@ //! - Example: Examine each expression to look for its type and do some check or other. //! - How: Implement `intravisit::Visitor` and override the `NestedFilter` type to //! `nested_filter::OnlyBodies` (and implement `nested_visit_map`), and use -//! `tcx.hir().visit_all_item_likes(&mut visitor)`. Within your +//! `tcx.hir().deep_visit_all_item_likes(&mut visitor)`. Within your //! `intravisit::Visitor` impl, implement methods like `visit_expr()` (don't forget to invoke //! `intravisit::walk_expr()` to keep walking the subparts). //! - Pro: Visitor methods for any kind of HIR node, not just item-like things. diff --git a/compiler/rustc_incremental/src/assert_dep_graph.rs b/compiler/rustc_incremental/src/assert_dep_graph.rs index 7cddd10203e2e..a89b9eafaa62d 100644 --- a/compiler/rustc_incremental/src/assert_dep_graph.rs +++ b/compiler/rustc_incremental/src/assert_dep_graph.rs @@ -75,7 +75,7 @@ pub fn assert_dep_graph(tcx: TyCtxt<'_>) { let mut visitor = IfThisChanged { tcx, if_this_changed: vec![], then_this_would_need: vec![] }; visitor.process_attrs(hir::CRATE_HIR_ID); - tcx.hir().visit_all_item_likes(&mut visitor); + tcx.hir().deep_visit_all_item_likes(&mut visitor); (visitor.if_this_changed, visitor.then_this_would_need) }; diff --git a/compiler/rustc_metadata/src/rmeta/encoder.rs b/compiler/rustc_metadata/src/rmeta/encoder.rs index fb408f35398fa..ebf6a5521702b 100644 --- a/compiler/rustc_metadata/src/rmeta/encoder.rs +++ b/compiler/rustc_metadata/src/rmeta/encoder.rs @@ -452,7 +452,7 @@ impl<'a, 'tcx> EncodeContext<'a, 'tcx> { return; } - self.tcx.hir().visit_all_item_likes(self); + self.tcx.hir().deep_visit_all_item_likes(self); } fn encode_def_path_table(&mut self) { diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs index 7b444c7c84068..b0a0131536bc0 100644 --- a/compiler/rustc_middle/src/hir/map/mod.rs +++ b/compiler/rustc_middle/src/hir/map/mod.rs @@ -606,14 +606,14 @@ impl<'hir> Map<'hir> { } /// Visits all items in the crate in some deterministic (but - /// unspecified) order. If you just need to process every item, - /// but don't care about nesting, this method is the best choice. + /// unspecified) order. If you need to process every item, + /// and care about nesting -- usually because your algorithm + /// follows lexical scoping rules -- then this method is the best choice. + /// If you don't care about nesting, you should use the `tcx.hir_crate_items()` query + /// or `items()` instead. /// - /// If you do care about nesting -- usually because your algorithm - /// follows lexical scoping rules -- then you want a different - /// approach. You should override `visit_nested_item` in your - /// visitor and then call `intravisit::walk_crate` instead. - pub fn visit_all_item_likes(self, visitor: &mut V) + /// Please see the notes in `intravisit.rs` for more information. + pub fn deep_visit_all_item_likes(self, visitor: &mut V) where V: Visitor<'hir>, { @@ -646,7 +646,10 @@ impl<'hir> Map<'hir> { }) } - pub fn visit_item_likes_in_module(self, module: LocalDefId, visitor: &mut V) + /// If you don't care about nesting, you should use the + /// `tcx.hir_module_items()` query or `module_items()` instead. + /// Please see notes in `deep_visit_all_item_likes`. + pub fn deep_visit_item_likes_in_module(self, module: LocalDefId, visitor: &mut V) where V: Visitor<'hir>, { diff --git a/compiler/rustc_mir_transform/src/lib.rs b/compiler/rustc_mir_transform/src/lib.rs index 40dbdbc687a62..d08382700a80a 100644 --- a/compiler/rustc_mir_transform/src/lib.rs +++ b/compiler/rustc_mir_transform/src/lib.rs @@ -170,7 +170,7 @@ fn mir_keys(tcx: TyCtxt<'_>, (): ()) -> FxIndexSet { intravisit::walk_struct_def(self, v) } } - tcx.hir().visit_all_item_likes(&mut GatherCtors { tcx, set: &mut set }); + tcx.hir().deep_visit_all_item_likes(&mut GatherCtors { tcx, set: &mut set }); set } diff --git a/compiler/rustc_passes/src/check_attr.rs b/compiler/rustc_passes/src/check_attr.rs index 0aaaf3df18530..3aa1d5f20493d 100644 --- a/compiler/rustc_passes/src/check_attr.rs +++ b/compiler/rustc_passes/src/check_attr.rs @@ -2384,7 +2384,7 @@ fn check_non_exported_macro_for_invalid_attrs(tcx: TyCtxt<'_>, item: &Item<'_>) fn check_mod_attrs(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { let check_attr_visitor = &mut CheckAttrVisitor { tcx }; - tcx.hir().visit_item_likes_in_module(module_def_id, check_attr_visitor); + tcx.hir().deep_visit_item_likes_in_module(module_def_id, check_attr_visitor); if module_def_id.is_top_level_module() { check_attr_visitor.check_attributes(CRATE_HIR_ID, DUMMY_SP, Target::Mod, None); check_invalid_crate_level_attr(tcx, tcx.hir().krate_attrs()); diff --git a/compiler/rustc_passes/src/intrinsicck.rs b/compiler/rustc_passes/src/intrinsicck.rs index 8bb6b08e94091..95f6f7ef864a2 100644 --- a/compiler/rustc_passes/src/intrinsicck.rs +++ b/compiler/rustc_passes/src/intrinsicck.rs @@ -17,7 +17,7 @@ use rustc_target::asm::{InlineAsmRegOrRegClass, InlineAsmType}; use rustc_target::spec::abi::Abi::RustIntrinsic; fn check_mod_intrinsics(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { - tcx.hir().visit_item_likes_in_module(module_def_id, &mut ItemVisitor { tcx }); + tcx.hir().deep_visit_item_likes_in_module(module_def_id, &mut ItemVisitor { tcx }); } pub fn provide(providers: &mut Providers) { diff --git a/compiler/rustc_passes/src/liveness.rs b/compiler/rustc_passes/src/liveness.rs index bd7bccf6ef154..ce5253adf10d8 100644 --- a/compiler/rustc_passes/src/liveness.rs +++ b/compiler/rustc_passes/src/liveness.rs @@ -140,7 +140,7 @@ fn live_node_kind_to_string(lnk: LiveNodeKind, tcx: TyCtxt<'_>) -> String { } fn check_mod_liveness(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { - tcx.hir().visit_item_likes_in_module(module_def_id, &mut IrMaps::new(tcx)); + tcx.hir().deep_visit_item_likes_in_module(module_def_id, &mut IrMaps::new(tcx)); } pub fn provide(providers: &mut Providers) { diff --git a/compiler/rustc_passes/src/loops.rs b/compiler/rustc_passes/src/loops.rs index 4f904dd087489..e0dac09870df7 100644 --- a/compiler/rustc_passes/src/loops.rs +++ b/compiler/rustc_passes/src/loops.rs @@ -31,7 +31,7 @@ struct CheckLoopVisitor<'a, 'hir> { } fn check_mod_loops(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { - tcx.hir().visit_item_likes_in_module( + tcx.hir().deep_visit_item_likes_in_module( module_def_id, &mut CheckLoopVisitor { sess: &tcx.sess, hir_map: tcx.hir(), cx: Normal }, ); diff --git a/compiler/rustc_passes/src/naked_functions.rs b/compiler/rustc_passes/src/naked_functions.rs index a8f7d3df2dedf..5d7768c8240de 100644 --- a/compiler/rustc_passes/src/naked_functions.rs +++ b/compiler/rustc_passes/src/naked_functions.rs @@ -14,7 +14,7 @@ use rustc_span::Span; use rustc_target::spec::abi::Abi; fn check_mod_naked_functions(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { - tcx.hir().visit_item_likes_in_module(module_def_id, &mut CheckNakedFunctions { tcx }); + tcx.hir().deep_visit_item_likes_in_module(module_def_id, &mut CheckNakedFunctions { tcx }); } crate fn provide(providers: &mut Providers) { diff --git a/compiler/rustc_passes/src/stability.rs b/compiler/rustc_passes/src/stability.rs index 4cd5cf0cc23a7..58195fce28197 100644 --- a/compiler/rustc_passes/src/stability.rs +++ b/compiler/rustc_passes/src/stability.rs @@ -661,7 +661,7 @@ fn stability_index(tcx: TyCtxt<'_>, (): ()) -> Index { /// Cross-references the feature names of unstable APIs with enabled /// features and possibly prints errors. fn check_mod_unstable_api_usage(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { - tcx.hir().visit_item_likes_in_module(module_def_id, &mut Checker { tcx }); + tcx.hir().deep_visit_item_likes_in_module(module_def_id, &mut Checker { tcx }); } pub(crate) fn provide(providers: &mut Providers) { @@ -837,7 +837,7 @@ pub fn check_unused_or_stable_features(tcx: TyCtxt<'_>) { let mut missing = MissingStabilityAnnotations { tcx, access_levels }; missing.check_missing_stability(CRATE_DEF_ID, tcx.hir().span(CRATE_HIR_ID)); tcx.hir().walk_toplevel_module(&mut missing); - tcx.hir().visit_all_item_likes(&mut missing); + tcx.hir().deep_visit_all_item_likes(&mut missing); } let declared_lang_features = &tcx.features().declared_lang_features; diff --git a/compiler/rustc_typeck/src/collect.rs b/compiler/rustc_typeck/src/collect.rs index 81a4f6e547deb..29134bd168cf9 100644 --- a/compiler/rustc_typeck/src/collect.rs +++ b/compiler/rustc_typeck/src/collect.rs @@ -59,7 +59,7 @@ struct OnlySelfBounds(bool); // Main entry point fn collect_mod_item_types(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { - tcx.hir().visit_item_likes_in_module(module_def_id, &mut CollectItemTypesVisitor { tcx }); + tcx.hir().deep_visit_item_likes_in_module(module_def_id, &mut CollectItemTypesVisitor { tcx }); } pub fn provide(providers: &mut Providers) { From 2e988794037e821cff9b8b978d3895b2c6532adb Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Sat, 7 May 2022 17:11:15 -0400 Subject: [PATCH 21/30] change back to using tcx.hir() visit-item method Signed-off-by: Miguel Guarniz --- compiler/rustc_passes/src/check_const.rs | 36 ++++--------------- compiler/rustc_passes/src/hir_id_validator.rs | 19 +--------- 2 files changed, 8 insertions(+), 47 deletions(-) diff --git a/compiler/rustc_passes/src/check_const.rs b/compiler/rustc_passes/src/check_const.rs index 0e4df831f3fe7..2405c63206038 100644 --- a/compiler/rustc_passes/src/check_const.rs +++ b/compiler/rustc_passes/src/check_const.rs @@ -10,7 +10,6 @@ use rustc_attr as attr; use rustc_errors::struct_span_err; use rustc_hir as hir; -use rustc_hir::def::DefKind; use rustc_hir::def_id::LocalDefId; use rustc_hir::intravisit::{self, Visitor}; use rustc_middle::hir::nested_filter; @@ -58,41 +57,15 @@ impl NonConstExpr { fn check_mod_const_bodies(tcx: TyCtxt<'_>, module_def_id: LocalDefId) { let mut vis = CheckConstVisitor::new(tcx); - let module = tcx.hir_module_items(module_def_id); - - for id in module.items() { - vis.visit_item(tcx.hir().item(id)); - check_item(tcx, id); - } - - for id in module.trait_items() { - vis.visit_trait_item(tcx.hir().trait_item(id)); - } - - for id in module.impl_items() { - vis.visit_impl_item(tcx.hir().impl_item(id)); - } - - for id in module.foreign_items() { - vis.visit_foreign_item(tcx.hir().foreign_item(id)); - } - - // for id in tcx.hir_module_items(module_def_id).items() { - // check_item(tcx, id); - // } + tcx.hir().deep_visit_item_likes_in_module(module_def_id, &mut vis); } pub(crate) fn provide(providers: &mut Providers) { *providers = Providers { check_mod_const_bodies, ..*providers }; } -fn check_item<'tcx>(tcx: TyCtxt<'tcx>, id: hir::ItemId) { +fn check_item<'tcx>(tcx: TyCtxt<'tcx>, item: &'tcx hir::Item<'tcx>) { let _: Option<_> = try { - if !matches!(tcx.def_kind(id.def_id), DefKind::Impl) { - None? - } - - let item = tcx.hir().item(id); if let hir::ItemKind::Impl(ref imp) = item.kind && let hir::Constness::Const = imp.constness { let trait_def_id = imp.of_trait.as_ref()?.trait_def_id()?; let ancestors = tcx @@ -274,6 +247,11 @@ impl<'tcx> Visitor<'tcx> for CheckConstVisitor<'tcx> { self.tcx.hir() } + fn visit_item(&mut self, item: &'tcx hir::Item<'tcx>) { + intravisit::walk_item(self, item); + check_item(self.tcx, item); + } + fn visit_anon_const(&mut self, anon: &'tcx hir::AnonConst) { let kind = Some(hir::ConstContext::Const); self.recurse_into(kind, None, |this| intravisit::walk_anon_const(this, anon)); diff --git a/compiler/rustc_passes/src/hir_id_validator.rs b/compiler/rustc_passes/src/hir_id_validator.rs index b91249badd4e4..762beefe8444a 100644 --- a/compiler/rustc_passes/src/hir_id_validator.rs +++ b/compiler/rustc_passes/src/hir_id_validator.rs @@ -3,7 +3,6 @@ use rustc_data_structures::sync::Lock; use rustc_hir as hir; use rustc_hir::def_id::{LocalDefId, CRATE_DEF_ID}; use rustc_hir::intravisit; -use rustc_hir::intravisit::Visitor; use rustc_hir::{HirId, ItemLocalId}; use rustc_middle::hir::map::Map; use rustc_middle::hir::nested_filter; @@ -27,23 +26,7 @@ pub fn check_crate(tcx: TyCtxt<'_>) { errors: &errors, }; - let module = tcx.hir_module_items(module_id); - - for id in module.items() { - v.visit_item(tcx.hir().item(id)) - } - - for id in module.trait_items() { - v.visit_trait_item(tcx.hir().trait_item(id)) - } - - for id in module.impl_items() { - v.visit_impl_item(tcx.hir().impl_item(id)) - } - - for id in module.foreign_items() { - v.visit_foreign_item(tcx.hir().foreign_item(id)) - } + tcx.hir().deep_visit_item_likes_in_module(module_id, &mut v); }); let errors = errors.into_inner(); From cad1fd2f16a0a2516c55b445452dbc84abee8d6d Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Sat, 7 May 2022 17:30:38 -0400 Subject: [PATCH 22/30] update rustdoc code to use new method name Signed-off-by: Miguel Guarniz --- compiler/rustc_middle/src/hir/nested_filter.rs | 2 +- src/librustdoc/scrape_examples.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_middle/src/hir/nested_filter.rs b/compiler/rustc_middle/src/hir/nested_filter.rs index 48efae8045bdd..d56e87bbb4745 100644 --- a/compiler/rustc_middle/src/hir/nested_filter.rs +++ b/compiler/rustc_middle/src/hir/nested_filter.rs @@ -8,7 +8,7 @@ use rustc_hir::intravisit::nested_filter::NestedFilter; /// constant arguments of types, e.g. in `let _: [(); /* HERE */];`. /// /// **This is the most common choice.** A very common pattern is -/// to use `visit_all_item_likes()` as an outer loop, +/// to use `deep_visit_all_item_likes()` as an outer loop, /// and to have the visitor that visits the contents of each item /// using this setting. pub struct OnlyBodies(()); diff --git a/src/librustdoc/scrape_examples.rs b/src/librustdoc/scrape_examples.rs index fc07f49f150ba..e0aed1e1ed434 100644 --- a/src/librustdoc/scrape_examples.rs +++ b/src/librustdoc/scrape_examples.rs @@ -303,7 +303,7 @@ crate fn run( // Run call-finder on all items let mut calls = FxHashMap::default(); let mut finder = FindCalls { calls: &mut calls, tcx, map: tcx.hir(), cx, target_crates }; - tcx.hir().visit_all_item_likes(&mut finder); + tcx.hir().deep_visit_all_item_likes(&mut finder); // Sort call locations within a given file in document order for fn_calls in calls.values_mut() { From df119428a2993e7cc8e7e0d7520ac80d84a1a547 Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Mon, 9 May 2022 18:44:04 -0400 Subject: [PATCH 23/30] change for_each_module's parameter to FnMut Signed-off-by: Miguel Guarniz --- compiler/rustc_middle/src/hir/map/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/hir/map/mod.rs b/compiler/rustc_middle/src/hir/map/mod.rs index b0a0131536bc0..9976b0e986204 100644 --- a/compiler/rustc_middle/src/hir/map/mod.rs +++ b/compiler/rustc_middle/src/hir/map/mod.rs @@ -672,7 +672,7 @@ impl<'hir> Map<'hir> { } } - pub fn for_each_module(self, f: impl Fn(LocalDefId)) { + pub fn for_each_module(self, mut f: impl FnMut(LocalDefId)) { let crate_items = self.tcx.hir_crate_items(()); for module in crate_items.submodules.iter() { f(*module) From 91223acde33e809e4b0327bdbb532613c56c5d86 Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Mon, 9 May 2022 18:46:49 -0400 Subject: [PATCH 24/30] use for_each_module instead of iterating over Item's Signed-off-by: Miguel Guarniz --- compiler/rustc_passes/src/debugger_visualizer.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/compiler/rustc_passes/src/debugger_visualizer.rs b/compiler/rustc_passes/src/debugger_visualizer.rs index dc63a2d8666bd..8f3cdd4d421e9 100644 --- a/compiler/rustc_passes/src/debugger_visualizer.rs +++ b/compiler/rustc_passes/src/debugger_visualizer.rs @@ -5,7 +5,7 @@ use rustc_data_structures::fx::FxHashSet; use rustc_expand::base::resolve_path; use rustc_hir as hir; use rustc_hir::def_id::CrateNum; -use rustc_hir::{HirId, Target}; +use rustc_hir::HirId; use rustc_middle::ty::query::Providers; use rustc_middle::ty::TyCtxt; use rustc_span::def_id::LOCAL_CRATE; @@ -86,12 +86,13 @@ fn debugger_visualizers<'tcx>(tcx: TyCtxt<'tcx>, cnum: CrateNum) -> Vec Date: Mon, 9 May 2022 18:54:48 -0400 Subject: [PATCH 25/30] avoid converting to DefId from LocalDefId when using query Signed-off-by: Miguel Guarniz --- compiler/rustc_passes/src/entry.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_passes/src/entry.rs b/compiler/rustc_passes/src/entry.rs index 266e1315ce26d..747332e6070dd 100644 --- a/compiler/rustc_passes/src/entry.rs +++ b/compiler/rustc_passes/src/entry.rs @@ -93,7 +93,7 @@ fn find_item(id: ItemId, ctxt: &mut EntryContext<'_>) { } EntryPointType::MainNamed => (), EntryPointType::OtherMain => { - ctxt.non_main_fns.push(ctxt.tcx.def_span(id.def_id.to_def_id())); + ctxt.non_main_fns.push(ctxt.tcx.def_span(id.def_id)); } EntryPointType::MainAttr => { if ctxt.attr_main_fn.is_none() { From 17e86d9ff9d4e3d84077febbeaf67344bcb7f6d9 Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Mon, 9 May 2022 19:05:29 -0400 Subject: [PATCH 26/30] remove unnecessary methods from HirIdValidator Signed-off-by: Miguel Guarniz --- compiler/rustc_passes/src/hir_id_validator.rs | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/compiler/rustc_passes/src/hir_id_validator.rs b/compiler/rustc_passes/src/hir_id_validator.rs index 762beefe8444a..23ff0a9115970 100644 --- a/compiler/rustc_passes/src/hir_id_validator.rs +++ b/compiler/rustc_passes/src/hir_id_validator.rs @@ -158,18 +158,4 @@ impl<'a, 'hir> intravisit::Visitor<'hir> for HirIdValidator<'a, 'hir> { let mut inner_visitor = self.new_visitor(self.hir_map); inner_visitor.check(i.def_id, |this| intravisit::walk_impl_item(this, i)); } - - fn visit_foreign_item_ref(&mut self, _: &'hir hir::ForeignItemRef) { - // Explicitly do nothing here. ForeignItemRefs contain hir::Visibility - // values that actually belong to an ForeignItem instead of the ItemKind::ForeignMod - // we are currently in. So for those it's correct that they have a - // different owner. - } - - fn visit_impl_item_ref(&mut self, _: &'hir hir::ImplItemRef) { - // Explicitly do nothing here. ImplItemRefs contain hir::Visibility - // values that actually belong to an ImplItem instead of the ItemKind::Impl - // we are currently in. So for those it's correct that they have a - // different owner. - } } From f1c256d1687662a8ca305976ba46e39d12199ae6 Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Mon, 9 May 2022 19:07:13 -0400 Subject: [PATCH 27/30] remove redundant branch Signed-off-by: Miguel Guarniz --- compiler/rustc_passes/src/debugger_visualizer.rs | 8 ++++---- compiler/rustc_privacy/src/lib.rs | 11 ----------- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/compiler/rustc_passes/src/debugger_visualizer.rs b/compiler/rustc_passes/src/debugger_visualizer.rs index 8f3cdd4d421e9..8305830bc988f 100644 --- a/compiler/rustc_passes/src/debugger_visualizer.rs +++ b/compiler/rustc_passes/src/debugger_visualizer.rs @@ -86,13 +86,13 @@ fn debugger_visualizers<'tcx>(tcx: TyCtxt<'tcx>, cnum: CrateNum) -> Vec PrivateItemsInPublicInterfacesChecker<'tcx> { let item_visibility = tcx.visibility(id.def_id); let def_kind = tcx.def_kind(id.def_id); - if matches!( - def_kind, - DefKind::ExternCrate - | DefKind::Mod - | DefKind::Use - | DefKind::Macro(_) - | DefKind::GlobalAsm - ) { - return; - } - match def_kind { DefKind::Const | DefKind::Static(_) | DefKind::Fn | DefKind::TyAlias => { self.check(id.def_id, item_visibility).generics().predicates().ty(); From 959636d53126fdc33452759dcba44d12a980674b Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Thu, 12 May 2022 11:34:37 -0400 Subject: [PATCH 28/30] avoid fetching HIR when handling Impl assoc items Signed-off-by: Miguel Guarniz --- compiler/rustc_passes/src/dead.rs | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/compiler/rustc_passes/src/dead.rs b/compiler/rustc_passes/src/dead.rs index 55a67070a9674..e78d9a5998284 100644 --- a/compiler/rustc_passes/src/dead.rs +++ b/compiler/rustc_passes/src/dead.rs @@ -511,18 +511,24 @@ fn check_item<'tcx>( } } DefKind::Impl => { - let item = tcx.hir().item(id); - if let hir::ItemKind::Impl(hir::Impl { ref of_trait, items, .. }) = item.kind { - if of_trait.is_some() { - worklist.push(item.def_id); - } - for impl_item_ref in *items { - let impl_item = tcx.hir().impl_item(impl_item_ref.id); - if of_trait.is_some() - || has_allow_dead_code_or_lang_attr(tcx, impl_item.hir_id()) - { - worklist.push(impl_item_ref.id.def_id); - } + let of_trait = tcx.impl_trait_ref(id.def_id); + + if of_trait.is_some() { + worklist.push(id.def_id); + } + + // get DefIds from another query + let local_def_ids = tcx + .associated_item_def_ids(id.def_id) + .iter() + .filter_map(|def_id| def_id.as_local()); + + // And we access the Map here to get HirId from LocalDefId + for id in local_def_ids { + if of_trait.is_some() + || has_allow_dead_code_or_lang_attr(tcx, tcx.hir().local_def_id_to_hir_id(id)) + { + worklist.push(id); } } } From f77658b470deb558ef3a270ca154a8afad230600 Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Thu, 12 May 2022 14:03:21 -0400 Subject: [PATCH 29/30] use opt_item_name to pattern match items with names Signed-off-by: Miguel Guarniz --- compiler/rustc_middle/src/ty/mod.rs | 2 +- compiler/rustc_passes/src/entry.rs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_middle/src/ty/mod.rs b/compiler/rustc_middle/src/ty/mod.rs index c4e0ebdc63876..c62b6f7e42940 100644 --- a/compiler/rustc_middle/src/ty/mod.rs +++ b/compiler/rustc_middle/src/ty/mod.rs @@ -1995,7 +1995,7 @@ impl<'tcx> TyCtxt<'tcx> { } /// Look up the name of a definition across crates. This does not look at HIR. - fn opt_item_name(self, def_id: DefId) -> Option { + pub fn opt_item_name(self, def_id: DefId) -> Option { if let Some(cnum) = def_id.as_crate_root() { Some(self.crate_name(cnum)) } else { diff --git a/compiler/rustc_passes/src/entry.rs b/compiler/rustc_passes/src/entry.rs index 747332e6070dd..b90d44e2af57c 100644 --- a/compiler/rustc_passes/src/entry.rs +++ b/compiler/rustc_passes/src/entry.rs @@ -58,8 +58,8 @@ fn entry_point_type(ctxt: &EntryContext<'_>, id: ItemId, at_root: bool) -> Entry } else if ctxt.tcx.sess.contains_name(attrs, sym::rustc_main) { EntryPointType::MainAttr } else { - let item = ctxt.tcx.hir().item(id); - if item.ident.name == sym::main { + if let Some(name) = ctxt.tcx.opt_item_name(id.def_id.to_def_id()) + && name == sym::main { if at_root { // This is a top-level function so can be `main`. EntryPointType::MainNamed From 48fd66613b17aabba3c20557d5802d2cdd122e71 Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Sat, 14 May 2022 12:23:23 -0400 Subject: [PATCH 30/30] allocate string only when error will be emitted Signed-off-by: Miguel Guarniz --- compiler/rustc_passes/src/check_const.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_passes/src/check_const.rs b/compiler/rustc_passes/src/check_const.rs index 2405c63206038..04d6e9f205abd 100644 --- a/compiler/rustc_passes/src/check_const.rs +++ b/compiler/rustc_passes/src/check_const.rs @@ -98,7 +98,7 @@ fn check_item<'tcx>(tcx: TyCtxt<'tcx>, item: &'tcx hir::Item<'tcx>) { .unwrap_or(false); if !is_implemented { - to_implement.push(tcx.item_name(trait_item_id).to_string()); + to_implement.push(trait_item_id); } } } @@ -106,13 +106,18 @@ fn check_item<'tcx>(tcx: TyCtxt<'tcx>, item: &'tcx hir::Item<'tcx>) { // all nonconst trait functions (not marked with #[default_method_body_is_const]) // must be implemented if !to_implement.is_empty() { + let not_implemented = to_implement + .into_iter() + .map(|did| tcx.item_name(did).to_string()) + .collect::>() + .join("`, `"); tcx .sess .struct_span_err( item.span, "const trait implementations may not use non-const default functions", ) - .note(&format!("`{}` not implemented", to_implement.join("`, `"))) + .note(&format!("`{}` not implemented", not_implemented)) .emit(); } }