cfg taken out of Attributes, put in Item

check item.is_fake() instead of self_id.is_some()

Remove empty branching in Attributes::from_ast

diverse small refacto after Josha review

cfg computation moved in merge_attrs

refacto use from_ast twice for coherence

take cfg out of Attributes and move it to Item

12 files changed, 104 insertions, 85 deletions

diff --git a/src/librustdoc/clean/auto_trait.rs b/src/librustdoc/clean/auto_trait.rs
index 010059a89c9..92eb6214f79 100644
--- a/src/librustdoc/clean/auto_trait.rs
+++ b/src/librustdoc/clean/auto_trait.rs
@@ -126,6 +126,7 @@ impl<'a, 'tcx> AutoTraitFinder<'a, 'tcx> {
                 synthetic: true,
                 blanket_impl: None,
             }),
+            cfg: None,
         })
     }
 
diff --git a/src/librustdoc/clean/blanket_impl.rs b/src/librustdoc/clean/blanket_impl.rs
index 50c0fb85d5a..3a14a1d23f2 100644
--- a/src/librustdoc/clean/blanket_impl.rs
+++ b/src/librustdoc/clean/blanket_impl.rs
@@ -128,6 +128,7 @@ impl<'a, 'tcx> BlanketImplFinder<'a, 'tcx> {
                         synthetic: false,
                         blanket_impl: Some(trait_ref.self_ty().clean(self.cx)),
                     }),
+                    cfg: None,
                 });
             }
         }
diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs
index 0f74639cbc6..674bf1a5814 100644
--- a/src/librustdoc/clean/inline.rs
+++ b/src/librustdoc/clean/inline.rs
@@ -1,6 +1,7 @@
 //! Support for inlining external documentation into the current AST.
 
 use std::iter::once;
+use std::sync::Arc;
 
 use rustc_ast as ast;
 use rustc_data_structures::fx::FxHashSet;
@@ -15,7 +16,7 @@ use rustc_span::hygiene::MacroKind;
 use rustc_span::symbol::{kw, sym, Symbol};
 use rustc_span::Span;
 
-use crate::clean::{self, Attributes, GetDefId, ToSource};
+use crate::clean::{self, Attributes, AttributesExt, GetDefId, ToSource};
 use crate::core::DocContext;
 use crate::formats::item_type::ItemType;
 
@@ -120,11 +121,16 @@ crate fn try_inline(
         _ => return None,
     };
 
-    let target_attrs = load_attrs(cx, did);
-    let attrs = box merge_attrs(cx, Some(parent_module), target_attrs, attrs_clone);
-
+    let (attrs, cfg) = merge_attrs(cx, Some(parent_module), load_attrs(cx, did), attrs_clone);
     cx.inlined.insert(did);
-    ret.push(clean::Item::from_def_id_and_attrs_and_parts(did, Some(name), kind, attrs, cx));
+    ret.push(clean::Item::from_def_id_and_attrs_and_parts(
+        did,
+        Some(name),
+        kind,
+        box attrs,
+        cx,
+        cfg,
+    ));
     Some(ret)
 }
 
@@ -288,22 +294,24 @@ fn merge_attrs(
     parent_module: Option<DefId>,
     old_attrs: Attrs<'_>,
     new_attrs: Option<Attrs<'_>>,
-) -> clean::Attributes {
+) -> (clean::Attributes, Option<Arc<clean::cfg::Cfg>>) {
     // NOTE: If we have additional attributes (from a re-export),
     // always insert them first. This ensure that re-export
     // doc comments show up before the original doc comments
     // when we render them.
     if let Some(inner) = new_attrs {
-        if let Some(new_id) = parent_module {
-            let diag = cx.sess().diagnostic();
-            Attributes::from_ast(diag, old_attrs, Some((inner, new_id)))
-        } else {
-            let mut both = inner.to_vec();
-            both.extend_from_slice(old_attrs);
-            both.clean(cx)
-        }
+        let mut both = inner.to_vec();
+        both.extend_from_slice(old_attrs);
+        (
+            if let Some(new_id) = parent_module {
+                Attributes::from_ast(old_attrs, Some((inner, new_id)))
+            } else {
+                Attributes::from_ast(&both, None)
+            },
+            both.cfg(cx.sess().diagnostic()),
+        )
     } else {
-        old_attrs.clean(cx)
+        (old_attrs.clean(cx), old_attrs.cfg(cx.sess().diagnostic()))
     }
 }
 
