From e6b14aab05621ac6c06ca6894b2dac26a4ba690c Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 9 Nov 2015 18:43:32 +0300 Subject: [PATCH 1/3] syntax: Merge parsing code for structures and variants --- src/libsyntax/parse/parser.rs | 64 ++++++------------- .../issue-12560-1.rs | 6 +- src/test/compile-fail/issue-16819.rs | 14 +++- 3 files changed, 33 insertions(+), 51 deletions(-) rename src/test/{parse-fail => compile-fail}/issue-12560-1.rs (79%) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 0905f26f1474..0a75d05f2cc1 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -4686,18 +4686,21 @@ impl<'a> Parser<'a> { VariantData::Unit(ast::DUMMY_NODE_ID) } else { // If we see: `struct Foo where T: Copy { ... }` - VariantData::Struct(try!(self.parse_record_struct_body()), ast::DUMMY_NODE_ID) + VariantData::Struct(try!(self.parse_record_struct_body(true)), ast::DUMMY_NODE_ID) } // No `where` so: `struct Foo;` } else if try!(self.eat(&token::Semi) ){ VariantData::Unit(ast::DUMMY_NODE_ID) // Record-style struct definition } else if self.token == token::OpenDelim(token::Brace) { - VariantData::Struct(try!(self.parse_record_struct_body()), ast::DUMMY_NODE_ID) + VariantData::Struct(try!(self.parse_record_struct_body(true)), ast::DUMMY_NODE_ID) // Tuple-style struct definition with optional where-clause. } else if self.token == token::OpenDelim(token::Paren) { - VariantData::Tuple(try!(self.parse_tuple_struct_body(&mut generics)), - ast::DUMMY_NODE_ID) + let body = VariantData::Tuple(try!(self.parse_tuple_struct_body(true)), + ast::DUMMY_NODE_ID); + generics.where_clause = try!(self.parse_where_clause()); + try!(self.expect(&token::Semi)); + body } else { let token_str = self.this_token_to_string(); return Err(self.fatal(&format!("expected `where`, `{{`, `(`, or `;` after struct \ @@ -4707,11 +4710,11 @@ impl<'a> Parser<'a> { Ok((class_name, ItemStruct(vdata, generics), None)) } - pub fn parse_record_struct_body(&mut self) -> PResult> { + pub fn parse_record_struct_body(&mut self, allow_pub: bool) -> PResult> { let mut fields = Vec::new(); if try!(self.eat(&token::OpenDelim(token::Brace)) ){ while self.token != token::CloseDelim(token::Brace) { - fields.push(try!(self.parse_struct_decl_field(true))); + fields.push(try!(self.parse_struct_decl_field(allow_pub))); } try!(self.bump()); @@ -4725,9 +4728,7 @@ impl<'a> Parser<'a> { Ok(fields) } - pub fn parse_tuple_struct_body(&mut self, - generics: &mut ast::Generics) - -> PResult> { + pub fn parse_tuple_struct_body(&mut self, allow_pub: bool) -> PResult> { // This is the case where we find `struct Foo(T) where T: Copy;` // Unit like structs are handled in parse_item_struct function let fields = try!(self.parse_unspanned_seq( @@ -4738,7 +4739,9 @@ impl<'a> Parser<'a> { let attrs = try!(p.parse_outer_attributes()); let lo = p.span.lo; let struct_field_ = ast::StructField_ { - kind: UnnamedField(try!(p.parse_visibility())), + kind: UnnamedField ( + if allow_pub { try!(p.parse_visibility()) } else { Inherited } + ), id: ast::DUMMY_NODE_ID, ty: try!(p.parse_ty_sum()), attrs: attrs, @@ -4746,8 +4749,6 @@ impl<'a> Parser<'a> { Ok(spanned(lo, p.span.hi, struct_field_)) })); - generics.where_clause = try!(self.parse_where_clause()); - try!(self.expect(&token::Semi)); Ok(fields) } @@ -5133,18 +5134,6 @@ impl<'a> Parser<'a> { Ok((ident, ItemTy(ty, tps), None)) } - /// Parse a structure-like enum variant definition - /// this should probably be renamed or refactored... - fn parse_struct_def(&mut self) -> PResult { - let mut fields: Vec = Vec::new(); - while self.token != token::CloseDelim(token::Brace) { - fields.push(try!(self.parse_struct_decl_field(false))); - } - try!(self.bump()); - - Ok(VariantData::Struct(fields, ast::DUMMY_NODE_ID)) - } - /// Parse the part of an "enum" decl following the '{' fn parse_enum_def(&mut self, _generics: &ast::Generics) -> PResult { let mut variants = Vec::new(); @@ -5157,34 +5146,21 @@ impl<'a> Parser<'a> { let struct_def; let mut disr_expr = None; let ident = try!(self.parse_ident()); - if try!(self.eat(&token::OpenDelim(token::Brace)) ){ + if self.check(&token::OpenDelim(token::Brace)) { // Parse a struct variant. all_nullary = false; - struct_def = try!(self.parse_struct_def()); + struct_def = VariantData::Struct(try!(self.parse_record_struct_body(false)), + ast::DUMMY_NODE_ID); } else if self.check(&token::OpenDelim(token::Paren)) { all_nullary = false; - let arg_tys = try!(self.parse_enum_variant_seq( - &token::OpenDelim(token::Paren), - &token::CloseDelim(token::Paren), - seq_sep_trailing_allowed(token::Comma), - |p| p.parse_ty_sum() - )); - let mut fields = Vec::new(); - for ty in arg_tys { - fields.push(Spanned { span: ty.span, node: ast::StructField_ { - ty: ty, - kind: ast::UnnamedField(ast::Inherited), - attrs: Vec::new(), - id: ast::DUMMY_NODE_ID, - }}); - } - struct_def = ast::VariantData::Tuple(fields, ast::DUMMY_NODE_ID); + struct_def = VariantData::Tuple(try!(self.parse_tuple_struct_body(false)), + ast::DUMMY_NODE_ID); } else if try!(self.eat(&token::Eq) ){ disr_expr = Some(try!(self.parse_expr_nopanic())); any_disr = disr_expr.as_ref().map(|expr| expr.span); - struct_def = ast::VariantData::Unit(ast::DUMMY_NODE_ID); + struct_def = VariantData::Unit(ast::DUMMY_NODE_ID); } else { - struct_def = ast::VariantData::Unit(ast::DUMMY_NODE_ID); + struct_def = VariantData::Unit(ast::DUMMY_NODE_ID); } let vr = ast::Variant_ { diff --git a/src/test/parse-fail/issue-12560-1.rs b/src/test/compile-fail/issue-12560-1.rs similarity index 79% rename from src/test/parse-fail/issue-12560-1.rs rename to src/test/compile-fail/issue-12560-1.rs index 96a17a8fb45e..de13cb465db9 100644 --- a/src/test/parse-fail/issue-12560-1.rs +++ b/src/test/compile-fail/issue-12560-1.rs @@ -8,15 +8,13 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -// compile-flags: -Z parse-only - // For style and consistency reasons, non-parametrized enum variants must // be used simply as `ident` instead of `ident ()`. // This test-case covers enum declaration. enum Foo { - Bar(), //~ ERROR nullary enum variants are written with no trailing `( )` - Baz(), //~ ERROR nullary enum variants are written with no trailing `( )` + Bar(), //~ ERROR empty tuple structs and enum variants are not allowed + Baz(), //~ ERROR empty tuple structs and enum variants are not allowed Bazar } diff --git a/src/test/compile-fail/issue-16819.rs b/src/test/compile-fail/issue-16819.rs index 065b29d29aca..b229b91c7cd1 100644 --- a/src/test/compile-fail/issue-16819.rs +++ b/src/test/compile-fail/issue-16819.rs @@ -10,9 +10,17 @@ struct TS ( //~ ERROR empty tuple structs and enum variants are not allowed #[cfg(untrue)] - int, + i32, ); -fn main() { - let s = S; +enum E { + TV ( //~ ERROR empty tuple structs and enum variants are not allowed + #[cfg(untrue)] + i32, + ) +} + +fn main() { + let s = TS; + let tv = E::TV; } From 2a01e263bcb2d826f2a398b4c34b59485a201274 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 9 Nov 2015 22:16:30 +0300 Subject: [PATCH 2/3] Improve error message --- src/libsyntax/feature_gate.rs | 2 ++ src/test/compile-fail/issue-12560-1.rs | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index a0c089aff217..94b5cf46dd1c 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -867,6 +867,8 @@ impl<'a, 'v> Visitor<'v> for PostExpansionVisitor<'a> { self.context.span_handler.span_err(span, "empty tuple structs and enum variants \ are not allowed, use unit structs and \ enum variants instead"); + self.context.span_handler.span_help(span, "remove trailing () to make a unit \ + struct or unit enum varian"); } } visit::walk_struct_def(self, s) diff --git a/src/test/compile-fail/issue-12560-1.rs b/src/test/compile-fail/issue-12560-1.rs index de13cb465db9..d0fb91a576c0 100644 --- a/src/test/compile-fail/issue-12560-1.rs +++ b/src/test/compile-fail/issue-12560-1.rs @@ -14,7 +14,9 @@ enum Foo { Bar(), //~ ERROR empty tuple structs and enum variants are not allowed + //~^ HELP remove trailing () to make a unit struct or unit enum varian Baz(), //~ ERROR empty tuple structs and enum variants are not allowed + //~^ HELP remove trailing () to make a unit struct or unit enum varian Bazar } From 649fc3895cecb1e277a6703ad4a1a60b2d7a9f98 Mon Sep 17 00:00:00 2001 From: Vadim Petrochenkov Date: Mon, 9 Nov 2015 22:34:15 +0300 Subject: [PATCH 3/3] Use enum ParsePub instead of bool in field parsing + typo --- src/libsyntax/feature_gate.rs | 4 +-- src/libsyntax/parse/parser.rs | 35 ++++++++++++++++++-------- src/test/compile-fail/issue-12560-1.rs | 4 +-- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/libsyntax/feature_gate.rs b/src/libsyntax/feature_gate.rs index 94b5cf46dd1c..c579a2c9c0ae 100644 --- a/src/libsyntax/feature_gate.rs +++ b/src/libsyntax/feature_gate.rs @@ -867,8 +867,8 @@ impl<'a, 'v> Visitor<'v> for PostExpansionVisitor<'a> { self.context.span_handler.span_err(span, "empty tuple structs and enum variants \ are not allowed, use unit structs and \ enum variants instead"); - self.context.span_handler.span_help(span, "remove trailing () to make a unit \ - struct or unit enum varian"); + self.context.span_handler.span_help(span, "remove trailing `()` to make a unit \ + struct or unit enum variant"); } } visit::walk_struct_def(self, s) diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 0a75d05f2cc1..fde1058a785c 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -113,6 +113,13 @@ pub enum BoundParsingMode { Modified, } +/// `pub` should be parsed in struct fields and not parsed in variant fields +#[derive(Clone, Copy, PartialEq)] +pub enum ParsePub { + Yes, + No, +} + /// Possibly accept an `token::Interpolated` expression (a pre-parsed expression /// dropped into the token stream, which happens while parsing the result of /// macro expansion). Placement of these is not as complex as I feared it would @@ -4686,17 +4693,19 @@ impl<'a> Parser<'a> { VariantData::Unit(ast::DUMMY_NODE_ID) } else { // If we see: `struct Foo where T: Copy { ... }` - VariantData::Struct(try!(self.parse_record_struct_body(true)), ast::DUMMY_NODE_ID) + VariantData::Struct(try!(self.parse_record_struct_body(ParsePub::Yes)), + ast::DUMMY_NODE_ID) } // No `where` so: `struct Foo;` } else if try!(self.eat(&token::Semi) ){ VariantData::Unit(ast::DUMMY_NODE_ID) // Record-style struct definition } else if self.token == token::OpenDelim(token::Brace) { - VariantData::Struct(try!(self.parse_record_struct_body(true)), ast::DUMMY_NODE_ID) + VariantData::Struct(try!(self.parse_record_struct_body(ParsePub::Yes)), + ast::DUMMY_NODE_ID) // Tuple-style struct definition with optional where-clause. } else if self.token == token::OpenDelim(token::Paren) { - let body = VariantData::Tuple(try!(self.parse_tuple_struct_body(true)), + let body = VariantData::Tuple(try!(self.parse_tuple_struct_body(ParsePub::Yes)), ast::DUMMY_NODE_ID); generics.where_clause = try!(self.parse_where_clause()); try!(self.expect(&token::Semi)); @@ -4710,11 +4719,11 @@ impl<'a> Parser<'a> { Ok((class_name, ItemStruct(vdata, generics), None)) } - pub fn parse_record_struct_body(&mut self, allow_pub: bool) -> PResult> { + pub fn parse_record_struct_body(&mut self, parse_pub: ParsePub) -> PResult> { let mut fields = Vec::new(); if try!(self.eat(&token::OpenDelim(token::Brace)) ){ while self.token != token::CloseDelim(token::Brace) { - fields.push(try!(self.parse_struct_decl_field(allow_pub))); + fields.push(try!(self.parse_struct_decl_field(parse_pub))); } try!(self.bump()); @@ -4728,7 +4737,7 @@ impl<'a> Parser<'a> { Ok(fields) } - pub fn parse_tuple_struct_body(&mut self, allow_pub: bool) -> PResult> { + pub fn parse_tuple_struct_body(&mut self, parse_pub: ParsePub) -> PResult> { // This is the case where we find `struct Foo(T) where T: Copy;` // Unit like structs are handled in parse_item_struct function let fields = try!(self.parse_unspanned_seq( @@ -4740,7 +4749,11 @@ impl<'a> Parser<'a> { let lo = p.span.lo; let struct_field_ = ast::StructField_ { kind: UnnamedField ( - if allow_pub { try!(p.parse_visibility()) } else { Inherited } + if parse_pub == ParsePub::Yes { + try!(p.parse_visibility()) + } else { + Inherited + } ), id: ast::DUMMY_NODE_ID, ty: try!(p.parse_ty_sum()), @@ -4776,12 +4789,12 @@ impl<'a> Parser<'a> { } /// Parse an element of a struct definition - fn parse_struct_decl_field(&mut self, allow_pub: bool) -> PResult { + fn parse_struct_decl_field(&mut self, parse_pub: ParsePub) -> PResult { let attrs = try!(self.parse_outer_attributes()); if try!(self.eat_keyword(keywords::Pub) ){ - if !allow_pub { + if parse_pub == ParsePub::No { let span = self.last_span; self.span_err(span, "`pub` is not allowed here"); } @@ -5149,11 +5162,11 @@ impl<'a> Parser<'a> { if self.check(&token::OpenDelim(token::Brace)) { // Parse a struct variant. all_nullary = false; - struct_def = VariantData::Struct(try!(self.parse_record_struct_body(false)), + struct_def = VariantData::Struct(try!(self.parse_record_struct_body(ParsePub::No)), ast::DUMMY_NODE_ID); } else if self.check(&token::OpenDelim(token::Paren)) { all_nullary = false; - struct_def = VariantData::Tuple(try!(self.parse_tuple_struct_body(false)), + struct_def = VariantData::Tuple(try!(self.parse_tuple_struct_body(ParsePub::No)), ast::DUMMY_NODE_ID); } else if try!(self.eat(&token::Eq) ){ disr_expr = Some(try!(self.parse_expr_nopanic())); diff --git a/src/test/compile-fail/issue-12560-1.rs b/src/test/compile-fail/issue-12560-1.rs index d0fb91a576c0..e005de01649a 100644 --- a/src/test/compile-fail/issue-12560-1.rs +++ b/src/test/compile-fail/issue-12560-1.rs @@ -14,9 +14,9 @@ enum Foo { Bar(), //~ ERROR empty tuple structs and enum variants are not allowed - //~^ HELP remove trailing () to make a unit struct or unit enum varian + //~^ HELP remove trailing `()` to make a unit struct or unit enum variant Baz(), //~ ERROR empty tuple structs and enum variants are not allowed - //~^ HELP remove trailing () to make a unit struct or unit enum varian + //~^ HELP remove trailing `()` to make a unit struct or unit enum variant Bazar }