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

Tweak diagnostics #92823

Merged
merged 1 commit into from
Feb 28, 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: 2 additions & 1 deletion compiler/rustc_expand/src/expand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ use rustc_data_structures::sync::Lrc;
use rustc_errors::{Applicability, PResult};
use rustc_feature::Features;
use rustc_parse::parser::{
AttemptLocalParseRecovery, ForceCollect, Parser, RecoverColon, RecoverComma,
AttemptLocalParseRecovery, CommaRecoveryMode, ForceCollect, Parser, RecoverColon, RecoverComma,
};
use rustc_parse::validate_attr;
use rustc_session::lint::builtin::{UNUSED_ATTRIBUTES, UNUSED_DOC_COMMENTS};
Expand Down Expand Up @@ -911,6 +911,7 @@ pub fn parse_ast_fragment<'a>(
None,
RecoverComma::No,
RecoverColon::Yes,
CommaRecoveryMode::LikelyTuple,
)?),
AstFragmentKind::Crate => AstFragment::Crate(this.parse_crate_mod()?),
AstFragmentKind::Arms
Expand Down
53 changes: 39 additions & 14 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
use super::pat::Expected;
use super::ty::{AllowPlus, IsAsCast};
use super::{
BlockMode, Parser, PathStyle, RecoverColon, RecoverComma, Restrictions, SemiColonMode, SeqSep,
TokenExpectType, TokenType,
BlockMode, CommaRecoveryMode, Parser, PathStyle, RecoverColon, RecoverComma, Restrictions,
SemiColonMode, SeqSep, TokenExpectType, TokenType,
};

use rustc_ast as ast;
Expand Down Expand Up @@ -2245,12 +2245,32 @@ impl<'a> Parser<'a> {
first_pat
}

crate fn maybe_recover_unexpected_block_label(&mut self) -> bool {
let Some(label) = self.eat_label().filter(|_| {
self.eat(&token::Colon) && self.token.kind == token::OpenDelim(token::Brace)
}) else {
return false;
};
let span = label.ident.span.to(self.prev_token.span);
let mut err = self.struct_span_err(span, "block label not supported here");
err.span_label(span, "not supported here");
err.tool_only_span_suggestion(
label.ident.span.until(self.token.span),
"remove this block label",
String::new(),
Applicability::MachineApplicable,
);
err.emit();
true
}

