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

Recover when unclosed char literal is parsed as a lifetime in some positions #101293

Merged
merged 1 commit into from
Oct 23, 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
3 changes: 3 additions & 0 deletions compiler/rustc_errors/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,9 @@ pub enum StashKey {
UnderscoreForArrayLengths,
EarlySyntaxWarning,
CallIntoMethod,
/// When an invalid lifetime e.g. `'2` should be reinterpreted
/// as a char literal in the parser
LifetimeIsChar,
}

fn default_track_diagnostic(_: &Diagnostic) {}
Expand Down
9 changes: 7 additions & 2 deletions compiler/rustc_parse/src/lexer/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ use rustc_ast::ast::{self, AttrStyle};
use rustc_ast::token::{self, CommentKind, Delimiter, Token, TokenKind};
use rustc_ast::tokenstream::TokenStream;
use rustc_ast::util::unicode::contains_text_flow_control_chars;
use rustc_errors::{error_code, Applicability, DiagnosticBuilder, ErrorGuaranteed, PResult};
use rustc_errors::{
error_code, Applicability, DiagnosticBuilder, ErrorGuaranteed, PResult, StashKey,
};
use rustc_lexer::unescape::{self, Mode};
use rustc_lexer::Cursor;
use rustc_lexer::{Base, DocStyle, RawStrError};
Expand Down Expand Up @@ -203,7 +205,10 @@ impl<'a> StringReader<'a> {
// this is necessary.
let lifetime_name = self.str_from(start);
if starts_with_number {
self.err_span_(start, self.pos, "lifetimes cannot start with a number");
let span = self.mk_sp(start, self.pos);
let mut diag = self.sess.struct_err("lifetimes cannot start with a number");
diag.set_span(span);
diag.stash(span, StashKey::LifetimeIsChar);
}
let ident = Symbol::intern(lifetime_name);
token::Lifetime(ident)
Expand Down
76 changes: 67 additions & 9 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,8 +42,10 @@ use rustc_ast::{AnonConst, BinOp, BinOpKind, FnDecl, FnRetTy, MacCall, Param, Ty
use rustc_ast::{Arm, Async, BlockCheckMode, Expr, ExprKind, Label, Movability, RangeLimits};
use rustc_ast::{ClosureBinder, StmtKind};
use rustc_ast_pretty::pprust;
use rustc_errors::IntoDiagnostic;
use rustc_errors::{Applicability, Diagnostic, PResult};
use rustc_errors::{
Applicability, Diagnostic, DiagnosticBuilder, ErrorGuaranteed, IntoDiagnostic, PResult,
StashKey,
};
use rustc_session::errors::ExprParenthesesNeeded;
use rustc_session::lint::builtin::BREAK_WITH_LABEL_AND_LOOP;
use rustc_session::lint::BuiltinLintDiagnostics;
Expand Down Expand Up @@ -1513,11 +1515,11 @@ impl<'a> Parser<'a> {
/// Parse `'label: $expr`. The label is already parsed.
fn parse_labeled_expr(
&mut self,
label: Label,
label_: Label,
mut consume_colon: bool,
) -> PResult<'a, P<Expr>> {
let lo = label.ident.span;
let label = Some(label);
let lo = label_.ident.span;
let label = Some(label_);
let ate_colon = self.eat(&token::Colon);
let expr = if self.eat_keyword(kw::While) {
self.parse_while_expr(label, lo)
Expand All @@ -1529,6 +1531,19 @@ impl<'a> Parser<'a> {
|| self.token.is_whole_block()
{
self.parse_block_expr(label, lo, BlockCheckMode::Default)
} else if !ate_colon
&& (matches!(self.token.kind, token::CloseDelim(_) | token::Comma)
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm using "close delim or comma or operator" here as a heuristic, because if we use something like "next token isn't colon" then we suggest 'a 0 => 'a' 0 which is nonsense.

|| self.token.is_op())
{
let lit = self.recover_unclosed_char(label_.ident, |self_| {
self_.sess.create_err(UnexpectedTokenAfterLabel {
span: self_.token.span,
remove_label: None,
enclose_in_block: None,
})
});
consume_colon = false;
Ok(self.mk_expr(lo, ExprKind::Lit(lit)))
} else if !ate_colon
&& (self.check_noexpect(&TokenKind::Comma) || self.check_noexpect(&TokenKind::Gt))
{
Expand Down Expand Up @@ -1603,6 +1618,39 @@ impl<'a> Parser<'a> {
Ok(expr)
}

/// Emit an error when a char is parsed as a lifetime because of a missing quote
pub(super) fn recover_unclosed_char(
&mut self,
lifetime: Ident,
err: impl FnOnce(&mut Self) -> DiagnosticBuilder<'a, ErrorGuaranteed>,
) -> ast::Lit {
if let Some(mut diag) =
self.sess.span_diagnostic.steal_diagnostic(lifetime.span, StashKey::LifetimeIsChar)
Copy link
Contributor

Choose a reason for hiding this comment

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

I missed the introduction of this more principled version of this mechanism :)

{
diag.span_suggestion_verbose(
lifetime.span.shrink_to_hi(),
"add `'` to close the char literal",
"'",
Applicability::MaybeIncorrect,
)
.emit();
} else {
err(self)
.span_suggestion_verbose(
lifetime.span.shrink_to_hi(),
"add `'` to close the char literal",
"'",
Applicability::MaybeIncorrect,
)
.emit();
}
ast::Lit {
token_lit: token::Lit::new(token::LitKind::Char, lifetime.name, None),
kind: ast::LitKind::Char(lifetime.name.as_str().chars().next().unwrap_or('_')),
span: lifetime.span,
}
}

/// Recover on the syntax `do catch { ... }` suggesting `try { ... }` instead.
fn recover_do_catch(&mut self) -> PResult<'a, P<Expr>> {
let lo = self.token.span;
Expand Down Expand Up @@ -1728,7 +1776,7 @@ impl<'a> Parser<'a> {
}

pub(super) fn parse_lit(&mut self) -> PResult<'a, Lit> {
self.parse_opt_lit().ok_or_else(|| {
self.parse_opt_lit().ok_or(()).or_else(|()| {
if let token::Interpolated(inner) = &self.token.kind {
let expr = match inner.as_ref() {
token::NtExpr(expr) => Some(expr),
Expand All @@ -1740,12 +1788,22 @@ impl<'a> Parser<'a> {
let mut err = InvalidInterpolatedExpression { span: self.token.span }
.into_diagnostic(&self.sess.span_diagnostic);
err.downgrade_to_delayed_bug();
return err;
return Err(err);
}
}
}
let msg = format!("unexpected token: {}", super::token_descr(&self.token));
self.struct_span_err(self.token.span, &msg)
let token = self.token.clone();
let err = |self_: &mut Self| {
let msg = format!("unexpected token: {}", super::token_descr(&token));
self_.struct_span_err(token.span, &msg)
};
// On an error path, eagerly consider a lifetime to be an unclosed character lit
if self.token.is_lifetime() {
let lt = self.expect_lifetime();
Ok(self.recover_unclosed_char(lt.ident, err))
} else {
Err(err(self))
}
})
}

Expand Down
20 changes: 20 additions & 0 deletions compiler/rustc_parse/src/parser/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -402,6 +402,25 @@ impl<'a> Parser<'a> {
} else {
PatKind::Path(qself, path)
}
} else if matches!(self.token.kind, token::Lifetime(_))
// In pattern position, we're totally fine with using "next token isn't colon"
// as a heuristic. We could probably just always try to recover if it's a lifetime,
// because we never have `'a: label {}` in a pattern position anyways, but it does
// keep us from suggesting something like `let 'a: Ty = ..` => `let 'a': Ty = ..`
&& !self.look_ahead(1, |token| matches!(token.kind, token::Colon))
Copy link
Member Author

@compiler-errors compiler-errors Sep 1, 2022

Choose a reason for hiding this comment

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

In pattern position, we're totally fine with using "next token isn't colon" as a heuristic. We could probably just always try to recover if it's a lifetime, because we never have 'a: label {} in a pattern position anyways, but it does keep us from suggesting something like let 'a: Ty = .. => let 'a': Ty = ..

Copy link
Contributor

Choose a reason for hiding this comment

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

Could we include this as a comment?

Copy link
Member Author

Choose a reason for hiding this comment

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

Can do

{
// Recover a `'a` as a `'a'` literal
let lt = self.expect_lifetime();
let lit = self.recover_unclosed_char(lt.ident, |self_| {
let expected = expected.unwrap_or("pattern");
let msg =
format!("expected {}, found {}", expected, super::token_descr(&self_.token));

let mut err = self_.struct_span_err(self_.token.span, &msg);
err.span_label(self_.token.span, format!("expected {}", expected));
err
});
PatKind::Lit(self.mk_expr(lo, ExprKind::Lit(lit)))
} else {
// Try to parse everything else as literal with optional minus
match self.parse_literal_maybe_minus() {
Expand Down Expand Up @@ -799,6 +818,7 @@ impl<'a> Parser<'a> {
|| t.kind == token::Dot // e.g. `.5` for recovery;
|| t.can_begin_literal_maybe_minus() // e.g. `42`.
|| t.is_whole_expr()
|| t.is_lifetime() // recover `'a` instead of `'a'`
})
}

Expand Down
1 change: 1 addition & 0 deletions src/test/ui/parser/issues/issue-93282.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,5 @@ fn foo() {
let x = 1;
bar('y, x);
//~^ ERROR expected
//~| ERROR mismatched types
}
27 changes: 26 additions & 1 deletion src/test/ui/parser/issues/issue-93282.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,11 @@ error: expected `while`, `for`, `loop` or `{` after a label
|
LL | f<'a,>
| ^ expected `while`, `for`, `loop` or `{` after a label
|
help: add `'` to close the char literal
|
LL | f<'a',>
Copy link
Member Author

@compiler-errors compiler-errors Sep 1, 2022

Choose a reason for hiding this comment

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

This is somewhat unfortunate, but teaching the parser to avoid suggesting this is just as hard as solving the turbofish problem I think, because at this point all the context we have is that we're parsing f < $EXPR where $EXPR begins with 'a

Copy link
Member Author

Choose a reason for hiding this comment

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

Hm, maybe I can avoid this if the prev token is <. Or maybe this suggestion is just nonsense and the PR needs to be scrapped.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's fine, this is best effort.

| +

error: expected one of `.`, `:`, `;`, `?`, `for`, `loop`, `while`, `}`, or an operator, found `,`
--> $DIR/issue-93282.rs:2:9
Expand All @@ -20,6 +25,26 @@ error: expected `while`, `for`, `loop` or `{` after a label
|
LL | bar('y, x);
| ^ expected `while`, `for`, `loop` or `{` after a label
|
help: add `'` to close the char literal
|
LL | bar('y', x);
| +

error[E0308]: mismatched types
--> $DIR/issue-93282.rs:13:9
|
LL | bar('y, x);
| --- ^^ expected `usize`, found `char`
| |
| arguments to this function are incorrect
|
note: function defined here
--> $DIR/issue-93282.rs:7:4
|
LL | fn bar(a: usize, b: usize) -> usize {
| ^^^ --------

error: aborting due to 3 previous errors
error: aborting due to 4 previous errors

For more information about this error, try `rustc --explain E0308`.
16 changes: 16 additions & 0 deletions src/test/ui/parser/label-is-actually-char.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
fn main() {
let c = 'a;
//~^ ERROR expected `while`, `for`, `loop` or `{` after a label
//~| HELP add `'` to close the char literal
match c {
'a'..='b => {}
//~^ ERROR unexpected token: `'b`
//~| HELP add `'` to close the char literal
_ => {}
}
let x = ['a, 'b];
//~^ ERROR expected `while`, `for`, `loop` or `{` after a label
//~| ERROR expected `while`, `for`, `loop` or `{` after a label
//~| HELP add `'` to close the char literal
//~| HELP add `'` to close the char literal
}
46 changes: 46 additions & 0 deletions src/test/ui/parser/label-is-actually-char.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
error: expected `while`, `for`, `loop` or `{` after a label
--> $DIR/label-is-actually-char.rs:2:15
|
LL | let c = 'a;
| ^ expected `while`, `for`, `loop` or `{` after a label
|
help: add `'` to close the char literal
|
LL | let c = 'a';
| +

error: unexpected token: `'b`
--> $DIR/label-is-actually-char.rs:6:15
|
LL | 'a'..='b => {}
| ^^
|
help: add `'` to close the char literal
|
LL | 'a'..='b' => {}
| +

error: expected `while`, `for`, `loop` or `{` after a label
--> $DIR/label-is-actually-char.rs:11:16
|
LL | let x = ['a, 'b];
| ^ expected `while`, `for`, `loop` or `{` after a label
|
help: add `'` to close the char literal
|
LL | let x = ['a', 'b];
| +

error: expected `while`, `for`, `loop` or `{` after a label
--> $DIR/label-is-actually-char.rs:11:20
|
LL | let x = ['a, 'b];
| ^ expected `while`, `for`, `loop` or `{` after a label
|
help: add `'` to close the char literal
|
LL | let x = ['a, 'b'];
| +

error: aborting due to 4 previous errors

16 changes: 8 additions & 8 deletions src/test/ui/parser/numeric-lifetime.stderr
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
error[E0308]: mismatched types
--> $DIR/numeric-lifetime.rs:6:20
|
LL | let x: usize = "";
| ----- ^^ expected `usize`, found `&str`
| |
| expected due to this

error: lifetimes cannot start with a number
--> $DIR/numeric-lifetime.rs:1:10
|
Expand All @@ -10,14 +18,6 @@ error: lifetimes cannot start with a number
LL | struct S<'1> { s: &'1 usize }
| ^^

error[E0308]: mismatched types
--> $DIR/numeric-lifetime.rs:6:20
|
LL | let x: usize = "";
| ----- ^^ expected `usize`, found `&str`
| |
| expected due to this

error: aborting due to 3 previous errors

For more information about this error, try `rustc --explain E0308`.
2 changes: 2 additions & 0 deletions src/test/ui/parser/require-parens-for-chained-comparison.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,11 +23,13 @@ fn main() {
//~^ ERROR expected one of
//~| HELP use `::<...>` instead of `<...>` to specify lifetime, type, or const arguments
//~| ERROR expected
//~| HELP add `'` to close the char literal

f<'_>();
//~^ comparison operators cannot be chained
//~| HELP use `::<...>` instead of `<...>` to specify lifetime, type, or const arguments
//~| ERROR expected
//~| HELP add `'` to close the char literal

let _ = f<u8>;
//~^ ERROR comparison operators cannot be chained
Expand Down
16 changes: 13 additions & 3 deletions src/test/ui/parser/require-parens-for-chained-comparison.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ error: expected `while`, `for`, `loop` or `{` after a label
|
LL | let _ = f<'_, i8>();
| ^ expected `while`, `for`, `loop` or `{` after a label
|
help: add `'` to close the char literal
|
LL | let _ = f<'_', i8>();
| +

error: expected one of `.`, `:`, `;`, `?`, `else`, `for`, `loop`, `while`, or an operator, found `,`
--> $DIR/require-parens-for-chained-comparison.rs:22:17
Expand All @@ -71,13 +76,18 @@ LL | let _ = f::<'_, i8>();
| ++

error: expected `while`, `for`, `loop` or `{` after a label
--> $DIR/require-parens-for-chained-comparison.rs:27:9
--> $DIR/require-parens-for-chained-comparison.rs:28:9
|
LL | f<'_>();
| ^ expected `while`, `for`, `loop` or `{` after a label
|
help: add `'` to close the char literal
|
LL | f<'_'>();
| +

error: comparison operators cannot be chained
--> $DIR/require-parens-for-chained-comparison.rs:27:6
--> $DIR/require-parens-for-chained-comparison.rs:28:6
|
LL | f<'_>();
| ^ ^
Expand All @@ -88,7 +98,7 @@ LL | f::<'_>();
| ++

error: comparison operators cannot be chained
--> $DIR/require-parens-for-chained-comparison.rs:32:14
--> $DIR/require-parens-for-chained-comparison.rs:34:14
|
LL | let _ = f<u8>;
| ^ ^
Expand Down