From 9a0391f711c898cfbf782bbfdf4cabc4041ae102 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 20 Jan 2020 16:43:20 +0100 Subject: [PATCH 01/11] let_chains: introduce `hir::ExprKind::Let(..)` --- src/librustc_ast_lowering/expr.rs | 30 +--------- src/librustc_hir/hir.rs | 7 +++ src/librustc_hir/intravisit.rs | 9 ++- src/librustc_hir/print.rs | 48 +++++++++++----- src/librustc_mir_build/hair/cx/expr.rs | 34 +++++++++++- .../hair/pattern/check_match.rs | 6 +- src/librustc_passes/liveness.rs | 21 +++++-- src/librustc_typeck/check/_match.rs | 12 ++-- src/librustc_typeck/check/expr.rs | 11 +++- src/librustc_typeck/expr_use_visitor.rs | 55 +++++++++---------- src/librustc_typeck/mem_categorization.rs | 1 + src/libsyntax/util/parser.rs | 2 +- .../disallowed-positions.rs | 3 - .../disallowed-positions.stderr | 37 ++----------- 14 files changed, 148 insertions(+), 128 deletions(-) diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs index 5dc855e935c07..be0106f591d67 100644 --- a/src/librustc_ast_lowering/expr.rs +++ b/src/librustc_ast_lowering/expr.rs @@ -243,10 +243,6 @@ impl<'hir> LoweringContext<'_, 'hir> { } } - /// 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. @@ -263,31 +259,7 @@ impl<'hir> LoweringContext<'_, 'hir> { .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, - ) + hir::ExprKind::Let(self.lower_pat(pat), self.lower_expr(scrutinee)) } fn lower_expr_if( diff --git a/src/librustc_hir/hir.rs b/src/librustc_hir/hir.rs index b62a7e413e303..dae38cd968dd8 100644 --- a/src/librustc_hir/hir.rs +++ b/src/librustc_hir/hir.rs @@ -1348,6 +1348,7 @@ impl Expr<'_> { ExprKind::Lit(_) => ExprPrecedence::Lit, ExprKind::Type(..) | ExprKind::Cast(..) => ExprPrecedence::Cast, ExprKind::DropTemps(ref expr, ..) => expr.precedence(), + ExprKind::Let(..) => ExprPrecedence::Let, ExprKind::Loop(..) => ExprPrecedence::Loop, ExprKind::Match(..) => ExprPrecedence::Match, ExprKind::Closure(..) => ExprPrecedence::Closure, @@ -1414,6 +1415,7 @@ impl Expr<'_> { | ExprKind::Break(..) | ExprKind::Continue(..) | ExprKind::Ret(..) + | ExprKind::Let(..) | ExprKind::Loop(..) | ExprKind::Assign(..) | ExprKind::InlineAsm(..) @@ -1563,6 +1565,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` which only occur as statements. + /// The `let Some(x) = foo()` in `if let Some(x) = foo()` is an example of `Let(..)`. + Let(&'hir Pat<'hir>, &'hir Expr<'hir>), /// A conditionless loop (can be exited with `break`, `continue`, or `return`). /// /// I.e., `'label: loop { }`. diff --git a/src/librustc_hir/intravisit.rs b/src/librustc_hir/intravisit.rs index 539a0eee0e312..f0395a05c75e4 100644 --- a/src/librustc_hir/intravisit.rs +++ b/src/librustc_hir/intravisit.rs @@ -462,8 +462,7 @@ pub fn walk_body<'v, V: Visitor<'v>>(visitor: &mut V, body: &'v Body<'v>) { } pub fn walk_local<'v, V: Visitor<'v>>(visitor: &mut V, local: &'v Local<'v>) { - // Intentionally visiting the expr first - the initialization expr - // dominates the local's definition. + // Visit the expr first. The initialization expr dominates the local's definition. walk_list!(visitor, visit_expr, &local.init); walk_list!(visitor, visit_attribute, local.attrs.iter()); visitor.visit_id(local.hir_id); @@ -1082,6 +1081,12 @@ 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) => { + // Visit the expr first. The initialization expr dominates the `let`'s definition. + // This is the same logic in `walk_local`. + visitor.visit_expr(expr); + visitor.visit_pat(pat); + } ExprKind::Loop(ref block, ref opt_label, _) => { walk_list!(visitor, visit_label, opt_label); visitor.visit_block(block); diff --git a/src/librustc_hir/print.rs b/src/librustc_hir/print.rs index b9598c9376146..c167c4704e934 100644 --- a/src/librustc_hir/print.rs +++ b/src/librustc_hir/print.rs @@ -1037,28 +1037,44 @@ 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(); - } + /// Print a `let pat = scrutinee` expression. + fn print_let(&mut self, pat: &hir::Pat<'_>, scrutinee: &hir::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()), + ) } - /// 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 { + fn print_expr_as_cond(&mut self, expr: &hir::Expr<'_>) { + self.print_expr_cond_paren(expr, Self::cond_needs_par(expr)) + } + + fn print_expr_maybe_paren(&mut self, expr: &hir::Expr<'_>, prec: i8) { + self.print_expr_cond_paren(expr, expr.precedence().order() < prec) + } + + /// Does `expr` need parenthesis when printed in a condition position? + fn cond_needs_par(expr: &hir::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) {}`. hir::ExprKind::Closure(..) | hir::ExprKind::Ret(..) | hir::ExprKind::Break(..) => true, _ => contains_exterior_struct_lit(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(); } @@ -1178,6 +1194,9 @@ impl<'a> State<'a> { // of `(x as i32) < ...`. We need to convince it _not_ to do that. (&hir::ExprKind::Cast { .. }, hir::BinOpKind::Lt) | (&hir::ExprKind::Cast { .. }, hir::BinOpKind::Shl) => parser::PREC_FORCE_PAREN, + (&hir::ExprKind::Let { .. }, _) if !parser::needs_par_as_let_scrutinee(prec) => { + parser::PREC_FORCE_PAREN + } _ => left_prec, }; @@ -1285,6 +1304,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::Loop(ref blk, opt_label, _) => { if let Some(label) = opt_label { self.print_ident(label.ident); diff --git a/src/librustc_mir_build/hair/cx/expr.rs b/src/librustc_mir_build/hair/cx/expr.rs index d6786ea247973..96812377a8203 100644 --- a/src/librustc_mir_build/hair/cx/expr.rs +++ b/src/librustc_mir_build/hair/cx/expr.rs @@ -443,8 +443,38 @@ fn make_mirror_unadjusted<'a, 'tcx>( }, Err(err) => bug!("invalid loop id for continue: {}", err), }, - hir::ExprKind::Match(ref discr, ref arms, _) => ExprKind::Match { - scrutinee: discr.to_ref(), + hir::ExprKind::Let(ref pat, ref scrutinee) => { + // FIXME(let_chains, Centril): Temporary solution while we cannot lower these to MIR. + // + // If we got here, the `let` expression is not allowed + // and we have emitted an error in HIR lowering. + // + // 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 span = expr.span; + let pat = cx.pattern_from_hir(pat); + let arm = |pattern, val| { + let literal = Const::from_bool(cx.tcx, val); + let kind = ExprKind::Literal { literal, user_ty: None }; + let body = Expr { temp_lifetime, ty: expr_ty, span, kind }.to_ref(); + let scope = + region::Scope { id: expr.hir_id.local_id, data: region::ScopeData::Node }; + let lint_level = LintLevel::Inherited; + Arm { pattern, guard: None, body, lint_level, scope, span } + }; + let else_arm = arm(Pat::wildcard_from_ty(pat.ty), false); + let then_arm = arm(pat, true); + ExprKind::Match { scrutinee: scrutinee.to_ref(), arms: vec![then_arm, else_arm] } + } + hir::ExprKind::Match(ref scrutinee, ref arms, _) => ExprKind::Match { + scrutinee: scrutinee.to_ref(), arms: arms.iter().map(|a| convert_arm(cx, a)).collect(), }, hir::ExprKind::Loop(ref body, _, _) => { diff --git a/src/librustc_mir_build/hair/pattern/check_match.rs b/src/librustc_mir_build/hair/pattern/check_match.rs index 49b7c2d41fcbb..9a28b24e8b358 100644 --- a/src/librustc_mir_build/hair/pattern/check_match.rs +++ b/src/librustc_mir_build/hair/pattern/check_match.rs @@ -28,9 +28,8 @@ crate fn check_match(tcx: TyCtxt<'_>, def_id: DefId) { Some(id) => tcx.hir().body_owned_by(id), }; - let mut visitor = - MatchVisitor { tcx, tables: tcx.body_tables(body_id), param_env: tcx.param_env(def_id) }; - visitor.visit_body(tcx.hir().body(body_id)); + MatchVisitor { tcx, tables: tcx.body_tables(body_id), param_env: tcx.param_env(def_id) } + .visit_body(tcx.hir().body(body_id)); } fn create_e0004(sess: &Session, sp: Span, error_message: String) -> DiagnosticBuilder<'_> { @@ -53,6 +52,7 @@ impl<'tcx> Visitor<'tcx> for MatchVisitor<'_, 'tcx> { fn visit_expr(&mut self, ex: &'tcx hir::Expr<'tcx>) { intravisit::walk_expr(self, ex); + // FIXME(let_chains): Deal with `::Let`. if let hir::ExprKind::Match(ref scrut, ref arms, source) = ex.kind { self.check_match(scrut, arms, source); } diff --git a/src/librustc_passes/liveness.rs b/src/librustc_passes/liveness.rs index 7718139f6e924..15f88d56e76f9 100644 --- a/src/librustc_passes/liveness.rs +++ b/src/librustc_passes/liveness.rs @@ -501,6 +501,11 @@ fn visit_expr<'tcx>(ir: &mut IrMaps<'tcx>, expr: &'tcx Expr<'tcx>) { ir.body_owner = old_body_owner; } + hir::ExprKind::Let(ref pat, _) => { + add_from_pat(ir, pat); + intravisit::walk_expr(ir, expr); + } + // live nodes required for interesting control flow: hir::ExprKind::Match(..) | hir::ExprKind::Loop(..) => { ir.add_live_node_for_node(expr.hir_id, ExprNode(expr.span)); @@ -1001,6 +1006,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), @@ -1367,8 +1377,9 @@ impl<'a, 'tcx> Visitor<'tcx> for Liveness<'a, 'tcx> { intravisit::walk_local(self, local); } - fn visit_expr(&mut self, ex: &'tcx Expr<'tcx>) { - check_expr(self, ex); + fn visit_expr(&mut self, expr: &'tcx Expr<'tcx>) { + check_expr(self, expr); + intravisit::walk_expr(self, expr); } fn visit_arm(&mut self, arm: &'tcx hir::Arm<'tcx>) { @@ -1403,6 +1414,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(..) @@ -1431,8 +1446,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/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index 686cdfbc089b4..35fcb64b36d09 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -39,7 +39,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // 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(arms, scrut) + self.demand_scrutinee_type(arms.iter().map(|a| a.pat), scrut) }; // If there are no arms, that is a diverging match; a special case. @@ -370,9 +370,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { ) } - fn demand_scrutinee_type( + pub(super) fn demand_scrutinee_type( &self, - arms: &'tcx [hir::Arm<'tcx>], + pats: impl Iterator>, scrut: &'tcx hir::Expr<'tcx>, ) -> Ty<'tcx> { // Not entirely obvious: if matches may create ref bindings, we want to @@ -427,10 +427,8 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // (once introduced) is populated by the time we get here. // // See #44848. - let contains_ref_bindings = arms - .iter() - .filter_map(|a| a.pat.contains_explicit_ref_binding()) - .max_by_key(|m| match *m { + let contains_ref_bindings = + pats.filter_map(|p| p.contains_explicit_ref_binding()).max_by_key(|m| match *m { hir::Mutability::Mut => 1, hir::Mutability::Not => 0, }); diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs index b4c2b85241f96..56a7a5042592c 100644 --- a/src/librustc_typeck/check/expr.rs +++ b/src/librustc_typeck/check/expr.rs @@ -248,11 +248,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } ExprKind::Ret(ref expr_opt) => self.check_expr_return(expr_opt.as_deref(), expr), + ExprKind::Let(ref pat, ref scrutinee) => self.check_expr_let(pat, scrutinee), ExprKind::Loop(ref body, _, source) => { self.check_expr_loop(body, source, expected, expr) } - ExprKind::Match(ref discrim, ref arms, match_src) => { - self.check_match(expr, &discrim, arms, expected, match_src) + ExprKind::Match(ref scrutinee, ref arms, match_src) => { + self.check_match(expr, &scrutinee, arms, expected, match_src) } ExprKind::Closure(capture, ref decl, body_id, _, gen) => { self.check_expr_closure(expr, capture, &decl, body_id, gen, expected) @@ -802,6 +803,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { } } + fn check_expr_let(&self, pat: &'tcx hir::Pat<'tcx>, scrut: &'tcx hir::Expr<'tcx>) -> Ty<'tcx> { + let scrut_ty = self.demand_scrutinee_type(core::iter::once(pat), scrut); + self.check_pat_top(pat, scrut_ty, Some(scrut.span), true); + self.tcx.types.bool + } + fn check_expr_loop( &self, body: &'tcx hir::Block<'tcx>, diff --git a/src/librustc_typeck/expr_use_visitor.rs b/src/librustc_typeck/expr_use_visitor.rs index 1d3ace933cc43..0b58d079c8c29 100644 --- a/src/librustc_typeck/expr_use_visitor.rs +++ b/src/librustc_typeck/expr_use_visitor.rs @@ -103,7 +103,7 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { let param_place = self.mc.cat_rvalue(param.hir_id, param.pat.span, param_ty); - self.walk_irrefutable_pat(¶m_place, ¶m.pat); + self.walk_pat(¶m_place, ¶m.pat); } self.consume_expr(&body.value); @@ -198,13 +198,17 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { self.consume_exprs(exprs); } - hir::ExprKind::Match(ref discr, arms, _) => { - let discr_place = return_if_err!(self.mc.cat_expr(&discr)); - self.borrow_expr(&discr, ty::ImmBorrow); + hir::ExprKind::Let(ref pat, ref scrutinee) => { + self.walk_local(scrutinee, pat); + } + + hir::ExprKind::Match(ref scrutinee, arms, _) => { + let scrut_place = return_if_err!(self.mc.cat_expr(&scrutinee)); + self.borrow_expr(&scrutinee, ty::ImmBorrow); - // treatment of the discriminant is handled while walking the arms. + // Treatment of the scrutinee is handled while walking the arms. for arm in arms { - self.walk_arm(&discr_place, arm); + self.walk_arm(&scrut_place, arm); } } @@ -297,10 +301,11 @@ 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); + match &stmt.kind { + hir::StmtKind::Local(hir::Local { pat, init: Some(scrutinee), .. }) => { + self.walk_local(scrutinee, pat); } + hir::StmtKind::Local(_) => {} hir::StmtKind::Item(_) => { // We don't visit nested items in this visitor, @@ -313,16 +318,12 @@ 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(&mut self, scrutinee: &hir::Expr<'_>, pat: &hir::Pat<'_>) { + // Variable declarations with initializers are considered + // "assigns", which is handled by `walk_pat`: + self.walk_expr(&scrutinee); + let scrut_place = return_if_err!(self.mc.cat_expr(&scrutinee)); + self.walk_pat(&scrut_place, &pat); } /// Indicates that the value of `blk` will be consumed, meaning either copied or moved @@ -455,8 +456,8 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { } } - fn walk_arm(&mut self, discr_place: &Place<'tcx>, arm: &hir::Arm<'_>) { - self.walk_pat(discr_place, &arm.pat); + fn walk_arm(&mut self, scrut_place: &Place<'tcx>, arm: &hir::Arm<'_>) { + self.walk_pat(scrut_place, &arm.pat); if let Some(hir::Guard::If(ref e)) = arm.guard { self.consume_expr(e) @@ -465,19 +466,13 @@ impl<'a, 'tcx> ExprUseVisitor<'a, 'tcx> { self.consume_expr(&arm.body); } - /// Walks a pat that occurs in isolation (i.e., top-level of fn argument or - /// let binding, and *not* a match arm or nested pat.) - fn walk_irrefutable_pat(&mut self, discr_place: &Place<'tcx>, pat: &hir::Pat<'_>) { - self.walk_pat(discr_place, pat); - } - /// The core driver for walking a pattern - fn walk_pat(&mut self, discr_place: &Place<'tcx>, pat: &hir::Pat<'_>) { - debug!("walk_pat(discr_place={:?}, pat={:?})", discr_place, pat); + fn walk_pat(&mut self, scrut_place: &Place<'tcx>, pat: &hir::Pat<'_>) { + debug!("walk_pat(scrut_place={:?}, pat={:?})", scrut_place, pat); let tcx = self.tcx(); let ExprUseVisitor { ref mc, ref mut delegate } = *self; - return_if_err!(mc.cat_pattern(discr_place.clone(), pat, |place, pat| { + return_if_err!(mc.cat_pattern(scrut_place.clone(), pat, |place, pat| { if let PatKind::Binding(_, canonical_id, ..) = pat.kind { debug!("walk_pat: binding place={:?} pat={:?}", place, pat,); if let Some(bm) = mc.tables.extract_binding_mode(tcx.sess, pat.hir_id, pat.span) { diff --git a/src/librustc_typeck/mem_categorization.rs b/src/librustc_typeck/mem_categorization.rs index d3d0aa2e5807f..2ae7862ce87b3 100644 --- a/src/librustc_typeck/mem_categorization.rs +++ b/src/librustc_typeck/mem_categorization.rs @@ -396,6 +396,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(..) diff --git a/src/libsyntax/util/parser.rs b/src/libsyntax/util/parser.rs index a0ed89a9caef1..b98cc96b3c647 100644 --- a/src/libsyntax/util/parser.rs +++ b/src/libsyntax/util/parser.rs @@ -367,7 +367,7 @@ pub fn prec_let_scrutinee_needs_par() -> usize { /// /// Conversely, suppose that we have `(let _ = a) OP b` and `order` is that of `OP`. /// Can we print this as `let _ = a OP b`? -crate fn needs_par_as_let_scrutinee(order: i8) -> bool { +pub fn needs_par_as_let_scrutinee(order: i8) -> bool { order <= prec_let_scrutinee_needs_par() as i8 } diff --git a/src/test/ui/rfc-2497-if-let-chains/disallowed-positions.rs b/src/test/ui/rfc-2497-if-let-chains/disallowed-positions.rs index d5756737f1791..71af704c69f0d 100644 --- a/src/test/ui/rfc-2497-if-let-chains/disallowed-positions.rs +++ b/src/test/ui/rfc-2497-if-let-chains/disallowed-positions.rs @@ -216,17 +216,14 @@ fn inside_const_generic_arguments() { if let A::<{ true && let 1 = 1 //~ ERROR `let` expressions are not supported here - //~| ERROR `match` is not allowed in a `const` }>::O = 5 {} while let A::<{ true && let 1 = 1 //~ ERROR `let` expressions are not supported here - //~| ERROR `match` is not allowed in a `const` }>::O = 5 {} if A::<{ true && let 1 = 1 //~ ERROR `let` expressions are not supported here - //~| ERROR `match` is not allowed in a `const` }>::O == 5 {} // In the cases above we have `ExprKind::Block` to help us out. diff --git a/src/test/ui/rfc-2497-if-let-chains/disallowed-positions.stderr b/src/test/ui/rfc-2497-if-let-chains/disallowed-positions.stderr index 7170adca60dc3..723d374616f5a 100644 --- a/src/test/ui/rfc-2497-if-let-chains/disallowed-positions.stderr +++ b/src/test/ui/rfc-2497-if-let-chains/disallowed-positions.stderr @@ -1,5 +1,5 @@ error: expected one of `,` or `>`, found `&&` - --> $DIR/disallowed-positions.rs:239:14 + --> $DIR/disallowed-positions.rs:236:14 | LL | true && let 1 = 1 | ^^ expected one of `,` or `>` @@ -482,7 +482,7 @@ LL | true && let 1 = 1 = note: as well as when nested within `&&` and parenthesis in those conditions error: `let` expressions are not supported here - --> $DIR/disallowed-positions.rs:223:17 + --> $DIR/disallowed-positions.rs:222:17 | LL | true && let 1 = 1 | ^^^^^^^^^ @@ -491,7 +491,7 @@ LL | true && let 1 = 1 = note: as well as when nested within `&&` and parenthesis in those conditions error: `let` expressions are not supported here - --> $DIR/disallowed-positions.rs:228:17 + --> $DIR/disallowed-positions.rs:226:17 | LL | true && let 1 = 1 | ^^^^^^^^^ @@ -513,33 +513,6 @@ warning: the feature `let_chains` is incomplete and may cause the compiler to cr LL | #![feature(let_chains)] // Avoid inflating `.stderr` with overzealous gates in this test. | ^^^^^^^^^^ -error[E0658]: `match` is not allowed in a `const` - --> $DIR/disallowed-positions.rs:218:17 - | -LL | true && let 1 = 1 - | ^^^^^^^^^ - | - = note: for more information, see https://github.com/rust-lang/rust/issues/49146 - = help: add `#![feature(const_if_match)]` to the crate attributes to enable - -error[E0658]: `match` is not allowed in a `const` - --> $DIR/disallowed-positions.rs:223:17 - | -LL | true && let 1 = 1 - | ^^^^^^^^^ - | - = note: for more information, see https://github.com/rust-lang/rust/issues/49146 - = help: add `#![feature(const_if_match)]` to the crate attributes to enable - -error[E0658]: `match` is not allowed in a `const` - --> $DIR/disallowed-positions.rs:228:17 - | -LL | true && let 1 = 1 - | ^^^^^^^^^ - | - = note: for more information, see https://github.com/rust-lang/rust/issues/49146 - = help: add `#![feature(const_if_match)]` to the crate attributes to enable - error[E0308]: mismatched types --> $DIR/disallowed-positions.rs:32:8 | @@ -986,7 +959,7 @@ LL | let 0 = 0?; = help: the trait `std::ops::Try` is not implemented for `{integer}` = note: required by `std::ops::Try::into_result` -error: aborting due to 106 previous errors +error: aborting due to 103 previous errors -Some errors have detailed explanations: E0277, E0308, E0600, E0614, E0658. +Some errors have detailed explanations: E0277, E0308, E0600, E0614. For more information about an error, try `rustc --explain E0277`. From 4810cf6e8e0b7634d7457fbb36e9fc70deb84cb3 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 22 Jan 2020 15:22:55 +0100 Subject: [PATCH 02/11] let_chains: adjust lowering disallowed positions is checked in ast_validation in next commit --- src/librustc_ast_lowering/expr.rs | 78 ++++++++++++------------------- 1 file changed, 29 insertions(+), 49 deletions(-) diff --git a/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs index be0106f591d67..9633c6014b037 100644 --- a/src/librustc_ast_lowering/expr.rs +++ b/src/librustc_ast_lowering/expr.rs @@ -73,7 +73,9 @@ 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) => { + hir::ExprKind::Let(self.lower_pat(pat), self.lower_expr(scrutinee)) + } ExprKind::If(ref cond, ref then, ref else_opt) => { self.lower_expr_if(e.span, cond, then, else_opt.as_deref()) } @@ -243,25 +245,6 @@ impl<'hir> LoweringContext<'_, 'hir> { } } - 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 in conditions of `if`- and `while`-expressions") - .note("as well as when nested within `&&` and parenthesis in those conditions") - .emit(); - } else { - self.sess - .struct_span_err(span, "expected expression, found statement (`let`)") - .note("variable declaration using `let` is a statement") - .emit(); - } - - hir::ExprKind::Let(self.lower_pat(pat), self.lower_expr(scrutinee)) - } - fn lower_expr_if( &mut self, span: Span, @@ -269,41 +252,38 @@ impl<'hir> LoweringContext<'_, 'hir> { 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), false), - Some(els) => (self.lower_expr(els), true), - }; - let else_arm = self.arm(else_pat, else_expr); - - // Handle then + scrutinee: - let then_expr = self.lower_block_expr(then); - let (then_pat, scrutinee, desugar) = match cond.kind { - // ` => `: - ExprKind::Let(ref pat, ref scrutinee) => { - let scrutinee = self.lower_expr(scrutinee); - let pat = self.lower_pat(pat); - (pat, scrutinee, hir::MatchSource::IfLetDesugar { contains_else_clause }) + // Lower the `cond` expression. + let cond_expr = self.lower_expr(cond); + // Normally, the `cond` of `if cond` will drop temporaries before evaluating the blocks. + // This is achieved by using `drop-temps { cond }`, equivalent to `{ let _t = $cond; _t }`. + // However, for backwards compatibility reasons, `if let pat = scrutinee`, like `match` + // does not drop the temporaries of `scrutinee` before evaluating the blocks. + let contains_else_clause = else_opt.is_some(); + let (scrutinee, desugar) = match cond.kind { + ExprKind::Let(..) => { + (cond_expr, hir::MatchSource::IfLetDesugar { contains_else_clause }) } - // `true => `: _ => { - // Lower condition: - let cond = self.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 `if cond { ... }` does not - // let temporaries live outside of `cond`. - let cond = self.expr_drop_temps(span_block, cond, ThinVec::new()); - let pat = self.pat_bool(span, true); - (pat, cond, hir::MatchSource::IfDesugar { contains_else_clause }) + let span = + self.mark_span_with_reason(DesugaringKind::CondTemporary, cond_expr.span, None); + let cond_expr = self.expr_drop_temps(span, cond_expr, ThinVec::new()); + (cond_expr, hir::MatchSource::IfDesugar { contains_else_clause }) } }; + + // `true => $then`: + let then_expr = self.lower_block_expr(then); + let then_pat = self.pat_bool(span, true); let then_arm = self.arm(then_pat, self.arena.alloc(then_expr)); + // `_ => else_block` where `else_block` is `{}` if there's `None`: + let else_pat = self.pat_wild(span); + let else_expr = match else_opt { + None => self.expr_block_empty(span), + Some(els) => self.lower_expr(els), + }; + let else_arm = self.arm(else_pat, else_expr); + hir::ExprKind::Match(scrutinee, arena_vec![self; then_arm, else_arm], desugar) } From b20fcd3f58898f620ca77cabff9078ef8308e4cb Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 25 Jan 2020 02:17:35 +0100 Subject: [PATCH 03/11] let_chains: adjust ast_validation --- src/librustc_ast_passes/ast_validation.rs | 78 ++++++++++++++++++----- 1 file changed, 63 insertions(+), 15 deletions(-) diff --git a/src/librustc_ast_passes/ast_validation.rs b/src/librustc_ast_passes/ast_validation.rs index 152086bfce0ea..b2f17e78c0273 100644 --- a/src/librustc_ast_passes/ast_validation.rs +++ b/src/librustc_ast_passes/ast_validation.rs @@ -64,6 +64,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 +99,27 @@ impl<'a> AstValidator<'a> { self.bound_context = 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 visit_assoc_ty_constraint_from_generic_args(&mut self, constraint: &'a AssocTyConstraint) { match constraint.kind { AssocTyConstraintKind::Equality { .. } => {} @@ -502,23 +526,46 @@ impl<'a> Visitor<'a> for AstValidator<'a> { } fn visit_expr(&mut self, expr: &'a Expr) { - match &expr.kind { - ExprKind::Closure(_, _, _, fn_decl, _, _) => { - self.check_fn_decl(fn_decl); - } - ExprKind::InlineAsm(..) if !self.session.target.target.options.allow_asm => { - struct_span_err!( - self.session, - expr.span, - E0472, - "asm! is unsupported on this target" - ) - .emit(); + self.with_let_allowed(false, |this, let_allowed| { + match &expr.kind { + ExprKind::Closure(_, _, _, fn_decl, _, _) => { + this.check_fn_decl(fn_decl); + } + ExprKind::InlineAsm(..) if !this.session.target.target.options.allow_asm => { + struct_span_err!( + this.session, + expr.span, + E0472, + "asm! is unsupported on this target" + ) + .emit(); + } + ExprKind::Let(..) if !let_allowed => this.ban_let_expr(expr), + // `(e)` or `e && e` does not impose additional constraints on `e`. + ExprKind::Paren(_) | ExprKind::Binary(Spanned { node: BinOpKind::And, .. }, ..) => { + this.with_let_allowed(let_allowed, |this, _| visit::walk_expr(this, expr)); + return; // We've already walked into `expr`. + } + // However, we do allow it in the condition of the `if` expression. + // We do not allow `let` in `then` and `opt_else` directly. + 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; // We've already walked into `expr`. + } + // The same logic applies to `While`. + 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; // We've already walked into `expr`. + } + _ => {} } - _ => {} - } - visit::walk_expr(self, expr); + visit::walk_expr(this, expr); + }); } fn visit_ty(&mut self, ty: &'a Ty) { @@ -1049,6 +1096,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); From d741d8bea462e1daa3f0f41ccebc79103b9a2bd6 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 22 Jan 2020 15:23:56 +0100 Subject: [PATCH 04/11] let_chains: adjust HAIR lowering with hack --- src/librustc_mir_build/hair/cx/expr.rs | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/librustc_mir_build/hair/cx/expr.rs b/src/librustc_mir_build/hair/cx/expr.rs index 96812377a8203..702e9c52ed397 100644 --- a/src/librustc_mir_build/hair/cx/expr.rs +++ b/src/librustc_mir_build/hair/cx/expr.rs @@ -444,7 +444,7 @@ fn make_mirror_unadjusted<'a, 'tcx>( Err(err) => bug!("invalid loop id for continue: {}", err), }, hir::ExprKind::Let(ref pat, ref scrutinee) => { - // FIXME(let_chains, Centril): Temporary solution while we cannot lower these to MIR. + // HACK(let_chains, Centril): Temporary solution while we cannot lower these to MIR. // // If we got here, the `let` expression is not allowed // and we have emitted an error in HIR lowering. @@ -473,6 +473,21 @@ fn make_mirror_unadjusted<'a, 'tcx>( let then_arm = arm(pat, true); ExprKind::Match { scrutinee: scrutinee.to_ref(), arms: vec![then_arm, else_arm] } } + hir::ExprKind::Match( + hir::Expr { kind: hir::ExprKind::Let(ref pat, ref scrutinee), .. }, + [ref then_arm, ref else_arm], + hir::MatchSource::IfLetDesugar { .. }, + ) => { + // HACK(let_chains, Centril): This is the desugaring for `if let`. + // We do not yet have `hair::ExprKind::Let`. + // Therefore, we lower: `match (let pat = scrutinee) { true => then, _ => elze }`, + // into: `match scrutinee { pat => then, _ => elze }`. + let mut then_arm = convert_arm(cx, then_arm); + let mut else_arm = convert_arm(cx, else_arm); + then_arm.pattern = cx.pattern_from_hir(pat); + else_arm.pattern = Pat::wildcard_from_ty(then_arm.pattern.ty); + ExprKind::Match { scrutinee: scrutinee.to_ref(), arms: vec![then_arm, else_arm] } + } hir::ExprKind::Match(ref scrutinee, ref arms, _) => ExprKind::Match { scrutinee: scrutinee.to_ref(), arms: arms.iter().map(|a| convert_arm(cx, a)).collect(), From 17a5a05e632e7b0cb3677699c72696a83218d69e Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 22 Jan 2020 15:24:21 +0100 Subject: [PATCH 05/11] let_chains: fix region scopes with hack --- src/librustc_passes/region.rs | 36 ++++++++++++++++++++++++++++++++--- 1 file changed, 33 insertions(+), 3 deletions(-) diff --git a/src/librustc_passes/region.rs b/src/librustc_passes/region.rs index e79ca5c78d6d6..d3250bfb2445a 100644 --- a/src/librustc_passes/region.rs +++ b/src/librustc_passes/region.rs @@ -159,7 +159,11 @@ fn resolve_block<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, blk: &'tcx h visitor.cx = prev_cx; } -fn resolve_arm<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, arm: &'tcx hir::Arm<'tcx>) { +fn resolve_arm<'tcx>( + visitor: &mut RegionResolutionVisitor<'tcx>, + arm: &'tcx hir::Arm<'tcx>, + hack_for_let: impl FnOnce(&mut RegionResolutionVisitor<'tcx>), +) { let prev_cx = visitor.cx; visitor.enter_scope(Scope { id: arm.hir_id.local_id, data: ScopeData::Node }); @@ -171,7 +175,11 @@ fn resolve_arm<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, arm: &'tcx hir visitor.terminating_scopes.insert(expr.hir_id.local_id); } - intravisit::walk_arm(visitor, arm); + // HACK(let_chains, Centril): See `resolve_expr` below wrt. `::Let`. + // The goal here is to visit the `pat` in `let pat = _` as-if it were part of the `arm`. + hack_for_let(visitor); + // HACK(let_chains, Centril: Restore to the following when we can! + //intravisit::walk_arm(visitor, arm); visitor.cx = prev_cx; } @@ -390,6 +398,28 @@ fn resolve_expr<'tcx>(visitor: &mut RegionResolutionVisitor<'tcx>, expr: &'tcx h } } + hir::ExprKind::Match( + hir::Expr { kind: hir::ExprKind::Let(ref pat, ref scrutinee), .. }, + [ref then_arm, ref else_arm], + hir::MatchSource::IfLetDesugar { .. }, + ) => { + // HACK(let_chains, Centril): In HAIR lowering we currently adjust this + // to `match scrutinee { pat => then_arm.body, _ => else_arm.body }`. + // For consistency, we have to adjust the scopes here as well wrt. `pat`. + // Moreover, we need to replicate the visiting order in + // `intravisit::walk_expr` with respect to `ExprKind::Match`. + visitor.visit_expr(scrutinee); + // This is for the `then_arm`. NOTE(Centril): Preserve the order in `intravisit`! + resolve_arm(visitor, then_arm, |visitor| { + visitor.visit_id(then_arm.hir_id); + // NOTE(Centril): We are using `pat` here as opposed to `then_arm.pat`! + visitor.visit_pat(pat); + assert!(then_arm.guard.is_none()); + visitor.visit_expr(&then_arm.body); + walk_list!(visitor, visit_attribute, then_arm.attrs); + }); + visitor.visit_arm(else_arm); + } _ => intravisit::walk_expr(visitor, expr), } @@ -775,7 +805,7 @@ impl<'tcx> Visitor<'tcx> for RegionResolutionVisitor<'tcx> { } fn visit_arm(&mut self, a: &'tcx Arm<'tcx>) { - resolve_arm(self, a); + resolve_arm(self, a, |this| intravisit::walk_arm(this, a)); } fn visit_pat(&mut self, p: &'tcx Pat<'tcx>) { resolve_pat(self, p); From 6f0279968959e8c6d20391a9481a99147beb1828 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 23 Jan 2020 03:30:56 +0100 Subject: [PATCH 06/11] check_match: extract common logic --- src/librustc_mir_build/hair/pattern/_match.rs | 2 +- src/librustc_mir_build/hair/pattern/check_match.rs | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/librustc_mir_build/hair/pattern/_match.rs b/src/librustc_mir_build/hair/pattern/_match.rs index a2ce224904b29..08ed6b521b502 100644 --- a/src/librustc_mir_build/hair/pattern/_match.rs +++ b/src/librustc_mir_build/hair/pattern/_match.rs @@ -586,7 +586,7 @@ impl<'a, 'tcx> MatchCheckCtxt<'a, 'tcx> { tcx: TyCtxt<'tcx>, param_env: ty::ParamEnv<'tcx>, module: DefId, - f: impl for<'b> FnOnce(MatchCheckCtxt<'b, 'tcx>) -> R, + f: impl FnOnce(MatchCheckCtxt<'_, 'tcx>) -> R, ) -> R { let pattern_arena = TypedArena::default(); diff --git a/src/librustc_mir_build/hair/pattern/check_match.rs b/src/librustc_mir_build/hair/pattern/check_match.rs index 9a28b24e8b358..53333bec52836 100644 --- a/src/librustc_mir_build/hair/pattern/check_match.rs +++ b/src/librustc_mir_build/hair/pattern/check_match.rs @@ -140,6 +140,11 @@ impl<'tcx> MatchVisitor<'_, 'tcx> { (pattern, pattern_ty) } + fn check_in_cx(&self, hir_id: HirId, f: impl FnOnce(MatchCheckCtxt<'_, 'tcx>)) { + let module = self.tcx.hir().get_module_parent(hir_id); + MatchCheckCtxt::create_and_enter(self.tcx, self.param_env, module, |cx| f(cx)); + } + fn check_match( &mut self, scrut: &hir::Expr<'_>, @@ -151,8 +156,7 @@ impl<'tcx> MatchVisitor<'_, 'tcx> { self.check_patterns(arm.guard.is_some(), &arm.pat); } - let module = self.tcx.hir().get_module_parent(scrut.hir_id); - MatchCheckCtxt::create_and_enter(self.tcx, self.param_env, module, |ref mut cx| { + self.check_in_cx(scrut.hir_id, |ref mut cx| { let mut have_errors = false; let inlined_arms: Vec<_> = arms @@ -180,8 +184,7 @@ impl<'tcx> MatchVisitor<'_, 'tcx> { } fn check_irrefutable(&self, pat: &'tcx Pat<'tcx>, origin: &str, sp: Option) { - let module = self.tcx.hir().get_module_parent(pat.hir_id); - MatchCheckCtxt::create_and_enter(self.tcx, self.param_env, module, |ref mut cx| { + self.check_in_cx(pat.hir_id, |ref mut cx| { let (pattern, pattern_ty) = self.lower_pattern(cx, pat, &mut false); let pats: Matrix<'_, '_> = vec![PatStack::from_pattern(pattern)].into_iter().collect(); From 5cf33a63e023d7ab31585e63bf7bee6af1959a57 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 25 Jan 2020 00:31:22 +0100 Subject: [PATCH 07/11] let_chains: match check 'let' exprs --- .../hair/pattern/check_match.rs | 65 ++++++++++++++----- 1 file changed, 47 insertions(+), 18 deletions(-) diff --git a/src/librustc_mir_build/hair/pattern/check_match.rs b/src/librustc_mir_build/hair/pattern/check_match.rs index 53333bec52836..7901bda87b593 100644 --- a/src/librustc_mir_build/hair/pattern/check_match.rs +++ b/src/librustc_mir_build/hair/pattern/check_match.rs @@ -51,10 +51,10 @@ impl<'tcx> Visitor<'tcx> for MatchVisitor<'_, 'tcx> { fn visit_expr(&mut self, ex: &'tcx hir::Expr<'tcx>) { intravisit::walk_expr(self, ex); - - // FIXME(let_chains): Deal with `::Let`. - 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) => self.check_let(pat, scrut), + _ => {} } } @@ -145,6 +145,24 @@ impl<'tcx> MatchVisitor<'_, 'tcx> { MatchCheckCtxt::create_and_enter(self.tcx, self.param_env, module, |cx| f(cx)); } + fn check_let(&mut self, pat: &'tcx hir::Pat<'tcx>, scrut: &hir::Expr<'_>) { + self.check_patterns(false, pat); + // FIXME(let_chains, Centril): Exhaustiveness of `let p = e` is unnecessary for soundness. + // Consider dropping the checks here, at least the non-linting part for perf reasons. + self.check_in_cx(scrut.hir_id, |ref mut cx| { + let mut have_errors = false; + let pattern = self.lower_pattern(cx, pat, &mut have_errors).0; + if have_errors { + return; + } + let wild = cx.pattern_arena.alloc(super::Pat::wildcard_from_ty(pattern.ty)); + wild.span = pattern.span; + let arms = &[(pattern, pat.hir_id, false), (wild, pat.hir_id, false)]; + let desugar = hir::MatchSource::IfLetDesugar { contains_else_clause: true }; + self.check_union_irrefutable(cx, scrut, arms, desugar) + }); + } + fn check_match( &mut self, scrut: &hir::Expr<'_>, @@ -158,36 +176,47 @@ impl<'tcx> MatchVisitor<'_, 'tcx> { self.check_in_cx(scrut.hir_id, |ref mut cx| { let mut have_errors = false; - let inlined_arms: Vec<_> = arms .iter() .map(|hir::Arm { pat, guard, .. }| { (self.lower_pattern(cx, pat, &mut have_errors).0, pat.hir_id, guard.is_some()) }) .collect(); - - // Bail out early if inlining failed. if have_errors { + // Inlining failed, bail out early. return; } - // Fourth, check for unreachable arms. - let matrix = check_arms(cx, &inlined_arms, source); - - // Fifth, check if the match is exhaustive. - let scrut_ty = self.tables.node_type(scrut.hir_id); - // Note: An empty match isn't the same as an empty matrix for diagnostics purposes, - // since an empty matrix can occur when there are arms, if those arms all have guards. - let is_empty_match = inlined_arms.is_empty(); - check_exhaustive(cx, scrut_ty, scrut.span, &matrix, scrut.hir_id, is_empty_match); + self.check_union_irrefutable(cx, scrut, &inlined_arms, source); }) } + fn check_union_irrefutable<'p>( + &self, + cx: &mut MatchCheckCtxt<'p, 'tcx>, + scrut: &hir::Expr<'_>, + arms: &[(&'p super::Pat<'tcx>, HirId, bool)], + source: hir::MatchSource, + ) { + // Check for unreachable arms. + let matrix = check_arms(cx, &arms, source); + + // Check if the match is exhaustive. + let scrut_ty = self.tables.node_type(scrut.hir_id); + // Note: An empty match isn't the same as an empty matrix for diagnostics purposes, + // since an empty matrix can occur when there are arms, if those arms all have guards. + check_exhaustive(cx, scrut_ty, scrut.span, &matrix, scrut.hir_id, arms.is_empty()); + } + fn check_irrefutable(&self, pat: &'tcx Pat<'tcx>, origin: &str, sp: Option) { self.check_in_cx(pat.hir_id, |ref mut cx| { - let (pattern, pattern_ty) = self.lower_pattern(cx, pat, &mut false); - let pats: Matrix<'_, '_> = vec![PatStack::from_pattern(pattern)].into_iter().collect(); + let mut has_errors = false; + let (pattern, pattern_ty) = self.lower_pattern(cx, pat, &mut has_errors); + if has_errors { + return; + } + let pats: Matrix<'_, '_> = vec![PatStack::from_pattern(pattern)].into_iter().collect(); let witnesses = match check_not_useful(cx, pattern_ty, &pats, pat.hir_id) { Ok(_) => return, Err(err) => err, From 7b69082223b6666e5722d44a69b8232425f95e16 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sat, 25 Jan 2020 01:14:07 +0100 Subject: [PATCH 08/11] let_chains: fix ICE in generator_interior --- .../check/generator_interior.rs | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/librustc_typeck/check/generator_interior.rs b/src/librustc_typeck/check/generator_interior.rs index 75cc2c132f8a8..434d2d7919d12 100644 --- a/src/librustc_typeck/check/generator_interior.rs +++ b/src/librustc_typeck/check/generator_interior.rs @@ -247,6 +247,25 @@ impl<'a, 'tcx> Visitor<'tcx> for InteriorVisitor<'a, 'tcx> { } _ => intravisit::walk_expr(self, expr), }, + ExprKind::Match( + Expr { kind: ExprKind::Let(pat, scrutinee), .. }, + [then_arm, else_arm], + hir::MatchSource::IfLetDesugar { .. }, + ) => { + // HACK(let_chains, Centril): In HAIR lowering we currently adjust this + // to `match scrutinee { pat => then_arm.body, _ => else_arm.body }`. + // For consistency, we need to replicate the visiting order in + // `intravisit::walk_expr` with respect to `ExprKind::Match`. + self.visit_expr(scrutinee); + // This is for the `then_arm`. NOTE(Centril): Preserve the order in `intravisit`! + self.visit_id(then_arm.hir_id); + // NOTE(Centril): We are using `pat` here as opposed to `then_arm.pat`! + self.visit_pat(pat); + assert!(then_arm.guard.is_none()); + self.visit_expr(&then_arm.body); + syntax::walk_list!(self, visit_attribute, then_arm.attrs); + self.visit_arm(else_arm); + } _ => intravisit::walk_expr(self, expr), } From deed92a69d2a6744a9e05c60357fae59c1eb7e8c Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Sun, 26 Jan 2020 21:01:46 +0100 Subject: [PATCH 09/11] let_chains: adjust for 'while let' --- src/librustc/infer/error_reporting/mod.rs | 16 +- src/librustc_ast_lowering/expr.rs | 138 +++++++----------- src/librustc_hir/hir.rs | 13 +- src/librustc_mir_build/hair/cx/expr.rs | 2 +- .../hair/pattern/check_match.rs | 11 +- src/librustc_passes/check_const.rs | 22 +-- src/librustc_passes/region.rs | 4 +- src/librustc_typeck/check/_match.rs | 11 +- src/librustc_typeck/check/expr.rs | 3 +- .../check/generator_interior.rs | 2 +- 10 files changed, 82 insertions(+), 140 deletions(-) diff --git a/src/librustc/infer/error_reporting/mod.rs b/src/librustc/infer/error_reporting/mod.rs index 58566bdcc3549..bfccd6837016b 100644 --- a/src/librustc/infer/error_reporting/mod.rs +++ b/src/librustc/infer/error_reporting/mod.rs @@ -100,8 +100,8 @@ pub(super) fn note_and_explain_region( Some(Node::Expr(expr)) => match expr.kind { hir::ExprKind::Call(..) => "call", hir::ExprKind::MethodCall(..) => "method call", - hir::ExprKind::Match(.., hir::MatchSource::IfLetDesugar { .. }) => "if let", - hir::ExprKind::Match(.., hir::MatchSource::WhileLetDesugar) => "while let", + hir::ExprKind::Match(.., hir::MatchSource::IfDesugar { .. }) => "if", + hir::ExprKind::Match(.., hir::MatchSource::WhileDesugar) => "while", hir::ExprKind::Match(.., hir::MatchSource::ForLoopDesugar) => "for", hir::ExprKind::Match(..) => "match", _ => "expression", @@ -610,10 +610,6 @@ impl<'a, 'tcx> InferCtxt<'a, 'tcx> { scrut_hir_id, .. }) => match source { - hir::MatchSource::IfLetDesugar { .. } => { - let msg = "`if let` arms have incompatible types"; - err.span_label(cause.span, msg); - } hir::MatchSource::TryDesugar => { if let Some(ty::error::ExpectedFound { expected, .. }) = exp_found { let scrut_expr = self.tcx.hir().expect_expr(scrut_hir_id); @@ -1985,9 +1981,6 @@ impl<'tcx> 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" } @@ -2023,10 +2016,7 @@ impl<'tcx> 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", - }, + MatchExpressionArm(_) => "`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/src/librustc_ast_lowering/expr.rs b/src/librustc_ast_lowering/expr.rs index 9633c6014b037..910f0ce8defc3 100644 --- a/src/librustc_ast_lowering/expr.rs +++ b/src/librustc_ast_lowering/expr.rs @@ -245,48 +245,68 @@ impl<'hir> LoweringContext<'_, 'hir> { } } - fn lower_expr_if( - &mut self, - span: Span, - cond: &Expr, - then: &Block, - else_opt: Option<&Expr>, - ) -> hir::ExprKind<'hir> { + /// Lower the condition of an `if` or `while` expression. + /// This entails some special handling of immediate `let` expressions as conditions. + /// Namely, if the given `cond` is not a `let` expression then it is wrapped in `drop-temps`. + fn lower_expr_cond(&mut self, cond: &Expr) -> &'hir hir::Expr<'hir> { // Lower the `cond` expression. - let cond_expr = self.lower_expr(cond); + let cond = self.lower_expr(cond); // Normally, the `cond` of `if cond` will drop temporaries before evaluating the blocks. // This is achieved by using `drop-temps { cond }`, equivalent to `{ let _t = $cond; _t }`. // However, for backwards compatibility reasons, `if let pat = scrutinee`, like `match` // does not drop the temporaries of `scrutinee` before evaluating the blocks. - let contains_else_clause = else_opt.is_some(); - let (scrutinee, desugar) = match cond.kind { - ExprKind::Let(..) => { - (cond_expr, hir::MatchSource::IfLetDesugar { contains_else_clause }) - } + match cond.kind { + hir::ExprKind::Let(..) => cond, _ => { - let span = - self.mark_span_with_reason(DesugaringKind::CondTemporary, cond_expr.span, None); - let cond_expr = self.expr_drop_temps(span, cond_expr, ThinVec::new()); - (cond_expr, hir::MatchSource::IfDesugar { contains_else_clause }) + let reason = DesugaringKind::CondTemporary; + let span = self.mark_span_with_reason(reason, cond.span, None); + self.expr_drop_temps(span, cond, ThinVec::new()) } - }; + } + } - // `true => $then`: + /// Lower `then` into `true => then`. + fn lower_then_arm(&mut self, span: Span, then: &Block) -> hir::Arm<'hir> { let then_expr = self.lower_block_expr(then); let then_pat = self.pat_bool(span, true); - let then_arm = self.arm(then_pat, self.arena.alloc(then_expr)); + self.arm(then_pat, self.arena.alloc(then_expr)) + } - // `_ => else_block` where `else_block` is `{}` if there's `None`: + fn lower_else_arm(&mut self, span: Span, else_expr: &'hir hir::Expr<'hir>) -> hir::Arm<'hir> { let else_pat = self.pat_wild(span); + self.arm(else_pat, else_expr) + } + + fn lower_expr_if( + &mut self, + span: Span, + cond: &Expr, + then: &Block, + else_opt: Option<&Expr>, + ) -> hir::ExprKind<'hir> { + let scrutinee = self.lower_expr_cond(cond); + let then_arm = self.lower_then_arm(span, then); let else_expr = match else_opt { - None => self.expr_block_empty(span), + None => self.expr_block_empty(span), // Use `{}` if there's no `else` block. Some(els) => self.lower_expr(els), }; - let else_arm = self.arm(else_pat, else_expr); - + let else_arm = self.lower_else_arm(span, else_expr); + let desugar = hir::MatchSource::IfDesugar { contains_else_clause: else_opt.is_some() }; hir::ExprKind::Match(scrutinee, arena_vec![self; then_arm, else_arm], desugar) } + /// We desugar: `'label: while $cond $body` into: + /// + /// ``` + /// 'label: loop { + /// match $cond { + /// true => $body, + /// _ => break, + /// } + /// } + /// ``` + /// + /// where `$cond` is wrapped in `drop-temps { $cond }` if it isn't a `Let` expression. fn lower_expr_while_in_loop_scope( &mut self, span: Span, @@ -294,71 +314,23 @@ impl<'hir> LoweringContext<'_, 'hir> { body: &Block, opt_label: Option