diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs index 13a9c789e8954..f54339429fa58 100644 --- a/src/librustdoc/clean/mod.rs +++ b/src/librustdoc/clean/mod.rs @@ -2776,7 +2776,12 @@ 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_) => { + // 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( item, @@ -2909,10 +2914,27 @@ fn clean_impl<'tcx>( impl_: &hir::Impl<'tcx>, def_id: LocalDefId, cx: &mut DocContext<'tcx>, + // 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. + is_inlined: bool, ) -> 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 is_inlined { + 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..5d1f4778f1c52 100644 --- a/src/librustdoc/json/conversions.rs +++ b/src/librustdoc/json/conversions.rs @@ -353,6 +353,8 @@ 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 77b3a2e9c9f2e..ac5e7805005eb 100644 --- a/src/librustdoc/passes/calculate_doc_coverage.rs +++ b/src/librustdoc/passes/calculate_doc_coverage.rs @@ -203,6 +203,10 @@ impl DocVisitor<'_> for CoverageCalculator<'_, '_> { // 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/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; diff --git a/src/librustdoc/passes/propagate_doc_cfg.rs b/src/librustdoc/passes/propagate_doc_cfg.rs index 54da158d4d39c..f73db253af062 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 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, } /// 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 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) + { + 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..bf4e842ceec3f 100644 --- a/src/librustdoc/passes/stripper.rs +++ b/src/librustdoc/passes/stripper.rs @@ -120,6 +120,10 @@ impl DocFolder for Stripper<'_, '_> { clean::ImplItem(..) => {} + // 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 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..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, @@ -426,12 +439,8 @@ 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_impl_to_current_mod(item, impl_); } return; } @@ -530,10 +539,10 @@ 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_impl_to_current_mod(item, impl_); } } } 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() {} } +}