Skip to content

Commit

Permalink
Rollup merge of #135428 - camelid:attr-cleanup, r=GuillaumeGomez
Browse files Browse the repository at this point in the history
rustdoc: Remove `AttributesExt` trait magic that added needless complexity

The new code is more explicit and avoids trait magic that added needless
complexity to this part of rustdoc.
  • Loading branch information
GuillaumeGomez authored Jan 15, 2025
2 parents 369d135 + f92b32c commit b1d047b
Show file tree
Hide file tree
Showing 5 changed files with 119 additions and 148 deletions.
17 changes: 10 additions & 7 deletions src/librustdoc/clean/inline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@ use rustc_span::symbol::{Symbol, sym};
use thin_vec::{ThinVec, thin_vec};
use tracing::{debug, trace};

use super::Item;
use super::{Item, extract_cfg_from_attrs};
use crate::clean::{
self, Attributes, AttributesExt, ImplKind, ItemId, Type, clean_bound_vars, clean_generics,
clean_impl_item, clean_middle_assoc_item, clean_middle_field, clean_middle_ty,
clean_poly_fn_sig, clean_trait_ref_with_constraints, clean_ty, clean_ty_alias_inner_type,
clean_ty_generics, clean_variant_def, utils,
self, Attributes, ImplKind, ItemId, Type, clean_bound_vars, clean_generics, clean_impl_item,
clean_middle_assoc_item, clean_middle_field, clean_middle_ty, clean_poly_fn_sig,
clean_trait_ref_with_constraints, clean_ty, clean_ty_alias_inner_type, clean_ty_generics,
clean_variant_def, utils,
};
use crate::core::DocContext;
use crate::formats::item_type::ItemType;
Expand Down Expand Up @@ -408,10 +408,13 @@ pub(crate) fn merge_attrs(
} else {
Attributes::from_hir(&both)
},
both.cfg(cx.tcx, &cx.cache.hidden_cfg),
extract_cfg_from_attrs(both.iter(), cx.tcx, &cx.cache.hidden_cfg),
)
} else {
(Attributes::from_hir(old_attrs), old_attrs.cfg(cx.tcx, &cx.cache.hidden_cfg))
(
Attributes::from_hir(old_attrs),
extract_cfg_from_attrs(old_attrs.iter(), cx.tcx, &cx.cache.hidden_cfg),
)
}
}

