Auto merge of rust-lang#159623 - camelid:good-impls, r=notriddle,GuillaumeGomez

rustdoc: Only build extern trait impls if needed



or, finally remove the `BadImplStripper`!

Building inlined impls is expensive, and most of them end up being
unneeded and stripped later in this function. So we should filter them
ahead of time. This requires inlining external auto traits when we
construct auto impls, since this no longer happens as a side effect.

We inline external impls when they are
* for generics (i.e., blanket impls)
* for primitive types (probably this should be handled below really)
* for a type (inlined) in the current crate
* of a trait (inlined) in the current crate
* of `Deref`
These rules are based on the existing filtering rules that are applied
after building the inlined impls.

After doing the filtering ahead of time, we can remove the downstream
filtering. Thus, we can finally remove `BadImplStripper` and unused
deref-following logic. It appears that all this `Deref` logic was pointless
in the first place since following derefs already happens elsewhere in
rustdoc, where it is actually needed.

Author: bors

src/librustdoc/clean/auto_trait.rs

@@ -114,6 +114,8 @@ fn synthesize_auto_trait_impl<'tcx>(
         auto_trait::AutoTraitResult::ExplicitImpl => return None,
     };
 
+    super::inline::record_extern_trait(cx, trait_def_id);
+
     Some(clean::Item {
         inner: Box::new(clean::ItemInner {
             name: None,

src/librustdoc/clean/inline.rs

@@ -14,7 +14,7 @@ use rustc_middle::ty::{self, TyCtxt};
 use rustc_span::def_id::LOCAL_CRATE;
 use rustc_span::hygiene::MacroKind;
 use rustc_span::symbol::{Symbol, sym};
-use tracing::{debug, trace};
+use tracing::{debug, instrument, trace};
 
 use super::{Item, extract_cfg_from_attrs};
 use crate::clean::{
@@ -453,6 +453,7 @@ pub(crate) fn merge_attrs(
 }
 
 /// Inline an `impl`, inherent or of a trait. The `did` must be for an `impl`.
+#[instrument(level = "debug", skip(cx, ret))]
 pub(crate) fn build_impl(
     cx: &mut DocContext<'_>,
     did: DefId,

src/librustdoc/passes/collect_trait_impls.rs

@@ -4,10 +4,9 @@
 
 use rustc_data_structures::fx::FxHashSet;
 use rustc_hir::attrs::{AttributeKind, DocAttribute};
-use rustc_hir::def_id::{DefId, DefIdMap, DefIdSet, LOCAL_CRATE};
+use rustc_hir::def_id::LOCAL_CRATE;
 use rustc_hir::{Attribute, find_attr};
 use rustc_middle::ty;
-use tracing::debug;
 
 use super::Pass;
 use crate::clean::*;
@@ -53,7 +52,25 @@ pub(crate) fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) ->
         for &cnum in tcx.crates(()) {
             for &impl_def_id in tcx.trait_impls_in_crate(cnum) {
                 cx.with_param_env(impl_def_id, |cx| {
-                    inline::build_impl(cx, impl_def_id, None, &mut new_items_external);
+                    let opt_trait_ref = tcx.impl_opt_trait_ref(impl_def_id);
+                    let self_ty = tcx.type_of(impl_def_id).instantiate_identity().skip_norm_wip();
+                    let self_ty =
+                        clean_middle_ty(ty::Binder::dummy(self_ty), cx, Some(impl_def_id), None);
+                    if self_ty.is_full_generic()
+                        || self_ty
+                            .primitive_type()
+                            .is_some_and(|primitive| prims.contains(&primitive))
+                        || self_ty
+                            .def_id(&cx.cache)
+                            .is_some_and(|did| crate_items.contains(&ItemId::DefId(did)))
+                        || opt_trait_ref.is_some_and(|trait_ref| {
+                            crate_items.contains(&ItemId::DefId(trait_ref.def_id()))
+                                || Some(trait_ref.def_id()) == tcx.lang_items().deref_trait()
+                                || tcx.is_doc_notable_trait(trait_ref.def_id())
+                        })
+                    {
+                        inline::build_impl(cx, impl_def_id, None, &mut new_items_external);
+                    }
                 });
             }
         }
@@ -126,86 +143,6 @@ pub(crate) fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) ->
         }
     });
 
-    let mut cleaner = BadImplStripper { prims, items: crate_items, cache: &cx.cache };
-    let mut type_did_to_deref_target: DefIdMap<&Type> = DefIdMap::default();
-
-    // Follow all `Deref` targets of included items and recursively add them as valid
-    fn add_deref_target(
-        cx: &DocContext<'_>,
-        map: &DefIdMap<&Type>,
-        cleaner: &mut BadImplStripper<'_>,
-        targets: &mut DefIdSet,
-        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(&cx.cache) {
-                // `impl Deref<Target = S> for S`
-                if !targets.insert(target_did) {
-                    // Avoid infinite cycles
-                    return;
-                }
-                cleaner.items.insert(target_did.into());
-                add_deref_target(cx, map, cleaner, targets, target_did);
-            }
-        }
-    }
-
-    // scan through included items ahead of time to splice in Deref targets to the "valid" sets
-    for it in new_items_external.iter().chain(new_items_local.iter()) {
-        if let ImplItem(Impl { ref for_, ref trait_, ref items, polarity, .. }) = it.kind
-            && trait_.as_ref().map(|t| t.def_id()) == tcx.lang_items().deref_trait()
-            && polarity != ty::ImplPolarity::Negative
-            && cleaner.keep_impl(for_, true)
-        {
-            let target = items
-                .iter()
-                .find_map(|item| match item.kind {
-                    AssocTypeItem(ref t, _) => 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(&cx.cache) {
-                cleaner.items.insert(did.into());
-            }
-            if let Some(for_did) = for_.def_id(&cx.cache)
-                && type_did_to_deref_target.insert(for_did, target).is_none()
-                // 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.
-                && cleaner.keep_impl_with_def_id(for_did.into())
-            {
-                let mut targets = DefIdSet::default();
-                targets.insert(for_did);
-                add_deref_target(
-                    cx,
-                    &type_did_to_deref_target,
-                    &mut cleaner,
-                    &mut targets,
-                    for_did,
-                );
-            }
-        }
-    }
-
-    // Filter out external items that are not needed
-    new_items_external.retain(|it| {
-        if let ImplItem(Impl { ref for_, ref trait_, ref kind, .. }) = it.kind {
-            cleaner.keep_impl(
-                for_,
-                trait_.as_ref().map(|t| t.def_id()) == tcx.lang_items().deref_trait(),
-            ) || trait_.as_ref().is_some_and(|t| cleaner.keep_impl_with_def_id(t.def_id().into()))
-                || kind.is_blanket()
-        } else {
-            true
-        }
-    });
-
     if let ModuleItem(Module { items, .. }) = &mut krate.module.inner.kind {
         items.extend(synth_impls);
         items.extend(new_items_external);
@@ -264,28 +201,3 @@ impl DocVisitor<'_> for ItemAndAliasCollector<'_> {
         self.visit_item_recur(i)
     }
 }
-
-struct BadImplStripper<'a> {
-    prims: FxHashSet<PrimitiveType>,
-    items: FxHashSet<ItemId>,
-    cache: &'a Cache,
-}
-
-impl BadImplStripper<'_> {
-    fn keep_impl(&self, ty: &Type, is_deref: bool) -> bool {
-        if let Generic(_) = ty {
-            // keep impls made on generics
-            true
-        } else if let Some(prim) = ty.primitive_type() {
-            self.prims.contains(&prim)
-        } else if let Some(did) = ty.def_id(self.cache) {
-            is_deref || self.keep_impl_with_def_id(did.into())
-        } else {
-            false
-        }
-    }
-
-    fn keep_impl_with_def_id(&self, item_id: ItemId) -> bool {
-        self.items.contains(&item_id)
-    }
-}

