rustdoc: preserve parent doc cfg for macro_export macros#155954
rustdoc: preserve parent doc cfg for macro_export macros#155954cijiugechu wants to merge 1 commit intorust-lang:mainfrom
macro_export macros#155954Conversation
|
r? @notriddle rustbot has assigned @notriddle. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
|
||
| /// This function goes through the attributes list (`new_attrs`) and extracts the attributes that | ||
| /// affect the cfg state propagated to detached items. | ||
| fn add_cfg_state_attributes(attrs: &mut Vec<Attribute>, new_attrs: &[Attribute]) { |
There was a problem hiding this comment.
I kept them(this function and add_only_cfg_attributes) separate because a shared helper would make the logic harder to follow.
macro_export macros
|
Please don't plumb a new attribute through When I change the code to call diff --git a/src/librustdoc/clean/auto_trait.rs b/src/librustdoc/clean/auto_trait.rs
index f0371f8482d..554b81b14cd 100644
--- a/src/librustdoc/clean/auto_trait.rs
+++ b/src/librustdoc/clean/auto_trait.rs
@@ -131,7 +131,6 @@ fn synthesize_auto_trait_impl<'tcx>(
item_id: clean::ItemId::Auto { trait_: trait_def_id, for_: item_def_id },
cfg: None,
inline_stmt_id: None,
- cfg_parent_ids: Default::default(),
}),
})
}
diff --git a/src/librustdoc/clean/blanket_impl.rs b/src/librustdoc/clean/blanket_impl.rs
index 6978d03bcf8..8dda831cac8 100644
--- a/src/librustdoc/clean/blanket_impl.rs
+++ b/src/librustdoc/clean/blanket_impl.rs
@@ -126,7 +126,6 @@ pub(crate) fn synthesize_blanket_impls(
})),
cfg: None,
inline_stmt_id: None,
- cfg_parent_ids: Default::default(),
}),
});
}
diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs
index 112516a564c..3ca22b0360b 100644
--- a/src/librustdoc/clean/inline.rs
+++ b/src/librustdoc/clean/inline.rs
@@ -725,7 +725,6 @@ fn build_module_items(
)),
cfg: None,
inline_stmt_id: None,
- cfg_parent_ids: Default::default(),
}),
});
} else if let Some(i) = try_inline(cx, res, item.ident.name, attrs, visited) {
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index 9b21ebb645c..4b52ce0e36c 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -101,10 +101,7 @@ pub(crate) fn clean_doc_module<'tcx>(doc: &DocModule<'tcx>, cx: &mut DocContext<
if matches!(item.kind, hir::ItemKind::Use(_, hir::UseKind::Glob)) {
return Vec::new();
}
- let mut v = clean_maybe_renamed_item(cx, item, entry.renamed, &entry.import_ids);
- for item in &mut v {
- item.inner.cfg_parent_ids = entry.cfg_parent_ids.iter().copied().collect();
- }
+ let v = clean_maybe_renamed_item(cx, item, entry.renamed, &entry.import_ids);
for item in &v {
if let Some(name) = item.name
&& (cx.document_hidden() || !item.is_doc_hidden())
diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
index cfb1ce268f0..e2db9132320 100644
--- a/src/librustdoc/clean/types.rs
+++ b/src/librustdoc/clean/types.rs
@@ -25,6 +25,7 @@
DocFragment, add_doc_fragment, attrs_to_doc_fragments, inner_docs, span_of_fragments,
};
use rustc_session::Session;
+use rustc_span::def_id::CRATE_DEF_ID;
use rustc_span::hygiene::MacroKind;
use rustc_span::symbol::{Symbol, kw, sym};
use rustc_span::{DUMMY_SP, FileName, Ident, Loc, RemapPathScopeComponents};
@@ -359,9 +360,6 @@ pub(crate) struct ItemInner {
/// The crate metadata doesn't hold this information, so the `use` statement
/// always belongs to the current crate.
pub(crate) inline_stmt_id: Option<LocalDefId>,
- /// Lexical parents whose cfg/doc(cfg) information applies even though the item is documented
- /// elsewhere, such as `#[macro_export] macro_rules!` items rendered at the crate root.
- pub(crate) cfg_parent_ids: ThinVec<LocalDefId>,
pub(crate) cfg: Option<Arc<Cfg>>,
}
@@ -408,6 +406,23 @@ fn is_field_vis_inherited(tcx: TyCtxt<'_>, def_id: DefId) -> bool {
}
impl Item {
+ pub(crate) fn cfg_parent_ids_for_detached_item(&self, tcx: TyCtxt<'_>) -> Vec<LocalDefId> {
+ let Some(def_id) = self.inline_stmt_id.or(self.item_id.as_local_def_id()) else {
+ return Vec::new();
+ };
+ let mut ids = Vec::new();
+ let mut next = def_id;
+ while let Some(parent) = tcx.opt_local_parent(next) {
+ if parent == CRATE_DEF_ID {
+ break;
+ }
+ ids.push(parent);
+ next = parent;
+ }
+ ids.reverse();
+ ids
+ }
+
/// Returns the effective stability of the item.
///
/// This method should only be called after the `propagate-stability` pass has been run.
@@ -560,7 +575,6 @@ pub(crate) fn from_def_id_and_attrs_and_parts(
name,
cfg,
inline_stmt_id: None,
- cfg_parent_ids: ThinVec::new(),
}),
}
}
@@ -2449,7 +2463,7 @@ mod size_asserts {
static_assert_size!(GenericParamDef, 40);
static_assert_size!(Generics, 16);
static_assert_size!(Item, 8);
- static_assert_size!(ItemInner, 144);
+ static_assert_size!(ItemInner, 136);
static_assert_size!(ItemKind, 48);
static_assert_size!(PathSegment, 32);
static_assert_size!(Type, 32);
diff --git a/src/librustdoc/passes/propagate_doc_cfg.rs b/src/librustdoc/passes/propagate_doc_cfg.rs
index 4cd6f6c240f..1334595272f 100644
--- a/src/librustdoc/passes/propagate_doc_cfg.rs
+++ b/src/librustdoc/passes/propagate_doc_cfg.rs
@@ -75,7 +75,7 @@ impl CfgPropagator<'_, '_> {
// (mostly `cfg` ones) will be missing.
fn merge_with_parent_attributes(&mut self, item: &mut Item) {
let mut attrs = Vec::new();
- // We only need to merge an item attributes with its parent's in case it's an impl as an
+ // We need to merge an item attributes with its parent's in case it's an impl as an
// impl might not be defined in the same module as the item it implements.
//
// Otherwise, `cfg_info` already tracks everything we need so nothing else to do!
@@ -88,13 +88,26 @@ fn merge_with_parent_attributes(&mut self, item: &mut Item) {
next_def_id = parent_def_id;
}
}
- for parent_def_id in &item.cfg_parent_ids {
- let mut parent_attrs = Vec::new();
- add_cfg_state_attributes(
- &mut parent_attrs,
- load_attrs(self.cx.tcx, parent_def_id.to_def_id()),
- );
- merge_attrs(self.cx.tcx, &[], Some((&parent_attrs, None)), &mut self.cfg_info);
+ // We also need to merge an item attributes with its parent's in case it's a macro with
+ // the `#[macro_export]` attribute, because it might not be defined at crate root.
+ if matches!(item.kind, ItemKind::MacroItem(_))
+ && item.inner.attrs.other_attrs.iter().any(|attr| {
+ matches!(
+ attr,
+ rustc_hir::Attribute::Parsed(
+ rustc_hir::attrs::AttributeKind::MacroExport { .. }
+ )
+ )
+ })
+ {
+ for parent_def_id in &item.cfg_parent_ids_for_detached_item(self.cx.tcx) {
+ let mut parent_attrs = Vec::new();
+ add_cfg_state_attributes(
+ &mut parent_attrs,
+ load_attrs(self.cx.tcx, parent_def_id.to_def_id()),
+ );
+ merge_attrs(self.cx.tcx, &[], Some((&parent_attrs, None)), &mut self.cfg_info);
+ }
}
let (_, cfg) = merge_attrs(
diff --git a/src/librustdoc/visit_ast.rs b/src/librustdoc/visit_ast.rs
index 7a6a9bdd54a..5f119c23841 100644
--- a/src/librustdoc/visit_ast.rs
+++ b/src/librustdoc/visit_ast.rs
@@ -72,7 +72,6 @@ pub(crate) struct ItemEntry<'hir> {
pub(crate) item: &'hir hir::Item<'hir>,
pub(crate) renamed: Option<Symbol>,
pub(crate) import_ids: Vec<LocalDefId>,
- pub(crate) cfg_parent_ids: Vec<LocalDefId>,
}
impl Module<'_> {
@@ -150,21 +149,6 @@ fn store_path(&mut self, did: DefId) {
self.exact_paths.entry(did).or_insert_with(|| def_id_to_path(tcx, did));
}
- fn cfg_parent_ids_for_detached_item(&self, def_id: LocalDefId) -> Vec<LocalDefId> {
- let tcx = self.cx.tcx;
- let mut ids = Vec::new();
- let mut next = def_id;
- while let Some(parent) = tcx.opt_local_parent(next) {
- if parent == CRATE_DEF_ID {
- break;
- }
- ids.push(parent);
- next = parent;
- }
- ids.reverse();
- ids
- }
-
pub(crate) fn visit(mut self) -> Module<'tcx> {
let root_module = self.cx.tcx.hir_root_module();
self.visit_mod_contents(CRATE_DEF_ID, root_module);
@@ -192,10 +176,9 @@ pub(crate) fn visit(mut self) -> Module<'tcx> {
{
let item = self.cx.tcx.hir_expect_item(local_def_id);
let (ident, _, _) = item.expect_macro();
- let cfg_parent_ids = self.cfg_parent_ids_for_detached_item(local_def_id);
top_level_module.items.insert(
(local_def_id, Some(ident.name)),
- ItemEntry { item, renamed: None, import_ids: Vec::new(), cfg_parent_ids },
+ ItemEntry { item, renamed: None, import_ids: Vec::new() },
);
}
}
@@ -436,17 +419,13 @@ fn add_to_current_mod(
.items
.entry(key)
.and_modify(|v| v.import_ids.push(import_id))
- .or_insert_with(|| ItemEntry {
- item,
- renamed,
- import_ids: vec![import_id],
- cfg_parent_ids: Vec::new(),
- });
+ .or_insert_with(|| ItemEntry { item, renamed, import_ids: vec![import_id] });
} else {
- self.modules.last_mut().unwrap().items.insert(
- key,
- ItemEntry { item, renamed, import_ids: Vec::new(), cfg_parent_ids: Vec::new() },
- );
+ self.modules
+ .last_mut()
+ .unwrap()
+ .items
+ .insert(key, ItemEntry { item, renamed, import_ids: Vec::new() });
}
}
}Is there something I missed? |
|
@rustbot author |
cbd8afc to
09e9d3f
Compare
No. I originally treated this as provenance metadata discovered in |
|
@rustbot ready |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
rustdoc: preserve parent doc cfg for `macro_export` macros
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (b90dfc5): comparison URL. Overall result: no relevant changes - no action neededBenchmarking means the PR may be perf-sensitive. Consider adding rollup=never if this change is not fit for rolling up. @rustbot label: -S-waiting-on-perf -perf-regression Instruction countThis perf run didn't have relevant results for this metric. Max RSS (memory usage)Results (primary 2.0%, secondary 2.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
CyclesResults (primary 2.2%, secondary 2.7%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Binary sizeResults (secondary 0.1%)A less reliable metric. May be of interest, but not used to determine the overall result above.
Bootstrap: 487.787s -> 482.444s (-1.10%) |
|
This sounds fine. @bors r+ |
The detached-item context is discovered before
propagate_doc_cfg, it is carried on the clean item and consumed by the pass.Closes #100916