Refactor path resolution and use Symbols instead of &str

Co-authored-by: Joshua Nelson <joshua@yottadb.com>

2 files changed, 136 insertions, 115 deletions

diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
index 903f44a0f93..1e07f8e2eac 100644
--- a/src/librustdoc/clean/types.rs
+++ b/src/librustdoc/clean/types.rs
@@ -12,6 +12,7 @@ use std::{slice, vec};
 use rustc_ast::attr;
 use rustc_ast::util::comments::beautify_doc_string;
 use rustc_ast::{self as ast, AttrStyle};
+use rustc_ast::{FloatTy, IntTy, UintTy};
 use rustc_data_structures::fx::{FxHashMap, FxHashSet};
 use rustc_hir as hir;
 use rustc_hir::def::Res;
@@ -1279,6 +1280,28 @@ impl GetDefId for Type {
 }
 
 impl PrimitiveType {
+    pub fn from_hir(prim: hir::PrimTy) -> PrimitiveType {
+        match prim {
+            hir::PrimTy::Int(IntTy::Isize) => PrimitiveType::Isize,
+            hir::PrimTy::Int(IntTy::I8) => PrimitiveType::I8,
+            hir::PrimTy::Int(IntTy::I16) => PrimitiveType::I16,
+            hir::PrimTy::Int(IntTy::I32) => PrimitiveType::I32,
+            hir::PrimTy::Int(IntTy::I64) => PrimitiveType::I64,
+            hir::PrimTy::Int(IntTy::I128) => PrimitiveType::I128,
+            hir::PrimTy::Uint(UintTy::Usize) => PrimitiveType::Usize,
+            hir::PrimTy::Uint(UintTy::U8) => PrimitiveType::U8,
+            hir::PrimTy::Uint(UintTy::U16) => PrimitiveType::U16,
+            hir::PrimTy::Uint(UintTy::U32) => PrimitiveType::U32,
+            hir::PrimTy::Uint(UintTy::U64) => PrimitiveType::U64,
+            hir::PrimTy::Uint(UintTy::U128) => PrimitiveType::U128,
+            hir::PrimTy::Float(FloatTy::F32) => PrimitiveType::F32,
+            hir::PrimTy::Float(FloatTy::F64) => PrimitiveType::F64,
+            hir::PrimTy::Str => PrimitiveType::Str,
+            hir::PrimTy::Bool => PrimitiveType::Bool,
+            hir::PrimTy::Char => PrimitiveType::Char,
+        }
+    }
+
     pub fn from_symbol(s: Symbol) -> Option<PrimitiveType> {
         match s {
             sym::isize => Some(PrimitiveType::Isize),
diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs
index d00c4f1f0d0..5f3048e986b 100644
--- a/src/librustdoc/passes/collect_intra_doc_links.rs
+++ b/src/librustdoc/passes/collect_intra_doc_links.rs
@@ -16,6 +16,7 @@ use rustc_session::lint::{
     Lint,
 };
 use rustc_span::hygiene::MacroKind;
+use rustc_span::symbol::sym;
 use rustc_span::symbol::Ident;
 use rustc_span::symbol::Symbol;
 use rustc_span::DUMMY_SP;
@@ -234,6 +235,52 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
         }
     }
 
+    fn resolve_primitive_associated_item(
+        &self,
+        prim_ty: hir::PrimTy,
+        prim: Res,
+        ns: Namespace,
+        module_id: DefId,
+        item_name: Symbol,
+        item_str: &'path str,
+    ) -> Result<(Res, Option<String>), ErrorKind<'path>> {
+        let cx = self.cx;
+
+        PrimitiveType::from_hir(prim_ty)
+            .impls(cx.tcx)
+            .into_iter()
+            .find_map(|&impl_| {
+                cx.tcx
+                    .associated_items(impl_)
+                    .find_by_name_and_namespace(
+                        cx.tcx,
+                        Ident::with_dummy_span(item_name),
+                        ns,
+                        impl_,
+                    )
+                    .map(|item| match item.kind {
+                        ty::AssocKind::Fn => "method",
+                        ty::AssocKind::Const => "associatedconstant",
+                        ty::AssocKind::Type => "associatedtype",
+                    })
+                    .map(|out| (prim, Some(format!("{}#{}.{}", prim_ty.name(), out, item_str))))
+            })
+            .ok_or_else(|| {
+                debug!(
+                    "returning primitive error for {}::{} in {} namespace",
+                    prim_ty.name(),
+                    item_name,
+                    ns.descr()
+                );
+                ResolutionFailure::NotResolved {
+                    module_id,
+                    partial_res: Some(prim),
+                    unresolved: item_str.into(),
+                }
+                .into()
+            })
+    }
+
     /// Resolves a string as a macro.
     fn macro_resolve(
         &self,
@@ -275,6 +322,20 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
         })
     }
 