@@ -414,8 +422,8 @@ crate fn build_impl(
 
     debug!("build_impl: impl {:?} for {:?}", trait_.def_id(), for_.def_id());
 
-    let attrs = box merge_attrs(cx, parent_module.into(), load_attrs(cx, did), attrs);
-    debug!("merged_attrs={:?}", attrs);
+    let (merged_attrs, cfg) = merge_attrs(cx, parent_module.into(), load_attrs(cx, did), attrs);
+    debug!("merged_attrs={:?}", merged_attrs);
 
     ret.push(clean::Item::from_def_id_and_attrs_and_parts(
         did,
@@ -432,8 +440,9 @@ crate fn build_impl(
             synthetic: false,
             blanket_impl: None,
         }),
-        attrs,
+        box merged_attrs,
         cx,
+        cfg,
     ));
 }
 
@@ -479,6 +488,7 @@ fn build_module(
                         },
                         true,
                     )),
+                    cfg: None,
                 });
             } else if let Some(i) = try_inline(cx, did, item.res, item.ident.name, None, visited) {
                 items.extend(i)
diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
index d1a6f8d0c24..60c5d68b7a0 100644
--- a/src/librustdoc/clean/mod.rs
+++ b/src/librustdoc/clean/mod.rs
@@ -122,8 +122,8 @@ impl Clean<Item> for doctree::Module<'_> {
 }
 
 impl Clean<Attributes> for [ast::Attribute] {
-    fn clean(&self, cx: &mut DocContext<'_>) -> Attributes {
-        Attributes::from_ast(cx.sess().diagnostic(), self, None)
+    fn clean(&self, _cx: &mut DocContext<'_>) -> Attributes {
+        Attributes::from_ast(self, None)
     }
 }
 
@@ -1998,6 +1998,7 @@ fn clean_extern_crate(
             return items;
         }
     }
+
     // FIXME: using `from_def_id_and_kind` breaks `rustdoc/masked` for some reason
     vec![Item {
         name: Some(name),
@@ -2005,6 +2006,7 @@ fn clean_extern_crate(
         def_id: crate_def_id,
         visibility: krate.vis.clean(cx),
         kind: box ExternCrateItem { src: orig_name },
+        cfg: attrs.cfg(cx.sess().diagnostic()),
     }]
 }
 
diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
index 788b421c7ca..5e47144588b 100644
--- a/src/librustdoc/clean/types.rs
+++ b/src/librustdoc/clean/types.rs
@@ -219,11 +219,13 @@ crate struct Item {
     /// E.g., struct vs enum vs function.
     crate kind: Box<ItemKind>,
     crate def_id: DefId,
+
+    crate cfg: Option<Arc<Cfg>>,
 }
 
 // `Item` is used a lot. Make sure it doesn't unintentionally get bigger.
 #[cfg(all(target_arch = "x86_64", target_pointer_width = "64"))]
