Improve code and add more comments

GuillaumeGomez · GuillaumeGomez · commit 1da7684c7d0f · 2025-12-10T12:28:07.000+01:00
diff --git a/compiler/rustc_hir/src/attrs/data_structures.rs b/compiler/rustc_hir/src/attrs/data_structures.rs
@@ -510,7 +510,7 @@ pub struct CfgHideShow {
     pub values: ThinVec<CfgInfo>,
 }
 
-#[derive(Clone, Debug, HashStable_Generic, Encodable, Decodable, PrintAttribute)]
+#[derive(Clone, Debug, Default, HashStable_Generic, Encodable, Decodable, PrintAttribute)]
 pub struct DocAttribute {
     pub aliases: FxIndexMap<Symbol, Span>,
     pub hidden: Option<Span>,
@@ -546,34 +546,6 @@ pub struct DocAttribute {
     pub no_crate_inject: Option<Span>,
 }
 
-impl Default for DocAttribute {
-    fn default() -> Self {
-        Self {
-            aliases: FxIndexMap::default(),
-            hidden: None,
-            inline: ThinVec::new(),
-            cfg: ThinVec::new(),
-            auto_cfg: ThinVec::new(),
-            auto_cfg_change: ThinVec::new(),
-            fake_variadic: None,
-            keyword: None,
-            attribute: None,
-            masked: None,
-            notable_trait: None,
-            search_unbox: None,
-            html_favicon_url: None,
-            html_logo_url: None,
-            html_playground_url: None,
-            html_root_url: None,
-            html_no_source: None,
-            issue_tracker_base_url: None,
-            rust_logo: None,
-            test_attrs: ThinVec::new(),
-            no_crate_inject: None,
-        }
-    }
-}
-
 /// Represents parsed *built-in* inert attributes.
 ///
 /// ## Overview
diff --git a/src/librustdoc/clean/cfg.rs b/src/librustdoc/clean/cfg.rs
@@ -59,11 +59,11 @@ fn is_all_cfg(cfg: &CfgEntry) -> bool {
     }
 }
 
