cfg_attr: avoid .outer_tokens

Centril · Centril · commit bbcda98d4107 · 2019-12-06T20:37:59.000+01:00
diff --git a/src/librustc_parse/config.rs b/src/librustc_parse/config.rs
@@ -8,18 +8,20 @@
 //!
 //! [#64197]: https://github.com/rust-lang/rust/issues/64197
 
-use crate::validate_attr;
+use crate::{parse_in, validate_attr};
 use rustc_feature::Features;
 use rustc_errors::Applicability;
 use syntax::attr::HasAttrs;
 use syntax::feature_gate::{feature_err, get_features};
 use syntax::attr;
-use syntax::ast;
+use syntax::ast::{self, Attribute, AttrItem, MetaItem};
 use syntax::edition::Edition;
 use syntax::mut_visit::*;
 use syntax::ptr::P;
+use syntax::tokenstream::DelimSpan;
 use syntax::sess::ParseSess;
 use syntax::util::map_in_place::MapInPlace;
+use syntax_pos::Span;
 use syntax_pos::symbol::sym;
 
 use smallvec::SmallVec;
@@ -72,6 +74,11 @@ macro_rules! configure {
     }
 }
 
+const CFG_ATTR_GRAMMAR_HELP: &str = "#[cfg_attr(condition, attribute, other_attribute, ...)]";
+const CFG_ATTR_NOTE_REF: &str = "for more information, visit \
+    <https://doc.rust-lang.org/reference/conditional-compilation.html\
+    #the-cfg_attr-attribute>";
+
 impl<'a> StripUnconfigured<'a> {
     pub fn configure<T: HasAttrs>(&mut self, mut node: T) -> Option<T> {
         self.process_cfg_attrs(&mut node);
@@ -97,34 +104,14 @@ impl<'a> StripUnconfigured<'a> {
     /// Gives a compiler warning when the `cfg_attr` contains no attributes and
     /// is in the original source file. Gives a compiler error if the syntax of
     /// the attribute is incorrect.
-    fn process_cfg_attr(&mut self, attr: ast::Attribute) -> Vec<ast::Attribute> {
+    fn process_cfg_attr(&mut self, attr: Attribute) -> Vec<Attribute> {
         if !attr.has_name(sym::cfg_attr) {
             return vec![attr];
         }
-        if let ast::MacArgs::Empty = attr.get_normal_item().args {
-            self.sess.span_diagnostic
-                .struct_span_err(
-                    attr.span,
-                    "malformed `cfg_attr` attribute input",
-                ).span_suggestion(
-                    attr.span,
-                    "missing condition and attribute",
-                    "#[cfg_attr(condition, attribute, other_attribute, ...)]".to_owned(),
-                    Applicability::HasPlaceholders,
-                ).note("for more information, visit \
-                       <https://doc.rust-lang.org/reference/conditional-compilation.html\
-                       #the-cfg_attr-attribute>")
-                .emit();
-            return vec![];
-        }
 
-        let res = crate::parse_in_attr(self.sess, &attr, |p| p.parse_cfg_attr());
-        let (cfg_predicate, expanded_attrs) = match res {
-            Ok(result) => result,
-            Err(mut e) => {
-                e.emit();
-                return vec![];
-            }
+        let (cfg_predicate, expanded_attrs) = match self.parse_cfg_attr(&attr) {
+            None => return vec![],
+            Some(r) => r,
         };
 
         // Lint on zero attributes in source.
@@ -135,24 +122,72 @@ impl<'a> StripUnconfigured<'a> {
         // At this point we know the attribute is considered used.
         attr::mark_used(&attr);
 
-        if attr::cfg_matches(&cfg_predicate, self.sess, self.features) {
-            // We call `process_cfg_attr` recursively in case there's a
-            // `cfg_attr` inside of another `cfg_attr`. E.g.
-            //  `#[cfg_attr(false, cfg_attr(true, some_attr))]`.
-            expanded_attrs.into_iter()
-            .flat_map(|(item, span)| self.process_cfg_attr(attr::mk_attr_from_item(
-                attr.style,
-                item,
-                span,
-            )))
+        if !attr::cfg_matches(&cfg_predicate, self.sess, self.features) {
+            return vec![];
+        }
+
+        // We call `process_cfg_attr` recursively in case there's a
+        // `cfg_attr` inside of another `cfg_attr`. E.g.
+        //  `#[cfg_attr(false, cfg_attr(true, some_attr))]`.
+        expanded_attrs
+            .into_iter()
+            .flat_map(|(item, span)| {
+                let attr = attr::mk_attr_from_item(attr.style, item, span);
+                self.process_cfg_attr(attr)
+            })
             .collect()
-        } else {
-            vec![]
+    }
+
+    fn parse_cfg_attr(&self, attr: &Attribute) -> Option<(MetaItem, Vec<(AttrItem, Span)>)> {
+        match &attr.get_normal_item().args {
+            ast::MacArgs::Delimited(dspan, delim, tts) if !tts.is_empty() => {
+                if let ast::MacDelimiter::Brace | ast::MacDelimiter::Bracket = delim {
+                    self.error_malformed_cfg_attr_wrong_delim(*dspan);
+                }
+                match parse_in(self.sess, tts.clone(), "`cfg_attr` input", |p| p.parse_cfg_attr()) {
+                    Ok(r) => return Some(r),
+                    Err(mut e) => e
+                        .help(&format!("the valid syntax is `{}`", CFG_ATTR_GRAMMAR_HELP))
+                        .note(CFG_ATTR_NOTE_REF)
+                        .emit(),
+                }
+            }
+            _ => self.error_malformed_cfg_attr_missing(attr.span),
         }
+        None
+    }
+
+    fn error_malformed_cfg_attr_wrong_delim(&self, dspan: DelimSpan) {
+        self.sess
+            .span_diagnostic
+            .struct_span_err(dspan.entire(), "wrong `cfg_attr` delimiters")
+            .multipart_suggestion(
+                "the delimiters should be `(` and `)`",
+                vec![
+                    (dspan.open, "(".to_string()),
+                    (dspan.close, ")".to_string()),
+                ],
+                Applicability::MachineApplicable,
+            )
+            .emit();
+    }
+
+    fn error_malformed_cfg_attr_missing(&self, span: Span) {
+        self.sess
+            .span_diagnostic
+            .struct_span_err(span, "malformed `cfg_attr` attribute input")
+            .span_suggestion(
+                span,
+                "missing condition and attribute",
+                CFG_ATTR_GRAMMAR_HELP.to_string(),
+                Applicability::HasPlaceholders,
+            )
+            .note(CFG_ATTR_NOTE_REF)
+            .emit();
     }
 
     /// Determines if a node with the given attributes should be included in this configuration.
-    pub fn in_cfg(&self, attrs: &[ast::Attribute]) -> bool {
+    pub fn in_cfg(&self, attrs: &[Attribute]) -> bool {
         attrs.iter().all(|attr| {
             if !is_cfg(attr) {
                 return true;
@@ -199,15 +234,15 @@ impl<'a> StripUnconfigured<'a> {
     }
 
     /// Visit attributes on expression and statements (but not attributes on items in blocks).
-    fn visit_expr_attrs(&mut self, attrs: &[ast::Attribute]) {
+    fn visit_expr_attrs(&mut self, attrs: &[Attribute]) {
         // flag the offending attributes
         for attr in attrs.iter() {
             self.maybe_emit_expr_attr_err(attr);
         }
     }
 
     /// If attributes are not allowed on expressions, emit an error for `attr`
-    pub fn maybe_emit_expr_attr_err(&self, attr: &ast::Attribute) {
+    pub fn maybe_emit_expr_attr_err(&self, attr: &Attribute) {
         if !self.features.map(|features| features.stmt_expr_attributes).unwrap_or(true) {
             let mut err = feature_err(self.sess,
                                       sym::stmt_expr_attributes,
@@ -350,7 +385,7 @@ impl<'a> MutVisitor for StripUnconfigured<'a> {
     }
 }
 
-fn is_cfg(attr: &ast::Attribute) -> bool {
+fn is_cfg(attr: &Attribute) -> bool {
     attr.check_name(sym::cfg)
 }
 
@@ -359,8 +394,8 @@ fn is_cfg(attr: &ast::Attribute) -> bool {
 pub fn process_configure_mod(
     sess: &ParseSess,
     cfg_mods: bool,
-    attrs: &[ast::Attribute],
-) -> (bool, Vec<ast::Attribute>) {
+    attrs: &[Attribute],
+) -> (bool, Vec<Attribute>) {
     // Don't perform gated feature checking.
     let mut strip_unconfigured = StripUnconfigured { sess, features: None };
     let mut attrs = attrs.to_owned();
diff --git a/src/librustc_parse/lib.rs b/src/librustc_parse/lib.rs
@@ -270,28 +270,31 @@ pub fn stream_to_parser_with_base_dir<'a>(
 }
 
 /// Runs the given subparser `f` on the tokens of the given `attr`'s item.
-pub fn parse_in_attr<'a, T>(
+pub fn parse_in<'a, T>(
     sess: &'a ParseSess,
-    attr: &ast::Attribute,
+    tts: TokenStream,
+    name: &'static str,
     mut f: impl FnMut(&mut Parser<'a>) -> PResult<'a, T>,
 ) -> PResult<'a, T> {
-    let mut parser = Parser::new(
-        sess,
-        // FIXME(#66940, Centril | petrochenkov): refactor this function so it doesn't
-        // require reconstructing and immediately re-parsing delimiters.
-        attr.get_normal_item().args.outer_tokens(),
-        None,
-        false,
-        false,
-        Some("attribute"),
-    );
+    let mut parser = Parser::new(sess, tts, None, false, false, Some(name));
     let result = f(&mut parser)?;
     if parser.token != token::Eof {
         parser.unexpected()?;
     }
     Ok(result)
 }
 
+/// Runs the given subparser `f` on the tokens of the given `attr`'s item.
+pub fn parse_in_attr<'a, T>(
+    sess: &'a ParseSess,
+    attr: &ast::Attribute,
+    f: impl FnMut(&mut Parser<'a>) -> PResult<'a, T>,
+) -> PResult<'a, T> {
+    // FIXME(#66940, Centril | petrochenkov): refactor this function so it doesn't
+    // require reconstructing and immediately re-parsing delimiters.
+    parse_in(sess, attr.get_normal_item().args.outer_tokens(), "attribute", f)
+}
+
 // NOTE(Centril): The following probably shouldn't be here but it acknowledges the
 // fact that architecturally, we are using parsing (read on below to understand why).
 
diff --git a/src/librustc_parse/parser/attr.rs b/src/librustc_parse/parser/attr.rs
@@ -238,22 +238,18 @@ impl<'a> Parser<'a> {
 
     /// Parses `cfg_attr(pred, attr_item_list)` where `attr_item_list` is comma-delimited.
     pub fn parse_cfg_attr(&mut self) -> PResult<'a, (ast::MetaItem, Vec<(ast::AttrItem, Span)>)> {
-        self.expect(&token::OpenDelim(token::Paren))?;
-
         let cfg_predicate = self.parse_meta_item()?;
         self.expect(&token::Comma)?;
 
         // Presumably, the majority of the time there will only be one attr.
         let mut expanded_attrs = Vec::with_capacity(1);
-
-        while !self.check(&token::CloseDelim(token::Paren)) {
-            let lo = self.token.span.lo();
+        while self.token.kind != token::Eof {
+            let lo = self.token.span;
             let item = self.parse_attr_item()?;
-            expanded_attrs.push((item, self.prev_span.with_lo(lo)));
-            self.expect_one_of(&[token::Comma], &[token::CloseDelim(token::Paren)])?;
+            expanded_attrs.push((item, lo.to(self.prev_span)));
+            self.eat(&token::Comma);
         }
 
-        self.expect(&token::CloseDelim(token::Paren))?;
         Ok((cfg_predicate, expanded_attrs))
     }
 
diff --git a/src/test/ui/conditional-compilation/cfg-attr-parse.rs b/src/test/ui/conditional-compilation/cfg-attr-parse.rs
@@ -1,11 +1,11 @@
 // Parse `cfg_attr` with varying numbers of attributes and trailing commas
 
 // Completely empty `cfg_attr` input
-#[cfg_attr()] //~ error: expected identifier, found `)`
+#[cfg_attr()] //~ error: malformed `cfg_attr` attribute input
 struct NoConfigurationPredicate;
 
 // Zero attributes, zero trailing comma (comma manatory here)
-#[cfg_attr(all())] //~ error: expected `,`, found `)`
+#[cfg_attr(all())] //~ error: expected `,`, found end of `cfg_attr`
 struct A0C0;
 
 // Zero attributes, one trailing comma
@@ -40,4 +40,16 @@ struct A2C1;
 #[cfg_attr(all(), must_use, deprecated,,)] //~ ERROR expected identifier
 struct A2C2;
 
+// Wrong delimiter `[`
+#[cfg_attr[all(),,]]
+//~^ ERROR wrong `cfg_attr` delimiters
+//~| ERROR expected identifier, found `,`
+struct BracketZero;
+
+// Wrong delimiter `{`
+#[cfg_attr{all(),,}]
+//~^ ERROR wrong `cfg_attr` delimiters
+//~| ERROR expected identifier, found `,`
+struct BraceZero;
+
 fn main() {}
diff --git a/src/test/ui/conditional-compilation/cfg-attr-parse.stderr b/src/test/ui/conditional-compilation/cfg-attr-parse.stderr
@@ -1,32 +1,86 @@
-error: expected identifier, found `)`
-  --> $DIR/cfg-attr-parse.rs:4:12
+error: malformed `cfg_attr` attribute input
+  --> $DIR/cfg-attr-parse.rs:4:1
    |
 LL | #[cfg_attr()]
-   |            ^ expected identifier
+   | ^^^^^^^^^^^^^ help: missing condition and attribute: `#[cfg_attr(condition, attribute, other_attribute, ...)]`
+   |
+   = note: for more information, visit <https://doc.rust-lang.org/reference/conditional-compilation.html#the-cfg_attr-attribute>
 
-error: expected `,`, found `)`
+error: expected `,`, found end of `cfg_attr` input
   --> $DIR/cfg-attr-parse.rs:8:17
    |
 LL | #[cfg_attr(all())]
    |                 ^ expected `,`
+   |
+   = help: the valid syntax is `#[cfg_attr(condition, attribute, other_attribute, ...)]`
+   = note: for more information, visit <https://doc.rust-lang.org/reference/conditional-compilation.html#the-cfg_attr-attribute>
 
 error: expected identifier, found `,`
   --> $DIR/cfg-attr-parse.rs:16:18
    |
 LL | #[cfg_attr(all(),,)]
    |                  ^ expected identifier
+   |
+   = help: the valid syntax is `#[cfg_attr(condition, attribute, other_attribute, ...)]`
+   = note: for more information, visit <https://doc.rust-lang.org/reference/conditional-compilation.html#the-cfg_attr-attribute>
 
 error: expected identifier, found `,`
   --> $DIR/cfg-attr-parse.rs:28:28
    |
 LL | #[cfg_attr(all(), must_use,,)]
    |                            ^ expected identifier
+   |
+   = help: the valid syntax is `#[cfg_attr(condition, attribute, other_attribute, ...)]`
+   = note: for more information, visit <https://doc.rust-lang.org/reference/conditional-compilation.html#the-cfg_attr-attribute>
 
 error: expected identifier, found `,`
   --> $DIR/cfg-attr-parse.rs:40:40
    |
 LL | #[cfg_attr(all(), must_use, deprecated,,)]
    |                                        ^ expected identifier
+   |
+   = help: the valid syntax is `#[cfg_attr(condition, attribute, other_attribute, ...)]`
+   = note: for more information, visit <https://doc.rust-lang.org/reference/conditional-compilation.html#the-cfg_attr-attribute>
+
+error: wrong `cfg_attr` delimiters
+  --> $DIR/cfg-attr-parse.rs:44:11
+   |
+LL | #[cfg_attr[all(),,]]
+   |           ^^^^^^^^^
+   |
+help: the delimiters should be `(` and `)`
+   |
+LL | #[cfg_attr(all(),,)]
+   |           ^       ^
+
+error: expected identifier, found `,`
+  --> $DIR/cfg-attr-parse.rs:44:18
+   |
+LL | #[cfg_attr[all(),,]]
+   |                  ^ expected identifier
+   |
+   = help: the valid syntax is `#[cfg_attr(condition, attribute, other_attribute, ...)]`
+   = note: for more information, visit <https://doc.rust-lang.org/reference/conditional-compilation.html#the-cfg_attr-attribute>
+
+error: wrong `cfg_attr` delimiters
+  --> $DIR/cfg-attr-parse.rs:50:11
+   |
+LL | #[cfg_attr{all(),,}]
+   |           ^^^^^^^^^
+   |
+help: the delimiters should be `(` and `)`
+   |
+LL | #[cfg_attr(all(),,)]
+   |           ^       ^
+
+error: expected identifier, found `,`
+  --> $DIR/cfg-attr-parse.rs:50:18
+   |
+LL | #[cfg_attr{all(),,}]
+   |                  ^ expected identifier
+   |
+   = help: the valid syntax is `#[cfg_attr(condition, attribute, other_attribute, ...)]`
+   = note: for more information, visit <https://doc.rust-lang.org/reference/conditional-compilation.html#the-cfg_attr-attribute>
 
-error: aborting due to 5 previous errors
+error: aborting due to 9 previous errors
 
diff --git a/src/test/ui/malformed/malformed-special-attrs.rs b/src/test/ui/malformed/malformed-special-attrs.rs
diff --git a/src/test/ui/malformed/malformed-special-attrs.stderr b/src/test/ui/malformed/malformed-special-attrs.stderr
