Skip to content

rustdoc: link sibling where possible, even when not One True Path #124102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 21 additions & 9 deletions src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -725,13 +725,25 @@ pub(crate) fn href_with_root_path(
}

let mut is_remote = false;
let (fqp, shortty, url_parts) = match cache.paths.get(&did) {
Some(&(ref fqp, shortty)) => (fqp, shortty, {
let module_fqp = to_module_fqp(shortty, fqp.as_slice());
debug!(?fqp, ?shortty, ?module_fqp);
href_relative_parts(module_fqp, relative_to).collect()
}),
None => {
let chain_fqp: Vec<Symbol>;
let (fqp, shortty, url_parts) =
if let Some(&(shortty, name)) = cx.current_module_linkable_items.get(&did) {
// If this item exists in the current module, then link to that.
// The path within the current module might not be the One True Path,
// but link to it anyway, since a relative path to a sibling is shorter.
if shortty == ItemType::Module {
(relative_to, shortty, UrlPartsBuilder::singleton(name.as_str()))
} else {
chain_fqp = relative_to.iter().copied().chain([name]).collect();
(&chain_fqp, shortty, UrlPartsBuilder::new())
}
} else if let Some(&(ref fqp, shortty)) = cache.paths.get(&did) {
// If this item exists in the current crate, then link to that.
(fqp, shortty, {
let module_fqp = to_module_fqp(shortty, fqp.as_slice());
href_relative_parts(module_fqp, relative_to).collect()
})
} else {
// Associated items are handled differently with "jump to def". The anchor is generated
// directly here whereas for intra-doc links, we have some extra computation being
// performed there.
Expand All @@ -746,8 +758,8 @@ pub(crate) fn href_with_root_path(
} else {
return generate_item_def_id_path(did, original_did, cx, root_path, def_kind);
}
}
};
};
debug!(?fqp, ?shortty, ?url_parts);
make_href(root_path, shortty, url_parts, fqp, is_remote)
}

Expand Down
42 changes: 39 additions & 3 deletions src/librustdoc/html/render/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ pub(crate) struct Context<'tcx> {
/// Current hierarchy of components leading down to what's currently being
/// rendered
pub(crate) current: Vec<Symbol>,
/// Set of visible DefIds in the current module.
/// This generates optimal link hrefs in the common case when
/// an entire module is glob-inlined and its children link to each other.
pub(crate) current_module_linkable_items: DefIdMap<(ItemType, Symbol)>,
/// The current destination folder of where HTML artifacts should be placed.
/// This changes as the context descends into the module hierarchy.
pub(crate) dst: PathBuf,
Expand Down Expand Up @@ -79,9 +83,9 @@ pub(crate) struct Context<'tcx> {

