Unify rustc and rustdoc parsing of cfg()

This extracts a new `parse_cfg` function that's used between both. - Treat `#[doc(cfg(x), cfg(y))]` the same as `#[doc(cfg(x)] #[doc(cfg(y))]`. Previously it would be completely ignored. - Treat `#[doc(inline, cfg(x))]` the same as `#[doc(inline)] #[doc(cfg(x))]`. Previously, the cfg would be ignored. - Pass the cfg predicate through to rustc_expand to be validated Co-authored-by: Vadim Petrochenkov <[email protected]>
Dylan-DPC-zz · May 3, 2021 · 6eb4735 · 6eb4735
1 parent 78c9639
commit 6eb4735
Show file tree

Hide file tree

Showing 10 changed files with 82 additions and 64 deletions.
diff --git a/compiler/rustc_expand/src/config.rs b/compiler/rustc_expand/src/config.rs
@@ -464,31 +464,9 @@ impl<'a> StripUnconfigured<'a> {
                     return true;
                 }
             };
-            let error = |span, msg, suggestion: &str| {
-                let mut err = self.sess.parse_sess.span_diagnostic.struct_span_err(span, msg);
-                if !suggestion.is_empty() {
-                    err.span_suggestion(
-                        span,
-                        "expected syntax is",
-                        suggestion.into(),
-                        Applicability::MaybeIncorrect,
-                    );
-                }
-                err.emit();
-                true
-            };
-            let span = meta_item.span;
-            match meta_item.meta_item_list() {
-                None => error(span, "`cfg` is not followed by parentheses", "cfg(/* predicate */)"),
-                Some([]) => error(span, "`cfg` predicate is not specified", ""),
-                Some([_, .., l]) => error(l.span(), "multiple `cfg` predicates are specified", ""),
-                Some([single]) => match single.meta_item() {
-                    Some(meta_item) => {
-                        attr::cfg_matches(meta_item, &self.sess.parse_sess, self.features)
-                    }
-                    None => error(single.span(), "`cfg` predicate key cannot be a literal", ""),
-                },
-            }
+            parse_cfg(&meta_item, &self.sess).map_or(true, |meta_item| {
+                attr::cfg_matches(&meta_item, &self.sess.parse_sess, self.features)
+            })
         })
     }
 
@@ -532,6 +510,32 @@ impl<'a> StripUnconfigured<'a> {
     }
 }
 
+pub fn parse_cfg<'a>(meta_item: &'a MetaItem, sess: &Session) -> Option<&'a MetaItem> {
+    let error = |span, msg, suggestion: &str| {
+        let mut err = sess.parse_sess.span_diagnostic.struct_span_err(span, msg);
+        if !suggestion.is_empty() {
+            err.span_suggestion(
+                span,
+                "expected syntax is",
+                suggestion.into(),
+                Applicability::HasPlaceholders,
+            );
+        }
+        err.emit();
+        None
+    };
+    let span = meta_item.span;
+    match meta_item.meta_item_list() {
+        None => error(span, "`cfg` is not followed by parentheses", "cfg(/* predicate */)"),
+        Some([]) => error(span, "`cfg` predicate is not specified", ""),
+        Some([_, .., l]) => error(l.span(), "multiple `cfg` predicates are specified", ""),
+        Some([single]) => match single.meta_item() {
+            Some(meta_item) => Some(meta_item),
+            None => error(single.span(), "`cfg` predicate key cannot be a literal", ""),
+        },
+    }
+}
+
 fn is_cfg(sess: &Session, attr: &Attribute) -> bool {
     sess.check_name(attr, sym::cfg)
 }
diff --git a/src/librustdoc/clean/inline.rs b/src/librustdoc/clean/inline.rs
@@ -307,10 +307,10 @@ fn merge_attrs(
             } else {
                 Attributes::from_ast(&both, None)
             },
-            both.cfg(cx.sess().diagnostic()),
+            both.cfg(cx.sess()),
         )
     } else {
-        (old_attrs.clean(cx), old_attrs.cfg(cx.sess().diagnostic()))
+        (old_attrs.clean(cx), old_attrs.cfg(cx.sess()))
     }
 }
 

diff --git a/src/librustdoc/clean/mod.rs b/src/librustdoc/clean/mod.rs
@@ -2006,7 +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()),
+        cfg: attrs.cfg(cx.sess()),
     }]
 }
 

diff --git a/src/librustdoc/clean/types.rs b/src/librustdoc/clean/types.rs
@@ -320,7 +320,7 @@ impl Item {
             kind,
             box ast_attrs.clean(cx),
             cx,
-            ast_attrs.cfg(cx.sess().diagnostic()),
+            ast_attrs.cfg(cx.sess()),
         )
     }
 
