Skip to content

Commit

Permalink
Rollup merge of rust-lang#91531 - notriddle:notriddle/issue-87647-exp…
Browse files Browse the repository at this point in the history
…ected-semicolon, r=estebank

Do not add `;` to expected tokens list when it's wrong

There's a few spots where semicolons are checked for to do error recovery, and should not be suggested (or checked for other stuff).

Fixes rust-lang#87647
  • Loading branch information
matthiaskrgr authored Dec 7, 2021
2 parents c31fc42 + 6611567 commit 915fb2a
Show file tree
Hide file tree
Showing 19 changed files with 165 additions and 51 deletions.
4 changes: 2 additions & 2 deletions compiler/rustc_parse/src/parser/attr.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::{AttrWrapper, Capturing, ForceCollect, Parser, PathStyle};
use super::{AttrWrapper, Capturing, FnParseMode, ForceCollect, Parser, PathStyle};
use rustc_ast as ast;
use rustc_ast::attr;
use rustc_ast::token::{self, Nonterminal};
Expand Down Expand Up @@ -177,7 +177,7 @@ impl<'a> Parser<'a> {
AttrWrapper::empty(),
true,
false,
|_| true,
FnParseMode { req_name: |_| true, req_body: true },
ForceCollect::No,
) {
Ok(Some(item)) => {
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1129,7 +1129,8 @@ impl<'a> Parser<'a> {
}

pub fn maybe_consume_incorrect_semicolon(&mut self, items: &[P<Item>]) -> bool {
if self.eat(&token::Semi) {
if self.token.kind == TokenKind::Semi {
self.bump();
let mut err = self.struct_span_err(self.prev_token.span, "expected item, found `;`");
err.span_suggestion_short(
self.prev_token.span,
Expand Down
120 changes: 98 additions & 22 deletions compiler/rustc_parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,24 +78,25 @@ pub(super) type ItemInfo = (Ident, ItemKind);

impl<'a> Parser<'a> {
pub fn parse_item(&mut self, force_collect: ForceCollect) -> PResult<'a, Option<P<Item>>> {
self.parse_item_(|_| true, force_collect).map(|i| i.map(P))
let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: true };
self.parse_item_(fn_parse_mode, force_collect).map(|i| i.map(P))
}

fn parse_item_(
&mut self,
req_name: ReqName,
fn_parse_mode: FnParseMode,
force_collect: ForceCollect,
) -> PResult<'a, Option<Item>> {
let attrs = self.parse_outer_attributes()?;
self.parse_item_common(attrs, true, false, req_name, force_collect)
self.parse_item_common(attrs, true, false, fn_parse_mode, force_collect)
}

pub(super) fn parse_item_common(
&mut self,
attrs: AttrWrapper,
mac_allowed: bool,
attrs_allowed: bool,
req_name: ReqName,
fn_parse_mode: FnParseMode,
force_collect: ForceCollect,
) -> PResult<'a, Option<Item>> {
// Don't use `maybe_whole` so that we have precise control
Expand All @@ -113,7 +114,8 @@ impl<'a> Parser<'a> {
let mut unclosed_delims = vec![];
let item =
self.collect_tokens_trailing_token(attrs, force_collect, |this: &mut Self, attrs| {
let item = this.parse_item_common_(attrs, mac_allowed, attrs_allowed, req_name);
let item =
this.parse_item_common_(attrs, mac_allowed, attrs_allowed, fn_parse_mode);
unclosed_delims.append(&mut this.unclosed_delims);
Ok((item?, TrailingToken::None))
})?;
Expand All @@ -127,12 +129,13 @@ impl<'a> Parser<'a> {
mut attrs: Vec<Attribute>,
mac_allowed: bool,
attrs_allowed: bool,
req_name: ReqName,
fn_parse_mode: FnParseMode,
) -> PResult<'a, Option<Item>> {
let lo = self.token.span;
let vis = self.parse_visibility(FollowedByType::No)?;
let mut def = self.parse_defaultness();
let kind = self.parse_item_kind(&mut attrs, mac_allowed, lo, &vis, &mut def, req_name)?;
let kind =
self.parse_item_kind(&mut attrs, mac_allowed, lo, &vis, &mut def, fn_parse_mode)?;
if let Some((ident, kind)) = kind {
self.error_on_unconsumed_default(def, &kind);
let span = lo.to(self.prev_token.span);
Expand Down Expand Up @@ -192,7 +195,7 @@ impl<'a> Parser<'a> {
lo: Span,
vis: &Visibility,
def: &mut Defaultness,
req_name: ReqName,
fn_parse_mode: FnParseMode,
) -> PResult<'a, Option<ItemInfo>> {
let def_final = def == &Defaultness::Final;
let mut def = || mem::replace(def, Defaultness::Final);
Expand All @@ -219,7 +222,7 @@ impl<'a> Parser<'a> {
(Ident::empty(), ItemKind::Use(tree))
} else if self.check_fn_front_matter(def_final) {
// FUNCTION ITEM
let (ident, sig, generics, body) = self.parse_fn(attrs, req_name, lo)?;
let (ident, sig, generics, body) = self.parse_fn(attrs, fn_parse_mode, lo)?;
(ident, ItemKind::Fn(Box::new(Fn { defaultness: def(), sig, generics, body })))
} else if self.eat_keyword(kw::Extern) {
if self.eat_keyword(kw::Crate) {
Expand Down Expand Up @@ -733,23 +736,26 @@ impl<'a> Parser<'a> {
&mut self,
force_collect: ForceCollect,
) -> PResult<'a, Option<Option<P<AssocItem>>>> {
self.parse_assoc_item(|_| true, force_collect)
let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: true };
self.parse_assoc_item(fn_parse_mode, force_collect)
}

pub fn parse_trait_item(
&mut self,
force_collect: ForceCollect,
) -> PResult<'a, Option<Option<P<AssocItem>>>> {
self.parse_assoc_item(|edition| edition >= Edition::Edition2018, force_collect)
let fn_parse_mode =
FnParseMode { req_name: |edition| edition >= Edition::Edition2018, req_body: false };
self.parse_assoc_item(fn_parse_mode, force_collect)
}

/// Parses associated items.
fn parse_assoc_item(
&mut self,
req_name: ReqName,
fn_parse_mode: FnParseMode,
force_collect: ForceCollect,
) -> PResult<'a, Option<Option<P<AssocItem>>>> {
Ok(self.parse_item_(req_name, force_collect)?.map(
Ok(self.parse_item_(fn_parse_mode, force_collect)?.map(
|Item { attrs, id, span, vis, ident, kind, tokens }| {
let kind = match AssocItemKind::try_from(kind) {
Ok(kind) => kind,
Expand Down Expand Up @@ -944,7 +950,8 @@ impl<'a> Parser<'a> {
&mut self,
force_collect: ForceCollect,
) -> PResult<'a, Option<Option<P<ForeignItem>>>> {
Ok(self.parse_item_(|_| true, force_collect)?.map(
let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: false };
Ok(self.parse_item_(fn_parse_mode, force_collect)?.map(
|Item { attrs, id, span, vis, ident, kind, tokens }| {
let kind = match ForeignItemKind::try_from(kind) {
Ok(kind) => kind,
Expand Down Expand Up @@ -1484,7 +1491,8 @@ impl<'a> Parser<'a> {
if !is_raw && ident.is_reserved() {
let err = if self.check_fn_front_matter(false) {
// We use `parse_fn` to get a span for the function
if let Err(mut db) = self.parse_fn(&mut Vec::new(), |_| true, lo) {
let fn_parse_mode = FnParseMode { req_name: |_| true, req_body: true };
if let Err(mut db) = self.parse_fn(&mut Vec::new(), fn_parse_mode, lo) {
db.delay_as_bug();
}
let mut err = self.struct_span_err(
Expand Down Expand Up @@ -1698,25 +1706,82 @@ impl<'a> Parser<'a> {
/// The parsing configuration used to parse a parameter list (see `parse_fn_params`).
///
/// The function decides if, per-parameter `p`, `p` must have a pattern or just a type.
///
/// This function pointer accepts an edition, because in edition 2015, trait declarations
/// were allowed to omit parameter names. In 2018, they became required.
type ReqName = fn(Edition) -> bool;

/// Parsing configuration for functions.
///
/// The syntax of function items is slightly different within trait definitions,
/// impl blocks, and modules. It is still parsed using the same code, just with
/// different flags set, so that even when the input is wrong and produces a parse
/// error, it still gets into the AST and the rest of the parser and
/// type checker can run.
#[derive(Clone, Copy)]
pub(crate) struct FnParseMode {
/// A function pointer that decides if, per-parameter `p`, `p` must have a
/// pattern or just a type. This field affects parsing of the parameters list.
///
/// ```text
/// fn foo(alef: A) -> X { X::new() }
/// -----^^ affects parsing this part of the function signature
/// |
/// if req_name returns false, then this name is optional
///
/// fn bar(A) -> X;
/// ^
/// |
/// if req_name returns true, this is an error
/// ```
///
/// Calling this function pointer should only return false if:
///
/// * The item is being parsed inside of a trait definition.
/// Within an impl block or a module, it should always evaluate
/// to true.
/// * The span is from Edition 2015. In particular, you can get a
/// 2015 span inside a 2021 crate using macros.
pub req_name: ReqName,
/// If this flag is set to `true`, then plain, semicolon-terminated function
/// prototypes are not allowed here.
///
/// ```text
/// fn foo(alef: A) -> X { X::new() }
/// ^^^^^^^^^^^^
/// |
/// this is always allowed
///
/// fn bar(alef: A, bet: B) -> X;
/// ^
/// |
/// if req_body is set to true, this is an error
/// ```
///
/// This field should only be set to false if the item is inside of a trait
/// definition or extern block. Within an impl block or a module, it should
/// always be set to true.
pub req_body: bool,
}

/// Parsing of functions and methods.
impl<'a> Parser<'a> {
/// Parse a function starting from the front matter (`const ...`) to the body `{ ... }` or `;`.
fn parse_fn(
&mut self,
attrs: &mut Vec<Attribute>,
req_name: ReqName,
fn_parse_mode: FnParseMode,
sig_lo: Span,
) -> PResult<'a, (Ident, FnSig, Generics, Option<P<Block>>)> {
let header = self.parse_fn_front_matter()?; // `const ... fn`
let ident = self.parse_ident()?; // `foo`
let mut generics = self.parse_generics()?; // `<'a, T, ...>`
let decl = self.parse_fn_decl(req_name, AllowPlus::Yes, RecoverReturnSign::Yes)?; // `(p: u8, ...)`
let decl =
self.parse_fn_decl(fn_parse_mode.req_name, AllowPlus::Yes, RecoverReturnSign::Yes)?; // `(p: u8, ...)`
generics.where_clause = self.parse_where_clause()?; // `where T: Ord`

let mut sig_hi = self.prev_token.span;
let body = self.parse_fn_body(attrs, &ident, &mut sig_hi)?; // `;` or `{ ... }`.
let body = self.parse_fn_body(attrs, &ident, &mut sig_hi, fn_parse_mode.req_body)?; // `;` or `{ ... }`.
let fn_sig_span = sig_lo.to(sig_hi);
Ok((ident, FnSig { header, decl, span: fn_sig_span }, generics, body))
}
Expand All @@ -1729,9 +1794,17 @@ impl<'a> Parser<'a> {
attrs: &mut Vec<Attribute>,
ident: &Ident,
sig_hi: &mut Span,
req_body: bool,
) -> PResult<'a, Option<P<Block>>> {
let (inner_attrs, body) = if self.eat(&token::Semi) {
let has_semi = if req_body {
self.token.kind == TokenKind::Semi
} else {
// Only include `;` in list of expected tokens if body is not required
self.check(&TokenKind::Semi)
};
let (inner_attrs, body) = if has_semi {
// Include the trailing semicolon in the span of the signature
self.expect_semi()?;
*sig_hi = self.prev_token.span;
(Vec::new(), None)
} else if self.check(&token::OpenDelim(token::Brace)) || self.token.is_whole_block() {
Expand All @@ -1752,9 +1825,12 @@ impl<'a> Parser<'a> {
.emit();
(Vec::new(), Some(self.mk_block_err(span)))
} else {
if let Err(mut err) =
self.expected_one_of_not_found(&[], &[token::Semi, token::OpenDelim(token::Brace)])
{
let expected = if req_body {
&[token::OpenDelim(token::Brace)][..]
} else {
&[token::Semi, token::OpenDelim(token::Brace)]
};
if let Err(mut err) = self.expected_one_of_not_found(&[], &expected) {
if self.token.kind == token::CloseDelim(token::Brace) {
// The enclosing `mod`, `trait` or `impl` is being closed, so keep the `fn` in
// the AST for typechecking.
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ use crate::lexer::UnmatchedBrace;
pub use attr_wrapper::AttrWrapper;
pub use diagnostics::AttemptLocalParseRecovery;
use diagnostics::Error;
pub(crate) use item::FnParseMode;
pub use pat::{RecoverColon, RecoverComma};
pub use path::PathStyle;

Expand Down
14 changes: 10 additions & 4 deletions compiler/rustc_parse/src/parser/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ use super::expr::LhsExpr;
use super::pat::RecoverComma;
use super::path::PathStyle;
use super::TrailingToken;
use super::{AttrWrapper, BlockMode, ForceCollect, Parser, Restrictions, SemiColonMode};
use super::{
AttrWrapper, BlockMode, FnParseMode, ForceCollect, Parser, Restrictions, SemiColonMode,
};
use crate::maybe_whole;

use rustc_ast as ast;
Expand Down Expand Up @@ -79,9 +81,13 @@ impl<'a> Parser<'a> {
} else {
self.parse_stmt_path_start(lo, attrs)
}?
} else if let Some(item) =
self.parse_item_common(attrs.clone(), false, true, |_| true, force_collect)?
{
} else if let Some(item) = self.parse_item_common(
attrs.clone(),
false,
true,
FnParseMode { req_name: |_| true, req_body: true },
force_collect,
)? {
// FIXME: Bad copy of attrs
self.mk_stmt(lo.to(item.span), StmtKind::Item(P(item)))
} else if self.eat(&token::Semi) {
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/fn/fn-recover-return-sign2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@

fn foo() => impl Fn() => bool {
//~^ ERROR return types are denoted using `->`
//~| ERROR expected one of `+`, `->`, `::`, `;`, `where`, or `{`, found `=>`
//~| ERROR expected one of `+`, `->`, `::`, `where`, or `{`, found `=>`
unimplemented!()
}
4 changes: 2 additions & 2 deletions src/test/ui/fn/fn-recover-return-sign2.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ error: return types are denoted using `->`
LL | fn foo() => impl Fn() => bool {
| ^^ help: use `->` instead

error: expected one of `+`, `->`, `::`, `;`, `where`, or `{`, found `=>`
error: expected one of `+`, `->`, `::`, `where`, or `{`, found `=>`
--> $DIR/fn-recover-return-sign2.rs:4:23
|
LL | fn foo() => impl Fn() => bool {
| ^^ expected one of `+`, `->`, `::`, `;`, `where`, or `{`
| ^^ expected one of `+`, `->`, `::`, `where`, or `{`

error: aborting due to 2 previous errors

2 changes: 1 addition & 1 deletion src/test/ui/parser/issues/issue-24780.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// to happen in #24780. For example, following should be an error:
// expected one of ..., `>`, ... found `>`.

fn foo() -> Vec<usize>> { //~ ERROR expected one of `!`, `+`, `::`, `;`, `where`, or `{`, found `>`
fn foo() -> Vec<usize>> { //~ ERROR expected one of `!`, `+`, `::`, `where`, or `{`, found `>`
Vec::new()
}

Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/parser/issues/issue-24780.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: expected one of `!`, `+`, `::`, `;`, `where`, or `{`, found `>`
error: expected one of `!`, `+`, `::`, `where`, or `{`, found `>`
--> $DIR/issue-24780.rs:5:23
|
LL | fn foo() -> Vec<usize>> {
| ^ expected one of `!`, `+`, `::`, `;`, `where`, or `{`
| ^ expected one of `!`, `+`, `::`, `where`, or `{`

error: aborting due to previous error

2 changes: 1 addition & 1 deletion src/test/ui/parser/issues/issue-58856-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ impl A {
//~^ ERROR cannot find type `A` in this scope
fn b(self>
//~^ ERROR expected one of `)`, `,`, or `:`, found `>`
//~| ERROR expected one of `->`, `;`, `where`, or `{`, found `>`
//~| ERROR expected one of `->`, `where`, or `{`, found `>`
}

fn main() {}
4 changes: 2 additions & 2 deletions src/test/ui/parser/issues/issue-58856-1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ LL | fn b(self>
| |
| unclosed delimiter

error: expected one of `->`, `;`, `where`, or `{`, found `>`
error: expected one of `->`, `where`, or `{`, found `>`
--> $DIR/issue-58856-1.rs:3:14
|
LL | impl A {
| - while parsing this item list starting here
LL |
LL | fn b(self>
| ^ expected one of `->`, `;`, `where`, or `{`
| ^ expected one of `->`, `where`, or `{`
...
LL | }
| - the item list ends here
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/parser/issues/issue-84148-1.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ LL | fn f(t:for<>t?)
| expected one of `(`, `)`, `+`, `,`, `::`, or `<`
| help: missing `,`

error: expected one of `->`, `;`, `where`, or `{`, found `<eof>`
error: expected one of `->`, `where`, or `{`, found `<eof>`
--> $DIR/issue-84148-1.rs:1:15
|
LL | fn f(t:for<>t?)
| ^ expected one of `->`, `;`, `where`, or `{`
| ^ expected one of `->`, `where`, or `{`

error: aborting due to 3 previous errors

Loading

0 comments on commit 915fb2a

Please sign in to comment.