Skip to content

Commit

Permalink
Unrolled build for rust-lang#123459
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#123459 - GuillaumeGomez:fix-123435, r=notriddle

Correctly handle inlining of doc hidden foreign items

Fixes rust-lang#123435.

In case a foreign item has doc(hidden) attribute, we simply merged its attributes with the re-export's, making it being removed once in the `strip_hidden` pass.

The solution was to use the same as for local reexported items: merge attributes, but not some of them (like `doc(hidden)`).

I originally checked if we could simply update `Item::is_doc_hidden` method to use `self.inline_stmt_id.is_some_and(|def_id| tcx.is_doc_hidden(def_id))` but unfortunately, it added (local) items that shouldn't be inlined. At least it unifies local and foreign items inlining, which I think is the best course of action here.

r? `@notriddle`
  • Loading branch information
rust-timer committed Apr 11, 2024
2 parents aa6a697 + 5a0be6f commit 067b7aa
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 23 deletions.
19 changes: 14 additions & 5 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ pub(crate) fn try_inline(

let import_def_id = attrs.and_then(|(_, def_id)| def_id);

let (attrs, cfg) = merge_attrs(cx, load_attrs(cx, did), attrs);

let kind = match res {
Res::Def(DefKind::Trait, did) => {
record_extern_fqn(cx, did, ItemType::Trait);
Expand Down Expand Up @@ -134,7 +132,11 @@ pub(crate) fn try_inline(
cx.with_param_env(did, |cx| clean::ConstantItem(build_const(cx, did)))
}
Res::Def(DefKind::Macro(kind), did) => {
let mac = build_macro(cx, did, name, import_def_id, kind, attrs.is_doc_hidden());
let is_doc_hidden = cx.tcx.is_doc_hidden(did)
|| attrs_without_docs
.map(|(attrs, _)| attrs)
.is_some_and(|attrs| utils::attrs_have_doc_flag(attrs.iter(), sym::hidden));
let mac = build_macro(cx, did, name, import_def_id, kind, is_doc_hidden);

let type_kind = match kind {
MacroKind::Bang => ItemType::Macro,
Expand All @@ -148,8 +150,14 @@ pub(crate) fn try_inline(
};

cx.inlined.insert(did.into());
let mut item =
clean::Item::from_def_id_and_attrs_and_parts(did, Some(name), kind, Box::new(attrs), cfg);
let mut item = crate::clean::generate_item_with_correct_attrs(
cx,
kind,
did,
name,
import_def_id.and_then(|def_id| def_id.as_local()),
None,
);
// The visibility needs to reflect the one from the reexport and not from the "source" DefId.
item.inline_stmt_id = import_def_id;
ret.push(item);
Expand Down Expand Up @@ -179,6 +187,7 @@ pub(crate) fn try_inline_glob(
.iter()
.filter(|child| !child.reexport_chain.is_empty())
.filter_map(|child| child.res.opt_def_id())
.filter(|def_id| !cx.tcx.is_doc_hidden(def_id))
.collect();
let attrs = cx.tcx.hir().attrs(import.hir_id());
let mut items = build_module_items(
Expand Down
22 changes: 11 additions & 11 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3007,22 +3007,22 @@ fn clean_use_statement_inner<'tcx>(
// were specifically asked for it
denied = true;
}
if !denied {
if let Some(mut items) = inline::try_inline(
if !denied
&& let Some(mut items) = inline::try_inline(
cx,
path.res,
name,
Some((attrs, Some(import_def_id))),
&mut Default::default(),
) {
items.push(Item::from_def_id_and_parts(
import_def_id,
None,
ImportItem(Import::new_simple(name, resolve_use_source(cx, path), false)),
cx,
));
return items;
}
)
{
items.push(Item::from_def_id_and_parts(
import_def_id,
None,
ImportItem(Import::new_simple(name, resolve_use_source(cx, path), false)),
cx,
));
return items;
}
Import::new_simple(name, resolve_use_source(cx, path), true)
};
Expand Down
9 changes: 8 additions & 1 deletion src/librustdoc/clean/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,14 @@ pub(crate) fn find_nearest_parent_module(tcx: TyCtxt<'_>, def_id: DefId) -> Opti
/// This function exists because it runs on `hir::Attributes` whereas the other is a
/// `clean::Attributes` method.
pub(crate) fn has_doc_flag(tcx: TyCtxt<'_>, did: DefId, flag: Symbol) -> bool {
tcx.get_attrs(did, sym::doc)
attrs_have_doc_flag(tcx.get_attrs(did, sym::doc), flag)
}

pub(crate) fn attrs_have_doc_flag<'a>(
mut attrs: impl Iterator<Item = &'a ast::Attribute>,
flag: Symbol,
) -> bool {
attrs
.any(|attr| attr.meta_item_list().is_some_and(|l| rustc_attr::list_contains_name(&l, flag)))
}

Expand Down
10 changes: 4 additions & 6 deletions src/librustdoc/formats/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -203,10 +203,10 @@ impl Cache {
impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
fn fold_item(&mut self, item: clean::Item) -> Option<clean::Item> {
if item.item_id.is_local() {
let is_stripped = matches!(*item.kind, clean::ItemKind::StrippedItem(..));
debug!(
"folding {} (stripped: {is_stripped:?}) \"{:?}\", id {:?}",
"folding {} (stripped: {:?}) \"{:?}\", id {:?}",
item.type_(),
item.is_stripped(),
item.name,
item.item_id
);
Expand Down Expand Up @@ -246,13 +246,11 @@ impl<'a, 'tcx> DocFolder for CacheBuilder<'a, 'tcx> {
// trait.
if let clean::TraitItem(ref t) = *item.kind {
self.cache.traits.entry(item.item_id.expect_def_id()).or_insert_with(|| (**t).clone());
}

// Collect all the implementors of traits.
if let clean::ImplItem(ref i) = *item.kind
} else if let clean::ImplItem(ref i) = *item.kind
&& let Some(trait_) = &i.trait_
&& !i.kind.is_blanket()
{
// Collect all the implementors of traits.
self.cache
.implementors
.entry(trait_.def_id())
Expand Down
14 changes: 14 additions & 0 deletions tests/rustdoc/inline_cross/inline_hidden.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,23 @@

extern crate rustdoc_hidden;

// @has inline_hidden/index.html
// Ensures this item is not inlined.
// @has - '//*[@id="reexport.Foo"]/code' 'pub use rustdoc_hidden::Foo;'
#[doc(no_inline)]
pub use rustdoc_hidden::Foo;

// Even if the foreign item has `doc(hidden)`, we should be able to inline it.
// @has - '//*[@class="item-name"]/a[@class="struct"]' 'Inlined'
#[doc(inline)]
pub use rustdoc_hidden::Foo as Inlined;

// Even with this import, we should not see `Foo`.
// @count - '//*[@class="item-name"]' 4
// @has - '//*[@class="item-name"]/a[@class="struct"]' 'Bar'
// @has - '//*[@class="item-name"]/a[@class="fn"]' 'foo'
pub use rustdoc_hidden::*;

// @has inline_hidden/fn.foo.html
// @!has - '//a/@title' 'Foo'
pub fn foo(_: Foo) {}

0 comments on commit 067b7aa

Please sign in to comment.