From d4c6c772c39780769786a299df7b522503d86fca Mon Sep 17 00:00:00 2001 From: Xavier Denis Date: Wed, 8 May 2024 17:01:47 +0200 Subject: [PATCH 1/5] Make a minimal amount of region APIs public --- .../rustc_borrowck/src/region_infer/mod.rs | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_borrowck/src/region_infer/mod.rs b/compiler/rustc_borrowck/src/region_infer/mod.rs index dd75548a15df7..f1a03b7447158 100644 --- a/compiler/rustc_borrowck/src/region_infer/mod.rs +++ b/compiler/rustc_borrowck/src/region_infer/mod.rs @@ -1328,14 +1328,20 @@ impl<'tcx> RegionInferenceContext<'tcx> { }) } - // Evaluate whether `sup_region == sub_region`. - fn eval_equal(&self, r1: RegionVid, r2: RegionVid) -> bool { + /// Evaluate whether `sup_region == sub_region`. + /// + /// Panics if called before `solve()` executes, + // This is `pub` because it's used by unstable external borrowck data users, see `consumers.rs`. + pub fn eval_equal(&self, r1: RegionVid, r2: RegionVid) -> bool { self.eval_outlives(r1, r2) && self.eval_outlives(r2, r1) } - // Evaluate whether `sup_region: sub_region`. + /// Evaluate whether `sup_region: sub_region`. + /// + /// Panics if called before `solve()` executes, + // This is `pub` because it's used by unstable external borrowck data users, see `consumers.rs`. #[instrument(skip(self), level = "debug", ret)] - fn eval_outlives(&self, sup_region: RegionVid, sub_region: RegionVid) -> bool { + pub fn eval_outlives(&self, sup_region: RegionVid, sub_region: RegionVid) -> bool { debug!( "sup_region's value = {:?} universal={:?}", self.region_value_str(sup_region), @@ -2248,7 +2254,10 @@ impl<'tcx> RegionInferenceContext<'tcx> { } /// Access to the SCC constraint graph. - pub(crate) fn constraint_sccs(&self) -> &Sccs { + /// This can be used to quickly under-approximate the regions which are equal to each other + /// and their relative orderings. + // This is `pub` because it's used by unstable external borrowck data users, see `consumers.rs`. + pub fn constraint_sccs(&self) -> &Sccs { self.constraint_sccs.as_ref() } From fd91925bce5f443bf963042f1ebaf0b3330212fe Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Thu, 9 May 2024 18:44:40 +1000 Subject: [PATCH 2/5] 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` 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` 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`. And safer, because it's difficult/impossible to set `recovered` to `Recovered::Yes` without having emitted an error. --- compiler/rustc_ast/src/ast.rs | 15 ++++--- compiler/rustc_ast_lowering/src/expr.rs | 4 +- compiler/rustc_ast_lowering/src/lib.rs | 3 +- compiler/rustc_builtin_macros/src/format.rs | 5 +-- compiler/rustc_expand/src/placeholders.rs | 5 ++- compiler/rustc_hir/src/hir.rs | 10 ++--- compiler/rustc_hir/src/intravisit.rs | 2 +- compiler/rustc_hir_analysis/src/collect.rs | 6 +-- compiler/rustc_hir_typeck/src/expr.rs | 2 +- .../rustc_hir_typeck/src/gather_locals.rs | 2 +- .../rustc_parse/src/parser/diagnostics.rs | 43 +++++++++---------- compiler/rustc_parse/src/parser/expr.rs | 40 +++++++++-------- compiler/rustc_parse/src/parser/item.rs | 24 +++++------ compiler/rustc_parse/src/parser/mod.rs | 22 ++-------- compiler/rustc_parse/src/parser/stmt.rs | 8 +--- tests/ui-fulldeps/pprust-expr-roundtrip.rs | 5 ++- 16 files changed, 91 insertions(+), 105 deletions(-) diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index 6d9656c720395..7951b7e7b75a0 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -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, P, Span, Option), + Let(P, P, Span, Recovered), /// An `if` block, with an optional `else` block. /// /// `if expr { block } else { expr }` @@ -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, - // FIXME: investigate making this a `Option` - recovered: bool, - }, + Struct { fields: ThinVec, recovered: Recovered }, /// Tuple variant. /// /// E.g., `Bar(..)` as in `enum Foo { Bar(..) }`. diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index 443b596b9177f..a553109092842 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -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) => { diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 6c54363e306d0..a76935761f0a4 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -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 { diff --git a/compiler/rustc_builtin_macros/src/format.rs b/compiler/rustc_builtin_macros/src/format.rs index 2450ac8f4b338..e557e3d0e6076 100644 --- a/compiler/rustc_builtin_macros/src/format.rs +++ b/compiler/rustc_builtin_macros/src/format.rs @@ -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}; @@ -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!(), } } diff --git a/compiler/rustc_expand/src/placeholders.rs b/compiler/rustc_expand/src/placeholders.rs index 7026425e167c8..e21f041d69afd 100644 --- a/compiler/rustc_expand/src/placeholders.rs +++ b/compiler/rustc_expand/src/placeholders.rs @@ -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, diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 244c479120dc2..7d991e21ff3d2 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -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, + pub recovered: ast::Recovered, } #[derive(Debug, Clone, Copy, HashStable_Generic)] @@ -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` - recovered: bool, - }, + Struct { fields: &'hir [FieldDef<'hir>], recovered: ast::Recovered }, /// A tuple variant. /// /// E.g., `Bar(..)` as in `enum Foo { Bar(..) }`. diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index fa89a4a44ef5a..0b095db953b08 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -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)); diff --git a/compiler/rustc_hir_analysis/src/collect.rs b/compiler/rustc_hir_analysis/src/collect.rs index 9198ceb8f59db..566f818f89958 100644 --- a/compiler/rustc_hir_analysis/src/collect.rs +++ b/compiler/rustc_hir_analysis/src/collect.rs @@ -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; @@ -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), diff --git a/compiler/rustc_hir_typeck/src/expr.rs b/compiler/rustc_hir_typeck/src/expr.rs index 7b552bb707743..9b10f7cbc6e25 100644 --- a/compiler/rustc_hir_typeck/src/expr.rs +++ b/compiler/rustc_hir_typeck/src/expr.rs @@ -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 { diff --git a/compiler/rustc_hir_typeck/src/gather_locals.rs b/compiler/rustc_hir_typeck/src/gather_locals.rs index fe0a46924de46..57d3f16fb6625 100644 --- a/compiler/rustc_hir_typeck/src/gather_locals.rs +++ b/compiler/rustc_hir_typeck/src/gather_locals.rs @@ -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 } } } diff --git a/compiler/rustc_parse/src/parser/diagnostics.rs b/compiler/rustc_parse/src/parser/diagnostics.rs index f256dbf436021..995803f9cd054 100644 --- a/compiler/rustc_parse/src/parser/diagnostics.rs +++ b/compiler/rustc_parse/src/parser/diagnostics.rs @@ -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}; @@ -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; @@ -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()) @@ -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)); } } @@ -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); } @@ -1251,7 +1250,7 @@ impl<'a> Parser<'a> { } } } - Ok((_, _, Recovered::Yes)) => {} + Ok((_, _, Recovered::Yes(_))) => {} Err(err) => { err.cancel(); } @@ -1284,13 +1283,13 @@ 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, - ) -> Recovered { + ) -> bool { if let ExprKind::Binary(op, l1, r1) = &inner_op.kind { if let ExprKind::Field(_, ident) = l1.kind && ident.as_str().parse::().is_err() @@ -1298,7 +1297,7 @@ impl<'a> Parser<'a> { { // The parser has encountered `foo.bar 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) => { @@ -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 } } } @@ -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). @@ -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 { @@ -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); } } diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 8ed2a6edf1aec..577003e94fb28 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -3,7 +3,7 @@ use super::diagnostics::SnapshotParser; use super::pat::{CommaRecoveryMode, Expected, RecoverColon, RecoverComma}; use super::ty::{AllowPlus, RecoverQPath, RecoverReturnSign}; use super::{ - AttrWrapper, BlockMode, ClosureSpans, ForceCollect, Parser, PathStyle, Recovered, Restrictions, + AttrWrapper, BlockMode, ClosureSpans, ForceCollect, Parser, PathStyle, Restrictions, SemiColonMode, SeqSep, TokenExpectType, TokenType, Trailing, TrailingToken, }; @@ -11,7 +11,7 @@ use crate::errors; use crate::maybe_recover_from_interpolated_ty_qpath; use ast::mut_visit::{noop_visit_expr, MutVisitor}; use ast::token::IdentIsRaw; -use ast::{CoroutineKind, ForLoopKind, GenBlockKind, MatchKind, Pat, Path, PathSegment}; +use ast::{CoroutineKind, ForLoopKind, GenBlockKind, MatchKind, Pat, Path, PathSegment, Recovered}; use core::mem; use core::ops::ControlFlow; use rustc_ast::ptr::P; @@ -2629,7 +2629,7 @@ impl<'a> Parser<'a> { CondChecker::new(self).visit_expr(&mut cond); - if let ExprKind::Let(_, _, _, None) = cond.kind { + if let ExprKind::Let(_, _, _, Recovered::No) = cond.kind { // Remove the last feature gating of a `let` expression since it's stable. self.psess.gated_spans.ungate_last(sym::let_chains, cond.span); } @@ -2639,7 +2639,7 @@ impl<'a> Parser<'a> { /// Parses a `let $pat = $expr` pseudo-expression. fn parse_expr_let(&mut self, restrictions: Restrictions) -> PResult<'a, P> { - let is_recovered = if !restrictions.contains(Restrictions::ALLOW_LET) { + let recovered = if !restrictions.contains(Restrictions::ALLOW_LET) { let err = errors::ExpectedExpressionFoundLet { span: self.token.span, reason: ForbiddenLetReason::OtherForbidden, @@ -2650,10 +2650,10 @@ impl<'a> Parser<'a> { // This was part of a closure, the that part of the parser recover. return Err(self.dcx().create_err(err)); } else { - Some(self.dcx().emit_err(err)) + Recovered::Yes(self.dcx().emit_err(err)) } } else { - None + Recovered::No }; self.bump(); // Eat `let` token let lo = self.prev_token.span; @@ -2674,7 +2674,7 @@ impl<'a> Parser<'a> { } let expr = self.parse_expr_assoc_with(1 + prec_let_scrutinee_needs_par(), None.into())?; let span = lo.to(expr.span); - Ok(self.mk_expr(span, ExprKind::Let(pat, expr, span, is_recovered))) + Ok(self.mk_expr(span, ExprKind::Let(pat, expr, span, recovered))) } /// Parses an `else { ... }` expression (`else` token already eaten). @@ -2998,7 +2998,7 @@ impl<'a> Parser<'a> { &mut self, first_expr: &P, arrow_span: Span, - ) -> Option> { + ) -> Option<(Span, ErrorGuaranteed)> { if self.token.kind != token::Semi { return None; } @@ -3023,7 +3023,7 @@ impl<'a> Parser<'a> { errors::MatchArmBodyWithoutBracesSugg::UseComma { semicolon: semi_sp } }, }); - this.mk_expr_err(span, guar) + (span, guar) }; // We might have either a `,` -> `;` typo, or a block without braces. We need // a more subtle parsing strategy. @@ -3143,9 +3143,12 @@ impl<'a> Parser<'a> { arm_body = Some(expr); this.eat(&token::Comma); Ok(Recovered::No) - } else if let Some(body) = this.parse_arm_body_missing_braces(&expr, arrow_span) { + } else if let Some((span, guar)) = + this.parse_arm_body_missing_braces(&expr, arrow_span) + { + let body = this.mk_expr_err(span, guar); arm_body = Some(body); - Ok(Recovered::Yes) + Ok(Recovered::Yes(guar)) } else { let expr_span = expr.span; arm_body = Some(expr); @@ -3223,10 +3226,10 @@ impl<'a> Parser<'a> { .is_ok(); if pattern_follows && snapshot.check(&TokenKind::FatArrow) { err.cancel(); - this.dcx().emit_err(errors::MissingCommaAfterMatchArm { + let guar = this.dcx().emit_err(errors::MissingCommaAfterMatchArm { span: arm_span.shrink_to_hi(), }); - return Ok(Recovered::Yes); + return Ok(Recovered::Yes(guar)); } Err(err) }); @@ -3904,15 +3907,16 @@ impl MutVisitor for CondChecker<'_> { let span = e.span; match e.kind { - ExprKind::Let(_, _, _, ref mut is_recovered @ None) => { + ExprKind::Let(_, _, _, ref mut recovered @ Recovered::No) => { if let Some(reason) = self.forbid_let_reason { - *is_recovered = - Some(self.parser.dcx().emit_err(errors::ExpectedExpressionFoundLet { + *recovered = Recovered::Yes(self.parser.dcx().emit_err( + errors::ExpectedExpressionFoundLet { span, reason, missing_let: self.missing_let, comparison: self.comparison, - })); + }, + )); } else { self.parser.psess.gated_spans.gate(sym::let_chains, span); } @@ -3980,7 +3984,7 @@ impl MutVisitor for CondChecker<'_> { self.visit_expr(op); self.forbid_let_reason = forbid_let_reason; } - ExprKind::Let(_, _, _, Some(_)) + ExprKind::Let(_, _, _, Recovered::Yes(_)) | ExprKind::Array(_) | ExprKind::ConstBlock(_) | ExprKind::Lit(_) diff --git a/compiler/rustc_parse/src/parser/item.rs b/compiler/rustc_parse/src/parser/item.rs index 3ac50a6e4a865..df6996dbc458b 100644 --- a/compiler/rustc_parse/src/parser/item.rs +++ b/compiler/rustc_parse/src/parser/item.rs @@ -1,8 +1,7 @@ use super::diagnostics::{dummy_arg, ConsumeClosingDelim}; use super::ty::{AllowPlus, RecoverQPath, RecoverReturnSign}; use super::{ - AttrWrapper, FollowedByType, ForceCollect, Parser, PathStyle, Recovered, Trailing, - TrailingToken, + AttrWrapper, FollowedByType, ForceCollect, Parser, PathStyle, Trailing, TrailingToken, }; use crate::errors::{self, MacroExpandsToAdtField}; use crate::fluent_generated as fluent; @@ -1540,8 +1539,8 @@ impl<'a> Parser<'a> { this.bump(); // } err.span_label(span, "while parsing this enum"); err.help(help); - err.emit(); - (thin_vec![], Recovered::Yes) + let guar = err.emit(); + (thin_vec![], Recovered::Yes(guar)) } }; VariantData::Struct { fields, recovered: recovered.into() } @@ -1699,16 +1698,15 @@ impl<'a> Parser<'a> { let mut recovered = Recovered::No; if self.eat(&token::OpenDelim(Delimiter::Brace)) { while self.token != token::CloseDelim(Delimiter::Brace) { - let field = self.parse_field_def(adt_ty).map_err(|e| { - self.consume_block(Delimiter::Brace, ConsumeClosingDelim::No); - recovered = Recovered::Yes; - e - }); - match field { - Ok(field) => fields.push(field), + match self.parse_field_def(adt_ty) { + Ok(field) => { + fields.push(field); + } Err(mut err) => { + self.consume_block(Delimiter::Brace, ConsumeClosingDelim::No); err.span_label(ident_span, format!("while parsing this {adt_ty}")); - err.emit(); + let guar = err.emit(); + recovered = Recovered::Yes(guar); break; } } @@ -2469,7 +2467,7 @@ impl<'a> Parser<'a> { // `self.expected_tokens`, therefore, do not use `self.unexpected()` which doesn't // account for this. match self.expect_one_of(&[], &[]) { - Ok(Recovered::Yes) => {} + Ok(Recovered::Yes(_)) => {} Ok(Recovered::No) => unreachable!(), Err(mut err) => { // Qualifier keywords ordering check diff --git a/compiler/rustc_parse/src/parser/mod.rs b/compiler/rustc_parse/src/parser/mod.rs index 7dedf038bbae8..bfb6c4a38858e 100644 --- a/compiler/rustc_parse/src/parser/mod.rs +++ b/compiler/rustc_parse/src/parser/mod.rs @@ -27,13 +27,12 @@ use rustc_ast::tokenstream::{TokenStream, TokenTree, TokenTreeCursor}; use rustc_ast::util::case::Case; use rustc_ast::{ self as ast, AnonConst, AttrArgs, AttrArgsEq, AttrId, ByRef, Const, CoroutineKind, DelimArgs, - Expr, ExprKind, Extern, HasAttrs, HasTokens, Mutability, StrLit, Unsafe, Visibility, + Expr, ExprKind, Extern, HasAttrs, HasTokens, Mutability, Recovered, StrLit, Unsafe, Visibility, VisibilityKind, DUMMY_NODE_ID, }; use rustc_ast_pretty::pprust; use rustc_data_structures::fx::FxHashMap; -use rustc_errors::PResult; -use rustc_errors::{Applicability, Diag, FatalError, MultiSpan}; +use rustc_errors::{Applicability, Diag, FatalError, MultiSpan, PResult}; use rustc_session::parse::ParseSess; use rustc_span::symbol::{kw, sym, Ident, Symbol}; use rustc_span::{Span, DUMMY_SP}; @@ -374,19 +373,6 @@ pub enum FollowedByType { No, } -/// Whether a function performed recovery -#[derive(Copy, Clone, Debug)] -pub enum Recovered { - No, - Yes, -} - -impl From for bool { - fn from(r: Recovered) -> bool { - matches!(r, Recovered::Yes) - } -} - #[derive(Copy, Clone, Debug)] pub enum Trailing { No, @@ -856,9 +842,9 @@ impl<'a> Parser<'a> { Ok(Recovered::No) => { self.current_closure.take(); } - Ok(Recovered::Yes) => { + Ok(Recovered::Yes(guar)) => { self.current_closure.take(); - recovered = Recovered::Yes; + recovered = Recovered::Yes(guar); break; } Err(mut expect_err) => { diff --git a/compiler/rustc_parse/src/parser/stmt.rs b/compiler/rustc_parse/src/parser/stmt.rs index 6601011665b41..d70afebf1b2da 100644 --- a/compiler/rustc_parse/src/parser/stmt.rs +++ b/compiler/rustc_parse/src/parser/stmt.rs @@ -11,14 +11,13 @@ use crate::errors; use crate::maybe_whole; use crate::errors::MalformedLoopLabel; -use crate::parser::Recovered; use ast::Label; use rustc_ast as ast; use rustc_ast::ptr::P; use rustc_ast::token::{self, Delimiter, TokenKind}; use rustc_ast::util::classify; use rustc_ast::{AttrStyle, AttrVec, LocalKind, MacCall, MacCallStmt, MacStmtStyle}; -use rustc_ast::{Block, BlockCheckMode, Expr, ExprKind, HasAttrs, Local, Stmt}; +use rustc_ast::{Block, BlockCheckMode, Expr, ExprKind, HasAttrs, Local, Recovered, Stmt}; use rustc_ast::{StmtKind, DUMMY_NODE_ID}; use rustc_errors::{Applicability, Diag, PResult}; use rustc_span::symbol::{kw, sym, Ident}; @@ -675,11 +674,8 @@ impl<'a> Parser<'a> { let replace_with_err = 'break_recover: { match expect_result { Ok(Recovered::No) => None, - Ok(Recovered::Yes) => { + Ok(Recovered::Yes(guar)) => { // Skip type error to avoid extra errors. - let guar = self - .dcx() - .span_delayed_bug(self.prev_token.span, "expected `;` or `}`"); Some(guar) } Err(e) => { diff --git a/tests/ui-fulldeps/pprust-expr-roundtrip.rs b/tests/ui-fulldeps/pprust-expr-roundtrip.rs index 475a69f4ad098..2b1fec9438717 100644 --- a/tests/ui-fulldeps/pprust-expr-roundtrip.rs +++ b/tests/ui-fulldeps/pprust-expr-roundtrip.rs @@ -180,7 +180,10 @@ fn iter_exprs(depth: usize, f: &mut dyn FnMut(P)) { 18 => { let pat = P(Pat { id: DUMMY_NODE_ID, kind: PatKind::Wild, span: DUMMY_SP, tokens: None }); - iter_exprs(depth - 1, &mut |e| g(ExprKind::Let(pat.clone(), e, DUMMY_SP, None))) + iter_exprs( + depth - 1, + &mut |e| g(ExprKind::Let(pat.clone(), e, DUMMY_SP, Recovered::No)) + ) } _ => panic!("bad counter value in iter_exprs"), } From 41d36a0951fb0139d722c8e0b27f5edbf379fa05 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Thu, 9 May 2024 13:09:47 +0200 Subject: [PATCH 3/5] interpret/miri: better errors on failing offset_from --- compiler/rustc_const_eval/messages.ftl | 15 +++++----- .../src/interpret/intrinsics.rs | 29 ++++++++++--------- .../ptr_offset_from_different_ints.rs | 21 ++++++++++++++ .../ptr_offset_from_different_ints.stderr | 15 ++++++++++ tests/ui/consts/offset_from_ub.rs | 8 ++--- tests/ui/consts/offset_from_ub.stderr | 6 ++-- 6 files changed, 67 insertions(+), 27 deletions(-) create mode 100644 src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.rs create mode 100644 src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.stderr diff --git a/compiler/rustc_const_eval/messages.ftl b/compiler/rustc_const_eval/messages.ftl index b79d7441acac7..20f0f27517ffe 100644 --- a/compiler/rustc_const_eval/messages.ftl +++ b/compiler/rustc_const_eval/messages.ftl @@ -69,9 +69,6 @@ const_eval_deref_function_pointer = accessing {$allocation} which contains a function const_eval_deref_vtable_pointer = accessing {$allocation} which contains a vtable -const_eval_different_allocations = - `{$name}` called on pointers into different allocations - const_eval_division_by_zero = dividing by zero const_eval_division_overflow = @@ -234,12 +231,18 @@ const_eval_not_enough_caller_args = const_eval_nullary_intrinsic_fail = could not evaluate nullary intrinsic +const_eval_offset_from_different_allocations = + `{$name}` called on pointers into different allocations +const_eval_offset_from_different_integers = + `{$name}` called on different pointers without provenance (i.e., without an associated allocation) const_eval_offset_from_overflow = `{$name}` called when first pointer is too far ahead of second - -const_eval_offset_from_test = out-of-bounds `offset_from` +const_eval_offset_from_test = + out-of-bounds `offset_from` const_eval_offset_from_underflow = `{$name}` called when first pointer is too far before second +const_eval_offset_from_unsigned_overflow = + `ptr_offset_from_unsigned` called when first pointer has smaller offset than second: {$a_offset} < {$b_offset} const_eval_operator_non_const = cannot call non-const operator in {const_eval_const_context}s @@ -381,8 +384,6 @@ const_eval_unreachable = entering unreachable code const_eval_unreachable_unwind = unwinding past a stack frame that does not allow unwinding -const_eval_unsigned_offset_from_overflow = - `ptr_offset_from_unsigned` called when first pointer has smaller offset than second: {$a_offset} < {$b_offset} const_eval_unsized_local = unsized locals are not supported const_eval_unstable_const_fn = `{$def_path}` is not yet stable as a const fn diff --git a/compiler/rustc_const_eval/src/interpret/intrinsics.rs b/compiler/rustc_const_eval/src/interpret/intrinsics.rs index 88ce5a7cbebba..c53f068c9a46a 100644 --- a/compiler/rustc_const_eval/src/interpret/intrinsics.rs +++ b/compiler/rustc_const_eval/src/interpret/intrinsics.rs @@ -8,21 +8,16 @@ use rustc_middle::ty::layout::{LayoutOf as _, ValidityRequirement}; use rustc_middle::ty::GenericArgsRef; use rustc_middle::ty::{Ty, TyCtxt}; use rustc_middle::{ - mir::{ - self, - interpret::{ - Allocation, ConstAllocation, GlobalId, InterpResult, PointerArithmetic, Scalar, - }, - BinOp, ConstValue, NonDivergingIntrinsic, - }, + mir::{self, BinOp, ConstValue, NonDivergingIntrinsic}, ty::layout::TyAndLayout, }; use rustc_span::symbol::{sym, Symbol}; use rustc_target::abi::Size; use super::{ - memory::MemoryKind, util::ensure_monomorphic_enough, CheckInAllocMsg, ImmTy, InterpCx, - MPlaceTy, Machine, OpTy, Pointer, + memory::MemoryKind, util::ensure_monomorphic_enough, Allocation, CheckInAllocMsg, + ConstAllocation, GlobalId, ImmTy, InterpCx, InterpResult, MPlaceTy, Machine, OpTy, Pointer, + PointerArithmetic, Scalar, }; use crate::fluent_generated as fluent; @@ -249,14 +244,22 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { match (self.ptr_try_get_alloc_id(a), self.ptr_try_get_alloc_id(b)) { (Err(a), Err(b)) => { // Neither pointer points to an allocation. - // If these are inequal or null, this *will* fail the deref check below. + // This is okay only if they are the same. + if a != b { + // We'd catch this below in the "dereferenceable" check, but + // show a nicer error for this particular case. + throw_ub_custom!( + fluent::const_eval_offset_from_different_integers, + name = intrinsic_name, + ); + } (a, b) } (Err(_), _) | (_, Err(_)) => { // We managed to find a valid allocation for one pointer, but not the other. // That means they are definitely not pointing to the same allocation. throw_ub_custom!( - fluent::const_eval_different_allocations, + fluent::const_eval_offset_from_different_allocations, name = intrinsic_name, ); } @@ -264,7 +267,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // Found allocation for both. They must be into the same allocation. if a_alloc_id != b_alloc_id { throw_ub_custom!( - fluent::const_eval_different_allocations, + fluent::const_eval_offset_from_different_allocations, name = intrinsic_name, ); } @@ -286,7 +289,7 @@ impl<'mir, 'tcx: 'mir, M: Machine<'mir, 'tcx>> InterpCx<'mir, 'tcx, M> { // a < b if intrinsic_name == sym::ptr_offset_from_unsigned { throw_ub_custom!( - fluent::const_eval_unsigned_offset_from_overflow, + fluent::const_eval_offset_from_unsigned_overflow, a_offset = a_offset, b_offset = b_offset, ); diff --git a/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.rs b/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.rs new file mode 100644 index 0000000000000..57b4fd0dedb7c --- /dev/null +++ b/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.rs @@ -0,0 +1,21 @@ +#![feature(strict_provenance)] +use core::ptr; + +fn main() { + unsafe { + let base = ptr::without_provenance::<()>(10); + let unit = &*base; + let p1 = unit as *const (); + + let base = ptr::without_provenance::<()>(11); + let unit = &*base; + let p2 = unit as *const (); + + // Seems to work because they are same pointer + // even though it's dangling. + let _ = p1.byte_offset_from(p1); + + // UB because different pointers. + let _ = p1.byte_offset_from(p2); //~ERROR: different pointers without provenance + } +} diff --git a/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.stderr b/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.stderr new file mode 100644 index 0000000000000..6e9e5633fecc6 --- /dev/null +++ b/src/tools/miri/tests/fail/intrinsics/ptr_offset_from_different_ints.stderr @@ -0,0 +1,15 @@ +error: Undefined Behavior: `ptr_offset_from` called on different pointers without provenance (i.e., without an associated allocation) + --> $DIR/ptr_offset_from_different_ints.rs:LL:CC + | +LL | let _ = p1.byte_offset_from(p2); + | ^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from` called on different pointers without provenance (i.e., without an associated allocation) + | + = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior + = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information + = note: BACKTRACE: + = note: inside `main` at $DIR/ptr_offset_from_different_ints.rs:LL:CC + +note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace + +error: aborting due to 1 previous error + diff --git a/tests/ui/consts/offset_from_ub.rs b/tests/ui/consts/offset_from_ub.rs index 51163e650d6aa..e0dd27079156f 100644 --- a/tests/ui/consts/offset_from_ub.rs +++ b/tests/ui/consts/offset_from_ub.rs @@ -42,7 +42,7 @@ pub const DIFFERENT_INT: isize = { // offset_from with two different integers: l let ptr1 = 8 as *const u8; let ptr2 = 16 as *const u8; unsafe { ptr_offset_from(ptr2, ptr1) } //~ERROR evaluation of constant value failed - //~| 0x8[noalloc] is a dangling pointer + //~| different pointers without provenance }; const OUT_OF_BOUNDS_1: isize = { @@ -81,13 +81,13 @@ pub const DIFFERENT_ALLOC_UNSIGNED: usize = { }; pub const TOO_FAR_APART1: isize = { - let ptr1 = ptr::null::(); + let ptr1 = &0u8 as *const u8; let ptr2 = ptr1.wrapping_add(isize::MAX as usize + 42); unsafe { ptr_offset_from(ptr2, ptr1) } //~ERROR evaluation of constant value failed //~| too far ahead }; pub const TOO_FAR_APART2: isize = { - let ptr1 = ptr::null::(); + let ptr1 = &0u8 as *const u8; let ptr2 = ptr1.wrapping_add(isize::MAX as usize + 42); unsafe { ptr_offset_from(ptr1, ptr2) } //~ERROR evaluation of constant value failed //~| too far before @@ -100,7 +100,7 @@ const WRONG_ORDER_UNSIGNED: usize = { //~| first pointer has smaller offset than second: 0 < 8 }; pub const TOO_FAR_APART_UNSIGNED: usize = { - let ptr1 = ptr::null::(); + let ptr1 = &0u8 as *const u8; let ptr2 = ptr1.wrapping_add(isize::MAX as usize + 42); // This would fit into a `usize` but we still don't allow it. unsafe { ptr_offset_from_unsigned(ptr2, ptr1) } //~ERROR evaluation of constant value failed diff --git a/tests/ui/consts/offset_from_ub.stderr b/tests/ui/consts/offset_from_ub.stderr index 4fbb2f00100a6..e3bac8d5e31ad 100644 --- a/tests/ui/consts/offset_from_ub.stderr +++ b/tests/ui/consts/offset_from_ub.stderr @@ -33,7 +33,7 @@ error[E0080]: evaluation of constant value failed --> $DIR/offset_from_ub.rs:44:14 | LL | unsafe { ptr_offset_from(ptr2, ptr1) } - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds `offset_from`: 0x8[noalloc] is a dangling pointer (it has no provenance) + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^ `ptr_offset_from` called on different pointers without provenance (i.e., without an associated allocation) error[E0080]: evaluation of constant value failed --> $DIR/offset_from_ub.rs:53:14 @@ -86,7 +86,7 @@ LL | unsafe { ptr_offset_from_unsigned(ptr2, ptr1) } error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: out-of-bounds `offset_from`: null pointer is a dangling pointer (it has no provenance) + = note: `ptr_offset_from` called on different pointers without provenance (i.e., without an associated allocation) | note: inside `std::ptr::const_ptr::::offset_from` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL @@ -99,7 +99,7 @@ LL | unsafe { ptr2.offset_from(ptr1) } error[E0080]: evaluation of constant value failed --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL | - = note: out-of-bounds `offset_from`: null pointer is a dangling pointer (it has no provenance) + = note: `ptr_offset_from` called on different pointers without provenance (i.e., without an associated allocation) | note: inside `std::ptr::const_ptr::::offset_from` --> $SRC_DIR/core/src/ptr/const_ptr.rs:LL:COL From 3c5255391232dab1a6a5764be671dbd5315fe81e Mon Sep 17 00:00:00 2001 From: Alex Macleod Date: Thu, 9 May 2024 13:18:09 +0100 Subject: [PATCH 4/5] Make `#![feature]` suggestion MaybeIncorrect --- compiler/rustc_middle/src/ty/context.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_middle/src/ty/context.rs b/compiler/rustc_middle/src/ty/context.rs index d2eacdf762f1b..94b13269bfdfe 100644 --- a/compiler/rustc_middle/src/ty/context.rs +++ b/compiler/rustc_middle/src/ty/context.rs @@ -2449,7 +2449,7 @@ impl<'tcx> TyCtxt<'tcx> { span, msg, format!("#![feature({feature})]\n"), - Applicability::MachineApplicable, + Applicability::MaybeIncorrect, ); } else { diag.help(msg); From 5120010b02629744c5bd5b87bd21f976fd902eb5 Mon Sep 17 00:00:00 2001 From: goofylfg <165781272+goofylfg@users.noreply.github.com> Date: Thu, 9 May 2024 12:41:48 +0000 Subject: [PATCH 5/5] chore: remove repetitive words --- compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs | 2 +- compiler/rustc_borrowck/src/region_infer/opaque_types.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs index 6c82dd1489ca2..6f0bd0543293c 100644 --- a/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs +++ b/compiler/rustc_borrowck/src/diagnostics/conflict_errors.rs @@ -494,7 +494,7 @@ impl<'cx, 'tcx> MirBorrowckCtxt<'cx, 'tcx> { { self.suggest_cloning(err, ty, expr, None, Some(move_spans)); } else if self.suggest_hoisting_call_outside_loop(err, expr) { - // The place where the the type moves would be misleading to suggest clone. + // The place where the type moves would be misleading to suggest clone. // #121466 self.suggest_cloning(err, ty, expr, None, Some(move_spans)); } diff --git a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs index 73ba5bee13b7f..f601a9d70739f 100644 --- a/compiler/rustc_borrowck/src/region_infer/opaque_types.rs +++ b/compiler/rustc_borrowck/src/region_infer/opaque_types.rs @@ -40,7 +40,7 @@ impl<'tcx> RegionInferenceContext<'tcx> { /// compares lifetimes directly, so we need to map the inference variables /// back to concrete lifetimes: `'static`, `ReEarlyParam` or `ReLateParam`. /// - /// First we map the regions in the the generic parameters `_Return<'1>` to + /// First we map the regions in the generic parameters `_Return<'1>` to /// their `external_name` giving `_Return<'a>`. This step is a bit involved. /// See the [rustc-dev-guide chapter] for more info. ///