Skip to content

Add ErrorGuaranteed to Recovered::Yes and use it more. #124919

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

Merged
merged 1 commit into from
May 9, 2024
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
15 changes: 9 additions & 6 deletions compiler/rustc_ast/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1422,7 +1422,7 @@ pub enum ExprKind {
/// of `if` / `while` expressions. (e.g., `if let 0 = x { .. }`).
///
/// `Span` represents the whole `let pat = expr` statement.
Let(P<Pat>, P<Expr>, Span, Option<ErrorGuaranteed>),
Let(P<Pat>, P<Expr>, Span, Recovered),
/// An `if` block, with an optional `else` block.
///
/// `if expr { block } else { expr }`
Expand Down Expand Up @@ -2881,17 +2881,20 @@ pub struct FieldDef {
pub is_placeholder: bool,
}

/// Was parsing recovery performed?
#[derive(Copy, Clone, Debug, Encodable, Decodable, HashStable_Generic)]
pub enum Recovered {
No,
Yes(ErrorGuaranteed),
}

/// Fields and constructor ids of enum variants and structs.
#[derive(Clone, Encodable, Decodable, Debug)]
pub enum VariantData {
/// Struct variant.
///
/// E.g., `Bar { .. }` as in `enum Foo { Bar { .. } }`.
Struct {
fields: ThinVec<FieldDef>,
// FIXME: investigate making this a `Option<ErrorGuaranteed>`
recovered: bool,
},
Struct { fields: ThinVec<FieldDef>, recovered: Recovered },
/// Tuple variant.
///
/// E.g., `Bar(..)` as in `enum Foo { Bar(..) }`.
Expand Down
4 changes: 2 additions & 2 deletions compiler/rustc_ast_lowering/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,13 +158,13 @@ impl<'hir> LoweringContext<'_, 'hir> {
let ohs = self.lower_expr(ohs);
hir::ExprKind::AddrOf(*k, *m, ohs)
}
ExprKind::Let(pat, scrutinee, span, is_recovered) => {
ExprKind::Let(pat, scrutinee, span, recovered) => {
hir::ExprKind::Let(self.arena.alloc(hir::LetExpr {
span: self.lower_span(*span),
pat: self.lower_pat(pat),
ty: None,
init: self.lower_expr(scrutinee),
is_recovered: *is_recovered,
recovered: *recovered,
}))
}
ExprKind::If(cond, then, else_opt) => {
Expand Down
3 changes: 2 additions & 1 deletion compiler/rustc_ast_lowering/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1283,7 +1283,8 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> {
fields.iter().enumerate().map(|f| this.lower_field_def(f)),
);
let span = t.span;
let variant_data = hir::VariantData::Struct { fields, recovered: false };
let variant_data =
hir::VariantData::Struct { fields, recovered: ast::Recovered::No };
// FIXME: capture the generics from the outer adt.
let generics = hir::Generics::empty();
let kind = match t.kind {
Expand Down
5 changes: 2 additions & 3 deletions compiler/rustc_builtin_macros/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,13 @@ use rustc_ast::{token, StmtKind};
use rustc_ast::{
Expr, ExprKind, FormatAlignment, FormatArgPosition, FormatArgPositionKind, FormatArgs,
FormatArgsPiece, FormatArgument, FormatArgumentKind, FormatArguments, FormatCount,
FormatDebugHex, FormatOptions, FormatPlaceholder, FormatSign, FormatTrait,
FormatDebugHex, FormatOptions, FormatPlaceholder, FormatSign, FormatTrait, Recovered,
};
use rustc_data_structures::fx::FxHashSet;
use rustc_errors::{Applicability, Diag, MultiSpan, PResult, SingleLabelManySpans};
use rustc_expand::base::*;
use rustc_lint_defs::builtin::NAMED_ARGUMENTS_USED_POSITIONALLY;
use rustc_lint_defs::{BufferedEarlyLint, BuiltinLintDiag, LintId};
use rustc_parse::parser::Recovered;
use rustc_parse_format as parse;
use rustc_span::symbol::{Ident, Symbol};
use rustc_span::{BytePos, ErrorGuaranteed, InnerSpan, Span};
Expand Down Expand Up @@ -112,7 +111,7 @@ fn parse_args<'a>(ecx: &ExtCtxt<'a>, sp: Span, tts: TokenStream) -> PResult<'a,
_ => return Err(err),
}
}
Ok(Recovered::Yes) => (),
Ok(Recovered::Yes(_)) => (),
Ok(Recovered::No) => unreachable!(),
}
}
Expand Down
5 changes: 4 additions & 1 deletion compiler/rustc_expand/src/placeholders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,10 @@ pub(crate) fn placeholder(
}]),
AstFragmentKind::Variants => AstFragment::Variants(smallvec![ast::Variant {
attrs: Default::default(),
data: ast::VariantData::Struct { fields: Default::default(), recovered: false },
data: ast::VariantData::Struct {
fields: Default::default(),
recovered: ast::Recovered::No
},
disr_expr: None,
id,
ident,
Expand Down
10 changes: 3 additions & 7 deletions compiler/rustc_hir/src/hir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1308,9 +1308,9 @@ pub struct LetExpr<'hir> {
pub pat: &'hir Pat<'hir>,
pub ty: Option<&'hir Ty<'hir>>,
pub init: &'hir Expr<'hir>,
/// `Some` when this let expressions is not in a syntanctically valid location.
/// `Recovered::Yes` when this let expressions is not in a syntanctically valid location.
/// Used to prevent building MIR in such situations.
pub is_recovered: Option<ErrorGuaranteed>,
pub recovered: ast::Recovered,
}

#[derive(Debug, Clone, Copy, HashStable_Generic)]
Expand Down Expand Up @@ -3030,11 +3030,7 @@ pub enum VariantData<'hir> {
/// A struct variant.
///
/// E.g., `Bar { .. }` as in `enum Foo { Bar { .. } }`.
Struct {
fields: &'hir [FieldDef<'hir>],
// FIXME: investigate making this a `Option<ErrorGuaranteed>`
recovered: bool,
},
Struct { fields: &'hir [FieldDef<'hir>], recovered: ast::Recovered },
/// A tuple variant.
///
/// E.g., `Bar(..)` as in `enum Foo { Bar(..) }`.
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir/src/intravisit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -768,7 +768,7 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>)
ExprKind::DropTemps(ref subexpression) => {
try_visit!(visitor.visit_expr(subexpression));
}
ExprKind::Let(LetExpr { span: _, pat, ty, init, is_recovered: _ }) => {
ExprKind::Let(LetExpr { span: _, pat, ty, init, recovered: _ }) => {
// match the visit order in walk_local
try_visit!(visitor.visit_expr(init));
try_visit!(visitor.visit_pat(pat));
Expand Down
6 changes: 2 additions & 4 deletions compiler/rustc_hir_analysis/src/collect.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
//! At present, however, we do run collection across all items in the
//! crate as a kind of pass. This should eventually be factored away.

use rustc_ast::Recovered;
use rustc_data_structures::captures::Captures;
use rustc_data_structures::fx::{FxHashSet, FxIndexMap};
use rustc_data_structures::unord::UnordMap;
Expand Down Expand Up @@ -1005,10 +1006,7 @@ fn lower_variant(
vis: tcx.visibility(f.def_id),
})
.collect();
let recovered = match def {
hir::VariantData::Struct { recovered, .. } => *recovered,
_ => false,
};
let recovered = matches!(def, hir::VariantData::Struct { recovered: Recovered::Yes(_), .. });
ty::VariantDef::new(
ident.name,
variant_did.map(LocalDefId::to_def_id),
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1271,7 +1271,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// otherwise check exactly as a let statement
self.check_decl((let_expr, hir_id).into());
// but return a bool, for this is a boolean expression
if let Some(error_guaranteed) = let_expr.is_recovered {
if let ast::Recovered::Yes(error_guaranteed) = let_expr.recovered {
self.set_tainted_by_errors(error_guaranteed);
Ty::new_error(self.tcx, error_guaranteed)
} else {
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_hir_typeck/src/gather_locals.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ impl<'a> From<&'a hir::LetStmt<'a>> for Declaration<'a> {

impl<'a> From<(&'a hir::LetExpr<'a>, HirId)> for Declaration<'a> {
fn from((let_expr, hir_id): (&'a hir::LetExpr<'a>, HirId)) -> Self {
let hir::LetExpr { pat, ty, span, init, is_recovered: _ } = *let_expr;
let hir::LetExpr { pat, ty, span, init, recovered: _ } = *let_expr;
Declaration { hir_id, pat, ty, span, init: Some(init), origin: DeclOrigin::LetExpr }
}
}
Expand Down
43 changes: 21 additions & 22 deletions compiler/rustc_parse/src/parser/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ use crate::fluent_generated as fluent;
use crate::parser;
use crate::parser::attr::InnerAttrPolicy;
use ast::token::IdentIsRaw;
use parser::Recovered;
use rustc_ast as ast;
use rustc_ast::ptr::P;
use rustc_ast::token::{self, Delimiter, Lit, LitKind, Token, TokenKind};
Expand All @@ -31,7 +30,7 @@ use rustc_ast::util::parser::AssocOp;
use rustc_ast::{
AngleBracketedArg, AngleBracketedArgs, AnonConst, AttrVec, BinOpKind, BindingMode, Block,
BlockCheckMode, Expr, ExprKind, GenericArg, Generics, HasTokens, Item, ItemKind, Param, Pat,
PatKind, Path, PathSegment, QSelf, Ty, TyKind,
PatKind, Path, PathSegment, QSelf, Recovered, Ty, TyKind,
};
use rustc_ast_pretty::pprust;
use rustc_data_structures::fx::FxHashSet;
Expand Down Expand Up @@ -527,14 +526,14 @@ impl<'a> Parser<'a> {
//
// let x = 32:
// let y = 42;
self.dcx().emit_err(ExpectedSemi {
let guar = self.dcx().emit_err(ExpectedSemi {
span: self.token.span,
token: self.token.clone(),
unexpected_token_label: None,
sugg: ExpectedSemiSugg::ChangeToSemi(self.token.span),
});
self.bump();
return Ok(Recovered::Yes);
return Ok(Recovered::Yes(guar));
} else if self.look_ahead(0, |t| {
t == &token::CloseDelim(Delimiter::Brace)
|| ((t.can_begin_expr() || t.can_begin_item())
Expand All @@ -552,13 +551,13 @@ impl<'a> Parser<'a> {
// let x = 32
// let y = 42;
let span = self.prev_token.span.shrink_to_hi();
self.dcx().emit_err(ExpectedSemi {
let guar = self.dcx().emit_err(ExpectedSemi {
span,
token: self.token.clone(),
unexpected_token_label: Some(self.token.span),
sugg: ExpectedSemiSugg::AddSemi(span),
});
return Ok(Recovered::Yes);
return Ok(Recovered::Yes(guar));
}
}

Expand Down Expand Up @@ -712,8 +711,8 @@ impl<'a> Parser<'a> {

if self.check_too_many_raw_str_terminators(&mut err) {
if expected.contains(&TokenType::Token(token::Semi)) && self.eat(&token::Semi) {
err.emit();
return Ok(Recovered::Yes);
let guar = err.emit();
return Ok(Recovered::Yes(guar));
} else {
return Err(err);
}
Expand Down Expand Up @@ -1251,7 +1250,7 @@ impl<'a> Parser<'a> {
}
}
}
Ok((_, _, Recovered::Yes)) => {}
Ok((_, _, Recovered::Yes(_))) => {}
Err(err) => {
err.cancel();
}
Expand Down Expand Up @@ -1284,21 +1283,21 @@ impl<'a> Parser<'a> {

/// Check to see if a pair of chained operators looks like an attempt at chained comparison,
/// e.g. `1 < x <= 3`. If so, suggest either splitting the comparison into two, or
/// parenthesising the leftmost comparison.
/// parenthesising the leftmost comparison. The return value indicates if recovery happened.
fn attempt_chained_comparison_suggestion(
&mut self,
err: &mut ComparisonOperatorsCannotBeChained,
inner_op: &Expr,
outer_op: &Spanned<AssocOp>,
) -> Recovered {
) -> bool {
if let ExprKind::Binary(op, l1, r1) = &inner_op.kind {
if let ExprKind::Field(_, ident) = l1.kind
&& ident.as_str().parse::<i32>().is_err()
&& !matches!(r1.kind, ExprKind::Lit(_))
{
// The parser has encountered `foo.bar<baz`, the likelihood of the turbofish
// suggestion being the only one to apply is high.
return Recovered::No;
return false;
}
return match (op.node, &outer_op.node) {
// `x == y == z`
Expand All @@ -1317,7 +1316,7 @@ impl<'a> Parser<'a> {
span: inner_op.span.shrink_to_hi(),
middle_term: expr_to_str(r1),
});
Recovered::No // Keep the current parse behavior, where the AST is `(x < y) < z`.
false // Keep the current parse behavior, where the AST is `(x < y) < z`.
}
// `x == y < z`
(BinOpKind::Eq, AssocOp::Less | AssocOp::LessEqual | AssocOp::Greater | AssocOp::GreaterEqual) => {
Expand All @@ -1331,12 +1330,12 @@ impl<'a> Parser<'a> {
left: r1.span.shrink_to_lo(),
right: r2.span.shrink_to_hi(),
});
Recovered::Yes
true
}
Err(expr_err) => {
expr_err.cancel();
self.restore_snapshot(snapshot);
Recovered::Yes
true
}
}
}
Expand All @@ -1351,19 +1350,19 @@ impl<'a> Parser<'a> {
left: l1.span.shrink_to_lo(),
right: r1.span.shrink_to_hi(),
});
Recovered::Yes
true
}
Err(expr_err) => {
expr_err.cancel();
self.restore_snapshot(snapshot);
Recovered::No
false
}
}
}
_ => Recovered::No,
_ => false
};
}
Recovered::No
false
}

/// Produces an error if comparison operators are chained (RFC #558).
Expand Down Expand Up @@ -1494,7 +1493,7 @@ impl<'a> Parser<'a> {
// misformatted turbofish, for instance), suggest a correct form.
let recovered = self
.attempt_chained_comparison_suggestion(&mut err, inner_op, outer_op);
if matches!(recovered, Recovered::Yes) {
if recovered {
let guar = self.dcx().emit_err(err);
mk_err_expr(self, inner_op.span.to(self.prev_token.span), guar)
} else {
Expand All @@ -1503,10 +1502,10 @@ impl<'a> Parser<'a> {
}
};
}
let recover =
let recovered =
self.attempt_chained_comparison_suggestion(&mut err, inner_op, outer_op);
let guar = self.dcx().emit_err(err);
if matches!(recover, Recovered::Yes) {
if recovered {
return mk_err_expr(self, inner_op.span.to(self.prev_token.span), guar);
}
}
Expand Down
Loading
Loading