manually crafted revert of PR rust-lang#80653, to address issue rust-…

…lang#82465. (update: placated tidy) (update: rebased post PR rust-lang#84707 ) merge me
Mark-Simulacrum · Jun 19, 2021 · 93d9ec7 · 93d9ec7
1 parent c5b8265
commit 93d9ec7
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 172 deletions.
diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs
@@ -6,7 +6,7 @@ use std::rc::Rc;
 use std::sync::mpsc::{channel, Receiver};
 
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
-use rustc_hir::def_id::{DefId, LOCAL_CRATE};
+use rustc_hir::def_id::LOCAL_CRATE;
 use rustc_middle::ty::TyCtxt;
 use rustc_session::Session;
 use rustc_span::edition::Edition;
@@ -51,9 +51,6 @@ crate struct Context<'tcx> {
     pub(super) render_redirect_pages: bool,
     /// The map used to ensure all generated 'id=' attributes are unique.
     pub(super) id_map: RefCell<IdMap>,
-    /// Tracks section IDs for `Deref` targets so they match in both the main
-    /// body and the sidebar.
-    pub(super) deref_id_map: RefCell<FxHashMap<DefId, String>>,
     /// Shared mutable state.
     ///
     /// Issue for improving the situation: [#82381][]
@@ -74,7 +71,7 @@ crate struct Context<'tcx> {
 
 // `Context` is cloned a lot, so we don't want the size to grow unexpectedly.
 #[cfg(target_arch = "x86_64")]
-rustc_data_structures::static_assert_size!(Context<'_>, 152);
+rustc_data_structures::static_assert_size!(Context<'_>, 112);
 
 /// Shared mutable state used in [`Context`] and elsewhere.
 crate struct SharedContext<'tcx> {
@@ -486,7 +483,6 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
             dst,
             render_redirect_pages: false,
             id_map: RefCell::new(id_map),
-            deref_id_map: RefCell::new(FxHashMap::default()),
             shared: Rc::new(scx),
             cache: Rc::new(cache),
         };
@@ -504,7 +500,6 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
             dst: self.dst.clone(),
             render_redirect_pages: self.render_redirect_pages,
             id_map: RefCell::new(IdMap::new()),
-            deref_id_map: RefCell::new(FxHashMap::default()),
             shared: Rc::clone(&self.shared),
             cache: Rc::clone(&self.cache),
         }

diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs
@@ -1045,17 +1045,12 @@ fn render_assoc_items(
                 RenderMode::Normal
             }
             AssocItemRender::DerefFor { trait_, type_, deref_mut_ } => {
-                let id =
-                    cx.derive_id(small_url_encode(format!("deref-methods-{:#}", type_.print(cx))));
-                debug!("Adding {} to deref id map", type_.print(cx));
-                cx.deref_id_map.borrow_mut().insert(type_.def_id_full(cache).unwrap(), id.clone());
                 write!(
                     w,
-                    "<h2 id=\"{id}\" class=\"small-section-header\">\
+                    "<h2 id=\"deref-methods\" class=\"small-section-header\">\
                          Methods from {trait_}&lt;Target = {type_}&gt;\
-                         <a href=\"#{id}\" class=\"anchor\"></a>\
+                         <a href=\"#deref-methods\" class=\"anchor\"></a>\
                      </h2>",
-                    id = id,
                     trait_ = trait_.print(cx),
                     type_ = type_.print(cx),
                 );
@@ -1080,6 +1075,9 @@ fn render_assoc_items(
             );
         }
     }
