Skip to content

Commit

Permalink
Rollup merge of rust-lang#112836 - GuillaumeGomez:rustdoc-invalid-fil…
Browse files Browse the repository at this point in the history
…e-creation, r=notriddle

[rustdoc] partially fix invalid files creation

Part of rust-lang#111249. It only removes generation for modules which shouldn't exist. For files, we need the compiler to keep re-export information alive for external items so we can actually have the right path to their location as it's currently not generating them correctly.

In case the item is inlined, it shouldn't (and neither should its children) get a file generated.

r? `@notriddle`
  • Loading branch information
GuillaumeGomez committed Jun 21, 2023
2 parents a7b90cf + 3ad595a commit e5a4b67
Show file tree
Hide file tree
Showing 9 changed files with 138 additions and 22 deletions.
22 changes: 13 additions & 9 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -358,15 +358,15 @@ fn is_field_vis_inherited(tcx: TyCtxt<'_>, def_id: DefId) -> bool {

impl Item {
pub(crate) fn stability<'tcx>(&self, tcx: TyCtxt<'tcx>) -> Option<Stability> {
self.item_id.as_def_id().and_then(|did| tcx.lookup_stability(did))
self.def_id().and_then(|did| tcx.lookup_stability(did))
}

pub(crate) fn const_stability<'tcx>(&self, tcx: TyCtxt<'tcx>) -> Option<ConstStability> {
self.item_id.as_def_id().and_then(|did| tcx.lookup_const_stability(did))
self.def_id().and_then(|did| tcx.lookup_const_stability(did))
}

pub(crate) fn deprecation(&self, tcx: TyCtxt<'_>) -> Option<Deprecation> {
self.item_id.as_def_id().and_then(|did| tcx.lookup_deprecation(did))
self.def_id().and_then(|did| tcx.lookup_deprecation(did))
}

pub(crate) fn inner_docs(&self, tcx: TyCtxt<'_>) -> bool {
Expand All @@ -391,7 +391,7 @@ impl Item {
panic!("blanket impl item has non-blanket ID")
}
}
_ => self.item_id.as_def_id().map(|did| rustc_span(did, tcx)),
_ => self.def_id().map(|did| rustc_span(did, tcx)),
}
}

Expand Down Expand Up @@ -501,7 +501,7 @@ impl Item {
}

pub(crate) fn is_crate(&self) -> bool {
self.is_mod() && self.item_id.as_def_id().map_or(false, |did| did.is_crate_root())
self.is_mod() && self.def_id().map_or(false, |did| did.is_crate_root())
}
pub(crate) fn is_mod(&self) -> bool {
self.type_() == ItemType::Module
Expand Down Expand Up @@ -638,11 +638,11 @@ impl Item {
}
let header = match *self.kind {
ItemKind::ForeignFunctionItem(_) => {
let def_id = self.item_id.as_def_id().unwrap();
let def_id = self.def_id().unwrap();
let abi = tcx.fn_sig(def_id).skip_binder().abi();
hir::FnHeader {
unsafety: if abi == Abi::RustIntrinsic {
intrinsic_operation_unsafety(tcx, self.item_id.as_def_id().unwrap())
intrinsic_operation_unsafety(tcx, self.def_id().unwrap())
} else {
hir::Unsafety::Unsafe
},
Expand All @@ -659,7 +659,7 @@ impl Item {
}
}
ItemKind::FunctionItem(_) | ItemKind::MethodItem(_, _) | ItemKind::TyMethodItem(_) => {
let def_id = self.item_id.as_def_id().unwrap();
let def_id = self.def_id().unwrap();
build_fn_header(def_id, tcx, tcx.asyncness(def_id))
}
_ => return None,
Expand Down Expand Up @@ -738,7 +738,7 @@ impl Item {
}
})
.collect();
if let Some(def_id) = self.item_id.as_def_id() &&
if let Some(def_id) = self.def_id() &&
!def_id.is_local() &&
// This check is needed because `adt_def` will panic if not a compatible type otherwise...
matches!(self.type_(), ItemType::Struct | ItemType::Enum | ItemType::Union)
Expand Down Expand Up @@ -787,6 +787,10 @@ impl Item {
pub fn is_doc_hidden(&self) -> bool {
self.attrs.is_doc_hidden()
}

pub fn def_id(&self) -> Option<DefId> {
self.item_id.as_def_id()
}
}

#[derive(Clone, Debug)]
Expand Down
5 changes: 5 additions & 0 deletions src/librustdoc/formats/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ pub(crate) struct Cache {
pub(crate) intra_doc_links: FxHashMap<ItemId, FxIndexSet<clean::ItemLink>>,
/// Cfg that have been hidden via #![doc(cfg_hide(...))]
pub(crate) hidden_cfg: FxHashSet<clean::cfg::Cfg>,

/// Contains the list of `DefId`s which have been inlined. It is used when generating files
/// to check if a stripped item should get its file generated or not: if it's inside a
/// `#[doc(hidden)]` item or a private one and not inlined, it shouldn't get a file.
pub(crate) inlined_items: DefIdSet,
}

/// This struct is used to wrap the `cache` and `tcx` in order to run `DocFolder`.
Expand Down
41 changes: 34 additions & 7 deletions src/librustdoc/html/render/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,8 @@ pub(crate) struct Context<'tcx> {
pub(crate) include_sources: bool,
/// Collection of all types with notable traits referenced in the current module.
pub(crate) types_with_notable_traits: FxHashSet<clean::Type>,
/// Field used during rendering, to know if we're inside an inlined item.
pub(crate) is_inside_inlined_module: bool,
}

