From bbcda98d4107e462aae97d0b2e7c948a0d16f02b Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 5 Dec 2019 06:45:50 +0100 Subject: [PATCH 1/3] cfg_attr: avoid .outer_tokens --- src/librustc_parse/config.rs | 123 +++++++++++------- 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, 177 insertions(+), 75 deletions(-) diff --git a/src/librustc_parse/config.rs b/src/librustc_parse/config.rs index 30e056e52d25a..b81111db95fcd 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, - ))) + 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,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 a222f3f00c463..f1967372f4d09 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 b2ae934ce6474..b2030a4570ef9 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 93aef72220cc4..8ca31c118369c 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 3dfbd6df256eb..3a590d3282d46 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: 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 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 index e67fbdd5ddd32..05b7ebe466662 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 319c05eadbf1d..6f535e03e6aec 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 From cbc9f683125b4eb21eb3137bbd2c5295650eeaa5 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 5 Dec 2019 13:53:56 +0100 Subject: [PATCH 2/3] derive: avoid parse_in_attr --- src/librustc_parse/lib.rs | 2 +- src/librustc_parse/parser/path.rs | 37 -------- src/libsyntax_expand/proc_macro.rs | 87 +++++++++++++------ .../ui/malformed/malformed-derive-entry.rs | 8 +- .../malformed/malformed-derive-entry.stderr | 27 ++++-- src/test/ui/span/macro-ty-params.rs | 1 - src/test/ui/span/macro-ty-params.stderr | 8 +- 7 files changed, 90 insertions(+), 80 deletions(-) diff --git a/src/librustc_parse/lib.rs b/src/librustc_parse/lib.rs index f1967372f4d09..9a7b32402534e 100644 --- a/src/librustc_parse/lib.rs +++ b/src/librustc_parse/lib.rs @@ -285,7 +285,7 @@ pub fn parse_in<'a, T>( } /// Runs the given subparser `f` on the tokens of the given `attr`'s item. -pub fn parse_in_attr<'a, T>( +fn parse_in_attr<'a, T>( sess: &'a ParseSess, attr: &ast::Attribute, f: impl FnMut(&mut Parser<'a>) -> PResult<'a, T>, diff --git a/src/librustc_parse/parser/path.rs b/src/librustc_parse/parser/path.rs index 70c3458e7c020..5334fc485e7a6 100644 --- a/src/librustc_parse/parser/path.rs +++ b/src/librustc_parse/parser/path.rs @@ -3,7 +3,6 @@ use crate::maybe_whole; use rustc_errors::{PResult, Applicability, pluralize}; use syntax::ast::{self, QSelf, Path, PathSegment, Ident, ParenthesizedArgs, AngleBracketedArgs}; use syntax::ast::{AnonConst, GenericArg, AssocTyConstraint, AssocTyConstraintKind, BlockCheckMode}; -use syntax::ast::MacArgs; use syntax::ThinVec; use syntax::token::{self, Token}; use syntax_pos::source_map::{Span, BytePos}; @@ -109,42 +108,6 @@ impl<'a> Parser<'a> { Ok(Path { segments, span: lo.to(self.prev_span) }) } - /// Like `parse_path`, but also supports parsing `Word` meta items into paths for - /// backwards-compatibility. This is used when parsing derive macro paths in `#[derive]` - /// attributes. - fn parse_path_allowing_meta(&mut self, style: PathStyle) -> PResult<'a, Path> { - let meta_ident = match self.token.kind { - token::Interpolated(ref nt) => match **nt { - token::NtMeta(ref item) => match item.args { - MacArgs::Empty => Some(item.path.clone()), - _ => None, - }, - _ => None, - }, - _ => None, - }; - if let Some(path) = meta_ident { - self.bump(); - return Ok(path); - } - self.parse_path(style) - } - - /// Parse a list of paths inside `#[derive(path_0, ..., path_n)]`. - pub fn parse_derive_paths(&mut self) -> PResult<'a, Vec> { - self.expect(&token::OpenDelim(token::Paren))?; - let mut list = Vec::new(); - while !self.eat(&token::CloseDelim(token::Paren)) { - let path = self.parse_path_allowing_meta(PathStyle::Mod)?; - list.push(path); - if !self.eat(&token::Comma) { - self.expect(&token::CloseDelim(token::Paren))?; - break - } - } - Ok(list) - } - pub(super) fn parse_path_segments( &mut self, segments: &mut Vec, diff --git a/src/libsyntax_expand/proc_macro.rs b/src/libsyntax_expand/proc_macro.rs index 8e56e2bb00b4b..ad2281a587910 100644 --- a/src/libsyntax_expand/proc_macro.rs +++ b/src/libsyntax_expand/proc_macro.rs @@ -1,7 +1,7 @@ use crate::base::{self, *}; use crate::proc_macro_server; -use syntax::ast::{self, ItemKind, MacArgs}; +use syntax::ast::{self, ItemKind, MetaItemKind, NestedMetaItem}; use syntax::errors::{Applicability, FatalError}; use syntax::symbol::sym; use syntax::token; @@ -171,34 +171,71 @@ crate fn collect_derives(cx: &mut ExtCtxt<'_>, attrs: &mut Vec) if !attr.has_name(sym::derive) { return true; } - if !attr.is_meta_item_list() { - cx.struct_span_err(attr.span, "malformed `derive` attribute input") - .span_suggestion( - attr.span, - "missing traits to be derived", - "#[derive(Trait1, Trait2, ...)]".to_owned(), - Applicability::HasPlaceholders, - ).emit(); - return false; - } - let parse_derive_paths = |attr: &ast::Attribute| { - if let MacArgs::Empty = attr.get_normal_item().args { - return Ok(Vec::new()); + // 1) First let's ensure that it's a meta item. + let nmis = match attr.meta_item_list() { + None => { + cx.struct_span_err(attr.span, "malformed `derive` attribute input") + .span_suggestion( + attr.span, + "missing traits to be derived", + "#[derive(Trait1, Trait2, ...)]".to_owned(), + Applicability::HasPlaceholders, + ) + .emit(); + return false; } - rustc_parse::parse_in_attr(cx.parse_sess, attr, |p| p.parse_derive_paths()) + Some(x) => x, }; - match parse_derive_paths(attr) { - Ok(traits) => { - result.extend(traits); - true - } - Err(mut e) => { - e.emit(); - false - } - } + let mut retain_in_fm = true; + let mut retain_in_map = true; + let traits = nmis + .into_iter() + // 2) Moreover, let's ensure we have a path and not `#[derive("foo")]`. + .filter_map(|nmi| match nmi { + NestedMetaItem::Literal(lit) => { + retain_in_fm = false; + cx.struct_span_err(lit.span, "expected path to a trait, found literal") + .help("for example, write `#[derive(Debug)]` for `Debug`") + .emit(); + None + } + NestedMetaItem::MetaItem(mi) => Some(mi), + }) + // 3) Finally, we only accept `#[derive($path_0, $path_1, ..)]` + // but not e.g. `#[derive($path_0 = "value", $path_1(abc))]`. + // In this case we can still at least determine that the user + // wanted this trait to be derived, so let's keep it. + .map(|mi| { + let mut traits_dont_accept = |title, action| { + retain_in_map = false; + let sp = mi.span.with_lo(mi.path.span.hi()); + cx.struct_span_err(sp, title) + .span_suggestion( + sp, + action, + String::new(), + Applicability::MachineApplicable, + ) + .emit(); + }; + match &mi.kind { + MetaItemKind::List(..) => traits_dont_accept( + "traits in `#[derive(...)]` don't accept arguments", + "remove the arguments", + ), + MetaItemKind::NameValue(..) => traits_dont_accept( + "traits in `#[derive(...)]` don't accept values", + "remove the value", + ), + MetaItemKind::Word => {} + } + mi.path + }); + + result.extend(traits); + retain_in_fm && retain_in_map }); result } diff --git a/src/test/ui/malformed/malformed-derive-entry.rs b/src/test/ui/malformed/malformed-derive-entry.rs index a6d886318e820..77fa2f566a8fc 100644 --- a/src/test/ui/malformed/malformed-derive-entry.rs +++ b/src/test/ui/malformed/malformed-derive-entry.rs @@ -1,7 +1,11 @@ -#[derive(Copy(Bad))] //~ ERROR expected one of `)`, `,`, or `::`, found `(` +#[derive(Copy(Bad))] +//~^ ERROR traits in `#[derive(...)]` don't accept arguments +//~| ERROR the trait bound struct Test1; -#[derive(Copy="bad")] //~ ERROR expected one of `)`, `,`, or `::`, found `=` +#[derive(Copy="bad")] +//~^ ERROR traits in `#[derive(...)]` don't accept values +//~| ERROR the trait bound struct Test2; #[derive] //~ ERROR malformed `derive` attribute input diff --git a/src/test/ui/malformed/malformed-derive-entry.stderr b/src/test/ui/malformed/malformed-derive-entry.stderr index 8d750b6683843..1f1ee39b049e3 100644 --- a/src/test/ui/malformed/malformed-derive-entry.stderr +++ b/src/test/ui/malformed/malformed-derive-entry.stderr @@ -1,20 +1,33 @@ -error: expected one of `)`, `,`, or `::`, found `(` +error: traits in `#[derive(...)]` don't accept arguments --> $DIR/malformed-derive-entry.rs:1:14 | LL | #[derive(Copy(Bad))] - | ^ expected one of `)`, `,`, or `::` + | ^^^^^ help: remove the arguments -error: expected one of `)`, `,`, or `::`, found `=` - --> $DIR/malformed-derive-entry.rs:4:14 +error: traits in `#[derive(...)]` don't accept values + --> $DIR/malformed-derive-entry.rs:6:14 | LL | #[derive(Copy="bad")] - | ^ expected one of `)`, `,`, or `::` + | ^^^^^^ help: remove the value error: malformed `derive` attribute input - --> $DIR/malformed-derive-entry.rs:7:1 + --> $DIR/malformed-derive-entry.rs:11:1 | LL | #[derive] | ^^^^^^^^^ help: missing traits to be derived: `#[derive(Trait1, Trait2, ...)]` -error: aborting due to 3 previous errors +error[E0277]: the trait bound `Test1: std::clone::Clone` is not satisfied + --> $DIR/malformed-derive-entry.rs:1:10 + | +LL | #[derive(Copy(Bad))] + | ^^^^ the trait `std::clone::Clone` is not implemented for `Test1` + +error[E0277]: the trait bound `Test2: std::clone::Clone` is not satisfied + --> $DIR/malformed-derive-entry.rs:6:10 + | +LL | #[derive(Copy="bad")] + | ^^^^ the trait `std::clone::Clone` is not implemented for `Test2` + +error: aborting due to 5 previous errors +For more information about this error, try `rustc --explain E0277`. diff --git a/src/test/ui/span/macro-ty-params.rs b/src/test/ui/span/macro-ty-params.rs index b077d590915cb..713b9eb542cfa 100644 --- a/src/test/ui/span/macro-ty-params.rs +++ b/src/test/ui/span/macro-ty-params.rs @@ -10,5 +10,4 @@ fn main() { foo::!(); //~ ERROR generic arguments in macro path foo::<>!(); //~ ERROR generic arguments in macro path m!(Default<>); //~ ERROR generic arguments in macro path - //~^ ERROR unexpected generic arguments in path } diff --git a/src/test/ui/span/macro-ty-params.stderr b/src/test/ui/span/macro-ty-params.stderr index 39b3edc67033d..21683b2fb8643 100644 --- a/src/test/ui/span/macro-ty-params.stderr +++ b/src/test/ui/span/macro-ty-params.stderr @@ -10,17 +10,11 @@ error: generic arguments in macro path LL | foo::<>!(); | ^^ -error: unexpected generic arguments in path - --> $DIR/macro-ty-params.rs:12:8 - | -LL | m!(Default<>); - | ^^^^^^^^^ - error: generic arguments in macro path --> $DIR/macro-ty-params.rs:12:15 | LL | m!(Default<>); | ^^ -error: aborting due to 4 previous errors +error: aborting due to 3 previous errors From 99191c2e717883bfec51b49df0e412a34849fc4a Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 5 Dec 2019 14:19:00 +0100 Subject: [PATCH 3/3] parse_meta: ditch parse_in_attr --- src/librustc_parse/config.rs | 25 ++---------- src/librustc_parse/lib.rs | 11 ------ src/librustc_parse/parser/attr.rs | 19 +++++++++- src/librustc_parse/validate_attr.rs | 38 +++++++++++++++++-- src/libsyntax_expand/proc_macro.rs | 10 ++--- src/test/ui/malformed/malformed-meta-delim.rs | 11 ++++++ .../ui/malformed/malformed-meta-delim.stderr | 24 ++++++++++++ .../expected-comma-found-token.rs | 2 +- .../expected-comma-found-token.stderr | 7 +--- 9 files changed, 99 insertions(+), 48 deletions(-) create mode 100644 src/test/ui/malformed/malformed-meta-delim.rs create mode 100644 src/test/ui/malformed/malformed-meta-delim.stderr diff --git a/src/librustc_parse/config.rs b/src/librustc_parse/config.rs index b81111db95fcd..072f6845853a8 100644 --- a/src/librustc_parse/config.rs +++ b/src/librustc_parse/config.rs @@ -18,7 +18,6 @@ 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; @@ -139,11 +138,10 @@ impl<'a> StripUnconfigured<'a> { } 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 attr.get_normal_item().args { + ast::MacArgs::Delimited(dspan, delim, ref tts) if !tts.is_empty() => { + let msg = "wrong `cfg_attr` delimiters"; + validate_attr::check_meta_bad_delim(self.sess, dspan, delim, msg); match parse_in(self.sess, tts.clone(), "`cfg_attr` input", |p| p.parse_cfg_attr()) { Ok(r) => return Some(r), Err(mut e) => e @@ -157,21 +155,6 @@ impl<'a> StripUnconfigured<'a> { 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 diff --git a/src/librustc_parse/lib.rs b/src/librustc_parse/lib.rs index 9a7b32402534e..72436f29244cf 100644 --- a/src/librustc_parse/lib.rs +++ b/src/librustc_parse/lib.rs @@ -284,17 +284,6 @@ pub fn parse_in<'a, T>( Ok(result) } -/// Runs the given subparser `f` on the tokens of the given `attr`'s item. -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 b2030a4570ef9..00fd6b8a25bc3 100644 --- a/src/librustc_parse/parser/attr.rs +++ b/src/librustc_parse/parser/attr.rs @@ -220,7 +220,7 @@ impl<'a> Parser<'a> { Ok(attrs) } - pub(super) fn parse_unsuffixed_lit(&mut self) -> PResult<'a, ast::Lit> { + crate fn parse_unsuffixed_lit(&mut self) -> PResult<'a, ast::Lit> { let lit = self.parse_lit()?; debug!("checking if {:?} is unusuffixed", lit); @@ -247,12 +247,27 @@ impl<'a> Parser<'a> { let lo = self.token.span; let item = self.parse_attr_item()?; expanded_attrs.push((item, lo.to(self.prev_span))); - self.eat(&token::Comma); + if !self.eat(&token::Comma) { + break; + } } Ok((cfg_predicate, expanded_attrs)) } + /// Matches `COMMASEP(meta_item_inner)`. + crate fn parse_meta_seq_top(&mut self) -> PResult<'a, Vec> { + // Presumably, the majority of the time there will only be one attr. + let mut nmis = Vec::with_capacity(1); + while self.token.kind != token::Eof { + nmis.push(self.parse_meta_item_inner()?); + if !self.eat(&token::Comma) { + break; + } + } + Ok(nmis) + } + /// Matches the following grammar (per RFC 1559). /// /// meta_item : PATH ( '=' UNSUFFIXED_LIT | '(' meta_item_inner? ')' )? ; diff --git a/src/librustc_parse/validate_attr.rs b/src/librustc_parse/validate_attr.rs index 97e9cb8dcdf6f..94d3fe7b55167 100644 --- a/src/librustc_parse/validate_attr.rs +++ b/src/librustc_parse/validate_attr.rs @@ -1,10 +1,13 @@ //! Meta-syntax validation logic of attributes for post-expansion. +use crate::parse_in; + use rustc_errors::{PResult, Applicability}; use rustc_feature::{AttributeTemplate, BUILTIN_ATTRIBUTE_MAP}; -use syntax::ast::{self, Attribute, AttrKind, Ident, MacArgs, MetaItem, MetaItemKind}; +use syntax::ast::{self, Attribute, AttrKind, Ident, MacArgs, MacDelimiter, MetaItem, MetaItemKind}; use syntax::attr::mk_name_value_item_str; use syntax::early_buffered_lints::ILL_FORMED_ATTRIBUTE_INPUT; +use syntax::tokenstream::DelimSpan; use syntax::sess::ParseSess; use syntax_pos::{Symbol, sym}; @@ -27,9 +30,20 @@ pub fn check_meta(sess: &ParseSess, attr: &Attribute) { pub fn parse_meta<'a>(sess: &'a ParseSess, attr: &Attribute) -> PResult<'a, MetaItem> { Ok(match attr.kind { AttrKind::Normal(ref item) => MetaItem { - path: item.path.clone(), - kind: super::parse_in_attr(sess, attr, |p| p.parse_meta_item_kind())?, span: attr.span, + path: item.path.clone(), + kind: match &attr.get_normal_item().args { + MacArgs::Empty => MetaItemKind::Word, + MacArgs::Eq(_, t) => { + let v = parse_in(sess, t.clone(), "name value", |p| p.parse_unsuffixed_lit())?; + MetaItemKind::NameValue(v) + } + MacArgs::Delimited(dspan, delim, t) => { + check_meta_bad_delim(sess, *dspan, *delim, "wrong meta list delimiters"); + let nmis = parse_in(sess, t.clone(), "meta list", |p| p.parse_meta_seq_top())?; + MetaItemKind::List(nmis) + } + } }, AttrKind::DocComment(comment) => { mk_name_value_item_str(Ident::new(sym::doc, attr.span), comment, attr.span) @@ -37,6 +51,24 @@ pub fn parse_meta<'a>(sess: &'a ParseSess, attr: &Attribute) -> PResult<'a, Meta }) } +crate fn check_meta_bad_delim(sess: &ParseSess, span: DelimSpan, delim: MacDelimiter, msg: &str) { + if let ast::MacDelimiter::Parenthesis = delim { + return; + } + + sess.span_diagnostic + .struct_span_err(span.entire(), msg) + .multipart_suggestion( + "the delimiters should be `(` and `)`", + vec![ + (span.open, "(".to_string()), + (span.close, ")".to_string()), + ], + Applicability::MachineApplicable, + ) + .emit(); +} + /// Checks that the given meta-item is compatible with this `AttributeTemplate`. fn is_attr_template_compatible(template: &AttributeTemplate, meta: &ast::MetaItemKind) -> bool { match meta { diff --git a/src/libsyntax_expand/proc_macro.rs b/src/libsyntax_expand/proc_macro.rs index ad2281a587910..520488c658676 100644 --- a/src/libsyntax_expand/proc_macro.rs +++ b/src/libsyntax_expand/proc_macro.rs @@ -188,14 +188,14 @@ crate fn collect_derives(cx: &mut ExtCtxt<'_>, attrs: &mut Vec) Some(x) => x, }; - let mut retain_in_fm = true; - let mut retain_in_map = true; + let mut error_reported_filter_map = false; + let mut error_reported_map = false; let traits = nmis .into_iter() // 2) Moreover, let's ensure we have a path and not `#[derive("foo")]`. .filter_map(|nmi| match nmi { NestedMetaItem::Literal(lit) => { - retain_in_fm = false; + error_reported_filter_map = true; cx.struct_span_err(lit.span, "expected path to a trait, found literal") .help("for example, write `#[derive(Debug)]` for `Debug`") .emit(); @@ -209,7 +209,7 @@ crate fn collect_derives(cx: &mut ExtCtxt<'_>, attrs: &mut Vec) // wanted this trait to be derived, so let's keep it. .map(|mi| { let mut traits_dont_accept = |title, action| { - retain_in_map = false; + error_reported_map = true; let sp = mi.span.with_lo(mi.path.span.hi()); cx.struct_span_err(sp, title) .span_suggestion( @@ -235,7 +235,7 @@ crate fn collect_derives(cx: &mut ExtCtxt<'_>, attrs: &mut Vec) }); result.extend(traits); - retain_in_fm && retain_in_map + !error_reported_filter_map && !error_reported_map }); result } diff --git a/src/test/ui/malformed/malformed-meta-delim.rs b/src/test/ui/malformed/malformed-meta-delim.rs new file mode 100644 index 0000000000000..5b1614b69a92b --- /dev/null +++ b/src/test/ui/malformed/malformed-meta-delim.rs @@ -0,0 +1,11 @@ +fn main() {} + +#[allow { foo_lint } ] +//~^ ERROR wrong meta list delimiters +//~| HELP the delimiters should be `(` and `)` +fn delim_brace() {} + +#[allow [ foo_lint ] ] +//~^ ERROR wrong meta list delimiters +//~| HELP the delimiters should be `(` and `)` +fn delim_bracket() {} diff --git a/src/test/ui/malformed/malformed-meta-delim.stderr b/src/test/ui/malformed/malformed-meta-delim.stderr new file mode 100644 index 0000000000000..407193d4adebb --- /dev/null +++ b/src/test/ui/malformed/malformed-meta-delim.stderr @@ -0,0 +1,24 @@ +error: wrong meta list delimiters + --> $DIR/malformed-meta-delim.rs:3:9 + | +LL | #[allow { foo_lint } ] + | ^^^^^^^^^^^^ + | +help: the delimiters should be `(` and `)` + | +LL | #[allow ( foo_lint ) ] + | ^ ^ + +error: wrong meta list delimiters + --> $DIR/malformed-meta-delim.rs:8:9 + | +LL | #[allow [ foo_lint ] ] + | ^^^^^^^^^^^^ + | +help: the delimiters should be `(` and `)` + | +LL | #[allow ( foo_lint ) ] + | ^ ^ + +error: aborting due to 2 previous errors + diff --git a/src/test/ui/on-unimplemented/expected-comma-found-token.rs b/src/test/ui/on-unimplemented/expected-comma-found-token.rs index 77c0ea17269f0..8fb34f21152ab 100644 --- a/src/test/ui/on-unimplemented/expected-comma-found-token.rs +++ b/src/test/ui/on-unimplemented/expected-comma-found-token.rs @@ -6,7 +6,7 @@ #[rustc_on_unimplemented( message="the message" - label="the label" //~ ERROR expected one of `)` or `,`, found `label` + label="the label" //~ ERROR expected `,`, found `label` )] trait T {} diff --git a/src/test/ui/on-unimplemented/expected-comma-found-token.stderr b/src/test/ui/on-unimplemented/expected-comma-found-token.stderr index 2e1d484e05a5a..048b72ee3bcdf 100644 --- a/src/test/ui/on-unimplemented/expected-comma-found-token.stderr +++ b/src/test/ui/on-unimplemented/expected-comma-found-token.stderr @@ -1,11 +1,8 @@ -error: expected one of `)` or `,`, found `label` +error: expected `,`, found `label` --> $DIR/expected-comma-found-token.rs:9:5 | LL | message="the message" - | - - | | - | expected one of `)` or `,` - | help: missing `,` + | - expected `,` LL | label="the label" | ^^^^^ unexpected token