+    if let AssocItemRender::DerefFor { .. } = what {
+        return;
+    }
     if !traits.is_empty() {
         let deref_impl = traits
             .iter()
@@ -1090,13 +1088,6 @@ fn render_assoc_items(
                 .any(|t| t.inner_impl().trait_.def_id_full(cache) == cx.cache.deref_mut_trait_did);
             render_deref_methods(w, cx, impl_, containing_item, has_deref_mut);
         }
-
-        // If we were already one level into rendering deref methods, we don't want to render
-        // anything after recursing into any further deref methods above.
-        if let AssocItemRender::DerefFor { .. } = what {
-            return;
-        }
-
         let (synthetic, concrete): (Vec<&&Impl>, Vec<&&Impl>) =
             traits.iter().partition(|t| t.inner_impl().synthetic);
         let (blanket_impl, concrete): (Vec<&&Impl>, _) =
@@ -2017,14 +2008,9 @@ fn sidebar_deref_methods(cx: &Context<'_>, out: &mut Buffer, impl_: &Impl, v: &V
                 .flat_map(|i| get_methods(i.inner_impl(), true, &mut used_links, deref_mut, c))
                 .collect::<Vec<_>>();
             if !ret.is_empty() {
-                let deref_id_map = cx.deref_id_map.borrow();
-                let id = deref_id_map
-                    .get(&real_target.def_id_full(c).unwrap())
-                    .expect("Deref section without derived id");
                 write!(
                     out,
-                    "<a class=\"sidebar-title\" href=\"#{}\">Methods from {}&lt;Target={}&gt;</a>",
-                    id,
+                    "<a class=\"sidebar-title\" href=\"#deref-methods\">Methods from {}&lt;Target={}&gt;</a>",
                     Escape(&format!("{:#}", impl_.inner_impl().trait_.as_ref().unwrap().print(cx))),
                     Escape(&format!("{:#}", real_target.print(cx))),
                 );

diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs
@@ -3,8 +3,7 @@ use crate::clean::*;
 use crate::core::DocContext;
 use crate::fold::DocFolder;
 
-use rustc_data_structures::fx::{FxHashMap, FxHashSet};
-use rustc_hir::def_id::DefId;
+use rustc_data_structures::fx::FxHashSet;
 use rustc_middle::ty::DefIdTree;
 use rustc_span::symbol::sym;
 
@@ -53,6 +52,39 @@ crate fn collect_trait_impls(krate: Crate, cx: &mut DocContext<'_>) -> Crate {
         }
     }
 
+    let mut cleaner = BadImplStripper { prims, items: crate_items };
+
+    // scan through included items ahead of time to splice in Deref targets to the "valid" sets
+    for it in &new_items {
+        if let ImplItem(Impl { ref for_, ref trait_, ref items, .. }) = *it.kind {
+            if cleaner.keep_impl(for_) && trait_.def_id() == cx.tcx.lang_items().deref_trait() {
+                let target = items
+                    .iter()
+                    .find_map(|item| match *item.kind {
+                        TypedefItem(ref t, true) => Some(&t.type_),
+                        _ => None,
+                    })
+                    .expect("Deref impl without Target type");
+
+                if let Some(prim) = target.primitive_type() {
+                    cleaner.prims.insert(prim);
+                } else if let Some(did) = target.def_id() {
+                    cleaner.items.insert(did.into());
+                }
+            }
+        }
+    }
+
+    new_items.retain(|it| {
+        if let ImplItem(Impl { ref for_, ref trait_, ref blanket_impl, .. }) = *it.kind {
+            cleaner.keep_impl(for_)
+                || trait_.as_ref().map_or(false, |t| cleaner.keep_impl(t))
+                || blanket_impl.is_some()
+        } else {
+            true
+        }
+    });
+
     // `tcx.crates()` doesn't include the local crate, and `tcx.all_trait_implementations`
     // doesn't work with it anyway, so pull them from the HIR map instead
     let mut extra_attrs = Vec::new();
@@ -84,73 +116,14 @@ crate fn collect_trait_impls(krate: Crate, cx: &mut DocContext<'_>) -> Crate {
         }
     }
 
-    let mut cleaner = BadImplStripper { prims, items: crate_items };
-
-    let mut type_did_to_deref_target: FxHashMap<DefId, &Type> = FxHashMap::default();
-    // Gather all type to `Deref` target edges.
-    for it in &new_items {
-        if let ImplItem(Impl { ref for_, ref trait_, ref items, .. }) = *it.kind {
-            if trait_.def_id() == cx.tcx.lang_items().deref_trait() {
-                let target = items.iter().find_map(|item| match *item.kind {
-                    TypedefItem(ref t, true) => Some(&t.type_),
-                    _ => None,
-                });
-                if let (Some(for_did), Some(target)) = (for_.def_id(), target) {
-                    type_did_to_deref_target.insert(for_did, target);
-                }
-            }
-        }
-    }
-    // Follow all `Deref` targets of included items and recursively add them as valid
-    fn add_deref_target(
-        map: &FxHashMap<DefId, &Type>,
-        cleaner: &mut BadImplStripper,
-        type_did: &DefId,
-    ) {
-        if let Some(target) = map.get(type_did) {
-            debug!("add_deref_target: type {:?}, target {:?}", type_did, target);
-            if let Some(target_prim) = target.primitive_type() {
-                cleaner.prims.insert(target_prim);
-            } else if let Some(target_did) = target.def_id() {
-                // `impl Deref<Target = S> for S`
-                if target_did == *type_did {
-                    // Avoid infinite cycles
-                    return;
-                }
-                cleaner.items.insert(target_did.into());
-                add_deref_target(map, cleaner, &target_did.into());
-            }
-        }
-    }
-    for type_did in type_did_to_deref_target.keys() {
-        // Since only the `DefId` portion of the `Type` instances is known to be same for both the
-        // `Deref` target type and the impl for type positions, this map of types is keyed by
-        // `DefId` and for convenience uses a special cleaner that accepts `DefId`s directly.
-        if cleaner.keep_impl_with_def_id(FakeDefId::Real(*type_did)) {
-            add_deref_target(&type_did_to_deref_target, &mut cleaner, type_did);
-        }
-    }
-
     let items = if let ModuleItem(Module { ref mut items, .. }) = *krate.module.kind {
         items
     } else {
         panic!("collect-trait-impls can't run");
     };
 
     items.extend(synth_impls);
-    for it in new_items.drain(..) {
-        if let ImplItem(Impl { ref for_, ref trait_, ref blanket_impl, .. }) = *it.kind {
-            if !(cleaner.keep_impl(for_)
-                || trait_.as_ref().map_or(false, |t| cleaner.keep_impl(t))
-                || blanket_impl.is_some())
-            {
-                continue;
-            }
-        }
-
-        items.push(it);
-    }
-
+    items.extend(new_items);
     krate
 }
 

