Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve parser diagnostics #95211

Merged
merged 1 commit into from
Jun 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ use std::ops::{Deref, DerefMut};

use std::mem::take;

use crate::parser;
use tracing::{debug, trace};

const TURBOFISH_SUGGESTION_STR: &str =
Expand Down Expand Up @@ -400,6 +401,35 @@ impl<'a> Parser<'a> {
.map(|x| TokenType::Token(x.clone()))
.chain(inedible.iter().map(|x| TokenType::Token(x.clone())))
.chain(self.expected_tokens.iter().cloned())
.filter_map(|token| {
// filter out suggestions which suggest the same token which was found and deemed incorrect
fn is_ident_eq_keyword(found: &TokenKind, expected: &TokenType) -> bool {
if let TokenKind::Ident(current_sym, _) = found {
if let TokenType::Keyword(suggested_sym) = expected {
return current_sym == suggested_sym;
}
}
false
}
if token != parser::TokenType::Token(self.token.kind.clone()) {
let eq = is_ident_eq_keyword(&self.token.kind, &token);
// if the suggestion is a keyword and the found token is an ident,
// the content of which are equal to the suggestion's content,
// we can remove that suggestion (see the return None statement below)

// if this isn't the case however, and the suggestion is a token the
// content of which is the same as the found token's, we remove it as well
if !eq {
if let TokenType::Token(kind) = &token {
if kind == &self.token.kind {
return None;
}
}
return Some(token);
}
}
return None;
})
.collect::<Vec<_>>();
expected.sort_by_cached_key(|x| x.to_string());
expected.dedup();
Expand Down
26 changes: 22 additions & 4 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -977,12 +977,26 @@ impl<'a> Parser<'a> {

fn parse_dot_or_call_expr_with_(&mut self, mut e: P<Expr>, lo: Span) -> PResult<'a, P<Expr>> {
loop {
if self.eat(&token::Question) {
let has_question = if self.prev_token.kind == TokenKind::Ident(kw::Return, false) {
// we are using noexpect here because we don't expect a `?` directly after a `return`
// which could be suggested otherwise
self.eat_noexpect(&token::Question)
terrarier2111 marked this conversation as resolved.
Show resolved Hide resolved
} else {
self.eat(&token::Question)
};
if has_question {
// `expr?`
e = self.mk_expr(lo.to(self.prev_token.span), ExprKind::Try(e), AttrVec::new());
continue;
}
if self.eat(&token::Dot) {
let has_dot = if self.prev_token.kind == TokenKind::Ident(kw::Return, false) {
// we are using noexpect here because we don't expect a `.` directly after a `return`
// which could be suggested otherwise
self.eat_noexpect(&token::Dot)
} else {
self.eat(&token::Dot)
};
if has_dot {
// expr.f
e = self.parse_dot_suffix_expr(lo, e)?;
continue;
Expand Down Expand Up @@ -1536,9 +1550,13 @@ impl<'a> Parser<'a> {
self.parse_for_expr(label, lo, attrs)
} else if self.eat_keyword(kw::Loop) {
self.parse_loop_expr(label, lo, attrs)
} else if self.check(&token::OpenDelim(Delimiter::Brace)) || self.token.is_whole_block() {
} else if self.check_noexpect(&token::OpenDelim(Delimiter::Brace))
|| self.token.is_whole_block()
{
self.parse_block_expr(label, lo, BlockCheckMode::Default, attrs)
} else if !ate_colon && (self.check(&TokenKind::Comma) || self.check(&TokenKind::Gt)) {
} else if !ate_colon
&& (self.check_noexpect(&TokenKind::Comma) || self.check_noexpect(&TokenKind::Gt))
{
// We're probably inside of a `Path<'a>` that needs a turbofish
let msg = "expected `while`, `for`, `loop` or `{` after a label";
self.struct_span_err(self.token.span, msg).span_label(self.token.span, msg).emit();
Expand Down
16 changes: 16 additions & 0 deletions compiler/rustc_parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,22 @@ impl<'a> Parser<'a> {
is_present
}

fn check_noexpect(&self, tok: &TokenKind) -> bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you double check if the parser has other self.token == use cases that can be turned into check_noexpect?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think that i am able to do that as from what I was able to see that would require a decent amount of work and I just wanted to finish up the changes in this pr.

self.token == *tok
}

/// Consumes a token 'tok' if it exists. Returns whether the given token was present.
///
/// the main purpose of this function is to reduce the cluttering of the suggestions list
/// which using the normal eat method could introduce in some cases.
pub fn eat_noexpect(&mut self, tok: &TokenKind) -> bool {
let is_present = self.check_noexpect(tok);
if is_present {
self.bump()
}
is_present
}

/// Consumes a token 'tok' if it exists. Returns whether the given token was present.
pub fn eat(&mut self, tok: &TokenKind) -> bool {
let is_present = self.check(tok);
Expand Down
20 changes: 16 additions & 4 deletions compiler/rustc_parse/src/parser/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use super::ty::{AllowPlus, RecoverQPath, RecoverReturnSign};
use super::{Parser, Restrictions, TokenType};
use crate::maybe_whole;
use rustc_ast::ptr::P;
use rustc_ast::token::{self, Delimiter, Token};
use rustc_ast::token::{self, Delimiter, Token, TokenKind};
use rustc_ast::{
self as ast, AngleBracketedArg, AngleBracketedArgs, AnonConst, AssocConstraint,
AssocConstraintKind, BlockCheckMode, GenericArg, GenericArgs, Generics, ParenthesizedArgs,
Expand Down Expand Up @@ -96,7 +96,7 @@ impl<'a> Parser<'a> {
/// ^ help: use double colon
/// ```
fn recover_colon_before_qpath_proj(&mut self) -> bool {
if self.token.kind != token::Colon
if !self.check_noexpect(&TokenKind::Colon)
|| self.look_ahead(1, |t| !t.is_ident() || t.is_reserved_ident())
{
return false;
Expand Down Expand Up @@ -478,7 +478,7 @@ impl<'a> Parser<'a> {
while let Some(arg) = self.parse_angle_arg(ty_generics)? {
args.push(arg);
if !self.eat(&token::Comma) {
if self.token.kind == token::Semi
if self.check_noexpect(&TokenKind::Semi)
&& self.look_ahead(1, |t| t.is_ident() || t.is_lifetime())
{
// Add `>` to the list of expected tokens.
Expand Down Expand Up @@ -517,7 +517,11 @@ impl<'a> Parser<'a> {
let arg = self.parse_generic_arg(ty_generics)?;
match arg {
Some(arg) => {
if self.check(&token::Colon) | self.check(&token::Eq) {
// we are using noexpect here because we first want to find out if either `=` or `:`
// is present and then use that info to push the other token onto the tokens list
let separated =
self.check_noexpect(&token::Colon) || self.check_noexpect(&token::Eq);
if separated && (self.check(&token::Colon) | self.check(&token::Eq)) {
compiler-errors marked this conversation as resolved.
Show resolved Hide resolved
let arg_span = arg.span();
let (binder, ident, gen_args) = match self.get_ident_from_generic_arg(&arg) {
Ok(ident_gen_args) => ident_gen_args,
Expand Down Expand Up @@ -553,6 +557,14 @@ impl<'a> Parser<'a> {
AssocConstraint { id: ast::DUMMY_NODE_ID, ident, gen_args, kind, span };
Ok(Some(AngleBracketedArg::Constraint(constraint)))
} else {
// we only want to suggest `:` and `=` in contexts where the previous token
// is an ident and the current token or the next token is an ident
if self.prev_token.is_ident()
&& (self.token.is_ident() || self.look_ahead(1, |token| token.is_ident()))
{
self.check(&token::Colon);
self.check(&token::Eq);
}
Ok(Some(AngleBracketedArg::Arg(arg)))
}
}
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_parse/src/parser/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -262,7 +262,10 @@ impl<'a> Parser<'a> {
if let Ok(snip) = self.span_to_snippet(pat.span) {
err.span_label(pat.span, format!("while parsing the type for `{}`", snip));
}
let err = if self.check(&token::Eq) {
// we use noexpect here because we don't actually expect Eq to be here
// but we are still checking for it in order to be able to handle it if
// it is there
let err = if self.check_noexpect(&token::Eq) {
err.emit();
None
} else {
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/parser/can-begin-expr-check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,5 +16,5 @@ pub fn main() {
return break as ();
}

return enum; //~ ERROR expected one of `.`, `;`, `?`, `}`, or an operator, found keyword `enum`
return enum; //~ ERROR expected one of `;`, `}`, or an operator, found keyword `enum`
}
4 changes: 2 additions & 2 deletions src/test/ui/parser/can-begin-expr-check.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: expected one of `.`, `;`, `?`, `}`, or an operator, found keyword `enum`
error: expected one of `;`, `}`, or an operator, found keyword `enum`
--> $DIR/can-begin-expr-check.rs:19:12
|
LL | return enum;
| ^^^^ expected one of `.`, `;`, `?`, `}`, or an operator
| ^^^^ expected one of `;`, `}`, or an operator

error: aborting due to previous error

4 changes: 2 additions & 2 deletions src/test/ui/parser/duplicate-visibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ fn main() {}

extern "C" { //~ NOTE while parsing this item list starting here
pub pub fn foo();
//~^ ERROR expected one of `(`, `async`, `const`, `default`, `extern`, `fn`, `pub`, `unsafe`, or `use`, found keyword `pub`
//~| NOTE expected one of 9 possible tokens
//~^ ERROR expected one of `(`, `async`, `const`, `default`, `extern`, `fn`, `unsafe`, or `use`, found keyword `pub`
//~| NOTE expected one of 8 possible tokens
//~| HELP there is already a visibility modifier, remove one
//~| NOTE explicit visibility first seen here
} //~ NOTE the item list ends here
4 changes: 2 additions & 2 deletions src/test/ui/parser/duplicate-visibility.stderr
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
error: expected one of `(`, `async`, `const`, `default`, `extern`, `fn`, `pub`, `unsafe`, or `use`, found keyword `pub`
error: expected one of `(`, `async`, `const`, `default`, `extern`, `fn`, `unsafe`, or `use`, found keyword `pub`
--> $DIR/duplicate-visibility.rs:4:9
|
LL | extern "C" {
| - while parsing this item list starting here
LL | pub pub fn foo();
| ^^^
| |
| expected one of 9 possible tokens
| expected one of 8 possible tokens
| help: there is already a visibility modifier, remove one
...
LL | }
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/parser/issues/issue-20616-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ type Type_1_<'a, T> = &'a T;
//type Type_1<'a T> = &'a T; // error: expected `,` or `>` after lifetime name, found `T`


type Type_2 = Type_1_<'static ()>; //~ error: expected one of `,`, `:`, `=`, or `>`, found `(`
type Type_2 = Type_1_<'static ()>; //~ error: expected one of `,` or `>`, found `(`


//type Type_3<T> = Box<T,,>; // error: expected type, found `,`
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/parser/issues/issue-20616-2.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: expected one of `,`, `:`, `=`, or `>`, found `(`
error: expected one of `,` or `>`, found `(`
--> $DIR/issue-20616-2.rs:12:31
|
LL | type Type_2 = Type_1_<'static ()>;
| ^ expected one of `,`, `:`, `=`, or `>`
| ^ expected one of `,` or `>`
|
help: you might have meant to end the type parameters here
|
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/parser/issues/issue-62660.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ struct Foo;

impl Foo {
pub fn foo(_: i32, self: Box<Self) {}
//~^ ERROR expected one of `!`, `(`, `+`, `,`, `::`, `:`, `<`, `=`, or `>`, found `)`
//~^ ERROR expected one of `!`, `(`, `+`, `,`, `::`, `<`, or `>`, found `)`
}

fn main() {}
4 changes: 2 additions & 2 deletions src/test/ui/parser/issues/issue-62660.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: expected one of `!`, `(`, `+`, `,`, `::`, `:`, `<`, `=`, or `>`, found `)`
error: expected one of `!`, `(`, `+`, `,`, `::`, `<`, or `>`, found `)`
--> $DIR/issue-62660.rs:7:38
|
LL | pub fn foo(_: i32, self: Box<Self) {}
| ^ expected one of 9 possible tokens
| ^ expected one of 7 possible tokens
|
help: you might have meant to end the type parameters here
|
Expand Down
2 changes: 1 addition & 1 deletion src/test/ui/parser/issues/issue-84117.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,4 @@ fn main() {
//~| ERROR expected one of `!`, `.`, `::`, `;`, `?`, `else`, `{`, or an operator, found `,`
//~| ERROR expected one of `!`, `.`, `::`, `;`, `?`, `else`, `{`, or an operator, found `,`
}
//~^ ERROR expected one of `,`, `:`, `=`, or `>`, found `}`
//~^ ERROR expected one of `,` or `>`, found `}`
4 changes: 2 additions & 2 deletions src/test/ui/parser/issues/issue-84117.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ error: expected one of `!`, `.`, `::`, `;`, `?`, `else`, `{`, or an operator, fo
LL | let outer_local:e_outer<&str, { let inner_local:e_inner<&str, }
| ^ expected one of 8 possible tokens

error: expected one of `,`, `:`, `=`, or `>`, found `}`
error: expected one of `,` or `>`, found `}`
--> $DIR/issue-84117.rs:8:1
|
LL | let outer_local:e_outer<&str, { let inner_local:e_inner<&str, }
| ----------- while parsing the type for `outer_local` - expected one of `,`, `:`, `=`, or `>`
| ----------- while parsing the type for `outer_local` - expected one of `,` or `>`
...
LL | }
| ^ unexpected token
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/parser/issues/issue-93282.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ error: expected `while`, `for`, `loop` or `{` after a label
LL | f<'a,>
| ^ expected `while`, `for`, `loop` or `{` after a label

error: expected one of `.`, `:`, `;`, `?`, `for`, `loop`, `while`, `{`, `}`, or an operator, found `,`
error: expected one of `.`, `:`, `;`, `?`, `for`, `loop`, `while`, `}`, or an operator, found `,`
--> $DIR/issue-93282.rs:2:9
|
LL | f<'a,>
| ^ expected one of 10 possible tokens
| ^ expected one of 9 possible tokens
|
help: use `::<...>` instead of `<...>` to specify lifetime, type, or const arguments
|
Expand Down
10 changes: 10 additions & 0 deletions src/test/ui/parser/issues/issue-93867.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
pub struct Entry<'a, K, V> {
k: &'a mut K,
v: V,
}

pub fn entry<'a, K, V>() -> Entry<'a K, V> {
// ^ missing comma
//~^^ expected one of `,` or `>`, found `K`
unimplemented!()
}
13 changes: 13 additions & 0 deletions src/test/ui/parser/issues/issue-93867.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
error: expected one of `,` or `>`, found `K`
--> $DIR/issue-93867.rs:6:38
|
LL | pub fn entry<'a, K, V>() -> Entry<'a K, V> {
| ^ expected one of `,` or `>`
|
help: you might have meant to end the type parameters here
|
LL | pub fn entry<'a, K, V>() -> Entry<'a> K, V> {
| +

error: aborting due to previous error

2 changes: 1 addition & 1 deletion src/test/ui/parser/lifetime-semicolon.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ struct Foo<'a, 'b> {
}

fn foo<'a, 'b>(_x: &mut Foo<'a, 'b>) {}
//~^ ERROR expected one of `,`, `:`, `=`, or `>`, found `;`
//~^ ERROR expected one of `,` or `>`, found `;`

fn main() {}
2 changes: 1 addition & 1 deletion src/test/ui/parser/lifetime-semicolon.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,6 @@ struct Foo<'a, 'b> {
}

fn foo<'a, 'b>(_x: &mut Foo<'a; 'b>) {}
//~^ ERROR expected one of `,`, `:`, `=`, or `>`, found `;`
//~^ ERROR expected one of `,` or `>`, found `;`

fn main() {}
4 changes: 2 additions & 2 deletions src/test/ui/parser/lifetime-semicolon.stderr
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
error: expected one of `,`, `:`, `=`, or `>`, found `;`
error: expected one of `,` or `>`, found `;`
--> $DIR/lifetime-semicolon.rs:7:31
|
LL | fn foo<'a, 'b>(_x: &mut Foo<'a; 'b>) {}
| ^ expected one of `,`, `:`, `=`, or `>`
| ^ expected one of `,` or `>`
|
help: use a comma to separate type parameters
|
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,11 +59,11 @@ error: expected `while`, `for`, `loop` or `{` after a label
LL | let _ = f<'_, i8>();
| ^ expected `while`, `for`, `loop` or `{` after a label

error: expected one of `.`, `:`, `;`, `?`, `else`, `for`, `loop`, `while`, `{`, or an operator, found `,`
error: expected one of `.`, `:`, `;`, `?`, `else`, `for`, `loop`, `while`, or an operator, found `,`
--> $DIR/require-parens-for-chained-comparison.rs:22:17
|
LL | let _ = f<'_, i8>();
| ^ expected one of 10 possible tokens
| ^ expected one of 9 possible tokens
|
help: use `::<...>` instead of `<...>` to specify lifetime, type, or const arguments
|
Expand Down