From 99191c2e717883bfec51b49df0e412a34849fc4a Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 5 Dec 2019 14:19:00 +0100 Subject: [PATCH] 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