From 7737d0ffdef6f3d7395e80291e3143522f46b95b Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 31 Jan 2020 06:43:33 +0100 Subject: [PATCH] parser: unify item list parsing. as a consequence, `trait X { #![attr] }` becomes legal. --- src/librustc_ast_pretty/pprust.rs | 1 + src/librustc_expand/expand.rs | 2 +- src/librustc_parse/parser/item.rs | 102 ++++++++---------- src/test/pretty/trait-inner-attr.rs | 7 ++ src/test/ui/parser/attrs-after-extern-mod.rs | 10 +- .../ui/parser/attrs-after-extern-mod.stderr | 2 +- .../ui/parser/doc-before-extern-rbrace.rs | 4 +- .../ui/parser/doc-before-extern-rbrace.stderr | 7 +- .../ui/parser/doc-inside-trait-item.stderr | 2 +- src/test/ui/parser/duplicate-visibility.rs | 3 + .../ui/parser/duplicate-visibility.stderr | 2 +- src/test/ui/parser/inner-attr-in-trait-def.rs | 9 ++ 12 files changed, 80 insertions(+), 71 deletions(-) create mode 100644 src/test/pretty/trait-inner-attr.rs create mode 100644 src/test/ui/parser/inner-attr-in-trait-def.rs diff --git a/src/librustc_ast_pretty/pprust.rs b/src/librustc_ast_pretty/pprust.rs index b1fa818d0a8..75938470b6f 100644 --- a/src/librustc_ast_pretty/pprust.rs +++ b/src/librustc_ast_pretty/pprust.rs @@ -1269,6 +1269,7 @@ impl<'a> State<'a> { self.print_where_clause(&generics.where_clause); self.s.word(" "); self.bopen(); + self.print_inner_attributes(&item.attrs); for trait_item in trait_items { self.print_assoc_item(trait_item); } diff --git a/src/librustc_expand/expand.rs b/src/librustc_expand/expand.rs index 90692fe1ec9..0968e92c203 100644 --- a/src/librustc_expand/expand.rs +++ b/src/librustc_expand/expand.rs @@ -867,7 +867,7 @@ pub fn parse_ast_fragment<'a>( AstFragmentKind::ForeignItems => { let mut items = SmallVec::new(); while this.token != token::Eof { - items.push(this.parse_foreign_item()?); + items.push(this.parse_foreign_item(&mut false)?); } AstFragment::ForeignItems(items) } diff --git a/src/librustc_parse/parser/item.rs b/src/librustc_parse/parser/item.rs index 0a8f3770862..f4ca84b005b 100644 --- a/src/librustc_parse/parser/item.rs +++ b/src/librustc_parse/parser/item.rs @@ -515,7 +515,7 @@ impl<'a> Parser<'a> { generics.where_clause = self.parse_where_clause()?; - let (impl_items, attrs) = self.parse_impl_body()?; + let (impl_items, attrs) = self.parse_item_list(|p, at_end| p.parse_impl_item(at_end))?; let item_kind = match ty_second { Some(ty_second) => { @@ -571,15 +571,21 @@ impl<'a> Parser<'a> { Ok((Ident::invalid(), item_kind, Some(attrs))) } - fn parse_impl_body(&mut self) -> PResult<'a, (Vec>, Vec)> { + fn parse_item_list( + &mut self, + mut parse_item: impl FnMut(&mut Parser<'a>, &mut bool) -> PResult<'a, T>, + ) -> PResult<'a, (Vec, Vec)> { self.expect(&token::OpenDelim(token::Brace))?; let attrs = self.parse_inner_attributes()?; - let mut impl_items = Vec::new(); + let mut items = Vec::new(); while !self.eat(&token::CloseDelim(token::Brace)) { + if self.recover_doc_comment_before_brace() { + continue; + } let mut at_end = false; - match self.parse_impl_item(&mut at_end) { - Ok(impl_item) => impl_items.push(impl_item), + match parse_item(self, &mut at_end) { + Ok(item) => items.push(item), Err(mut err) => { err.emit(); if !at_end { @@ -589,7 +595,30 @@ impl<'a> Parser<'a> { } } } - Ok((impl_items, attrs)) + Ok((items, attrs)) + } + + /// Recover on a doc comment before `}`. + fn recover_doc_comment_before_brace(&mut self) -> bool { + if let token::DocComment(_) = self.token.kind { + if self.look_ahead(1, |tok| tok == &token::CloseDelim(token::Brace)) { + struct_span_err!( + self.diagnostic(), + self.token.span, + E0584, + "found a documentation comment that doesn't document anything", + ) + .span_label(self.token.span, "this doc comment doesn't document anything") + .help( + "doc comments must come before what they document, maybe a \ + comment was intended with `//`?", + ) + .emit(); + self.bump(); + return true; + } + } + false } /// Parses defaultness (i.e., `default` or nothing). @@ -660,39 +689,8 @@ impl<'a> Parser<'a> { } else { // It's a normal trait. tps.where_clause = self.parse_where_clause()?; - self.expect(&token::OpenDelim(token::Brace))?; - let mut trait_items = vec![]; - while !self.eat(&token::CloseDelim(token::Brace)) { - if let token::DocComment(_) = self.token.kind { - if self.look_ahead(1, |tok| tok == &token::CloseDelim(token::Brace)) { - struct_span_err!( - self.diagnostic(), - self.token.span, - E0584, - "found a documentation comment that doesn't document anything", - ) - .help( - "doc comments must come before what they document, maybe a \ - comment was intended with `//`?", - ) - .emit(); - self.bump(); - continue; - } - } - let mut at_end = false; - match self.parse_trait_item(&mut at_end) { - Ok(item) => trait_items.push(item), - Err(mut e) => { - e.emit(); - if !at_end { - self.consume_block(token::Brace, ConsumeClosingDelim::Yes); - break; - } - } - } - } - Ok((ident, ItemKind::Trait(is_auto, unsafety, tps, bounds, trait_items), None)) + let (items, attrs) = self.parse_item_list(|p, at_end| p.parse_trait_item(at_end))?; + Ok((ident, ItemKind::Trait(is_auto, unsafety, tps, bounds, items), Some(attrs))) } } @@ -942,26 +940,18 @@ impl<'a> Parser<'a> { &mut self, lo: Span, abi: Option, - visibility: Visibility, + vis: Visibility, mut attrs: Vec, ) -> PResult<'a, P> { - self.expect(&token::OpenDelim(token::Brace))?; - - attrs.extend(self.parse_inner_attributes()?); - - let mut foreign_items = vec![]; - while !self.eat(&token::CloseDelim(token::Brace)) { - foreign_items.push(self.parse_foreign_item()?); - } - - let prev_span = self.prev_span; - let m = ast::ForeignMod { abi, items: foreign_items }; - let invalid = Ident::invalid(); - Ok(self.mk_item(lo.to(prev_span), invalid, ItemKind::ForeignMod(m), visibility, attrs)) + let (items, iattrs) = self.parse_item_list(|p, at_end| p.parse_foreign_item(at_end))?; + attrs.extend(iattrs); + let span = lo.to(self.prev_span); + let m = ast::ForeignMod { abi, items }; + Ok(self.mk_item(span, Ident::invalid(), ItemKind::ForeignMod(m), vis, attrs)) } /// Parses a foreign item (one in an `extern { ... }` block). - pub fn parse_foreign_item(&mut self) -> PResult<'a, P> { + pub fn parse_foreign_item(&mut self, at_end: &mut bool) -> PResult<'a, P> { maybe_whole!(self, NtForeignItem, |ni| ni); let mut attrs = self.parse_outer_attributes()?; @@ -973,7 +963,7 @@ impl<'a> Parser<'a> { self.parse_item_foreign_type()? } else if self.check_fn_front_matter() { // FOREIGN FUNCTION ITEM - let (ident, sig, generics, body) = self.parse_fn(&mut false, &mut attrs, |_| true)?; + let (ident, sig, generics, body) = self.parse_fn(at_end, &mut attrs, |_| true)?; (ident, ForeignItemKind::Fn(sig, generics, body)) } else if self.is_static_global() { // FOREIGN STATIC ITEM @@ -991,7 +981,7 @@ impl<'a> Parser<'a> { ) .emit(); self.parse_item_foreign_static()? - } else if let Some(mac) = self.parse_assoc_macro_invoc("extern", Some(&vis), &mut false)? { + } else if let Some(mac) = self.parse_assoc_macro_invoc("extern", Some(&vis), at_end)? { (Ident::invalid(), ForeignItemKind::Macro(mac)) } else { if !attrs.is_empty() { diff --git a/src/test/pretty/trait-inner-attr.rs b/src/test/pretty/trait-inner-attr.rs new file mode 100644 index 00000000000..bb4fb1459bd --- /dev/null +++ b/src/test/pretty/trait-inner-attr.rs @@ -0,0 +1,7 @@ +// pp-exact + +trait Foo { + #![allow(bar)] +} + +fn main() { } diff --git a/src/test/ui/parser/attrs-after-extern-mod.rs b/src/test/ui/parser/attrs-after-extern-mod.rs index 4bdd16471cd..ea899dca7b2 100644 --- a/src/test/ui/parser/attrs-after-extern-mod.rs +++ b/src/test/ui/parser/attrs-after-extern-mod.rs @@ -1,13 +1,7 @@ -// Constants (static variables) can be used to match in patterns, but mutable -// statics cannot. This ensures that there's some form of error if this is -// attempted. +// Make sure there's an error when given `extern { ... #[attr] }`. -extern crate libc; +fn main() {} extern { - static mut rust_dbg_static_mut: libc::c_int; - pub fn rust_dbg_static_mut_check_four(); #[cfg(stage37)] //~ ERROR expected item after attributes } - -pub fn main() {} diff --git a/src/test/ui/parser/attrs-after-extern-mod.stderr b/src/test/ui/parser/attrs-after-extern-mod.stderr index cecdab4d631..a02e738a2c3 100644 --- a/src/test/ui/parser/attrs-after-extern-mod.stderr +++ b/src/test/ui/parser/attrs-after-extern-mod.stderr @@ -1,5 +1,5 @@ error: expected item after attributes - --> $DIR/attrs-after-extern-mod.rs:10:19 + --> $DIR/attrs-after-extern-mod.rs:6:19 | LL | #[cfg(stage37)] | ^ diff --git a/src/test/ui/parser/doc-before-extern-rbrace.rs b/src/test/ui/parser/doc-before-extern-rbrace.rs index 695d4da1dca..040206b80ff 100644 --- a/src/test/ui/parser/doc-before-extern-rbrace.rs +++ b/src/test/ui/parser/doc-before-extern-rbrace.rs @@ -1,4 +1,6 @@ +fn main() {} + extern { /// hi - //~^ ERROR expected item after doc comment + //~^ ERROR found a documentation comment that doesn't document anything } diff --git a/src/test/ui/parser/doc-before-extern-rbrace.stderr b/src/test/ui/parser/doc-before-extern-rbrace.stderr index 8cc9c916a7a..0edceb268a7 100644 --- a/src/test/ui/parser/doc-before-extern-rbrace.stderr +++ b/src/test/ui/parser/doc-before-extern-rbrace.stderr @@ -1,8 +1,11 @@ -error: expected item after doc comment - --> $DIR/doc-before-extern-rbrace.rs:2:5 +error[E0584]: found a documentation comment that doesn't document anything + --> $DIR/doc-before-extern-rbrace.rs:4:5 | LL | /// hi | ^^^^^^ this doc comment doesn't document anything + | + = help: doc comments must come before what they document, maybe a comment was intended with `//`? error: aborting due to previous error +For more information about this error, try `rustc --explain E0584`. diff --git a/src/test/ui/parser/doc-inside-trait-item.stderr b/src/test/ui/parser/doc-inside-trait-item.stderr index 261e27b6e0d..246255a0a46 100644 --- a/src/test/ui/parser/doc-inside-trait-item.stderr +++ b/src/test/ui/parser/doc-inside-trait-item.stderr @@ -2,7 +2,7 @@ error[E0584]: found a documentation comment that doesn't document anything --> $DIR/doc-inside-trait-item.rs:3:5 | LL | /// empty doc - | ^^^^^^^^^^^^^ + | ^^^^^^^^^^^^^ this doc comment doesn't document anything | = help: doc comments must come before what they document, maybe a comment was intended with `//`? diff --git a/src/test/ui/parser/duplicate-visibility.rs b/src/test/ui/parser/duplicate-visibility.rs index a8f0b7d61b9..edbf508ecdb 100644 --- a/src/test/ui/parser/duplicate-visibility.rs +++ b/src/test/ui/parser/duplicate-visibility.rs @@ -1,4 +1,7 @@ // error-pattern: expected one of `(`, `async`, `const`, `extern`, `fn` + +fn main() {} + extern { pub pub fn foo(); } diff --git a/src/test/ui/parser/duplicate-visibility.stderr b/src/test/ui/parser/duplicate-visibility.stderr index cba4058e482..92cf3487969 100644 --- a/src/test/ui/parser/duplicate-visibility.stderr +++ b/src/test/ui/parser/duplicate-visibility.stderr @@ -1,5 +1,5 @@ error: expected one of `(`, `async`, `const`, `extern`, `fn`, `static`, `type`, or `unsafe`, found keyword `pub` - --> $DIR/duplicate-visibility.rs:3:9 + --> $DIR/duplicate-visibility.rs:6:9 | LL | pub pub fn foo(); | ^^^ expected one of 8 possible tokens diff --git a/src/test/ui/parser/inner-attr-in-trait-def.rs b/src/test/ui/parser/inner-attr-in-trait-def.rs new file mode 100644 index 00000000000..8dba6b362cd --- /dev/null +++ b/src/test/ui/parser/inner-attr-in-trait-def.rs @@ -0,0 +1,9 @@ +// check-pass + +#![deny(non_camel_case_types)] + +fn main() {} + +trait foo_bar { + #![allow(non_camel_case_types)] +}