/// Some special error handling for the "top-level" patterns in a match arm,
/// `for` loop, `let`, &c. (in contrast to subpatterns within such).
crate fn maybe_recover_unexpected_comma(
&mut self,
lo: Span,
rc: RecoverComma,
rt: CommaRecoveryMode,
) -> PResult<'a, ()> {
if rc == RecoverComma::No || self.token != token::Comma {
return Ok(());
Expand All @@ -2270,20 +2290,25 @@ impl<'a> Parser<'a> {
let seq_span = lo.to(self.prev_token.span);
let mut err = self.struct_span_err(comma_span, "unexpected `,` in pattern");
if let Ok(seq_snippet) = self.span_to_snippet(seq_span) {
const MSG: &str = "try adding parentheses to match on a tuple...";

err.span_suggestion(
seq_span,
MSG,
format!("({})", seq_snippet),
Applicability::MachineApplicable,
);
err.span_suggestion(
seq_span,
"...or a vertical bar to match on multiple alternatives",
seq_snippet.replace(',', " |"),
err.multipart_suggestion(
&format!(
"try adding parentheses to match on a tuple{}",
if let CommaRecoveryMode::LikelyTuple = rt { "" } else { "..." },
),
vec![
(seq_span.shrink_to_lo(), "(".to_string()),
(seq_span.shrink_to_hi(), ")".to_string()),
],
Applicability::MachineApplicable,
);
if let CommaRecoveryMode::EitherTupleOrPipe = rt {
err.span_suggestion(
seq_span,
"...or a vertical bar to match on multiple alternatives",
seq_snippet.replace(',', " |"),
Applicability::MachineApplicable,
);
}
}
Err(err)
}
Expand Down
56 changes: 46 additions & 10 deletions compiler/rustc_parse/src/parser/expr.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use super::pat::{RecoverColon, RecoverComma, PARAM_EXPECTED};
use super::pat::{CommaRecoveryMode, RecoverColon, RecoverComma, PARAM_EXPECTED};
use super::ty::{AllowPlus, RecoverQPath, RecoverReturnSign};
use super::{
AttrWrapper, BlockMode, ClosureSpans, ForceCollect, Parser, PathStyle, Restrictions, TokenType,
Expand Down Expand Up @@ -1286,18 +1286,27 @@ impl<'a> Parser<'a> {
} else if let Some(label) = self.eat_label() {
self.parse_labeled_expr(label, attrs, true)
} else if self.eat_keyword(kw::Loop) {
self.parse_loop_expr(None, self.prev_token.span, attrs)
let sp = self.prev_token.span;
self.parse_loop_expr(None, self.prev_token.span, attrs).map_err(|mut err| {
err.span_label(sp, "while parsing this `loop` expression");
err
})
} else if self.eat_keyword(kw::Continue) {
let kind = ExprKind::Continue(self.eat_label());
Ok(self.mk_expr(lo.to(self.prev_token.span), kind, attrs))
} else if self.eat_keyword(kw::Match) {
let match_sp = self.prev_token.span;
self.parse_match_expr(attrs).map_err(|mut err| {
err.span_label(match_sp, "while parsing this match expression");
err.span_label(match_sp, "while parsing this `match` expression");
err
})
} else if self.eat_keyword(kw::Unsafe) {
let sp = self.prev_token.span;
self.parse_block_expr(None, lo, BlockCheckMode::Unsafe(ast::UserProvided), attrs)
.map_err(|mut err| {
err.span_label(sp, "while parsing this `unsafe` expression");
err
})
} else if self.check_inline_const(0) {
self.parse_const_block(lo.to(self.token.span), false)
} else if self.is_do_catch_block() {
Expand Down Expand Up @@ -2160,7 +2169,12 @@ impl<'a> Parser<'a> {
/// The `let` token has already been eaten.
fn parse_let_expr(&mut self, attrs: AttrVec) -> PResult<'a, P<Expr>> {
let lo = self.prev_token.span;
let pat = self.parse_pat_allow_top_alt(None, RecoverComma::Yes, RecoverColon::Yes)?;
let pat = self.parse_pat_allow_top_alt(
None,
RecoverComma::Yes,
RecoverColon::Yes,
CommaRecoveryMode::LikelyTuple,
)?;
self.expect(&token::Eq)?;
let expr = self.with_res(self.restrictions | Restrictions::NO_STRUCT_LITERAL, |this| {
this.parse_assoc_expr_with(1 + prec_let_scrutinee_needs_par(), None.into())
Expand Down Expand Up @@ -2223,7 +2237,12 @@ impl<'a> Parser<'a> {
_ => None,
};

let pat = self.parse_pat_allow_top_alt(None, RecoverComma::Yes, RecoverColon::Yes)?;
let pat = self.parse_pat_allow_top_alt(
None,
RecoverComma::Yes,
RecoverColon::Yes,
CommaRecoveryMode::LikelyTuple,
)?;
if !self.eat_keyword(kw::In) {
self.error_missing_in_for_loop();
}
Expand Down Expand Up @@ -2266,8 +2285,15 @@ impl<'a> Parser<'a> {
lo: Span,
mut attrs: AttrVec,
) -> PResult<'a, P<Expr>> {
let cond = self.parse_cond_expr()?;
let (iattrs, body) = self.parse_inner_attrs_and_block()?;
let cond = self.parse_cond_expr().map_err(|mut err| {
err.span_label(lo, "while parsing the condition of this `while` expression");
err
})?;
let (iattrs, body) = self.parse_inner_attrs_and_block().map_err(|mut err| {
err.span_label(lo, "while parsing the body of this `while` expression");
err.span_label(cond.span, "this `while` condition successfully parsed");
err
})?;
attrs.extend(iattrs);
Ok(self.mk_expr(lo.to(self.prev_token.span), ExprKind::While(cond, body, opt_label), attrs))
}
Expand All @@ -2284,7 +2310,7 @@ impl<'a> Parser<'a> {
Ok(self.mk_expr(lo.to(self.prev_token.span), ExprKind::Loop(body, opt_label), attrs))
}

fn eat_label(&mut self) -> Option<Label> {
crate fn eat_label(&mut self) -> Option<Label> {
self.token.lifetime().map(|ident| {
self.bump();
Label { ident }
Expand All @@ -2305,7 +2331,12 @@ impl<'a> Parser<'a> {
Applicability::MaybeIncorrect, // speculative
);
}
return Err(e);
if self.maybe_recover_unexpected_block_label() {
e.cancel();
self.bump();
} else {
return Err(e);
}
}
attrs.extend(self.parse_inner_attributes()?);

Expand Down Expand Up @@ -2441,7 +2472,12 @@ impl<'a> Parser<'a> {
let attrs = self.parse_outer_attributes()?;
self.collect_tokens_trailing_token(attrs, ForceCollect::No, |this, attrs| {
let lo = this.token.span;
let pat = this.parse_pat_allow_top_alt(None, RecoverComma::Yes, RecoverColon::Yes)?;
let pat = this.parse_pat_allow_top_alt(
None,
RecoverComma::Yes,
RecoverColon::Yes,
CommaRecoveryMode::EitherTupleOrPipe,
)?;
let guard = if this.eat_keyword(kw::If) {
let if_span = this.prev_token.span;
let cond = this.parse_expr()?;
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_parse/src/parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub use attr_wrapper::AttrWrapper;
pub use diagnostics::AttemptLocalParseRecovery;
use diagnostics::Error;
pub(crate) use item::FnParseMode;
pub use pat::{RecoverColon, RecoverComma};
pub use pat::{CommaRecoveryMode, RecoverColon, RecoverComma};
pub use path::PathStyle;

use rustc_ast::ptr::P;
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_parse/src/parser/nonterminal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use rustc_ast_pretty::pprust;
use rustc_errors::PResult;
use rustc_span::symbol::{kw, Ident};

use crate::parser::pat::{RecoverColon, RecoverComma};
use crate::parser::pat::{CommaRecoveryMode, RecoverColon, RecoverComma};
use crate::parser::{FollowedByType, ForceCollect, Parser, PathStyle};

impl<'a> Parser<'a> {
Expand Down Expand Up @@ -125,7 +125,7 @@ impl<'a> Parser<'a> {
token::NtPat(self.collect_tokens_no_attrs(|this| match kind {
NonterminalKind::PatParam { .. } => this.parse_pat_no_top_alt(None),
NonterminalKind::PatWithOr { .. } => {
this.parse_pat_allow_top_alt(None, RecoverComma::No, RecoverColon::No)
this.parse_pat_allow_top_alt(None, RecoverComma::No, RecoverColon::No, CommaRecoveryMode::EitherTupleOrPipe)
}
_ => unreachable!(),
})?)
Expand Down
51 changes: 42 additions & 9 deletions compiler/rustc_parse/src/parser/pat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,13 @@ pub enum RecoverColon {
No,
}

/// Whether or not to recover a `a, b` when parsing patterns as `(a, b)` or that *and* `a | b`.
#[derive(PartialEq, Copy, Clone)]
pub enum CommaRecoveryMode {
LikelyTuple,
EitherTupleOrPipe,
}

/// The result of `eat_or_separator`. We want to distinguish which case we are in to avoid
/// emitting duplicate diagnostics.
#[derive(Debug, Clone, Copy)]
Expand Down Expand Up @@ -68,8 +75,9 @@ impl<'a> Parser<'a> {
expected: Expected,
rc: RecoverComma,
ra: RecoverColon,
rt: CommaRecoveryMode,
) -> PResult<'a, P<Pat>> {
self.parse_pat_allow_top_alt_inner(expected, rc, ra).map(|(pat, _)| pat)
self.parse_pat_allow_top_alt_inner(expected, rc, ra, rt).map(|(pat, _)| pat)
}

/// Returns the pattern and a bool indicating whether we recovered from a trailing vert (true =
Expand All @@ -79,6 +87,7 @@ impl<'a> Parser<'a> {
expected: Expected,
rc: RecoverComma,
ra: RecoverColon,
rt: CommaRecoveryMode,
) -> PResult<'a, (P<Pat>, bool)> {
// Keep track of whether we recovered from a trailing vert so that we can avoid duplicated
// suggestions (which bothers rustfix).
Expand All @@ -92,7 +101,7 @@ impl<'a> Parser<'a> {

// Parse the first pattern (`p_0`).
let first_pat = self.parse_pat_no_top_alt(expected)?;
self.maybe_recover_unexpected_comma(first_pat.span, rc)?;
self.maybe_recover_unexpected_comma(first_pat.span, rc, rt)?;

// If the next token is not a `|`,
// this is not an or-pattern and we should exit here.
Expand Down Expand Up @@ -130,7 +139,7 @@ impl<'a> Parser<'a> {
err.span_label(lo, WHILE_PARSING_OR_MSG);
err
})?;
self.maybe_recover_unexpected_comma(pat.span, rc)?;
self.maybe_recover_unexpected_comma(pat.span, rc, rt)?;
pats.push(pat);
}
let or_pattern_span = lo.to(self.prev_token.span);
Expand All @@ -155,8 +164,12 @@ impl<'a> Parser<'a> {
// We use `parse_pat_allow_top_alt` regardless of whether we actually want top-level
// or-patterns so that we can detect when a user tries to use it. This allows us to print a
// better error message.
let (pat, trailing_vert) =
self.parse_pat_allow_top_alt_inner(expected, rc, RecoverColon::No)?;
let (pat, trailing_vert) = self.parse_pat_allow_top_alt_inner(
expected,
rc,
RecoverColon::No,
CommaRecoveryMode::LikelyTuple,
)?;
let colon = self.eat(&token::Colon);

if let PatKind::Or(pats) = &pat.kind {
Expand Down Expand Up @@ -315,7 +328,12 @@ impl<'a> Parser<'a> {
} else if self.check(&token::OpenDelim(token::Bracket)) {
// Parse `[pat, pat,...]` as a slice pattern.
let (pats, _) = self.parse_delim_comma_seq(token::Bracket, |p| {
p.parse_pat_allow_top_alt(None, RecoverComma::No, RecoverColon::No)
p.parse_pat_allow_top_alt(
None,
RecoverComma::No,
RecoverColon::No,
CommaRecoveryMode::EitherTupleOrPipe,
)
})?;
PatKind::Slice(pats)
} else if self.check(&token::DotDot) && !self.is_pat_range_end_start(1) {
Expand Down Expand Up @@ -529,7 +547,12 @@ impl<'a> Parser<'a> {
/// Parse a tuple or parenthesis pattern.
fn parse_pat_tuple_or_parens(&mut self) -> PResult<'a, PatKind> {
let (fields, trailing_comma) = self.parse_paren_comma_seq(|p| {
p.parse_pat_allow_top_alt(None, RecoverComma::No, RecoverColon::No)
p.parse_pat_allow_top_alt(
None,
RecoverComma::No,
RecoverColon::No,
CommaRecoveryMode::LikelyTuple,
)
})?;

// Here, `(pat,)` is a tuple pattern.
Expand Down Expand Up @@ -873,7 +896,12 @@ impl<'a> Parser<'a> {
/// Parse tuple struct or tuple variant pattern (e.g. `Foo(...)` or `Foo::Bar(...)`).
fn parse_pat_tuple_struct(&mut self, qself: Option<QSelf>, path: Path) -> PResult<'a, PatKind> {
let (fields, _) = self.parse_paren_comma_seq(|p| {
p.parse_pat_allow_top_alt(None, RecoverComma::No, RecoverColon::No)
p.parse_pat_allow_top_alt(
None,
RecoverComma::No,
RecoverColon::No,
CommaRecoveryMode::EitherTupleOrPipe,
)
})?;
if qself.is_some() {
self.sess.gated_spans.gate(sym::more_qualified_paths, path.span);
Expand Down Expand Up @@ -1033,7 +1061,12 @@ impl<'a> Parser<'a> {
// Parsing a pattern of the form `fieldname: pat`.
let fieldname = self.parse_field_name()?;
self.bump();
let pat = self.parse_pat_allow_top_alt(None, RecoverComma::No, RecoverColon::No)?;
let pat = self.parse_pat_allow_top_alt(
None,
RecoverComma::No,
RecoverColon::No,
CommaRecoveryMode::EitherTupleOrPipe,
)?;
hi = pat.span;
(pat, fieldname, false)
} else {
Expand Down
Loading