Skip to content

Commit 4a2709d

Browse files
Fix weird rustdoc output when single and glob reexport conflict on a name
1 parent a413f77 commit 4a2709d

File tree

3 files changed

+83
-38
lines changed

3 files changed

+83
-38
lines changed

src/librustdoc/clean/inline.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,12 @@ pub(crate) fn try_inline(
152152
};
153153

154154
cx.inlined.insert(did.into());
155+
let import_def_ids: &[LocalDefId] = match import_def_id {
156+
Some(import_def_id) => &[import_def_id],
157+
None => &[],
158+
};
155159
let mut item =
156-
crate::clean::generate_item_with_correct_attrs(cx, kind, did, name, import_def_id, None);
160+
crate::clean::generate_item_with_correct_attrs(cx, kind, did, name, import_def_ids, None);
157161
// The visibility needs to reflect the one from the reexport and not from the "source" DefId.
158162
item.inner.inline_stmt_id = import_def_id;
159163
ret.push(item);

src/librustdoc/clean/mod.rs

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,12 @@ pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext<
9494
// This covers the case where somebody does an import which should pull in an item,
9595
// but there's already an item with the same namespace and same name. Rust gives
9696
// priority to the not-imported one, so we should, too.
97-
items.extend(doc.items.values().flat_map(|(item, renamed, import_id)| {
97+
items.extend(doc.items.values().flat_map(|(item, renamed, import_ids)| {
9898
// First, lower everything other than glob imports.
9999
if matches!(item.kind, hir::ItemKind::Use(_, hir::UseKind::Glob)) {
100100
return Vec::new();
101101
}
102-
let v = clean_maybe_renamed_item(cx, item, *renamed, *import_id);
102+
let v = clean_maybe_renamed_item(cx, item, *renamed, import_ids);
103103
for item in &v {
104104
if let Some(name) = item.name
105105
&& (cx.render_options.document_hidden || !item.is_doc_hidden())
@@ -157,12 +157,16 @@ pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext<
157157
});
158158

159159
let kind = ModuleItem(Module { items, span });
160+
let import_def_ids: &[LocalDefId] = match doc.import_id {
161+
Some(import_def_id) => &[import_def_id],
162+
None => &[],
163+
};
160164
generate_item_with_correct_attrs(
161165
cx,
162166
kind,
163167
doc.def_id.to_def_id(),
164168
doc.name,
165-
doc.import_id,
169+
import_def_ids,
166170
doc.renamed,
167171
)
168172
}
@@ -182,22 +186,29 @@ fn generate_item_with_correct_attrs(
182186
kind: ItemKind,
183187
def_id: DefId,
184188
name: Symbol,
185-
import_id: Option<LocalDefId>,
189+
import_ids: &[LocalDefId],
186190
renamed: Option<Symbol>,
187191
) -> Item {
188192
let target_attrs = inline::load_attrs(cx, def_id);
189-
let attrs = if let Some(import_id) = import_id {
190-
// glob reexports are treated the same as `#[doc(inline)]` items.
191-
//
192-
// For glob re-exports the item may or may not exist to be re-exported (potentially the cfgs
193-
// on the path up until the glob can be removed, and only cfgs on the globbed item itself
194-
// matter), for non-inlined re-exports see #85043.
195-
let is_inline = hir_attr_lists(inline::load_attrs(cx, import_id.to_def_id()), sym::doc)
196-
.get_word_attr(sym::inline)
197-
.is_some()
198-
|| (is_glob_import(cx.tcx, import_id)
199-
&& (cx.render_options.document_hidden || !cx.tcx.is_doc_hidden(def_id)));
200-
let mut attrs = get_all_import_attributes(cx, import_id, def_id, is_inline);
193+
let attrs = if !import_ids.is_empty() {
194+
let mut attrs = Vec::with_capacity(import_ids.len());
195+
let mut is_inline = false;
196+
197+
for import_id in import_ids.iter().copied() {
198+
// glob reexports are treated the same as `#[doc(inline)]` items.
199+
//
200+
// For glob re-exports the item may or may not exist to be re-exported (potentially the
201+
// cfgs on the path up until the glob can be removed, and only cfgs on the globbed item
202+
// itself matter), for non-inlined re-exports see #85043.
203+
let import_is_inline =
204+
hir_attr_lists(inline::load_attrs(cx, import_id.to_def_id()), sym::doc)
205+
.get_word_attr(sym::inline)
206+
.is_some()
207+
|| (is_glob_import(cx.tcx, import_id)
208+
&& (cx.render_options.document_hidden || !cx.tcx.is_doc_hidden(def_id)));
209+
attrs.extend(get_all_import_attributes(cx, import_id, def_id, is_inline));
210+
is_inline = is_inline || import_is_inline;
211+
}
201212
add_without_unwanted_attributes(&mut attrs, target_attrs, is_inline, None);
202213
attrs
203214
} else {
@@ -216,7 +227,8 @@ fn generate_item_with_correct_attrs(
216227

217228
let name = renamed.or(Some(name));
218229
let mut item = Item::from_def_id_and_attrs_and_parts(def_id, name, kind, attrs, cfg);
219-
item.inner.inline_stmt_id = import_id;
230+
// FIXME (GuillaumeGomez): Should we also make `inline_stmt_id` a `Vec` instead of an `Option`?
231+
item.inner.inline_stmt_id = import_ids.first().copied();
220232
item
221233
}
222234

@@ -2759,7 +2771,7 @@ fn clean_maybe_renamed_item<'tcx>(
27592771
cx: &mut DocContext<'tcx>,
27602772
item: &hir::Item<'tcx>,
27612773
renamed: Option<Symbol>,
2762-
import_id: Option<LocalDefId>,
2774+
import_ids: &[LocalDefId],
27632775
) -> Vec<Item> {
27642776
use hir::ItemKind;
27652777

@@ -2831,7 +2843,7 @@ fn clean_maybe_renamed_item<'tcx>(
28312843
})),
28322844
item.owner_id.def_id.to_def_id(),
28332845
name,
2834-
import_id,
2846+
import_ids,
28352847
renamed,
28362848
));
28372849
return ret;
@@ -2886,7 +2898,7 @@ fn clean_maybe_renamed_item<'tcx>(
28862898
kind,
28872899
item.owner_id.def_id.to_def_id(),
28882900
name,
2889-
import_id,
2901+
import_ids,
28902902
renamed,
28912903
)]
28922904
})

