diff options
| author | Caio <c410.f3r@gmail.com> | 2021-08-08 11:49:13 -0300 |
|---|---|---|
| committer | Caio <c410.f3r@gmail.com> | 2021-08-15 16:18:26 -0300 |
| commit | 6aa9937a768bf13e5f7bd0ee6dd8579403b39058 (patch) | |
| tree | ad4c8d3c2233b953d6088456650d26972c3d581b /compiler | |
| parent | 2d9f2eae84dc3988c5463fdd1b124a9b695447d7 (diff) | |
| download | rust-6aa9937a768bf13e5f7bd0ee6dd8579403b39058.tar.gz rust-6aa9937a768bf13e5f7bd0ee6dd8579403b39058.zip | |
Introduce hir::ExprKind::Let - Take 2
Diffstat (limited to 'compiler')
32 files changed, 634 insertions, 724 deletions
diff --git a/compiler/rustc_ast/src/ast.rs b/compiler/rustc_ast/src/ast.rs index bd6e4c30fc3..575a00cdd0e 100644 --- a/compiler/rustc_ast/src/ast.rs +++ b/compiler/rustc_ast/src/ast.rs @@ -1302,7 +1302,9 @@ pub enum ExprKind { Type(P<Expr>, P<Ty>), /// A `let pat = expr` expression that is only semantically allowed in the condition /// of `if` / `while` expressions. (e.g., `if let 0 = x { .. }`). - Let(P<Pat>, P<Expr>), + /// + /// `Span` represents the whole `let pat = expr` statement. + Let(P<Pat>, P<Expr>, Span), /// An `if` block, with an optional `else` block. /// /// `if expr { block } else { expr }` diff --git a/compiler/rustc_ast/src/mut_visit.rs b/compiler/rustc_ast/src/mut_visit.rs index 87950b44083..c8245831187 100644 --- a/compiler/rustc_ast/src/mut_visit.rs +++ b/compiler/rustc_ast/src/mut_visit.rs @@ -1237,7 +1237,7 @@ pub fn noop_visit_expr<T: MutVisitor>( vis.visit_ty(ty); } ExprKind::AddrOf(_, _, ohs) => vis.visit_expr(ohs), - ExprKind::Let(pat, scrutinee) => { + ExprKind::Let(pat, scrutinee, _) => { vis.visit_pat(pat); vis.visit_expr(scrutinee); } diff --git a/compiler/rustc_ast/src/visit.rs b/compiler/rustc_ast/src/visit.rs index 1ebfcf36711..a377763983a 100644 --- a/compiler/rustc_ast/src/visit.rs +++ b/compiler/rustc_ast/src/visit.rs @@ -779,9 +779,9 @@ pub fn walk_expr<'a, V: Visitor<'a>>(visitor: &mut V, expression: &'a Expr) { visitor.visit_expr(subexpression); visitor.visit_ty(typ) } - ExprKind::Let(ref pat, ref scrutinee) => { + ExprKind::Let(ref pat, ref expr, _) => { visitor.visit_pat(pat); - visitor.visit_expr(scrutinee); + visitor.visit_expr(expr); } ExprKind::If(ref head_expression, ref if_block, ref optional_else) => { visitor.visit_expr(head_expression); diff --git a/compiler/rustc_ast_lowering/src/expr.rs b/compiler/rustc_ast_lowering/src/expr.rs index 3bf7c7c37a4..bf7589e84ad 100644 --- a/compiler/rustc_ast_lowering/src/expr.rs +++ b/compiler/rustc_ast_lowering/src/expr.rs @@ -86,32 +86,12 @@ impl<'hir> LoweringContext<'_, 'hir> { let ohs = self.lower_expr(ohs); hir::ExprKind::AddrOf(k, m, ohs) } - ExprKind::Let(ref pat, ref scrutinee) => { - self.lower_expr_let(e.span, pat, scrutinee) + ExprKind::Let(ref pat, ref scrutinee, span) => { + hir::ExprKind::Let(self.lower_pat(pat), self.lower_expr(scrutinee), span) + } + ExprKind::If(ref cond, ref then, ref else_opt) => { + self.lower_expr_if(cond, then, else_opt.as_deref()) } - ExprKind::If(ref cond, ref then, ref else_opt) => match cond.kind { - ExprKind::Let(ref pat, ref scrutinee) => { - self.lower_expr_if_let(e.span, pat, scrutinee, then, else_opt.as_deref()) - } - ExprKind::Paren(ref paren) => match paren.peel_parens().kind { - ExprKind::Let(ref pat, ref scrutinee) => { - // A user has written `if (let Some(x) = foo) {`, we want to avoid - // confusing them with mentions of nightly features. - // If this logic is changed, you will also likely need to touch - // `unused::UnusedParens::check_expr`. - self.if_let_expr_with_parens(cond, &paren.peel_parens()); - self.lower_expr_if_let( - e.span, - pat, - scrutinee, - then, - else_opt.as_deref(), - ) - } - _ => self.lower_expr_if(cond, then, else_opt.as_deref()), - }, - _ => self.lower_expr_if(cond, then, else_opt.as_deref()), - }, ExprKind::While(ref cond, ref body, opt_label) => self .with_loop_scope(e.id, |this| { this.lower_expr_while_in_loop_scope(e.span, cond, body, opt_label) @@ -368,126 +348,51 @@ impl<'hir> LoweringContext<'_, 'hir> { hir::ExprKind::Call(f, self.lower_exprs(&real_args)) } - fn if_let_expr_with_parens(&mut self, cond: &Expr, paren: &Expr) { - let start = cond.span.until(paren.span); - let end = paren.span.shrink_to_hi().until(cond.span.shrink_to_hi()); - self.sess - .struct_span_err( - vec![start, end], - "invalid parentheses around `let` expression in `if let`", - ) - .multipart_suggestion( - "`if let` needs to be written without parentheses", - vec![(start, String::new()), (end, String::new())], - rustc_errors::Applicability::MachineApplicable, - ) - .emit(); - // Ideally, we'd remove the feature gating of a `let` expression since we are already - // complaining about it here, but `feature_gate::check_crate` has already run by now: - // self.sess.parse_sess.gated_spans.ungate_last(sym::let_chains, paren.span); - } - - /// Emit an error and lower `ast::ExprKind::Let(pat, scrutinee)` into: - /// ```rust - /// match scrutinee { pats => true, _ => false } - /// ``` - fn lower_expr_let(&mut self, span: Span, pat: &Pat, scrutinee: &Expr) -> hir::ExprKind<'hir> { - // If we got here, the `let` expression is not allowed. - - if self.sess.opts.unstable_features.is_nightly_build() { - self.sess - .struct_span_err(span, "`let` expressions are not supported here") - .note( - "only supported directly without parentheses in conditions of `if`- and \ - `while`-expressions, as well as in `let` chains within parentheses", - ) - .emit(); - } else { - self.sess - .struct_span_err(span, "expected expression, found statement (`let`)") - .note("variable declaration using `let` is a statement") - .emit(); - } - - // For better recovery, we emit: - // ``` - // match scrutinee { pat => true, _ => false } - // ``` - // While this doesn't fully match the user's intent, it has key advantages: - // 1. We can avoid using `abort_if_errors`. - // 2. We can typeck both `pat` and `scrutinee`. - // 3. `pat` is allowed to be refutable. - // 4. The return type of the block is `bool` which seems like what the user wanted. - let scrutinee = self.lower_expr(scrutinee); - let then_arm = { - let pat = self.lower_pat(pat); - let expr = self.expr_bool(span, true); - self.arm(pat, expr) - }; - let else_arm = { - let pat = self.pat_wild(span); - let expr = self.expr_bool(span, false); - self.arm(pat, expr) - }; - hir::ExprKind::Match( - scrutinee, - arena_vec![self; then_arm, else_arm], - hir::MatchSource::Normal, - ) - } - fn lower_expr_if( &mut self, cond: &Expr, then: &Block, else_opt: Option<&Expr>, ) -> hir::ExprKind<'hir> { - let cond = self.lower_expr(cond); - let wrapped_cond = match cond.kind { - hir::ExprKind::Let(..) => cond, - _ => self.expr_drop_temps(cond.span, cond, AttrVec::new()), - }; + let lowered_cond = self.lower_expr(cond); + let new_cond = self.manage_let_cond(lowered_cond); let then_expr = self.lower_block_expr(then); if let Some(rslt) = else_opt { - hir::ExprKind::If( - wrapped_cond, - self.arena.alloc(then_expr), - Some(self.lower_expr(rslt)), - ) + hir::ExprKind::If(new_cond, self.arena.alloc(then_expr), Some(self.lower_expr(rslt))) } else { - hir::ExprKind::If(wrapped_cond, self.arena.alloc(then_expr), None) + hir::ExprKind::If(new_cond, self.arena.alloc(then_expr), None) } } - fn lower_expr_if_let( - &mut self, - span: Span, - pat: &Pat, - scrutinee: &Expr, - then: &Block, - else_opt: Option<&Expr>, - ) -> hir::ExprKind<'hir> { - // FIXME(#53667): handle lowering of && and parens. - - // `_ => else_block` where `else_block` is `{}` if there's `None`: - let else_pat = self.pat_wild(span); - let (else_expr, contains_else_clause) = match else_opt { - None => (self.expr_block_empty(span.shrink_to_hi()), false), - Some(els) => (self.lower_expr(els), true), - }; - let else_arm = self.arm(else_pat, else_expr); - - // Handle then + scrutinee: - let scrutinee = self.lower_expr(scrutinee); - let then_pat = self.lower_pat(pat); - - let then_expr = self.lower_block_expr(then); - let then_arm = self.arm(then_pat, self.arena.alloc(then_expr)); - - let desugar = hir::MatchSource::IfLetDesugar { contains_else_clause }; - hir::ExprKind::Match(scrutinee, arena_vec![self; then_arm, else_arm], desugar) + // If `cond` kind is `let`, returns `let`. Otherwise, wraps and returns `cond` + // in a temporary block. + fn manage_let_cond(&mut self, cond: &'hir hir::Expr<'hir>) -> &'hir hir::Expr<'hir> { + match cond.kind { + hir::ExprKind::Let(..) => cond, + _ => { + let span_block = + self.mark_span_with_reason(DesugaringKind::CondTemporary, cond.span, None); + self.expr_drop_temps(span_block, cond, AttrVec::new()) + } + } } + // We desugar: `'label: while $cond $body` into: + // + // ``` + // 'label: loop { + // if { let _t = $cond; _t } { + // $body + // } + // else { + // break; + // } + // } + // ``` + // + // Wrap in a construct equivalent to `{ let _t = $cond; _t }` + // to preserve drop semantics since `while $cond { ... }` does not + // let temporaries live outside of `cond`. fn lower_expr_while_in_loop_scope( &mut self, span: Span, @@ -495,72 +400,17 @@ impl<'hir> LoweringContext<'_, 'hir> { body: &Block, opt_label: Option<Label>, ) -> hir::ExprKind<'hir> { - // FIXME(#53667): handle lowering of && and parens. - - // Note that the block AND the condition are evaluated in the loop scope. - // This is done to allow `break` from inside the condition of the loop. - - // `_ => break`: - let else_arm = { - let else_pat = self.pat_wild(span); - let else_expr = self.expr_break(span, ThinVec::new()); - self.arm(else_pat, else_expr) - }; - - // Handle then + scrutinee: - let (then_pat, scrutinee, desugar, source) = match cond.kind { - ExprKind::Let(ref pat, ref scrutinee) => { - // to: - // - // [opt_ident]: loop { - // match <sub_expr> { - // <pat> => <body>, - // _ => break - // } - // } - let scrutinee = self.with_loop_condition_scope(|t| t.lower_expr(scrutinee)); - let pat = self.lower_pat(pat); - (pat, scrutinee, hir::MatchSource::WhileLetDesugar, hir::LoopSource::WhileLet) - } - _ => { - // We desugar: `'label: while $cond $body` into: - // - // ``` - // 'label: loop { - // match drop-temps { $cond } { - // true => $body, - // _ => break, - // } - // } - // ``` - - // Lower condition: - let cond = self.with_loop_condition_scope(|this| this.lower_expr(cond)); - let span_block = - self.mark_span_with_reason(DesugaringKind::CondTemporary, cond.span, None); - // Wrap in a construct equivalent to `{ let _t = $cond; _t }` - // to preserve drop semantics since `while cond { ... }` does not - // let temporaries live outside of `cond`. - let cond = self.expr_drop_temps(span_block, cond, ThinVec::new()); - // `true => <then>`: - let pat = self.pat_bool(span, true); - (pat, cond, hir::MatchSource::WhileDesugar, hir::LoopSource::While) - } - }; - let then_expr = self.lower_block_expr(body); - let then_arm = self.arm(then_pat, self.arena.alloc(then_expr)); - - // `match <scrutinee> { ... }` - let match_expr = - self.expr_match(span, scrutinee, arena_vec![self; then_arm, else_arm], desugar); - - // `[opt_ident]: loop { ... }` - hir::ExprKind::Loop( - self.block_expr(self.arena.alloc(match_expr)), - opt_label, - source, - span.with_hi(cond.span.hi()), - ) + let lowered_cond = self.with_loop_condition_scope(|t| t.lower_expr(cond)); + let new_cond = self.manage_let_cond(lowered_cond); + let then = self.lower_block_expr(body); + let expr_break = self.expr_break(span, ThinVec::new()); + let stmt_break = self.stmt_expr(span, expr_break); + let else_blk = self.block_all(span, arena_vec![self; stmt_break], None); + let else_expr = self.arena.alloc(self.expr_block(else_blk, ThinVec::new())); + let if_kind = hir::ExprKind::If(new_cond, self.arena.alloc(then), Some(else_expr)); + let if_expr = self.expr(span, if_kind, ThinVec::new()); + let block = self.block_expr(self.arena.alloc(if_expr)); + hir::ExprKind::Loop(block, opt_label, hir::LoopSource::While, span.with_hi(cond.span.hi())) } /// Desugar `try { <stmts>; <expr> }` into `{ <stmts>; ::std::ops::Try::from_output(<expr>) }`, @@ -620,7 +470,7 @@ impl<'hir> LoweringContext<'_, 'hir> { fn lower_arm(&mut self, arm: &Arm) -> hir::Arm<'hir> { let pat = self.lower_pat(&arm.pat); let guard = arm.guard.as_ref().map(|cond| { - if let ExprKind::Let(ref pat, ref scrutinee) = cond.kind { + if let ExprKind::Let(ref pat, ref scrutinee, _) = cond.kind { hir::Guard::IfLet(self.lower_pat(pat), self.lower_expr(scrutinee)) } else { hir::Guard::If(self.lower_expr(cond)) @@ -1468,7 +1318,7 @@ impl<'hir> LoweringContext<'_, 'hir> { // `::std::option::Option::None => break` let break_arm = { let break_expr = - self.with_loop_scope(e.id, |this| this.expr_break(e.span, ThinVec::new())); + self.with_loop_scope(e.id, |this| this.expr_break_alloc(e.span, ThinVec::new())); let pat = self.pat_none(e.span); self.arm(pat, break_expr) }; @@ -1681,12 +1531,6 @@ impl<'hir> LoweringContext<'_, 'hir> { // Helper methods for building HIR. // ========================================================================= - /// Constructs a `true` or `false` literal expression. - pub(super) fn expr_bool(&mut self, span: Span, val: bool) -> &'hir hir::Expr<'hir> { - let lit = Spanned { span, node: LitKind::Bool(val) }; - self.arena.alloc(self.expr(span, hir::ExprKind::Lit(lit), ThinVec::new())) - } - /// Wrap the given `expr` in a terminating scope using `hir::ExprKind::DropTemps`. /// /// In terms of drop order, it has the same effect as wrapping `expr` in @@ -1721,9 +1565,14 @@ impl<'hir> LoweringContext<'_, 'hir> { self.expr(span, hir::ExprKind::Match(arg, arms, source), ThinVec::new()) } - fn expr_break(&mut self, span: Span, attrs: AttrVec) -> &'hir hir::Expr<'hir> { + fn expr_break(&mut self, span: Span, attrs: AttrVec) -> hir::Expr<'hir> { let expr_break = hir::ExprKind::Break(self.lower_loop_destination(None), None); - self.arena.alloc(self.expr(span, expr_break, attrs)) + self.expr(span, expr_break, attrs) + } + + fn expr_break_alloc(&mut self, span: Span, attrs: AttrVec) -> &'hir hir::Expr<'hir> { + let expr_break = self.expr_break(span, attrs); + self.arena.alloc(expr_break) } fn expr_mut_addr_of(&mut self, span: Span, e: &'hir hir::Expr<'hir>) -> hir::Expr<'hir> { diff --git a/compiler/rustc_ast_lowering/src/lib.rs b/compiler/rustc_ast_lowering/src/lib.rs index 581f177ad14..d41b3b81466 100644 --- a/compiler/rustc_ast_lowering/src/lib.rs +++ b/compiler/rustc_ast_lowering/src/lib.rs @@ -2537,12 +2537,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { self.arena.alloc(blk) } - /// Constructs a `true` or `false` literal pattern. - fn pat_bool(&mut self, span: Span, val: bool) -> &'hir hir::Pat<'hir> { - let expr = self.expr_bool(span, val); - self.pat(span, hir::PatKind::Lit(expr)) - } - fn pat_cf_continue(&mut self, span: Span, pat: &'hir hir::Pat<'hir>) -> &'hir hir::Pat<'hir> { let field = self.single_pat_field(span, pat); self.pat_lang_item_variant(span, hir::LangItem::ControlFlowContinue, field) @@ -2624,10 +2618,6 @@ impl<'a, 'hir> LoweringContext<'a, 'hir> { ) } - fn pat_wild(&mut self, span: Span) -> &'hir hir::Pat<'hir> { - self.pat(span, hir::PatKind::Wild) - } - fn pat(&mut self, span: Span, kind: hir::PatKind<'hir>) -> &'hir hir::Pat<'hir> { self.arena.alloc(hir::Pat { hir_id: self.next_id(), diff --git a/compiler/rustc_ast_passes/src/ast_validation.rs b/compiler/rustc_ast_passes/src/ast_validation.rs index 0bdc4258bfb..cff1b70dda9 100644 --- a/compiler/rustc_ast_passes/src/ast_validation.rs +++ b/compiler/rustc_ast_passes/src/ast_validation.rs @@ -18,6 +18,7 @@ use rustc_parse::validate_attr; use rustc_session::lint::builtin::PATTERNS_IN_FNS_WITHOUT_BODY; use rustc_session::lint::{BuiltinLintDiagnostics, LintBuffer}; use rustc_session::Session; +use rustc_span::source_map::Spanned; use rustc_span::symbol::{kw, sym, Ident}; use rustc_span::Span; use std::mem; @@ -80,6 +81,9 @@ struct AstValidator<'a> { /// certain positions. is_assoc_ty_bound_banned: bool, + /// Used to allow `let` expressions in certain syntactic locations. + is_let_allowed: bool, + lint_buffer: &'a mut LintBuffer, } @@ -96,6 +100,27 @@ impl<'a> AstValidator<'a> { self.is_impl_trait_banned = old; } + fn with_let_allowed(&mut self, allowed: bool, f: impl FnOnce(&mut Self, bool)) { + let old = mem::replace(&mut self.is_let_allowed, allowed); + f(self, old); + self.is_let_allowed = old; + } + + /// Emits an error banning the `let` expression provided in the given location. + fn ban_let_expr(&self, expr: &'a Expr) { + let sess = &self.session; + if sess.opts.unstable_features.is_nightly_build() { + sess.struct_span_err(expr.span, "`let` expressions are not supported here") + .note("only supported directly in conditions of `if`- and `while`-expressions") + .note("as well as when nested within `&&` and parenthesis in those conditions") + .emit(); + } else { + sess.struct_span_err(expr.span, "expected expression, found statement (`let`)") + .note("variable declaration using `let` is a statement") + .emit(); + } + } + fn with_banned_assoc_ty_bound(&mut self, f: impl FnOnce(&mut Self)) { let old = mem::replace(&mut self.is_assoc_ty_bound_banned, true); f(self); @@ -978,20 +1003,49 @@ impl<'a> Visitor<'a> for AstValidator<'a> { } fn visit_expr(&mut self, expr: &'a Expr) { - match &expr.kind { - ExprKind::LlvmInlineAsm(..) if !self.session.target.allow_asm => { + self.with_let_allowed(false, |this, let_allowed| match &expr.kind { + ExprKind::If(cond, then, opt_else) => { + this.visit_block(then); + walk_list!(this, visit_expr, opt_else); + this.with_let_allowed(true, |this, _| this.visit_expr(cond)); + return; + } + ExprKind::Let(..) if !let_allowed => this.ban_let_expr(expr), + ExprKind::LlvmInlineAsm(..) if !this.session.target.allow_asm => { struct_span_err!( - self.session, + this.session, expr.span, E0472, "llvm_asm! is unsupported on this target" ) .emit(); } - _ => {} - } - - visit::walk_expr(self, expr); + ExprKind::Match(expr, arms) => { + this.visit_expr(expr); + for arm in arms { + this.visit_expr(&arm.body); + this.visit_pat(&arm.pat); + walk_list!(this, visit_attribute, &arm.attrs); + if let Some(ref guard) = arm.guard { + if let ExprKind::Let(_, ref expr, _) = guard.kind { + this.with_let_allowed(true, |this, _| this.visit_expr(expr)); + return; + } + } + } + } + ExprKind::Paren(_) | ExprKind::Binary(Spanned { node: BinOpKind::And, .. }, ..) => { + this.with_let_allowed(let_allowed, |this, _| visit::walk_expr(this, expr)); + return; + } + ExprKind::While(cond, then, opt_label) => { + walk_list!(this, visit_label, opt_label); + this.visit_block(then); + this.with_let_allowed(true, |this, _| this.visit_expr(cond)); + return; + } + _ => visit::walk_expr(this, expr), + }); } fn visit_ty(&mut self, ty: &'a Ty) { @@ -1634,6 +1688,7 @@ pub fn check_crate(session: &Session, krate: &Crate, lints: &mut LintBuffer) -> bound_context: None, is_impl_trait_banned: false, is_assoc_ty_bound_banned: false, + is_let_allowed: false, lint_buffer: lints, }; visit::walk_crate(&mut validator, krate); diff --git a/compiler/rustc_ast_pretty/src/pprust/state.rs b/compiler/rustc_ast_pretty/src/pprust/state.rs index 763c1b12bf8..b09c668273a 100644 --- a/compiler/rustc_ast_pretty/src/pprust/state.rs +++ b/compiler/rustc_ast_pretty/src/pprust/state.rs @@ -1587,19 +1587,14 @@ impl<'a> State<'a> { self.ann.post(self, AnnNode::Block(blk)) } - /// Print a `let pat = scrutinee` expression. - crate fn print_let(&mut self, pat: &ast::Pat, scrutinee: &ast::Expr) { + /// Print a `let pat = expr` expression. + crate fn print_let(&mut self, pat: &ast::Pat, expr: &ast::Expr) { self.s.word("let "); - self.print_pat(pat); self.s.space(); - self.word_space("="); - self.print_expr_cond_paren( - scrutinee, - Self::cond_needs_par(scrutinee) - || parser::needs_par_as_let_scrutinee(scrutinee.precedence().order()), - ) + let npals = || parser::needs_par_as_let_scrutinee(expr.precedence().order()); + self.print_expr_cond_paren(expr, Self::cond_needs_par(expr) || npals()) } fn print_else(&mut self, els: Option<&ast::Expr>) { @@ -1632,10 +1627,8 @@ impl<'a> State<'a> { crate fn print_if(&mut self, test: &ast::Expr, blk: &ast::Block, elseopt: Option<&ast::Expr>) { self.head("if"); - self.print_expr_as_cond(test); self.s.space(); - self.print_block(blk); self.print_else(elseopt) } @@ -1668,13 +1661,13 @@ impl<'a> State<'a> { self.print_expr_cond_paren(expr, Self::cond_needs_par(expr)) } - /// Does `expr` need parenthesis when printed in a condition position? + // Does `expr` need parenthesis when printed in a condition position? + // + // These cases need parens due to the parse error observed in #26461: `if return {}` + // parses as the erroneous construct `if (return {})`, not `if (return) {}`. fn cond_needs_par(expr: &ast::Expr) -> bool { match expr.kind { - // These cases need parens due to the parse error observed in #26461: `if return {}` - // parses as the erroneous construct `if (return {})`, not `if (return) {}`. - ast::ExprKind::Closure(..) | ast::ExprKind::Ret(..) | ast::ExprKind::Break(..) => true, - + ast::ExprKind::Break(..) | ast::ExprKind::Closure(..) | ast::ExprKind::Ret(..) => true, _ => parser::contains_exterior_struct_lit(expr), } } @@ -1919,7 +1912,7 @@ impl<'a> State<'a> { self.word_space(":"); self.print_type(ty); } - ast::ExprKind::Let(ref pat, ref scrutinee) => { + ast::ExprKind::Let(ref pat, ref scrutinee, _) => { self.print_let(pat, scrutinee); } ast::ExprKind::If(ref test, ref blk, ref elseopt) => { diff --git a/compiler/rustc_expand/src/base.rs b/compiler/rustc_expand/src/base.rs index b454737fb80..3ed99eab95c 100644 --- a/compiler/rustc_expand/src/base.rs +++ b/compiler/rustc_expand/src/base.rs @@ -354,6 +354,7 @@ pub trait MacResult { fn make_expr(self: Box<Self>) -> Option<P<ast::Expr>> { None } + /// Creates zero or more items. fn make_items(self: Box<Self>) -> Option<SmallVec<[P<ast::Item>; 1]>> { None diff --git a/compiler/rustc_hir/src/hir.rs b/compiler/rustc_hir/src/hir.rs index 3879d605292..4e233ed1457 100644 --- a/compiler/rustc_hir/src/hir.rs +++ b/compiler/rustc_hir/src/hir.rs @@ -1482,6 +1482,7 @@ impl Expr<'_> { ExprKind::Type(..) | ExprKind::Cast(..) => ExprPrecedence::Cast, ExprKind::DropTemps(ref expr, ..) => expr.precedence(), ExprKind::If(..) => ExprPrecedence::If, + ExprKind::Let(..) => ExprPrecedence::Let, ExprKind::Loop(..) => ExprPrecedence::Loop, ExprKind::Match(..) => ExprPrecedence::Match, ExprKind::Closure(..) => ExprPrecedence::Closure, @@ -1552,6 +1553,7 @@ impl Expr<'_> { | ExprKind::Break(..) | ExprKind::Continue(..) | ExprKind::Ret(..) + | ExprKind::Let(..) | ExprKind::Loop(..) | ExprKind::Assign(..) | ExprKind::InlineAsm(..) @@ -1634,6 +1636,7 @@ impl Expr<'_> { | ExprKind::Break(..) | ExprKind::Continue(..) | ExprKind::Ret(..) + | ExprKind::Let(..) | ExprKind::Loop(..) | ExprKind::Assign(..) | ExprKind::InlineAsm(..) @@ -1725,6 +1728,11 @@ pub enum ExprKind<'hir> { /// This construct only exists to tweak the drop order in HIR lowering. /// An example of that is the desugaring of `for` loops. DropTemps(&'hir Expr<'hir>), + /// A `let $pat = $expr` expression. + /// + /// These are not `Local` and only occur as expressions. + /// The `let Some(x) = foo()` in `if let Some(x) = foo()` is an example of `Let(..)`. + Let(&'hir Pat<'hir>, &'hir Expr<'hir>, Span), /// An `if` block, with an optional else block. /// /// I.e., `if <expr> { <expr> } else { <expr> }`. @@ -1884,15 +1892,6 @@ pub enum LocalSource { pub enum MatchSource { /// A `match _ { .. }`. Normal, - /// An `if let _ = _ { .. }` (optionally with `else { .. }`). - IfLetDesugar { contains_else_clause: bool }, - /// An `if let _ = _ => { .. }` match guard. - IfLetGuardDesugar, - /// A `while _ { .. }` (which was desugared to a `loop { match _ { .. } }`). - WhileDesugar, - /// A `while let _ = _ { .. }` (which was desugared to a - /// `loop { match _ { .. } }`). - WhileLetDesugar, /// A desugared `for _ in _ { .. }` loop. ForLoopDesugar, /// A desugared `?` operator. @@ -1902,12 +1901,11 @@ pub enum MatchSource { } impl MatchSource { - pub fn name(self) -> &'static str { + #[inline] + pub const fn name(self) -> &'static str { use MatchSource::*; match self { Normal => "match", - IfLetDesugar { .. } | IfLetGuardDesugar => "if", - WhileDesugar | WhileLetDesugar => "while", ForLoopDesugar => "for", TryDesugar => "?", AwaitDesugar => ".await", @@ -1922,8 +1920,6 @@ pub enum LoopSource { Loop, /// A `while _ { .. }` loop. While, - /// A `while let _ = _ { .. }` loop. - WhileLet, /// A `for _ in _ { .. }` loop. ForLoop, } @@ -1932,7 +1928,7 @@ impl LoopSource { pub fn name(self) -> &'static str { match self { LoopSource::Loop => "loop", - LoopSource::While | LoopSource::WhileLet => "while", + LoopSource::While => "while", LoopSource::ForLoop => "for", } } diff --git a/compiler/rustc_hir/src/intravisit.rs b/compiler/rustc_hir/src/intravisit.rs index ae186d66004..1cbe3b12c82 100644 --- a/compiler/rustc_hir/src/intravisit.rs +++ b/compiler/rustc_hir/src/intravisit.rs @@ -1163,6 +1163,10 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr<'v>) ExprKind::DropTemps(ref subexpression) => { visitor.visit_expr(subexpression); } + ExprKind::Let(ref pat, ref expr, _) => { + visitor.visit_expr(expr); + visitor.visit_pat(pat); + } ExprKind::If(ref cond, ref then, ref else_opt) => { visitor.visit_expr(cond); visitor.visit_expr(then); diff --git a/compiler/rustc_hir_pretty/src/lib.rs b/compiler/rustc_hir_pretty/src/lib.rs index 9ad40db73ce..9c286ad5ccf 100644 --- a/compiler/rustc_hir_pretty/src/lib.rs +++ b/compiler/rustc_hir_pretty/src/lib.rs @@ -1092,53 +1092,30 @@ impl<'a> State<'a> { } fn print_else(&mut self, els: Option<&hir::Expr<'_>>) { - match els { - Some(else_) => { - match else_.kind { - // "another else-if" - hir::ExprKind::If(ref i, ref then, ref e) => { - self.cbox(INDENT_UNIT - 1); - self.ibox(0); - self.s.word(" else if "); - self.print_expr_as_cond(&i); - self.s.space(); - self.print_expr(&then); - self.print_else(e.as_ref().map(|e| &**e)) - } - // "final else" - hir::ExprKind::Block(ref b, _) => { - self.cbox(INDENT_UNIT - 1); - self.ibox(0); - self.s.word(" else "); - self.print_block(&b) - } - hir::ExprKind::Match(ref expr, arms, _) => { - // else if let desugared to match - assert!(arms.len() == 2, "if let desugars to match with two arms"); - - self.s.word(" else "); - self.s.word("{"); - - self.cbox(INDENT_UNIT); - self.ibox(INDENT_UNIT); - self.word_nbsp("match"); - self.print_expr_as_cond(&expr); - self.s.space(); - self.bopen(); - for arm in arms { - self.print_arm(arm); - } - self.bclose(expr.span); - - self.s.word("}"); - } - // BLEAH, constraints would be great here - _ => { - panic!("print_if saw if with weird alternative"); - } + if let Some(els_inner) = els { + match els_inner.kind { + // Another `else if` block. + hir::ExprKind::If(ref i, ref then, ref e) => { + self.cbox(INDENT_UNIT - 1); + self.ibox(0); + self.s.word(" else if "); + self.print_expr_as_cond(&i); + self.s.space(); + self.print_expr(&then); + self.print_else(e.as_ref().map(|e| &**e)) + } + // Final `else` block. + hir::ExprKind::Block(ref b, _) => { + self.cbox(INDENT_UNIT - 1); + self.ibox(0); + self.s.word(" else "); + self.print_block(&b) + } + // Constraints would be great here! + _ => { + panic!("print_if saw if with weird alternative"); } } - _ => {} } } @@ -1165,37 +1142,52 @@ impl<'a> State<'a> { self.pclose() } - pub fn print_expr_maybe_paren(&mut self, expr: &hir::Expr<'_>, prec: i8) { - let needs_par = expr.precedence().order() < prec; - if needs_par { - self.popen(); - } - self.print_expr(expr); - if needs_par { - self.pclose(); - } + fn print_expr_maybe_paren(&mut self, expr: &hir::Expr<'_>, prec: i8) { + self.print_expr_cond_paren(expr, expr.precedence().order() < prec) } - /// Print an expr using syntax that's acceptable in a condition position, such as the `cond` in + /// Prints an expr using syntax that's acceptable in a condition position, such as the `cond` in /// `if cond { ... }`. pub fn print_expr_as_cond(&mut self, expr: &hir::Expr<'_>) { - let needs_par = match expr.kind { - // These cases need parens due to the parse error observed in #26461: `if return {}` - // parses as the erroneous construct `if (return {})`, not `if (return) {}`. - hir::ExprKind::Closure(..) | hir::ExprKind::Ret(..) | hir::ExprKind::Break(..) => true, - - _ => contains_exterior_struct_lit(expr), - }; + self.print_expr_cond_paren(expr, Self::cond_needs_par(expr)) + } + /// Prints `expr` or `(expr)` when `needs_par` holds. + fn print_expr_cond_paren(&mut self, expr: &hir::Expr<'_>, needs_par: bool) { if needs_par { self.popen(); } - self.print_expr(expr); + if let hir::ExprKind::DropTemps(ref actual_expr) = expr.kind { + self.print_expr(actual_expr); + } else { + self.print_expr(expr); + } if needs_par { self.pclose(); } } + /// Print a `let pat = expr` expression. + fn print_let(&mut self, pat: &hir::Pat<'_>, expr: &hir::Expr<'_>) { + self.s.word("let "); + self.print_pat(pat); + self.s.space(); + self.word_space("="); + let npals = || parser::needs_par_as_let_scrutinee(expr.precedence().order()); + self.print_expr_cond_paren(expr, Self::cond_needs_par(expr) || npals()) + } + + // Does `expr` need parenthesis when printed in a condition position? + // + // These cases need parens due to the parse error observed in #26461: `if return {}` + // parses as the erroneous construct `if (return {})`, not `if (return) {}`. + fn cond_needs_par(expr: &hir::Expr<'_>) -> bool { + match expr.kind { + hir::ExprKind::Break(..) | hir::ExprKind::Closure(..) | hir::ExprKind::Ret(..) => true, + _ => contains_exterior_struct_lit(expr), + } + } + fn print_expr_vec(&mut self, exprs: &[hir::Expr<'_>]) { self.ibox(INDENT_UNIT); self.s.word("["); @@ -1314,6 +1306,9 @@ impl<'a> State<'a> { (&hir::ExprKind::Cast { .. }, hir::BinOpKind::Lt | hir::BinOpKind::Shl) => { parser::PREC_FORCE_PAREN } + (&hir::ExprKind::Let { .. }, _) if !parser::needs_par_as_let_scrutinee(prec) => { + parser::PREC_FORCE_PAREN + } _ => left_prec, }; @@ -1531,6 +1526,9 @@ impl<'a> State<'a> { // Print `}`: self.bclose_maybe_open(expr.span, true); } + hir::ExprKind::Let(ref pat, ref scrutinee, _) => { + self.print_let(pat, scrutinee); + } hir::ExprKind::If(ref test, ref blk, ref elseopt) => { self.print_if(&test, &blk, elseopt.as_ref().map(|e| &**e)); } diff --git a/compiler/rustc_infer/src/infer/error_reporting/mod.rs b/compiler/rustc_infer/src/infer/error_reporting/mod.rs index 50048534aae..b628381d5aa 100644 --- a/compiler/rustc_infer/src/infer/error_reporting/mod.rs +++ b/compiler/rustc_infer/src/infer/error_reporting/mod.rs @@ -644,17 +644,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { scrut_span, .. }) => match source { - hir::MatchSource::IfLetDesugar { .. } => { - let msg = "`if let` arms have incompatible types"; - err.span_label(cause.span, msg); - if let Some(ret_sp) = opt_suggest_box_span { - self.suggest_boxing_for_return_impl_trait( - err, - ret_sp, - prior_arms.iter().chain(std::iter::once(&arm_span)).map(|s| *s), - ); - } - } hir::MatchSource::TryDesugar => { if let Some(ty::error::ExpectedFound { expected, .. }) = exp_found { let scrut_expr = self.tcx.hir().expect_expr(scrut_hir_id); @@ -2581,9 +2570,6 @@ impl<'tcx> ObligationCauseExt<'tcx> for ObligationCause<'tcx> { CompareImplTypeObligation { .. } => Error0308("type not compatible with trait"), MatchExpressionArm(box MatchExpressionArmCause { source, .. }) => { Error0308(match source { - hir::MatchSource::IfLetDesugar { .. } => { - "`if let` arms have incompatible types" - } hir::MatchSource::TryDesugar => { "try expression alternatives have incompatible types" } @@ -2619,10 +2605,6 @@ impl<'tcx> ObligationCauseExt<'tcx> for ObligationCause<'tcx> { CompareImplMethodObligation { .. } => "method type is compatible with trait", CompareImplTypeObligation { .. } => "associated type is compatible with trait", ExprAssignable => "expression is assignable", - MatchExpressionArm(box MatchExpressionArmCause { source, .. }) => match source { - hir::MatchSource::IfLetDesugar { .. } => "`if let` arms have compatible types", - _ => "`match` arms have compatible types", - }, IfExpression { .. } => "`if` and `else` have incompatible types", IfExpressionWithNoElse => "`if` missing an `else` returns `()`", MainFunctionType => "`main` function has the correct type", diff --git a/compiler/rustc_lint/src/unused.rs b/compiler/rustc_lint/src/unused.rs index 2d2897f3f98..f59ad0ac82d 100644 --- a/compiler/rustc_lint/src/unused.rs +++ b/compiler/rustc_lint/src/unused.rs @@ -614,7 +614,8 @@ trait UnusedDelimLint { let (value, ctx, followed_by_block, left_pos, right_pos) = match e.kind { // Do not lint `unused_braces` in `if let` expressions. If(ref cond, ref block, _) - if !matches!(cond.kind, Let(_, _)) || Self::LINT_EXPR_IN_PATTERN_MATCHING_CTX => + if !matches!(cond.kind, Let(_, _, _)) + || Self::LINT_EXPR_IN_PATTERN_MATCHING_CTX => { let left = e.span.lo() + rustc_span::BytePos(2); let right = block.span.lo(); @@ -623,7 +624,8 @@ trait UnusedDelimLint { // Do not lint `unused_braces` in `while let` expressions. While(ref cond, ref block, ..) - if !matches!(cond.kind, Let(_, _)) || Self::LINT_EXPR_IN_PATTERN_MATCHING_CTX => + if !matches!(cond.kind, Let(_, _, _)) + || Self::LINT_EXPR_IN_PATTERN_MATCHING_CTX => { let left = e.span.lo() + rustc_span::BytePos(5); let right = block.span.lo(); @@ -774,7 +776,7 @@ impl UnusedDelimLint for UnusedParens { self.emit_unused_delims_expr(cx, value, ctx, left_pos, right_pos) } } - ast::ExprKind::Let(_, ref expr) => { + ast::ExprKind::Let(_, ref expr, _) => { self.check_unused_delims_expr( cx, expr, @@ -828,7 +830,7 @@ impl UnusedParens { impl EarlyLintPass for UnusedParens { fn check_expr(&mut self, cx: &EarlyContext<'_>, e: &ast::Expr) { match e.kind { - ExprKind::Let(ref pat, _) | ExprKind::ForLoop(ref pat, ..) => { + ExprKind::Let(ref pat, _, _) | ExprKind::ForLoop(ref pat, ..) => { self.check_unused_parens_pat(cx, pat, false, false); } // We ignore parens in cases like `if (((let Some(0) = Some(1))))` because we already @@ -1012,7 +1014,7 @@ impl UnusedDelimLint for UnusedBraces { } } } - ast::ExprKind::Let(_, ref expr) => { + ast::ExprKind::Let(_, ref expr, _) => { self.check_unused_delims_expr( cx, expr, diff --git a/compiler/rustc_middle/src/thir.rs b/compiler/rustc_middle/src/thir.rs index cdefc9effa1..48337b8cb96 100644 --- a/compiler/rustc_middle/src/thir.rs +++ b/compiler/rustc_middle/src/thir.rs @@ -292,6 +292,10 @@ pub enum ExprKind<'tcx> { Loop { body: ExprId, }, + Let { + expr: ExprId, + pat: Pat<'tcx>, + }, /// A `match` expression. Match { scrutinee: ExprId, diff --git a/compiler/rustc_mir_build/src/build/expr/as_place.rs b/compiler/rustc_mir_build/src/build/expr/as_place.rs index bedb8b1c58b..4a546a50215 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_place.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_place.rs @@ -565,6 +565,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { | ExprKind::If { .. } | ExprKind::Loop { .. } | ExprKind::Block { .. } + | ExprKind::Let { .. } | ExprKind::Assign { .. } | ExprKind::AssignOp { .. } | ExprKind::Break { .. } diff --git a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs index 69786c14ee8..1719f96f26d 100644 --- a/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs +++ b/compiler/rustc_mir_build/src/build/expr/as_rvalue.rs @@ -284,6 +284,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { | ExprKind::LogicalOp { .. } | ExprKind::Call { .. } | ExprKind::Field { .. } + | ExprKind::Let { .. } | ExprKind::Deref { .. } | ExprKind::Index { .. } | ExprKind::VarRef { .. } diff --git a/compiler/rustc_mir_build/src/build/expr/category.rs b/compiler/rustc_mir_build/src/build/expr/category.rs index c834ce6ce68..fcda52e5581 100644 --- a/compiler/rustc_mir_build/src/build/expr/category.rs +++ b/compiler/rustc_mir_build/src/build/expr/category.rs @@ -46,6 +46,7 @@ impl Category { ExprKind::LogicalOp { .. } | ExprKind::Match { .. } | ExprKind::If { .. } + | ExprKind::Let { .. } | ExprKind::NeverToAny { .. } | ExprKind::Use { .. } | ExprKind::Adt { .. } diff --git a/compiler/rustc_mir_build/src/build/expr/into.rs b/compiler/rustc_mir_build/src/build/expr/into.rs index d7b3a85c15d..3878cf4db99 100644 --- a/compiler/rustc_mir_build/src/build/expr/into.rs +++ b/compiler/rustc_mir_build/src/build/expr/into.rs @@ -53,48 +53,66 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { this.match_expr(destination, expr_span, block, &this.thir[scrutinee], arms) } ExprKind::If { cond, then, else_opt } => { - let place = unpack!( - block = this.as_temp( - block, - Some(this.local_scope()), - &this.thir[cond], - Mutability::Mut - ) - ); - let operand = Operand::Move(Place::from(place)); - - let mut then_block = this.cfg.start_new_block(); - let mut else_block = this.cfg.start_new_block(); - let term = TerminatorKind::if_(this.tcx, operand, then_block, else_block); - this.cfg.terminate(block, source_info, term); - - unpack!( - then_block = this.expr_into_dest(destination, then_block, &this.thir[then]) - ); - else_block = if let Some(else_opt) = else_opt { - unpack!(this.expr_into_dest(destination, else_block, &this.thir[else_opt])) + let local_scope = this.local_scope(); + let (mut then_blk, mut else_blk) = + this.then_else_blocks(block, &this.thir[cond], local_scope, source_info); + unpack!(then_blk = this.expr_into_dest(destination, then_blk, &this.thir[then])); + else_blk = if let Some(else_opt) = else_opt { + unpack!(this.expr_into_dest(destination, else_blk, &this.thir[else_opt])) } else { // Body of the `if` expression without an `else` clause must return `()`, thus // we implicitly generate a `else {}` if it is not specified. let correct_si = this.source_info(expr_span.shrink_to_hi()); - this.cfg.push_assign_unit(else_block, correct_si, destination, this.tcx); - else_block + this.cfg.push_assign_unit(else_blk, correct_si, destination, this.tcx); + else_blk }; let join_block = this.cfg.start_new_block(); this.cfg.terminate( - then_block, + then_blk, source_info, TerminatorKind::Goto { target: join_block }, ); this.cfg.terminate( - else_block, + else_blk, source_info, TerminatorKind::Goto { target: join_block }, ); join_block.unit() } + ExprKind::Let { ref pat, expr } => { + let (true_block, false_block) = + this.lower_let(block, &this.thir[expr], pat, expr_span); + + let join_block = this.cfg.start_new_block(); + + this.cfg.push_assign_constant( + true_block, + source_info, + destination, + Constant { + span: expr_span, + user_ty: None, + literal: ty::Const::from_bool(this.tcx, true).into(), + }, + ); + + this.cfg.push_assign_constant( + false_block, + source_info, + destination, + Constant { + span: expr_span, + user_ty: None, + literal: ty::Const::from_bool(this.tcx, false).into(), + }, + ); + + this.cfg.goto(true_block, source_info, join_block); + this.cfg.goto(false_block, source_info, join_block); + join_block.unit() + } ExprKind::NeverToAny { source } => { let source = &this.thir[source]; let is_call = diff --git a/compiler/rustc_mir_build/src/build/matches/mod.rs b/compiler/rustc_mir_build/src/build/matches/mod.rs index 9ca4425a1b5..f2a8109eb05 100644 --- a/compiler/rustc_mir_build/src/build/matches/mod.rs +++ b/compiler/rustc_mir_build/src/build/matches/mod.rs @@ -38,11 +38,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { pub(crate) fn then_else_blocks( &mut self, mut block: BasicBlock, - expr: ExprRef<'tcx>, + expr: &Expr<'tcx>, + scope: region::Scope, source_info: SourceInfo, ) -> (BasicBlock, BasicBlock) { let this = self; - let expr = this.hir.mirror(expr); let expr_span = expr.span; match expr.kind { @@ -52,24 +52,23 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { let else_block = unpack!( then_block = this.in_scope(region_scope, lint_level, |this| { let (then_block, else_block) = - this.then_else_blocks(block, value, source_info); + this.then_else_blocks(block, &this.thir[value], scope, source_info); then_block.and(else_block) }) ); (then_block, else_block) } - ExprKind::Let { expr, pat } => { - // TODO: Use correct span. - this.lower_let(block, &expr, &pat, expr_span) + ExprKind::Let { expr, ref pat } => { + // FIXME: Use correct span. + this.lower_let(block, &this.thir[expr], pat, expr_span) } _ => { - let local_scope = Some(this.local_scope()); - let place = - unpack!(block = this.as_temp(block, local_scope, expr, Mutability::Mut)); + let mutability = Mutability::Mut; + let place = unpack!(block = this.as_temp(block, Some(scope), expr, mutability)); let operand = Operand::Move(Place::from(place)); let then_block = this.cfg.start_new_block(); let else_block = this.cfg.start_new_block(); - let term = TerminatorKind::if_(this.hir.tcx(), operand, then_block, else_block); + let term = TerminatorKind::if_(this.tcx, operand, then_block, else_block); this.cfg.terminate(block, source_info, term); (then_block, else_block) } @@ -1699,6 +1698,46 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { // Pat binding - used for `let` and function parameters as well. impl<'a, 'tcx> Builder<'a, 'tcx> { + pub fn lower_let( + &mut self, + mut block: BasicBlock, + expr: &Expr<'tcx>, + pat: &Pat<'tcx>, + span: Span, + ) -> (BasicBlock, BasicBlock) { + let expr_span = expr.span; + let expr_place_builder = unpack!(block = self.lower_scrutinee(block, expr, expr_span)); + let mut guard_candidate = Candidate::new(expr_place_builder.clone(), &pat, false); + let wildcard = Pat::wildcard_from_ty(pat.ty); + let mut otherwise_candidate = Candidate::new(expr_place_builder.clone(), &wildcard, false); + let fake_borrow_temps = self.lower_match_tree( + block, + pat.span, + false, + &mut [&mut guard_candidate, &mut otherwise_candidate], + ); + let mut opt_expr_place: Option<(Option<&Place<'tcx>>, Span)> = None; + let expr_place: Place<'tcx>; + if let Ok(expr_builder) = + expr_place_builder.try_upvars_resolved(self.tcx, self.typeck_results) + { + expr_place = expr_builder.into_place(self.tcx, self.typeck_results); + opt_expr_place = Some((Some(&expr_place), expr_span)); + } + self.declare_bindings(None, pat.span.to(span), pat, ArmHasGuard(false), opt_expr_place); + let post_guard_block = self.bind_pattern( + self.source_info(pat.span), + guard_candidate, + None, + &fake_borrow_temps, + expr.span, + None, + None, + ); + let otherwise_post_guard_block = otherwise_candidate.pre_binding_block.unwrap(); + (post_guard_block, otherwise_post_guard_block) + } + /// Initializes each of the bindings from the candidate by /// moving/copying/ref'ing the source as appropriate. Tests the guard, if /// any, and then branches to the arm. Returns the block for the case where @@ -1852,48 +1891,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> { (e.span, self.test_bool(block, e, source_info)) } Guard::IfLet(ref pat, scrutinee) => { - let scrutinee = &self.thir[scrutinee]; - let scrutinee_span = scrutinee.span; - let scrutinee_place_builder = - unpack!(block = self.lower_scrutinee(block, scrutinee, scrutinee_span)); - let mut guard_candidate = - Candidate::new(scrutinee_place_builder.clone(), &pat, false); - let wildcard = Pat::wildcard_from_ty(pat.ty); - let mut otherwise_candidate = - Candidate::new(scrutinee_place_builder.clone(), &wildcard, false); - let fake_borrow_temps = self.lower_match_tree( - block, - pat.span, - false, - &mut [&mut guard_candidate, &mut otherwise_candidate], - ); - let mut opt_scrutinee_place: Option<(Option<&Place<'tcx>>, Span)> = None; - let scrutinee_place: Place<'tcx>; - if let Ok(scrutinee_builder) = - scrutinee_place_builder.try_upvars_resolved(self.tcx, self.typeck_results) - { - scrutinee_place = - scrutinee_builder.into_place(self.tcx, self.typeck_results); - opt_scrutinee_place = Some((Some(&scrutinee_place), scrutinee_span)); - } - self.declare_bindings( - None, - pat.span.to(arm_span.unwrap()), - pat, - ArmHasGuard(false), - opt_scrutinee_place, - ); - let post_guard_block = self.bind_pattern( - self.source_info(pat.span), - guard_candidate, - None, - &fake_borrow_temps, - scrutinee_span, - None, - None, - ); - let otherwise_post_guard_block = otherwise_candidate.pre_binding_block.unwrap(); - (scrutinee_span, (post_guard_block, otherwise_post_guard_block)) + let s = &self.thir[scrutinee]; + (s.span, self.lower_let(block, s, pat, arm_span.unwrap())) } }; let source_info = self.source_info(guard_span); diff --git a/compiler/rustc_mir_build/src/check_unsafety.rs b/compiler/rustc_mir_build/src/check_unsafety.rs index 42e4fc3839e..05a5fcef16a 100644 --- a/compiler/rustc_mir_build/src/check_unsafety.rs +++ b/compiler/rustc_mir_build/src/check_unsafety.rs @@ -325,6 +325,7 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> { | ExprKind::Return { .. } | ExprKind::Yield { .. } | ExprKind::Loop { .. } + | ExprKind::Let { .. } | ExprKind::Match { .. } | ExprKind::Box { .. } | ExprKind::If { .. } @@ -475,6 +476,14 @@ impl<'a, 'tcx> Visitor<'a, 'tcx> for UnsafetyVisitor<'a, 'tcx> { } } } + ExprKind::Let { expr: expr_id, .. } => { + let let_expr = &self.thir[expr_id]; + if let ty::Adt(adt_def, _) = let_expr.ty.kind() { + if adt_def.is_union() { + self.requires_unsafe(expr.span, AccessToUnionField); + } + } + } _ => {} } visit::walk_expr(self, expr); diff --git a/compiler/rustc_mir_build/src/thir/cx/expr.rs b/compiler/rustc_mir_build/src/thir/cx/expr.rs index c3908ddd4fb..2b0aa41a4bd 100644 --- a/compiler/rustc_mir_build/src/thir/cx/expr.rs +++ b/compiler/rustc_mir_build/src/thir/cx/expr.rs @@ -590,6 +590,9 @@ impl<'tcx> Cx<'tcx> { }, Err(err) => bug!("invalid loop id for continue: {}", err), }, + hir::ExprKind::Let(ref pat, ref expr, _) => { + ExprKind::Let { expr: self.mirror_expr(expr), pat: self.pattern_from_hir(pat) } + } hir::ExprKind::If(cond, then, else_opt) => ExprKind::If { cond: self.mirror_expr(cond), then: self.mirror_expr(then), diff --git a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs index 57ea6c86e7a..50cbe0f71f5 100644 --- a/compiler/rustc_mir_build/src/thir/pattern/check_match.rs +++ b/compiler/rustc_mir_build/src/thir/pattern/check_match.rs @@ -55,9 +55,10 @@ impl<'tcx> Visitor<'tcx> for MatchVisitor<'_, 'tcx> { fn visit_expr(&mut self, ex: &'tcx hir::Expr<'tcx>) { intravisit::walk_expr(self, ex); - - if let hir::ExprKind::Match(ref scrut, ref arms, source) = ex.kind { - self.check_match(scrut, arms, source); + match &ex.kind { + hir::ExprKind::Match(scrut, arms, source) => self.check_match(scrut, arms, *source), + hir::ExprKind::Let(pat, scrut, span) => self.check_let(pat, scrut, *span), + _ => {} } } @@ -117,6 +118,31 @@ impl<'tcx> MatchVisitor<'_, 'tcx> { check_for_bindings_named_same_as_variants(self, pat); } + fn let_source(&mut self, pat: &'tcx hir::Pat<'tcx>, _expr: &hir::Expr<'_>) -> LetSource { + let hir = self.tcx.hir(); + let parent = hir.get_parent_node(pat.hir_id); + let parent_parent = hir.get_parent_node(parent); + let parent_parent_node = hir.get(parent_parent); + + let parent_parent_parent = hir.get_parent_node(parent_parent); + let parent_parent_parent_parent = hir.get_parent_node(parent_parent_parent); + let parent_parent_parent_parent_node = hir.get(parent_parent_parent_parent); + + if let hir::Node::Expr(hir::Expr { + kind: hir::ExprKind::Loop(_, _, hir::LoopSource::While, _), + .. + }) = parent_parent_parent_parent_node + { + LetSource::WhileLet + } else if let hir::Node::Expr(hir::Expr { kind: hir::ExprKind::If { .. }, .. }) = + parent_parent_node + { + LetSource::IfLet + } else { + LetSource::GenericLet + } + } + fn lower_pattern<'p>( &self, cx: &mut MatchCheckCtxt<'p, 'tcx>, @@ -144,6 +170,14 @@ impl<'tcx> MatchVisitor<'_, 'tcx> { } } + fn check_let(&mut self, pat: &'tcx hir::Pat<'tcx>, expr: &hir::Expr<'_>, span: Span) { + self.check_patterns(pat); + let ls = self.let_source(pat, expr); + let mut cx = self.new_cx(expr.hir_id); + let tpat = self.lower_pattern(&mut cx, pat, &mut false).0; + check_let_reachability(&mut cx, ls, pat.hir_id, &tpat, span); + } + fn check_match( &mut self, scrut: &hir::Expr<'_>, @@ -158,7 +192,13 @@ impl<'tcx> MatchVisitor<'_, 'tcx> { if let Some(hir::Guard::IfLet(ref pat, _)) = arm.guard { self.check_patterns(pat); let tpat = self.lower_pattern(&mut cx, pat, &mut false).0; - check_if_let_guard(&mut cx, &tpat, pat.hir_id); + check_let_reachability( + &mut cx, + LetSource::IfLetGuard, + pat.hir_id, + &tpat, + tpat.span, + ); } } @@ -181,8 +221,16 @@ impl<'tcx> MatchVisitor<'_, 'tcx> { let scrut_ty = self.typeck_results.expr_ty_adjusted(scrut); let report = compute_match_usefulness(&cx, &arms, scrut.hir_id, scrut_ty); - // Report unreachable arms. - report_arm_reachability(&cx, &report, source); + report_arm_reachability(&cx, &report, |_, arm_span, arm_hir_id, catchall| { + match source { + hir::MatchSource::ForLoopDesugar | hir::MatchSource::Normal => { + unreachable_pattern(cx.tcx, arm_span, arm_hir_id, catchall); + } + // Unreachable patterns in try and await expressions occur when one of + // the arms are an uninhabited type. Which is OK. + hir::MatchSource::AwaitDesugar | hir::MatchSource::TryDesugar => {} + } + }); // Check if the match is exhaustive. // Note: An empty match isn't the same as an empty matrix for diagnostics purposes, @@ -349,89 +397,99 @@ fn unreachable_pattern(tcx: TyCtxt<'_>, span: Span, id: HirId, catchall: Option< }); } -fn irrefutable_let_pattern(tcx: TyCtxt<'_>, span: Span, id: HirId, source: hir::MatchSource) { - tcx.struct_span_lint_hir(IRREFUTABLE_LET_PATTERNS, id, span, |lint| match source { - hir::MatchSource::IfLetDesugar { .. } => { - let mut diag = lint.build("irrefutable `if let` pattern"); - diag.note("this pattern will always match, so the `if let` is useless"); - diag.help("consider replacing the `if let` with a `let`"); +fn irrefutable_let_pattern(id: HirId, ls: LetSource, span: Span, tcx: TyCtxt<'_>) { + macro_rules! emit_diag { + ( + $lint:expr, + $source_name:expr, + $note_sufix:expr, + $help_sufix:expr + ) => {{ + let mut diag = $lint.build(concat!("irrefutable ", $source_name, " pattern")); + diag.note(concat!("this pattern will always match, so the ", $note_sufix)); + diag.help(concat!("consider ", $help_sufix)); diag.emit() + }}; + } + + tcx.struct_span_lint_hir(IRREFUTABLE_LET_PATTERNS, id, span, |lint| match ls { + LetSource::GenericLet => { + emit_diag!(lint, "`let`", "`let` is useless", "removing `let`"); } - hir::MatchSource::WhileLetDesugar => { - let mut diag = lint.build("irrefutable `while let` pattern"); - diag.note("this pattern will always match, so the loop will never exit"); - diag.help("consider instead using a `loop { ... }` with a `let` inside it"); - diag.emit() + LetSource::IfLet => { + emit_diag!( + lint, + "`if let`", + "`if let` is useless", + "replacing the `if let` with a `let`" + ); } - hir::MatchSource::IfLetGuardDesugar => { - let mut diag = lint.build("irrefutable `if let` guard pattern"); - diag.note("this pattern will always match, so the guard is useless"); - diag.help("consider removing the guard and adding a `let` inside the match arm"); - diag.emit() + LetSource::IfLetGuard => { + emit_diag!( + lint, + "`if let` guard", + "guard is useless", + "removing the guard and adding a `let` inside the match arm" + ); } - _ => { - bug!( - "expected `if let`, `while let`, or `if let` guard HIR match source, found {:?}", - source, - ) + LetSource::WhileLet => { + emit_diag!( + lint, + "`while let`", + "loop will never exit", + "instead using a `loop { ... }` with a `let` inside it" + ); } }); } -fn check_if_let_guard<'p, 'tcx>( +fn check_let_reachability<'p, 'tcx>( cx: &mut MatchCheckCtxt<'p, 'tcx>, - pat: &'p super::Pat<'tcx>, + ls: LetSource, pat_id: HirId, + pat: &'p super::Pat<'tcx>, + span: Span, ) { let arms = [MatchArm { pat, hir_id: pat_id, has_guard: false }]; let report = compute_match_usefulness(&cx, &arms, pat_id, pat.ty); - report_arm_reachability(&cx, &report, hir::MatchSource::IfLetGuardDesugar); + + report_arm_reachability(&cx, &report, |arm_index, arm_span, arm_hir_id, _| { + match ls { + LetSource::IfLet | LetSource::WhileLet => { + match arm_index { + // The arm with the user-specified pattern. + 0 => unreachable_pattern(cx.tcx, arm_span, arm_hir_id, None), + // The arm with the wildcard pattern. + 1 => irrefutable_let_pattern(pat_id, ls, arm_span, cx.tcx), + _ => bug!(), + } + } + LetSource::IfLetGuard if arm_index == 0 => { + unreachable_pattern(cx.tcx, arm_span, arm_hir_id, None); + } + _ => {} + } + }); if report.non_exhaustiveness_witnesses.is_empty() { // The match is exhaustive, i.e. the `if let` pattern is irrefutable. - irrefutable_let_pattern(cx.tcx, pat.span, pat_id, hir::MatchSource::IfLetGuardDesugar) + irrefutable_let_pattern(pat_id, ls, span, cx.tcx); } } /// Report unreachable arms, if any. -fn report_arm_reachability<'p, 'tcx>( +fn report_arm_reachability<'p, 'tcx, F>( cx: &MatchCheckCtxt<'p, 'tcx>, report: &UsefulnessReport<'p, 'tcx>, - source: hir::MatchSource, -) { + unreachable: F, +) where + F: Fn(usize, Span, HirId, Option<Span>), +{ use Reachability::*; let mut catchall = None; for (arm_index, (arm, is_useful)) in report.arm_usefulness.iter().enumerate() { match is_useful { - Unreachable => { - match source { - hir::MatchSource::WhileDesugar => bug!(), - - hir::MatchSource::IfLetDesugar { .. } | hir::MatchSource::WhileLetDesugar => { - // Check which arm we're on. - match arm_index { - // The arm with the user-specified pattern. - 0 => unreachable_pattern(cx.tcx, arm.pat.span, arm.hir_id, None), - // The arm with the wildcard pattern. - 1 => irrefutable_let_pattern(cx.tcx, arm.pat.span, arm.hir_id, source), - _ => bug!(), - } - } - - hir::MatchSource::IfLetGuardDesugar => { - assert_eq!(arm_index, 0); - unreachable_pattern(cx.tcx, arm.pat.span, arm.hir_id, None); - } - - hir::MatchSource::ForLoopDesugar | hir::MatchSource::Normal => { - unreachable_pattern(cx.tcx, arm.pat.span, arm.hir_id, catchall); - } - - // Unreachable patterns in try and await expressions occur when one of - // the arms are an uninhabited type. Which is OK. - hir::MatchSource::AwaitDesugar | hir::MatchSource::TryDesugar => {} - } - } + Unreachable => unreachable(arm_index, arm.pat.span, arm.hir_id, catchall), Reachable(unreachables) if unreachables.is_empty() => {} // The arm is reachable, but contains unreachable subpatterns (from or-patterns). Reachable(unreachables) => { @@ -723,3 +781,11 @@ fn check_borrow_conflicts_in_at_patterns(cx: &MatchVisitor<'_, '_>, pat: &Pat<'_ err.emit(); } } + +#[derive(Clone, Copy, Debug)] +pub enum LetSource { + GenericLet, + IfLet, + IfLetGuard, + WhileLet, +} diff --git a/compiler/rustc_mir_build/src/thir/visit.rs b/compiler/rustc_mir_build/src/thir/visit.rs index ce5d4362c08..dda6c74d4de 100644 --- a/compiler/rustc_mir_build/src/thir/visit.rs +++ b/compiler/rustc_mir_build/src/thir/visit.rs @@ -57,6 +57,9 @@ pub fn walk_expr<'a, 'tcx: 'a, V: Visitor<'a, 'tcx>>(visitor: &mut V, expr: &Exp Use { source } => visitor.visit_expr(&visitor.thir()[source]), NeverToAny { source } => visitor.visit_expr(&visitor.thir()[source]), Pointer { source, cast: _ } => visitor.visit_expr(&visitor.thir()[source]), + Let { expr, .. } => { + visitor.visit_expr(&visitor.thir()[expr]); + } Loop { body } => visitor.visit_expr(&visitor.thir()[body]), Match { scrutinee, ref arms } => { visitor.visit_expr(&visitor.thir()[scrutinee]); diff --git a/compiler/rustc_parse/src/parser/expr.rs b/compiler/rustc_parse/src/parser/expr.rs index 824f2316e58..6259eff5a1a 100644 --- a/compiler/rustc_parse/src/parser/expr.rs +++ b/compiler/rustc_parse/src/parser/expr.rs @@ -1867,7 +1867,7 @@ impl<'a> Parser<'a> { })?; let span = lo.to(expr.span); self.sess.gated_spans.gate(sym::let_chains, span); - Ok(self.mk_expr(span, ExprKind::Let(pat, expr), attrs)) + Ok(self.mk_expr(span, ExprKind::Let(pat, expr, span), attrs)) } /// Parses an `else { ... }` expression (`else` token already eaten). diff --git a/compiler/rustc_passes/src/check_const.rs b/compiler/rustc_passes/src/check_const.rs index f6a93f5e02d..4a82252a32b 100644 --- a/compiler/rustc_passes/src/check_const.rs +++ b/compiler/rustc_passes/src/check_const.rs @@ -48,11 +48,8 @@ impl NonConstExpr { Self::Match(TryDesugar) => &[sym::const_try], - Self::Match(IfLetGuardDesugar) => bug!("`if let` guard outside a `match` expression"), - // All other expressions are allowed. - Self::Loop(Loop | While | WhileLet) - | Self::Match(WhileDesugar | WhileLetDesugar | Normal | IfLetDesugar { .. }) => &[], + Self::Loop(Loop | While) | Self::Match(Normal) => &[], }; Some(gates) @@ -277,9 +274,7 @@ impl<'tcx> Visitor<'tcx> for CheckConstVisitor<'tcx> { hir::ExprKind::Match(_, _, source) => { let non_const_expr = match source { // These are handled by `ExprKind::Loop` above. - hir::MatchSource::WhileDesugar - | hir::MatchSource::WhileLetDesugar - | hir::MatchSource::ForLoopDesugar => None, + hir::MatchSource::ForLoopDesugar => None, _ => Some(NonConstExpr::Match(*source)), }; diff --git a/compiler/rustc_passes/src/liveness.rs b/compiler/rustc_passes/src/liveness.rs index f2c2521ab43..2cd780e1b9b 100644 --- a/compiler/rustc_passes/src/liveness.rs +++ b/compiler/rustc_passes/src/liveness.rs @@ -429,6 +429,11 @@ impl<'tcx> Visitor<'tcx> for IrMaps<'tcx> { intravisit::walk_expr(self, expr); } + hir::ExprKind::Let(ref pat, ..) => { + self.add_from_pat(pat); + intravisit::walk_expr(self, expr); + } + // live nodes required for interesting control flow: hir::ExprKind::If(..) | hir::ExprKind::Match(..) | hir::ExprKind::Loop(..) => { self.add_live_node_for_node(expr.hir_id, ExprNode(expr.span)); @@ -852,6 +857,11 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { }) } + hir::ExprKind::Let(ref pat, ref scrutinee, _) => { + let succ = self.propagate_through_expr(scrutinee, succ); + self.define_bindings_in_pat(pat, succ) + } + // Note that labels have been resolved, so we don't need to look // at the label ident hir::ExprKind::Loop(ref blk, ..) => self.propagate_through_loop(expr, &blk, succ), @@ -1303,6 +1313,7 @@ impl<'a, 'tcx> Visitor<'tcx> for Liveness<'a, 'tcx> { fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) { check_expr(self, ex); + intravisit::walk_expr(self, ex); } fn visit_arm(&mut self, arm: &'tcx hir::Arm<'tcx>) { @@ -1358,6 +1369,10 @@ fn check_expr<'tcx>(this: &mut Liveness<'_, 'tcx>, expr: &'tcx Expr<'tcx>) { } } + hir::ExprKind::Let(ref pat, ..) => { + this.check_unused_vars_in_pat(pat, None, |_, _, _, _| {}); + } + // no correctness conditions related to liveness hir::ExprKind::Call(..) | hir::ExprKind::MethodCall(..) @@ -1388,8 +1403,6 @@ fn check_expr<'tcx>(this: &mut Liveness<'_, 'tcx>, expr: &'tcx Expr<'tcx>) { | hir::ExprKind::Type(..) | hir::ExprKind::Err => {} } - - intravisit::walk_expr(this, expr); } impl<'tcx> Liveness<'_, 'tcx> { diff --git a/compiler/rustc_passes/src/naked_functions.rs b/compiler/rustc_passes/src/naked_functions.rs index 899503cc556..d3ecd18a93c 100644 --- a/compiler/rustc_passes/src/naked_functions.rs +++ b/compiler/rustc_passes/src/naked_functions.rs @@ -221,6 +221,7 @@ impl<'tcx> CheckInlineAssembly<'tcx> { | ExprKind::Index(..) | ExprKind::Path(..) | ExprKind::AddrOf(..) + | ExprKind::Let(..) | ExprKind::Break(..) | ExprKind::Continue(..) | ExprKind::Ret(..) diff --git a/compiler/rustc_resolve/src/late.rs b/compiler/rustc_resolve/src/late.rs index a21d8197bdb..00d291946df 100644 --- a/compiler/rustc_resolve/src/late.rs +++ b/compiler/rustc_resolve/src/late.rs @@ -2309,7 +2309,7 @@ impl<'a: 'ast, 'b, 'ast> LateResolutionVisitor<'a, 'b, 'ast> { self.resolve_expr(e, Some(&expr)); } - ExprKind::Let(ref pat, ref scrutinee) => { + ExprKind::Let(ref pat, ref scrutinee, _) => { self.visit_expr(scrutinee); self.resolve_pattern_top(pat, PatternSource::Let); } diff --git a/compiler/rustc_typeck/src/check/_match.rs b/compiler/rustc_typeck/src/check/_match.rs index 4f8c8ca899f..846c5fc09f2 100644 --- a/compiler/rustc_typeck/src/check/_match.rs +++ b/compiler/rustc_typeck/src/check/_match.rs @@ -24,27 +24,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) -> Ty<'tcx> { let tcx = self.tcx; - use hir::MatchSource::*; - let (source_if, if_no_else, force_scrutinee_bool) = match match_src { - IfLetDesugar { contains_else_clause, .. } => (true, !contains_else_clause, false), - WhileDesugar => (false, false, true), - _ => (false, false, false), - }; - - // Type check the discriminant and get its type. - let scrutinee_ty = if force_scrutinee_bool { - // Here we want to ensure: - // - // 1. That default match bindings are *not* accepted in the condition of an - // `if` expression. E.g. given `fn foo() -> &bool;` we reject `if foo() { .. }`. - // - // 2. By expecting `bool` for `expr` we get nice diagnostics for e.g. `if x = y { .. }`. - // - // FIXME(60707): Consider removing hack with principled solution. - self.check_expr_has_type_or_error(scrut, self.tcx.types.bool, |_| {}) - } else { - self.demand_scrutinee_type(scrut, arms_contain_ref_bindings(arms), arms.is_empty()) - }; + let acrb = arms_contain_ref_bindings(arms); + let scrutinee_ty = self.demand_scrutinee_type(scrut, acrb, arms.is_empty()); // If there are no arms, that is a diverging match; a special case. if arms.is_empty() { @@ -52,7 +33,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { return tcx.types.never; } - self.warn_arms_when_scrutinee_diverges(arms, match_src); + self.warn_arms_when_scrutinee_diverges(arms); // Otherwise, we have to union together the types that the arms produce and so forth. let scrut_diverges = self.diverges.replace(Diverges::Maybe); @@ -112,128 +93,95 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } self.diverges.set(Diverges::Maybe); - let arm_ty = if source_if - && if_no_else - && i != 0 - && self.if_fallback_coercion( - expr.span, - &arms[0].body, - &mut coercion, - |hir_id, span| self.coercion_reason_match(hir_id, span), - ) { - tcx.ty_error() - } else { - // Only call this if this is not an `if` expr with an expected type and no `else` - // clause to avoid duplicated type errors. (#60254) - self.check_expr_with_expectation(&arm.body, expected) - }; + + let arm_ty = self.check_expr_with_expectation(&arm.body, expected); all_arms_diverge &= self.diverges.get(); let opt_suggest_box_span = self.opt_suggest_box_span(arm.body.span, arm_ty, orig_expected); - if source_if { - let then_expr = &arms[0].body; - match (i, if_no_else) { - (0, _) => coercion.coerce(self, &self.misc(expr.span), &arm.body, arm_ty), - (_, true) => {} // Handled above to avoid duplicated type errors (#60254). - (_, _) => { - let then_ty = prior_arm_ty.unwrap(); - let cause = self.if_cause( + let (arm_span, semi_span) = + self.get_appropriate_arm_semicolon_removal_span(&arms, i, prior_arm_ty, arm_ty); + let (span, code) = match i { + // The reason for the first arm to fail is not that the match arms diverge, + // but rather that there's a prior obligation that doesn't hold. + 0 => (arm_span, ObligationCauseCode::BlockTailExpression(arm.body.hir_id)), + _ => ( + expr.span, + ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause { + arm_span, + scrut_span: scrut.span, + semi_span, + source: match_src, + prior_arms: other_arms.clone(), + last_ty: prior_arm_ty.unwrap(), + scrut_hir_id: scrut.hir_id, + opt_suggest_box_span, + }), + ), + }; + let cause = self.cause(span, code); + + // This is the moral equivalent of `coercion.coerce(self, cause, arm.body, arm_ty)`. + // We use it this way to be able to expand on the potential error and detect when a + // `match` tail statement could be a tail expression instead. If so, we suggest + // removing the stray semicolon. + coercion.coerce_inner( + self, + &cause, + Some(&arm.body), + arm_ty, + Some(&mut |err: &mut DiagnosticBuilder<'_>| { + let can_coerce_to_return_ty = match self.ret_coercion.as_ref() { + Some(ret_coercion) if self.in_tail_expr => { + let ret_ty = ret_coercion.borrow().expected_ty(); + let ret_ty = self.inh.infcx.shallow_resolve(ret_ty); + self.can_coerce(arm_ty, ret_ty) + && prior_arm_ty.map_or(true, |t| self.can_coerce(t, ret_ty)) + // The match arms need to unify for the case of `impl Trait`. + && !matches!(ret_ty.kind(), ty::Opaque(..)) + } + _ => false, + }; + if let (Expectation::IsLast(stmt), Some(ret), true) = + (orig_expected, self.ret_type_span, can_coerce_to_return_ty) + { + let semi_span = expr.span.shrink_to_hi().with_hi(stmt.hi()); + let mut ret_span: MultiSpan = semi_span.into(); + ret_span.push_span_label( expr.span, - then_expr, - &arm.body, - then_ty, - arm_ty, - opt_suggest_box_span, + "this could be implicitly returned but it is a statement, not a \ + tail expression" + .to_owned(), ); - coercion.coerce(self, &cause, &arm.body, arm_ty); - } - } - } else { - let (arm_span, semi_span) = - self.get_appropriate_arm_semicolon_removal_span(&arms, i, prior_arm_ty, arm_ty); - let (span, code) = match i { - // The reason for the first arm to fail is not that the match arms diverge, - // but rather that there's a prior obligation that doesn't hold. - 0 => (arm_span, ObligationCauseCode::BlockTailExpression(arm.body.hir_id)), - _ => ( - expr.span, - ObligationCauseCode::MatchExpressionArm(box MatchExpressionArmCause { - arm_span, - scrut_span: scrut.span, + ret_span.push_span_label( + ret, + "the `match` arms can conform to this return type".to_owned(), + ); + ret_span.push_span_label( semi_span, - source: match_src, - prior_arms: other_arms.clone(), - last_ty: prior_arm_ty.unwrap(), - scrut_hir_id: scrut.hir_id, - opt_suggest_box_span, - }), - ), - }; - let cause = self.cause(span, code); - - // This is the moral equivalent of `coercion.coerce(self, cause, arm.body, arm_ty)`. - // We use it this way to be able to expand on the potential error and detect when a - // `match` tail statement could be a tail expression instead. If so, we suggest - // removing the stray semicolon. - coercion.coerce_inner( - self, - &cause, - Some(&arm.body), - arm_ty, - Some(&mut |err: &mut DiagnosticBuilder<'_>| { - let can_coerce_to_return_ty = match self.ret_coercion.as_ref() { - Some(ret_coercion) if self.in_tail_expr => { - let ret_ty = ret_coercion.borrow().expected_ty(); - let ret_ty = self.inh.infcx.shallow_resolve(ret_ty); - self.can_coerce(arm_ty, ret_ty) - && prior_arm_ty.map_or(true, |t| self.can_coerce(t, ret_ty)) - // The match arms need to unify for the case of `impl Trait`. - && !matches!(ret_ty.kind(), ty::Opaque(..)) - } - _ => false, - }; - if let (Expectation::IsLast(stmt), Some(ret), true) = - (orig_expected, self.ret_type_span, can_coerce_to_return_ty) - { - let semi_span = expr.span.shrink_to_hi().with_hi(stmt.hi()); - let mut ret_span: MultiSpan = semi_span.into(); - ret_span.push_span_label( - expr.span, - "this could be implicitly returned but it is a statement, not a \ - tail expression" - .to_owned(), - ); - ret_span.push_span_label( - ret, - "the `match` arms can conform to this return type".to_owned(), - ); - ret_span.push_span_label( - semi_span, - "the `match` is a statement because of this semicolon, consider \ - removing it" - .to_owned(), - ); - err.span_note( - ret_span, - "you might have meant to return the `match` expression", - ); - err.tool_only_span_suggestion( - semi_span, - "remove this semicolon", - String::new(), - Applicability::MaybeIncorrect, - ); - } - }), - false, - ); + "the `match` is a statement because of this semicolon, consider \ + removing it" + .to_owned(), + ); + err.span_note( + ret_span, + "you might have meant to return the `match` expression", + ); + err.tool_only_span_suggestion( + semi_span, + "remove this semicolon", + String::new(), + Applicability::MaybeIncorrect, + ); + } + }), + false, + ); - other_arms.push(arm_span); - if other_arms.len() > 5 { - other_arms.remove(0); - } + other_arms.push(arm_span); + if other_arms.len() > 5 { + other_arms.remove(0); } prior_arm_ty = Some(arm_ty); } @@ -283,39 +231,27 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// When the previously checked expression (the scrutinee) diverges, /// warn the user about the match arms being unreachable. - fn warn_arms_when_scrutinee_diverges( - &self, - arms: &'tcx [hir::Arm<'tcx>], - source: hir::MatchSource, - ) { - use hir::MatchSource::*; - let msg = match source { - IfLetDesugar { .. } => "block in `if` expression", - WhileDesugar { .. } | WhileLetDesugar { .. } => "block in `while` expression", - _ => "arm", - }; + fn warn_arms_when_scrutinee_diverges(&self, arms: &'tcx [hir::Arm<'tcx>]) { for arm in arms { - self.warn_if_unreachable(arm.body.hir_id, arm.body.span, msg); + self.warn_if_unreachable(arm.body.hir_id, arm.body.span, "arm"); } } /// Handle the fallback arm of a desugared if(-let) like a missing else. /// /// Returns `true` if there was an error forcing the coercion to the `()` type. - pub(crate) fn if_fallback_coercion<F, T>( + pub(super) fn if_fallback_coercion<T>( &self, span: Span, then_expr: &'tcx hir::Expr<'tcx>, coercion: &mut CoerceMany<'tcx, '_, T>, - ret_reason: F, ) -> bool where - F: Fn(hir::HirId, Span) -> Option<(Span, String)>, T: AsCoercionSite, { // If this `if` expr is the parent's function return expr, // the cause of the type coercion is the return type, point at it. (#25228) - let ret_reason = ret_reason(then_expr.hir_id, span); + let ret_reason = self.maybe_get_coercion_reason(then_expr.hir_id, span); let cause = self.cause(span, ObligationCauseCode::IfExpressionWithNoElse); let mut error = false; coercion.coerce_forced_unit( @@ -338,55 +274,34 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { error } - pub(crate) fn coercion_reason_if( - &self, - hir_id: hir::HirId, - span: Span, - ) -> Option<(Span, String)> { - self.coercion_reason_inner(hir_id, span, 1) - } - - pub(crate) fn coercion_reason_match( - &self, - hir_id: hir::HirId, - span: Span, - ) -> Option<(Span, String)> { - self.coercion_reason_inner(hir_id, span, 2) - } - - fn coercion_reason_inner( - &self, - hir_id: hir::HirId, - span: Span, - parent_index: usize, - ) -> Option<(Span, String)> { - let hir = self.tcx.hir(); - let mut parent_iter = hir.parent_iter(hir_id); - let (_, node) = parent_iter.nth(parent_index)?; - match node { - hir::Node::Block(block) => { - let expr = block.expr?; - // check that the body's parent is an fn - let (_, parent) = parent_iter.nth(1)?; - if let hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(..), .. }) = parent { - // check that the `if` expr without `else` is the fn body's expr - if expr.span == span { - let (fn_decl, _) = self.get_fn_decl(hir_id)?; + fn maybe_get_coercion_reason(&self, hir_id: hir::HirId, sp: Span) -> Option<(Span, String)> { + let node = { + let rslt = self.tcx.hir().get_parent_node(self.tcx.hir().get_parent_node(hir_id)); + self.tcx.hir().get(rslt) + }; + if let hir::Node::Block(block) = node { + // check that the body's parent is an fn + let parent = self + .tcx + .hir() + .get(self.tcx.hir().get_parent_node(self.tcx.hir().get_parent_node(block.hir_id))); + if let (Some(expr), hir::Node::Item(hir::Item { kind: hir::ItemKind::Fn(..), .. })) = + (&block.expr, parent) + { + // check that the `if` expr without `else` is the fn body's expr + if expr.span == sp { + return self.get_fn_decl(hir_id).and_then(|(fn_decl, _)| { let span = fn_decl.output.span(); let snippet = self.tcx.sess.source_map().span_to_snippet(span).ok()?; - return Some(( - span, - format!("expected `{}` because of this return type", snippet), - )); - } + Some((span, format!("expected `{}` because of this return type", snippet))) + }); } - None } - hir::Node::Local(hir::Local { ty: Some(_), pat, .. }) => { - Some((pat.span, "expected because of this assignment".to_string())) - } - _ => None, } + if let hir::Node::Local(hir::Local { ty: Some(_), pat, .. }) = node { + return Some((pat.span, "expected because of this assignment".to_string())); + } + None } pub(crate) fn if_cause( @@ -492,7 +407,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) } - fn demand_scrutinee_type( + pub(super) fn demand_scrutinee_type( &self, scrut: &'tcx hir::Expr<'tcx>, contains_ref_bindings: Option<hir::Mutability>, diff --git a/compiler/rustc_typeck/src/check/expr.rs b/compiler/rustc_typeck/src/check/expr.rs index a5362856511..23ce4275d40 100644 --- a/compiler/rustc_typeck/src/check/expr.rs +++ b/compiler/rustc_typeck/src/check/expr.rs @@ -187,7 +187,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // Warn for non-block expressions with diverging children. match expr.kind { - ExprKind::Block(..) | ExprKind::If(..) | ExprKind::Loop(..) | ExprKind::Match(..) => {} + ExprKind::Block(..) + | ExprKind::If(..) + | ExprKind::Let(..) + | ExprKind::Loop(..) + | ExprKind::Match(..) => {} // If `expr` is a result of desugaring the try block and is an ok-wrapped // diverging expression (e.g. it arose from desugaring of `try { return }`), // we skip issuing a warning because it is autogenerated code. @@ -262,6 +266,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } ExprKind::Ret(ref expr_opt) => self.check_expr_return(expr_opt.as_deref(), expr), + ExprKind::Let(pat, let_expr, _) => self.check_expr_let(let_expr, pat), ExprKind::Loop(body, _, source, _) => { self.check_expr_loop(body, source, expected, expr) } @@ -802,7 +807,11 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) -> Ty<'tcx> { let cond_ty = self.check_expr_has_type_or_error(cond_expr, self.tcx.types.bool, |_| {}); - self.warn_if_unreachable(cond_expr.hir_id, then_expr.span, "block in `if` expression"); + self.warn_if_unreachable( + cond_expr.hir_id, + then_expr.span, + "block in `if` or `while` expression", + ); let cond_diverges = self.diverges.get(); self.diverges.set(Diverges::Maybe); @@ -837,9 +846,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // We won't diverge unless both branches do (or the condition does). self.diverges.set(cond_diverges | then_diverges & else_diverges); } else { - self.if_fallback_coercion(sp, then_expr, &mut coerce, |hir_id, span| { - self.coercion_reason_if(hir_id, span) - }); + self.if_fallback_coercion(sp, then_expr, &mut coerce); // If the condition is false we can't diverge. self.diverges.set(cond_diverges); @@ -875,26 +882,16 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { }; if !lhs.is_syntactic_place_expr() { // Do not suggest `if let x = y` as `==` is way more likely to be the intention. - let mut span_err = || { - // Likely `if let` intended. + let hir = self.tcx.hir(); + if let hir::Node::Expr(hir::Expr { kind: ExprKind::If { .. }, .. }) = + hir.get(hir.get_parent_node(hir.get_parent_node(expr.hir_id))) + { err.span_suggestion_verbose( expr.span.shrink_to_lo(), "you might have meant to use pattern matching", "let ".to_string(), applicability, ); - }; - if let hir::Node::Expr(hir::Expr { - kind: ExprKind::Match(_, _, hir::MatchSource::WhileDesugar), - .. - }) = self.tcx.hir().get( - self.tcx.hir().get_parent_node(self.tcx.hir().get_parent_node(expr.hir_id)), - ) { - span_err(); - } else if let hir::Node::Expr(hir::Expr { kind: ExprKind::If { .. }, .. }) = - self.tcx.hir().get(self.tcx.hir().get_parent_node(expr.hir_id)) - { - span_err(); } } if eq { @@ -929,6 +926,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } + fn check_expr_let(&self, expr: &'tcx hir::Expr<'tcx>, pat: &'tcx hir::Pat<'tcx>) -> Ty<'tcx> { + self.warn_if_unreachable(expr.hir_id, expr.span, "block in `let` expression"); + let expr_ty = self.demand_scrutinee_type(expr, pat.contains_explicit_ref_binding(), false); + self.check_pat_top(pat, expr_ty, Some(expr.span), true); + self.tcx.types.bool + } + fn check_expr_loop( &self, body: &'tcx hir::Block<'tcx>, @@ -943,7 +947,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { Some(CoerceMany::new(coerce_to)) } - hir::LoopSource::While | hir::LoopSource::WhileLet | hir::LoopSource::ForLoop => None, + hir::LoopSource::While | hir::LoopSource::ForLoop => None, }; let ctxt = BreakableCtxt { diff --git a/compiler/rustc_typeck/src/expr_use_visitor.rs b/compiler/rustc_typeck/src/expr_use_visitor.rs index 1d7852d964c..3b241317aa3 100644 --- a/compiler/rustc_typeck/src/expr_use_visitor.rs +++ b/compiler/rustc_typeck/src/expr_use_visitor.rs @@ -229,6 +229,10 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { } } + hir::ExprKind::Let(ref pat, ref expr, _) => { + self.walk_local(expr, pat, |t| t.borrow_expr(&expr, ty::ImmBorrow)); + } + hir::ExprKind::Match(ref discr, arms, _) => { let discr_place = return_if_err!(self.mc.cat_expr(&discr)); @@ -428,10 +432,12 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { fn walk_stmt(&mut self, stmt: &hir::Stmt<'_>) { match stmt.kind { - hir::StmtKind::Local(ref local) => { - self.walk_local(&local); + hir::StmtKind::Local(hir::Local { pat, init: Some(ref expr), .. }) => { + self.walk_local(expr, pat, |_| {}); } + hir::StmtKind::Local(_) => {} + hir::StmtKind::Item(_) => { // We don't visit nested items in this visitor, // only the fn body we were given. @@ -443,16 +449,14 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { } } - fn walk_local(&mut self, local: &hir::Local<'_>) { - if let Some(ref expr) = local.init { - // Variable declarations with - // initializers are considered - // "assigns", which is handled by - // `walk_pat`: - self.walk_expr(&expr); - let init_place = return_if_err!(self.mc.cat_expr(&expr)); - self.walk_irrefutable_pat(&init_place, &local.pat); - } + fn walk_local<F>(&mut self, expr: &hir::Expr<'_>, pat: &hir::Pat<'_>, mut f: F) + where + F: FnMut(&mut Self), + { + self.walk_expr(&expr); + let expr_place = return_if_err!(self.mc.cat_expr(&expr)); + f(self); + self.walk_irrefutable_pat(&expr_place, &pat); } /// Indicates that the value of `blk` will be consumed, meaning either copied or moved diff --git a/compiler/rustc_typeck/src/mem_categorization.rs b/compiler/rustc_typeck/src/mem_categorization.rs index 14af11097cf..f876d0f2513 100644 --- a/compiler/rustc_typeck/src/mem_categorization.rs +++ b/compiler/rustc_typeck/src/mem_categorization.rs @@ -368,6 +368,7 @@ impl<'a, 'tcx> MemCategorizationContext<'a, 'tcx> { | hir::ExprKind::Tup(..) | hir::ExprKind::Binary(..) | hir::ExprKind::Block(..) + | hir::ExprKind::Let(..) | hir::ExprKind::Loop(..) | hir::ExprKind::Match(..) | hir::ExprKind::Lit(..) |