Expand Down
20 changes: 13 additions & 7 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,7 @@ fn generate_item_with_correct_attrs(
// For glob re-exports the item may or may not exist to be re-exported (potentially the cfgs
// on the path up until the glob can be removed, and only cfgs on the globbed item itself
// matter), for non-inlined re-exports see #85043.
let is_inline = inline::load_attrs(cx, import_id.to_def_id())
.lists(sym::doc)
let is_inline = hir_attr_lists(inline::load_attrs(cx, import_id.to_def_id()), sym::doc)
.get_word_attr(sym::inline)
.is_some()
|| (is_glob_import(cx.tcx, import_id)
Expand All @@ -199,8 +198,14 @@ fn generate_item_with_correct_attrs(
// We only keep the item's attributes.
target_attrs.iter().map(|attr| (Cow::Borrowed(attr), None)).collect()
};

let cfg = attrs.cfg(cx.tcx, &cx.cache.hidden_cfg);
let cfg = extract_cfg_from_attrs(
attrs.iter().map(move |(attr, _)| match attr {
Cow::Borrowed(attr) => *attr,
Cow::Owned(attr) => attr,
}),
cx.tcx,
&cx.cache.hidden_cfg,
);
let attrs = Attributes::from_hir_iter(attrs.iter().map(|(attr, did)| (&**attr, *did)), false);

let name = renamed.or(Some(name));
Expand Down Expand Up @@ -979,13 +984,14 @@ fn clean_proc_macro<'tcx>(
) -> ItemKind {
let attrs = cx.tcx.hir().attrs(item.hir_id());
if kind == MacroKind::Derive
&& let Some(derive_name) = attrs.lists(sym::proc_macro_derive).find_map(|mi| mi.ident())
&& let Some(derive_name) =
hir_attr_lists(attrs, sym::proc_macro_derive).find_map(|mi| mi.ident())
{
*name = derive_name.name;
}

let mut helpers = Vec::new();
for mi in attrs.lists(sym::proc_macro_derive) {
for mi in hir_attr_lists(attrs, sym::proc_macro_derive) {
if !mi.has_name(sym::attributes) {
continue;
}
Expand Down Expand Up @@ -2985,7 +2991,7 @@ fn clean_use_statement_inner<'tcx>(

let visibility = cx.tcx.visibility(import.owner_id);
let attrs = cx.tcx.hir().attrs(import.hir_id());
let inline_attr = attrs.lists(sym::doc).get_word_attr(sym::inline);
let inline_attr = hir_attr_lists(attrs, sym::doc).get_word_attr(sym::inline);
let pub_underscore = visibility.is_public() && name == kw::Underscore;
let current_mod = cx.tcx.parent_module_from_def_id(import.owner_id.def_id);
let import_def_id = import.owner_id.def_id;
Expand Down
217 changes: 89 additions & 128 deletions src/librustdoc/clean/types.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use std::borrow::Cow;
use std::hash::Hash;
use std::path::PathBuf;
use std::sync::{Arc, OnceLock as OnceCell};
Expand Down Expand Up @@ -485,7 +484,7 @@ impl Item {
name,
kind,
Attributes::from_hir(hir_attrs),
hir_attrs.cfg(cx.tcx, &cx.cache.hidden_cfg),
extract_cfg_from_attrs(hir_attrs.iter(), cx.tcx, &cx.cache.hidden_cfg),
)
}

Expand Down Expand Up @@ -996,147 +995,107 @@ pub(crate) struct Module {
pub(crate) span: Span,
}

pub(crate) trait AttributesExt {
type AttributeIterator<'a>: Iterator<Item = ast::MetaItemInner>
where
Self: 'a;
type Attributes<'a>: Iterator<Item = &'a hir::Attribute>
where
Self: 'a;

fn lists(&self, name: Symbol) -> Self::AttributeIterator<'_>;

fn iter(&self) -> Self::Attributes<'_>;

fn cfg(&self, tcx: TyCtxt<'_>, hidden_cfg: &FxHashSet<Cfg>) -> Option<Arc<Cfg>> {
let sess = tcx.sess;
let doc_cfg_active = tcx.features().doc_cfg();
let doc_auto_cfg_active = tcx.features().doc_auto_cfg();

fn single<T: IntoIterator>(it: T) -> Option<T::Item> {
let mut iter = it.into_iter();
let item = iter.next()?;
if iter.next().is_some() {
return None;
}
Some(item)
pub(crate) fn hir_attr_lists<'a, I: IntoIterator<Item = &'a hir::Attribute>>(
attrs: I,
name: Symbol,
) -> impl Iterator<Item = ast::MetaItemInner> + use<'a, I> {
attrs
.into_iter()
.filter(move |attr| attr.has_name(name))
.filter_map(ast::attr::AttributeExt::meta_item_list)
.flatten()
}

pub(crate) fn extract_cfg_from_attrs<'a, I: Iterator<Item = &'a hir::Attribute> + Clone>(
attrs: I,
tcx: TyCtxt<'_>,
hidden_cfg: &FxHashSet<Cfg>,
) -> Option<Arc<Cfg>> {
let sess = tcx.sess;
let doc_cfg_active = tcx.features().doc_cfg();
let doc_auto_cfg_active = tcx.features().doc_auto_cfg();

fn single<T: IntoIterator>(it: T) -> Option<T::Item> {
let mut iter = it.into_iter();
let item = iter.next()?;
if iter.next().is_some() {
return None;
}
Some(item)
}

let mut cfg = if doc_cfg_active || doc_auto_cfg_active {
let mut doc_cfg = self
.iter()
.filter(|attr| attr.has_name(sym::doc))
.flat_map(|attr| attr.meta_item_list().unwrap_or_default())
let mut cfg = if doc_cfg_active || doc_auto_cfg_active {
let mut doc_cfg = attrs
.clone()
.filter(|attr| attr.has_name(sym::doc))
.flat_map(|attr| attr.meta_item_list().unwrap_or_default())
.filter(|attr| attr.has_name(sym::cfg))
.peekable();
if doc_cfg.peek().is_some() && doc_cfg_active {
doc_cfg
.filter_map(|attr| Cfg::parse(&attr).ok())
.fold(Cfg::True, |cfg, new_cfg| cfg & new_cfg)
} else if doc_auto_cfg_active {
// If there is no `doc(cfg())`, then we retrieve the `cfg()` attributes (because
// `doc(cfg())` overrides `cfg()`).
attrs
.clone()
.filter(|attr| attr.has_name(sym::cfg))
.peekable();
if doc_cfg.peek().is_some() && doc_cfg_active {
doc_cfg
.filter_map(|attr| Cfg::parse(&attr).ok())
.fold(Cfg::True, |cfg, new_cfg| cfg & new_cfg)
} else if doc_auto_cfg_active {
// If there is no `doc(cfg())`, then we retrieve the `cfg()` attributes (because
// `doc(cfg())` overrides `cfg()`).
self.iter()
.filter(|attr| attr.has_name(sym::cfg))
.filter_map(|attr| single(attr.meta_item_list()?))
.filter_map(|attr| {
Cfg::parse_without(attr.meta_item()?, hidden_cfg).ok().flatten()
})
.fold(Cfg::True, |cfg, new_cfg| cfg & new_cfg)
} else {
Cfg::True
}
.filter_map(|attr| single(attr.meta_item_list()?))
.filter_map(|attr| Cfg::parse_without(attr.meta_item()?, hidden_cfg).ok().flatten())
.fold(Cfg::True, |cfg, new_cfg| cfg & new_cfg)
} else {
Cfg::True
};

for attr in self.iter() {
// #[doc]
if attr.doc_str().is_none() && attr.has_name(sym::doc) {
// #[doc(...)]
if let Some(list) = attr.meta_item_list() {
for item in list {
// #[doc(hidden)]
if !item.has_name(sym::cfg) {
continue;
}
// #[doc(cfg(...))]
if let Some(cfg_mi) = item
.meta_item()
.and_then(|item| rustc_expand::config::parse_cfg(item, sess))
{
match Cfg::parse(cfg_mi) {
Ok(new_cfg) => cfg &= new_cfg,
Err(e) => {
sess.dcx().span_err(e.span, e.msg);
}
}
} else {
Cfg::True
};

for attr in attrs.clone() {
// #[doc]
if attr.doc_str().is_none() && attr.has_name(sym::doc) {
// #[doc(...)]
if let Some(list) = attr.meta_item_list() {
for item in list {
// #[doc(hidden)]
if !item.has_name(sym::cfg) {
continue;
}
// #[doc(cfg(...))]
if let Some(cfg_mi) = item
.meta_item()
.and_then(|item| rustc_expand::config::parse_cfg(item, sess))
{
match Cfg::parse(cfg_mi) {
Ok(new_cfg) => cfg &= new_cfg,
Err(e) => {
sess.dcx().span_err(e.span, e.msg);
}
}
}
}
}
}
}

// treat #[target_feature(enable = "feat")] attributes as if they were
// #[doc(cfg(target_feature = "feat"))] attributes as well
for attr in self.lists(sym::target_feature) {
if attr.has_name(sym::enable) {
if attr.value_str().is_some() {
// Clone `enable = "feat"`, change to `target_feature = "feat"`.
// Unwrap is safe because `value_str` succeeded above.
let mut meta = attr.meta_item().unwrap().clone();
meta.path = ast::Path::from_ident(Ident::with_dummy_span(sym::target_feature));

if let Ok(feat_cfg) = Cfg::parse(&ast::MetaItemInner::MetaItem(meta)) {
cfg &= feat_cfg;
}
// treat #[target_feature(enable = "feat")] attributes as if they were
// #[doc(cfg(target_feature = "feat"))] attributes as well
for attr in hir_attr_lists(attrs, sym::target_feature) {
if attr.has_name(sym::enable) {
if attr.value_str().is_some() {
// Clone `enable = "feat"`, change to `target_feature = "feat"`.
// Unwrap is safe because `value_str` succeeded above.
let mut meta = attr.meta_item().unwrap().clone();
meta.path = ast::Path::from_ident(Ident::with_dummy_span(sym::target_feature));

if let Ok(feat_cfg) = Cfg::parse(&ast::MetaItemInner::MetaItem(meta)) {
cfg &= feat_cfg;
}
}
}

if cfg == Cfg::True { None } else { Some(Arc::new(cfg)) }
}
}

impl AttributesExt for [hir::Attribute] {
type AttributeIterator<'a> = impl Iterator<Item = ast::MetaItemInner> + 'a;
type Attributes<'a> = impl Iterator<Item = &'a hir::Attribute> + 'a;

fn lists(&self, name: Symbol) -> Self::AttributeIterator<'_> {
self.iter()
.filter(move |attr| attr.has_name(name))
.filter_map(ast::attr::AttributeExt::meta_item_list)
.flatten()
}

fn iter(&self) -> Self::Attributes<'_> {
self.iter()
}
}

impl AttributesExt for [(Cow<'_, hir::Attribute>, Option<DefId>)] {
type AttributeIterator<'a>
= impl Iterator<Item = ast::MetaItemInner> + 'a
where
Self: 'a;
type Attributes<'a>
= impl Iterator<Item = &'a hir::Attribute> + 'a
where
Self: 'a;

fn lists(&self, name: Symbol) -> Self::AttributeIterator<'_> {
AttributesExt::iter(self)
.filter(move |attr| attr.has_name(name))
.filter_map(hir::Attribute::meta_item_list)
.flatten()
}

fn iter(&self) -> Self::Attributes<'_> {
self.iter().map(move |(attr, _)| match attr {
Cow::Borrowed(attr) => *attr,
Cow::Owned(attr) => attr,
})
}
if cfg == Cfg::True { None } else { Some(Arc::new(cfg)) }
}

pub(crate) trait NestedAttributesExt {
Expand Down Expand Up @@ -1202,7 +1161,7 @@ pub(crate) struct Attributes {

impl Attributes {
pub(crate) fn lists(&self, name: Symbol) -> impl Iterator<Item = ast::MetaItemInner> + '_ {
self.other_attrs.lists(name)
hir_attr_lists(&self.other_attrs[..], name)
}

pub(crate) fn has_doc_flag(&self, flag: Symbol) -> bool {
Expand Down Expand Up @@ -1269,7 +1228,9 @@ impl Attributes {
pub(crate) fn get_doc_aliases(&self) -> Box<[Symbol]> {
let mut aliases = FxIndexSet::default();

for attr in self.other_attrs.lists(sym::doc).filter(|a| a.has_name(sym::alias)) {
for attr in
hir_attr_lists(&self.other_attrs[..], sym::doc).filter(|a| a.has_name(sym::alias))
{
if let Some(values) = attr.meta_item_list() {
for l in values {
if let Some(lit) = l.lit()
Expand Down
7 changes: 4 additions & 3 deletions src/librustdoc/doctest/rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ use rustc_span::source_map::SourceMap;
use rustc_span::{BytePos, DUMMY_SP, FileName, Pos, Span};

use super::{DocTestVisitor, ScrapedDocTest};
use crate::clean::Attributes;
use crate::clean::types::AttributesExt;
use crate::clean::{Attributes, extract_cfg_from_attrs};
use crate::html::markdown::{self, ErrorCodes, LangString, MdRelLine};

struct RustCollector {
Expand Down Expand Up @@ -97,7 +96,9 @@ impl HirCollector<'_> {
nested: F,
) {
let ast_attrs = self.tcx.hir().attrs(self.tcx.local_def_id_to_hir_id(def_id));
if let Some(ref cfg) = ast_attrs.cfg(self.tcx, &FxHashSet::default()) {
if let Some(ref cfg) =
extract_cfg_from_attrs(ast_attrs.iter(), self.tcx, &FxHashSet::default())
{
if !cfg.matches(&self.tcx.sess.psess, Some(self.tcx.features())) {
return;
}
Expand Down
6 changes: 3 additions & 3 deletions src/librustdoc/visit_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use tracing::debug;

use crate::clean::cfg::Cfg;
use crate::clean::utils::{inherits_doc_hidden, should_ignore_res};
use crate::clean::{AttributesExt, NestedAttributesExt, reexport_chain};
use crate::clean::{NestedAttributesExt, hir_attr_lists, reexport_chain};
use crate::core;

/// This module is used to store stuff from Rust's AST in a more convenient
Expand Down Expand Up @@ -247,8 +247,8 @@ impl<'a, 'tcx> RustdocVisitor<'a, 'tcx> {
let document_hidden = self.cx.render_options.document_hidden;
let use_attrs = tcx.hir().attrs(tcx.local_def_id_to_hir_id(def_id));
// Don't inline `doc(hidden)` imports so they can be stripped at a later stage.
let is_no_inline = use_attrs.lists(sym::doc).has_word(sym::no_inline)
|| (document_hidden && use_attrs.lists(sym::doc).has_word(sym::hidden));
let is_no_inline = hir_attr_lists(use_attrs, sym::doc).has_word(sym::no_inline)
|| (document_hidden && hir_attr_lists(use_attrs, sym::doc).has_word(sym::hidden));

if is_no_inline {
return false;
Expand Down

0 comments on commit b1d047b

Please sign in to comment.