rustdoc: Cleanup CacheBuilder code for building search index

[camelid]

This code was very convoluted and hard to reason about. It is now (I hope) much
clearer and more suitable for both future enhancements and future cleanups.

I'm doing this as a precursor, with no UI changes, to changing rustdoc to
ignore blanket impls in type-based search.

r? @notriddle

[camelid]

I realized that these changes are actually completely independent, so this is no longer based on the Self type PR. It will have a small conflict once the other is merged, though, since there is one function that is changed in both.

[camelid]

Well that's weird... those tests passed locally.

[notriddle]

Here's a suggested fix, along with a much longer comment for a remaining, ugly fact of the search engine builder:

diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs
index d2d0f5a4380..6454bc58cd6 100644
--- a/src/librustdoc/formats/cache.rs
+++ b/src/librustdoc/formats/cache.rs
@@ -486,10 +486,26 @@ fn add_item_to_search_index(tcx: TyCtxt<'_>, cache: &mut Cache, item: &clean::It
                 ParentStackItem::Type(item_id) => item_id.as_def_id(),
             };
             let Some(parent_did) = parent_did else { return };
-            // The current stack not necessarily has correlation
-            // for where the type was defined. On the other
-            // hand, `paths` always has the right
-            // information if present.
+            // The current stack reflects the CacheBuilder's recursive
+            // walk over HIR. For associated items, this is the module
+            // where the `impl` block is defined. That's an implementation
+            // detail that we don't want to affect the search engine.
+            //
+            // In particular, you can arrange things like this:
+            //
+            //     #![crate_name="me"]
+            //     mod private_mod {
+            //         impl Clone for MyThing { fn clone(&self) -> MyThing { MyThing } }
+            //     }
+            //     pub struct MyThing;
+            //
+            // When that happens, we need to:
+            // - ignore the `cache.stripped_mod` flag, since the Clone impl is actually
+            //   part of the public API even though it's defined in a private module
+            // - present the method as `me::MyThing::clone`, its publicly-visible path
+            // - deal with the fact that the recursive walk hasn't actually reached `MyThing`
+            //   until it's already past `private_mod`, since that's first, and doesn't know
+            //   yet if `MyThing` will actually be public or not (it could be re-exported)
             match cache.paths.get(&parent_did) {
                 Some((fqp, _)) => (Some(parent_did), &fqp[..fqp.len() - 1]),
                 None => {
@@ -500,7 +516,8 @@ fn add_item_to_search_index(tcx: TyCtxt<'_>, cache: &mut Cache, item: &clean::It
         }
         _ => {
             // Don't index if item is crate root, which is inserted later on when serializing the index.
-            if item_def_id.is_crate_root() {
+            // Don't index if containing module is stripped (i.e., private),
+            if item_def_id.is_crate_root() || cache.stripped_mod {
                 return;
             }
             (None, &*cache.stack)

[notriddle]

@bors r+

[bors]

📌 Commit 007d9e1 has been approved by notriddle

It is now in the queue for this repository.