diff --git a/src/test/rustdoc-ui/deref-recursive-cycle.rs b/src/test/rustdoc-ui/deref-recursive-cycle.rs
diff --git a/src/test/rustdoc/deref-recursive-pathbuf.rs b/src/test/rustdoc/deref-recursive-pathbuf.rs
diff --git a/src/test/rustdoc/deref-recursive.rs b/src/test/rustdoc/deref-recursive.rs
diff --git a/src/test/rustdoc/deref-typedef.rs b/src/test/rustdoc/deref-typedef.rs
@@ -1,12 +1,12 @@
 #![crate_name = "foo"]
 
 // @has 'foo/struct.Bar.html'
-// @has '-' '//*[@id="deref-methods-FooJ"]' 'Methods from Deref<Target = FooJ>'
+// @has '-' '//*[@id="deref-methods"]' 'Methods from Deref<Target = FooJ>'
 // @has '-' '//*[@class="impl-items"]//*[@id="method.foo_a"]' 'pub fn foo_a(&self)'
 // @has '-' '//*[@class="impl-items"]//*[@id="method.foo_b"]' 'pub fn foo_b(&self)'
 // @has '-' '//*[@class="impl-items"]//*[@id="method.foo_c"]' 'pub fn foo_c(&self)'
 // @has '-' '//*[@class="impl-items"]//*[@id="method.foo_j"]' 'pub fn foo_j(&self)'
-// @has '-' '//*[@class="sidebar-title"][@href="#deref-methods-FooJ"]' 'Methods from Deref<Target=FooJ>'
+// @has '-' '//*[@class="sidebar-title"][@href="#deref-methods"]' 'Methods from Deref<Target=FooJ>'
 // @has '-' '//*[@class="sidebar-links"]/a[@href="#method.foo_a"]' 'foo_a'
 // @has '-' '//*[@class="sidebar-links"]/a[@href="#method.foo_b"]' 'foo_b'
 // @has '-' '//*[@class="sidebar-links"]/a[@href="#method.foo_c"]' 'foo_c'