From 3f21c31bf4f3ebab5e982a2cff8befbf1763294b Mon Sep 17 00:00:00 2001
From: Vadim Petrochenkov <vadim.petrochenkov@gmail.com>
Date: Sat, 21 May 2022 15:35:07 +0300
Subject: [PATCH] rustdoc: Some link resolution caching cleanup

---
 .../passes/collect_intra_doc_links.rs         | 51 ++++++-------------
 1 file changed, 16 insertions(+), 35 deletions(-)

diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs
index b20402e532a..ffc765f4040 100644
--- a/src/librustdoc/passes/collect_intra_doc_links.rs
+++ b/src/librustdoc/passes/collect_intra_doc_links.rs
@@ -277,11 +277,6 @@ struct DiagnosticInfo<'a> {
     link_range: Range<usize>,
 }
 
-#[derive(Clone, Debug, Hash)]
-struct CachedLink {
-    res: (Res, Option<UrlFragment>),
-}
-
 struct LinkCollector<'a, 'tcx> {
     cx: &'a mut DocContext<'tcx>,
     /// A stack of modules used to decide what scope to resolve in.
@@ -291,7 +286,7 @@ struct LinkCollector<'a, 'tcx> {
     mod_ids: Vec<DefId>,
     /// Cache the resolved links so we can avoid resolving (and emitting errors for) the same link.
     /// The link will be `None` if it could not be resolved (i.e. the error was cached).
-    visited_links: FxHashMap<ResolutionInfo, Option<CachedLink>>,
+    visited_links: FxHashMap<ResolutionInfo, Option<(Res, Option<UrlFragment>)>>,
 }
 
 impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
@@ -1060,6 +1055,9 @@ impl LinkCollector<'_, '_> {
                 extra_fragment: extra_fragment.clone(),
             },
             diag_info.clone(), // this struct should really be Copy, but Range is not :(
+            // For reference-style links we want to report only one error so unsuccessful
+            // resolutions are cached, for other links we want to report an error every
+            // time so they are not cached.
             matches!(ori_link.kind, LinkType::Reference | LinkType::Shortcut),
         )?;
 
@@ -1256,26 +1254,20 @@ impl LinkCollector<'_, '_> {
         &mut self,
         key: ResolutionInfo,
         diag: DiagnosticInfo<'_>,
-        cache_resolution_failure: bool,
+        // If errors are cached then they are only reported on first ocurrence
+        // which we want in some cases but not in others.
+        cache_errors: bool,
     ) -> Option<(Res, Option<UrlFragment>)> {
-        if let Some(ref cached) = self.visited_links.get(&key) {
-            match cached {
-                Some(cached) => {
-                    return Some(cached.res.clone());
-                }
-                None if cache_resolution_failure => return None,
-                None => {
-                    // Although we hit the cache and found a resolution error, this link isn't
-                    // supposed to cache those. Run link resolution again to emit the expected
-                    // resolution error.
-                }
+        if let Some(res) = self.visited_links.get(&key) {
+            if res.is_some() || cache_errors {
+                return res.clone();
             }
         }
 
         let res = self.resolve_with_disambiguator(&key, diag.clone()).and_then(|(res, def_id)| {
             let fragment = match (&key.extra_fragment, def_id) {
                 (Some(_), Some(def_id)) => {
-                    report_anchor_conflict(self.cx, diag, Res::from_def_id(self.cx.tcx, def_id));
+                    report_anchor_conflict(self.cx, diag, def_id);
                     return None;
                 }
                 (Some(u_frag), None) => Some(UrlFragment::UserWritten(u_frag.clone())),
@@ -1285,21 +1277,10 @@ impl LinkCollector<'_, '_> {
             Some((res, fragment))
         });
 
-        // Cache only if resolved successfully - don't silence duplicate errors
-        if let Some(res) = res {
-            // Store result for the actual namespace
-            self.visited_links.insert(key, Some(CachedLink { res: res.clone() }));
-
-            Some(res)
-        } else {
-            if cache_resolution_failure {
-                // For reference-style links we only want to report one resolution error
-                // so let's cache them as well.
-                self.visited_links.insert(key, None);
-            }
-
-            None
+        if res.is_some() || cache_errors {
+            self.visited_links.insert(key, res.clone());
         }
+        res
     }
 
     /// After parsing the disambiguator, resolve the main part of the link.
@@ -1875,8 +1856,8 @@ fn report_multiple_anchors(cx: &DocContext<'_>, diag_info: DiagnosticInfo<'_>) {
     anchor_failure(cx, diag_info, &msg, 1)
 }
 
-fn report_anchor_conflict(cx: &DocContext<'_>, diag_info: DiagnosticInfo<'_>, res: Res) {
-    let (link, kind) = (diag_info.ori_link, res.descr());
+fn report_anchor_conflict(cx: &DocContext<'_>, diag_info: DiagnosticInfo<'_>, def_id: DefId) {
+    let (link, kind) = (diag_info.ori_link, Res::from_def_id(cx.tcx, def_id).descr());
     let msg = format!("`{link}` contains an anchor, but links to {kind}s are already anchored");
     anchor_failure(cx, diag_info, &msg, 0)
 }