+    fn resolve_path(&self, path_str: &str, ns: Namespace, module_id: DefId) -> Option<Res> {
+        let result = self.cx.enter_resolver(|resolver| {
+            resolver.resolve_str_path_error(DUMMY_SP, &path_str, ns, module_id)
+        });
+        debug!("{} resolved to {:?} in namespace {:?}", path_str, result, ns);
+        match result.map(|(_, res)| res) {
+            Ok(Res::Err) | Err(()) => is_bool_value(path_str, ns).map(|(_, res)| res),
+
+            // resolver doesn't know about true and false so we'll have to resolve them
+            // manually as bool
+            Ok(res) => Some(res.map_id(|_| panic!("unexpected node_id"))),
+        }
+    }
+
     /// Resolves a string as a path within a particular namespace. Also returns an optional
     /// URL fragment in the case of variants and methods.
     fn resolve<'path>(
@@ -287,32 +348,15 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
     ) -> Result<(Res, Option<String>), ErrorKind<'path>> {
         let cx = self.cx;
 
-        let result = cx.enter_resolver(|resolver| {
-            resolver.resolve_str_path_error(DUMMY_SP, &path_str, ns, module_id)
-        });
-        debug!("{} resolved to {:?} in namespace {:?}", path_str, result, ns);
-        let result = match result.map(|(_, res)| res) {
-            Ok(Res::Err) | Err(()) => {
-                // resolver doesn't know about true and false so we'll have to resolve them
-                // manually as bool
-                if let Some((_, res)) = is_bool_value(path_str, ns) { Ok(res) } else { Err(()) }
-            }
-            Ok(res) => Ok(res.map_id(|_| panic!("unexpected node_id"))),
-        };
-
-        if let Ok(res) = result {
+        if let Some(res) = self.resolve_path(path_str, ns, module_id) {
             match res {
                 Res::Def(DefKind::AssocFn | DefKind::AssocConst, _) => {
-                    if ns != ValueNS {
-                        return Err(ResolutionFailure::WrongNamespace(res, ns).into());
-                    }
+                    assert_eq!(ns, ValueNS);
                     // Fall through: In case this is a trait item, skip the
                     // early return and try looking for the trait.
                 }
                 Res::Def(DefKind::AssocTy, _) => {
-                    if ns != TypeNS {
-                        return Err(ResolutionFailure::WrongNamespace(res, ns).into());
-                    }
+                    assert_eq!(ns, TypeNS);
                     // Fall through: In case this is a trait item, skip the
                     // early return and try looking for the trait.
                 }
@@ -362,70 +406,29 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
                 }
             })?;
 