// `Context` is cloned a lot, so we don't want the size to grow unexpectedly.
Expand Down Expand Up @@ -171,6 +173,19 @@ impl<'tcx> Context<'tcx> {
}

fn render_item(&mut self, it: &clean::Item, is_module: bool) -> String {
let mut render_redirect_pages = self.render_redirect_pages;
// If the item is stripped but inlined, links won't point to the item so no need to generate
// a file for it.
if it.is_stripped() &&
let Some(def_id) = it.def_id() &&
def_id.is_local()
{
if self.is_inside_inlined_module || self.shared.cache.inlined_items.contains(&def_id) {
// For now we're forced to generate a redirect page for stripped items until
// `record_extern_fqn` correctly points to external items.
render_redirect_pages = true;
}
}
let mut title = String::new();
if !is_module {
title.push_str(it.name.unwrap().as_str());
Expand Down Expand Up @@ -205,7 +220,7 @@ impl<'tcx> Context<'tcx> {
tyname.as_str()
};

if !self.render_redirect_pages {
if !render_redirect_pages {
let clone_shared = Rc::clone(&self.shared);
let page = layout::Page {
css_class: tyname_s,
Expand Down Expand Up @@ -545,6 +560,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
shared: Rc::new(scx),
include_sources,
types_with_notable_traits: FxHashSet::default(),
is_inside_inlined_module: false,
};

if emit_crate {
Expand Down Expand Up @@ -574,6 +590,7 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {
shared: Rc::clone(&self.shared),
include_sources: self.include_sources,
types_with_notable_traits: FxHashSet::default(),
is_inside_inlined_module: self.is_inside_inlined_module,
}
}

Expand Down Expand Up @@ -768,12 +785,22 @@ impl<'tcx> FormatRenderer<'tcx> for Context<'tcx> {

info!("Recursing into {}", self.dst.display());

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");
self.shared.fs.write(joint_dst, buf)?;
if !item.is_stripped() {
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");
self.shared.fs.write(joint_dst, buf)?;
}
}
if !self.is_inside_inlined_module {
if let Some(def_id) = item.def_id() && self.cache().inlined_items.contains(&def_id) {
self.is_inside_inlined_module = true;
}
} else if item.is_doc_hidden() {
// We're not inside an inlined module anymore since this one cannot be re-exported.
self.is_inside_inlined_module = false;
}

// Render sidebar-items.js used throughout this module.
Expand Down
7 changes: 5 additions & 2 deletions src/librustdoc/visit_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
return false;
}

let ret = match tcx.hir().get_by_def_id(res_did) {
let inlined = match tcx.hir().get_by_def_id(res_did) {
// Bang macros are handled a bit on their because of how they are handled by the
// compiler. If they have `#[doc(hidden)]` and the re-export doesn't have
// `#[doc(inline)]`, then we don't inline it.
Expand Down Expand Up @@ -344,7 +344,10 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
_ => false,
};
self.view_item_stack.remove(&res_did);
ret
if inlined {
self.cx.cache.inlined_items.insert(res_did.to_def_id());
}
inlined
}

/// Returns `true` if the item is visible, meaning it's not `#[doc(hidden)]` or private.
Expand Down
23 changes: 23 additions & 0 deletions tests/rustdoc/files-creation-hidden.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
#![crate_name="foo"]

// @!has "foo/struct.Foo.html"
#[doc(hidden)]
pub struct Foo;

// @!has "foo/struct.Bar.html"
pub use crate::Foo as Bar;

// @!has "foo/struct.Baz.html"
#[doc(hidden)]
pub use crate::Foo as Baz;

// @!has "foo/foo/index.html"
#[doc(hidden)]
pub mod foo {}

// @!has "foo/bar/index.html"
pub use crate::foo as bar;

// @!has "foo/baz/index.html"
#[doc(hidden)]
pub use crate::foo as baz;
18 changes: 18 additions & 0 deletions tests/rustdoc/files-creation-private.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
#![crate_name="foo"]

// @!has "foo/priv/index.html"
// @!has "foo/priv/struct.Foo.html"
mod private {
pub struct Foo;
}

// @has "foo/struct.Bar.html"
pub use crate::private::Foo as Bar;

// @!has "foo/foo/index.html"
mod foo {
pub mod subfoo {}
}

// @has "foo/bar/index.html"
pub use crate::foo::subfoo as bar;
8 changes: 4 additions & 4 deletions tests/rustdoc/issue-111064-reexport-trait-from-hidden-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
#![crate_name = "foo"]

// @!has 'foo/hidden/index.html'
// FIXME: add missing `@` for the two next tests once issue is fixed!
// To be done in <https://github.com/rust-lang/rust/issues/111249>.
// !has 'foo/hidden/inner/index.html'
// !has 'foo/hidden/inner/trait.Foo.html'
// @!has 'foo/hidden/inner/index.html'
// FIXME: Should be `@!has`: https://github.com/rust-lang/rust/issues/111249
// @has 'foo/hidden/inner/trait.Foo.html'
// @matchesraw - '<meta http-equiv="refresh" content="0;URL=../../../foo/visible/trait.Foo.html">'
#[doc(hidden)]
pub mod hidden {
pub mod inner {
Expand Down
34 changes: 34 additions & 0 deletions tests/rustdoc/issue-111249-file-creation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#![crate_name = "foo"]
#![feature(no_core)]
#![no_core]

// The following five should not fail!
// @!has 'foo/hidden/index.html'
// @!has 'foo/hidden/inner/index.html'
// FIXME: Should be `@!has`: https://github.com/rust-lang/rust/issues/111249
// @has 'foo/hidden/inner/trait.Foo.html'
// @matchesraw - '<meta http-equiv="refresh" content="0;URL=../../../foo/visible/trait.Foo.html">'
// @!has 'foo/hidden/inner/inner_hidden/index.html'
// @!has 'foo/hidden/inner/inner_hidden/trait.HiddenFoo.html'
#[doc(hidden)]
pub mod hidden {
pub mod inner {
pub trait Foo {}

#[doc(hidden)]
pub mod inner_hidden {
pub trait HiddenFoo {}
}
}
}

// @has 'foo/visible/index.html'
// @has 'foo/visible/trait.Foo.html'
#[doc(inline)]
pub use hidden::inner as visible;

// @has 'foo/struct.Bar.html'
// @count - '//*[@id="impl-Foo-for-Bar"]' 1
pub struct Bar;

impl visible::Foo for Bar {}
2 changes: 2 additions & 0 deletions tests/rustdoc/redirect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@ pub trait Foo {}
// @has - '//code' 'pub use reexp_stripped::Bar'
// @has - '//code/a' 'Bar'
// @has - '//a[@href="../reexp_stripped/hidden/struct.Bar.html"]' 'Bar'
// FIXME: Should be `@!has`: https://github.com/rust-lang/rust/issues/111249
// @has reexp_stripped/hidden/struct.Bar.html
// @matchesraw - '<meta http-equiv="refresh" content="0;URL=../../reexp_stripped/struct.Bar.html">'
// @has 'reexp_stripped/struct.Bar.html'
// @has - '//a[@href="struct.Bar.html"]' 'Bar'
#[doc(no_inline)]
Expand Down

0 comments on commit e5a4b67

Please sign in to comment.