From 7c3c5889888809dafe3906b472e07a9cdf399f03 Mon Sep 17 00:00:00 2001 From: Miguel Guarniz Date: Thu, 28 Apr 2022 13:03:17 -0400 Subject: [PATCH] remove ItemLikeVisitor impl for InherentOverlapChecker Signed-off-by: Miguel Guarniz --- .../src/coherence/inherent_impls_overlap.rs | 345 +++++++++--------- 1 file changed, 166 insertions(+), 179 deletions(-) diff --git a/compiler/rustc_typeck/src/coherence/inherent_impls_overlap.rs b/compiler/rustc_typeck/src/coherence/inherent_impls_overlap.rs index cf71e0f300c..6ddaecb91b2 100644 --- a/compiler/rustc_typeck/src/coherence/inherent_impls_overlap.rs +++ b/compiler/rustc_typeck/src/coherence/inherent_impls_overlap.rs @@ -1,8 +1,8 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet}; use rustc_errors::struct_span_err; use rustc_hir as hir; +use rustc_hir::def::DefKind; use rustc_hir::def_id::DefId; -use rustc_hir::itemlikevisit::ItemLikeVisitor; use rustc_index::vec::IndexVec; use rustc_middle::traits::specialization_graph::OverlapMode; use rustc_middle::ty::{self, TyCtxt}; @@ -12,7 +12,10 @@ use smallvec::SmallVec; use std::collections::hash_map::Entry; pub fn crate_inherent_impls_overlap_check(tcx: TyCtxt<'_>, (): ()) { - tcx.hir().visit_all_item_likes(&mut InherentOverlapChecker { tcx }); + let mut inherent_overlap_checker = InherentOverlapChecker { tcx }; + for id in tcx.hir().items() { + inherent_overlap_checker.check_item(id); + } } struct InherentOverlapChecker<'tcx> { @@ -121,200 +124,184 @@ impl<'tcx> InherentOverlapChecker<'tcx> { || true, ); } -} -impl<'tcx> ItemLikeVisitor<'_> for InherentOverlapChecker<'tcx> { - fn visit_item(&mut self, item: &hir::Item<'_>) { - match item.kind { - hir::ItemKind::Enum(..) - | hir::ItemKind::Struct(..) - | hir::ItemKind::Trait(..) - | hir::ItemKind::Union(..) => { - let impls = self.tcx.inherent_impls(item.def_id); + fn check_item(&mut self, id: hir::ItemId) { + let def_kind = self.tcx.hir().def_kind(id.def_id); + if !matches!(def_kind, DefKind::Enum | DefKind::Struct | DefKind::Trait | DefKind::Union) { + return; + } - // If there is only one inherent impl block, - // there is nothing to overlap check it with - if impls.len() <= 1 { - return; + let impls = self.tcx.inherent_impls(id.def_id); + + // If there is only one inherent impl block, + // there is nothing to overlap check it with + if impls.len() <= 1 { + return; + } + + let overlap_mode = OverlapMode::get(self.tcx, id.def_id.to_def_id()); + + let impls_items = impls + .iter() + .map(|impl_def_id| (impl_def_id, self.tcx.associated_items(*impl_def_id))) + .collect::>(); + + // Perform a O(n^2) algorithm for small n, + // otherwise switch to an allocating algorithm with + // faster asymptotic runtime. + const ALLOCATING_ALGO_THRESHOLD: usize = 500; + if impls.len() < ALLOCATING_ALGO_THRESHOLD { + for (i, &(&impl1_def_id, impl_items1)) in impls_items.iter().enumerate() { + for &(&impl2_def_id, impl_items2) in &impls_items[(i + 1)..] { + if self.impls_have_common_items(impl_items1, impl_items2) { + self.check_for_overlapping_inherent_impls( + overlap_mode, + impl1_def_id, + impl2_def_id, + ); + } } + } + } else { + // Build a set of connected regions of impl blocks. + // Two impl blocks are regarded as connected if they share + // an item with the same unhygienic identifier. + // After we have assembled the connected regions, + // run the O(n^2) algorithm on each connected region. + // This is advantageous to running the algorithm over the + // entire graph when there are many connected regions. - let overlap_mode = OverlapMode::get(self.tcx, item.def_id.to_def_id()); + rustc_index::newtype_index! { + pub struct RegionId { + ENCODABLE = custom + } + } + struct ConnectedRegion { + idents: SmallVec<[Symbol; 8]>, + impl_blocks: FxHashSet, + } + let mut connected_regions: IndexVec = Default::default(); + // Reverse map from the Symbol to the connected region id. + let mut connected_region_ids = FxHashMap::default(); - let impls_items = impls - .iter() - .map(|impl_def_id| (impl_def_id, self.tcx.associated_items(*impl_def_id))) - .collect::>(); - - // Perform a O(n^2) algorithm for small n, - // otherwise switch to an allocating algorithm with - // faster asymptotic runtime. - const ALLOCATING_ALGO_THRESHOLD: usize = 500; - if impls.len() < ALLOCATING_ALGO_THRESHOLD { - for (i, &(&impl1_def_id, impl_items1)) in impls_items.iter().enumerate() { - for &(&impl2_def_id, impl_items2) in &impls_items[(i + 1)..] { - if self.impls_have_common_items(impl_items1, impl_items2) { - self.check_for_overlapping_inherent_impls( - overlap_mode, - impl1_def_id, - impl2_def_id, - ); - } + for (i, &(&_impl_def_id, impl_items)) in impls_items.iter().enumerate() { + if impl_items.len() == 0 { + continue; + } + // First obtain a list of existing connected region ids + let mut idents_to_add = SmallVec::<[Symbol; 8]>::new(); + let mut ids = impl_items + .in_definition_order() + .filter_map(|item| { + let entry = connected_region_ids.entry(item.name); + if let Entry::Occupied(e) = &entry { + Some(*e.get()) + } else { + idents_to_add.push(item.name); + None + } + }) + .collect::>(); + // Sort the id list so that the algorithm is deterministic + ids.sort_unstable(); + ids.dedup(); + let ids = ids; + match &ids[..] { + // Create a new connected region + [] => { + let id_to_set = connected_regions.next_index(); + // Update the connected region ids + for ident in &idents_to_add { + connected_region_ids.insert(*ident, id_to_set); + } + connected_regions.insert( + id_to_set, + ConnectedRegion { + idents: idents_to_add, + impl_blocks: std::iter::once(i).collect(), + }, + ); + } + // Take the only id inside the list + &[id_to_set] => { + let region = connected_regions[id_to_set].as_mut().unwrap(); + region.impl_blocks.insert(i); + region.idents.extend_from_slice(&idents_to_add); + // Update the connected region ids + for ident in &idents_to_add { + connected_region_ids.insert(*ident, id_to_set); } } - } else { - // Build a set of connected regions of impl blocks. - // Two impl blocks are regarded as connected if they share - // an item with the same unhygienic identifier. - // After we have assembled the connected regions, - // run the O(n^2) algorithm on each connected region. - // This is advantageous to running the algorithm over the - // entire graph when there are many connected regions. - - rustc_index::newtype_index! { - pub struct RegionId { - ENCODABLE = custom + // We have multiple connected regions to merge. + // In the worst case this might add impl blocks + // one by one and can thus be O(n^2) in the size + // of the resulting final connected region, but + // this is no issue as the final step to check + // for overlaps runs in O(n^2) as well. + &[id_to_set, ..] => { + let mut region = connected_regions.remove(id_to_set).unwrap(); + region.impl_blocks.insert(i); + region.idents.extend_from_slice(&idents_to_add); + // Update the connected region ids + for ident in &idents_to_add { + connected_region_ids.insert(*ident, id_to_set); } - } - struct ConnectedRegion { - idents: SmallVec<[Symbol; 8]>, - impl_blocks: FxHashSet, - } - let mut connected_regions: IndexVec = Default::default(); - // Reverse map from the Symbol to the connected region id. - let mut connected_region_ids = FxHashMap::default(); - for (i, &(&_impl_def_id, impl_items)) in impls_items.iter().enumerate() { - if impl_items.len() == 0 { - continue; + // Remove other regions from ids. + for &id in ids.iter() { + if id == id_to_set { + continue; + } + let r = connected_regions.remove(id).unwrap(); + for ident in r.idents.iter() { + connected_region_ids.insert(*ident, id_to_set); + } + region.idents.extend_from_slice(&r.idents); + region.impl_blocks.extend(r.impl_blocks); } - // First obtain a list of existing connected region ids - let mut idents_to_add = SmallVec::<[Symbol; 8]>::new(); - let mut ids = impl_items - .in_definition_order() - .filter_map(|item| { - let entry = connected_region_ids.entry(item.name); - if let Entry::Occupied(e) = &entry { - Some(*e.get()) - } else { - idents_to_add.push(item.name); - None - } - }) - .collect::>(); - // Sort the id list so that the algorithm is deterministic - ids.sort_unstable(); - ids.dedup(); - let ids = ids; - match &ids[..] { - // Create a new connected region - [] => { - let id_to_set = connected_regions.next_index(); - // Update the connected region ids - for ident in &idents_to_add { - connected_region_ids.insert(*ident, id_to_set); - } - connected_regions.insert( - id_to_set, - ConnectedRegion { - idents: idents_to_add, - impl_blocks: std::iter::once(i).collect(), - }, - ); - } - // Take the only id inside the list - &[id_to_set] => { - let region = connected_regions[id_to_set].as_mut().unwrap(); - region.impl_blocks.insert(i); - region.idents.extend_from_slice(&idents_to_add); - // Update the connected region ids - for ident in &idents_to_add { - connected_region_ids.insert(*ident, id_to_set); - } - } - // We have multiple connected regions to merge. - // In the worst case this might add impl blocks - // one by one and can thus be O(n^2) in the size - // of the resulting final connected region, but - // this is no issue as the final step to check - // for overlaps runs in O(n^2) as well. - &[id_to_set, ..] => { - let mut region = connected_regions.remove(id_to_set).unwrap(); - region.impl_blocks.insert(i); - region.idents.extend_from_slice(&idents_to_add); - // Update the connected region ids - for ident in &idents_to_add { - connected_region_ids.insert(*ident, id_to_set); - } - // Remove other regions from ids. - for &id in ids.iter() { - if id == id_to_set { - continue; - } - let r = connected_regions.remove(id).unwrap(); - for ident in r.idents.iter() { - connected_region_ids.insert(*ident, id_to_set); - } - region.idents.extend_from_slice(&r.idents); - region.impl_blocks.extend(r.impl_blocks); - } - - connected_regions.insert(id_to_set, region); - } - } + connected_regions.insert(id_to_set, region); } + } + } - debug!( - "churning through {} components (sum={}, avg={}, var={}, max={})", - connected_regions.len(), - impls.len(), - impls.len() / connected_regions.len(), - { - let avg = impls.len() / connected_regions.len(); - let s = connected_regions - .iter() - .flatten() - .map(|r| r.impl_blocks.len() as isize - avg as isize) - .map(|v| v.abs() as usize) - .sum::(); - s / connected_regions.len() - }, - connected_regions - .iter() - .flatten() - .map(|r| r.impl_blocks.len()) - .max() - .unwrap() - ); - // List of connected regions is built. Now, run the overlap check - // for each pair of impl blocks in the same connected region. - for region in connected_regions.into_iter().flatten() { - let mut impl_blocks = - region.impl_blocks.into_iter().collect::>(); - impl_blocks.sort_unstable(); - for (i, &impl1_items_idx) in impl_blocks.iter().enumerate() { - let &(&impl1_def_id, impl_items1) = &impls_items[impl1_items_idx]; - for &impl2_items_idx in impl_blocks[(i + 1)..].iter() { - let &(&impl2_def_id, impl_items2) = &impls_items[impl2_items_idx]; - if self.impls_have_common_items(impl_items1, impl_items2) { - self.check_for_overlapping_inherent_impls( - overlap_mode, - impl1_def_id, - impl2_def_id, - ); - } - } + debug!( + "churning through {} components (sum={}, avg={}, var={}, max={})", + connected_regions.len(), + impls.len(), + impls.len() / connected_regions.len(), + { + let avg = impls.len() / connected_regions.len(); + let s = connected_regions + .iter() + .flatten() + .map(|r| r.impl_blocks.len() as isize - avg as isize) + .map(|v| v.abs() as usize) + .sum::(); + s / connected_regions.len() + }, + connected_regions.iter().flatten().map(|r| r.impl_blocks.len()).max().unwrap() + ); + // List of connected regions is built. Now, run the overlap check + // for each pair of impl blocks in the same connected region. + for region in connected_regions.into_iter().flatten() { + let mut impl_blocks = + region.impl_blocks.into_iter().collect::>(); + impl_blocks.sort_unstable(); + for (i, &impl1_items_idx) in impl_blocks.iter().enumerate() { + let &(&impl1_def_id, impl_items1) = &impls_items[impl1_items_idx]; + for &impl2_items_idx in impl_blocks[(i + 1)..].iter() { + let &(&impl2_def_id, impl_items2) = &impls_items[impl2_items_idx]; + if self.impls_have_common_items(impl_items1, impl_items2) { + self.check_for_overlapping_inherent_impls( + overlap_mode, + impl1_def_id, + impl2_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<'_>) {} }