From e0fe4be4657493fb1cd292911df0d9470d8c38e3 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Tue, 1 Oct 2019 12:11:38 +0200 Subject: [PATCH 1/7] syntax: cleanup associated const parsing. --- src/libsyntax/parse/parser/item.rs | 71 ++++++++++++++++-------------- 1 file changed, 39 insertions(+), 32 deletions(-) diff --git a/src/libsyntax/parse/parser/item.rs b/src/libsyntax/parse/parser/item.rs index 2ac0352764fa0..6a832d5f3014e 100644 --- a/src/libsyntax/parse/parser/item.rs +++ b/src/libsyntax/parse/parser/item.rs @@ -4,7 +4,7 @@ use crate::maybe_whole; use crate::ptr::P; use crate::ast::{ self, DUMMY_NODE_ID, Ident, Attribute, AttrStyle, - Item, ItemKind, ImplItem, TraitItem, TraitItemKind, + Item, ItemKind, ImplItem, ImplItemKind, TraitItem, TraitItemKind, UseTree, UseTreeKind, PathSegment, IsAuto, Constness, IsAsync, Unsafety, Defaultness, Visibility, VisibilityKind, Mutability, FnDecl, FnHeader, MethodSig, Block, @@ -727,16 +727,7 @@ impl<'a> Parser<'a> { }; (name, kind, generics) } else if self.is_const_item() { - // This parses the grammar: - // ImplItemConst = "const" Ident ":" Ty "=" Expr ";" - self.expect_keyword(kw::Const)?; - let name = self.parse_ident()?; - self.expect(&token::Colon)?; - let typ = self.parse_ty()?; - self.expect(&token::Eq)?; - let expr = self.parse_expr()?; - self.expect(&token::Semi)?; - (name, ast::ImplItemKind::Const(typ, expr), Generics::default()) + self.parse_impl_const()? } else { let (name, inner_attrs, generics, kind) = self.parse_impl_method(&vis, at_end)?; attrs.extend(inner_attrs); @@ -785,12 +776,25 @@ impl<'a> Parser<'a> { !self.is_keyword_ahead(1, &[kw::Fn, kw::Unsafe]) } + /// This parses the grammar: + /// ImplItemConst = "const" Ident ":" Ty "=" Expr ";" + fn parse_impl_const(&mut self) -> PResult<'a, (Ident, ImplItemKind, Generics)> { + self.expect_keyword(kw::Const)?; + let name = self.parse_ident()?; + self.expect(&token::Colon)?; + let typ = self.parse_ty()?; + self.expect(&token::Eq)?; + let expr = self.parse_expr()?; + self.expect(&token::Semi)?; + Ok((name, ImplItemKind::Const(typ, expr), Generics::default())) + } + /// Parses a method or a macro invocation in a trait impl. fn parse_impl_method( &mut self, vis: &Visibility, at_end: &mut bool - ) -> PResult<'a, (Ident, Vec, Generics, ast::ImplItemKind)> { + ) -> PResult<'a, (Ident, Vec, Generics, ImplItemKind)> { // FIXME: code copied from `parse_macro_use_or_failure` -- use abstraction! if let Some(mac) = self.parse_assoc_macro_invoc("impl", Some(vis), at_end)? { // method macro @@ -935,30 +939,20 @@ impl<'a> Parser<'a> { Ok(item) } - fn parse_trait_item_(&mut self, - at_end: &mut bool, - mut attrs: Vec) -> PResult<'a, TraitItem> { + fn parse_trait_item_( + &mut self, + at_end: &mut bool, + mut attrs: Vec, + ) -> PResult<'a, TraitItem> { let lo = self.token.span; self.eat_bad_pub(); let (name, kind, generics) = if self.eat_keyword(kw::Type) { self.parse_trait_item_assoc_ty()? } else if self.is_const_item() { - self.expect_keyword(kw::Const)?; - let ident = self.parse_ident()?; - self.expect(&token::Colon)?; - let ty = self.parse_ty()?; - let default = if self.eat(&token::Eq) { - let expr = self.parse_expr()?; - self.expect(&token::Semi)?; - Some(expr) - } else { - self.expect(&token::Semi)?; - None - }; - (ident, TraitItemKind::Const(ty, default), Generics::default()) + self.parse_trait_item_const()? } else if let Some(mac) = self.parse_assoc_macro_invoc("trait", None, &mut false)? { // trait item macro. - (Ident::invalid(), ast::TraitItemKind::Macro(mac), Generics::default()) + (Ident::invalid(), TraitItemKind::Macro(mac), Generics::default()) } else { // This is somewhat dubious; We don't want to allow // argument names to be left off if there is a definition... @@ -966,7 +960,7 @@ impl<'a> Parser<'a> { // We don't allow argument names to be left off in edition 2018. let (ident, sig, generics) = self.parse_method_sig(|t| t.span.rust_2018())?; let body = self.parse_trait_method_body(at_end, &mut attrs)?; - (ident, ast::TraitItemKind::Method(sig, body), generics) + (ident, TraitItemKind::Method(sig, body), generics) }; Ok(TraitItem { @@ -980,6 +974,20 @@ impl<'a> Parser<'a> { }) } + fn parse_trait_item_const(&mut self) -> PResult<'a, (Ident, TraitItemKind, Generics)> { + self.expect_keyword(kw::Const)?; + let ident = self.parse_ident()?; + self.expect(&token::Colon)?; + let ty = self.parse_ty()?; + let default = if self.eat(&token::Eq) { + Some(self.parse_expr()?) + } else { + None + }; + self.expect(&token::Semi)?; + Ok((ident, TraitItemKind::Const(ty, default), Generics::default())) + } + /// Parse the "body" of a method in a trait item definition. /// This can either be `;` when there's no body, /// or e.g. a block when the method is a provided one. @@ -1020,8 +1028,7 @@ impl<'a> Parser<'a> { /// Parses the following grammar: /// /// TraitItemAssocTy = Ident ["<"...">"] [":" [GenericBounds]] ["where" ...] ["=" Ty] - fn parse_trait_item_assoc_ty(&mut self) - -> PResult<'a, (Ident, TraitItemKind, Generics)> { + fn parse_trait_item_assoc_ty(&mut self) -> PResult<'a, (Ident, TraitItemKind, Generics)> { let ident = self.parse_ident()?; let mut generics = self.parse_generics()?; From 3bdbfbe8b9c78bcec9019e7452a2a9396ecc861e Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Tue, 1 Oct 2019 12:40:56 +0200 Subject: [PATCH 2/7] syntax: de-dups in item parsing. --- src/libsyntax/parse/parser/item.rs | 94 ++++++++++++++---------------- 1 file changed, 44 insertions(+), 50 deletions(-) diff --git a/src/libsyntax/parse/parser/item.rs b/src/libsyntax/parse/parser/item.rs index 6a832d5f3014e..95137f93b8810 100644 --- a/src/libsyntax/parse/parser/item.rs +++ b/src/libsyntax/parse/parser/item.rs @@ -98,7 +98,7 @@ impl<'a> Parser<'a> { let lo = self.token.span; - let visibility = self.parse_visibility(false)?; + let vis = self.parse_visibility(false)?; if self.eat_keyword(kw::Use) { // USE ITEM @@ -106,15 +106,14 @@ impl<'a> Parser<'a> { self.expect(&token::Semi)?; let span = lo.to(self.prev_span); - let item = - self.mk_item(span, Ident::invalid(), item_, visibility, attrs); + let item = self.mk_item(span, Ident::invalid(), item_, vis, attrs); return Ok(Some(item)); } if self.eat_keyword(kw::Extern) { let extern_sp = self.prev_span; if self.eat_keyword(kw::Crate) { - return Ok(Some(self.parse_item_extern_crate(lo, visibility, attrs)?)); + return Ok(Some(self.parse_item_extern_crate(lo, vis, attrs)?)); } let opt_abi = self.parse_opt_abi()?; @@ -128,10 +127,10 @@ impl<'a> Parser<'a> { constness: respan(fn_span, Constness::NotConst), abi: opt_abi.unwrap_or(Abi::C), }; - return self.parse_item_fn(lo, visibility, attrs, header); + return self.parse_item_fn(lo, vis, attrs, header); } else if self.check(&token::OpenDelim(token::Brace)) { return Ok(Some( - self.parse_item_foreign_mod(lo, opt_abi, visibility, attrs, extern_sp)?, + self.parse_item_foreign_mod(lo, opt_abi, vis, attrs, extern_sp)?, )); } @@ -142,10 +141,8 @@ impl<'a> Parser<'a> { self.bump(); // STATIC ITEM let m = self.parse_mutability(); - let (ident, item_, extra_attrs) = self.parse_item_const(Some(m))?; - let span = lo.to(self.prev_span); - let attrs = maybe_append(attrs, extra_attrs); - return Ok(Some(self.mk_item(span, ident, item_, visibility, attrs))); + let info = self.parse_item_const(Some(m)); + return self.mk_item_with_info(attrs, lo, vis, info); } if self.eat_keyword(kw::Const) { let const_span = self.prev_span; @@ -168,7 +165,7 @@ impl<'a> Parser<'a> { constness: respan(const_span, Constness::Const), abi, }; - return self.parse_item_fn(lo, visibility, attrs, header); + return self.parse_item_fn(lo, vis, attrs, header); } // CONST ITEM @@ -184,10 +181,8 @@ impl<'a> Parser<'a> { ) .emit(); } - let (ident, item_, extra_attrs) = self.parse_item_const(None)?; - let span = lo.to(self.prev_span); - let attrs = maybe_append(attrs, extra_attrs); - return Ok(Some(self.mk_item(span, ident, item_, visibility, attrs))); + let info = self.parse_item_const(None); + return self.mk_item_with_info(attrs, lo, vis, info); } // Parses `async unsafe? fn`. @@ -212,7 +207,7 @@ impl<'a> Parser<'a> { constness: respan(fn_span, Constness::NotConst), abi: Abi::Rust, }; - return self.parse_item_fn(lo, visibility, attrs, header); + return self.parse_item_fn(lo, vis, attrs, header); } } if self.check_keyword(kw::Unsafe) && @@ -227,10 +222,8 @@ impl<'a> Parser<'a> { self.expect_keyword(kw::Trait)?; IsAuto::Yes }; - let (ident, item_, extra_attrs) = self.parse_item_trait(is_auto, Unsafety::Unsafe)?; - let span = lo.to(self.prev_span); - let attrs = maybe_append(attrs, extra_attrs); - return Ok(Some(self.mk_item(span, ident, item_, visibility, attrs))); + let info = self.parse_item_trait(is_auto, Unsafety::Unsafe); + return self.mk_item_with_info(attrs, lo, vis, info); } if self.check_keyword(kw::Impl) || self.check_keyword(kw::Unsafe) && @@ -241,10 +234,8 @@ impl<'a> Parser<'a> { let defaultness = self.parse_defaultness(); let unsafety = self.parse_unsafety(); self.expect_keyword(kw::Impl)?; - let (ident, item_, extra_attrs) = self.parse_item_impl(unsafety, defaultness)?; - let span = lo.to(self.prev_span); - let attrs = maybe_append(attrs, extra_attrs); - return Ok(Some(self.mk_item(span, ident, item_, visibility, attrs))); + let info = self.parse_item_impl(unsafety, defaultness); + return self.mk_item_with_info(attrs, lo, vis, info); } if self.check_keyword(kw::Fn) { // FUNCTION ITEM @@ -256,7 +247,7 @@ impl<'a> Parser<'a> { constness: respan(fn_span, Constness::NotConst), abi: Abi::Rust, }; - return self.parse_item_fn(lo, visibility, attrs, header); + return self.parse_item_fn(lo, vis, attrs, header); } if self.check_keyword(kw::Unsafe) && self.look_ahead(1, |t| *t != token::OpenDelim(token::Brace)) { @@ -273,14 +264,12 @@ impl<'a> Parser<'a> { constness: respan(fn_span, Constness::NotConst), abi, }; - return self.parse_item_fn(lo, visibility, attrs, header); + return self.parse_item_fn(lo, vis, attrs, header); } if self.eat_keyword(kw::Mod) { // MODULE ITEM - let (ident, item_, extra_attrs) = self.parse_item_mod(&attrs[..])?; - let span = lo.to(self.prev_span); - let attrs = maybe_append(attrs, extra_attrs); - return Ok(Some(self.mk_item(span, ident, item_, visibility, attrs))); + let info = self.parse_item_mod(&attrs[..]); + return self.mk_item_with_info(attrs, lo, vis, info); } if let Some(type_) = self.eat_type() { let (ident, alias, generics) = type_?; @@ -290,14 +279,12 @@ impl<'a> Parser<'a> { AliasKind::OpaqueTy(bounds) => ItemKind::OpaqueTy(bounds, generics), }; let span = lo.to(self.prev_span); - return Ok(Some(self.mk_item(span, ident, item_, visibility, attrs))); + return Ok(Some(self.mk_item(span, ident, item_, vis, attrs))); } if self.eat_keyword(kw::Enum) { // ENUM ITEM - let (ident, item_, extra_attrs) = self.parse_item_enum()?; - let span = lo.to(self.prev_span); - let attrs = maybe_append(attrs, extra_attrs); - return Ok(Some(self.mk_item(span, ident, item_, visibility, attrs))); + let info = self.parse_item_enum(); + return self.mk_item_with_info(attrs, lo, vis, info); } if self.check_keyword(kw::Trait) || (self.check_keyword(kw::Auto) @@ -311,33 +298,27 @@ impl<'a> Parser<'a> { IsAuto::Yes }; // TRAIT ITEM - let (ident, item_, extra_attrs) = self.parse_item_trait(is_auto, Unsafety::Normal)?; - let span = lo.to(self.prev_span); - let attrs = maybe_append(attrs, extra_attrs); - return Ok(Some(self.mk_item(span, ident, item_, visibility, attrs))); + let info = self.parse_item_trait(is_auto, Unsafety::Normal); + return self.mk_item_with_info(attrs, lo, vis, info); } if self.eat_keyword(kw::Struct) { // STRUCT ITEM - let (ident, item_, extra_attrs) = self.parse_item_struct()?; - let span = lo.to(self.prev_span); - let attrs = maybe_append(attrs, extra_attrs); - return Ok(Some(self.mk_item(span, ident, item_, visibility, attrs))); + let info = self.parse_item_struct(); + return self.mk_item_with_info(attrs, lo, vis, info); } if self.is_union_item() { // UNION ITEM self.bump(); - let (ident, item_, extra_attrs) = self.parse_item_union()?; - let span = lo.to(self.prev_span); - let attrs = maybe_append(attrs, extra_attrs); - return Ok(Some(self.mk_item(span, ident, item_, visibility, attrs))); + let info = self.parse_item_union(); + return self.mk_item_with_info(attrs, lo, vis, info); } - if let Some(macro_def) = self.eat_macro_def(&attrs, &visibility, lo)? { + if let Some(macro_def) = self.eat_macro_def(&attrs, &vis, lo)? { return Ok(Some(macro_def)); } // Verify whether we have encountered a struct or method definition where the user forgot to // add the `struct` or `fn` keyword after writing `pub`: `pub S {}` - if visibility.node.is_pub() && + if vis.node.is_pub() && self.check_ident() && self.look_ahead(1, |t| *t != token::Not) { @@ -428,7 +409,20 @@ impl<'a> Parser<'a> { return Err(err); } } - self.parse_macro_use_or_failure(attrs, macros_allowed, attributes_allowed, lo, visibility) + self.parse_macro_use_or_failure(attrs, macros_allowed, attributes_allowed, lo, vis) + } + + fn mk_item_with_info( + &self, + attrs: Vec, + lo: Span, + vis: Visibility, + info: PResult<'a, ItemInfo>, + ) -> PResult<'a, Option>> { + let (ident, item, extra_attrs) = info?; + let span = lo.to(self.prev_span); + let attrs = maybe_append(attrs, extra_attrs); + Ok(Some(self.mk_item(span, ident, item, vis, attrs))) } fn recover_first_param(&mut self) -> &'static str { From 090f3fd0c958e4ea8e7a6dfe33ae647d339c0544 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Tue, 1 Oct 2019 13:48:54 +0200 Subject: [PATCH 3/7] syntax: further item parsing cleanup --- src/libsyntax/parse/parser/item.rs | 59 ++++++++++++++++++------------ 1 file changed, 35 insertions(+), 24 deletions(-) diff --git a/src/libsyntax/parse/parser/item.rs b/src/libsyntax/parse/parser/item.rs index 95137f93b8810..004c179b7c7fd 100644 --- a/src/libsyntax/parse/parser/item.rs +++ b/src/libsyntax/parse/parser/item.rs @@ -141,14 +141,14 @@ impl<'a> Parser<'a> { self.bump(); // STATIC ITEM let m = self.parse_mutability(); - let info = self.parse_item_const(Some(m)); + let info = self.parse_item_const(Some(m))?; return self.mk_item_with_info(attrs, lo, vis, info); } + if self.eat_keyword(kw::Const) { let const_span = self.prev_span; if [kw::Fn, kw::Unsafe, kw::Extern].iter().any(|k| self.check_keyword(*k)) { // CONST FUNCTION ITEM - let unsafety = self.parse_unsafety(); if self.check_keyword(kw::Extern) { @@ -157,7 +157,7 @@ impl<'a> Parser<'a> { ); } let abi = self.parse_extern_abi()?; - self.bump(); // 'fn' + self.bump(); // `fn` let header = FnHeader { unsafety, @@ -181,7 +181,8 @@ impl<'a> Parser<'a> { ) .emit(); } - let info = self.parse_item_const(None); + + let info = self.parse_item_const(None)?; return self.mk_item_with_info(attrs, lo, vis, info); } @@ -210,6 +211,7 @@ impl<'a> Parser<'a> { return self.parse_item_fn(lo, vis, attrs, header); } } + if self.check_keyword(kw::Unsafe) && self.is_keyword_ahead(1, &[kw::Trait, kw::Auto]) { @@ -222,21 +224,24 @@ impl<'a> Parser<'a> { self.expect_keyword(kw::Trait)?; IsAuto::Yes }; - let info = self.parse_item_trait(is_auto, Unsafety::Unsafe); + let info = self.parse_item_trait(is_auto, Unsafety::Unsafe)?; return self.mk_item_with_info(attrs, lo, vis, info); } + if self.check_keyword(kw::Impl) || self.check_keyword(kw::Unsafe) && self.is_keyword_ahead(1, &[kw::Impl]) || self.check_keyword(kw::Default) && - self.is_keyword_ahead(1, &[kw::Impl, kw::Unsafe]) { + self.is_keyword_ahead(1, &[kw::Impl, kw::Unsafe]) + { // IMPL ITEM let defaultness = self.parse_defaultness(); let unsafety = self.parse_unsafety(); self.expect_keyword(kw::Impl)?; - let info = self.parse_item_impl(unsafety, defaultness); + let info = self.parse_item_impl(unsafety, defaultness)?; return self.mk_item_with_info(attrs, lo, vis, info); } + if self.check_keyword(kw::Fn) { // FUNCTION ITEM self.bump(); @@ -249,8 +254,10 @@ impl<'a> Parser<'a> { }; return self.parse_item_fn(lo, vis, attrs, header); } + if self.check_keyword(kw::Unsafe) - && self.look_ahead(1, |t| *t != token::OpenDelim(token::Brace)) { + && self.look_ahead(1, |t| *t != token::OpenDelim(token::Brace)) + { // UNSAFE FUNCTION ITEM self.bump(); // `unsafe` // `{` is also expected after `unsafe`; in case of error, include it in the diagnostic. @@ -266,11 +273,13 @@ impl<'a> Parser<'a> { }; return self.parse_item_fn(lo, vis, attrs, header); } + if self.eat_keyword(kw::Mod) { // MODULE ITEM - let info = self.parse_item_mod(&attrs[..]); + let info = self.parse_item_mod(&attrs[..])?; return self.mk_item_with_info(attrs, lo, vis, info); } + if let Some(type_) = self.eat_type() { let (ident, alias, generics) = type_?; // TYPE ITEM @@ -281,37 +290,41 @@ impl<'a> Parser<'a> { let span = lo.to(self.prev_span); return Ok(Some(self.mk_item(span, ident, item_, vis, attrs))); } + if self.eat_keyword(kw::Enum) { // ENUM ITEM - let info = self.parse_item_enum(); + let info = self.parse_item_enum()?; return self.mk_item_with_info(attrs, lo, vis, info); } + if self.check_keyword(kw::Trait) || (self.check_keyword(kw::Auto) && self.is_keyword_ahead(1, &[kw::Trait])) { - let is_auto = if self.eat_keyword(kw::Trait) { - IsAuto::No - } else { - self.expect_keyword(kw::Auto)?; - self.expect_keyword(kw::Trait)?; + // TRAIT ITEM + let is_auto = if self.eat_keyword(kw::Auto) { IsAuto::Yes + } else { + IsAuto::No }; - // TRAIT ITEM - let info = self.parse_item_trait(is_auto, Unsafety::Normal); + self.expect_keyword(kw::Trait)?; + let info = self.parse_item_trait(is_auto, Unsafety::Normal)?; return self.mk_item_with_info(attrs, lo, vis, info); } + if self.eat_keyword(kw::Struct) { // STRUCT ITEM - let info = self.parse_item_struct(); + let info = self.parse_item_struct()?; return self.mk_item_with_info(attrs, lo, vis, info); } + if self.is_union_item() { // UNION ITEM self.bump(); - let info = self.parse_item_union(); + let info = self.parse_item_union()?; return self.mk_item_with_info(attrs, lo, vis, info); } + if let Some(macro_def) = self.eat_macro_def(&attrs, &vis, lo)? { return Ok(Some(macro_def)); } @@ -417,9 +430,9 @@ impl<'a> Parser<'a> { attrs: Vec, lo: Span, vis: Visibility, - info: PResult<'a, ItemInfo>, + info: ItemInfo, ) -> PResult<'a, Option>> { - let (ident, item, extra_attrs) = info?; + let (ident, item, extra_attrs) = info; let span = lo.to(self.prev_span); let attrs = maybe_append(attrs, extra_attrs); Ok(Some(self.mk_item(span, ident, item, vis, attrs))) @@ -1195,10 +1208,8 @@ impl<'a> Parser<'a> { let allow_c_variadic = header.abi == Abi::C && header.unsafety == Unsafety::Unsafe; let (ident, decl, generics) = self.parse_fn_sig(allow_c_variadic)?; let (inner_attrs, body) = self.parse_inner_attrs_and_block()?; - let span = lo.to(self.prev_span); let kind = ItemKind::Fn(decl, header, generics, body); - let attrs = maybe_append(attrs, Some(inner_attrs)); - Ok(Some(self.mk_item(span, ident, kind, vis, attrs))) + self.mk_item_with_info(attrs, lo, vis, (ident, kind, Some(inner_attrs))) } /// Parse the "signature", including the identifier, parameters, and generics of a function. From 7f9638d5d4b0b0f4ad592c135e155cdef1ddfb72 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Tue, 1 Oct 2019 14:19:08 +0200 Subject: [PATCH 4/7] syntax: unify trait parsing a bit. --- src/libsyntax/parse/parser/item.rs | 29 ++++++++++++----------------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/src/libsyntax/parse/parser/item.rs b/src/libsyntax/parse/parser/item.rs index 004c179b7c7fd..17b23651e8072 100644 --- a/src/libsyntax/parse/parser/item.rs +++ b/src/libsyntax/parse/parser/item.rs @@ -217,14 +217,7 @@ impl<'a> Parser<'a> { { // UNSAFE TRAIT ITEM self.bump(); // `unsafe` - let is_auto = if self.eat_keyword(kw::Trait) { - IsAuto::No - } else { - self.expect_keyword(kw::Auto)?; - self.expect_keyword(kw::Trait)?; - IsAuto::Yes - }; - let info = self.parse_item_trait(is_auto, Unsafety::Unsafe)?; + let info = self.parse_item_trait(Unsafety::Unsafe)?; return self.mk_item_with_info(attrs, lo, vis, info); } @@ -302,13 +295,7 @@ impl<'a> Parser<'a> { && self.is_keyword_ahead(1, &[kw::Trait])) { // TRAIT ITEM - let is_auto = if self.eat_keyword(kw::Auto) { - IsAuto::Yes - } else { - IsAuto::No - }; - self.expect_keyword(kw::Trait)?; - let info = self.parse_item_trait(is_auto, Unsafety::Normal)?; + let info = self.parse_item_trait(Unsafety::Normal)?; return self.mk_item_with_info(attrs, lo, vis, info); } @@ -860,8 +847,16 @@ impl<'a> Parser<'a> { Ok(FnHeader { constness, unsafety, asyncness, abi }) } - /// Parses `trait Foo { ... }` or `trait Foo = Bar;`. - fn parse_item_trait(&mut self, is_auto: IsAuto, unsafety: Unsafety) -> PResult<'a, ItemInfo> { + /// Parses `auto? trait Foo { ... }` or `trait Foo = Bar;`. + fn parse_item_trait(&mut self, unsafety: Unsafety) -> PResult<'a, ItemInfo> { + // Parse optional `auto` prefix. + let is_auto = if self.eat_keyword(kw::Auto) { + IsAuto::Yes + } else { + IsAuto::No + }; + + self.expect_keyword(kw::Trait)?; let ident = self.parse_ident()?; let mut tps = self.parse_generics()?; From a7ba754b6c021bc6244cde8c52d3d0b352082560 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 3 Oct 2019 03:39:46 +0200 Subject: [PATCH 5/7] syntax: unify and simplify fn signature parsing. --- src/libsyntax/parse/parser.rs | 96 +++++++++++---------------- src/libsyntax/parse/parser/item.rs | 55 +++++++-------- src/libsyntax/parse/parser/ty.rs | 14 ++-- src/test/ui/parser/issue-33413.rs | 1 + src/test/ui/parser/issue-33413.stderr | 14 +++- 5 files changed, 88 insertions(+), 92 deletions(-) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 93165b0964903..4a457f5a43caa 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -11,7 +11,7 @@ mod stmt; mod generics; use crate::ast::{ - self, DUMMY_NODE_ID, AttrStyle, Attribute, BindingMode, CrateSugar, FnDecl, Ident, + self, DUMMY_NODE_ID, AttrStyle, Attribute, BindingMode, CrateSugar, Ident, IsAsync, MacDelimiter, Mutability, Param, StrStyle, SelfKind, TyKind, Visibility, VisibilityKind, Unsafety, }; @@ -56,6 +56,17 @@ crate enum BlockMode { Ignore, } +/// The parsing configuration used to parse a parameter list (see `parse_fn_params`). +struct ParamCfg { + /// Is `self` is allowed as the first parameter? + is_self_allowed: bool, + /// Is `...` allowed as the tail of the parameter list? + allow_c_variadic: bool, + /// `is_name_required` decides if, per-parameter, + /// the parameter must have a pattern or just a type. + is_name_required: fn(&token::Token) -> bool, +} + /// Like `maybe_whole_expr`, but for things other than expressions. #[macro_export] macro_rules! maybe_whole { @@ -1094,26 +1105,18 @@ impl<'a> Parser<'a> { res } - fn parse_fn_params( - &mut self, - named_params: bool, - allow_c_variadic: bool, - ) -> PResult<'a, Vec> { + /// Parses the parameter list of a function, including the `(` and `)` delimiters. + fn parse_fn_params(&mut self, mut cfg: ParamCfg) -> PResult<'a, Vec> { let sp = self.token.span; - let do_not_enforce_named_params_for_c_variadic = |token: &token::Token| { - match token.kind { - token::DotDotDot => false, - _ => named_params, - } - }; + let is_trait_item = cfg.is_self_allowed; let mut c_variadic = false; + // Parse the arguments, starting out with `self` being possibly allowed... let (params, _) = self.parse_paren_comma_seq(|p| { - match p.parse_param_general( - false, - false, - allow_c_variadic, - do_not_enforce_named_params_for_c_variadic, - ) { + let param = p.parse_param_general(&cfg, is_trait_item); + // ...now that we've parsed the first argument, `self` is no longer allowed. + cfg.is_self_allowed = false; + + match param { Ok(param) => Ok( if let TyKind::CVarArgs = param.ty.kind { c_variadic = true; @@ -1144,7 +1147,10 @@ impl<'a> Parser<'a> { } })?; - let params: Vec<_> = params.into_iter().filter_map(|x| x).collect(); + let mut params: Vec<_> = params.into_iter().filter_map(|x| x).collect(); + + // Replace duplicated recovered params with `_` pattern to avoid unecessary errors. + self.deduplicate_recovered_params_names(&mut params); if c_variadic && params.len() <= 1 { self.span_err( @@ -1156,79 +1162,53 @@ impl<'a> Parser<'a> { Ok(params) } - /// Parses the parameter list and result type of a function that may have a `self` parameter. - fn parse_fn_decl_with_self( - &mut self, - is_name_required: impl Copy + Fn(&token::Token) -> bool, - ) -> PResult<'a, P> { - // Parse the arguments, starting out with `self` being allowed... - let mut is_self_allowed = true; - let (mut inputs, _): (Vec<_>, _) = self.parse_paren_comma_seq(|p| { - let res = p.parse_param_general(is_self_allowed, true, false, is_name_required); - // ...but now that we've parsed the first argument, `self` is no longer allowed. - is_self_allowed = false; - res - })?; - - // Replace duplicated recovered params with `_` pattern to avoid unecessary errors. - self.deduplicate_recovered_params_names(&mut inputs); - - Ok(P(FnDecl { - inputs, - output: self.parse_ret_ty(true)?, - })) - } - /// Skips unexpected attributes and doc comments in this position and emits an appropriate /// error. /// This version of parse param doesn't necessarily require identifier names. - fn parse_param_general( - &mut self, - is_self_allowed: bool, - is_trait_item: bool, - allow_c_variadic: bool, - is_name_required: impl Fn(&token::Token) -> bool, - ) -> PResult<'a, Param> { + fn parse_param_general(&mut self, cfg: &ParamCfg, is_trait_item: bool) -> PResult<'a, Param> { let lo = self.token.span; let attrs = self.parse_outer_attributes()?; // Possibly parse `self`. Recover if we parsed it and it wasn't allowed here. if let Some(mut param) = self.parse_self_param()? { param.attrs = attrs.into(); - return if is_self_allowed { + return if cfg.is_self_allowed { Ok(param) } else { self.recover_bad_self_param(param, is_trait_item) }; } - let is_name_required = is_name_required(&self.token); + let is_name_required = match self.token.kind { + token::DotDotDot => false, + _ => (cfg.is_name_required)(&self.token), + }; let (pat, ty) = if is_name_required || self.is_named_param() { debug!("parse_param_general parse_pat (is_name_required:{})", is_name_required); let pat = self.parse_fn_param_pat()?; if let Err(mut err) = self.expect(&token::Colon) { - if let Some(ident) = self.parameter_without_type( + return if let Some(ident) = self.parameter_without_type( &mut err, pat, is_name_required, - is_self_allowed, + cfg.is_self_allowed, is_trait_item, ) { err.emit(); - return Ok(dummy_arg(ident)); + Ok(dummy_arg(ident)) } else { - return Err(err); - } + Err(err) + }; } self.eat_incorrect_doc_comment_for_param_type(); - (pat, self.parse_ty_common(true, true, allow_c_variadic)?) + (pat, self.parse_ty_common(true, true, cfg.allow_c_variadic)?) } else { debug!("parse_param_general ident_to_pat"); let parser_snapshot_before_ty = self.clone(); self.eat_incorrect_doc_comment_for_param_type(); - let mut ty = self.parse_ty_common(true, true, allow_c_variadic); + let mut ty = self.parse_ty_common(true, true, cfg.allow_c_variadic); if ty.is_ok() && self.token != token::Comma && self.token != token::CloseDelim(token::Paren) { // This wasn't actually a type, but a pattern looking like a type, diff --git a/src/libsyntax/parse/parser/item.rs b/src/libsyntax/parse/parser/item.rs index 17b23651e8072..d2ea0829595c0 100644 --- a/src/libsyntax/parse/parser/item.rs +++ b/src/libsyntax/parse/parser/item.rs @@ -1,4 +1,4 @@ -use super::{Parser, PResult, PathStyle, SemiColonMode, BlockMode}; +use super::{Parser, PResult, PathStyle, SemiColonMode, BlockMode, ParamCfg}; use crate::maybe_whole; use crate::ptr::P; @@ -805,14 +805,15 @@ impl<'a> Parser<'a> { /// of a method. The body is not parsed as that differs between `trait`s and `impl`s. fn parse_method_sig( &mut self, - is_name_required: impl Copy + Fn(&token::Token) -> bool, + is_name_required: fn(&token::Token) -> bool, ) -> PResult<'a, (Ident, MethodSig, Generics)> { let header = self.parse_fn_front_matter()?; - let (ident, mut generics) = self.parse_fn_header()?; - let decl = self.parse_fn_decl_with_self(is_name_required)?; - let sig = MethodSig { header, decl }; - generics.where_clause = self.parse_where_clause()?; - Ok((ident, sig, generics)) + let (ident, decl, generics) = self.parse_fn_sig(ParamCfg { + is_self_allowed: true, + allow_c_variadic: false, + is_name_required, + })?; + Ok((ident, MethodSig { header, decl }, generics)) } /// Parses all the "front matter" for a `fn` declaration, up to @@ -1200,36 +1201,34 @@ impl<'a> Parser<'a> { attrs: Vec, header: FnHeader, ) -> PResult<'a, Option>> { - let allow_c_variadic = header.abi == Abi::C && header.unsafety == Unsafety::Unsafe; - let (ident, decl, generics) = self.parse_fn_sig(allow_c_variadic)?; + let (ident, decl, generics) = self.parse_fn_sig(ParamCfg { + is_self_allowed: false, + allow_c_variadic: header.abi == Abi::C && header.unsafety == Unsafety::Unsafe, + is_name_required: |_| true, + })?; let (inner_attrs, body) = self.parse_inner_attrs_and_block()?; let kind = ItemKind::Fn(decl, header, generics, body); self.mk_item_with_info(attrs, lo, vis, (ident, kind, Some(inner_attrs))) } /// Parse the "signature", including the identifier, parameters, and generics of a function. - fn parse_fn_sig( - &mut self, - allow_c_variadic: bool, - ) -> PResult<'a, (Ident, P, Generics)> { - let (ident, mut generics) = self.parse_fn_header()?; - let decl = self.parse_fn_decl(allow_c_variadic)?; + fn parse_fn_sig(&mut self, cfg: ParamCfg) -> PResult<'a, (Ident, P, Generics)> { + let ident = self.parse_ident()?; + let mut generics = self.parse_generics()?; + let decl = self.parse_fn_decl(cfg, true)?; generics.where_clause = self.parse_where_clause()?; Ok((ident, decl, generics)) } - /// Parses the name and optional generic types of a function header. - fn parse_fn_header(&mut self) -> PResult<'a, (Ident, Generics)> { - let id = self.parse_ident()?; - let generics = self.parse_generics()?; - Ok((id, generics)) - } - /// Parses the parameter list and result type of a function declaration. - fn parse_fn_decl(&mut self, allow_c_variadic: bool) -> PResult<'a, P> { + pub(super) fn parse_fn_decl( + &mut self, + cfg: ParamCfg, + ret_allow_plus: bool, + ) -> PResult<'a, P> { Ok(P(FnDecl { - inputs: self.parse_fn_params(true, allow_c_variadic)?, - output: self.parse_ret_ty(true)?, + inputs: self.parse_fn_params(cfg)?, + output: self.parse_ret_ty(ret_allow_plus)?, })) } @@ -1353,7 +1352,11 @@ impl<'a> Parser<'a> { extern_sp: Span, ) -> PResult<'a, ForeignItem> { self.expect_keyword(kw::Fn)?; - let (ident, decl, generics) = self.parse_fn_sig(true)?; + let (ident, decl, generics) = self.parse_fn_sig(super::ParamCfg { + is_self_allowed: false, + allow_c_variadic: true, + is_name_required: |_| true, + })?; let span = lo.to(self.token.span); self.parse_semi_or_incorrect_foreign_fn_body(&ident, extern_sp)?; Ok(ast::ForeignItem { diff --git a/src/libsyntax/parse/parser/ty.rs b/src/libsyntax/parse/parser/ty.rs index 41ee2a1599d74..c9446a880b0b6 100644 --- a/src/libsyntax/parse/parser/ty.rs +++ b/src/libsyntax/parse/parser/ty.rs @@ -4,7 +4,7 @@ use crate::{maybe_whole, maybe_recover_from_interpolated_ty_qpath}; use crate::ptr::P; use crate::ast::{self, Ty, TyKind, MutTy, BareFnTy, FunctionRetTy, GenericParam, Lifetime, Ident}; use crate::ast::{TraitBoundModifier, TraitObjectSyntax, GenericBound, GenericBounds, PolyTraitRef}; -use crate::ast::{Mutability, AnonConst, FnDecl, Mac}; +use crate::ast::{Mutability, AnonConst, Mac}; use crate::parse::token::{self, Token}; use crate::source_map::Span; use crate::symbol::{kw}; @@ -288,12 +288,12 @@ impl<'a> Parser<'a> { }; self.expect_keyword(kw::Fn)?; - let inputs = self.parse_fn_params(false, true)?; - let ret_ty = self.parse_ret_ty(false)?; - let decl = P(FnDecl { - inputs, - output: ret_ty, - }); + let cfg = super::ParamCfg { + is_self_allowed: false, + allow_c_variadic: true, + is_name_required: |_| false, + }; + let decl = self.parse_fn_decl(cfg, false)?; Ok(TyKind::BareFn(P(BareFnTy { abi, unsafety, diff --git a/src/test/ui/parser/issue-33413.rs b/src/test/ui/parser/issue-33413.rs index 22f80a8aae866..7291732cebe4a 100644 --- a/src/test/ui/parser/issue-33413.rs +++ b/src/test/ui/parser/issue-33413.rs @@ -3,6 +3,7 @@ struct S; impl S { fn f(*, a: u8) -> u8 {} //~^ ERROR expected parameter name, found `*` + //~| ERROR mismatched types } fn main() {} diff --git a/src/test/ui/parser/issue-33413.stderr b/src/test/ui/parser/issue-33413.stderr index 9e1178e8ac1f3..7e5c348e36cea 100644 --- a/src/test/ui/parser/issue-33413.stderr +++ b/src/test/ui/parser/issue-33413.stderr @@ -4,5 +4,17 @@ error: expected parameter name, found `*` LL | fn f(*, a: u8) -> u8 {} | ^ expected parameter name -error: aborting due to previous error +error[E0308]: mismatched types + --> $DIR/issue-33413.rs:4:23 + | +LL | fn f(*, a: u8) -> u8 {} + | - ^^ expected u8, found () + | | + | implicitly returns `()` as its body has no tail or `return` expression + | + = note: expected type `u8` + found type `()` + +error: aborting due to 2 previous errors +For more information about this error, try `rustc --explain E0308`. From 9c6582a2c0c5a527e3fc3ce581244d85b3b71761 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 7 Oct 2019 06:18:47 +0200 Subject: [PATCH 6/7] syntax: use `parse_extern_abi` more. --- src/libsyntax/parse/parser/ty.rs | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/libsyntax/parse/parser/ty.rs b/src/libsyntax/parse/parser/ty.rs index c9446a880b0b6..018b5951e6e2e 100644 --- a/src/libsyntax/parse/parser/ty.rs +++ b/src/libsyntax/parse/parser/ty.rs @@ -9,8 +9,6 @@ use crate::parse::token::{self, Token}; use crate::source_map::Span; use crate::symbol::{kw}; -use rustc_target::spec::abi::Abi; - use errors::{Applicability, pluralise}; /// Returns `true` if `IDENT t` can start a type -- `IDENT::a::b`, `IDENT`, @@ -281,12 +279,7 @@ impl<'a> Parser<'a> { */ let unsafety = self.parse_unsafety(); - let abi = if self.eat_keyword(kw::Extern) { - self.parse_opt_abi()?.unwrap_or(Abi::C) - } else { - Abi::Rust - }; - + let abi = self.parse_extern_abi()?; self.expect_keyword(kw::Fn)?; let cfg = super::ParamCfg { is_self_allowed: false, From 6b23c22ccad4044476b65fc8f3e32794f1884781 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 7 Oct 2019 06:31:02 +0200 Subject: [PATCH 7/7] syntax: refactor with new `fn parse_use_tree_glob_or_nested`. --- src/libsyntax/parse/parser/item.rs | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/libsyntax/parse/parser/item.rs b/src/libsyntax/parse/parser/item.rs index d2ea0829595c0..3c60c88e2aa58 100644 --- a/src/libsyntax/parse/parser/item.rs +++ b/src/libsyntax/parse/parser/item.rs @@ -1077,21 +1077,13 @@ impl<'a> Parser<'a> { ); } - if self.eat(&token::BinOp(token::Star)) { - UseTreeKind::Glob - } else { - UseTreeKind::Nested(self.parse_use_tree_list()?) - } + self.parse_use_tree_glob_or_nested()? } else { // `use path::*;` or `use path::{...};` or `use path;` or `use path as bar;` prefix = self.parse_path(PathStyle::Mod)?; if self.eat(&token::ModSep) { - if self.eat(&token::BinOp(token::Star)) { - UseTreeKind::Glob - } else { - UseTreeKind::Nested(self.parse_use_tree_list()?) - } + self.parse_use_tree_glob_or_nested()? } else { UseTreeKind::Simple(self.parse_rename()?, DUMMY_NODE_ID, DUMMY_NODE_ID) } @@ -1100,6 +1092,15 @@ impl<'a> Parser<'a> { Ok(UseTree { prefix, kind, span: lo.to(self.prev_span) }) } + /// Parses `*` or `{...}`. + fn parse_use_tree_glob_or_nested(&mut self) -> PResult<'a, UseTreeKind> { + Ok(if self.eat(&token::BinOp(token::Star)) { + UseTreeKind::Glob + } else { + UseTreeKind::Nested(self.parse_use_tree_list()?) + }) + } + /// Parses a `UseTreeKind::Nested(list)`. /// /// ```