-fn strip_hidden(cfg: &CfgEntry, hidden: &FxHashSet<SimpleCfg>) -> Option<CfgEntry> {
+fn strip_hidden(cfg: &CfgEntry, hidden: &FxHashSet<NameValueCfg>) -> Option<CfgEntry> {
     match cfg {
         CfgEntry::Bool(..) => Some(cfg.clone()),
         CfgEntry::NameValue { .. } => {
-            if !hidden.contains(&SimpleCfg::from(cfg)) {
+            if !hidden.contains(&NameValueCfg::from(cfg)) {
                 Some(cfg.clone())
             } else {
                 None
@@ -109,7 +109,7 @@ impl Cfg {
     /// Parses a `MetaItemInner` into a `Cfg`.
     fn parse_nested(
         nested_cfg: &MetaItemInner,
-        exclude: &FxHashSet<SimpleCfg>,
+        exclude: &FxHashSet<NameValueCfg>,
     ) -> Result<Option<Cfg>, InvalidCfgError> {
         match nested_cfg {
             MetaItemInner::MetaItem(cfg) => Cfg::parse_without(cfg, exclude),
@@ -124,7 +124,7 @@ impl Cfg {
 
     fn parse_without(
         cfg: &MetaItem,
-        exclude: &FxHashSet<SimpleCfg>,
+        exclude: &FxHashSet<NameValueCfg>,
     ) -> Result<Option<Cfg>, InvalidCfgError> {
         let name = match cfg.ident() {
             Some(ident) => ident.name,
@@ -137,7 +137,7 @@ impl Cfg {
         };
         match cfg.kind {
             MetaItemKind::Word => {
-                if exclude.contains(&SimpleCfg::new(name)) {
+                if exclude.contains(&NameValueCfg::new(name)) {
                     Ok(None)
                 } else {
                     Ok(Some(Cfg(CfgEntry::NameValue {
@@ -150,7 +150,7 @@ impl Cfg {
             }
             MetaItemKind::NameValue(ref lit) => match lit.kind {
                 LitKind::Str(value, _) => {
-                    if exclude.contains(&SimpleCfg::new_value(name, value)) {
+                    if exclude.contains(&NameValueCfg::new_value(name, value)) {
                         Ok(None)
                     } else {
                         Ok(Some(Cfg(CfgEntry::NameValue {
@@ -666,12 +666,12 @@ impl fmt::Display for Display<'_> {
 }
 
 #[derive(Clone, Copy, Debug, PartialEq, Eq, Hash)]
-struct SimpleCfg {
+struct NameValueCfg {
     name: Symbol,
     value: Option<Symbol>,
 }
 
-impl SimpleCfg {
+impl NameValueCfg {
     fn new(name: Symbol) -> Self {
         Self { name, value: None }
     }
@@ -681,18 +681,18 @@ impl SimpleCfg {
     }
 }
 
-impl<'a> From<&'a CfgEntry> for SimpleCfg {
+impl<'a> From<&'a CfgEntry> for NameValueCfg {
     fn from(cfg: &'a CfgEntry) -> Self {
         match cfg {
             CfgEntry::NameValue { name, value, .. } => {
-                SimpleCfg { name: *name, value: (*value).map(|(v, _)| v) }
+                NameValueCfg { name: *name, value: (*value).map(|(v, _)| v) }
             }
-            _ => SimpleCfg { name: sym::empty, value: None },
+            _ => NameValueCfg { name: sym::empty, value: None },
         }
     }
 }
 
-impl<'a> From<&'a attrs::CfgInfo> for SimpleCfg {
+impl<'a> From<&'a attrs::CfgInfo> for NameValueCfg {
     fn from(cfg: &'a attrs::CfgInfo) -> Self {
         Self { name: cfg.name, value: cfg.value.map(|(value, _)| value) }
     }
@@ -703,7 +703,7 @@ impl<'a> From<&'a attrs::CfgInfo> for SimpleCfg {
 pub(crate) struct CfgInfo {
     /// List of currently active `doc(auto_cfg(hide(...)))` cfgs, minus currently active
     /// `doc(auto_cfg(show(...)))` cfgs.
-    hidden_cfg: FxHashSet<SimpleCfg>,
+    hidden_cfg: FxHashSet<NameValueCfg>,
     /// Current computed `cfg`. Each time we enter a new item, this field is updated as well while
     /// taking into account the `hidden_cfg` information.
     current_cfg: Cfg,
@@ -719,9 +719,9 @@ impl Default for CfgInfo {
     fn default() -> Self {
         Self {
             hidden_cfg: FxHashSet::from_iter([
-                SimpleCfg::new(sym::test),
-                SimpleCfg::new(sym::doc),
-                SimpleCfg::new(sym::doctest),
+                NameValueCfg::new(sym::test),
+                NameValueCfg::new(sym::doc),
+                NameValueCfg::new(sym::doctest),
             ]),
             current_cfg: Cfg(CfgEntry::Bool(true, DUMMY_SP)),
             auto_cfg_active: true,
@@ -761,7 +761,7 @@ fn handle_auto_cfg_hide_show(
     new_hide_attrs: &mut FxHashMap<(Symbol, Option<Symbol>), rustc_span::Span>,
 ) {
     for value in &attr.values {
-        let simple = SimpleCfg::from(value);
+        let simple = NameValueCfg::from(value);
         if attr.kind == HideOrShow::Show {
             if let Some(span) = new_hide_attrs.get(&(simple.name, simple.value)) {
                 show_hide_show_conflict_error(tcx, attr_span, *span);
diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
@@ -229,34 +229,28 @@ impl ExternalCrate {
     }
 
     pub(crate) fn keywords(&self, tcx: TyCtxt<'_>) -> impl Iterator<Item = (DefId, Symbol)> {
-        self.retrieve_keywords_or_documented_attributes(tcx, true)
+        self.retrieve_keywords_or_documented_attributes(tcx, |d| d.keyword.map(|(v, _)| v))
     }
     pub(crate) fn documented_attributes(
         &self,
         tcx: TyCtxt<'_>,
     ) -> impl Iterator<Item = (DefId, Symbol)> {
-        self.retrieve_keywords_or_documented_attributes(tcx, false)
+        self.retrieve_keywords_or_documented_attributes(tcx, |d| d.attribute.map(|(v, _)| v))
     }
 
-    fn retrieve_keywords_or_documented_attributes(
+    fn retrieve_keywords_or_documented_attributes<F: Fn(&DocAttribute) -> Option<Symbol>>(
         &self,
         tcx: TyCtxt<'_>,
-        look_for_keyword: bool,
+        callback: F,
     ) -> impl Iterator<Item = (DefId, Symbol)> {
         let as_target = move |did: DefId, tcx: TyCtxt<'_>| -> Option<(DefId, Symbol)> {
             tcx.get_all_attrs(did)
                 .iter()
                 .find_map(|attr| match attr {
-                    Attribute::Parsed(AttributeKind::Doc(d)) => {
-                        if look_for_keyword {
-                            d.keyword
-                        } else {
-                            d.attribute
-                        }
-                    }
+                    Attribute::Parsed(AttributeKind::Doc(d)) => callback(d),
                     _ => None,
                 })
-                .map(|(value, _)| (did, value))
+                .map(|value| (did, value))
         };
         self.mapped_root_modules(tcx, as_target)
     }
diff --git a/src/librustdoc/doctest/rust.rs b/src/librustdoc/doctest/rust.rs
@@ -143,13 +143,22 @@ impl HirCollector<'_> {
                         if let TokenTree::Ident(i) = token {
                             let i = i.to_string();
                             let peek = iter.peek();
+                            // From this ident, we can have things like:
+                            //
+                            // * Group: `allow(...)`
+                            // * Name/value: `crate_name = "..."`
+                            // * Tokens: `html_no_url`
+                            //
+                            // So we peek next element to know what case we are in.
                             match peek {
                                 Some(TokenTree::Group(g)) => {
                                     let g = g.to_string();
                                     iter.next();
                                     // Add the additional attributes to the global_crate_attrs vector
                                     self.collector.global_crate_attrs.push(format!("{i}{g}"));
                                 }
+                                // If next item is `=`, it means it's a name value so we will need
+                                // to get the value as well.
                                 Some(TokenTree::Punct(p)) if p.as_char() == '=' => {
                                     let p = p.to_string();
                                     iter.next();
diff --git a/src/librustdoc/passes/collect_trait_impls.rs b/src/librustdoc/passes/collect_trait_impls.rs
@@ -68,6 +68,9 @@ pub(crate) fn collect_trait_impls(mut krate: Crate, cx: &mut DocContext<'_>) ->
             while let Some(did) = parent {
                 attr_buf.extend(tcx.get_all_attrs(did).iter().filter_map(|attr| match attr {
                     Attribute::Parsed(AttributeKind::Doc(d)) if !d.cfg.is_empty() => {
+                        // The only doc attributes we're interested into for trait impls are the
+                        // `cfg`s for the `doc_cfg` feature. So we create a new empty `DocAttribute`
+                        // and then only clone the actual `DocAttribute::cfg` field.
                         let mut new_attr = DocAttribute::default();
                         new_attr.cfg = d.cfg.clone();
                         Some(Attribute::Parsed(AttributeKind::Doc(Box::new(new_attr))))
