Skip to content

Fix missing attribute merge on glob foreign re-exports #114012

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

Merged
merged 2 commits into from
Jul 26, 2023
Merged
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
16 changes: 13 additions & 3 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ pub(crate) fn try_inline_glob(
current_mod: LocalDefId,
visited: &mut DefIdSet,
inlined_names: &mut FxHashSet<(ItemType, Symbol)>,
import: &hir::Item<'_>,
) -> Option<Vec<clean::Item>> {
let did = res.opt_def_id()?;
if did.is_local() {
Expand All @@ -158,7 +159,15 @@ pub(crate) fn try_inline_glob(
.filter(|child| !child.reexport_chain.is_empty())
.filter_map(|child| child.res.opt_def_id())
.collect();
let mut items = build_module_items(cx, did, visited, inlined_names, Some(&reexports));
let attrs = cx.tcx.hir().attrs(import.hir_id());
let mut items = build_module_items(
cx,
did,
visited,
inlined_names,
Some(&reexports),
Some((attrs, Some(import.owner_id.def_id.to_def_id()))),
);
items.retain(|item| {
if let Some(name) = item.name {
// If an item with the same type and name already exists,
Expand Down Expand Up @@ -547,7 +556,7 @@ pub(crate) fn build_impl(
}

fn build_module(cx: &mut DocContext<'_>, did: DefId, visited: &mut DefIdSet) -> clean::Module {
let items = build_module_items(cx, did, visited, &mut FxHashSet::default(), None);
let items = build_module_items(cx, did, visited, &mut FxHashSet::default(), None, None);

let span = clean::Span::new(cx.tcx.def_span(did));
clean::Module { items, span }
Expand All @@ -559,6 +568,7 @@ fn build_module_items(
visited: &mut DefIdSet,
inlined_names: &mut FxHashSet<(ItemType, Symbol)>,
allowed_def_ids: Option<&DefIdSet>,
attrs: Option<(&[ast::Attribute], Option<DefId>)>,
) -> Vec<clean::Item> {
let mut items = Vec::new();

Expand Down Expand Up @@ -613,7 +623,7 @@ fn build_module_items(
cfg: None,
inline_stmt_id: None,
});
} else if let Some(i) = try_inline(cx, res, item.ident.name, None, visited) {
} else if let Some(i) = try_inline(cx, res, item.ident.name, attrs, visited) {
items.extend(i)
}
}
Expand Down
37 changes: 24 additions & 13 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,25 +132,31 @@ pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext<
});

let kind = ModuleItem(Module { items, span });
generate_item_with_correct_attrs(cx, kind, doc.def_id, doc.name, doc.import_id, doc.renamed)
generate_item_with_correct_attrs(
cx,
kind,
doc.def_id.to_def_id(),
doc.name,
doc.import_id,
doc.renamed,
)
}

fn generate_item_with_correct_attrs(
cx: &mut DocContext<'_>,
kind: ItemKind,
local_def_id: LocalDefId,
def_id: DefId,
name: Symbol,
import_id: Option<LocalDefId>,
renamed: Option<Symbol>,
) -> Item {
let def_id = local_def_id.to_def_id();
let target_attrs = inline::load_attrs(cx, def_id);
let attrs = if let Some(import_id) = import_id {
let is_inline = inline::load_attrs(cx, import_id.to_def_id())
.lists(sym::doc)
.get_word_attr(sym::inline)
.is_some();
let mut attrs = get_all_import_attributes(cx, import_id, local_def_id, is_inline);
let mut attrs = get_all_import_attributes(cx, import_id, def_id, is_inline);
add_without_unwanted_attributes(&mut attrs, target_attrs, is_inline, None);
attrs
} else {
Expand Down Expand Up @@ -2308,10 +2314,10 @@ fn clean_bare_fn_ty<'tcx>(
pub(crate) fn reexport_chain<'tcx>(
tcx: TyCtxt<'tcx>,
import_def_id: LocalDefId,
target_def_id: LocalDefId,
target_def_id: DefId,
) -> &'tcx [Reexport] {
for child in tcx.module_children_local(tcx.local_parent(import_def_id)) {
if child.res.opt_def_id() == Some(target_def_id.to_def_id())
if child.res.opt_def_id() == Some(target_def_id)
&& child.reexport_chain.first().and_then(|r| r.id()) == Some(import_def_id.to_def_id())
{
return &child.reexport_chain;
Expand All @@ -2324,7 +2330,7 @@ pub(crate) fn reexport_chain<'tcx>(
fn get_all_import_attributes<'hir>(
cx: &mut DocContext<'hir>,
import_def_id: LocalDefId,
target_def_id: LocalDefId,
target_def_id: DefId,
is_inline: bool,
) -> Vec<(Cow<'hir, ast::Attribute>, Option<DefId>)> {
let mut attrs = Vec::new();
Expand Down Expand Up @@ -2541,7 +2547,7 @@ fn clean_maybe_renamed_item<'tcx>(
vec![generate_item_with_correct_attrs(
cx,
kind,
item.owner_id.def_id,
item.owner_id.def_id.to_def_id(),
name,
import_id,
renamed,
Expand Down Expand Up @@ -2694,6 +2700,7 @@ fn clean_use_statement_inner<'tcx>(
let inline_attr = attrs.lists(sym::doc).get_word_attr(sym::inline);
let pub_underscore = visibility.is_public() && name == kw::Underscore;
let current_mod = cx.tcx.parent_module_from_def_id(import.owner_id.def_id);
let import_def_id = import.owner_id.def_id.to_def_id();

// The parent of the module in which this import resides. This
// is the same as `current_mod` if that's already the top
Expand Down Expand Up @@ -2744,9 +2751,14 @@ fn clean_use_statement_inner<'tcx>(
let inner = if kind == hir::UseKind::Glob {
if !denied {
let mut visited = DefIdSet::default();
if let Some(items) =
inline::try_inline_glob(cx, path.res, current_mod, &mut visited, inlined_names)
{
if let Some(items) = inline::try_inline_glob(
cx,
path.res,
current_mod,
&mut visited,
inlined_names,
import,
) {
return items;
}
}
Expand All @@ -2762,7 +2774,6 @@ fn clean_use_statement_inner<'tcx>(
denied = true;
}
if !denied {
let import_def_id = import.owner_id.to_def_id();
if let Some(mut items) = inline::try_inline(
cx,
path.res,
Expand All @@ -2782,7 +2793,7 @@ fn clean_use_statement_inner<'tcx>(
Import::new_simple(name, resolve_use_source(cx, path), true)
};

vec![Item::from_def_id_and_parts(import.owner_id.to_def_id(), None, ImportItem(inner), cx)]
vec![Item::from_def_id_and_parts(import_def_id, None, ImportItem(inner), cx)]
}

fn clean_maybe_renamed_foreign_item<'tcx>(
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/visit_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
// made reachable by cross-crate inlining which we're checking here.
// (this is done here because we need to know this upfront).
crate::visit_lib::lib_embargo_visit_item(self.cx, ori_res_did);
if is_hidden {
if is_hidden || glob {
return false;
}
// We store inlined foreign items otherwise, it'd mean that the `use` item would be kept
Expand Down Expand Up @@ -376,7 +376,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
return true;
}
let tcx = self.cx.tcx;
let item_def_id = reexport_chain(tcx, import_def_id, target_def_id)
let item_def_id = reexport_chain(tcx, import_def_id, target_def_id.to_def_id())
.iter()
.flat_map(|reexport| reexport.id())
.map(|id| id.expect_local())
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
#![crate_name = "colors"]

pub struct Color;
19 changes: 19 additions & 0 deletions tests/rustdoc/issue-113982-doc_auto_cfg-reexport-foreign.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// aux-build: issue-113982-doc_auto_cfg-reexport-foreign.rs

#![feature(no_core, doc_auto_cfg)]
#![no_core]
#![crate_name = "foo"]

extern crate colors;

// @has 'foo/index.html' '//*[@class="stab portability"]' 'Non-colors'
// @has 'foo/struct.Color.html' '//*[@class="stab portability"]' \
// 'Available on non-crate feature colors only.'
#[cfg(not(feature = "colors"))]
pub use colors::*;

// @has 'foo/index.html' '//*[@class="stab portability"]' 'Non-fruits'
// @has 'foo/struct.Red.html' '//*[@class="stab portability"]' \
// 'Available on non-crate feature fruits only.'
#[cfg(not(feature = "fruits"))]
pub use colors::Color as Red;