From 6fb4ce9f88ee0651a7a05f512a155de21f395e11 Mon Sep 17 00:00:00 2001
From: Guillaume Gomez <guillaume.gomez@huawei.com>
Date: Sun, 16 Jul 2023 23:22:16 +0200
Subject: [PATCH 1/3] Fix invalid display of inlined re-export
---
src/librustdoc/clean/mod.rs | 19 +++++++++++++++---
src/librustdoc/visit_ast.rs | 40 ++++++++++++++++++++++++-------------
2 files changed, 42 insertions(+), 17 deletions(-)
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index 2d00c53951fe..56545983d425 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -90,6 +90,19 @@ pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext<
}
v
}));
+ items.extend(doc.inlined_foreigns.values().flat_map(|(res, renamed, local_import_id)| {
+ let Some(def_id) = res.opt_def_id() else { return Vec::new() };
+ let name = renamed.unwrap_or_else(|| cx.tcx.item_name(def_id));
+ let import = cx.tcx.hir().expect_item(*local_import_id);
+ match import.kind {
+ hir::ItemKind::Use(path, kind) => {
+ let hir::UsePath { segments, span, .. } = *path;
+ let path = hir::Path { segments, res: *res, span };
+ clean_use_statement_inner(import, name, &path, kind, cx, &mut Default::default())
+ }
+ _ => unreachable!(),
+ }
+ }));
items.extend(doc.items.values().flat_map(|(item, renamed, _)| {
// Now we actually lower the imports, skipping everything else.
if let hir::ItemKind::Use(path, hir::UseKind::Glob) = item.kind {
@@ -2652,9 +2665,6 @@ fn clean_use_statement<'tcx>(
let mut items = Vec::new();
let hir::UsePath { segments, ref res, span } = *path;
for &res in res {
- if let Res::Def(DefKind::Ctor(..), _) | Res::SelfCtor(..) = res {
- continue;
- }
let path = hir::Path { segments, res, span };
items.append(&mut clean_use_statement_inner(import, name, &path, kind, cx, inlined_names));
}
@@ -2669,6 +2679,9 @@ fn clean_use_statement_inner<'tcx>(
cx: &mut DocContext<'tcx>,
inlined_names: &mut FxHashSet<(ItemType, Symbol)>,
) -> Vec<Item> {
+ if let Res::Def(DefKind::Ctor(..), _) | Res::SelfCtor(..) = path.res {
+ return Vec::new();
+ }
// We need this comparison because some imports (for std types for example)
// are "inserted" as well but directly by the compiler and they should not be
// taken into account.
diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs
index 265123ddf6c8..cee3995eff41 100644
--- a/src/librustdoc/visit_ast.rs
+++ b/src/librustdoc/visit_ast.rs
@@ -35,6 +35,9 @@ pub(crate) struct Module<'hir> {
(LocalDefId, Option<Symbol>),
(&'hir hir::Item<'hir>, Option<Symbol>, Option<LocalDefId>),
>,
+ /// Same as for `items`.
+ pub(crate) inlined_foreigns:
+ FxIndexMap<(DefId, Option<Symbol>), (Res, Option<Symbol>, LocalDefId)>,
pub(crate) foreigns: Vec<(&'hir hir::ForeignItem<'hir>, Option<Symbol>)>,
}
@@ -54,6 +57,7 @@ impl Module<'_> {
import_id,
mods: Vec::new(),
items: FxIndexMap::default(),
+ inlined_foreigns: FxIndexMap::default(),
foreigns: Vec::new(),
}
}
@@ -272,21 +276,30 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
return false;
}
- // For cross-crate impl inlining we need to know whether items are
- // reachable in documentation -- a previously unreachable item can be
- // made reachable by cross-crate inlining which we're checking here.
- // (this is done here because we need to know this upfront).
- if !ori_res_did.is_local() && !is_no_inline {
- crate::visit_lib::lib_embargo_visit_item(self.cx, ori_res_did);
- return false;
- }
-
+ let is_hidden = !document_hidden && tcx.is_doc_hidden(ori_res_did);
let Some(res_did) = ori_res_did.as_local() else {
- return false;
+ // For cross-crate impl inlining we need to know whether items are
+ // reachable in documentation -- a previously unreachable item can be
+ // 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 {
+ return false;
+ }
+ // We store inlined foreign items otherwise, it'd mean that the `use` item would be kept
+ // around. It's not a problem unless this `use` imports both a local AND a foreign item.
+ // If a local item is inlined, its `use` is not supposed to still be around in `clean`,
+ // which would make appear the `use` in the generated documentation like the local item
+ // was not inlined even though it actually was.
+ self.modules
+ .last_mut()
+ .unwrap()
+ .inlined_foreigns
+ .insert((ori_res_did, renamed), (res, renamed, def_id));
+ return true;
};
let is_private = !self.cx.cache.effective_visibilities.is_directly_public(tcx, ori_res_did);
- let is_hidden = !document_hidden && tcx.is_doc_hidden(ori_res_did);
let item = tcx.hir().get_by_def_id(res_did);
if !please_inline {
@@ -314,7 +327,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
return false;
}
- let inlined = match tcx.hir().get_by_def_id(res_did) {
+ let inlined = match item {
// 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.
@@ -346,7 +359,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
};
self.view_item_stack.remove(&res_did);
if inlined {
- self.cx.cache.inlined_items.insert(res_did.to_def_id());
+ self.cx.cache.inlined_items.insert(ori_res_did);
}
inlined
}
@@ -483,7 +496,6 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
continue;
}
}
-
self.add_to_current_mod(item, renamed, import_id);
}
}
From c845a53aa23c1252f1cc15528799d652512b877e Mon Sep 17 00:00:00 2001
From: Guillaume Gomez <guillaume.gomez@huawei.com>
Date: Sun, 16 Jul 2023 15:54:23 +0200
Subject: [PATCH 2/3] Add regression test for #105735
---
.../issue-105735-overlapping-reexport-2.rs | 25 +++++++++++++++++++
.../issue-105735-overlapping-reexport.rs | 21 ++++++++++++++++
2 files changed, 46 insertions(+)
create mode 100644 tests/rustdoc/issue-105735-overlapping-reexport-2.rs
create mode 100644 tests/rustdoc/issue-105735-overlapping-reexport.rs
diff --git a/tests/rustdoc/issue-105735-overlapping-reexport-2.rs b/tests/rustdoc/issue-105735-overlapping-reexport-2.rs
new file mode 100644
index 000000000000..910824839489
--- /dev/null
+++ b/tests/rustdoc/issue-105735-overlapping-reexport-2.rs
@@ -0,0 +1,25 @@
+// Regression test to ensure that both `AtomicU8` items are displayed but not the re-export.
+
+#![crate_name = "foo"]
+#![no_std]
+
+// @has 'foo/index.html'
+// @has - '//*[@class="item-name"]/a[@class="type"]' 'AtomicU8'
+// @has - '//*[@class="item-name"]/a[@class="constant"]' 'AtomicU8'
+// We also ensure we don't have another item displayed.
+// @count - '//*[@id="main-content"]/*[@class="small-section-header"]' 2
+// @has - '//*[@id="main-content"]/*[@class="small-section-header"]' 'Type Definitions'
+// @has - '//*[@id="main-content"]/*[@class="small-section-header"]' 'Constants'
+
+mod other {
+ pub type AtomicU8 = ();
+}
+
+mod thing {
+ pub use crate::other::AtomicU8;
+
+ #[allow(non_upper_case_globals)]
+ pub const AtomicU8: () = ();
+}
+
+pub use crate::thing::AtomicU8;
diff --git a/tests/rustdoc/issue-105735-overlapping-reexport.rs b/tests/rustdoc/issue-105735-overlapping-reexport.rs
new file mode 100644
index 000000000000..50f2450b90a0
--- /dev/null
+++ b/tests/rustdoc/issue-105735-overlapping-reexport.rs
@@ -0,0 +1,21 @@
+// Regression test to ensure that both `AtomicU8` items are displayed but not the re-export.
+
+#![crate_name = "foo"]
+#![no_std]
+
+// @has 'foo/index.html'
+// @has - '//*[@class="item-name"]/a[@class="struct"]' 'AtomicU8'
+// @has - '//*[@class="item-name"]/a[@class="constant"]' 'AtomicU8'
+// We also ensure we don't have another item displayed.
+// @count - '//*[@id="main-content"]/*[@class="small-section-header"]' 2
+// @has - '//*[@id="main-content"]/*[@class="small-section-header"]' 'Structs'
+// @has - '//*[@id="main-content"]/*[@class="small-section-header"]' 'Constants'
+
+mod thing {
+ pub use core::sync::atomic::AtomicU8;
+
+ #[allow(non_upper_case_globals)]
+ pub const AtomicU8: () = ();
+}
+
+pub use crate::thing::AtomicU8;
From afec6d242bf308139503e4051fc4251f785b377a Mon Sep 17 00:00:00 2001
From: Guillaume Gomez <guillaume.gomez@huawei.com>
Date: Mon, 17 Jul 2023 20:33:54 +0200
Subject: [PATCH 3/3] Remove unneeded `Option<Symbol>` in `foreign_items`
---
src/librustdoc/clean/mod.rs | 2 +-
src/librustdoc/visit_ast.rs | 5 ++---
2 files changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index 56545983d425..d14953f1bb78 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -90,7 +90,7 @@ pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext<
}
v
}));
- items.extend(doc.inlined_foreigns.values().flat_map(|(res, renamed, local_import_id)| {
+ items.extend(doc.inlined_foreigns.iter().flat_map(|((_, renamed), (res, local_import_id))| {
let Some(def_id) = res.opt_def_id() else { return Vec::new() };
let name = renamed.unwrap_or_else(|| cx.tcx.item_name(def_id));
let import = cx.tcx.hir().expect_item(*local_import_id);
diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs
index cee3995eff41..66737a015215 100644
--- a/src/librustdoc/visit_ast.rs
+++ b/src/librustdoc/visit_ast.rs
@@ -36,8 +36,7 @@ pub(crate) struct Module<'hir> {
(&'hir hir::Item<'hir>, Option<Symbol>, Option<LocalDefId>),
>,
/// Same as for `items`.
- pub(crate) inlined_foreigns:
- FxIndexMap<(DefId, Option<Symbol>), (Res, Option<Symbol>, LocalDefId)>,
+ pub(crate) inlined_foreigns: FxIndexMap<(DefId, Option<Symbol>), (Res, LocalDefId)>,
pub(crate) foreigns: Vec<(&'hir hir::ForeignItem<'hir>, Option<Symbol>)>,
}
@@ -295,7 +294,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
.last_mut()
.unwrap()
.inlined_foreigns
- .insert((ori_res_did, renamed), (res, renamed, def_id));
+ .insert((ori_res_did, renamed), (res, def_id));
return true;
};