diff --git a/compiler/rustc_resolve/src/build_reduced_graph.rs b/compiler/rustc_resolve/src/build_reduced_graph.rs index e68d6fdeea55b..dffec44ddbcc3 100644 --- a/compiler/rustc_resolve/src/build_reduced_graph.rs +++ b/compiler/rustc_resolve/src/build_reduced_graph.rs @@ -1268,7 +1268,6 @@ impl<'a, 'b> BuildReducedGraphVisitor<'a, 'b> { }; let binding = (res, vis, span, expansion).to_name_binding(self.r.arenas); self.r.set_binding_parent_module(binding, parent_scope.module); - self.r.all_macro_rules.insert(ident.name, res); if is_macro_export { let module = self.r.graph_root; self.r.define(module, ident, MacroNS, (res, vis, span, expansion, IsMacroExport)); diff --git a/compiler/rustc_resolve/src/lib.rs b/compiler/rustc_resolve/src/lib.rs index 8d3c46c29a861..6c0148a17a1b8 100644 --- a/compiler/rustc_resolve/src/lib.rs +++ b/compiler/rustc_resolve/src/lib.rs @@ -59,7 +59,7 @@ use rustc_span::{Span, DUMMY_SP}; use smallvec::{smallvec, SmallVec}; use std::cell::{Cell, RefCell}; use std::collections::BTreeSet; -use std::{cmp, fmt, mem, ptr}; +use std::{cmp, fmt, ptr}; use tracing::debug; use diagnostics::{ImportSuggestion, LabelSuggestion, Suggestion}; @@ -966,8 +966,6 @@ pub struct Resolver<'a> { registered_attrs: FxHashSet, registered_tools: RegisteredTools, macro_use_prelude: FxHashMap>, - /// FIXME: The only user of this is a doc link resolution hack for rustdoc. - all_macro_rules: FxHashMap, macro_map: FxHashMap>, dummy_ext_bang: Lrc, dummy_ext_derive: Lrc, @@ -1360,7 +1358,6 @@ impl<'a> Resolver<'a> { registered_attrs, registered_tools, macro_use_prelude: FxHashMap::default(), - all_macro_rules: Default::default(), macro_map: FxHashMap::default(), dummy_ext_bang: Lrc::new(SyntaxExtension::dummy_bang(session.edition())), dummy_ext_derive: Lrc::new(SyntaxExtension::dummy_derive(session.edition())), @@ -1912,11 +1909,6 @@ impl<'a> Resolver<'a> { } } - // For rustdoc. - pub fn take_all_macro_rules(&mut self) -> FxHashMap { - mem::take(&mut self.all_macro_rules) - } - /// For rustdoc. /// For local modules returns only reexports, for external modules returns all children. pub fn module_children_or_reexports(&self, def_id: DefId) -> Vec { @@ -1928,8 +1920,12 @@ impl<'a> Resolver<'a> { } /// For rustdoc. - pub fn macro_rules_scope(&self, def_id: LocalDefId) -> MacroRulesScopeRef<'a> { - *self.macro_rules_scopes.get(&def_id).expect("not a `macro_rules` item") + pub fn macro_rules_scope(&self, def_id: LocalDefId) -> (MacroRulesScopeRef<'a>, Res) { + let scope = *self.macro_rules_scopes.get(&def_id).expect("not a `macro_rules` item"); + match scope.get() { + MacroRulesScope::Binding(mb) => (scope, mb.binding.res()), + _ => unreachable!(), + } } /// Retrieves the span of the given `DefId` if `DefId` is in the local crate. diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs index 22b2f8c0c8ec3..c25a0d3b14997 100644 --- a/src/librustdoc/passes/collect_intra_doc_links.rs +++ b/src/librustdoc/passes/collect_intra_doc_links.rs @@ -11,12 +11,12 @@ use rustc_hir::def::{DefKind, Namespace, PerNS}; use rustc_hir::def_id::{DefId, CRATE_DEF_ID}; use rustc_hir::Mutability; use rustc_middle::ty::{DefIdTree, Ty, TyCtxt}; -use rustc_middle::{bug, span_bug, ty}; +use rustc_middle::{bug, ty}; use rustc_resolve::ParentScope; use rustc_session::lint::Lint; use rustc_span::hygiene::MacroKind; use rustc_span::symbol::{sym, Ident, Symbol}; -use rustc_span::{BytePos, DUMMY_SP}; +use rustc_span::BytePos; use smallvec::{smallvec, SmallVec}; use std::borrow::Cow; @@ -48,18 +48,6 @@ fn collect_intra_doc_links(krate: Crate, cx: &mut DocContext<'_>) -> Crate { krate } -/// Top-level errors emitted by this pass. -enum ErrorKind<'a> { - Resolve(Box>), - AnchorFailure(AnchorFailure), -} - -impl<'a> From> for ErrorKind<'a> { - fn from(err: ResolutionFailure<'a>) -> Self { - ErrorKind::Resolve(box err) - } -} - #[derive(Copy, Clone, Debug, Hash)] enum Res { Def(DefKind, DefId), @@ -97,12 +85,8 @@ impl Res { } } - fn as_hir_res(self) -> Option { - match self { - Res::Def(kind, id) => Some(rustc_hir::def::Res::Def(kind, id)), - // FIXME: maybe this should handle the subset of PrimitiveType that fits into hir::PrimTy? - Res::Primitive(_) => None, - } + fn from_def_id(tcx: TyCtxt<'_>, def_id: DefId) -> Res { + Res::Def(tcx.def_kind(def_id), def_id) } /// Used for error reporting. @@ -160,8 +144,25 @@ impl TryFrom for Res { } } -/// A link failed to resolve. -#[derive(Clone, Debug)] +/// The link failed to resolve. [`resolution_failure`] should look to see if there's +/// a more helpful error that can be given. +#[derive(Debug)] +struct UnresolvedPath<'a> { + /// Item on which the link is resolved, used for resolving `Self`. + item_id: ItemId, + /// The scope the link was resolved in. + module_id: DefId, + /// If part of the link resolved, this has the `Res`. + /// + /// In `[std::io::Error::x]`, `std::io::Error` would be a partial resolution. + partial_res: Option, + /// The remaining unresolved path segments. + /// + /// In `[std::io::Error::x]`, `x` would be unresolved. + unresolved: Cow<'a, str>, +} + +#[derive(Debug)] enum ResolutionFailure<'a> { /// This resolved, but with the wrong namespace. WrongNamespace { @@ -173,35 +174,10 @@ enum ResolutionFailure<'a> { /// even though `Result`'s actual namespace is [`Namespace::TypeNS`]. expected_ns: Namespace, }, - /// The link failed to resolve. [`resolution_failure`] should look to see if there's - /// a more helpful error that can be given. - NotResolved { - /// Item on which the link is resolved, used for resolving `Self`. - item_id: ItemId, - /// The scope the link was resolved in. - module_id: DefId, - /// If part of the link resolved, this has the `Res`. - /// - /// In `[std::io::Error::x]`, `std::io::Error` would be a partial resolution. - partial_res: Option, - /// The remaining unresolved path segments. - /// - /// In `[std::io::Error::x]`, `x` would be unresolved. - unresolved: Cow<'a, str>, - }, - /// This happens when rustdoc can't determine the parent scope for an item. - /// It is always a bug in rustdoc. - NoParentItem, - /// This link has malformed generic parameters; e.g., the angle brackets are unbalanced. - MalformedGenerics(MalformedGenerics), - /// Used to communicate that this should be ignored, but shouldn't be reported to the user. - /// - /// This happens when there is no disambiguator and one of the namespaces - /// failed to resolve. - Dummy, + NotResolved(UnresolvedPath<'a>), } -#[derive(Clone, Debug)] +#[derive(Clone, Copy, Debug)] enum MalformedGenerics { /// This link has unbalanced angle brackets. /// @@ -242,35 +218,6 @@ enum MalformedGenerics { EmptyAngleBrackets, } -impl ResolutionFailure<'_> { - /// This resolved fully (not just partially) but is erroneous for some other reason - /// - /// Returns the full resolution of the link, if present. - fn full_res(&self) -> Option { - match self { - Self::WrongNamespace { res, expected_ns: _ } => Some(*res), - _ => None, - } - } -} - -#[derive(Clone, Copy)] -enum AnchorFailure { - /// User error: `[std#x#y]` is not valid - MultipleAnchors, - /// The anchor provided by the user conflicts with Rustdoc's generated anchor. - /// - /// This is an unfortunate state of affairs. Not every item that can be - /// linked to has its own page; sometimes it is a subheading within a page, - /// like for associated items. In those cases, rustdoc uses an anchor to - /// link to the subheading. Since you can't have two anchors for the same - /// link, Rustdoc disallows having a user-specified anchor. - /// - /// Most of the time this is fine, because you can just link to the page of - /// the item if you want to provide your own anchor. - RustdocAnchorConflict(Res), -} - #[derive(Clone, Debug, Hash, PartialEq, Eq)] crate enum UrlFragment { Item(ItemFragment), @@ -302,24 +249,32 @@ crate enum FragmentKind { VariantField, } -impl ItemFragment { - /// Create a fragment for an associated item. - #[instrument(level = "debug")] - fn from_assoc_item(item: &ty::AssocItem) -> Self { - let def_id = item.def_id; - match item.kind { - ty::AssocKind::Fn => { - if item.defaultness.has_value() { - ItemFragment(FragmentKind::Method, def_id) +impl FragmentKind { + fn from_def_id(tcx: TyCtxt<'_>, def_id: DefId) -> FragmentKind { + match tcx.def_kind(def_id) { + DefKind::AssocFn => { + if tcx.associated_item(def_id).defaultness.has_value() { + FragmentKind::Method } else { - ItemFragment(FragmentKind::TyMethod, def_id) + FragmentKind::TyMethod } } - ty::AssocKind::Const => ItemFragment(FragmentKind::AssociatedConstant, def_id), - ty::AssocKind::Type => ItemFragment(FragmentKind::AssociatedType, def_id), + DefKind::AssocConst => FragmentKind::AssociatedConstant, + DefKind::AssocTy => FragmentKind::AssociatedType, + DefKind::Variant => FragmentKind::Variant, + DefKind::Field => { + if tcx.def_kind(tcx.parent(def_id)) == DefKind::Variant { + FragmentKind::VariantField + } else { + FragmentKind::StructField + } + } + kind => bug!("unexpected associated item kind: {:?}", kind), } } +} +impl ItemFragment { /// Render the fragment, including the leading `#`. crate fn render(&self, s: &mut String, tcx: TyCtxt<'_>) -> std::fmt::Result { write!(s, "#")?; @@ -389,9 +344,9 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { path_str: &'path str, item_id: ItemId, module_id: DefId, - ) -> Result<(Res, Option), ErrorKind<'path>> { + ) -> Result<(Res, DefId), UnresolvedPath<'path>> { let tcx = self.cx.tcx; - let no_res = || ResolutionFailure::NotResolved { + let no_res = || UnresolvedPath { item_id, module_id, partial_res: None, @@ -418,42 +373,27 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { let ty_res = self.resolve_path(&path, TypeNS, item_id, module_id).ok_or_else(no_res)?; match ty_res { - Res::Def(DefKind::Enum, did) => { - if tcx - .inherent_impls(did) - .iter() - .flat_map(|imp| tcx.associated_items(*imp).in_definition_order()) - .any(|item| item.name == variant_name) - { - // This is just to let `fold_item` know that this shouldn't be considered; - // it's a bug for the error to make it to the user - return Err(ResolutionFailure::Dummy.into()); - } - match tcx.type_of(did).kind() { - ty::Adt(def, _) if def.is_enum() => { - if let Some(field) = def.all_fields().find(|f| f.name == variant_field_name) - { - Ok((ty_res, Some(ItemFragment(FragmentKind::VariantField, field.did)))) - } else { - Err(ResolutionFailure::NotResolved { - item_id, - module_id, - partial_res: Some(Res::Def(DefKind::Enum, def.did())), - unresolved: variant_field_name.to_string().into(), - } - .into()) - } + Res::Def(DefKind::Enum, did) => match tcx.type_of(did).kind() { + ty::Adt(def, _) if def.is_enum() => { + if let Some(field) = def.all_fields().find(|f| f.name == variant_field_name) { + Ok((ty_res, field.did)) + } else { + Err(UnresolvedPath { + item_id, + module_id, + partial_res: Some(Res::Def(DefKind::Enum, def.did())), + unresolved: variant_field_name.to_string().into(), + }) } - _ => unreachable!(), } - } - _ => Err(ResolutionFailure::NotResolved { + _ => unreachable!(), + }, + _ => Err(UnresolvedPath { item_id, module_id, partial_res: Some(ty_res), unresolved: variant_name.to_string().into(), - } - .into()), + }), } } @@ -463,35 +403,13 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { prim_ty: PrimitiveType, ns: Namespace, item_name: Symbol, - ) -> Option<(Res, ItemFragment)> { + ) -> Option<(Res, DefId)> { let tcx = self.cx.tcx; prim_ty.impls(tcx).find_map(|impl_| { tcx.associated_items(impl_) .find_by_name_and_namespace(tcx, Ident::with_dummy_span(item_name), ns, impl_) - .map(|item| { - let fragment = ItemFragment::from_assoc_item(item); - (Res::Primitive(prim_ty), fragment) - }) - }) - } - - /// Resolves a string as a macro. - /// - /// FIXME(jynelson): Can this be unified with `resolve()`? - fn resolve_macro( - &self, - path_str: &'a str, - item_id: ItemId, - module_id: DefId, - ) -> Result> { - self.resolve_path(path_str, MacroNS, item_id, module_id).ok_or_else(|| { - ResolutionFailure::NotResolved { - item_id, - module_id, - partial_res: None, - unresolved: path_str.into(), - } + .map(|item| (Res::Primitive(prim_ty), item.def_id)) }) } @@ -585,43 +503,22 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ns: Namespace, item_id: ItemId, module_id: DefId, - user_fragment: &Option, - ) -> Result<(Res, Option), ErrorKind<'path>> { - let (res, rustdoc_fragment) = self.resolve_inner(path_str, ns, item_id, 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, - item_id: ItemId, - module_id: DefId, - ) -> Result<(Res, Option), ErrorKind<'path>> { + ) -> Result<(Res, Option), UnresolvedPath<'path>> { if let Some(res) = self.resolve_path(path_str, ns, item_id, module_id) { - match res { - // FIXME(#76467): make this fallthrough to lookup the associated - // item a separate function. - 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); - } - // Not a trait item; just return what we found. - _ => return Ok((res, None)), - } + return Ok(match res { + Res::Def( + DefKind::AssocFn | DefKind::AssocConst | DefKind::AssocTy | DefKind::Variant, + def_id, + ) => (Res::from_def_id(self.cx.tcx, self.cx.tcx.parent(def_id)), Some(def_id)), + _ => ((res, None)), + }); + } else if ns == MacroNS { + return Err(UnresolvedPath { + item_id, + module_id, + partial_res: None, + unresolved: path_str.into(), + }); } // Try looking for methods and associated items. @@ -637,7 +534,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { // So we can be sure that `rustc_resolve` was accurate when it said it wasn't resolved. .ok_or_else(|| { debug!("found no `::`, assumming {} was correctly not in scope", item_name); - ResolutionFailure::NotResolved { + UnresolvedPath { item_id, module_id, partial_res: None, @@ -652,24 +549,21 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { resolve_primitive(&path_root, TypeNS) .or_else(|| self.resolve_path(&path_root, TypeNS, item_id, module_id)) .and_then(|ty_res| { - let (res, fragment) = - self.resolve_associated_item(ty_res, item_name, ns, module_id)?; - - Some(Ok((res, Some(fragment)))) + self.resolve_associated_item(ty_res, item_name, ns, module_id).map(Ok) }) .unwrap_or_else(|| { if ns == Namespace::ValueNS { self.variant_field(path_str, item_id, module_id) } else { - Err(ResolutionFailure::NotResolved { + Err(UnresolvedPath { item_id, module_id, partial_res: None, unresolved: path_root.into(), - } - .into()) + }) } }) + .map(|(res, def_id)| (res, Some(def_id))) } /// Convert a DefId to a Res, where possible. @@ -694,7 +588,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ty::FnPtr(_) => Res::Primitive(Fn), ty::Never => Res::Primitive(Never), ty::Adt(ty::AdtDef(Interned(&ty::AdtDefData { did, .. }, _)), _) | ty::Foreign(did) => { - Res::Def(self.cx.tcx.def_kind(did), did) + Res::from_def_id(self.cx.tcx, did) } ty::Projection(_) | ty::Closure(..) @@ -751,23 +645,18 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { item_name: Symbol, ns: Namespace, module_id: DefId, - ) -> Option<(Res, ItemFragment)> { + ) -> Option<(Res, DefId)> { let tcx = self.cx.tcx; match root_res { Res::Primitive(prim) => { self.resolve_primitive_associated_item(prim, ns, item_name).or_else(|| { - let assoc_item = self - .primitive_type_to_ty(prim) + self.primitive_type_to_ty(prim) .map(|ty| { resolve_associated_trait_item(ty, module_id, item_name, ns, self.cx) }) - .flatten(); - - assoc_item.map(|item| { - let fragment = ItemFragment::from_assoc_item(&item); - (root_res, fragment) - }) + .flatten() + .map(|item| (root_res, item.def_id)) }) } Res::Def(DefKind::TyAlias, did) => { @@ -788,10 +677,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { ty::Adt(adt_def, _) => { for variant in adt_def.variants() { if variant.name == item_name { - return Some(( - root_res, - ItemFragment(FragmentKind::Variant, variant.def_id), - )); + return Some((root_res, variant.def_id)); } } } @@ -832,8 +718,7 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { debug!("got associated item {:?}", assoc_item); if let Some(item) = assoc_item { - let fragment = ItemFragment::from_assoc_item(&item); - return Some((root_res, fragment)); + return Some((root_res, item.def_id)); } if ns != Namespace::ValueNS { @@ -861,59 +746,22 @@ impl<'a, 'tcx> LinkCollector<'a, 'tcx> { }; let field = def.non_enum_variant().fields.iter().find(|item| item.name == item_name)?; - Some((root_res, ItemFragment(FragmentKind::StructField, field.did))) + Some((root_res, 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 = ItemFragment::from_assoc_item(item); let res = Res::Def(item.kind.as_def_kind(), item.def_id); - (res, fragment) + (res, item.def_id) }), _ => None, } } +} - /// Used for reporting better errors. - /// - /// Returns whether the link resolved 'fully' in another namespace. - /// 'fully' here means that all parts of the link resolved, not just some path segments. - /// This returns the `Res` even if it was erroneous for some reason - /// (such as having invalid URL fragments or being in the wrong namespace). - fn check_full_res( - &mut self, - ns: Namespace, - path_str: &str, - item_id: ItemId, - module_id: DefId, - extra_fragment: &Option, - ) -> Option { - // resolve() can't be used for macro namespace - let result = match ns { - Namespace::MacroNS => self - .resolve_macro(path_str, item_id, module_id) - .map(|res| (res, None)) - .map_err(ErrorKind::from), - Namespace::TypeNS | Namespace::ValueNS => { - self.resolve(path_str, ns, item_id, module_id, extra_fragment) - } - }; - - let res = match result { - 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, - }; - res - } +fn full_res(tcx: TyCtxt<'_>, (base, assoc_item): (Res, Option)) -> Res { + assoc_item.map_or(base, |def_id| Res::from_def_id(tcx, def_id)) } /// Look to see if a resolved item has an associated item named `item_name`. @@ -1093,14 +941,23 @@ impl<'a, 'tcx> DocVisitor for LinkCollector<'a, 'tcx> { } enum PreprocessingError { - Anchor(AnchorFailure), + /// User error: `[std#x#y]` is not valid + MultipleAnchors, Disambiguator(Range, String), - Resolution(ResolutionFailure<'static>, String, Option), + MalformedGenerics(MalformedGenerics, String), } -impl From for PreprocessingError { - fn from(err: AnchorFailure) -> Self { - Self::Anchor(err) +impl PreprocessingError { + fn report(&self, cx: &DocContext<'_>, diag_info: DiagnosticInfo<'_>) { + match self { + PreprocessingError::MultipleAnchors => report_multiple_anchors(cx, diag_info), + PreprocessingError::Disambiguator(range, msg) => { + disambiguator_error(cx, diag_info, range.clone(), msg) + } + PreprocessingError::MalformedGenerics(err, path_str) => { + report_malformed_generics(cx, diag_info, *err, path_str) + } + } } } @@ -1145,7 +1002,7 @@ fn preprocess_link( let extra_fragment = parts.next(); if parts.next().is_some() { // A valid link can't have multiple #'s - return Some(Err(AnchorFailure::MultipleAnchors.into())); + return Some(Err(PreprocessingError::MultipleAnchors)); } // Parse and strip the disambiguator from the link, if present. @@ -1173,13 +1030,9 @@ fn preprocess_link( let path_str = if path_str.contains(['<', '>'].as_slice()) { match strip_generics_from_path(path_str) { Ok(path) => path, - Err(err_kind) => { + Err(err) => { debug!("link has malformed generics: {}", path_str); - return Some(Err(PreprocessingError::Resolution( - err_kind, - path_str.to_owned(), - disambiguator, - ))); + return Some(Err(PreprocessingError::MalformedGenerics(err, path_str.to_owned()))); } } } else { @@ -1229,32 +1082,10 @@ impl LinkCollector<'_, '_> { link_range: ori_link.range.clone(), }; - let PreprocessingInfo { path_str, disambiguator, extra_fragment, link_text } = match pp_link - { - Ok(x) => x, - Err(err) => { - match err { - PreprocessingError::Anchor(err) => anchor_failure(self.cx, diag_info, *err), - PreprocessingError::Disambiguator(range, msg) => { - disambiguator_error(self.cx, diag_info, range.clone(), msg) - } - PreprocessingError::Resolution(err, path_str, disambiguator) => { - resolution_failure( - self, - diag_info, - path_str, - *disambiguator, - smallvec![err.clone()], - ); - } - } - return None; - } - }; + let PreprocessingInfo { path_str, disambiguator, extra_fragment, link_text } = + pp_link.as_ref().map_err(|err| err.report(self.cx, diag_info.clone())).ok()?; let disambiguator = *disambiguator; - let inner_docs = item.inner_docs(self.cx.tcx); - // In order to correctly resolve intra-doc links we need to // pick a base AST node to work from. If the documentation for // this module came from an inner comment (//!) then we anchor @@ -1266,21 +1097,10 @@ impl LinkCollector<'_, '_> { // we've already pushed this node onto the resolution stack but // for outer comments we explicitly try and resolve against the // parent_node first. + let inner_docs = item.inner_docs(self.cx.tcx); let base_node = if item.is_mod() && inner_docs { self.mod_ids.last().copied() } else { parent_node }; - - let Some(module_id) = base_node else { - // This is a bug. - debug!("attempting to resolve item without parent module: {}", path_str); - resolution_failure( - self, - diag_info, - path_str, - disambiguator, - smallvec![ResolutionFailure::NoParentItem], - ); - return None; - }; + let module_id = base_node.expect("doc link without parent module"); let (mut res, fragment) = self.resolve_with_disambiguator_cached( ResolutionInfo { @@ -1504,7 +1324,21 @@ impl LinkCollector<'_, '_> { } } - let res = self.resolve_with_disambiguator(&key, diag); + let res = self.resolve_with_disambiguator(&key, diag.clone()).and_then(|(res, def_id)| { + let fragment = match (&key.extra_fragment, def_id) { + (Some(_), Some(def_id)) => { + report_anchor_conflict(self.cx, diag, Res::from_def_id(self.cx.tcx, def_id)); + return None; + } + (Some(u_frag), None) => Some(UrlFragment::UserWritten(u_frag.clone())), + (None, Some(def_id)) => Some(UrlFragment::Item(ItemFragment( + FragmentKind::from_def_id(self.cx.tcx, def_id), + def_id, + ))), + (None, None) => None, + }; + Some((res, fragment)) + }); // Cache only if resolved successfully - don't silence duplicate errors if let Some(res) = res { @@ -1529,103 +1363,59 @@ impl LinkCollector<'_, '_> { &mut self, key: &ResolutionInfo, diag: DiagnosticInfo<'_>, - ) -> Option<(Res, Option)> { + ) -> Option<(Res, Option)> { let disambiguator = key.dis; let path_str = &key.path_str; let item_id = key.item_id; let base_node = key.module_id; - let extra_fragment = &key.extra_fragment; match disambiguator.map(Disambiguator::ns) { - Some(expected_ns @ (ValueNS | TypeNS)) => { - match self.resolve(path_str, expected_ns, item_id, base_node, extra_fragment) { + Some(expected_ns) => { + match self.resolve(path_str, expected_ns, item_id, base_node) { Ok(res) => Some(res), - Err(ErrorKind::Resolve(box mut kind)) => { + Err(err) => { // We only looked in one namespace. Try to give a better error if possible. - if kind.full_res().is_none() { - let other_ns = if expected_ns == ValueNS { TypeNS } else { ValueNS }; - // FIXME: really it should be `resolution_failure` that does this, not `resolve_with_disambiguator` - // See https://github.com/rust-lang/rust/pull/76955#discussion_r493953382 for a good approach - for new_ns in [other_ns, MacroNS] { - if let Some(res) = self.check_full_res( - new_ns, - path_str, - item_id, - base_node, - extra_fragment, - ) { - kind = ResolutionFailure::WrongNamespace { res, expected_ns }; + // FIXME: really it should be `resolution_failure` that does this, not `resolve_with_disambiguator`. + // See https://github.com/rust-lang/rust/pull/76955#discussion_r493953382 for a good approach. + let mut err = ResolutionFailure::NotResolved(err); + for other_ns in [TypeNS, ValueNS, MacroNS] { + if other_ns != expected_ns { + if let Ok(res) = + self.resolve(path_str, other_ns, item_id, base_node) + { + err = ResolutionFailure::WrongNamespace { + res: full_res(self.cx.tcx, res), + expected_ns, + }; break; } } } - resolution_failure(self, diag, path_str, disambiguator, smallvec![kind]); + resolution_failure(self, diag, path_str, disambiguator, smallvec![err]); // This could just be a normal link or a broken link // we could potentially check if something is // "intra-doc-link-like" and warn in that case. None } - Err(ErrorKind::AnchorFailure(msg)) => { - anchor_failure(self.cx, diag, msg); - None - } } } None => { // Try everything! - let mut candidates = PerNS { - macro_ns: self - .resolve_macro(path_str, item_id, base_node) - .map(|res| (res, extra_fragment.clone().map(UrlFragment::UserWritten))), - type_ns: match self.resolve( - path_str, - TypeNS, - item_id, - base_node, - extra_fragment, - ) { - Ok(res) => { - debug!("got res in TypeNS: {:?}", res); - Ok(res) - } - Err(ErrorKind::AnchorFailure(msg)) => { - anchor_failure(self.cx, diag, msg); - return None; - } - Err(ErrorKind::Resolve(box kind)) => Err(kind), - }, - value_ns: match self.resolve( - path_str, - ValueNS, - item_id, - base_node, - extra_fragment, - ) { - Ok(res) => Ok(res), - Err(ErrorKind::AnchorFailure(msg)) => { - anchor_failure(self.cx, diag, msg); - return None; - } - Err(ErrorKind::Resolve(box kind)) => Err(kind), - } - .and_then(|(res, fragment)| { - // Constructors are picked up in the type namespace. + let mut candidate = |ns| { + self.resolve(path_str, ns, item_id, base_node) + .map_err(ResolutionFailure::NotResolved) + }; + + let candidates = PerNS { + macro_ns: candidate(MacroNS), + type_ns: candidate(TypeNS), + value_ns: candidate(ValueNS).and_then(|(res, def_id)| { match res { + // Constructors are picked up in the type namespace. Res::Def(DefKind::Ctor(..), _) => { Err(ResolutionFailure::WrongNamespace { res, expected_ns: TypeNS }) } - _ => { - match (fragment, extra_fragment.clone()) { - (Some(fragment), Some(_)) => { - // Shouldn't happen but who knows? - Ok((res, Some(fragment))) - } - (fragment, None) => Ok((res, fragment)), - (None, fragment) => { - Ok((res, fragment.map(UrlFragment::UserWritten))) - } - } - } + _ => Ok((res, def_id)), } }), }; @@ -1649,38 +1439,17 @@ impl LinkCollector<'_, '_> { } else if len == 2 && is_derive_trait_collision(&candidates) { Some(candidates.type_ns.unwrap()) } else { - if is_derive_trait_collision(&candidates) { - candidates.macro_ns = Err(ResolutionFailure::Dummy); - } + let ignore_macro = is_derive_trait_collision(&candidates); // If we're reporting an ambiguity, don't mention the namespaces that failed - let candidates = candidates.map(|candidate| candidate.ok().map(|(res, _)| res)); + let mut candidates = + candidates.map(|candidate| candidate.ok().map(|(res, _)| res)); + if ignore_macro { + candidates.macro_ns = None; + } ambiguity_error(self.cx, diag, path_str, candidates.present_items().collect()); None } } - Some(MacroNS) => { - match self.resolve_macro(path_str, item_id, base_node) { - 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] { - if let Some(res) = self.check_full_res( - ns, - path_str, - item_id, - base_node, - extra_fragment, - ) { - kind = - ResolutionFailure::WrongNamespace { res, expected_ns: MacroNS }; - break; - } - } - resolution_failure(self, diag, path_str, disambiguator, smallvec![kind]); - None - } - } - } } } } @@ -1995,12 +1764,12 @@ fn resolution_failure( } variants_seen.push(variant); - if let ResolutionFailure::NotResolved { + if let ResolutionFailure::NotResolved(UnresolvedPath { item_id, module_id, partial_res, unresolved, - } = &mut failure + }) = &mut failure { use DefKind::*; @@ -2026,11 +1795,9 @@ fn resolution_failure( }; name = start; for ns in [TypeNS, ValueNS, MacroNS] { - if let Some(res) = - collector.check_full_res(ns, start, item_id, module_id, &None) - { + if let Ok(res) = collector.resolve(start, ns, item_id, module_id) { debug!("found partial_res={:?}", res); - *partial_res = Some(res); + *partial_res = Some(full_res(collector.cx.tcx, res)); *unresolved = end.into(); break 'outer; } @@ -2130,7 +1897,6 @@ fn resolution_failure( } let note = match failure { ResolutionFailure::NotResolved { .. } => unreachable!("handled above"), - ResolutionFailure::Dummy => continue, ResolutionFailure::WrongNamespace { res, expected_ns } => { suggest_disambiguator(res, diag, path_str, diag_info.ori_link, sp); @@ -2140,38 +1906,6 @@ fn resolution_failure( expected_ns.descr() ) } - ResolutionFailure::NoParentItem => { - // FIXME(eddyb) this doesn't belong here, whatever made - // the `ResolutionFailure::NoParentItem` should emit an - // immediate or delayed `span_bug` about the issue. - tcx.sess.delay_span_bug( - sp.unwrap_or(DUMMY_SP), - "intra-doc link missing parent item", - ); - - "BUG: all intra-doc links should have a parent item".to_owned() - } - ResolutionFailure::MalformedGenerics(variant) => match variant { - MalformedGenerics::UnbalancedAngleBrackets => { - String::from("unbalanced angle brackets") - } - MalformedGenerics::MissingType => { - String::from("missing type for generic parameters") - } - MalformedGenerics::HasFullyQualifiedSyntax => { - diag.note("see https://github.com/rust-lang/rust/issues/74563 for more information"); - String::from("fully-qualified syntax is unsupported") - } - MalformedGenerics::InvalidPathSeparator => { - String::from("has invalid path separator") - } - MalformedGenerics::TooManyAngleBrackets => { - String::from("too many angle brackets") - } - MalformedGenerics::EmptyAngleBrackets => { - String::from("empty angle brackets") - } - }, }; if let Some(span) = sp { diag.span_label(span, ¬e); @@ -2183,22 +1917,24 @@ fn resolution_failure( ); } -/// Report an anchor failure. -fn anchor_failure(cx: &DocContext<'_>, diag_info: DiagnosticInfo<'_>, failure: AnchorFailure) { - let (msg, anchor_idx) = match failure { - AnchorFailure::MultipleAnchors => { - (format!("`{}` contains multiple anchors", diag_info.ori_link), 1) - } - AnchorFailure::RustdocAnchorConflict(res) => ( - format!( - "`{}` contains an anchor, but links to {kind}s are already anchored", - diag_info.ori_link, - kind = res.descr(), - ), - 0, - ), - }; +fn report_multiple_anchors(cx: &DocContext<'_>, diag_info: DiagnosticInfo<'_>) { + let msg = format!("`{}` contains multiple anchors", diag_info.ori_link); + anchor_failure(cx, diag_info, &msg, 1) +} + +fn report_anchor_conflict(cx: &DocContext<'_>, diag_info: DiagnosticInfo<'_>, res: Res) { + let (link, kind) = (diag_info.ori_link, res.descr()); + let msg = format!("`{link}` contains an anchor, but links to {kind}s are already anchored"); + anchor_failure(cx, diag_info, &msg, 0) +} +/// Report an anchor failure. +fn anchor_failure( + cx: &DocContext<'_>, + diag_info: DiagnosticInfo<'_>, + msg: &str, + anchor_idx: usize, +) { report_diagnostic(cx.tcx, BROKEN_INTRA_DOC_LINKS, &msg, &diag_info, |diag, sp| { if let Some(mut sp) = sp { if let Some((fragment_offset, _)) = @@ -2208,13 +1944,6 @@ fn anchor_failure(cx: &DocContext<'_>, diag_info: DiagnosticInfo<'_>, failure: A } diag.span_label(sp, "invalid anchor"); } - if let AnchorFailure::RustdocAnchorConflict(Res::Primitive(_)) = failure { - if let Some(sp) = sp { - span_bug!(sp, "anchors should be allowed now"); - } else { - bug!("anchors should be allowed now"); - } - } }); } @@ -2235,6 +1964,40 @@ fn disambiguator_error( }); } +fn report_malformed_generics( + cx: &DocContext<'_>, + diag_info: DiagnosticInfo<'_>, + err: MalformedGenerics, + path_str: &str, +) { + report_diagnostic( + cx.tcx, + BROKEN_INTRA_DOC_LINKS, + &format!("unresolved link to `{}`", path_str), + &diag_info, + |diag, sp| { + let note = match err { + MalformedGenerics::UnbalancedAngleBrackets => "unbalanced angle brackets", + MalformedGenerics::MissingType => "missing type for generic parameters", + MalformedGenerics::HasFullyQualifiedSyntax => { + diag.note( + "see https://github.com/rust-lang/rust/issues/74563 for more information", + ); + "fully-qualified syntax is unsupported" + } + MalformedGenerics::InvalidPathSeparator => "has invalid path separator", + MalformedGenerics::TooManyAngleBrackets => "too many angle brackets", + MalformedGenerics::EmptyAngleBrackets => "empty angle brackets", + }; + if let Some(span) = sp { + diag.span_label(span, note); + } else { + diag.note(note); + } + }, + ); +} + /// Report an ambiguity error, where there were multiple possible resolutions. fn ambiguity_error( cx: &DocContext<'_>, @@ -2331,17 +2094,6 @@ fn privacy_error(cx: &DocContext<'_>, diag_info: &DiagnosticInfo<'_>, path_str: }); } -/// Given an enum variant's res, return the res of its enum and the associated fragment. -fn handle_variant( - cx: &DocContext<'_>, - res: Res, -) -> Result<(Res, Option), ErrorKind<'static>> { - let parent = cx.tcx.parent(res.def_id(cx.tcx)); - let parent_def = Res::Def(DefKind::Enum, parent); - let variant = cx.tcx.expect_variant_res(res.as_hir_res().unwrap()); - Ok((parent_def, Some(ItemFragment(FragmentKind::Variant, variant.def_id)))) -} - /// Resolve a primitive type or value. fn resolve_primitive(path_str: &str, ns: Namespace) -> Option { if ns != TypeNS { @@ -2381,7 +2133,7 @@ fn resolve_primitive(path_str: &str, ns: Namespace) -> Option { Some(Res::Primitive(prim)) } -fn strip_generics_from_path(path_str: &str) -> Result> { +fn strip_generics_from_path(path_str: &str) -> Result { let mut stripped_segments = vec![]; let mut path = path_str.chars().peekable(); let mut segment = Vec::new(); @@ -2396,9 +2148,7 @@ fn strip_generics_from_path(path_str: &str) -> Result { @@ -2406,14 +2156,10 @@ fn strip_generics_from_path(path_str: &str) -> Result { - return Err(ResolutionFailure::MalformedGenerics( - MalformedGenerics::TooManyAngleBrackets, - )); + return Err(MalformedGenerics::TooManyAngleBrackets); } Some('>') => { - return Err(ResolutionFailure::MalformedGenerics( - MalformedGenerics::EmptyAngleBrackets, - )); + return Err(MalformedGenerics::EmptyAngleBrackets); } Some(chr) => { segment.push(chr); @@ -2441,16 +2187,10 @@ fn strip_generics_from_path(path_str: &str) -> Result, -) -> Result> { +fn strip_generics_from_path_segment(segment: Vec) -> Result { let mut stripped_segment = String::new(); let mut param_depth = 0; @@ -2465,9 +2205,7 @@ fn strip_generics_from_path_segment( if latest_generics_chunk.contains(" as ") { // The segment tries to use fully-qualified syntax, which is currently unsupported. // Give a helpful error message instead of completely ignoring the angle brackets. - return Err(ResolutionFailure::MalformedGenerics( - MalformedGenerics::HasFullyQualifiedSyntax, - )); + return Err(MalformedGenerics::HasFullyQualifiedSyntax); } } else { if param_depth == 0 { @@ -2482,6 +2220,6 @@ fn strip_generics_from_path_segment( Ok(stripped_segment) } else { // The segment has unbalanced angle brackets, e.g. `Vec>` - Err(ResolutionFailure::MalformedGenerics(MalformedGenerics::UnbalancedAngleBrackets)) + Err(MalformedGenerics::UnbalancedAngleBrackets) } } diff --git a/src/librustdoc/passes/collect_intra_doc_links/early.rs b/src/librustdoc/passes/collect_intra_doc_links/early.rs index 07d05cab1d1d7..0ac27087a9782 100644 --- a/src/librustdoc/passes/collect_intra_doc_links/early.rs +++ b/src/librustdoc/passes/collect_intra_doc_links/early.rs @@ -40,6 +40,7 @@ crate fn early_resolve_intra_doc_links( traits_in_scope: Default::default(), all_traits: Default::default(), all_trait_impls: Default::default(), + all_macro_rules: Default::default(), document_private_items, }; @@ -64,7 +65,7 @@ crate fn early_resolve_intra_doc_links( traits_in_scope: link_resolver.traits_in_scope, all_traits: Some(link_resolver.all_traits), all_trait_impls: Some(link_resolver.all_trait_impls), - all_macro_rules: link_resolver.resolver.take_all_macro_rules(), + all_macro_rules: link_resolver.all_macro_rules, } } @@ -82,6 +83,7 @@ struct EarlyDocLinkResolver<'r, 'ra> { traits_in_scope: DefIdMap>, all_traits: Vec, all_trait_impls: Vec, + all_macro_rules: FxHashMap>, document_private_items: bool, } @@ -134,24 +136,21 @@ impl<'ra> EarlyDocLinkResolver<'_, 'ra> { // using privacy, private traits and impls from other crates are never documented in // the current crate, and links in their doc comments are not resolved. for &def_id in &all_traits { - if self.resolver.cstore().visibility_untracked(def_id) == Visibility::Public { + if self.resolver.cstore().visibility_untracked(def_id).is_public() { self.resolve_doc_links_extern_impl(def_id, false); } } for &(trait_def_id, impl_def_id, simplified_self_ty) in &all_trait_impls { - if self.resolver.cstore().visibility_untracked(trait_def_id) - == Visibility::Public + if self.resolver.cstore().visibility_untracked(trait_def_id).is_public() && simplified_self_ty.and_then(|ty| ty.def()).map_or(true, |ty_def_id| { - self.resolver.cstore().visibility_untracked(ty_def_id) - == Visibility::Public + self.resolver.cstore().visibility_untracked(ty_def_id).is_public() }) { self.resolve_doc_links_extern_impl(impl_def_id, false); } } for (ty_def_id, impl_def_id) in all_inherent_impls { - if self.resolver.cstore().visibility_untracked(ty_def_id) == Visibility::Public - { + if self.resolver.cstore().visibility_untracked(ty_def_id).is_public() { self.resolve_doc_links_extern_impl(impl_def_id, true); } } @@ -178,8 +177,7 @@ impl<'ra> EarlyDocLinkResolver<'_, 'ra> { self.resolver.cstore().associated_item_def_ids_untracked(def_id, self.sess), ); for assoc_def_id in assoc_item_def_ids { - if !is_inherent - || self.resolver.cstore().visibility_untracked(assoc_def_id) == Visibility::Public + if !is_inherent || self.resolver.cstore().visibility_untracked(assoc_def_id).is_public() { self.resolve_doc_links_extern_outer(assoc_def_id, def_id); } @@ -279,7 +277,7 @@ impl<'ra> EarlyDocLinkResolver<'_, 'ra> { for child in self.resolver.module_children_or_reexports(module_id) { // This condition should give a superset of `denied` from `fn clean_use_statement`. - if child.vis == Visibility::Public + if child.vis.is_public() || self.document_private_items && child.vis != Visibility::Restricted(module_id) && module_id.is_local() @@ -343,8 +341,10 @@ impl Visitor<'_> for EarlyDocLinkResolver<'_, '_> { self.all_trait_impls.push(self.resolver.local_def_id(item.id).to_def_id()); } ItemKind::MacroDef(macro_def) if macro_def.macro_rules => { - self.parent_scope.macro_rules = + let (macro_rules_scope, res) = self.resolver.macro_rules_scope(self.resolver.local_def_id(item.id)); + self.parent_scope.macro_rules = macro_rules_scope; + self.all_macro_rules.insert(item.ident.name, res); } _ => {} }