diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 491f7b2fa69..00c6e38839f 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -1,6 +1,5 @@ use std::cell::RefCell; use std::default::Default; -use std::fmt::Write; use std::hash::Hash; use std::lazy::SyncOnceCell as OnceCell; use std::path::PathBuf; @@ -496,7 +495,7 @@ impl Item { if let Ok((mut href, ..)) = href(*did, cx) { debug!(?href); if let Some(ref fragment) = *fragment { - write!(href, "{}", fragment).unwrap() + fragment.render(&mut href, cx.tcx()).unwrap() } Some(RenderedLink { original_text: s.clone(), diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 49ff4517a4e..9d1a8b3f80f 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -13,7 +13,7 @@ use rustc_hir::def::{ PerNS, }; use rustc_hir::def_id::{CrateNum, DefId}; -use rustc_middle::ty::{Ty, TyCtxt}; +use rustc_middle::ty::{DefIdTree, Ty, TyCtxt}; use rustc_middle::{bug, span_bug, ty}; use rustc_resolve::ParentScope; use rustc_session::lint::Lint; @@ -25,8 +25,8 @@ use smallvec::{smallvec, SmallVec}; use pulldown_cmark::LinkType; use std::borrow::Cow; -use std::cell::Cell; use std::convert::{TryFrom, TryInto}; +use std::fmt::Write; use std::mem; use std::ops::Range; @@ -47,12 +47,8 @@ crate const COLLECT_INTRA_DOC_LINKS: Pass = Pass { }; fn collect_intra_doc_links(krate: Crate, cx: &mut DocContext<'_>) -> Crate { - let mut collector = LinkCollector { - cx, - mod_ids: Vec::new(), - kind_side_channel: Cell::new(None), - visited_links: FxHashMap::default(), - }; + let mut collector = + LinkCollector { cx, mod_ids: Vec::new(), visited_links: FxHashMap::default() }; collector.visit_crate(&krate); krate } @@ -240,53 +236,73 @@ enum AnchorFailure { #[derive(Clone, Debug, Hash, PartialEq, Eq)] crate enum UrlFragment { - Method(Symbol), - TyMethod(Symbol), - AssociatedConstant(Symbol), - AssociatedType(Symbol), - - StructField(Symbol), - Variant(Symbol), - VariantField { variant: Symbol, field: Symbol }, - + Item(ItemFragment), UserWritten(String), } impl UrlFragment { - /// Create a fragment for an associated item. - /// - /// `is_prototype` is whether this associated item is a trait method - /// without a default definition. - fn from_assoc_item(name: Symbol, kind: ty::AssocKind, is_prototype: bool) -> Self { - match kind { - ty::AssocKind::Fn => { - if is_prototype { - UrlFragment::TyMethod(name) - } else { - UrlFragment::Method(name) - } - } - ty::AssocKind::Const => UrlFragment::AssociatedConstant(name), - ty::AssocKind::Type => UrlFragment::AssociatedType(name), + /// Render the fragment, including the leading `#`. + crate fn render(&self, s: &mut String, tcx: TyCtxt<'_>) -> std::fmt::Result { + match self { + UrlFragment::Item(frag) => frag.render(s, tcx), + UrlFragment::UserWritten(raw) => write!(s, "#{}", raw), } } } -/// Render the fragment, including the leading `#`. -impl std::fmt::Display for UrlFragment { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!(f, "#")?; - match self { - UrlFragment::Method(name) => write!(f, "method.{}", name), - UrlFragment::TyMethod(name) => write!(f, "tymethod.{}", name), - UrlFragment::AssociatedConstant(name) => write!(f, "associatedconstant.{}", name), - UrlFragment::AssociatedType(name) => write!(f, "associatedtype.{}", name), - UrlFragment::StructField(name) => write!(f, "structfield.{}", name), - UrlFragment::Variant(name) => write!(f, "variant.{}", name), - UrlFragment::VariantField { variant, field } => { - write!(f, "variant.{}.field.{}", variant, field) +#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] +crate struct ItemFragment(FragmentKind, DefId); + +#[derive(Copy, Clone, Debug, Hash, PartialEq, Eq)] +crate enum FragmentKind { + Method, + TyMethod, + AssociatedConstant, + AssociatedType, + + StructField, + Variant, + VariantField, +} + +impl ItemFragment { + /// Create a fragment for an associated item. + /// + /// `is_prototype` is whether this associated item is a trait method + /// without a default definition. + fn from_assoc_item(def_id: DefId, kind: ty::AssocKind, is_prototype: bool) -> Self { + match kind { + ty::AssocKind::Fn => { + if is_prototype { + ItemFragment(FragmentKind::TyMethod, def_id) + } else { + ItemFragment(FragmentKind::Method, def_id) + } + } + ty::AssocKind::Const => ItemFragment(FragmentKind::AssociatedConstant, def_id), + ty::AssocKind::Type => ItemFragment(FragmentKind::AssociatedType, def_id), + } + } + + /// Render the fragment, including the leading `#`. + crate fn render(&self, s: &mut String, tcx: TyCtxt<'_>) -> std::fmt::Result { + write!(s, "#")?; + match *self { + ItemFragment(kind, def_id) => { + let name = tcx.item_name(def_id); + match kind { + FragmentKind::Method => write!(s, "method.{}", name), + FragmentKind::TyMethod => write!(s, "tymethod.{}", name), + FragmentKind::AssociatedConstant => write!(s, "associatedconstant.{}", name), + FragmentKind::AssociatedType => write!(s, "associatedtype.{}", name), + FragmentKind::StructField => write!(s, "structfield.{}", name), + FragmentKind::Variant => write!(s, "variant.{}", name), + FragmentKind::VariantField => { + let variant = tcx.item_name(tcx.parent(def_id).unwrap()); + write!(s, "variant.{}.field.{}", variant, name) + } + } } - UrlFragment::UserWritten(raw) => write!(f, "{}", raw), } } } @@ -296,7 +312,7 @@ struct ResolutionInfo { module_id: DefId, dis: Option, path_str: String, - extra_fragment: Option, + extra_fragment: Option, } #[derive(Clone)] @@ -310,7 +326,6 @@ struct DiagnosticInfo<'a> { #[derive(Clone, Debug, Hash)] struct CachedLink { pub res: (Res, Option), - pub side_channel: Option<(DefKind, DefId)>, } struct LinkCollector<'a, 'tcx> { @@ -320,10 +335,6 @@ struct LinkCollector<'a, 'tcx> { /// The last module will be used if the parent scope of the current item is /// unknown. mod_ids: Vec, - /// This is used to store the kind of associated items, - /// because `clean` and the disambiguator code expect them to be different. - /// See the code for associated items on inherent impls for details. - kind_side_channel: Cell>, /// 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>, @@ -340,7 +351,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { &self, path_str: &'path str, module_id: DefId, - ) -> Result<(Res, Option), ErrorKind<'path>> { + ) -> Result<(Res, Option), ErrorKind<'path>> { let tcx = self.cx.tcx; let no_res = || ResolutionFailure::NotResolved { module_id, @@ -387,14 +398,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { } match tcx.type_of(did).kind() { ty::Adt(def, _) if def.is_enum() => { - if def.all_fields().any(|item| item.ident.name == variant_field_name) { - Ok(( - ty_res, - Some(UrlFragment::VariantField { - variant: variant_name, - field: variant_field_name, - }), - )) + if let Some(field) = + def.all_fields().find(|f| f.ident.name == variant_field_name) + { + Ok((ty_res, Some(ItemFragment(FragmentKind::VariantField, field.did)))) } else { Err(ResolutionFailure::NotResolved { module_id, @@ -422,7 +429,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { prim_ty: PrimitiveType, ns: Namespace, item_name: Symbol, - ) -> Option<(Res, UrlFragment, Option<(DefKind, DefId)>)> { + ) -> Option<(Res, ItemFragment)> { let tcx = self.cx.tcx; prim_ty.impls(tcx).into_iter().find_map(|&impl_| { @@ -430,8 +437,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, impl_) .map(|item| { let kind = item.kind; - let fragment = UrlFragment::from_assoc_item(item_name, kind, false); - (Res::Primitive(prim_ty), fragment, Some((kind.as_def_kind(), item.def_id))) + let fragment = ItemFragment::from_assoc_item(item.def_id, kind, false); + (Res::Primitive(prim_ty), fragment) }) }) } @@ -505,8 +512,30 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { path_str: &'path str, ns: Namespace, module_id: DefId, - extra_fragment: &Option, + user_fragment: &Option, ) -> Result<(Res, Option), ErrorKind<'path>> { + let (res, rustdoc_fragment) = self.resolve_inner(path_str, ns, module_id)?; + let chosen_fragment = match (user_fragment, rustdoc_fragment) { + (Some(_), Some(r_frag)) => { + let diag_res = match r_frag { + ItemFragment(_, did) => Res::Def(self.cx.tcx.def_kind(did), did), + }; + let failure = AnchorFailure::RustdocAnchorConflict(diag_res); + return Err(ErrorKind::AnchorFailure(failure)); + } + (Some(u_frag), None) => Some(UrlFragment::UserWritten(u_frag.clone())), + (None, Some(r_frag)) => Some(UrlFragment::Item(r_frag)), + (None, None) => None, + }; + Ok((res, chosen_fragment)) + } + + fn resolve_inner<'path>( + &mut self, + path_str: &'path str, + ns: Namespace, + module_id: DefId, + ) -> Result<(Res, Option), ErrorKind<'path>> { if let Some(res) = self.resolve_path(path_str, ns, module_id) { match res { // FIXME(#76467): make this fallthrough to lookup the associated @@ -514,10 +543,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { Res::Def(DefKind::AssocFn | DefKind::AssocConst, _) => assert_eq!(ns, ValueNS), Res::Def(DefKind::AssocTy, _) => assert_eq!(ns, TypeNS), Res::Def(DefKind::Variant, _) => { - return handle_variant(self.cx, res, extra_fragment); + return handle_variant(self.cx, res); } // Not a trait item; just return what we found. - _ => return Ok((res, extra_fragment.clone())), + _ => return Ok((res, None)), } } @@ -548,23 +577,10 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { resolve_primitive(&path_root, TypeNS) .or_else(|| self.resolve_path(&path_root, TypeNS, module_id)) .and_then(|ty_res| { - let (res, fragment, side_channel) = + let (res, fragment) = self.resolve_associated_item(ty_res, item_name, ns, module_id)?; - let result = if extra_fragment.is_some() { - // NOTE: can never be a primitive since `side_channel.is_none()` only when `res` - // is a trait (and the side channel DefId is always an associated item). - let diag_res = side_channel.map_or(res, |(k, r)| Res::Def(k, r)); - Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(diag_res))) - } else { - // 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. - if let Some((kind, id)) = side_channel { - self.kind_side_channel.set(Some((kind, id))); - } - Ok((res, Some(fragment))) - }; - Some(result) + + Some(Ok((res, Some(fragment)))) }) .unwrap_or_else(|| { if ns == Namespace::ValueNS { @@ -661,7 +677,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { item_name: Symbol, ns: Namespace, module_id: DefId, - ) -> Option<(Res, UrlFragment, Option<(DefKind, DefId)>)> { + ) -> Option<(Res, ItemFragment)> { let tcx = self.cx.tcx; match root_res { @@ -676,11 +692,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { assoc_item.map(|item| { let kind = item.kind; - let fragment = UrlFragment::from_assoc_item(item_name, kind, false); - // 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. - (root_res, fragment, Some((kind.as_def_kind(), item.def_id))) + let fragment = ItemFragment::from_assoc_item(item.def_id, kind, false); + (root_res, fragment) }) }) } @@ -730,11 +743,8 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { if let Some(item) = assoc_item { let kind = item.kind; - let fragment = UrlFragment::from_assoc_item(item_name, kind, false); - // 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. - return Some((root_res, fragment, Some((kind.as_def_kind(), item.def_id)))); + let fragment = ItemFragment::from_assoc_item(item.def_id, kind, false); + return Some((root_res, fragment)); } if ns != Namespace::ValueNS { @@ -765,23 +775,19 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { .fields .iter() .find(|item| item.ident.name == item_name)?; - Some(( - root_res, - UrlFragment::StructField(field.ident.name), - Some((DefKind::Field, field.did)), - )) + Some((root_res, ItemFragment(FragmentKind::StructField, field.did))) } Res::Def(DefKind::Trait, did) => tcx .associated_items(did) .find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, did) .map(|item| { - let fragment = UrlFragment::from_assoc_item( - item_name, + let fragment = ItemFragment::from_assoc_item( + item.def_id, item.kind, !item.defaultness.has_value(), ); let res = Res::Def(item.kind.as_def_kind(), item.def_id); - (res, fragment, None) + (res, fragment) }), _ => None, } @@ -798,23 +804,32 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ns: Namespace, path_str: &str, module_id: DefId, - extra_fragment: &Option, + extra_fragment: &Option, ) -> Option { // resolve() can't be used for macro namespace let result = match ns { - Namespace::MacroNS => self.resolve_macro(path_str, module_id).map_err(ErrorKind::from), + Namespace::MacroNS => self + .resolve_macro(path_str, module_id) + .map(|res| (res, None)) + .map_err(ErrorKind::from), Namespace::TypeNS | Namespace::ValueNS => { - self.resolve(path_str, ns, module_id, extra_fragment).map(|(res, _)| res) + self.resolve(path_str, ns, module_id, extra_fragment) } }; let res = match result { - Ok(res) => Some(res), + Ok((res, frag)) => { + if let Some(UrlFragment::Item(ItemFragment(_, id))) = frag { + Some(Res::Def(self.cx.tcx.def_kind(id), id)) + } else { + Some(res) + } + } Err(ErrorKind::Resolve(box kind)) => kind.full_res(), Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res))) => Some(res), Err(ErrorKind::AnchorFailure(AnchorFailure::MultipleAnchors)) => None, }; - self.kind_side_channel.take().map(|(kind, id)| Res::Def(kind, id)).or(res) + res } } @@ -912,8 +927,6 @@ fn is_derive_trait_collision(ns: &PerNS DocVisitor for LinkCollector<'a, 'tcx> { fn visit_item(&mut self, item: &Item) { - use rustc_middle::ty::DefIdTree; - let parent_node = item.def_id.as_def_id().and_then(|did| find_nearest_parent_module(self.cx.tcx, did)); if parent_node.is_some() { @@ -1033,7 +1046,7 @@ impl From for PreprocessingError<'_> { struct PreprocessingInfo { path_str: String, disambiguator: Option, - extra_fragment: Option, + extra_fragment: Option, link_text: String, } @@ -1119,7 +1132,7 @@ fn preprocess_link<'a>( Some(Ok(PreprocessingInfo { path_str, disambiguator, - extra_fragment: extra_fragment.map(|frag| UrlFragment::UserWritten(frag.to_owned())), + extra_fragment: extra_fragment.map(|frag| frag.to_owned()), link_text: link_text.to_owned(), })) } @@ -1286,7 +1299,11 @@ impl LinkCollector<'_, '_> { }; let verify = |kind: DefKind, id: DefId| { - let (kind, id) = self.kind_side_channel.take().unwrap_or((kind, id)); + let (kind, id) = if let Some(UrlFragment::Item(ItemFragment(_, id))) = fragment { + (self.cx.tcx.def_kind(id), id) + } else { + (kind, id) + }; debug!("intra-doc link to {} resolved to {:?} (id: {:?})", path_str, res, id); // Disallow e.g. linking to enums with `struct@` @@ -1330,7 +1347,9 @@ impl LinkCollector<'_, '_> { match res { Res::Primitive(prim) => { - if let Some((kind, id)) = self.kind_side_channel.take() { + if let Some(UrlFragment::Item(ItemFragment(_, id))) = fragment { + let kind = self.cx.tcx.def_kind(id); + // We're actually resolving an associated item of a primitive, so we need to // verify the disambiguator (if any) matches the type of the associated item. // This case should really follow the same flow as the `Res::Def` branch below, @@ -1347,22 +1366,7 @@ impl LinkCollector<'_, '_> { && item.def_id.is_local() && !self.cx.tcx.features().intra_doc_pointers { - let span = super::source_span_for_markdown_range( - self.cx.tcx, - dox, - &ori_link.range, - &item.attrs, - ) - .unwrap_or_else(|| item.attr_span(self.cx.tcx)); - - rustc_session::parse::feature_err( - &self.cx.tcx.sess.parse_sess, - sym::intra_doc_pointers, - span, - "linking to associated items of raw pointers is experimental", - ) - .note("rustdoc does not allow disambiguating between `*const` and `*mut`, and pointers are unstable until it does") - .emit(); + self.report_rawptr_assoc_feature_gate(dox, &ori_link, item); } } else { match disambiguator { @@ -1389,6 +1393,20 @@ impl LinkCollector<'_, '_> { } } + fn report_rawptr_assoc_feature_gate(&self, dox: &str, ori_link: &MarkdownLink, item: &Item) { + let span = + super::source_span_for_markdown_range(self.cx.tcx, dox, &ori_link.range, &item.attrs) + .unwrap_or_else(|| item.attr_span(self.cx.tcx)); + rustc_session::parse::feature_err( + &self.cx.tcx.sess.parse_sess, + sym::intra_doc_pointers, + span, + "linking to associated items of raw pointers is experimental", + ) + .note("rustdoc does not allow disambiguating between `*const` and `*mut`, and pointers are unstable until it does") + .emit(); + } + fn resolve_with_disambiguator_cached( &mut self, key: ResolutionInfo, @@ -1399,7 +1417,6 @@ impl LinkCollector<'_, '_> { if let Some(ref cached) = self.visited_links.get(&key) { match cached { Some(cached) => { - self.kind_side_channel.set(cached.side_channel); return Some(cached.res.clone()); } None if cache_resolution_failure => return None, @@ -1416,13 +1433,7 @@ impl LinkCollector<'_, '_> { // 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(), - side_channel: self.kind_side_channel.clone().into_inner(), - }), - ); + self.visited_links.insert(key, Some(CachedLink { res: res.clone() })); Some(res) } else { @@ -1484,7 +1495,7 @@ impl LinkCollector<'_, '_> { let mut candidates = PerNS { macro_ns: self .resolve_macro(path_str, base_node) - .map(|res| (res, extra_fragment.clone())), + .map(|res| (res, extra_fragment.clone().map(UrlFragment::UserWritten))), type_ns: match self.resolve(path_str, TypeNS, base_node, extra_fragment) { Ok(res) => { debug!("got res in TypeNS: {:?}", res); @@ -1516,7 +1527,10 @@ impl LinkCollector<'_, '_> { // Shouldn't happen but who knows? Ok((res, Some(fragment))) } - (fragment, None) | (None, fragment) => Ok((res, fragment)), + (fragment, None) => Ok((res, fragment)), + (None, fragment) => { + Ok((res, fragment.map(UrlFragment::UserWritten))) + } } } } @@ -1553,7 +1567,7 @@ impl LinkCollector<'_, '_> { } Some(MacroNS) => { match self.resolve_macro(path_str, base_node) { - Ok(res) => Some((res, extra_fragment.clone())), + Ok(res) => Some((res, extra_fragment.clone().map(UrlFragment::UserWritten))), Err(mut kind) => { // `resolve_macro` only looks in the macro namespace. Try to give a better error if possible. for ns in [TypeNS, ValueNS] { @@ -2272,20 +2286,13 @@ fn privacy_error(cx: &DocContext<'_>, diag_info: &DiagnosticInfo<'_>, path_str: 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() { - // NOTE: `res` can never be a primitive since this function is only called when `tcx.def_kind(res) == DefKind::Variant`. - return Err(ErrorKind::AnchorFailure(AnchorFailure::RustdocAnchorConflict(res))); - } +) -> Result<(Res, Option), ErrorKind<'static>> { cx.tcx .parent(res.def_id(cx.tcx)) .map(|parent| { let parent_def = Res::Def(DefKind::Enum, parent); let variant = cx.tcx.expect_variant_res(res.as_hir_res().unwrap()); - (parent_def, Some(UrlFragment::Variant(variant.ident.name))) + (parent_def, Some(ItemFragment(FragmentKind::Variant, variant.def_id))) }) .ok_or_else(|| ResolutionFailure::NoParentItem.into()) }