From 953210cfb257a6fc5d2e2b1563935a4a2de3daeb Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 16 Mar 2026 17:14:23 +0100 Subject: [PATCH 1/5] Fix `doc_cfg` not working as expected on trait impls --- src/librustdoc/clean/mod.rs | 23 +++++++++++++-- src/librustdoc/clean/types.rs | 6 +++- src/librustdoc/fold.rs | 3 +- src/librustdoc/formats/cache.rs | 2 ++ src/librustdoc/formats/item_type.rs | 2 +- src/librustdoc/json/conversions.rs | 1 + src/librustdoc/passes/propagate_doc_cfg.rs | 27 ++++++++++++++++-- src/librustdoc/passes/propagate_stability.rs | 3 +- src/librustdoc/passes/stripper.rs | 3 ++ src/librustdoc/visit.rs | 3 +- src/librustdoc/visit_ast.rs | 30 ++++++++++++++------ 11 files changed, 84 insertions(+), 19 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 13a9c789e8954..2c4f85947244a 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -2776,7 +2776,9 @@ fn clean_maybe_renamed_item<'tcx>( // These kinds of item either don't need a `name` or accept a `None` one so we handle them // before. match item.kind { - ItemKind::Impl(ref impl_) => return clean_impl(impl_, item.owner_id.def_id, cx), + ItemKind::Impl(ref impl_) => { + return clean_impl(impl_, item.owner_id.def_id, cx, renamed); + } ItemKind::Use(path, kind) => { return clean_use_statement( item, @@ -2909,10 +2911,27 @@ fn clean_impl<'tcx>( impl_: &hir::Impl<'tcx>, def_id: LocalDefId, cx: &mut DocContext<'tcx>, + // If `renamed` is some, then this is an inlined impl and it will be handled later on in the + // code. In here, we will generate a placeholder for it in order to be able to compute its + // `doc_cfg` info. + renamed: Option, ) -> Vec { let tcx = cx.tcx; let mut ret = Vec::new(); - let trait_ = impl_.of_trait.map(|t| clean_trait_ref(&t.trait_ref, cx)); + let trait_ = match impl_.of_trait { + Some(t) => { + if renamed.is_some() { + return vec![Item::from_def_id_and_parts( + def_id.to_def_id(), + None, + PlaceholderImplItem, + cx, + )]; + } + Some(clean_trait_ref(&t.trait_ref, cx)) + } + None => None, + }; let items = impl_ .items .iter() diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs index 431e9ff476f5f..ad70fc1096691 100644 --- a/src/librustdoc/clean/types.rs +++ b/src/librustdoc/clean/types.rs @@ -885,6 +885,9 @@ pub(crate) enum ItemKind { TraitItem(Box), TraitAliasItem(TraitAlias), ImplItem(Box), + /// This variant is used only as a placeholder for trait impls in order to correctly compute + /// `doc_cfg` as trait impls are added to `clean::Crate` after we went through the whole tree. + PlaceholderImplItem, /// A required method in a trait declaration meaning it's only a function signature. RequiredMethodItem(Box, Defaultness), /// A method in a trait impl or a provided method in a trait declaration. @@ -964,7 +967,8 @@ impl ItemKind { | AssocTypeItem(..) | StrippedItem(_) | KeywordItem - | AttributeItem => [].iter(), + | AttributeItem + | PlaceholderImplItem => [].iter(), } } } diff --git a/src/librustdoc/fold.rs b/src/librustdoc/fold.rs index c970fdbbc93af..8b9db4638e473 100644 --- a/src/librustdoc/fold.rs +++ b/src/librustdoc/fold.rs @@ -97,7 +97,8 @@ pub(crate) trait DocFolder: Sized { | RequiredAssocTypeItem(..) | AssocTypeItem(..) | KeywordItem - | AttributeItem => kind, + | AttributeItem + | PlaceholderImplItem => kind, } } diff --git a/src/librustdoc/formats/cache.rs b/src/librustdoc/formats/cache.rs index 5a97d8e5b5f42..35071f47a182b 100644 --- a/src/librustdoc/formats/cache.rs +++ b/src/librustdoc/formats/cache.rs @@ -389,6 +389,8 @@ impl DocFolder for CacheBuilder<'_, '_> { // So would rather leave them to an expert, // as at least the list is better than `_ => {}`. } + + clean::PlaceholderImplItem => return None, } // Maintain the parent stack. diff --git a/src/librustdoc/formats/item_type.rs b/src/librustdoc/formats/item_type.rs index 6830c1ec6560d..eb3492e4625be 100644 --- a/src/librustdoc/formats/item_type.rs +++ b/src/librustdoc/formats/item_type.rs @@ -122,7 +122,7 @@ impl<'a> From<&'a clean::Item> for ItemType { clean::StaticItem(..) => ItemType::Static, clean::ConstantItem(..) => ItemType::Constant, clean::TraitItem(..) => ItemType::Trait, - clean::ImplItem(..) => ItemType::Impl, + clean::ImplItem(..) | clean::PlaceholderImplItem => ItemType::Impl, clean::RequiredMethodItem(..) => ItemType::TyMethod, clean::MethodItem(..) => ItemType::Method, clean::StructFieldItem(..) => ItemType::StructField, diff --git a/src/librustdoc/json/conversions.rs b/src/librustdoc/json/conversions.rs index bc9ad1606b8a4..740fccca9e886 100644 --- a/src/librustdoc/json/conversions.rs +++ b/src/librustdoc/json/conversions.rs @@ -353,6 +353,7 @@ fn from_clean_item(item: &clean::Item, renderer: &JsonRenderer<'_>) -> ItemEnum name: name.as_ref().unwrap().to_string(), rename: src.map(|x| x.to_string()), }, + PlaceholderImplItem => unreachable!(), } } diff --git a/src/librustdoc/passes/propagate_doc_cfg.rs b/src/librustdoc/passes/propagate_doc_cfg.rs index 54da158d4d39c..1430feae6c45a 100644 --- a/src/librustdoc/passes/propagate_doc_cfg.rs +++ b/src/librustdoc/passes/propagate_doc_cfg.rs @@ -1,10 +1,11 @@ //! Propagates [`#[doc(cfg(...))]`](https://github.com/rust-lang/rust/issues/43781) to child items. +use rustc_data_structures::fx::FxHashMap; use rustc_hir::Attribute; use rustc_hir::attrs::{AttributeKind, DocAttribute}; use crate::clean::inline::{load_attrs, merge_attrs}; -use crate::clean::{CfgInfo, Crate, Item, ItemKind}; +use crate::clean::{CfgInfo, Crate, Item, ItemId, ItemKind}; use crate::core::DocContext; use crate::fold::DocFolder; use crate::passes::Pass; @@ -17,7 +18,8 @@ pub(crate) const PROPAGATE_DOC_CFG: Pass = Pass { pub(crate) fn propagate_doc_cfg(cr: Crate, cx: &mut DocContext<'_>) -> Crate { if cx.tcx.features().doc_cfg() { - CfgPropagator { cx, cfg_info: CfgInfo::default() }.fold_crate(cr) + CfgPropagator { cx, cfg_info: CfgInfo::default(), impl_cfg_info: FxHashMap::default() } + .fold_crate(cr) } else { cr } @@ -26,6 +28,10 @@ pub(crate) fn propagate_doc_cfg(cr: Crate, cx: &mut DocContext<'_>) -> Crate { struct CfgPropagator<'a, 'tcx> { cx: &'a mut DocContext<'tcx>, cfg_info: CfgInfo, + + /// To ensure the `doc_cfg` feature works with how `rustdoc` handles impl, we need to store + /// the `cfg` info of `impl`s placeholder to use them later on the "real" impl item. + impl_cfg_info: FxHashMap, } /// This function goes through the attributes list (`new_attrs`) and extract the `cfg` tokens from @@ -78,7 +84,22 @@ impl DocFolder for CfgPropagator<'_, '_> { fn fold_item(&mut self, mut item: Item) -> Option { let old_cfg_info = self.cfg_info.clone(); - self.merge_with_parent_attributes(&mut item); + // If we have an impl, we check if it has an associated `cfg` "context", and if so we will + // use this context instead of the actual (wrong) one. + if let ItemKind::ImplItem(_) = item.kind + && let Some(cfg_info) = self.impl_cfg_info.remove(&item.item_id) + { + self.cfg_info = cfg_info; + } + + if let ItemKind::PlaceholderImplItem = item.kind { + // If we have a placeholder impl, we store the current `cfg` "context" to be used + // on the actual impl later on (the impls are generated after we go through the whole + // AST so they're stored in the `krate` object at the end). + self.impl_cfg_info.insert(item.item_id, self.cfg_info.clone()); + } else { + self.merge_with_parent_attributes(&mut item); + } let result = self.fold_item_recur(item); self.cfg_info = old_cfg_info; diff --git a/src/librustdoc/passes/propagate_stability.rs b/src/librustdoc/passes/propagate_stability.rs index 5139ca301dd3d..c8691fd012bf6 100644 --- a/src/librustdoc/passes/propagate_stability.rs +++ b/src/librustdoc/passes/propagate_stability.rs @@ -107,7 +107,8 @@ impl DocFolder for StabilityPropagator<'_, '_> { | ItemKind::AssocTypeItem(..) | ItemKind::PrimitiveItem(..) | ItemKind::KeywordItem - | ItemKind::AttributeItem => own_stability, + | ItemKind::AttributeItem + | ItemKind::PlaceholderImplItem => own_stability, ItemKind::StrippedItem(..) => unreachable!(), } diff --git a/src/librustdoc/passes/stripper.rs b/src/librustdoc/passes/stripper.rs index 99d22526f85b7..e348c6b5fc3fc 100644 --- a/src/librustdoc/passes/stripper.rs +++ b/src/librustdoc/passes/stripper.rs @@ -120,6 +120,9 @@ impl DocFolder for Stripper<'_, '_> { clean::ImplItem(..) => {} + // They have no use anymore, so always remove them. + clean::PlaceholderImplItem => return None, + // tymethods etc. have no control over privacy clean::RequiredMethodItem(..) | clean::RequiredAssocConstItem(..) diff --git a/src/librustdoc/visit.rs b/src/librustdoc/visit.rs index 9f6bf009a0540..9cf7d6b29b101 100644 --- a/src/librustdoc/visit.rs +++ b/src/librustdoc/visit.rs @@ -50,7 +50,8 @@ pub(crate) trait DocVisitor<'a>: Sized { | RequiredAssocTypeItem(..) | AssocTypeItem(..) | KeywordItem - | AttributeItem => {} + | AttributeItem + | PlaceholderImplItem => {} } } diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index fd6ea21847c19..0ec14e80b2153 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -426,12 +426,16 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { // } // Bar::bar(); // ``` - if let hir::ItemKind::Impl(impl_) = item.kind && - // Don't duplicate impls when inlining or if it's implementing a trait, we'll pick - // them up regardless of where they're located. - impl_.of_trait.is_none() - { - self.add_to_current_mod(item, None, None); + if let hir::ItemKind::Impl(impl_) = item.kind { + self.add_to_current_mod( + item, + if impl_.of_trait.is_none() { + None + } else { + Some(rustc_span::symbol::kw::Impl) + }, + None, + ); } return; } @@ -530,10 +534,18 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { } } hir::ItemKind::Impl(impl_) => { - // Don't duplicate impls when inlining or if it's implementing a trait, we'll pick + // Don't duplicate impls when inlining, we'll pick // them up regardless of where they're located. - if !self.inlining && impl_.of_trait.is_none() { - self.add_to_current_mod(item, None, None); + if !self.inlining { + self.add_to_current_mod( + item, + if impl_.of_trait.is_none() { + None + } else { + Some(rustc_span::symbol::kw::Impl) + }, + None, + ); } } } From 73fe4c5a3bad04459d53e4364f9facb7583fa63d Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 16 Mar 2026 17:14:39 +0100 Subject: [PATCH 2/5] Add regression test for #153655 --- .../doc-cfg/trait-impls-manual.rs | 92 +++++++++++++++++++ tests/rustdoc-html/doc-cfg/trait-impls.rs | 92 +++++++++++++++++++ 2 files changed, 184 insertions(+) create mode 100644 tests/rustdoc-html/doc-cfg/trait-impls-manual.rs create mode 100644 tests/rustdoc-html/doc-cfg/trait-impls.rs diff --git a/tests/rustdoc-html/doc-cfg/trait-impls-manual.rs b/tests/rustdoc-html/doc-cfg/trait-impls-manual.rs new file mode 100644 index 0000000000000..fbd96cc46d800 --- /dev/null +++ b/tests/rustdoc-html/doc-cfg/trait-impls-manual.rs @@ -0,0 +1,92 @@ +// This test ensures that `doc_cfg` feature is working as expected on trait impls. +// Regression test for . + +#![feature(doc_cfg)] +#![doc(auto_cfg(hide( + target_pointer_width = "64", +)))] + +#![crate_name = "foo"] + +pub trait Trait { + fn f(&self) {} +} + +pub trait Bob { + fn bob(&self) {} +} + +pub trait Foo { + fn foo(&self) {} +} + +pub struct X; + +//@has 'foo/struct.X.html' +//@count - '//*[@id="impl-Bob-for-X"]' 1 +//@count - '//*[@id="impl-Bob-for-X"]/*[@class="item-info"]' 0 +//@count - '//*[@id="impl-Trait-for-X"]' 1 +//@count - '//*[@id="impl-Trait-for-X"]/*[@class="item-info"]' 0 + +// If you need to update this XPath, in particular `item-info`, update all +// the others in this file. +//@count - '//*[@id="impl-Foo-for-X"]/*[@class="item-info"]' 1 + +//@has 'foo/trait.Trait.html' +//@count - '//*[@id="impl-Trait-for-X"]' 1 +//@count - '//*[@id="impl-Trait-for-X"]/*[@class="item-info"]' 0 +#[doc(cfg(any(target_pointer_width = "64", target_arch = "wasm32")))] +#[doc(auto_cfg(hide(target_arch = "wasm32")))] +mod imp { + impl super::Trait for super::X { fn f(&self) {} } +} + +//@has 'foo/trait.Bob.html' +//@count - '//*[@id="impl-Bob-for-X"]' 1 +//@count - '//*[@id="impl-Bob-for-X"]/*[@class="item-info"]' 0 +#[doc(cfg(any(target_pointer_width = "64", target_arch = "wasm32")))] +#[doc(auto_cfg = false)] +mod imp2 { + impl super::Bob for super::X { fn bob(&self) {} } +} + +//@has 'foo/trait.Foo.html' +//@count - '//*[@id="impl-Foo-for-X"]/*[@class="item-info"]' 1 +// We use this to force xpath tests to be updated if `item-info` class is changed. +#[doc(cfg(any(target_pointer_width = "64", target_arch = "wasm32")))] +mod imp3 { + impl super::Foo for super::X { fn foo(&self) {} } +} + +pub struct Y; + +//@has 'foo/struct.Y.html' +//@count - '//*[@id="implementations-list"]/*[@class="impl-items"]' 1 +//@count - '//*[@id="implementations-list"]/*[@class="impl-items"]/*[@class="item-info"]' 0 +#[doc(cfg(any(target_pointer_width = "64", target_arch = "wasm32")))] +#[doc(auto_cfg(hide(target_arch = "wasm32")))] +mod imp4 { + impl super::Y { pub fn plain_auto() {} } +} + +pub struct Z; + +//@has 'foo/struct.Z.html' +//@count - '//*[@id="implementations-list"]/*[@class="impl-items"]' 1 +//@count - '//*[@id="implementations-list"]/*[@class="impl-items"]/*[@class="item-info"]' 0 +#[doc(cfg(any(target_pointer_width = "64", target_arch = "wasm32")))] +#[doc(auto_cfg = false)] +mod imp5 { + impl super::Z { pub fn plain_auto() {} } +} + +// The "witness" which has the item info. +pub struct W; + +//@has 'foo/struct.W.html' +//@count - '//*[@id="implementations-list"]/*[@class="impl-items"]' 1 +//@count - '//*[@id="implementations-list"]/*[@class="impl-items"]/*[@class="item-info"]' 1 +#[doc(cfg(any(target_pointer_width = "64", target_arch = "wasm32")))] +mod imp6 { + impl super::W { pub fn plain_auto() {} } +} diff --git a/tests/rustdoc-html/doc-cfg/trait-impls.rs b/tests/rustdoc-html/doc-cfg/trait-impls.rs new file mode 100644 index 0000000000000..581d171123d00 --- /dev/null +++ b/tests/rustdoc-html/doc-cfg/trait-impls.rs @@ -0,0 +1,92 @@ +// This test ensures that `doc_cfg` feature is working as expected on trait impls. +// Regression test for . + +#![feature(doc_cfg)] +#![doc(auto_cfg(hide( + target_pointer_width = "64", +)))] + +#![crate_name = "foo"] + +pub trait Trait { + fn f(&self) {} +} + +pub trait Bob { + fn bob(&self) {} +} + +pub trait Foo { + fn foo(&self) {} +} + +pub struct X; + +//@has 'foo/struct.X.html' +//@count - '//*[@id="impl-Bob-for-X"]' 1 +//@count - '//*[@id="impl-Bob-for-X"]/*[@class="item-info"]' 0 +//@count - '//*[@id="impl-Trait-for-X"]' 1 +//@count - '//*[@id="impl-Trait-for-X"]/*[@class="item-info"]' 0 + +// If you need to update this XPath, in particular `item-info`, update all +// the others in this file. +//@count - '//*[@id="impl-Foo-for-X"]/*[@class="item-info"]' 1 + +//@has 'foo/trait.Trait.html' +//@count - '//*[@id="impl-Trait-for-X"]' 1 +//@count - '//*[@id="impl-Trait-for-X"]/*[@class="item-info"]' 0 +#[cfg(any(target_pointer_width = "64", target_arch = "wasm32"))] +#[doc(auto_cfg(hide(target_arch = "wasm32")))] +mod imp { + impl super::Trait for super::X { fn f(&self) {} } +} + +//@has 'foo/trait.Bob.html' +//@count - '//*[@id="impl-Bob-for-X"]' 1 +//@count - '//*[@id="impl-Bob-for-X"]/*[@class="item-info"]' 0 +#[cfg(any(target_pointer_width = "64", target_arch = "wasm32"))] +#[doc(auto_cfg = false)] +mod imp2 { + impl super::Bob for super::X { fn bob(&self) {} } +} + +//@has 'foo/trait.Foo.html' +//@count - '//*[@id="impl-Foo-for-X"]/*[@class="item-info"]' 1 +// We use this to force xpath tests to be updated if `item-info` class is changed. +#[cfg(any(target_pointer_width = "64", target_arch = "wasm32"))] +mod imp3 { + impl super::Foo for super::X { fn foo(&self) {} } +} + +pub struct Y; + +//@has 'foo/struct.Y.html' +//@count - '//*[@id="implementations-list"]/*[@class="impl-items"]' 1 +//@count - '//*[@id="implementations-list"]/*[@class="impl-items"]/*[@class="item-info"]' 0 +#[cfg(any(target_pointer_width = "64", target_arch = "wasm32"))] +#[doc(auto_cfg(hide(target_arch = "wasm32")))] +mod imp4 { + impl super::Y { pub fn plain_auto() {} } +} + +pub struct Z; + +//@has 'foo/struct.Z.html' +//@count - '//*[@id="implementations-list"]/*[@class="impl-items"]' 1 +//@count - '//*[@id="implementations-list"]/*[@class="impl-items"]/*[@class="item-info"]' 0 +#[cfg(any(target_pointer_width = "64", target_arch = "wasm32"))] +#[doc(auto_cfg = false)] +mod imp5 { + impl super::Z { pub fn plain_auto() {} } +} + +// The "witness" which has the item info. +pub struct W; + +//@has 'foo/struct.W.html' +//@count - '//*[@id="implementations-list"]/*[@class="impl-items"]' 1 +//@count - '//*[@id="implementations-list"]/*[@class="impl-items"]/*[@class="item-info"]' 1 +#[cfg(any(target_pointer_width = "64", target_arch = "wasm32"))] +mod imp6 { + impl super::W { pub fn plain_auto() {} } +} From 6fcd01151ecb79a00b1f5fb59b791aa0f5cb1be1 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 16 Mar 2026 20:15:57 +0100 Subject: [PATCH 3/5] Correctly handle rustdoc `PlaceholderImplItem` in lints --- src/librustdoc/passes/calculate_doc_coverage.rs | 2 +- src/librustdoc/passes/check_doc_test_visibility.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index 77b3a2e9c9f2e..fcaf665a1fda8 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -199,7 +199,7 @@ impl DocVisitor<'_> for CoverageCalculator<'_, '_> { } match i.kind { - clean::StrippedItem(..) => { + clean::StrippedItem(..) | clean::PlaceholderImplItem => { // don't count items in stripped modules return; } diff --git a/src/librustdoc/passes/check_doc_test_visibility.rs b/src/librustdoc/passes/check_doc_test_visibility.rs index a1578ab40209a..ff7535fea41cb 100644 --- a/src/librustdoc/passes/check_doc_test_visibility.rs +++ b/src/librustdoc/passes/check_doc_test_visibility.rs @@ -80,6 +80,7 @@ pub(crate) fn should_have_doc_example(cx: &DocContext<'_>, item: &clean::Item) - | clean::ImplAssocConstItem(..) | clean::RequiredAssocTypeItem(..) | clean::ImplItem(_) + | clean::PlaceholderImplItem ) { return false; From 24e40f002cff39fe916a534c960067859c75515f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 17 Mar 2026 11:32:53 +0100 Subject: [PATCH 4/5] Add more comments to explain code --- src/librustdoc/clean/mod.rs | 5 ++++- src/librustdoc/json/conversions.rs | 1 + src/librustdoc/passes/calculate_doc_coverage.rs | 6 +++++- src/librustdoc/passes/stripper.rs | 3 ++- src/librustdoc/visit_ast.rs | 8 ++++++++ 5 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 2c4f85947244a..47da69110f097 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -2777,6 +2777,9 @@ fn clean_maybe_renamed_item<'tcx>( // before. match item.kind { ItemKind::Impl(ref impl_) => { + // `renamed` is passed down to `clean_impl` because we use it as a marker to + // indicate that this is an inlined impl and that we should generate an impl + // placeholder and not a "real" impl item. return clean_impl(impl_, item.owner_id.def_id, cx, renamed); } ItemKind::Use(path, kind) => { @@ -2911,7 +2914,7 @@ fn clean_impl<'tcx>( impl_: &hir::Impl<'tcx>, def_id: LocalDefId, cx: &mut DocContext<'tcx>, - // If `renamed` is some, then this is an inlined impl and it will be handled later on in the + // If `renamed` is some, then `impl_` is an inlined impl and it will be handled later on in the // code. In here, we will generate a placeholder for it in order to be able to compute its // `doc_cfg` info. renamed: Option, diff --git a/src/librustdoc/json/conversions.rs b/src/librustdoc/json/conversions.rs index 740fccca9e886..5d1f4778f1c52 100644 --- a/src/librustdoc/json/conversions.rs +++ b/src/librustdoc/json/conversions.rs @@ -353,6 +353,7 @@ fn from_clean_item(item: &clean::Item, renderer: &JsonRenderer<'_>) -> ItemEnum name: name.as_ref().unwrap().to_string(), rename: src.map(|x| x.to_string()), }, + // All placeholder impl items should have been removed in the stripper passes. PlaceholderImplItem => unreachable!(), } } diff --git a/src/librustdoc/passes/calculate_doc_coverage.rs b/src/librustdoc/passes/calculate_doc_coverage.rs index fcaf665a1fda8..ac5e7805005eb 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -199,10 +199,14 @@ impl DocVisitor<'_> for CoverageCalculator<'_, '_> { } match i.kind { - clean::StrippedItem(..) | clean::PlaceholderImplItem => { + clean::StrippedItem(..) => { // don't count items in stripped modules return; } + clean::PlaceholderImplItem => { + // The "real" impl items are handled below. + return; + } // docs on `use` and `extern crate` statements are not displayed, so they're not // worth counting clean::ImportItem(..) | clean::ExternCrateItem { .. } => {} diff --git a/src/librustdoc/passes/stripper.rs b/src/librustdoc/passes/stripper.rs index e348c6b5fc3fc..bf4e842ceec3f 100644 --- a/src/librustdoc/passes/stripper.rs +++ b/src/librustdoc/passes/stripper.rs @@ -120,7 +120,8 @@ impl DocFolder for Stripper<'_, '_> { clean::ImplItem(..) => {} - // They have no use anymore, so always remove them. + // Since the `doc_cfg` propagation was handled before the current pass, we can (and + // should) remove all placeholder impl items. clean::PlaceholderImplItem => return None, // tymethods etc. have no control over privacy diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 0ec14e80b2153..8425ed2d63c1c 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -429,6 +429,10 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { if let hir::ItemKind::Impl(impl_) = item.kind { self.add_to_current_mod( item, + // The symbol here is used as a "sentinel" value and has no meaning in + // itself. It just tells that this is an inlined impl and that it should not + // be cleaned as a normal `ImplItem` but instead as a `PlaceholderImplItem`. + // It's to ensure that `doc_cfg` inheritance works as expected. if impl_.of_trait.is_none() { None } else { @@ -539,6 +543,10 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { if !self.inlining { self.add_to_current_mod( item, + // The symbol here is used as a "sentinel" value and has no meaning in + // itself. It just tells that this is an inlined impl and that it should not + // be cleaned as a normal `ImplItem` but instead as a `PlaceholderImplItem`. + // It's to ensure that `doc_cfg` inheritance works as expected. if impl_.of_trait.is_none() { None } else { From 19876d196fb27db7d8208ba7e84d51007ef28524 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Mon, 23 Mar 2026 21:47:29 +0100 Subject: [PATCH 5/5] Simplify code and improve code comments --- src/librustdoc/clean/mod.rs | 16 ++++----- src/librustdoc/passes/propagate_doc_cfg.rs | 4 +-- src/librustdoc/visit_ast.rs | 41 ++++++++-------------- 3 files changed, 25 insertions(+), 36 deletions(-) diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 47da69110f097..f54339429fa58 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -2777,10 +2777,10 @@ fn clean_maybe_renamed_item<'tcx>( // before. match item.kind { ItemKind::Impl(ref impl_) => { - // `renamed` is passed down to `clean_impl` because we use it as a marker to - // indicate that this is an inlined impl and that we should generate an impl - // placeholder and not a "real" impl item. - return clean_impl(impl_, item.owner_id.def_id, cx, renamed); + // If `renamed` is `Some()` for an `impl`, it means it's been inlined because we use + // it as a marker to indicate that this is an inlined impl and that we should + // generate an impl placeholder and not a "real" impl item. + return clean_impl(impl_, item.owner_id.def_id, cx, renamed.is_some()); } ItemKind::Use(path, kind) => { return clean_use_statement( @@ -2914,16 +2914,16 @@ fn clean_impl<'tcx>( impl_: &hir::Impl<'tcx>, def_id: LocalDefId, cx: &mut DocContext<'tcx>, - // If `renamed` is some, then `impl_` is an inlined impl and it will be handled later on in the - // code. In here, we will generate a placeholder for it in order to be able to compute its + // If true, this is an inlined impl and it will be handled later on in the code. + // In here, we will generate a placeholder for it in order to be able to compute its // `doc_cfg` info. - renamed: Option, + is_inlined: bool, ) -> Vec { let tcx = cx.tcx; let mut ret = Vec::new(); let trait_ = match impl_.of_trait { Some(t) => { - if renamed.is_some() { + if is_inlined { return vec![Item::from_def_id_and_parts( def_id.to_def_id(), None, diff --git a/src/librustdoc/passes/propagate_doc_cfg.rs b/src/librustdoc/passes/propagate_doc_cfg.rs index 1430feae6c45a..f73db253af062 100644 --- a/src/librustdoc/passes/propagate_doc_cfg.rs +++ b/src/librustdoc/passes/propagate_doc_cfg.rs @@ -29,7 +29,7 @@ struct CfgPropagator<'a, 'tcx> { cx: &'a mut DocContext<'tcx>, cfg_info: CfgInfo, - /// To ensure the `doc_cfg` feature works with how `rustdoc` handles impl, we need to store + /// To ensure the `doc_cfg` feature works with how `rustdoc` handles impls, we need to store /// the `cfg` info of `impl`s placeholder to use them later on the "real" impl item. impl_cfg_info: FxHashMap, } @@ -85,7 +85,7 @@ impl DocFolder for CfgPropagator<'_, '_> { let old_cfg_info = self.cfg_info.clone(); // If we have an impl, we check if it has an associated `cfg` "context", and if so we will - // use this context instead of the actual (wrong) one. + // use that context instead of the actual (wrong) one. if let ItemKind::ImplItem(_) = item.kind && let Some(cfg_info) = self.impl_cfg_info.remove(&item.item_id) { diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs index 8425ed2d63c1c..906289ba755e3 100644 --- a/src/librustdoc/visit_ast.rs +++ b/src/librustdoc/visit_ast.rs @@ -372,6 +372,19 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { && !inherits_doc_hidden(tcx, item_def_id, None) } + #[inline] + fn add_impl_to_current_mod(&mut self, item: &'tcx hir::Item<'_>, impl_: hir::Impl<'_>) { + self.add_to_current_mod( + item, + // The symbol here is used as a "sentinel" value and has no meaning in + // itself. It just tells that this is an inlined impl and that it should not + // be cleaned as a normal `ImplItem` but instead as a `PlaceholderImplItem`. + // It's to ensure that `doc_cfg` inheritance works as expected. + if impl_.of_trait.is_none() { None } else { Some(rustc_span::symbol::kw::Impl) }, + None, + ); + } + #[inline] fn add_to_current_mod( &mut self, @@ -427,19 +440,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { // Bar::bar(); // ``` if let hir::ItemKind::Impl(impl_) = item.kind { - self.add_to_current_mod( - item, - // The symbol here is used as a "sentinel" value and has no meaning in - // itself. It just tells that this is an inlined impl and that it should not - // be cleaned as a normal `ImplItem` but instead as a `PlaceholderImplItem`. - // It's to ensure that `doc_cfg` inheritance works as expected. - if impl_.of_trait.is_none() { - None - } else { - Some(rustc_span::symbol::kw::Impl) - }, - None, - ); + self.add_impl_to_current_mod(item, impl_); } return; } @@ -541,19 +542,7 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> { // Don't duplicate impls when inlining, we'll pick // them up regardless of where they're located. if !self.inlining { - self.add_to_current_mod( - item, - // The symbol here is used as a "sentinel" value and has no meaning in - // itself. It just tells that this is an inlined impl and that it should not - // be cleaned as a normal `ImplItem` but instead as a `PlaceholderImplItem`. - // It's to ensure that `doc_cfg` inheritance works as expected. - if impl_.of_trait.is_none() { - None - } else { - Some(rustc_span::symbol::kw::Impl) - }, - None, - ); + self.add_impl_to_current_mod(item, impl_); } } }