From bbcda98d4107e462aae97d0b2e7c948a0d16f02b Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 5 Dec 2019 06:45:50 +0100 Subject: [PATCH] cfg_attr: avoid .outer_tokens --- src/librustc_parse/config.rs | 125 +++++++++++------- src/librustc_parse/lib.rs | 27 ++-- src/librustc_parse/parser/attr.rs | 12 +- .../conditional-compilation/cfg-attr-parse.rs | 16 ++- .../cfg-attr-parse.stderr | 64 ++++++++- .../ui/malformed/malformed-special-attrs.rs | 2 +- .../malformed/malformed-special-attrs.stderr | 8 +- 7 files changed, 178 insertions(+), 76 deletions(-) diff --git a/src/librustc_parse/config.rs b/src/librustc_parse/config.rs index 30e056e52d2..b81111db95f 100644 --- 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 \ + "; + impl<'a> StripUnconfigured<'a> { pub fn configure(&mut self, mut node: T) -> Option { 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 { + fn process_cfg_attr(&mut self, attr: Attribute) -> Vec { 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 \ - ") - .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, - ))) - .collect() - } else { - vec![] + 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() + } + + 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,7 +234,7 @@ 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); @@ -207,7 +242,7 @@ impl<'a> StripUnconfigured<'a> { } /// 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) { + attrs: &[Attribute], +) -> (bool, Vec) { // 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 index a222f3f00c4..f1967372f4d 100644 --- a/src/librustc_parse/lib.rs +++ b/src/librustc_parse/lib.rs @@ -270,21 +270,13 @@ 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()?; @@ -292,6 +284,17 @@ pub fn parse_in_attr<'a, T>( 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 index b2ae934ce64..b2030a4570e 100644 --- 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 index 93aef72220c..8ca31c11836 100644 --- 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 index 3dfbd6df256..3a590d3282d 100644 --- 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 -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 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 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 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 -error: aborting due to 5 previous errors +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 + +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 + +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 index e67fbdd5ddd..05b7ebe4666 100644 --- a/src/test/ui/malformed/malformed-special-attrs.rs +++ b/src/test/ui/malformed/malformed-special-attrs.rs @@ -1,7 +1,7 @@ #[cfg_attr] //~ ERROR malformed `cfg_attr` attribute struct S1; -#[cfg_attr = ""] //~ ERROR expected `(`, found `=` +#[cfg_attr = ""] //~ ERROR malformed `cfg_attr` attribute struct S2; #[derive] //~ ERROR malformed `derive` attribute diff --git a/src/test/ui/malformed/malformed-special-attrs.stderr b/src/test/ui/malformed/malformed-special-attrs.stderr index 319c05eadbf..6f535e03e6a 100644 --- a/src/test/ui/malformed/malformed-special-attrs.stderr +++ b/src/test/ui/malformed/malformed-special-attrs.stderr @@ -6,11 +6,13 @@ LL | #[cfg_attr] | = note: for more information, visit -error: expected `(`, found `=` - --> $DIR/malformed-special-attrs.rs:4:12 +error: malformed `cfg_attr` attribute input + --> $DIR/malformed-special-attrs.rs:4:1 | LL | #[cfg_attr = ""] - | ^ expected `(` + | ^^^^^^^^^^^^^^^^ help: missing condition and attribute: `#[cfg_attr(condition, attribute, other_attribute, ...)]` + | + = note: for more information, visit error: malformed `derive` attribute input --> $DIR/malformed-special-attrs.rs:7:1