From cbc9f683125b4eb21eb3137bbd2c5295650eeaa5 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 5 Dec 2019 13:53:56 +0100 Subject: [PATCH] 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 f1967372f4d0..9a7b32402534 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 70c3458e7c02..5334fc485e7a 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 8e56e2bb00b4..ad2281a58791 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 a6d886318e82..77fa2f566a8f 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 8d750b668384..1f1ee39b049e 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 b077d590915c..713b9eb542cf 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 39b3edc67033..21683b2fb864 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