Skip to content

Commit 9d21092

Browse files
borsbors
committed
Auto merge of rust-lang#115677 - matthewjasper:let-expr-recovery, r=b-naber
Improve invalid let expression handling - Move all of the checks for valid let expression positions to parsing. - Add a field to ExprKind::Let in AST/HIR to mark whether it's in a valid location. - Suppress some later errors and MIR construction for invalid let expressions. - Fix a (drop) scope issue that was also responsible for rust-lang#104172. Fixes rust-lang#104172 Fixes rust-lang#104868
2 parents 8142a31 + e324a59 commit 9d21092

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

48 files changed

+2207
-2019
lines changed

Diff for: ‎compiler/rustc_ast/src/ast.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ use rustc_macros::HashStable_Generic;
3333
use rustc_serialize::{Decodable, Decoder, Encodable, Encoder};
3434
use rustc_span::source_map::{respan, Spanned};
3535
use rustc_span::symbol::{kw, sym, Ident, Symbol};
36-
use rustc_span::{Span, DUMMY_SP};
36+
use rustc_span::{ErrorGuaranteed, Span, DUMMY_SP};
3737
use std::fmt;
3838
use std::mem;
3939
use thin_vec::{thin_vec, ThinVec};
@@ -1426,7 +1426,7 @@ pub enum ExprKind {
14261426
/// of `if` / `while` expressions. (e.g., `if let 0 = x { .. }`).
14271427
///
14281428
/// `Span` represents the whole `let pat = expr` statement.
1429-
Let(P<Pat>, P<Expr>, Span),
1429+
Let(P<Pat>, P<Expr>, Span, Option<ErrorGuaranteed>),
14301430
/// An `if` block, with an optional `else` block.
14311431
///
14321432
/// `if expr { block } else { expr }`

Diff for: ‎compiler/rustc_ast/src/mut_visit.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1366,7 +1366,7 @@ pub fn noop_visit_expr<T: MutVisitor>(
13661366
vis.visit_ty(ty);
13671367
}
13681368
ExprKind::AddrOf(_, _, ohs) => vis.visit_expr(ohs),
1369-
ExprKind::Let(pat, scrutinee, _) => {
1369+
ExprKind::Let(pat, scrutinee, _, _) => {
13701370
vis.visit_pat(pat);
13711371
vis.visit_expr(scrutinee);
13721372
}

Diff for: ‎compiler/rustc_ast/src/util/classify.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ pub fn expr_trailing_brace(mut expr: &ast::Expr) -> Option<&ast::Expr> {
3636
| AssignOp(_, _, e)
3737
| Binary(_, _, e)
3838
| Break(_, Some(e))
39-
| Let(_, e, _)
39+
| Let(_, e, _, _)
4040
| Range(_, Some(e), _)
4141
| Ret(Some(e))
4242
| Unary(_, e)

Diff for: ‎compiler/rustc_ast/src/visit.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -827,7 +827,7 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) {
827827
visitor.visit_expr(subexpression);
828828
visitor.visit_ty(typ)
829829
}
830-
ExprKind::Let(pat, expr, _) => {
830+
ExprKind::Let(pat, expr, _, _) => {
831831
visitor.visit_pat(pat);
832832
visitor.visit_expr(expr);
833833
}

Diff for: ‎compiler/rustc_ast_lowering/src/expr.rs

+4-2
Original file line numberDiff line numberDiff line change
@@ -152,13 +152,14 @@ impl<'hir> LoweringContext<'_, 'hir> {
152152
let ohs = self.lower_expr(ohs);
153153
hir::ExprKind::AddrOf(*k, *m, ohs)
154154
}
155-
ExprKind::Let(pat, scrutinee, span) => {
155+
ExprKind::Let(pat, scrutinee, span, is_recovered) => {
156156
hir::ExprKind::Let(self.arena.alloc(hir::Let {
157157
hir_id: self.next_id(),
158158
span: self.lower_span(*span),
159159
pat: self.lower_pat(pat),
160160
ty: None,
161161
init: self.lower_expr(scrutinee),
162+
is_recovered: *is_recovered,
162163
}))
163164
}
164165
ExprKind::If(cond, then, else_opt) => {
@@ -558,13 +559,14 @@ impl<'hir> LoweringContext<'_, 'hir> {
558559
fn lower_arm(&mut self, arm: &Arm) -> hir::Arm<'hir> {
559560
let pat = self.lower_pat(&arm.pat);
560561
let guard = arm.guard.as_ref().map(|cond| {
561-
if let ExprKind::Let(pat, scrutinee, span) = &cond.kind {
562+
if let ExprKind::Let(pat, scrutinee, span, is_recovered) = &cond.kind {
562563
hir::Guard::IfLet(self.arena.alloc(hir::Let {
563564
hir_id: self.next_id(),
564565
span: self.lower_span(*span),
565566
pat: self.lower_pat(pat),
566567
ty: None,
567568
init: self.lower_expr(scrutinee),
569+
is_recovered: *is_recovered,
568570
}))
569571
} else {
570572
hir::Guard::If(self.lower_expr(cond))

Diff for: ‎compiler/rustc_ast_passes/messages.ftl

-10
Original file line numberDiff line numberDiff line change
@@ -117,16 +117,6 @@ ast_passes_forbidden_default =
117117
`default` is only allowed on items in trait impls
118118
.label = `default` because of this
119119
120-
ast_passes_forbidden_let =
121-
`let` expressions are not supported here
122-
.note = only supported directly in conditions of `if` and `while` expressions
123-
.not_supported_or = `||` operators are not supported in let chain expressions
124-
.not_supported_parentheses = `let`s wrapped in parentheses are not supported in a context with let chains
125-
126-
ast_passes_forbidden_let_stable =
127-
expected expression, found statement (`let`)
128-
.note = variable declaration using `let` is a statement
129-
130120
ast_passes_forbidden_lifetime_bound =
131121
lifetime bounds cannot be used in this context
132122

Diff for: ‎compiler/rustc_ast_passes/src/ast_validation.rs

-103
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,12 @@ use rustc_ast::{walk_list, StaticItem};
1414
use rustc_ast_pretty::pprust::{self, State};
1515
use rustc_data_structures::fx::FxIndexMap;
1616
use rustc_feature::Features;
17-
use rustc_macros::Subdiagnostic;
1817
use rustc_parse::validate_attr;
1918
use rustc_session::lint::builtin::{
2019
DEPRECATED_WHERE_CLAUSE_LOCATION, MISSING_ABI, PATTERNS_IN_FNS_WITHOUT_BODY,
2120
};
2221
use rustc_session::lint::{BuiltinLintDiagnostics, LintBuffer};
2322
use rustc_session::Session;
24-
use rustc_span::source_map::Spanned;
2523
use rustc_span::symbol::{kw, sym, Ident};
2624
use rustc_span::Span;
2725
use rustc_target::spec::abi;
@@ -69,9 +67,6 @@ struct AstValidator<'a> {
6967
/// or `Foo::Bar<impl Trait>`
7068
is_impl_trait_banned: bool,
7169

72-
/// See [ForbiddenLetReason]
73-
forbidden_let_reason: Option<ForbiddenLetReason>,
74-
7570
lint_buffer: &'a mut LintBuffer,
7671
}
7772

@@ -118,26 +113,6 @@ impl<'a> AstValidator<'a> {
118113
self.with_tilde_const(Some(ctx), f)
119114
}
120115

121-
fn with_let_management(
122-
&mut self,
123-
forbidden_let_reason: Option<ForbiddenLetReason>,
124-
f: impl FnOnce(&mut Self, Option<ForbiddenLetReason>),
125-
) {
126-
let old = mem::replace(&mut self.forbidden_let_reason, forbidden_let_reason);
127-
f(self, old);
128-
self.forbidden_let_reason = old;
129-
}
130-
131-
/// Emits an error banning the `let` expression provided in the given location.
132-
fn ban_let_expr(&self, expr: &'a Expr, forbidden_let_reason: ForbiddenLetReason) {
133-
let sess = &self.session;
134-
if sess.opts.unstable_features.is_nightly_build() {
135-
sess.emit_err(errors::ForbiddenLet { span: expr.span, reason: forbidden_let_reason });
136-
} else {
137-
sess.emit_err(errors::ForbiddenLetStable { span: expr.span });
138-
}
139-
}
140-
141116
fn check_type_alias_where_clause_location(
142117
&mut self,
143118
ty_alias: &TyAlias,
@@ -779,67 +754,6 @@ impl<'a> Visitor<'a> for AstValidator<'a> {
779754
validate_attr::check_attr(&self.session.parse_sess, attr);
780755
}
781756

782-
fn visit_expr(&mut self, expr: &'a Expr) {
783-
self.with_let_management(Some(ForbiddenLetReason::GenericForbidden), |this, forbidden_let_reason| {
784-
match &expr.kind {
785-
ExprKind::Binary(Spanned { node: BinOpKind::Or, span }, lhs, rhs) => {
786-
let local_reason = Some(ForbiddenLetReason::NotSupportedOr(*span));
787-
this.with_let_management(local_reason, |this, _| this.visit_expr(lhs));
788-
this.with_let_management(local_reason, |this, _| this.visit_expr(rhs));
789-
}
790-
ExprKind::If(cond, then, opt_else) => {
791-
this.visit_block(then);
792-
walk_list!(this, visit_expr, opt_else);
793-
this.with_let_management(None, |this, _| this.visit_expr(cond));
794-
return;
795-
}
796-
ExprKind::Let(..) if let Some(elem) = forbidden_let_reason => {
797-
this.ban_let_expr(expr, elem);
798-
},
799-
ExprKind::Match(scrutinee, arms) => {
800-
this.visit_expr(scrutinee);
801-
for arm in arms {
802-
this.visit_expr(&arm.body);
803-
this.visit_pat(&arm.pat);
804-
walk_list!(this, visit_attribute, &arm.attrs);
805-
if let Some(guard) = &arm.guard {
806-
this.with_let_management(None, |this, _| {
807-
this.visit_expr(guard)
808-
});
809-
}
810-
}
811-
}
812-
ExprKind::Paren(local_expr) => {
813-
fn has_let_expr(expr: &Expr) -> bool {
814-
match &expr.kind {
815-
ExprKind::Binary(_, lhs, rhs) => has_let_expr(lhs) || has_let_expr(rhs),
816-
ExprKind::Let(..) => true,
817-
_ => false,
818-
}
819-
}
820-
let local_reason = if has_let_expr(local_expr) {
821-
Some(ForbiddenLetReason::NotSupportedParentheses(local_expr.span))
822-
}
823-
else {
824-
forbidden_let_reason
825-
};
826-
this.with_let_management(local_reason, |this, _| this.visit_expr(local_expr));
827-
}
828-
ExprKind::Binary(Spanned { node: BinOpKind::And, .. }, ..) => {
829-
this.with_let_management(forbidden_let_reason, |this, _| visit::walk_expr(this, expr));
830-
return;
831-
}
832-
ExprKind::While(cond, then, opt_label) => {
833-
walk_list!(this, visit_label, opt_label);
834-
this.visit_block(then);
835-
this.with_let_management(None, |this, _| this.visit_expr(cond));
836-
return;
837-
}
838-
_ => visit::walk_expr(this, expr),
839-
}
840-
});
841-
}
842-
843757
fn visit_ty(&mut self, ty: &'a Ty) {
844758
self.visit_ty_common(ty);
845759
self.deny_anon_struct_or_union(ty);
@@ -1601,26 +1515,9 @@ pub fn check_crate(
16011515
outer_impl_trait: None,
16021516
disallow_tilde_const: None,
16031517
is_impl_trait_banned: false,
1604-
forbidden_let_reason: Some(ForbiddenLetReason::GenericForbidden),
16051518
lint_buffer: lints,
16061519
};
16071520
visit::walk_crate(&mut validator, krate);
16081521

16091522
validator.has_proc_macro_decls
16101523
}
1611-
1612-
/// Used to forbid `let` expressions in certain syntactic locations.
1613-
#[derive(Clone, Copy, Subdiagnostic)]
1614-
pub(crate) enum ForbiddenLetReason {
1615-
/// `let` is not valid and the source environment is not important
1616-
GenericForbidden,
1617-
/// A let chain with the `||` operator
1618-
#[note(ast_passes_not_supported_or)]
1619-
NotSupportedOr(#[primary_span] Span),
1620-
/// A let chain with invalid parentheses
1621-
///
1622-
/// For example, `let 1 = 1 && (expr && expr)` is allowed
1623-
/// but `(let 1 = 1 && (let 1 = 1 && (let 1 = 1))) && let a = 1` is not
1624-
#[note(ast_passes_not_supported_parentheses)]
1625-
NotSupportedParentheses(#[primary_span] Span),
1626-
}

Diff for: ‎compiler/rustc_ast_passes/src/errors.rs

-19
Original file line numberDiff line numberDiff line change
@@ -5,27 +5,8 @@ use rustc_errors::AddToDiagnostic;
55
use rustc_macros::{Diagnostic, Subdiagnostic};
66
use rustc_span::{symbol::Ident, Span, Symbol};
77

8-
use crate::ast_validation::ForbiddenLetReason;
98
use crate::fluent_generated as fluent;
109

11-
#[derive(Diagnostic)]
12-
#[diag(ast_passes_forbidden_let)]
13-
#[note]
14-
pub struct ForbiddenLet {
15-
#[primary_span]
16-
pub span: Span,
17-
#[subdiagnostic]
18-
pub(crate) reason: ForbiddenLetReason,
19-
}
20-
21-
#[derive(Diagnostic)]
22-
#[diag(ast_passes_forbidden_let_stable)]
23-
#[note]
24-
pub struct ForbiddenLetStable {
25-
#[primary_span]
26-
pub span: Span,
27-
}
28-
2910
#[derive(Diagnostic)]
3011
#[diag(ast_passes_keyword_lifetime)]
3112
pub struct KeywordLifetime {

Diff for: ‎compiler/rustc_ast_pretty/src/pprust/state/expr.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ impl<'a> State<'a> {
352352
self.end();
353353
self.word(")");
354354
}
355-
ast::ExprKind::Let(pat, scrutinee, _) => {
355+
ast::ExprKind::Let(pat, scrutinee, _, _) => {
356356
self.print_let(pat, scrutinee);
357357
}
358358
ast::ExprKind::If(test, blk, elseopt) => self.print_if(test, blk, elseopt.as_deref()),

Diff for: ‎compiler/rustc_builtin_macros/src/assert/context.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,7 @@ impl<'cx, 'a> Context<'cx, 'a> {
241241
self.manage_cond_expr(prefix);
242242
self.manage_cond_expr(suffix);
243243
}
244-
ExprKind::Let(_, local_expr, _) => {
244+
ExprKind::Let(_, local_expr, _, _) => {
245245
self.manage_cond_expr(local_expr);
246246
}
247247
ExprKind::Match(local_expr, _) => {

Diff for: ‎compiler/rustc_hir/src/hir.rs

+4
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ use rustc_macros::HashStable_Generic;
1919
use rustc_span::hygiene::MacroKind;
2020
use rustc_span::source_map::Spanned;
2121
use rustc_span::symbol::{kw, sym, Ident, Symbol};
22+
use rustc_span::ErrorGuaranteed;
2223
use rustc_span::{def_id::LocalDefId, BytePos, Span, DUMMY_SP};
2324
use rustc_target::asm::InlineAsmRegOrRegClass;
2425
use rustc_target::spec::abi::Abi;
@@ -1415,6 +1416,9 @@ pub struct Let<'hir> {
14151416
pub pat: &'hir Pat<'hir>,
14161417
pub ty: Option<&'hir Ty<'hir>>,
14171418
pub init: &'hir Expr<'hir>,
1419+
/// `Some` when this let expressions is not in a syntanctically valid location.
1420+
/// Used to prevent building MIR in such situations.
1421+
pub is_recovered: Option<ErrorGuaranteed>,
14181422
}
14191423

14201424
#[derive(Debug, Clone, Copy, HashStable_Generic)]

Diff for: ‎compiler/rustc_hir_analysis/src/check/region.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,7 @@ fn resolve_block<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, blk: &'tcx h
149149
// From now on, we continue normally.
150150
visitor.cx = prev_cx;
151151
}
152-
hir::StmtKind::Local(..) | hir::StmtKind::Item(..) => {
152+
hir::StmtKind::Local(..) => {
153153
// Each declaration introduces a subscope for bindings
154154
// introduced by the declaration; this subscope covers a
155155
// suffix of the block. Each subscope in a block has the
@@ -163,6 +163,10 @@ fn resolve_block<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, blk: &'tcx h
163163
visitor.cx.var_parent = visitor.cx.parent;
164164
visitor.visit_stmt(statement)
165165
}
166+
hir::StmtKind::Item(..) => {
167+
// Don't create scopes for items, since they won't be
168+
// lowered to THIR and MIR.
169+
}
166170
hir::StmtKind::Expr(..) | hir::StmtKind::Semi(..) => visitor.visit_stmt(statement),
167171
}
168172
}

Diff for: ‎compiler/rustc_hir_typeck/src/expr.rs

+6-1
Original file line numberDiff line numberDiff line change
@@ -1203,7 +1203,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
12031203
// otherwise check exactly as a let statement
12041204
self.check_decl(let_expr.into());
12051205
// but return a bool, for this is a boolean expression
1206-
self.tcx.types.bool
1206+
if let Some(error_guaranteed) = let_expr.is_recovered {
1207+
self.set_tainted_by_errors(error_guaranteed);
1208+
Ty::new_error(self.tcx, error_guaranteed)
1209+
} else {
1210+
self.tcx.types.bool
1211+
}
12071212
}
12081213

12091214
fn check_expr_loop(

Diff for: ‎compiler/rustc_hir_typeck/src/gather_locals.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ impl<'a> From<&'a hir::Local<'a>> for Declaration<'a> {
5050

5151
impl<'a> From<&'a hir::Let<'a>> for Declaration<'a> {
5252
fn from(let_expr: &'a hir::Let<'a>) -> Self {
53-
let hir::Let { hir_id, pat, ty, span, init } = *let_expr;
53+
let hir::Let { hir_id, pat, ty, span, init, is_recovered: _ } = *let_expr;
5454
Declaration { hir_id, pat, ty, span, init: Some(init), origin: DeclOrigin::LetExpr }
5555
}
5656
}

Diff for: ‎compiler/rustc_lint/src/unused.rs

+5-7
Original file line numberDiff line numberDiff line change
@@ -816,8 +816,7 @@ trait UnusedDelimLint {
816816
let (value, ctx, followed_by_block, left_pos, right_pos, is_kw) = match e.kind {
817817
// Do not lint `unused_braces` in `if let` expressions.
818818
If(ref cond, ref block, _)
819-
if !matches!(cond.kind, Let(_, _, _))
820-
|| Self::LINT_EXPR_IN_PATTERN_MATCHING_CTX =>
819+
if !matches!(cond.kind, Let(..)) || Self::LINT_EXPR_IN_PATTERN_MATCHING_CTX =>
821820
{
822821
let left = e.span.lo() + rustc_span::BytePos(2);
823822
let right = block.span.lo();
@@ -826,8 +825,7 @@ trait UnusedDelimLint {
826825

827826
// Do not lint `unused_braces` in `while let` expressions.
828827
While(ref cond, ref block, ..)
829-
if !matches!(cond.kind, Let(_, _, _))
830-
|| Self::LINT_EXPR_IN_PATTERN_MATCHING_CTX =>
828+
if !matches!(cond.kind, Let(..)) || Self::LINT_EXPR_IN_PATTERN_MATCHING_CTX =>
831829
{
832830
let left = e.span.lo() + rustc_span::BytePos(5);
833831
let right = block.span.lo();
@@ -1003,7 +1001,7 @@ impl UnusedDelimLint for UnusedParens {
10031001
self.emit_unused_delims_expr(cx, value, ctx, left_pos, right_pos, is_kw)
10041002
}
10051003
}
1006-
ast::ExprKind::Let(_, ref expr, _) => {
1004+
ast::ExprKind::Let(_, ref expr, _, _) => {
10071005
self.check_unused_delims_expr(
10081006
cx,
10091007
expr,
@@ -1067,7 +1065,7 @@ impl EarlyLintPass for UnusedParens {
10671065
}
10681066

10691067
match e.kind {
1070-
ExprKind::Let(ref pat, _, _) | ExprKind::ForLoop(ref pat, ..) => {
1068+
ExprKind::Let(ref pat, _, _, _) | ExprKind::ForLoop(ref pat, ..) => {
10711069
self.check_unused_parens_pat(cx, pat, false, false, (true, true));
10721070
}
10731071
// We ignore parens in cases like `if (((let Some(0) = Some(1))))` because we already
@@ -1311,7 +1309,7 @@ impl UnusedDelimLint for UnusedBraces {
13111309
}
13121310
}
13131311
}
1314-
ast::ExprKind::Let(_, ref expr, _) => {
1312+
ast::ExprKind::Let(_, ref expr, _, _) => {
13151313
self.check_unused_delims_expr(
13161314
cx,
13171315
expr,

0 commit comments

Comments
 (0)