-rustc_data_structures::static_assert_size!(Item, 40);
+rustc_data_structures::static_assert_size!(Item, 48);
 
 impl fmt::Debug for Item {
     fn fmt(&self, fmt: &mut fmt::Formatter<'_>) -> fmt::Result {
@@ -235,6 +237,7 @@ impl fmt::Debug for Item {
             .field("kind", &self.kind)
             .field("visibility", &self.visibility)
             .field("def_id", def_id)
+            .field("cfg", &self.cfg)
             .finish()
     }
 }
@@ -262,6 +265,10 @@ impl Item {
         if self.is_fake() { None } else { tcx.lookup_deprecation(self.def_id) }
     }
 
+    crate fn inner_docs(&self, tcx: TyCtxt<'_>) -> bool {
+        if self.is_fake() { false } else { tcx.get_attrs(self.def_id).inner_docs() }
+    }
+
     crate fn span(&self, tcx: TyCtxt<'_>) -> Span {
         let kind = match &*self.kind {
             ItemKind::StrippedItem(k) => k,
@@ -305,12 +312,15 @@ impl Item {
         kind: ItemKind,
         cx: &mut DocContext<'_>,
     ) -> Item {
+        let ast_attrs = cx.tcx.get_attrs(def_id);
+
         Self::from_def_id_and_attrs_and_parts(
             def_id,
             name,
             kind,
-            box cx.tcx.get_attrs(def_id).clean(cx),
+            box ast_attrs.clean(cx),
             cx,
+            ast_attrs.cfg(cx.sess().diagnostic()),
         )
     }
 
@@ -320,6 +330,7 @@ impl Item {
         kind: ItemKind,
         attrs: Box<Attributes>,
         cx: &mut DocContext<'_>,
+        cfg: Option<Arc<Cfg>>,
     ) -> Item {
         debug!("name={:?}, def_id={:?}", name, def_id);
 
@@ -329,6 +340,7 @@ impl Item {
             name,
             attrs,
             visibility: cx.tcx.visibility(def_id).clean(cx),
+            cfg,
         }
     }
 
@@ -668,6 +680,8 @@ crate trait AttributesExt {
     fn inner_docs(&self) -> bool;
 
     fn other_attrs(&self) -> Vec<ast::Attribute>;
+
+    fn cfg(&self, diagnostic: &::rustc_errors::Handler) -> Option<Arc<Cfg>>;
 }
 
 impl AttributesExt for [ast::Attribute] {
@@ -691,6 +705,41 @@ impl AttributesExt for [ast::Attribute] {
     fn other_attrs(&self) -> Vec<ast::Attribute> {
         self.iter().filter(|attr| attr.doc_str().is_none()).cloned().collect()
     }
+
+    fn cfg(&self, diagnostic: &::rustc_errors::Handler) -> Option<Arc<Cfg>> {
+        let mut cfg = Cfg::True;
+
+        for attr in self.iter() {
+            if attr.doc_str().is_none() && attr.has_name(sym::doc) {
+                if let Some(mi) = attr.meta() {
+                    if let Some(cfg_mi) = Attributes::extract_cfg(&mi) {
+                        // Extracted #[doc(cfg(...))]
+                        match Cfg::parse(cfg_mi) {
+                            Ok(new_cfg) => cfg &= new_cfg,
+                            Err(e) => diagnostic.span_err(e.span, e.msg),
+                        }
+                    }
+                }
+            }
+        }
+
+        for attr in self.lists(sym::target_feature) {
+            if attr.has_name(sym::enable) {
+                if let Some(feat) = attr.value_str() {
+                    let meta = attr::mk_name_value_item_str(
+                        Ident::with_dummy_span(sym::target_feature),
+                        feat,
+                        DUMMY_SP,
+                    );
+                    if let Ok(feat_cfg) = Cfg::parse(&meta) {
+                        cfg &= feat_cfg;
+                    }
+                }
+            }
+        }
+
+        if cfg == Cfg::True { None } else { Some(Arc::new(cfg)) }
+    }
 }
 
 crate trait NestedAttributesExt {
@@ -799,7 +848,6 @@ impl<'a> FromIterator<&'a DocFragment> for String {
 crate struct Attributes {
     crate doc_strings: Vec<DocFragment>,
     crate other_attrs: Vec<ast::Attribute>,
-    crate cfg: Option<Arc<Cfg>>,
 }
 
 #[derive(Clone, Debug, Default, PartialEq, Eq, Hash)]
@@ -914,12 +962,10 @@ impl Attributes {
     }
 
     crate fn from_ast(
-        diagnostic: &::rustc_errors::Handler,
         attrs: &[ast::Attribute],
         additional_attrs: Option<(&[ast::Attribute], DefId)>,
     ) -> Attributes {
         let mut doc_strings: Vec<DocFragment> = vec![];
-        let mut cfg = Cfg::True;
         let mut doc_line = 0;
 
         fn update_need_backline(doc_strings: &mut Vec<DocFragment>, frag: &DocFragment) {
@@ -967,14 +1013,7 @@ impl Attributes {
             } else {
                 if attr.has_name(sym::doc) {
                     if let Some(mi) = attr.meta() {
-                        if let Some(cfg_mi) = Attributes::extract_cfg(&mi) {
-                            // Extracted #[doc(cfg(...))]
-                            match Cfg::parse(cfg_mi) {
-                                Ok(new_cfg) => cfg &= new_cfg,
-                                Err(e) => diagnostic.span_err(e.span, e.msg),
-                            }
-                        } else if let Some((filename, contents)) = Attributes::extract_include(&mi)
-                        {
+                        if let Some((filename, contents)) = Attributes::extract_include(&mi) {
                             let line = doc_line;
                             doc_line += contents.as_str().lines().count();
                             let frag = DocFragment {
@@ -1004,28 +1043,7 @@ impl Attributes {
             .filter_map(clean_attr)
             .collect();
 
-        // treat #[target_feature(enable = "feat")] attributes as if they were
-        // #[doc(cfg(target_feature = "feat"))] attributes as well
-        for attr in attrs.lists(sym::target_feature) {
-            if attr.has_name(sym::enable) {
-                if let Some(feat) = attr.value_str() {
-                    let meta = attr::mk_name_value_item_str(
-                        Ident::with_dummy_span(sym::target_feature),
-                        feat,
-                        DUMMY_SP,
-                    );
-                    if let Ok(feat_cfg) = Cfg::parse(&meta) {
-                        cfg &= feat_cfg;
-                    }
-                }
-            }
-        }
-
-        Attributes {
-            doc_strings,
-            other_attrs,
-            cfg: if cfg == Cfg::True { None } else { Some(Arc::new(cfg)) },
-        }
+        Attributes { doc_strings, other_attrs }
     }
 
     /// Finds the `doc` attribute as a NameValue and returns the corresponding
@@ -1091,7 +1109,6 @@ impl Attributes {
 impl PartialEq for Attributes {
     fn eq(&self, rhs: &Self) -> bool {
         self.doc_strings == rhs.doc_strings
-            && self.cfg == rhs.cfg
             && self
                 .other_attrs
                 .iter()
@@ -1105,7 +1122,6 @@ impl Eq for Attributes {}
 impl Hash for Attributes {
     fn hash<H: Hasher>(&self, hasher: &mut H) {
         self.doc_strings.hash(hasher);
-        self.cfg.hash(hasher);
         for attr in &self.other_attrs {
             attr.id.hash(hasher);
         }
diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs
index b51655ea203..466d1b65406 100644
--- a/src/librustdoc/doctest.rs
+++ b/src/librustdoc/doctest.rs
@@ -1093,9 +1093,9 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> {
         nested: F,
     ) {
         let ast_attrs = self.tcx.hir().attrs(hir_id);
+        let mut attrs = Attributes::from_ast(ast_attrs, None);
 
-        let mut attrs = Attributes::from_ast(self.sess.diagnostic(), ast_attrs, None);
-        if let Some(ref cfg) = attrs.cfg {
+        if let Some(ref cfg) = ast_attrs.cfg(self.sess.diagnostic()) {
             if !cfg.matches(&self.sess.parse_sess, Some(&self.sess.features_untracked())) {
                 return;
             }
diff --git a/src/librustdoc/html/render/cache.rs b/src/librustdoc/html/render/cache.rs
index d1aa6fba463..f4296a04e59 100644
--- a/src/librustdoc/html/render/cache.rs
+++ b/src/librustdoc/html/render/cache.rs
@@ -9,8 +9,7 @@ use serde::ser::{Serialize, SerializeStruct, Serializer};
 
 use crate::clean;
 use crate::clean::types::{
-    AttributesExt, FnDecl, FnRetTy, GenericBound, Generics, GetDefId, Type, TypeKind,
-    WherePredicate,
+    AttributesExt, FnDecl, FnRetTy, GenericBound, Generics, GetDefId, Type, WherePredicate,
 };
 use crate::formats::cache::Cache;
 use crate::formats::item_type::ItemType;
diff --git a/src/librustdoc/html/render/mod.rs b/src/librustdoc/html/render/mod.rs
index 67ccf2137bf..248bd46da85 100644
--- a/src/librustdoc/html/render/mod.rs
+++ b/src/librustdoc/html/render/mod.rs
@@ -608,17 +608,12 @@ fn document_item_info(
 }
 
 fn portability(item: &clean::Item, parent: Option<&clean::Item>) -> Option<String> {
-    let cfg = match (&item.attrs.cfg, parent.and_then(|p| p.attrs.cfg.as_ref())) {
+    let cfg = match (&item.cfg, parent.and_then(|p| p.cfg.as_ref())) {
         (Some(cfg), Some(parent_cfg)) => cfg.simplify_with(parent_cfg),
         (cfg, _) => cfg.as_deref().cloned(),
     };
 
-    debug!(
-        "Portability {:?} - {:?} = {:?}",
-        item.attrs.cfg,
-        parent.and_then(|p| p.attrs.cfg.as_ref()),
-        cfg
-    );
+    debug!("Portability {:?} - {:?} = {:?}", item.cfg, parent.and_then(|p| p.cfg.as_ref()), cfg);
 
     Some(format!("<div class=\"stab portability\">{}</div>", cfg?.render_long_html()))
 }
diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs
index f1d79ab097b..1bb1db00e88 100644
--- a/src/librustdoc/html/render/print_item.rs
+++ b/src/librustdoc/html/render/print_item.rs
@@ -1,10 +1,11 @@
+use clean::AttributesExt;
+
 use std::cmp::Ordering;
 
 use rustc_data_structures::fx::FxHashMap;
 use rustc_hir as hir;
 use rustc_hir::def::CtorKind;
 use rustc_hir::def_id::DefId;
-use rustc_middle::dep_graph::DepContext;
 use rustc_middle::middle::stability;
 use rustc_middle::ty::TyCtxt;
 use rustc_span::hygiene::MacroKind;
@@ -284,16 +285,14 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl
 
             clean::ImportItem(ref import) => {
                 let (stab, stab_tags) = if let Some(import_def_id) = import.source.did {
-                    let import_attrs = Box::new(clean::Attributes::from_ast(
-                        cx.tcx().sess().diagnostic(),
-                        cx.tcx().get_attrs(import_def_id),
-                        None,
-                    ));
+                    let ast_attrs = cx.tcx().get_attrs(import_def_id);
+                    let import_attrs = Box::new(clean::Attributes::from_ast(ast_attrs, None));
 
                     // Just need an item with the correct def_id and attrs
                     let import_item = clean::Item {
                         def_id: import_def_id,
                         attrs: import_attrs,
+                        cfg: ast_attrs.cfg(cx.tcx().sess.diagnostic()),
                         ..myitem.clone()
                     };
 
@@ -400,12 +399,12 @@ fn extra_info_tags(item: &clean::Item, parent: &clean::Item, tcx: TyCtxt<'_>) ->
         tags += &tag_html("unstable", "", "Experimental");
     }
 
-    let cfg = match (&item.attrs.cfg, parent.attrs.cfg.as_ref()) {
+    let cfg = match (&item.cfg, parent.cfg.as_ref()) {
         (Some(cfg), Some(parent_cfg)) => cfg.simplify_with(parent_cfg),
         (cfg, _) => cfg.as_deref().cloned(),
     };
 
-    debug!("Portability {:?} - {:?} = {:?}", item.attrs.cfg, parent.attrs.cfg, cfg);
+    debug!("Portability {:?} - {:?} = {:?}", item.cfg, parent.cfg, cfg);
     if let Some(ref cfg) = cfg {
         tags += &tag_html("portability", &cfg.render_long_plain(), &cfg.render_short_html());
     }
diff --git a/src/librustdoc/json/conversions.rs b/src/librustdoc/json/conversions.rs
index 3d5dc7dbec0..2d8c347c3c1 100644
--- a/src/librustdoc/json/conversions.rs
+++ b/src/librustdoc/json/conversions.rs
@@ -41,7 +41,7 @@ impl JsonRenderer<'_> {
             .map(rustc_ast_pretty::pprust::attribute_to_string)
             .collect();
         let span = item.span(self.tcx);
-        let clean::Item { name, attrs: _, kind: _, visibility, def_id } = item;
+        let clean::Item { name, attrs: _, kind: _, visibility, def_id, cfg: _ } = item;
         let inner = match *item.kind {
             clean::StrippedItem(_) => return None,
             _ => from_clean_item(item, self.tcx),
diff --git a/src/librustdoc/passes/collect_intra_doc_links.rs b/src/librustdoc/passes/collect_intra_doc_links.rs
index 7f479b62c90..f1064756fdd 100644
--- a/src/librustdoc/passes/collect_intra_doc_links.rs
+++ b/src/librustdoc/passes/collect_intra_doc_links.rs
@@ -2,7 +2,6 @@
 //!
 //! [RFC 1946]: https://github.com/rust-lang/rfcs/blob/master/text/1946-intra-rustdoc-links.md
 
-use clean::AttributesExt;
 use rustc_ast as ast;
 use rustc_data_structures::{fx::FxHashMap, stable_set::FxHashSet};
 use rustc_errors::{Applicability, DiagnosticBuilder};
@@ -854,10 +853,7 @@ impl<'a, 'tcx> DocFolder for LinkCollector<'a, 'tcx> {
             }
         });
 
-        let inner_docs = match self_id {
-            Some(did) => self.cx.tcx.get_attrs(did).inner_docs(),
-            None => false,
-        };
+        let inner_docs = item.inner_docs(self.cx.tcx);
 
         if item.is_mod() && inner_docs {
             self.mod_ids.push(item.def_id);
@@ -1056,7 +1052,7 @@ impl LinkCollector<'_, '_> {
             };
         let mut path_str = &*path_str;
 
-        let inner_docs = self.cx.tcx.get_attrs(item.def_id).inner_docs();
+        let inner_docs = item.inner_docs(self.cx.tcx);
 
         // In order to correctly resolve intra-doc links we need to
         // pick a base AST node to work from.  If the documentation for
diff --git a/src/librustdoc/passes/propagate_doc_cfg.rs b/src/librustdoc/passes/propagate_doc_cfg.rs
index 2369ff78b1c..f5a362bfbe8 100644
--- a/src/librustdoc/passes/propagate_doc_cfg.rs
+++ b/src/librustdoc/passes/propagate_doc_cfg.rs
@@ -24,7 +24,7 @@ impl DocFolder for CfgPropagator {
     fn fold_item(&mut self, mut item: Item) -> Option<Item> {
         let old_parent_cfg = self.parent_cfg.clone();
 
-        let new_cfg = match (self.parent_cfg.take(), item.attrs.cfg.take()) {
+        let new_cfg = match (self.parent_cfg.take(), item.cfg.take()) {
             (None, None) => None,
             (Some(rc), None) | (None, Some(rc)) => Some(rc),
             (Some(mut a), Some(b)) => {
@@ -34,7 +34,7 @@ impl DocFolder for CfgPropagator {
             }
         };
         self.parent_cfg = new_cfg.clone();
-        item.attrs.cfg = new_cfg;
+        item.cfg = new_cfg;
 
         let result = self.fold_item_recur(item);
         self.parent_cfg = old_parent_cfg;