src/librustdoc/visit_ast.rs

Lines changed: 46 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,28 @@ pub(crate) struct Module<'hir> {
3232
pub(crate) def_id: LocalDefId,
3333
pub(crate) renamed: Option<Symbol>,
3434
pub(crate) import_id: Option<LocalDefId>,
35-
/// The key is the item `ItemId` and the value is: (item, renamed, import_id).
35+
/// The key is the item `ItemId` and the value is: (item, renamed, Vec<import_id>).
3636
/// We use `FxIndexMap` to keep the insert order.
37+
///
38+
/// `import_id` needs to be a `Vec` because we live in a dark world where you can have code
39+
/// like:
40+
///
41+
/// ```
42+
/// mod raw {
43+
/// pub fn foo() {}
44+
/// }
45+
///
46+
/// /// Foobar
47+
/// pub use raw::foo;
48+
///
49+
/// pub use raw::*;
50+
/// ```
51+
///
52+
/// So in this case, we don't want to have two items but just one with attributes from both
53+
/// imports to be merged.
3754
pub(crate) items: FxIndexMap<
3855
(LocalDefId, Option<Symbol>),
39-
(&'hir hir::Item<'hir>, Option<Symbol>, Option<LocalDefId>),
56+
(&'hir hir::Item<'hir>, Option<Symbol>, Vec<LocalDefId>),
4057
>,
4158
/// Same as for `items`.
4259
pub(crate) inlined_foreigns: FxIndexMap<(DefId, Option<Symbol>), (Res, LocalDefId)>,
@@ -145,7 +162,9 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
145162
{
146163
let item = self.cx.tcx.hir_expect_item(local_def_id);
147164
let (ident, _, _) = item.expect_macro();
148-
top_level_module.items.insert((local_def_id, Some(ident.name)), (item, None, None));
165+
top_level_module
166+
.items
167+
.insert((local_def_id, Some(ident.name)), (item, None, Vec::new()));
149168
}
150169
}
151170

