From caa770aee128c1e79479960319bd2b8ccfd0be63 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 15 Apr 2022 21:52:43 +0200 Subject: [PATCH 1/2] Fix rustdoc duplicated blanket impls issue --- src/librustdoc/formats/cache.rs | 21 ++++++++++++++++++--- src/librustdoc/formats/mod.rs | 22 +++++++++++++++++++++- 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index e138e434c4e04..06e7c9e763df9 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -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>, tcx: TyCtxt<'tcx>, } @@ -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()); + } } } } @@ -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"); diff --git a/src/librustdoc/formats/mod.rs b/src/librustdoc/formats/mod.rs index 4f0c5a9edee71..3e36318eb71b0 100644 --- a/src/librustdoc/formats/mod.rs +++ b/src/librustdoc/formats/mod.rs @@ -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. @@ -40,4 +40,24 @@ impl Impl { crate fn trait_did(&self) -> Option { 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 + ) + } + } + } } From 6d10fd0b5b2a2ac2c79556d42608476241d68b36 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 15 Apr 2022 21:53:02 +0200 Subject: [PATCH 2/2] Add regression test for rustdoc duplicated blanket impls --- src/test/rustdoc/duplicated_impl.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100644 src/test/rustdoc/duplicated_impl.rs diff --git a/src/test/rustdoc/duplicated_impl.rs b/src/test/rustdoc/duplicated_impl.rs new file mode 100644 index 0000000000000..4e901b31c9076 --- /dev/null +++ b/src/test/rustdoc/duplicated_impl.rs @@ -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 Something 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 { } +pub struct Whatever; +impl Something for T {}