@@ -332,7 +332,7 @@ impl Item {
         cx: &mut DocContext<'_>,
         cfg: Option<Arc<Cfg>>,
     ) -> Item {
-        debug!("name={:?}, def_id={:?}", name, def_id);
+        trace!("name={:?}, def_id={:?}", name, def_id);
 
         Item {
             def_id,
@@ -681,7 +681,7 @@ crate trait AttributesExt {
 
     fn other_attrs(&self) -> Vec<ast::Attribute>;
 
-    fn cfg(&self, diagnostic: &::rustc_errors::Handler) -> Option<Arc<Cfg>>;
+    fn cfg(&self, sess: &Session) -> Option<Arc<Cfg>>;
 }
 
 impl AttributesExt for [ast::Attribute] {
@@ -706,17 +706,28 @@ impl AttributesExt for [ast::Attribute] {
         self.iter().filter(|attr| attr.doc_str().is_none()).cloned().collect()
     }
 
-    fn cfg(&self, diagnostic: &::rustc_errors::Handler) -> Option<Arc<Cfg>> {
+    fn cfg(&self, sess: &Session) -> Option<Arc<Cfg>> {
         let mut cfg = Cfg::True;
 
         for attr in self.iter() {
+            // #[doc]
             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),
+                // #[doc(...)]
+                if let Some(list) = attr.meta().as_ref().and_then(|mi| mi.meta_item_list()) {
+                    for item in list {
+                        // #[doc(include)]
+                        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.span_err(e.span, e.msg),
+                            }
                         }
                     }
                 }
@@ -883,29 +894,6 @@ impl Attributes {
         self.other_attrs.lists(name)
     }
 
-    /// Extracts the content from an attribute `#[doc(cfg(content))]`.
-    crate fn extract_cfg(mi: &ast::MetaItem) -> Option<&ast::MetaItem> {
-        use rustc_ast::NestedMetaItem::MetaItem;
-
-        if let ast::MetaItemKind::List(ref nmis) = mi.kind {
-            if nmis.len() == 1 {
-                if let MetaItem(ref cfg_mi) = nmis[0] {
-                    if cfg_mi.has_name(sym::cfg) {
-                        if let ast::MetaItemKind::List(ref cfg_nmis) = cfg_mi.kind {
-                            if cfg_nmis.len() == 1 {
-                                if let MetaItem(ref content_mi) = cfg_nmis[0] {
-                                    return Some(content_mi);
-                                }
-                            }
-                        }
-                    }
-                }
-            }
-        }
-
-        None
-    }
-
     /// Reads a `MetaItem` from within an attribute, looks for whether it is a
     /// `#[doc(include="file")]`, and returns the filename and contents of the file as loaded from
     /// its expansion.

diff --git a/src/librustdoc/doctest.rs b/src/librustdoc/doctest.rs
@@ -1095,7 +1095,7 @@ impl<'a, 'hir, 'tcx> HirCollector<'a, 'hir, 'tcx> {
         let ast_attrs = self.tcx.hir().attrs(hir_id);
         let mut attrs = Attributes::from_ast(ast_attrs, None);
 
-        if let Some(ref cfg) = ast_attrs.cfg(self.sess.diagnostic()) {
+        if let Some(ref cfg) = ast_attrs.cfg(self.sess) {
             if !cfg.matches(&self.sess.parse_sess, Some(&self.sess.features_untracked())) {
                 return;
             }

diff --git a/src/librustdoc/html/render/context.rs b/src/librustdoc/html/render/context.rs
@@ -154,7 +154,7 @@ impl<'tcx> Context<'tcx> {
         &self.cache
     }
 
-    fn sess(&self) -> &'tcx Session {
+    pub(super) fn sess(&self) -> &'tcx Session {
         &self.shared.tcx.sess
     }
 

diff --git a/src/librustdoc/html/render/print_item.rs b/src/librustdoc/html/render/print_item.rs
@@ -292,7 +292,7 @@ fn item_module(w: &mut Buffer, cx: &Context<'_>, item: &clean::Item, items: &[cl
                     let import_item = clean::Item {
                         def_id: import_def_id,
                         attrs: import_attrs,
-                        cfg: ast_attrs.cfg(cx.tcx().sess.diagnostic()),
+                        cfg: ast_attrs.cfg(cx.sess()),
                         ..myitem.clone()
                     };
 

diff --git a/src/test/rustdoc-ui/invalid-cfg.rs b/src/test/rustdoc-ui/invalid-cfg.rs
@@ -0,0 +1,4 @@
+#![feature(doc_cfg)]
+#[doc(cfg = "x")] //~ ERROR not followed by parentheses
+#[doc(cfg(x, y))] //~ ERROR multiple `cfg` predicates
+struct S {}
diff --git a/src/test/rustdoc-ui/invalid-cfg.stderr b/src/test/rustdoc-ui/invalid-cfg.stderr
@@ -0,0 +1,14 @@
+error: `cfg` is not followed by parentheses
+  --> $DIR/invalid-cfg.rs:2:7
+   |
+LL | #[doc(cfg = "x")]
+   |       ^^^^^^^^^ help: expected syntax is: `cfg(/* predicate */)`
+
+error: multiple `cfg` predicates are specified
+  --> $DIR/invalid-cfg.rs:3:14
+   |
+LL | #[doc(cfg(x, y))]
+   |              ^
+
+error: aborting due to 2 previous errors
+
diff --git a/src/test/rustdoc/doc-cfg.rs b/src/test/rustdoc/doc-cfg.rs
@@ -91,3 +91,11 @@ pub unsafe fn uses_target_feature() {
 pub fn uses_cfg_target_feature() {
     uses_target_feature();
 }
+
+// multiple attributes should be allowed
+// @has doc_cfg/fn.multiple_attrs.html \
+//  '//*[@id="main"]/*[@class="item-info"]/*[@class="stab portability"]' \
+//  'This is supported on x and y and z only.'
+#[doc(inline, cfg(x))]
+#[doc(cfg(y), cfg(z))]
+pub fn multiple_attrs() {}