From 5a9bbba2809e1a7fcf381f1e04cb81e5074e1f55 Mon Sep 17 00:00:00 2001
From: Noah Lev <camelidcamel@gmail.com>
Date: Sun, 31 Oct 2021 20:18:52 -0700
Subject: [PATCH] rustdoc: Add `DocVisitor`

`DocFolder` allows transforming the docs, accomplished by making its
methods take and return types by-value. However, several of the rustdoc
`DocFolder` impls only *visit* the docs; they don't change anything.
Passing around types by-value is thus unnecessary, confusing, and
potentially inefficient for those impls.

`DocVisitor` is very similar to `DocFolder`, except that its methods
take shared references and return nothing (i.e., the unit type). This
should both be more efficient and make the code clearer.

There is an additional reason to add `DocVisitor`, too. As part of my
cleanup of `external_traits`, I'm planning to add a `fn cache(&mut self)
-> &mut Cache` method to `DocFolder` so that `external_traits` can be
retrieved explicitly from the `Cache`, rather than implicitly via
`Crate.external_traits` (which is an `Rc<RefCell<...>>`). However, some
of the `DocFolder` impls that could be turned into `DocVisitor` impls
only have a shared reference to the `Cache`, because they are used
during rendering. (They have to access the `Cache` via
`html::render::Context.shared.cache`, which involves an `Rc`.)

Since `DocVisitor` does not mutate any of the types it's visiting, its
equivalent `cache()` method will only need a shared reference to the
`Cache`, avoiding the problem described above.
---
 src/librustdoc/fold.rs  | 29 +++++++++++------------
 src/librustdoc/lib.rs   |  1 +
 src/librustdoc/visit.rs | 51 +++++++++++++++++++++++++++++++++++++++++
 3 files changed, 66 insertions(+), 15 deletions(-)
 create mode 100644 src/librustdoc/visit.rs

diff --git a/src/librustdoc/fold.rs b/src/librustdoc/fold.rs
index 4984d917124..9009a9bd41b 100644
--- a/src/librustdoc/fold.rs
+++ b/src/librustdoc/fold.rs
@@ -46,22 +46,21 @@ crate trait DocFolder: Sized {
                 i.items = i.items.into_iter().filter_map(|x| self.fold_item(x)).collect();
                 ImplItem(i)
             }
-            VariantItem(i) => {
-                match i {
-                    Variant::Struct(mut j) => {
-                        let num_fields = j.fields.len();
-                        j.fields = j.fields.into_iter().filter_map(|x| self.fold_item(x)).collect();
-                        j.fields_stripped |= num_fields != j.fields.len()
-                            || j.fields.iter().any(|f| f.is_stripped());
-                        VariantItem(Variant::Struct(j))
-                    }
-                    Variant::Tuple(fields) => {
-                        let fields = fields.into_iter().filter_map(|x| self.fold_item(x)).collect();
-                        VariantItem(Variant::Tuple(fields))
-                    }
-                    Variant::CLike => VariantItem(Variant::CLike),
+            VariantItem(i) => match i {
+                Variant::Struct(mut j) => {
+                    let num_fields = j.fields.len();
+                    j.fields = j.fields.into_iter().filter_map(|x| self.fold_item(x)).collect();
+                    j.fields_stripped |=
+                        num_fields != j.fields.len() || j.fields.iter().any(|f| f.is_stripped());
+                    VariantItem(Variant::Struct(j))
                 }
-            }
+                Variant::Tuple(fields) => {
+                    let fields = fields.into_iter().filter_map(|x| self.fold_item(x)).collect();
+                    VariantItem(Variant::Tuple(fields))
+                }
+                Variant::CLike => VariantItem(Variant::CLike),
+            },
+            // FIXME: list all cases explicitly
             x => x,
         }
     }
diff --git a/src/librustdoc/lib.rs b/src/librustdoc/lib.rs
index b50fbf58bae..7eeb9d1fcaa 100644
--- a/src/librustdoc/lib.rs
+++ b/src/librustdoc/lib.rs
@@ -121,6 +121,7 @@ mod markdown;
 mod passes;
 mod scrape_examples;
 mod theme;
+mod visit;
 mod visit_ast;
 mod visit_lib;
 
diff --git a/src/librustdoc/visit.rs b/src/librustdoc/visit.rs
new file mode 100644
index 00000000000..9edf6e11bec
--- /dev/null
+++ b/src/librustdoc/visit.rs
@@ -0,0 +1,51 @@
+use crate::clean::*;
+
+crate trait DocVisitor: Sized {
+    fn visit_item(&mut self, item: &Item) {
+        self.visit_item_recur(item)
+    }
+
+    /// don't override!
+    fn visit_inner_recur(&mut self, kind: &ItemKind) {
+        match kind {
+            StrippedItem(..) => unreachable!(),
+            ModuleItem(i) => {
+                self.visit_mod(i);
+                return;
+            }
+            StructItem(i) => i.fields.iter().for_each(|x| self.visit_item(x)),
+            UnionItem(i) => i.fields.iter().for_each(|x| self.visit_item(x)),
+            EnumItem(i) => i.variants.iter().for_each(|x| self.visit_item(x)),
+            TraitItem(i) => i.items.iter().for_each(|x| self.visit_item(x)),
+            ImplItem(i) => i.items.iter().for_each(|x| self.visit_item(x)),
+            VariantItem(i) => match i {
+                Variant::Struct(j) => j.fields.iter().for_each(|x| self.visit_item(x)),
+                Variant::Tuple(fields) => fields.iter().for_each(|x| self.visit_item(x)),
+                Variant::CLike => {}
+            },
+            // FIXME: list all cases explicitly
+            _ => return,
+        }
+    }
+
+    /// don't override!
+    fn visit_item_recur(&mut self, item: &Item) {
+        match &*item.kind {
+            StrippedItem(i) => self.visit_inner_recur(i),
+            _ => self.visit_inner_recur(&item.kind),
+        }
+    }
+
+    fn visit_mod(&mut self, m: &Module) {
+        m.items.iter().for_each(|i| self.visit_item(i))
+    }
+
+    fn visit_crate(&mut self, c: &Crate) {
+        self.visit_item(&c.module);
+
+        let external_traits = c.external_traits.borrow();
+        for v in external_traits.values() {
+            v.trait_.items.iter().for_each(|i| self.visit_item(i))
+        }
+    }
+}