-        if let Some((path, prim)) = is_primitive(&path_root, TypeNS) {
-            let impls =
-                primitive_impl(cx, &path).ok_or_else(|| ResolutionFailure::NotResolved {
+        let ty_res = if let Some(ty_res) = is_primitive(&path_root, TypeNS)
+            .map(|(_, res)| res)
+            .or_else(|| self.resolve_path(&path_root, TypeNS, module_id))
+        {
+            ty_res
+        } else {
+            // FIXME: this is duplicated on the end of this function.
+            return if ns == Namespace::ValueNS {
+                self.variant_field(path_str, current_item, module_id)
+            } else {
+                Err(ResolutionFailure::NotResolved {
                     module_id,
-                    partial_res: Some(prim),
-                    unresolved: item_str.into(),
-                })?;
-            for &impl_ in impls {
-                let link = cx
-                    .tcx
-                    .associated_items(impl_)
-                    .find_by_name_and_namespace(
-                        cx.tcx,
-                        Ident::with_dummy_span(item_name),
-                        ns,
-                        impl_,
-                    )
-                    .map(|item| match item.kind {
-                        ty::AssocKind::Fn => "method",
-                        ty::AssocKind::Const => "associatedconstant",
-                        ty::AssocKind::Type => "associatedtype",
-                    })
-                    .map(|out| (prim, Some(format!("{}#{}.{}", path, out, item_str))));
-                if let Some(link) = link {
-                    return Ok(link);
+                    partial_res: None,
+                    unresolved: path_root.into(),
                 }
-            }
-            debug!(
-                "returning primitive error for {}::{} in {} namespace",
-                path,
-                item_name,
-                ns.descr()
-            );
-            return Err(ResolutionFailure::NotResolved {
-                module_id,
-                partial_res: Some(prim),
-                unresolved: item_str.into(),
-            }
-            .into());
-        }
-
-        let ty_res = cx
-            .enter_resolver(|resolver| {
-                // only types can have associated items
-                resolver.resolve_str_path_error(DUMMY_SP, &path_root, TypeNS, module_id)
-            })
-            .map(|(_, res)| res);
-        let ty_res = match ty_res {
-            Err(()) | Ok(Res::Err) => {
-                return if ns == Namespace::ValueNS {
-                    self.variant_field(path_str, current_item, module_id)
-                } else {
-                    Err(ResolutionFailure::NotResolved {
-                        module_id,
-                        partial_res: None,
-                        unresolved: path_root.into(),
-                    }
-                    .into())
-                };
-            }
-            Ok(res) => res,
+                .into())
+            };
         };
-        let ty_res = ty_res.map_id(|_| panic!("unexpected node_id"));
+
         let res = match ty_res {
+            Res::PrimTy(prim) => Some(self.resolve_primitive_associated_item(
+                prim, ty_res, ns, module_id, item_name, item_str,
+            )),
             Res::Def(DefKind::Struct | DefKind::Union | DefKind::Enum | DefKind::TyAlias, did) => {
                 debug!("looking for associated item named {} for item {:?}", item_name, did);
                 // Checks if item_name belongs to `impl SomeItem`
@@ -465,7 +468,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
                     Some(if extra_fragment.is_some() {
                         Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(ty_res)))
                     } else {
-                        // HACK(jynelson): `clean` expects the type, not the associated item.
+                        // HACK(jynelson): `clean` expects the type, not the associated item
                         // but the disambiguator logic expects the associated item.
                         // Store the kind in a side channel so that only the disambiguator logic looks at it.
                         self.kind_side_channel.set(Some((kind.as_def_kind(), id)));
@@ -511,13 +514,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> {
                         _ => None,
                     }
                 } else {
-                    // We already know this isn't in ValueNS, so no need to check variant_field
-                    return Err(ResolutionFailure::NotResolved {
-                        module_id,
-                        partial_res: Some(ty_res),
-                        unresolved: item_str.into(),
-                    }
-                    .into());
+                    None
                 }
             }
             Res::Def(DefKind::Trait, did) => cx
@@ -1089,7 +1086,7 @@ impl LinkCollector<'_, '_> {
                         return None;
                     }
                     res = prim;
-                    fragment = Some(path.to_owned());
+                    fragment = Some((*path.as_str()).to_owned());
                 } else {
                     // `[char]` when a `char` module is in scope
                     let candidates = vec![res, prim];
@@ -1956,44 +1953,45 @@ fn handle_variant(
     )
 }
 
-const PRIMITIVES: &[(&str, Res)] = &[
-    ("u8", Res::PrimTy(hir::PrimTy::Uint(rustc_ast::UintTy::U8))),
-    ("u16", Res::PrimTy(hir::PrimTy::Uint(rustc_ast::UintTy::U16))),
-    ("u32", Res::PrimTy(hir::PrimTy::Uint(rustc_ast::UintTy::U32))),
-    ("u64", Res::PrimTy(hir::PrimTy::Uint(rustc_ast::UintTy::U64))),
-    ("u128", Res::PrimTy(hir::PrimTy::Uint(rustc_ast::UintTy::U128))),
-    ("usize", Res::PrimTy(hir::PrimTy::Uint(rustc_ast::UintTy::Usize))),
-    ("i8", Res::PrimTy(hir::PrimTy::Int(rustc_ast::IntTy::I8))),
-    ("i16", Res::PrimTy(hir::PrimTy::Int(rustc_ast::IntTy::I16))),
-    ("i32", Res::PrimTy(hir::PrimTy::Int(rustc_ast::IntTy::I32))),
-    ("i64", Res::PrimTy(hir::PrimTy::Int(rustc_ast::IntTy::I64))),
-    ("i128", Res::PrimTy(hir::PrimTy::Int(rustc_ast::IntTy::I128))),
-    ("isize", Res::PrimTy(hir::PrimTy::Int(rustc_ast::IntTy::Isize))),
-    ("f32", Res::PrimTy(hir::PrimTy::Float(rustc_ast::FloatTy::F32))),
-    ("f64", Res::PrimTy(hir::PrimTy::Float(rustc_ast::FloatTy::F64))),
-    ("str", Res::PrimTy(hir::PrimTy::Str)),
-    ("bool", Res::PrimTy(hir::PrimTy::Bool)),
-    ("char", Res::PrimTy(hir::PrimTy::Char)),
+// FIXME: At this point, this is basically a copy of the PrimitiveTypeTable
+const PRIMITIVES: &[(Symbol, Res)] = &[
+    (sym::u8, Res::PrimTy(hir::PrimTy::Uint(rustc_ast::UintTy::U8))),
+    (sym::u16, Res::PrimTy(hir::PrimTy::Uint(rustc_ast::UintTy::U16))),
+    (sym::u32, Res::PrimTy(hir::PrimTy::Uint(rustc_ast::UintTy::U32))),
+    (sym::u64, Res::PrimTy(hir::PrimTy::Uint(rustc_ast::UintTy::U64))),
+    (sym::u128, Res::PrimTy(hir::PrimTy::Uint(rustc_ast::UintTy::U128))),
+    (sym::usize, Res::PrimTy(hir::PrimTy::Uint(rustc_ast::UintTy::Usize))),
+    (sym::i8, Res::PrimTy(hir::PrimTy::Int(rustc_ast::IntTy::I8))),
+    (sym::i16, Res::PrimTy(hir::PrimTy::Int(rustc_ast::IntTy::I16))),
+    (sym::i32, Res::PrimTy(hir::PrimTy::Int(rustc_ast::IntTy::I32))),
+    (sym::i64, Res::PrimTy(hir::PrimTy::Int(rustc_ast::IntTy::I64))),
+    (sym::i128, Res::PrimTy(hir::PrimTy::Int(rustc_ast::IntTy::I128))),
+    (sym::isize, Res::PrimTy(hir::PrimTy::Int(rustc_ast::IntTy::Isize))),
+    (sym::f32, Res::PrimTy(hir::PrimTy::Float(rustc_ast::FloatTy::F32))),
+    (sym::f64, Res::PrimTy(hir::PrimTy::Float(rustc_ast::FloatTy::F64))),
+    (sym::str, Res::PrimTy(hir::PrimTy::Str)),
+    (sym::bool, Res::PrimTy(hir::PrimTy::Bool)),
+    (sym::char, Res::PrimTy(hir::PrimTy::Char)),
 ];
 
-fn is_primitive(path_str: &str, ns: Namespace) -> Option<(&'static str, Res)> {
+fn is_primitive(path_str: &str, ns: Namespace) -> Option<(Symbol, Res)> {
     is_bool_value(path_str, ns).or_else(|| {
-        if ns == TypeNS { PRIMITIVES.iter().find(|x| x.0 == path_str).copied() } else { None }
+        if ns == TypeNS {
+            PRIMITIVES.iter().find(|x| x.0.as_str() == path_str).copied()
+        } else {
+            None
+        }
     })
 }
 
-fn is_bool_value(path_str: &str, ns: Namespace) -> Option<(&'static str, Res)> {
+fn is_bool_value(path_str: &str, ns: Namespace) -> Option<(Symbol, Res)> {
     if ns == TypeNS && (path_str == "true" || path_str == "false") {
-        Some(("bool", Res::PrimTy(hir::PrimTy::Bool)))
+        Some((sym::bool, Res::PrimTy(hir::PrimTy::Bool)))
     } else {
         None
     }
 }
 
-fn primitive_impl(cx: &DocContext<'_>, path_str: &str) -> Option<&'static SmallVec<[DefId; 4]>> {
-    Some(PrimitiveType::from_symbol(Symbol::intern(path_str))?.impls(cx.tcx))
-}
-
 fn strip_generics_from_path(path_str: &str) -> Result<String, ResolutionFailure<'static>> {
     let mut stripped_segments = vec![];
     let mut path = path_str.chars().peekable();