From 90e0fedb5622e2043042927f9accde4aa5ed1e9d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?D=C3=A1niel=20Buga?= <bugadani@gmail.com>
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<String>,
extra_fragment: &Option<String>,
) -> Option<Res> {
- let check_full_res_inner = |this: &Self, result: Result<Res, ErrorKind<'_>>| {
- 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?= <bugadani@gmail.com>
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<String>,
) -> Result<(Res, Option<String>), 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?= <bugadani@gmail.com>
Date: Fri, 9 Oct 2020 17:54:43 +0200
Subject: [PATCH 3/6] Apply suggestions from code review
Co-authored-by: Joshua Nelson <joshua@yottadb.com>
---
.../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<String>,
extra_fragment: &Option<String>,
) -> Option<Res> {
- // 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<String>,
) -> Result<(Res, Option<String>), 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?= <bugadani@gmail.com>
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 <joshua@yottadb.com>
---
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<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 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<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();
From b385598c9658fef4a143a770c21db38b7872c134 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?D=C3=A1niel=20Buga?= <bugadani@gmail.com>
Date: Sat, 10 Oct 2020 09:30:10 +0200
Subject: [PATCH 5/6] Apply suggestions from code review
Co-authored-by: Joshua Nelson <joshua@yottadb.com>
---
.../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?= <bugadani@gmail.com>
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],