Skip to content

Commit

Permalink
Unrolled build for rust-lang#124919
Browse files Browse the repository at this point in the history
Rollup merge of rust-lang#124919 - nnethercote:Recovered-Yes-ErrorGuaranteed, r=compiler-errors

Add `ErrorGuaranteed` to `Recovered::Yes` and use it more.

The starting point for this was identical comments on two different fields, in `ast::VariantData::Struct` and `hir::VariantData::Struct`:
```
    // FIXME: investigate making this a `Option<ErrorGuaranteed>`
    recovered: bool
```
I tried that, and then found that I needed to add an `ErrorGuaranteed` to `Recovered::Yes`. Then I ended up using `Recovered` instead of `Option<ErrorGuaranteed>` for these two places and elsewhere, which required moving `ErrorGuaranteed` from `rustc_parse` to `rustc_ast`.

This makes things more consistent, because `Recovered` is used in more places, and there are fewer uses of `bool` and
`Option<ErrorGuaranteed>`. And safer, because it's difficult/impossible to set `recovered` to `Recovered::Yes` without having emitted an error.

r? `@oli-obk`
  • Loading branch information
rust-timer committed May 9, 2024
2 parents 238c1e7 + fd91925 commit 7a8f03b
Show file tree
Hide file tree
Showing 16 changed files with 91 additions and 105 deletions.
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

0 comments on commit 7a8f03b

Please sign in to comment.