Skip to content

Commit 7c3c588

Browse files
kckeikskckeiks
committed
remove ItemLikeVisitor impl for InherentOverlapChecker
Signed-off-by: Miguel Guarniz <mi9uel9@gmail.com>
1 parent 63849b6 commit 7c3c588

File tree

1 file changed

+163
-176
lines changed

1 file changed

+163
-176
lines changed

‎compiler/rustc_typeck/src/coherence/inherent_impls_overlap.rs

Lines changed: 163 additions & 176 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
22
use rustc_errors::struct_span_err;
33
use rustc_hir as hir;
4+
use rustc_hir::def::DefKind;
45
use rustc_hir::def_id::DefId;
5-
use rustc_hir::itemlikevisit::ItemLikeVisitor;
66
use rustc_index::vec::IndexVec;
77
use rustc_middle::traits::specialization_graph::OverlapMode;
88
use rustc_middle::ty::{self, TyCtxt};
@@ -12,7 +12,10 @@ use smallvec::SmallVec;
1212
use std::collections::hash_map::Entry;
1313

1414
pub fn crate_inherent_impls_overlap_check(tcx: TyCtxt<'_>, (): ()) {
15-
tcx.hir().visit_all_item_likes(&mut InherentOverlapChecker { tcx });
15+
let mut inherent_overlap_checker = InherentOverlapChecker { tcx };
16+
for id in tcx.hir().items() {
17+
inherent_overlap_checker.check_item(id);
18+
}
1619
}
1720

1821
struct InherentOverlapChecker<'tcx> {
@@ -121,200 +124,184 @@ impl<'tcx> InherentOverlapChecker<'tcx> {
121124
|| true,
122125
);
123126
}
124-
}
125127