tests/rustdoc-html/notable-trait/spotlight-from-dependency.odd.html

@@ -5,7 +5,7 @@
     // Provided methods
     fn <a href="#method.func" class="fn">func</a>(self)
        <span class="where">where Self: <a class="trait" href="{{channel}}/core/marker/trait.Sized.html" title="trait core::marker::Sized">Sized</a></span> { ... }
-<span class="item-spacer" />    fn <a href="#method.lines" class="fn">lines</a>(self) -&gt; <a class="struct" href="{{channel}}/alloc/io/util/struct.Lines.html" title="struct alloc::io::util::Lines">Lines</a>&lt;Self&gt;
+<span class="item-spacer" />    fn <a href="#method.lines" class="fn">lines</a>(self) -&gt; <a class="struct" href="{{channel}}/alloc/io/util/struct.Lines.html" title="struct alloc::io::util::Lines">Lines</a>&lt;Self&gt; <a href="#" class="tooltip" data-notable-ty="Lines&lt;Self&gt;">&#9432;</a>
        <span class="where">where Self: <a class="trait" href="{{channel}}/core/marker/trait.Sized.html" title="trait core::marker::Sized">Sized</a></span> { ... }
 <span class="item-spacer" />    fn <a href="#method.merge" class="fn">merge</a>&lt;T&gt;(self, a: T)
        <span class="where">where Self: <a class="trait" href="{{channel}}/core/marker/trait.Sized.html" title="trait core::marker::Sized">Sized</a>,