@@ -227,7 +246,6 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
227246
) -> bool {
228247
debug!("maybe_inline_local (renamed: {renamed:?}) res: {res:?}");
229248

230-
let glob = renamed.is_none();
231249
if renamed == Some(kw::Underscore) {
232250
// We never inline `_` reexports.
233251
return false;
@@ -252,14 +270,15 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
252270
return false;
253271
}
254272

273+
let is_glob = renamed.is_none();
255274
let is_hidden = !document_hidden && tcx.is_doc_hidden(ori_res_did);
256275
let Some(res_did) = ori_res_did.as_local() else {
257276
// For cross-crate impl inlining we need to know whether items are
258277
// reachable in documentation -- a previously unreachable item can be
259278
// made reachable by cross-crate inlining which we're checking here.
260279
// (this is done here because we need to know this upfront).
261280
crate::visit_lib::lib_embargo_visit_item(self.cx, ori_res_did);
262-
if is_hidden || glob {
281+
if is_hidden || is_glob {
263282
return false;
264283
}
265284
// We store inlined foreign items otherwise, it'd mean that the `use` item would be kept
@@ -307,10 +326,10 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
307326
// Bang macros are handled a bit on their because of how they are handled by the
308327
// compiler. If they have `#[doc(hidden)]` and the re-export doesn't have
309328
// `#[doc(inline)]`, then we don't inline it.
310-
Node::Item(_) if is_bang_macro && !please_inline && renamed.is_some() && is_hidden => {
329+
Node::Item(_) if is_bang_macro && !please_inline && !is_glob && is_hidden => {
311330
return false;
312331
}
313-
Node::Item(&hir::Item { kind: hir::ItemKind::Mod(_, m), .. }) if glob => {
332+
Node::Item(&hir::Item { kind: hir::ItemKind::Mod(_, m), .. }) if is_glob => {
314333
let prev = mem::replace(&mut self.inlining, true);
315334
for &i in m.item_ids {
316335
let i = tcx.hir_item(i);
@@ -319,13 +338,13 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
319338
self.inlining = prev;
320339
true
321340
}
322-
Node::Item(it) if !glob => {
341+
Node::Item(it) if !is_glob => {
323342
let prev = mem::replace(&mut self.inlining, true);
324343
self.visit_item_inner(it, renamed, Some(def_id));
325344
self.inlining = prev;
326345
true
327346
}
328-
Node::ForeignItem(it) if !glob => {
347+
Node::ForeignItem(it) if !is_glob => {
329348
let prev = mem::replace(&mut self.inlining, true);
330349
self.visit_foreign_item_inner(it, renamed);
331350
self.inlining = prev;
@@ -369,8 +388,8 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
369388
fn add_to_current_mod(
370389
&mut self,
371390
item: &'tcx hir::Item<'_>,
372-
renamed: Option<Symbol>,
373-
parent_id: Option<LocalDefId>,
391+
mut renamed: Option<Symbol>,
392+
import_id: Option<LocalDefId>,
374393
) {
375394
if self.is_importable_from_parent
376395
// If we're inside an item, only impl blocks and `macro_rules!` with the `macro_export`
@@ -383,11 +402,21 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
383402
_ => false,
384403
}
385404
{
386-
self.modules
387-
.last_mut()
388-
.unwrap()
389-
.items
390-
.insert((item.owner_id.def_id, renamed), (item, renamed, parent_id));
405+
if renamed == item.kind.ident().map(|ident| ident.name) {
406+
renamed = None;
407+
}
408+
let key = (item.owner_id.def_id, renamed);
409+
if let Some(import_id) = import_id {
410+
self.modules
411+
.last_mut()
412+
.unwrap()
413+
.items
414+
.entry(key)
415+
.and_modify(|v| v.2.push(import_id))
416+
.or_insert_with(|| (item, renamed, vec![import_id]));
417+
} else {
418+
self.modules.last_mut().unwrap().items.insert(key, (item, renamed, Vec::new()));
419+
}
391420
}
392421
}
393422

@@ -459,7 +488,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
459488
_ => false,
460489
});
461490
let ident = match kind {
462-
hir::UseKind::Single(ident) => Some(renamed.unwrap_or(ident.name)),
491+
hir::UseKind::Single(ident) => Some(ident.name),
463492
hir::UseKind::Glob => None,
464493
hir::UseKind::ListStem => unreachable!(),
465494
};

0 commit comments

Comments
 (0)