126-
impl<'tcx> ItemLikeVisitor<'_> for InherentOverlapChecker<'tcx> {
127-
fn visit_item(&mut self, item: &hir::Item<'_>) {
128-
match item.kind {
129-
hir::ItemKind::Enum(..)
130-
| hir::ItemKind::Struct(..)
131-
| hir::ItemKind::Trait(..)
132-
| hir::ItemKind::Union(..) => {
133-
let impls = self.tcx.inherent_impls(item.def_id);
128+
fn check_item(&mut self, id: hir::ItemId) {
129+
let def_kind = self.tcx.hir().def_kind(id.def_id);
130+
if !matches!(def_kind, DefKind::Enum | DefKind::Struct | DefKind::Trait | DefKind::Union) {
131+
return;
132+
}
134133

135-
// If there is only one inherent impl block,
136-
// there is nothing to overlap check it with
137-
if impls.len() <= 1 {
138-
return;
139-
}
134+
let impls = self.tcx.inherent_impls(id.def_id);
140135

141-
let overlap_mode = OverlapMode::get(self.tcx, item.def_id.to_def_id());
136+
// If there is only one inherent impl block,
137+
// there is nothing to overlap check it with
138+
if impls.len() <= 1 {
139+
return;
140+
}
142141

143-
let impls_items = impls
144-
.iter()
145-
.map(|impl_def_id| (impl_def_id, self.tcx.associated_items(*impl_def_id)))
146-
.collect::<SmallVec<[_; 8]>>();
142+
let overlap_mode = OverlapMode::get(self.tcx, id.def_id.to_def_id());
147143

148-
// Perform a O(n^2) algorithm for small n,
149-
// otherwise switch to an allocating algorithm with
150-
// faster asymptotic runtime.
151-
const ALLOCATING_ALGO_THRESHOLD: usize = 500;
152-
if impls.len() < ALLOCATING_ALGO_THRESHOLD {
153-
for (i, &(&impl1_def_id, impl_items1)) in impls_items.iter().enumerate() {
154-
for &(&impl2_def_id, impl_items2) in &impls_items[(i + 1)..] {
155-
if self.impls_have_common_items(impl_items1, impl_items2) {
156-
self.check_for_overlapping_inherent_impls(
157-
overlap_mode,
158-
impl1_def_id,
159-
impl2_def_id,
160-
);
161-
}
162-
}
144+
let impls_items = impls
145+
.iter()
146+
.map(|impl_def_id| (impl_def_id, self.tcx.associated_items(*impl_def_id)))
147+
.collect::<SmallVec<[_; 8]>>();
148+
149+
// Perform a O(n^2) algorithm for small n,
150+
// otherwise switch to an allocating algorithm with
151+
// faster asymptotic runtime.
152+
const ALLOCATING_ALGO_THRESHOLD: usize = 500;
153+
if impls.len() < ALLOCATING_ALGO_THRESHOLD {
154+
for (i, &(&impl1_def_id, impl_items1)) in impls_items.iter().enumerate() {
155+
for &(&impl2_def_id, impl_items2) in &impls_items[(i + 1)..] {
156+
if self.impls_have_common_items(impl_items1, impl_items2) {
157+
self.check_for_overlapping_inherent_impls(
158+
overlap_mode,
159+
impl1_def_id,
160+
impl2_def_id,
161+
);
163162
}
164-
} else {
165-
// Build a set of connected regions of impl blocks.
166-
// Two impl blocks are regarded as connected if they share
167-
// an item with the same unhygienic identifier.
168-
// After we have assembled the connected regions,
169-
// run the O(n^2) algorithm on each connected region.
170-
// This is advantageous to running the algorithm over the
171-
// entire graph when there are many connected regions.
163+
}
164+
}
165+
} else {
166+
// Build a set of connected regions of impl blocks.
167+
// Two impl blocks are regarded as connected if they share
168+
// an item with the same unhygienic identifier.
169+
// After we have assembled the connected regions,
170+
// run the O(n^2) algorithm on each connected region.
171+
// This is advantageous to running the algorithm over the
172+
// entire graph when there are many connected regions.
172173

173-
rustc_index::newtype_index! {
174-
pub struct RegionId {
175-
ENCODABLE = custom
174+
rustc_index::newtype_index! {
175+
pub struct RegionId {
176+
ENCODABLE = custom
177+
}
178+
}
179+
struct ConnectedRegion {
180+
idents: SmallVec<[Symbol; 8]>,
181+
impl_blocks: FxHashSet<usize>,
182+
}
183+
let mut connected_regions: IndexVec<RegionId, _> = Default::default();
184+
// Reverse map from the Symbol to the connected region id.
185+
let mut connected_region_ids = FxHashMap::default();
186+
187+
for (i, &(&_impl_def_id, impl_items)) in impls_items.iter().enumerate() {
188+
if impl_items.len() == 0 {
189+
continue;
190+
}
191+
// First obtain a list of existing connected region ids
192+
let mut idents_to_add = SmallVec::<[Symbol; 8]>::new();
193+
let mut ids = impl_items
194+
.in_definition_order()
195+
.filter_map(|item| {
196+
let entry = connected_region_ids.entry(item.name);
197+
if let Entry::Occupied(e) = &entry {
198+
Some(*e.get())
199+
} else {
200+
idents_to_add.push(item.name);
201+
None
176202
}
203+
})
204+
.collect::<SmallVec<[RegionId; 8]>>();
205+
// Sort the id list so that the algorithm is deterministic
206+
ids.sort_unstable();
207+
ids.dedup();
208+
let ids = ids;
209+
match &ids[..] {
210+
// Create a new connected region
211+
[] => {
212+
let id_to_set = connected_regions.next_index();
213+
// Update the connected region ids
214+
for ident in &idents_to_add {
215+
connected_region_ids.insert(*ident, id_to_set);
216+
}
217+
connected_regions.insert(
218+
id_to_set,
219+
ConnectedRegion {
220+
idents: idents_to_add,
221+
impl_blocks: std::iter::once(i).collect(),
222+
},
223+
);
177224
}
178-
struct ConnectedRegion {
179-
idents: SmallVec<[Symbol; 8]>,
180-
impl_blocks: FxHashSet<usize>,
225+
// Take the only id inside the list
226+
&[id_to_set] => {
227+
let region = connected_regions[id_to_set].as_mut().unwrap();
228+
region.impl_blocks.insert(i);
229+
region.idents.extend_from_slice(&idents_to_add);
230+
// Update the connected region ids
231+
for ident in &idents_to_add {
232+
connected_region_ids.insert(*ident, id_to_set);
233+
}
181234
}
182-
let mut connected_regions: IndexVec<RegionId, _> = Default::default();
183-
// Reverse map from the Symbol to the connected region id.
184-
let mut connected_region_ids = FxHashMap::default();
185-
186-
for (i, &(&_impl_def_id, impl_items)) in impls_items.iter().enumerate() {
187-
if impl_items.len() == 0 {
188-
continue;
235+
// We have multiple connected regions to merge.
236+
// In the worst case this might add impl blocks
237+
// one by one and can thus be O(n^2) in the size
238+
// of the resulting final connected region, but
239+
// this is no issue as the final step to check
240+
// for overlaps runs in O(n^2) as well.
241+
&[id_to_set, ..] => {
242+
let mut region = connected_regions.remove(id_to_set).unwrap();
243+
region.impl_blocks.insert(i);
244+
region.idents.extend_from_slice(&idents_to_add);
245+
// Update the connected region ids
246+
for ident in &idents_to_add {
247+
connected_region_ids.insert(*ident, id_to_set);
189248
}
190-
// First obtain a list of existing connected region ids
191-
let mut idents_to_add = SmallVec::<[Symbol; 8]>::new();
192-
let mut ids = impl_items
193-
.in_definition_order()
194-
.filter_map(|item| {
195-
let entry = connected_region_ids.entry(item.name);
196-
if let Entry::Occupied(e) = &entry {
197-
Some(*e.get())
198-
} else {
199-
idents_to_add.push(item.name);
200-
None
201-
}
202-
})
203-
.collect::<SmallVec<[RegionId; 8]>>();
204-
// Sort the id list so that the algorithm is deterministic
205-
ids.sort_unstable();
206-
ids.dedup();
207-
let ids = ids;
208-
match &ids[..] {
209-
// Create a new connected region
210-
[] => {
211-
let id_to_set = connected_regions.next_index();
212-
// Update the connected region ids
213-
for ident in &idents_to_add {
214-
connected_region_ids.insert(*ident, id_to_set);
215-
}
216-
connected_regions.insert(
217-
id_to_set,
218-
ConnectedRegion {
219-
idents: idents_to_add,
220-
impl_blocks: std::iter::once(i).collect(),
221-
},
222-
);
223-
}
224-
// Take the only id inside the list
225-
&[id_to_set] => {
226-
let region = connected_regions[id_to_set].as_mut().unwrap();
227-
region.impl_blocks.insert(i);
228-
region.idents.extend_from_slice(&idents_to_add);
229-
// Update the connected region ids
230-
for ident in &idents_to_add {
231-
connected_region_ids.insert(*ident, id_to_set);
232-
}
233-
}
234-
// We have multiple connected regions to merge.
235-
// In the worst case this might add impl blocks
236-
// one by one and can thus be O(n^2) in the size
237-
// of the resulting final connected region, but
238-
// this is no issue as the final step to check
239-
// for overlaps runs in O(n^2) as well.
240-
&[id_to_set, ..] => {
241-
let mut region = connected_regions.remove(id_to_set).unwrap();
242-
region.impl_blocks.insert(i);
243-
region.idents.extend_from_slice(&idents_to_add);
244-
// Update the connected region ids
245-
for ident in &idents_to_add {
246-
connected_region_ids.insert(*ident, id_to_set);
247-
}
248-
249-
// Remove other regions from ids.
250-
for &id in ids.iter() {
251-
if id == id_to_set {
252-
continue;
253-
}
254-
let r = connected_regions.remove(id).unwrap();
255-
for ident in r.idents.iter() {
256-
connected_region_ids.insert(*ident, id_to_set);
257-
}
258-
region.idents.extend_from_slice(&r.idents);
259-
region.impl_blocks.extend(r.impl_blocks);
260-
}
261249

262-
connected_regions.insert(id_to_set, region);
250+
// Remove other regions from ids.
251+
for &id in ids.iter() {
252+
if id == id_to_set {
253+
continue;
254+
}
255+
let r = connected_regions.remove(id).unwrap();
256+
for ident in r.idents.iter() {
257+
connected_region_ids.insert(*ident, id_to_set);
263258
}
259+
region.idents.extend_from_slice(&r.idents);
260+
region.impl_blocks.extend(r.impl_blocks);
264261
}
262+
263+
connected_regions.insert(id_to_set, region);
265264
}
265+
}
266+
}
266267

267-
debug!(
268-
"churning through {} components (sum={}, avg={}, var={}, max={})",
269-
connected_regions.len(),
270-
impls.len(),
271-
impls.len() / connected_regions.len(),
272-
{
273-
let avg = impls.len() / connected_regions.len();
274-
let s = connected_regions
275-
.iter()
276-
.flatten()
277-
.map(|r| r.impl_blocks.len() as isize - avg as isize)
278-
.map(|v| v.abs() as usize)
279-
.sum::<usize>();
280-
s / connected_regions.len()
281-
},
282-
connected_regions
283-
.iter()
284-
.flatten()
285-
.map(|r| r.impl_blocks.len())
286-
.max()
287-
.unwrap()
288-
);
289-
// List of connected regions is built. Now, run the overlap check
290-
// for each pair of impl blocks in the same connected region.
291-
for region in connected_regions.into_iter().flatten() {
292-
let mut impl_blocks =
293-
region.impl_blocks.into_iter().collect::<SmallVec<[usize; 8]>>();
294-
impl_blocks.sort_unstable();
295-
for (i, &impl1_items_idx) in impl_blocks.iter().enumerate() {
296-
let &(&impl1_def_id, impl_items1) = &impls_items[impl1_items_idx];
297-
for &impl2_items_idx in impl_blocks[(i + 1)..].iter() {
298-
let &(&impl2_def_id, impl_items2) = &impls_items[impl2_items_idx];
299-
if self.impls_have_common_items(impl_items1, impl_items2) {
300-
self.check_for_overlapping_inherent_impls(
301-
overlap_mode,
302-
impl1_def_id,
303-
impl2_def_id,
304-
);
305-
}
306-
}
268+
debug!(
269+
"churning through {} components (sum={}, avg={}, var={}, max={})",
270+
connected_regions.len(),
271+
impls.len(),
272+
impls.len() / connected_regions.len(),
273+
{
274+
let avg = impls.len() / connected_regions.len();
275+
let s = connected_regions
276+
.iter()
277+
.flatten()
278+
.map(|r| r.impl_blocks.len() as isize - avg as isize)
279+
.map(|v| v.abs() as usize)
280+
.sum::<usize>();
281+
s / connected_regions.len()
282+
},
283+
connected_regions.iter().flatten().map(|r| r.impl_blocks.len()).max().unwrap()
284+
);
285+
// List of connected regions is built. Now, run the overlap check
286+
// for each pair of impl blocks in the same connected region.
287+
for region in connected_regions.into_iter().flatten() {
288+
let mut impl_blocks =
289+
region.impl_blocks.into_iter().collect::<SmallVec<[usize; 8]>>();
290+
impl_blocks.sort_unstable();
291+
for (i, &impl1_items_idx) in impl_blocks.iter().enumerate() {
292+
let &(&impl1_def_id, impl_items1) = &impls_items[impl1_items_idx];
293+
for &impl2_items_idx in impl_blocks[(i + 1)..].iter() {
294+
let &(&impl2_def_id, impl_items2) = &impls_items[impl2_items_idx];
295+
if self.impls_have_common_items(impl_items1, impl_items2) {
296+
self.check_for_overlapping_inherent_impls(
297+
overlap_mode,
298+
impl1_def_id,
299+
impl2_def_id,
300+
);
307301
}
308302
}
309303
}
310304
}
311-
_ => {}
312305
}
313306
}
314-
315-
fn visit_trait_item(&mut self, _trait_item: &hir::TraitItem<'_>) {}
316-
317-
fn visit_impl_item(&mut self, _impl_item: &hir::ImplItem<'_>) {}
318-
319-
fn visit_foreign_item(&mut self, _foreign_item: &hir::ForeignItem<'_>) {}
320307
}

0 commit comments

Comments
 (0)