From d648c10e5bc313af758951c1e6f8ae4782712627 Mon Sep 17 00:00:00 2001 From: Ariel Ben-Yehuda Date: Thu, 13 Apr 2017 22:37:05 +0300 Subject: [PATCH] libsyntax/parse: improve associated item error reporting Fixes #41161. Fixes #41239. --- src/libsyntax/ext/expand.rs | 4 +- src/libsyntax/parse/parser.rs | 167 +++++++++++-------- src/test/parse-fail/issue-21153.rs | 5 +- src/test/parse-fail/trait-pub-assoc-const.rs | 2 +- src/test/parse-fail/trait-pub-assoc-ty.rs | 2 +- src/test/parse-fail/trait-pub-method.rs | 2 +- src/test/ui/did_you_mean/issue-40006.rs | 12 ++ src/test/ui/did_you_mean/issue-40006.stderr | 68 +++++++- src/test/ui/token/issue-41155.stderr | 10 +- 9 files changed, 191 insertions(+), 81 deletions(-) diff --git a/src/libsyntax/ext/expand.rs b/src/libsyntax/ext/expand.rs index 1b3352f73ade7..20a858b80df47 100644 --- a/src/libsyntax/ext/expand.rs +++ b/src/libsyntax/ext/expand.rs @@ -617,14 +617,14 @@ impl<'a> Parser<'a> { ExpansionKind::TraitItems => { let mut items = SmallVector::new(); while self.token != token::Eof { - items.push(self.parse_trait_item()?); + items.push(self.parse_trait_item(&mut false)?); } Expansion::TraitItems(items) } ExpansionKind::ImplItems => { let mut items = SmallVector::new(); while self.token != token::Eof { - items.push(self.parse_impl_item()?); + items.push(self.parse_impl_item(&mut false)?); } Expansion::ImplItems(items) } diff --git a/src/libsyntax/parse/parser.rs b/src/libsyntax/parse/parser.rs index 58be43526fd9d..dfb82d40d568d 100644 --- a/src/libsyntax/parse/parser.rs +++ b/src/libsyntax/parse/parser.rs @@ -85,12 +85,18 @@ pub enum PathStyle { Expr, } -#[derive(Clone, Copy, PartialEq)] +#[derive(Clone, Copy, Debug, PartialEq)] pub enum SemiColonMode { Break, Ignore, } +#[derive(Clone, Copy, Debug, PartialEq)] +pub enum BlockMode { + Break, + Ignore, +} + /// 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 @@ -1204,7 +1210,7 @@ impl<'a> Parser<'a> { } /// Parse the items in a trait declaration - pub fn parse_trait_item(&mut self) -> PResult<'a, TraitItem> { + pub fn parse_trait_item(&mut self, at_end: &mut bool) -> PResult<'a, TraitItem> { maybe_whole!(self, NtTraitItem, |x| x); let mut attrs = self.parse_outer_attributes()?; let lo = self.span; @@ -1214,7 +1220,7 @@ impl<'a> Parser<'a> { self.expect(&token::Semi)?; (ident, TraitItemKind::Type(bounds, default)) } else if self.is_const_item() { - self.expect_keyword(keywords::Const)?; + self.expect_keyword(keywords::Const)?; let ident = self.parse_ident()?; self.expect(&token::Colon)?; let ty = self.parse_ty()?; @@ -1231,9 +1237,17 @@ impl<'a> Parser<'a> { } else if self.token.is_path_start() { // trait item macro. // code copied from parse_macro_use_or_failure... abstraction! + let prev_span = self.prev_span; let lo = self.span; let pth = self.parse_path(PathStyle::Mod)?; - self.expect(&token::Not)?; + + if pth.segments.len() == 1 { + if !self.eat(&token::Not) { + return Err(self.missing_assoc_item_kind_err("trait", prev_span)); + } + } else { + self.expect(&token::Not)?; + } // eat a matched-delimiter token tree: let (delim, tts) = self.expect_delimited_token_tree()?; @@ -1246,25 +1260,7 @@ impl<'a> Parser<'a> { } else { let (constness, unsafety, abi) = match self.parse_fn_front_matter() { Ok(cua) => cua, - Err(e) => { - loop { - match self.token { - token::Eof => break, - token::CloseDelim(token::Brace) | - token::Semi => { - self.bump(); - break; - } - token::OpenDelim(token::Brace) => { - self.parse_token_tree(); - break; - } - _ => self.bump(), - } - } - - return Err(e); - } + Err(e) => return Err(e), }; let ident = self.parse_ident()?; @@ -1289,11 +1285,13 @@ impl<'a> Parser<'a> { let body = match self.token { token::Semi => { self.bump(); + *at_end = true; debug!("parse_trait_methods(): parsing required method"); None } token::OpenDelim(token::Brace) => { debug!("parse_trait_methods(): parsing provided method"); + *at_end = true; let (inner_attrs, body) = self.parse_inner_attrs_and_block()?; attrs.extend(inner_attrs.iter().cloned()); Some(body) @@ -1315,18 +1313,6 @@ impl<'a> Parser<'a> { }) } - - /// Parse the items in a trait declaration - pub fn parse_trait_items(&mut self) -> PResult<'a, Vec> { - self.parse_unspanned_seq( - &token::OpenDelim(token::Brace), - &token::CloseDelim(token::Brace), - SeqSep::none(), - |p| -> PResult<'a, TraitItem> { - p.parse_trait_item() - }) - } - /// Parse optional return type [ -> TY ] in function decl pub fn parse_ret_ty(&mut self) -> PResult<'a, FunctionRetTy> { if self.eat(&token::RArrow) { @@ -3641,22 +3627,33 @@ impl<'a> Parser<'a> { // // We terminate when we find an unmatched `}` (without consuming it). fn recover_stmt(&mut self) { - self.recover_stmt_(SemiColonMode::Ignore) + self.recover_stmt_(SemiColonMode::Ignore, BlockMode::Ignore) } + // If `break_on_semi` is `Break`, then we will stop consuming tokens after // finding (and consuming) a `;` outside of `{}` or `[]` (note that this is // approximate - it can mean we break too early due to macros, but that // shoud only lead to sub-optimal recovery, not inaccurate parsing). - fn recover_stmt_(&mut self, break_on_semi: SemiColonMode) { + // + // If `break_on_block` is `Break`, then we will stop consuming tokens + // after finding (and consuming) a brace-delimited block. + fn recover_stmt_(&mut self, break_on_semi: SemiColonMode, break_on_block: BlockMode) { let mut brace_depth = 0; let mut bracket_depth = 0; - debug!("recover_stmt_ enter loop"); + let mut in_block = false; + debug!("recover_stmt_ enter loop (semi={:?}, block={:?})", + break_on_semi, break_on_block); loop { debug!("recover_stmt_ loop {:?}", self.token); match self.token { token::OpenDelim(token::DelimToken::Brace) => { brace_depth += 1; self.bump(); + if break_on_block == BlockMode::Break && + brace_depth == 1 && + bracket_depth == 0 { + in_block = true; + } } token::OpenDelim(token::DelimToken::Bracket) => { bracket_depth += 1; @@ -3669,6 +3666,10 @@ impl<'a> Parser<'a> { } brace_depth -= 1; self.bump(); + if in_block && bracket_depth == 0 && brace_depth == 0 { + debug!("recover_stmt_ return - block end {:?}", self.token); + return; + } } token::CloseDelim(token::DelimToken::Bracket) => { bracket_depth -= 1; @@ -3700,7 +3701,7 @@ impl<'a> Parser<'a> { fn parse_stmt_(&mut self, macro_legacy_warnings: bool) -> Option { self.parse_stmt_without_recovery(macro_legacy_warnings).unwrap_or_else(|mut e| { e.emit(); - self.recover_stmt_(SemiColonMode::Break); + self.recover_stmt_(SemiColonMode::Break, BlockMode::Ignore); None }) } @@ -3974,7 +3975,7 @@ impl<'a> Parser<'a> { e.span_suggestion(stmt_span, "try placing this code inside a block", sugg); } Err(mut e) => { - self.recover_stmt_(SemiColonMode::Break); + self.recover_stmt_(SemiColonMode::Break, BlockMode::Ignore); self.cancel(&mut e); } _ => () @@ -4663,7 +4664,7 @@ impl<'a> Parser<'a> { } /// Parse an impl item. - pub fn parse_impl_item(&mut self) -> PResult<'a, ImplItem> { + pub fn parse_impl_item(&mut self, at_end: &mut bool) -> PResult<'a, ImplItem> { maybe_whole!(self, NtImplItem, |x| x); let mut attrs = self.parse_outer_attributes()?; @@ -4686,7 +4687,7 @@ impl<'a> Parser<'a> { self.expect(&token::Semi)?; (name, ast::ImplItemKind::Const(typ, expr)) } else { - let (name, inner_attrs, node) = self.parse_impl_method(&vis)?; + let (name, inner_attrs, node) = self.parse_impl_method(&vis, at_end)?; attrs.extend(inner_attrs); (name, node) }; @@ -4731,43 +4732,50 @@ impl<'a> Parser<'a> { } } + fn missing_assoc_item_kind_err(&mut self, item_type: &str, prev_span: Span) + -> DiagnosticBuilder<'a> + { + // Given this code `path(`, it seems like this is not + // setting the visibility of a macro invocation, but rather + // a mistyped method declaration. + // Create a diagnostic pointing out that `fn` is missing. + // + // x | pub path(&self) { + // | ^ missing `fn`, `type`, or `const` + // pub path( + // ^^ `sp` below will point to this + let sp = prev_span.between(self.prev_span); + let mut err = self.diagnostic().struct_span_err( + sp, + &format!("missing `fn`, `type`, or `const` for {}-item declaration", + item_type)); + err.span_label(sp, &"missing `fn`, `type`, or `const`"); + err + } + /// Parse a method or a macro invocation in a trait impl. - fn parse_impl_method(&mut self, vis: &Visibility) + fn parse_impl_method(&mut self, vis: &Visibility, at_end: &mut bool) -> PResult<'a, (Ident, Vec, ast::ImplItemKind)> { // code copied from parse_macro_use_or_failure... abstraction! if self.token.is_path_start() { // Method macro. let prev_span = self.prev_span; - // Before complaining about trying to set a macro as `pub`, - // check if `!` comes after the path. - let err = self.complain_if_pub_macro_diag(&vis, prev_span); let lo = self.span; let pth = self.parse_path(PathStyle::Mod)?; - let bang_err = self.expect(&token::Not); - if let Err(mut err) = err { - if let Err(mut bang_err) = bang_err { - // Given this code `pub path(`, it seems like this is not setting the - // visibility of a macro invocation, but rather a mistyped method declaration. - // Create a diagnostic pointing out that `fn` is missing. - // - // x | pub path(&self) { - // | ^ missing `fn` for method declaration - - err.cancel(); - bang_err.cancel(); - // pub path( - // ^^ `sp` below will point to this - let sp = prev_span.between(self.prev_span); - err = self.diagnostic() - .struct_span_err(sp, "missing `fn` for method declaration"); - err.span_label(sp, &"missing `fn`"); + if pth.segments.len() == 1 { + if !self.eat(&token::Not) { + return Err(self.missing_assoc_item_kind_err("impl", prev_span)); } - return Err(err); + } else { + self.expect(&token::Not)?; } + self.complain_if_pub_macro(&vis, prev_span); + // eat a matched-delimiter token tree: + *at_end = true; let (delim, tts) = self.expect_delimited_token_tree()?; if delim != token::Brace { self.expect(&token::Semi)? @@ -4781,6 +4789,7 @@ impl<'a> Parser<'a> { let mut generics = self.parse_generics()?; let decl = self.parse_fn_decl_with_self(|p| p.parse_arg())?; generics.where_clause = self.parse_where_clause()?; + *at_end = true; let (inner_attrs, body) = self.parse_inner_attrs_and_block()?; Ok((ident, inner_attrs, ast::ImplItemKind::Method(ast::MethodSig { generics: generics, @@ -4806,8 +4815,21 @@ impl<'a> Parser<'a> { tps.where_clause = self.parse_where_clause()?; - let meths = self.parse_trait_items()?; - Ok((ident, ItemKind::Trait(unsafety, tps, bounds, meths), None)) + self.expect(&token::OpenDelim(token::Brace))?; + let mut trait_items = vec![]; + while !self.eat(&token::CloseDelim(token::Brace)) { + 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.recover_stmt_(SemiColonMode::Break, BlockMode::Break); + } + } + } + } + Ok((ident, ItemKind::Trait(unsafety, tps, bounds, trait_items), None)) } /// Parses items implementations variants @@ -4882,7 +4904,16 @@ impl<'a> Parser<'a> { let mut impl_items = vec![]; while !self.eat(&token::CloseDelim(token::Brace)) { - impl_items.push(self.parse_impl_item()?); + let mut at_end = false; + match self.parse_impl_item(&mut at_end) { + Ok(item) => impl_items.push(item), + Err(mut e) => { + e.emit(); + if !at_end { + self.recover_stmt_(SemiColonMode::Break, BlockMode::Break); + } + } + } } Ok((keywords::Invalid.ident(), diff --git a/src/test/parse-fail/issue-21153.rs b/src/test/parse-fail/issue-21153.rs index c03e0ef73217c..b6d95ffb911ca 100644 --- a/src/test/parse-fail/issue-21153.rs +++ b/src/test/parse-fail/issue-21153.rs @@ -10,7 +10,6 @@ // compile-flags: -Z parse-only -trait MyTrait: Iterator { - Item = T; //~ ERROR expected one of `!` or `::`, found `=` - //~| ERROR expected item, found `=` +trait MyTrait: Iterator { //~ ERROR missing `fn`, `type`, or `const` + Item = T; } diff --git a/src/test/parse-fail/trait-pub-assoc-const.rs b/src/test/parse-fail/trait-pub-assoc-const.rs index adce0d7bbf4b6..6bc24e2081d19 100644 --- a/src/test/parse-fail/trait-pub-assoc-const.rs +++ b/src/test/parse-fail/trait-pub-assoc-const.rs @@ -10,7 +10,7 @@ trait Foo { pub const Foo: u32; - //~^ ERROR expected one of `const`, `extern`, `fn`, `type`, or `unsafe`, found `pub` + //~^ ERROR expected one of `const`, `extern`, `fn`, `type`, `unsafe`, or `}`, found `pub` } fn main() {} diff --git a/src/test/parse-fail/trait-pub-assoc-ty.rs b/src/test/parse-fail/trait-pub-assoc-ty.rs index dab6c433aba4c..493ff087eaf9e 100644 --- a/src/test/parse-fail/trait-pub-assoc-ty.rs +++ b/src/test/parse-fail/trait-pub-assoc-ty.rs @@ -10,7 +10,7 @@ trait Foo { pub type Foo; - //~^ ERROR expected one of `const`, `extern`, `fn`, `type`, or `unsafe`, found `pub` + //~^ ERROR expected one of `const`, `extern`, `fn`, `type`, `unsafe`, or `}`, found `pub` } fn main() {} diff --git a/src/test/parse-fail/trait-pub-method.rs b/src/test/parse-fail/trait-pub-method.rs index 7cb9363830c43..c2ee3c31d885b 100644 --- a/src/test/parse-fail/trait-pub-method.rs +++ b/src/test/parse-fail/trait-pub-method.rs @@ -10,7 +10,7 @@ trait Foo { pub fn foo(); - //~^ ERROR expected one of `const`, `extern`, `fn`, `type`, or `unsafe`, found `pub` + //~^ ERROR expected one of `const`, `extern`, `fn`, `type`, `unsafe`, or `}`, found `pub` } fn main() {} diff --git a/src/test/ui/did_you_mean/issue-40006.rs b/src/test/ui/did_you_mean/issue-40006.rs index cf75929bae20c..d68c25faa8a84 100644 --- a/src/test/ui/did_you_mean/issue-40006.rs +++ b/src/test/ui/did_you_mean/issue-40006.rs @@ -8,8 +8,20 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. +impl X { + Y +} + struct S; +trait X { + X() {} + fn xxx() { ### } + L = M; + Z = { 2 + 3 }; + ::Y (); +} + impl S { pub hello_method(&self) { println!("Hello"); diff --git a/src/test/ui/did_you_mean/issue-40006.stderr b/src/test/ui/did_you_mean/issue-40006.stderr index 460958027ad0f..29ff0cee3af5c 100644 --- a/src/test/ui/did_you_mean/issue-40006.stderr +++ b/src/test/ui/did_you_mean/issue-40006.stderr @@ -1,8 +1,68 @@ -error: missing `fn` for method declaration - --> $DIR/issue-40006.rs:14:8 +error: missing `fn`, `type`, or `const` for impl-item declaration + --> $DIR/issue-40006.rs:11:9 | -14 | pub hello_method(&self) { - | ^ missing `fn` +11 | impl X { + | _________^ starting here... +12 | | Y + | |____^ ...ending here: missing `fn`, `type`, or `const` + +error: missing `fn`, `type`, or `const` for trait-item declaration + --> $DIR/issue-40006.rs:17:10 + | +17 | trait X { + | __________^ starting here... +18 | | X() {} + | |____^ ...ending here: missing `fn`, `type`, or `const` + +error: expected `[`, found `#` + --> $DIR/issue-40006.rs:19:17 + | +19 | fn xxx() { ### } + | ^ + +error: missing `fn`, `type`, or `const` for trait-item declaration + --> $DIR/issue-40006.rs:19:21 + | +19 | fn xxx() { ### } + | _____________________^ starting here... +20 | | L = M; + | |____^ ...ending here: missing `fn`, `type`, or `const` + +error: missing `fn`, `type`, or `const` for trait-item declaration + --> $DIR/issue-40006.rs:20:11 + | +20 | L = M; + | ___________^ starting here... +21 | | Z = { 2 + 3 }; + | |____^ ...ending here: missing `fn`, `type`, or `const` + +error: expected one of `const`, `extern`, `fn`, `type`, `unsafe`, or `}`, found `;` + --> $DIR/issue-40006.rs:21:18 + | +21 | Z = { 2 + 3 }; + | ^ expected one of `const`, `extern`, `fn`, `type`, `unsafe`, or `}` here + +error: expected one of `!` or `::`, found `(` + --> $DIR/issue-40006.rs:22:9 + | +22 | ::Y (); + | -^ unexpected token + | | + | expected one of `!` or `::` here + +error: missing `fn`, `type`, or `const` for impl-item declaration + --> $DIR/issue-40006.rs:26:8 + | +26 | pub hello_method(&self) { + | ^ missing `fn`, `type`, or `const` + +error[E0038]: the trait `X` cannot be made into an object + --> $DIR/issue-40006.rs:11:6 + | +11 | impl X { + | ^ the trait `X` cannot be made into an object + | + = note: method `xxx` has no receiver error: aborting due to previous error diff --git a/src/test/ui/token/issue-41155.stderr b/src/test/ui/token/issue-41155.stderr index 0da3abd4eaf9c..a6ad1206b14d0 100644 --- a/src/test/ui/token/issue-41155.stderr +++ b/src/test/ui/token/issue-41155.stderr @@ -6,5 +6,13 @@ error: expected one of `(`, `const`, `default`, `extern`, `fn`, `type`, or `unsa 13 | } | ^ unexpected token -error: aborting due to previous error +error[E0412]: cannot find type `S` in this scope + --> $DIR/issue-41155.rs:11:6 + | +11 | impl S { + | ^ not found in this scope + +error: main function not found + +error: aborting due to 3 previous errors