From 90e0fedb5622e2043042927f9accde4aa5ed1e9d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 9 Oct 2020 11:16:57 +0200 Subject: [PATCH 1/6] Clean up check_full_res --- .../passes/collect_intra_doc_links.rs | 35 +++++++------------ 1 file changed, 13 insertions(+), 22 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index fb79272768e87..d3efbc3f53466 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -571,30 +571,21 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { current_item: &Option, extra_fragment: &Option, ) -> Option { - let check_full_res_inner = |this: &Self, result: Result>| { - let res = match result { - Ok(res) => Some(res), - Err(ErrorKind::Resolve(box kind)) => kind.full_res(), - Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res))) => { - Some(res) - } - Err(ErrorKind::AnchorFailure(AnchorFailure::MultipleAnchors)) => None, - }; - this.kind_side_channel.take().map(|(kind, id)| Res::Def(kind, id)).or(res) - }; - // cannot be used for macro namespace - let check_full_res = |this: &Self, ns| { - let result = this.resolve(path_str, ns, current_item, module_id, extra_fragment); - check_full_res_inner(this, result.map(|(res, _)| res)) + // resolve can't be used for macro namespace + let result = match ns { + Namespace::MacroNS => self.macro_resolve(path_str, module_id).map_err(ErrorKind::from), + Namespace::TypeNS | Namespace::ValueNS => self + .resolve(path_str, ns, current_item, module_id, extra_fragment) + .map(|(res, _)| res), }; - let check_full_res_macro = |this: &Self| { - let result = this.macro_resolve(path_str, module_id); - check_full_res_inner(this, result.map_err(ErrorKind::from)) + + let res = match result { + Ok(res) => Some(res), + Err(ErrorKind::Resolve(box kind)) => kind.full_res(), + Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res))) => Some(res), + Err(ErrorKind::AnchorFailure(AnchorFailure::MultipleAnchors)) => None, }; - match ns { - Namespace::MacroNS => check_full_res_macro(self), - Namespace::TypeNS | Namespace::ValueNS => check_full_res(self, ns), - } + self.kind_side_channel.take().map(|(kind, id)| Res::Def(kind, id)).or(res) } } From 3a640f2f3af4d7167720f97b24d0d8530e095aa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 9 Oct 2020 12:04:44 +0200 Subject: [PATCH 2/6] Clean up hard to follow control flow --- .../passes/collect_intra_doc_links.rs | 87 ++++++++++--------- 1 file changed, 45 insertions(+), 42 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index d3efbc3f53466..61a61a925b057 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -291,20 +291,40 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { 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 { - Ok((_, Res::Err)) => Err(()), - x => x, + 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 { - let res = res.map_id(|_| panic!("unexpected node_id")); - // In case this is a trait item, skip the - // early return and try looking for the trait. - let value = match res { - Res::Def(DefKind::AssocFn | DefKind::AssocConst, _) => true, - Res::Def(DefKind::AssocTy, _) => false, + if let Ok(res) = result { + match res { + Res::Def(DefKind::AssocFn | DefKind::AssocConst, _) => { + if ns != ValueNS { + return Err(ResolutionFailure::WrongNamespace(res, ns).into()); + } else { + // In case this is a trait item, skip the + // early return and try looking for the trait. + } + } + Res::Def(DefKind::AssocTy, _) => { + if ns == ValueNS { + return Err(ResolutionFailure::WrongNamespace(res, ns).into()); + } else { + // In case this is a trait item, skip the + // early return and try looking for the trait. + } + } Res::Def(DefKind::Variant, _) => { - return handle_variant(cx, res, extra_fragment); + if extra_fragment.is_some() { + return Err(ErrorKind::AnchorFailure( + AnchorFailure::RustdocAnchorConflict(res), + )); + } + return handle_variant(cx, res); } // Not a trait item; just return what we found. Res::PrimTy(ty) => { @@ -321,17 +341,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { _ => { return Ok((res, extra_fragment.clone())); } - }; - - if value != (ns == ValueNS) { - return Err(ResolutionFailure::WrongNamespace(res, ns).into()); } - // FIXME: why is this necessary? - } else if let Some((path, prim)) = is_primitive(path_str, ns) { - if extra_fragment.is_some() { - return Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(prim))); - } - return Ok((prim, Some(path.to_owned()))); } // Try looking for methods and associated items. @@ -1936,21 +1946,17 @@ fn privacy_error( fn handle_variant( cx: &DocContext<'_>, res: Res, - extra_fragment: &Option, ) -> Result<(Res, Option), ErrorKind<'static>> { use rustc_middle::ty::DefIdTree; - if extra_fragment.is_some() { - return Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res))); - } - let parent = if let Some(parent) = cx.tcx.parent(res.def_id()) { - parent - } else { - return Err(ResolutionFailure::NoParentItem.into()); - }; - let parent_def = Res::Def(DefKind::Enum, parent); - let variant = cx.tcx.expect_variant_res(res); - Ok((parent_def, Some(format!("variant.{}", variant.ident.name)))) + cx.tcx.parent(res.def_id()).map_or_else( + || Err(ResolutionFailure::NoParentItem.into()), + |parent| { + let parent_def = Res::Def(DefKind::Enum, parent); + let variant = cx.tcx.expect_variant_res(res); + Ok((parent_def, Some(format!("variant.{}", variant.ident.name)))) + }, + ) } const PRIMITIVES: &[(&str, Res)] = &[ @@ -1970,19 +1976,16 @@ const PRIMITIVES: &[(&str, Res)] = &[ ("f64", Res::PrimTy(hir::PrimTy::Float(rustc_ast::FloatTy::F64))), ("str", Res::PrimTy(hir::PrimTy::Str)), ("bool", Res::PrimTy(hir::PrimTy::Bool)), - ("true", Res::PrimTy(hir::PrimTy::Bool)), - ("false", Res::PrimTy(hir::PrimTy::Bool)), ("char", Res::PrimTy(hir::PrimTy::Char)), ]; fn is_primitive(path_str: &str, ns: Namespace) -> Option<(&'static str, Res)> { - if ns == TypeNS { - PRIMITIVES - .iter() - .filter(|x| x.0 == path_str) - .copied() - .map(|x| if x.0 == "true" || x.0 == "false" { ("bool", x.1) } else { x }) - .next() + if ns == TypeNS { PRIMITIVES.iter().find(|x| x.0 == path_str).copied() } else { None } +} + +fn is_bool_value(path_str: &str, ns: Namespace) -> Option<(&'static str, Res)> { + if ns == TypeNS && (path_str == "true" || path_str == "false") { + Some(("bool", Res::PrimTy(hir::PrimTy::Bool))) } else { None } From 3858c0fdddfffb4252c9b53be6373b7f3727922f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Fri, 9 Oct 2020 17:54:43 +0200 Subject: [PATCH 3/6] Apply suggestions from code review Co-authored-by: Joshua Nelson --- .../passes/collect_intra_doc_links.rs | 29 +++++++++---------- src/test/rustdoc-ui/intra-links-ambiguity.rs | 4 +++ .../rustdoc-ui/intra-links-ambiguity.stderr | 17 ++++++++++- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 61a61a925b057..d00c4f1f0d0a3 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -305,26 +305,19 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { Res::Def(DefKind::AssocFn | DefKind::AssocConst, _) => { if ns != ValueNS { return Err(ResolutionFailure::WrongNamespace(res, ns).into()); - } else { - // In case this is a trait item, skip the - // early return and try looking for the trait. } + // 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 == ValueNS { + if ns != TypeNS { return Err(ResolutionFailure::WrongNamespace(res, ns).into()); - } else { - // In case this is a trait item, skip the - // early return and try looking for the trait. } + // Fall through: In case this is a trait item, skip the + // early return and try looking for the trait. } Res::Def(DefKind::Variant, _) => { - if extra_fragment.is_some() { - return Err(ErrorKind::AnchorFailure( - AnchorFailure::RustdocAnchorConflict(res), - )); - } - return handle_variant(cx, res); + return handle_variant(cx, res, extra_fragment); } // Not a trait item; just return what we found. Res::PrimTy(ty) => { @@ -581,7 +574,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { current_item: &Option, extra_fragment: &Option, ) -> Option { - // resolve can't be used for macro namespace + // resolve() can't be used for macro namespace let result = match ns { Namespace::MacroNS => self.macro_resolve(path_str, module_id).map_err(ErrorKind::from), Namespace::TypeNS | Namespace::ValueNS => self @@ -1946,9 +1939,13 @@ fn privacy_error( fn handle_variant( cx: &DocContext<'_>, res: Res, + extra_fragment: &Option, ) -> Result<(Res, Option), ErrorKind<'static>> { use rustc_middle::ty::DefIdTree; + if extra_fragment.is_some() { + return Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res))); + } cx.tcx.parent(res.def_id()).map_or_else( || Err(ResolutionFailure::NoParentItem.into()), |parent| { @@ -1980,7 +1977,9 @@ const PRIMITIVES: &[(&str, Res)] = &[ ]; fn is_primitive(path_str: &str, ns: Namespace) -> Option<(&'static str, Res)> { - if ns == TypeNS { PRIMITIVES.iter().find(|x| x.0 == path_str).copied() } else { None } + is_bool_value(path_str, ns).or_else(|| { + if ns == TypeNS { PRIMITIVES.iter().find(|x| x.0 == path_str).copied() } else { None } + }) } fn is_bool_value(path_str: &str, ns: Namespace) -> Option<(&'static str, Res)> { diff --git a/src/test/rustdoc-ui/intra-links-ambiguity.rs b/src/test/rustdoc-ui/intra-links-ambiguity.rs index d1597cdca66ab..f63435337cfbc 100644 --- a/src/test/rustdoc-ui/intra-links-ambiguity.rs +++ b/src/test/rustdoc-ui/intra-links-ambiguity.rs @@ -34,3 +34,7 @@ pub mod foo { /// /// Ambiguous non-implied shortcut link [`foo::bar`]. //~ERROR `foo::bar` pub struct Docs {} + +/// [true] //~ ERROR `true` is both a module and a builtin type +/// [primitive@true] +pub mod r#true {} diff --git a/src/test/rustdoc-ui/intra-links-ambiguity.stderr b/src/test/rustdoc-ui/intra-links-ambiguity.stderr index 17891ca05efa1..21b92a9673755 100644 --- a/src/test/rustdoc-ui/intra-links-ambiguity.stderr +++ b/src/test/rustdoc-ui/intra-links-ambiguity.stderr @@ -82,5 +82,20 @@ help: to link to the function, add parentheses LL | /// Ambiguous non-implied shortcut link [`foo::bar()`]. | ^^^^^^^^^^^^ -error: aborting due to 5 previous errors +error: `true` is both a module and a builtin type + --> $DIR/intra-links-ambiguity.rs:38:6 + | +LL | /// [true] + | ^^^^ ambiguous link + | +help: to link to the module, prefix with `mod@` + | +LL | /// [mod@true] + | ^^^^^^^^ +help: to link to the builtin type, prefix with `prim@` + | +LL | /// [prim@true] + | ^^^^^^^^^ + +error: aborting due to 6 previous errors From a5fbfab8c0c2f36dbb631d77df272b3965bd074c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Sat, 10 Oct 2020 02:13:34 +0200 Subject: [PATCH 4/6] Refactor path resolution and use Symbols instead of &str Co-authored-by: Joshua Nelson --- src/librustdoc/clean/types.rs | 23 ++ .../passes/collect_intra_doc_links.rs | 228 +++++++++--------- 2 files changed, 136 insertions(+), 115 deletions(-) diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 903f44a0f93df..1e07f8e2eac24 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 { 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 d00c4f1f0d0a3..5f3048e986bf8 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), 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 { + 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), 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> { let mut stripped_segments = vec![]; let mut path = path_str.chars().peekable(); From b385598c9658fef4a143a770c21db38b7872c134 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Sat, 10 Oct 2020 09:30:10 +0200 Subject: [PATCH 5/6] Apply suggestions from code review Co-authored-by: Joshua Nelson --- .../passes/collect_intra_doc_links.rs | 38 +++++++++++-------- 1 file changed, 22 insertions(+), 16 deletions(-) diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 5f3048e986bf8..8be9482acffde 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -238,7 +238,6 @@ 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, @@ -263,7 +262,12 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ty::AssocKind::Const => "associatedconstant", ty::AssocKind::Type => "associatedtype", }) - .map(|out| (prim, Some(format!("{}#{}.{}", prim_ty.name(), out, item_str)))) + .map(|out| { + ( + Res::PrimTy(prim_ty), + Some(format!("{}#{}.{}", prim_ty.name(), out, item_str)), + ) + }) }) .ok_or_else(|| { debug!( @@ -274,7 +278,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ); ResolutionFailure::NotResolved { module_id, - partial_res: Some(prim), + partial_res: Some(Res::PrimTy(prim_ty)), unresolved: item_str.into(), } .into() @@ -328,10 +332,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }); 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::Err) | Err(()) => is_bool_value(path_str, ns).map(|(_, res)| res), Ok(res) => Some(res.map_id(|_| panic!("unexpected node_id"))), } } @@ -406,6 +409,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } })?; + // FIXME: are these both necessary? 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)) @@ -426,9 +430,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }; 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::PrimTy(prim) => Some( + self.resolve_primitive_associated_item(prim, 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` @@ -1086,7 +1090,7 @@ impl LinkCollector<'_, '_> { return None; } res = prim; - fragment = Some((*path.as_str()).to_owned()); + fragment = Some(path.as_str().to_string()); } else { // `[char]` when a `char` module is in scope let candidates = vec![res, prim]; @@ -1943,14 +1947,14 @@ fn handle_variant( if extra_fragment.is_some() { return Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res))); } - cx.tcx.parent(res.def_id()).map_or_else( - || Err(ResolutionFailure::NoParentItem.into()), - |parent| { + cx.tcx + .parent(res.def_id()) + .map(|parent| { let parent_def = Res::Def(DefKind::Enum, parent); let variant = cx.tcx.expect_variant_res(res); - Ok((parent_def, Some(format!("variant.{}", variant.ident.name)))) - }, - ) + (parent_def, Some(format!("variant.{}", variant.ident.name))) + }) + .ok_or_else(|| ResolutionFailure::NoParentItem.into()) } // FIXME: At this point, this is basically a copy of the PrimitiveTypeTable @@ -1977,7 +1981,9 @@ const PRIMITIVES: &[(Symbol, 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.as_str() == path_str).copied() + // FIXME: this should be replaced by a lookup in PrimitiveTypeTable + let maybe_primitive = Symbol::intern(path_str); + PRIMITIVES.iter().find(|x| x.0 == maybe_primitive).copied() } else { None } From 725e3d1df30bc39fe76510c92679fbcb3536f4cb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?D=C3=A1niel=20Buga?= Date: Sat, 10 Oct 2020 10:20:10 +0200 Subject: [PATCH 6/6] Re-enable test case --- src/test/rustdoc/primitive-link.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/test/rustdoc/primitive-link.rs b/src/test/rustdoc/primitive-link.rs index 8f69b894a223d..3041ff77684ca 100644 --- a/src/test/rustdoc/primitive-link.rs +++ b/src/test/rustdoc/primitive-link.rs @@ -7,8 +7,7 @@ // @has foo/struct.Foo.html '//*[@class="docblock"]/p/a[@href="https://doc.rust-lang.org/nightly/std/primitive.i32.html"]' 'std::primitive::i32' // @has foo/struct.Foo.html '//*[@class="docblock"]/p/a[@href="https://doc.rust-lang.org/nightly/std/primitive.str.html"]' 'std::primitive::str' -// FIXME: this doesn't resolve -// @ has foo/struct.Foo.html '//*[@class="docblock"]/p/a[@href="https://doc.rust-lang.org/nightly/std/primitive.i32.html#associatedconstant.MAX"]' 'std::primitive::i32::MAX' +// @has foo/struct.Foo.html '//*[@class="docblock"]/p/a[@href="https://doc.rust-lang.org/nightly/std/primitive.i32.html#associatedconstant.MAX"]' 'std::primitive::i32::MAX' /// It contains [`u32`] and [i64]. /// It also links to [std::primitive::i32], [std::primitive::str],