// `Context` is cloned a lot, so we don't want the size to grow unexpectedly.
#[cfg(all(not(windows), target_pointer_width = "64"))]
rustc_data_structures::static_assert_size!(Context<'_>, 160);
rustc_data_structures::static_assert_size!(Context<'_>, 192);
#[cfg(all(windows, target_pointer_width = "64"))]
rustc_data_structures::static_assert_size!(Context<'_>, 168);
rustc_data_structures::static_assert_size!(Context<'_>, 200);

/// Shared mutable state used in [`Context`] and elsewhere.
pub(crate) struct SharedContext<'tcx> {
Expand Down Expand Up @@ -561,6 +565,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {

let mut cx = Context {
current: Vec::new(),
current_module_linkable_items: Default::default(),
dst,
render_redirect_pages: false,
id_map,
Expand Down Expand Up @@ -591,6 +596,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
fn make_child_renderer(&self) -> Self {
Self {
current: self.current.clone(),
current_module_linkable_items: self.current_module_linkable_items.clone(),
dst: self.dst.clone(),
render_redirect_pages: self.render_redirect_pages,
deref_id_map: Default::default(),
Expand Down Expand Up @@ -785,8 +791,38 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
info!("Recursing into {}", self.dst.display());

if !item.is_stripped() {
// Populate a list of items that are directly in the module
// or inlined into it.
// When an intra-doc link or named type needs linked,
// these are preferred over more distant paths.
let (clean::StrippedItem(box clean::ModuleItem(ref module))
| clean::ModuleItem(ref module)) = *item.kind
else {
unreachable!()
};
self.current_module_linkable_items = Default::default();
for item in &module.items {
if item.is_stripped() {
continue;
}
if let Some(def_id) = item.def_id()
&& let Some(name) = item.name
{
if let Some((path, item_type)) = self.cache().paths.get(&def_id)
&& *item_type == item.type_()
&& &path[path.len() - 1..] == &self.current[..]
{
// Avoid populating this list with no-ops.
// If this module is already the One True Path,
// that's sufficient.
continue;
}
self.current_module_linkable_items.insert(def_id, (item.type_(), name));
}
}
// Render the current module to HTML.
// Buf will be empty if the module is stripped and there is no redirect for it.
let buf = self.render_item(item, true);
// buf will be empty if the module is stripped and there is no redirect for it
if !buf.is_empty() {
self.shared.ensure_dir(&self.dst)?;
let joint_dst = self.dst.join("index.html");
Expand Down
45 changes: 45 additions & 0 deletions tests/rustdoc/intra-doc/glob-inline-siblings.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#![deny(rustdoc::broken_intra_doc_links)]
#![allow(nonstandard_style)]

// Reduced case from `core::arch::{arm, aarch64}`.
// Both versions of arm should link to the struct that they inlined.
// aarch64 should not link to the inlined copy in arm 32.

mod arm_shared {
pub struct uint32x4_t;
pub struct uint32x4x2_t(pub uint32x4_t, pub uint32x4_t);
pub fn vabaq_u32(
_a: uint32x4_t,
_b: uint32x4_t,
_c: uint32x4_t
) -> uint32x4_t {
uint32x4_t
}
}

pub mod arm {
pub use crate::arm_shared::*;
// @has glob_inline_siblings/arm/fn.vabaq_u32.html '//a[@href="struct.uint32x4_t.html"]' 'uint32x4_t'
// @has glob_inline_siblings/arm/struct.uint32x4x2_t.html '//a[@href="struct.uint32x4_t.html"]' 'uint32x4_t'
// @!has glob_inline_siblings/arm/fn.vabaq_u32.html '//a[@href="../aarch64/struct.uint32x4_t.html"]' 'uint32x4_t'
// @!has glob_inline_siblings/arm/struct.uint32x4x2_t.html '//a[@href="../aarch64/struct.uint32x4_t.html"]' 'uint32x4_t'

/// [uint32x4_t]
// @has glob_inline_siblings/arm/struct.LocalThing.html '//a[@href="struct.uint32x4_t.html"]' 'uint32x4_t'
// @!has glob_inline_siblings/arm/struct.LocalThing.html '//a[@href="../aarch64/struct.uint32x4_t.html"]' 'uint32x4_t'
pub struct LocalThing;
}


pub mod aarch64 {
pub use crate::arm_shared::*;
// @has glob_inline_siblings/aarch64/fn.vabaq_u32.html '//a[@href="struct.uint32x4_t.html"]' 'uint32x4_t'
// @has glob_inline_siblings/aarch64/struct.uint32x4x2_t.html '//a[@href="struct.uint32x4_t.html"]' 'uint32x4_t'
// @!has glob_inline_siblings/aarch64/fn.vabaq_u32.html '//a[@href="../arm/struct.uint32x4_t.html"]' 'uint32x4_t'
// @!has glob_inline_siblings/aarch64/struct.uint32x4x2_t.html '//a[@href="../arm/struct.uint32x4_t.html"]' 'uint32x4_t'

/// [uint32x4_t]
// @has glob_inline_siblings/aarch64/struct.LocalThing.html '//a[@href="struct.uint32x4_t.html"]' 'uint32x4_t'
// @!has glob_inline_siblings/aarch64/struct.LocalThing.html '//a[@href="../arm/struct.uint32x4_t.html"]' 'uint32x4_t'
pub struct LocalThing;
}