-
Notifications
You must be signed in to change notification settings - Fork 13.4k
Fix doc cfg on reexports #101006
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
Fix doc cfg on reexports #101006
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,29 +2,53 @@ | |
use std::sync::Arc; | ||
|
||
use crate::clean::cfg::Cfg; | ||
use crate::clean::inline::{load_attrs, merge_attrs}; | ||
use crate::clean::{Crate, Item}; | ||
use crate::core::DocContext; | ||
use crate::fold::DocFolder; | ||
use crate::passes::Pass; | ||
|
||
use rustc_hir::def_id::LocalDefId; | ||
|
||
pub(crate) const PROPAGATE_DOC_CFG: Pass = Pass { | ||
name: "propagate-doc-cfg", | ||
run: propagate_doc_cfg, | ||
description: "propagates `#[doc(cfg(...))]` to child items", | ||
}; | ||
|
||
pub(crate) fn propagate_doc_cfg(cr: Crate, _: &mut DocContext<'_>) -> Crate { | ||
CfgPropagator { parent_cfg: None }.fold_crate(cr) | ||
pub(crate) fn propagate_doc_cfg(cr: Crate, cx: &mut DocContext<'_>) -> Crate { | ||
CfgPropagator { parent_cfg: None, parent: None, cx }.fold_crate(cr) | ||
} | ||
|
||
struct CfgPropagator { | ||
struct CfgPropagator<'a, 'tcx> { | ||
parent_cfg: Option<Arc<Cfg>>, | ||
parent: Option<LocalDefId>, | ||
cx: &'a mut DocContext<'tcx>, | ||
} | ||
|
||
impl DocFolder for CfgPropagator { | ||
impl<'a, 'tcx> DocFolder for CfgPropagator<'a, 'tcx> { | ||
fn fold_item(&mut self, mut item: Item) -> Option<Item> { | ||
let old_parent_cfg = self.parent_cfg.clone(); | ||
|
||
if item.kind.is_non_assoc() && | ||
let Some(def_id) = item.item_id.as_def_id().and_then(|def_id| def_id.as_local()) { | ||
let hir = self.cx.tcx.hir(); | ||
let hir_id = hir.local_def_id_to_hir_id(def_id); | ||
let expected_parent = hir.get_parent_item(hir_id); | ||
|
||
// If parents are different, it means that `item` is a reexport and we need to compute | ||
// the actual `cfg` by iterating through its "real" parents. | ||
if self.parent != Some(expected_parent) { | ||
let mut attrs = Vec::new(); | ||
for (parent_hir_id, _) in hir.parent_iter(hir_id) { | ||
let def_id = hir.local_def_id(parent_hir_id).to_def_id(); | ||
attrs.extend_from_slice(load_attrs(self.cx, def_id)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like this is going to wind up copying real cfg, and not just doc(cfg). Do we want that? The way There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We need to keep both until we call There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry I'm a bit lost: isn't it how it should be? The reexports should display the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The docs are claiming things that aren't true: This thing claims that Basically, if this PR is going to be merged, then There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't agree on that but it confirms that allowing to disable/enable doc_auto_cfg is a must have. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As long as we agree that doc_auto_cfg needs to be optional, I’m fine with merging this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this PR just highlighted it. The discussion now will be either it's enabled by default or not and what the attribute to enable/disable should look like. |
||
} | ||
let (_, cfg) = | ||
merge_attrs(self.cx, None, item.attrs.other_attrs.as_slice(), Some(&attrs)); | ||
item.cfg = cfg; | ||
} | ||
} | ||
let new_cfg = match (self.parent_cfg.take(), item.cfg.take()) { | ||
(None, None) => None, | ||
(Some(rc), None) | (None, Some(rc)) => Some(rc), | ||
|
@@ -37,8 +61,15 @@ impl DocFolder for CfgPropagator { | |
self.parent_cfg = new_cfg.clone(); | ||
item.cfg = new_cfg; | ||
|
||
let old_parent = | ||
if let Some(def_id) = item.item_id.as_def_id().and_then(|def_id| def_id.as_local()) { | ||
self.parent.replace(def_id) | ||
} else { | ||
self.parent.take() | ||
}; | ||
let result = self.fold_item_recur(item); | ||
self.parent_cfg = old_parent_cfg; | ||
self.parent = old_parent; | ||
|
||
Some(result) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
#![feature(doc_cfg)] | ||
#![feature(no_core)] | ||
|
||
#![crate_name = "foo"] | ||
#![no_core] | ||
|
||
// @has 'foo/index.html' | ||
// @has - '//*[@class="item-left module-item"]/*[@class="stab portability"]' 'foobar' | ||
// @has - '//*[@class="item-left module-item"]/*[@class="stab portability"]' 'bar' | ||
|
||
#[doc(cfg(feature = "foobar"))] | ||
mod imp_priv { | ||
// @has 'foo/struct.BarPriv.html' | ||
// @has - '//*[@id="main-content"]/*[@class="item-info"]/*[@class="stab portability"]' \ | ||
// 'Available on crate feature foobar only.' | ||
pub struct BarPriv {} | ||
impl BarPriv { | ||
pub fn test() {} | ||
} | ||
} | ||
#[doc(cfg(feature = "foobar"))] | ||
pub use crate::imp_priv::*; | ||
|
||
pub mod bar { | ||
// @has 'foo/bar/struct.Bar.html' | ||
// @has - '//*[@id="main-content"]/*[@class="item-info"]/*[@class="stab portability"]' \ | ||
// 'Available on crate feature bar only.' | ||
#[doc(cfg(feature = "bar"))] | ||
pub struct Bar; | ||
} | ||
|
||
#[doc(cfg(feature = "bar"))] | ||
pub use bar::Bar; |
Uh oh!
There was an error while loading. Please reload this page.