Skip to content

Commit

Permalink
Auto merge of #96091 - GuillaumeGomez:duplicated-blanket-impls, r=not…
Browse files Browse the repository at this point in the history
…riddle

Fix rustdoc duplicated blanket impls

Fixes #96036.

I think it'll not be great performance-wise but I couldn't find another way to prevent that unfortunately...

r? `@notriddle`
  • Loading branch information
bors committed Apr 17, 2022
2 parents 1ec2c13 + 6d10fd0 commit ad4e98e
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 4 deletions.
21 changes: 18 additions & 3 deletions src/librustdoc/formats/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,8 @@ crate struct Cache {
/// This struct is used to wrap the `cache` and `tcx` in order to run `DocFolder`.
struct CacheBuilder<'a, 'tcx> {
cache: &'a mut Cache,
/// This field is used to prevent duplicated impl blocks.
impl_ids: FxHashMap<DefId, FxHashSet<DefId>>,
tcx: TyCtxt<'tcx>,
}

Expand Down Expand Up @@ -170,12 +172,19 @@ impl Cache {
.insert(def_id, (vec![crate_name, prim.as_sym()], ItemType::Primitive));
}

krate = CacheBuilder { tcx, cache: &mut cx.cache }.fold_crate(krate);
let (krate, mut impl_ids) = {
let mut cache_builder =
CacheBuilder { tcx, cache: &mut cx.cache, impl_ids: FxHashMap::default() };
krate = cache_builder.fold_crate(krate);
(krate, cache_builder.impl_ids)
};

for (trait_did, dids, impl_) in cx.cache.orphan_trait_impls.drain(..) {
if cx.cache.traits.contains_key(&trait_did) {
for did in dids {
cx.cache.impls.entry(did).or_default().push(impl_.clone());
if impl_ids.entry(did).or_default().insert(impl_.def_id()) {
cx.cache.impls.entry(did).or_default().push(impl_.clone());
}
}
}
}
Expand Down Expand Up @@ -467,7 +476,13 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
let impl_item = Impl { impl_item: item };
if impl_item.trait_did().map_or(true, |d| self.cache.traits.contains_key(&d)) {
for did in dids {
self.cache.impls.entry(did).or_insert_with(Vec::new).push(impl_item.clone());
if self.impl_ids.entry(did).or_default().insert(impl_item.def_id()) {
self.cache
.impls
.entry(did)
.or_insert_with(Vec::new)
.push(impl_item.clone());
}
}
} else {
let trait_did = impl_item.trait_did().expect("no trait did");
Expand Down
22 changes: 21 additions & 1 deletion src/librustdoc/formats/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use rustc_hir::def_id::DefId;

crate use renderer::{run_format, FormatRenderer};

use crate::clean;
use crate::clean::{self, ItemId};

/// Specifies whether rendering directly implemented trait items or ones from a certain Deref
/// impl.
Expand Down Expand Up @@ -40,4 +40,24 @@ impl Impl {
crate fn trait_did(&self) -> Option<DefId> {
self.inner_impl().trait_.as_ref().map(|t| t.def_id())
}

/// This function is used to extract a `DefId` to be used as a key for the `Cache::impls` field.
///
/// It allows to prevent having duplicated implementations showing up (the biggest issue was
/// with blanket impls).
///
/// It panics if `self` is a `ItemId::Primitive`.
crate fn def_id(&self) -> DefId {
match self.impl_item.item_id {
ItemId::Blanket { impl_id, .. } => impl_id,
ItemId::Auto { trait_, .. } => trait_,
ItemId::DefId(def_id) => def_id,
ItemId::Primitive(_, _) => {
panic!(
"Unexpected ItemId::Primitive in expect_def_id: {:?}",
self.impl_item.item_id
)
}
}
}
}
14 changes: 14 additions & 0 deletions src/test/rustdoc/duplicated_impl.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
// This test ensures that the same implementation doesn't show more than once.
// It's a regression test for https://github.com/rust-lang/rust/issues/96036.

#![crate_name = "foo"]

// We check that there is only one "impl<T> Something<Whatever> for T" listed in the
// blanket implementations.

// @has 'foo/struct.Whatever.html'
// @count - '//*[@id="blanket-implementations-list"]/section[@class="impl has-srclink"]' 1

pub trait Something<T> { }
pub struct Whatever;
impl<T> Something<Whatever> for T {}

0 comments on commit ad4e98e

Please sign in to comment.