From 5cccda1770f76b8184ea55e4735ce88f824f7fd8 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Mon, 11 Mar 2019 16:43:27 +0100 Subject: [PATCH 1/5] Remove hir::ExprKind::If and replace it with lowering to hir::ExprKind::Match. --- src/librustc/cfg/construct.rs | 41 -- src/librustc/hir/intravisit.rs | 5 - src/librustc/hir/lowering.rs | 124 ++--- src/librustc/hir/mod.rs | 10 +- src/librustc/hir/print.rs | 63 --- src/librustc/ich/impls_syntax.rs | 1 + src/librustc/middle/expr_use_visitor.rs | 8 - src/librustc/middle/liveness.rs | 25 +- src/librustc/middle/mem_categorization.rs | 2 +- src/librustc/middle/region.rs | 11 - src/librustc_errors/diagnostic_builder.rs | 12 + src/librustc_mir/build/expr/as_place.rs | 1 - src/librustc_mir/build/expr/as_rvalue.rs | 1 - src/librustc_mir/build/expr/category.rs | 1 - src/librustc_mir/build/expr/into.rs | 37 -- src/librustc_mir/hair/cx/expr.rs | 7 - src/librustc_mir/hair/mod.rs | 5 - src/librustc_mir/hair/pattern/check_match.rs | 1 + src/librustc_passes/rvalue_promotion.rs | 9 - src/librustc_typeck/check/_match.rs | 465 +++++++++++++------ src/librustc_typeck/check/coercion.rs | 8 +- src/librustc_typeck/check/mod.rs | 229 +-------- src/libsyntax_pos/hygiene.rs | 5 + 23 files changed, 436 insertions(+), 635 deletions(-) diff --git a/src/librustc/cfg/construct.rs b/src/librustc/cfg/construct.rs index 2592af7d4ad5a..2e54165be1f1b 100644 --- a/src/librustc/cfg/construct.rs +++ b/src/librustc/cfg/construct.rs @@ -166,47 +166,6 @@ impl<'a, 'tcx> CFGBuilder<'a, 'tcx> { self.add_ast_node(expr.hir_id.local_id, &[blk_exit]) } - hir::ExprKind::If(ref cond, ref then, None) => { - // - // [pred] - // | - // v 1 - // [cond] - // | - // / \ - // / \ - // v 2 * - // [then] | - // | | - // v 3 v 4 - // [..expr..] - // - let cond_exit = self.expr(&cond, pred); // 1 - let then_exit = self.expr(&then, cond_exit); // 2 - self.add_ast_node(expr.hir_id.local_id, &[cond_exit, then_exit]) // 3,4 - } - - hir::ExprKind::If(ref cond, ref then, Some(ref otherwise)) => { - // - // [pred] - // | - // v 1 - // [cond] - // | - // / \ - // / \ - // v 2 v 3 - // [then][otherwise] - // | | - // v 4 v 5 - // [..expr..] - // - let cond_exit = self.expr(&cond, pred); // 1 - let then_exit = self.expr(&then, cond_exit); // 2 - let else_exit = self.expr(&otherwise, cond_exit); // 3 - self.add_ast_node(expr.hir_id.local_id, &[then_exit, else_exit]) // 4, 5 - } - hir::ExprKind::While(ref cond, ref body, _) => { // // [pred] diff --git a/src/librustc/hir/intravisit.rs b/src/librustc/hir/intravisit.rs index 0c73d97394fda..38d6d710868c0 100644 --- a/src/librustc/hir/intravisit.rs +++ b/src/librustc/hir/intravisit.rs @@ -1032,11 +1032,6 @@ pub fn walk_expr<'v, V: Visitor<'v>>(visitor: &mut V, expression: &'v Expr) { ExprKind::DropTemps(ref subexpression) => { visitor.visit_expr(subexpression); } - ExprKind::If(ref head_expression, ref if_block, ref optional_else) => { - visitor.visit_expr(head_expression); - visitor.visit_expr(if_block); - walk_list!(visitor, visit_expr, optional_else); - } ExprKind::While(ref subexpression, ref block, ref opt_label) => { walk_list!(visitor, visit_label, opt_label); visitor.visit_expr(subexpression); diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 4d5707560b048..7ccfb826e37b7 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -62,6 +62,7 @@ use syntax::ext::hygiene::Mark; use syntax::print::pprust; use syntax::ptr::P; use syntax::source_map::{respan, CompilerDesugaringKind, Spanned}; +use syntax::source_map::CompilerDesugaringKind::IfTemporary; use syntax::std_inject; use syntax::symbol::{keywords, Symbol}; use syntax::tokenstream::{TokenStream, TokenTree}; @@ -4115,31 +4116,46 @@ impl<'a> LoweringContext<'a> { } // More complicated than you might expect because the else branch // might be `if let`. - ExprKind::If(ref cond, ref blk, ref else_opt) => { - let else_opt = else_opt.as_ref().map(|els| { - match els.node { + ExprKind::If(ref cond, ref then, ref else_opt) => { + // `true => then`: + let then_pat = self.pat_bool(e.span, true); + let then_blk = self.lower_block(then, false); + let then_expr = self.expr_block(then_blk, ThinVec::new()); + let then_arm = self.arm(hir_vec![then_pat], P(then_expr)); + + // `_ => else_block` where `else_block` is `{}` if there's `None`: + let else_pat = self.pat_wild(e.span); + let else_expr = match else_opt { + None => self.expr_block_empty(e.span), + Some(els) => match els.node { ExprKind::IfLet(..) => { // Wrap the `if let` expr in a block. - let span = els.span; - let els = P(self.lower_expr(els)); - let blk = P(hir::Block { - stmts: hir_vec![], - expr: Some(els), - hir_id: self.next_id(), - rules: hir::DefaultBlock, - span, - targeted_by_break: false, - }); - P(self.expr_block(blk, ThinVec::new())) + let els = self.lower_expr(els); + let blk = self.block_all(els.span, hir_vec![], Some(P(els))); + self.expr_block(P(blk), ThinVec::new()) } - _ => P(self.lower_expr(els)), + _ => self.lower_expr(els), } - }); - - let then_blk = self.lower_block(blk, false); - let then_expr = self.expr_block(then_blk, ThinVec::new()); + }; + let else_arm = self.arm(hir_vec![else_pat], P(else_expr)); + + // Lower condition: + let span_block = self + .sess + .source_map() + .mark_span_with_reason(IfTemporary, cond.span, None); + let cond = self.lower_expr(cond); + // Wrap in a construct equivalent to `{ let _t = $cond; _t }` to preserve drop + // semantics since `if cond { ... }` don't let temporaries live outside of `cond`. + let cond = self.expr_drop_temps(span_block, P(cond), ThinVec::new()); - hir::ExprKind::If(P(self.lower_expr(cond)), P(then_expr), else_opt) + hir::ExprKind::Match( + P(cond), + vec![then_arm, else_arm].into(), + hir::MatchSource::IfDesugar { + contains_else_clause: else_opt.is_some() + }, + ) } ExprKind::While(ref cond, ref body, opt_label) => self.with_loop_scope(e.id, |this| { hir::ExprKind::While( @@ -4486,16 +4502,16 @@ impl<'a> LoweringContext<'a> { arms.push(self.arm(pats, body_expr)); } - // _ => [|()] + // _ => [|{}] { let wildcard_arm: Option<&Expr> = else_opt.as_ref().map(|p| &**p); let wildcard_pattern = self.pat_wild(e.span); let body = if let Some(else_expr) = wildcard_arm { - P(self.lower_expr(else_expr)) + self.lower_expr(else_expr) } else { - P(self.expr_tuple(e.span, hir_vec![])) + self.expr_block_empty(e.span) }; - arms.push(self.arm(hir_vec![wildcard_pattern], body)); + arms.push(self.arm(hir_vec![wildcard_pattern], P(body))); } let contains_else_clause = else_opt.is_some(); @@ -4658,11 +4674,7 @@ impl<'a> LoweringContext<'a> { ThinVec::new(), )) }; - let match_stmt = hir::Stmt { - hir_id: self.next_id(), - node: hir::StmtKind::Expr(match_expr), - span: head_sp, - }; + let match_stmt = self.stmt(head_sp, hir::StmtKind::Expr(match_expr)); let next_expr = P(self.expr_ident(head_sp, next_ident, next_pat_hid)); @@ -4685,11 +4697,7 @@ impl<'a> LoweringContext<'a> { let body_block = self.with_loop_scope(e.id, |this| this.lower_block(body, false)); let body_expr = P(self.expr_block(body_block, ThinVec::new())); - let body_stmt = hir::Stmt { - hir_id: self.next_id(), - node: hir::StmtKind::Expr(body_expr), - span: body.span, - }; + let body_stmt = self.stmt(body.span, hir::StmtKind::Expr(body_expr)); let loop_block = P(self.block_all( e.span, @@ -4869,12 +4877,7 @@ impl<'a> LoweringContext<'a> { .into_iter() .map(|item_id| { let item_id = hir::ItemId { id: self.lower_node_id(item_id) }; - - hir::Stmt { - hir_id: self.next_id(), - node: hir::StmtKind::Item(item_id), - span: s.span, - } + self.stmt(s.span, hir::StmtKind::Item(item_id)) }) .collect(); ids.push({ @@ -5174,28 +5177,32 @@ impl<'a> LoweringContext<'a> { } } + fn stmt(&mut self, span: Span, node: hir::StmtKind) -> hir::Stmt { + hir::Stmt { span, node, hir_id: self.next_id() } + } + fn stmt_let_pat( &mut self, - sp: Span, - ex: Option>, + span: Span, + init: Option>, pat: P, source: hir::LocalSource, ) -> hir::Stmt { let local = hir::Local { pat, ty: None, - init: ex, + init, hir_id: self.next_id(), - span: sp, - attrs: ThinVec::new(), + span, source, + attrs: ThinVec::new() }; + self.stmt(span, hir::StmtKind::Local(P(local))) + } - hir::Stmt { - hir_id: self.next_id(), - node: hir::StmtKind::Local(P(local)), - span: sp - } + fn expr_block_empty(&mut self, span: Span) -> hir::Expr { + let blk = self.block_all(span, hir_vec![], None); + self.expr_block(P(blk), ThinVec::new()) } fn block_expr(&mut self, expr: P) -> hir::Block { @@ -5235,6 +5242,13 @@ impl<'a> LoweringContext<'a> { ) } + /// Constructs a `true` or `false` literal pattern. + fn pat_bool(&mut self, span: Span, val: bool) -> P { + let lit = Spanned { span, node: LitKind::Bool(val) }; + let expr = self.expr(span, hir::ExprKind::Lit(lit), ThinVec::new()); + self.pat(span, hir::PatKind::Lit(P(expr))) + } + fn pat_ok(&mut self, span: Span, pat: P) -> P { self.pat_std_enum(span, &["result", "Result", "Ok"], hir_vec![pat]) } @@ -5622,15 +5636,7 @@ impl<'a> LoweringContext<'a> { &["task", "Poll", "Pending"], hir_vec![], ); - let empty_block = P(hir::Block { - stmts: hir_vec![], - expr: None, - hir_id: self.next_id(), - rules: hir::DefaultBlock, - span, - targeted_by_break: false, - }); - let empty_block = P(self.expr_block(empty_block, ThinVec::new())); + let empty_block = P(self.expr_block_empty(span)); self.arm(hir_vec![pending_pat], empty_block) }; diff --git a/src/librustc/hir/mod.rs b/src/librustc/hir/mod.rs index 01de7917e6e23..9f7fa6c5557ef 100644 --- a/src/librustc/hir/mod.rs +++ b/src/librustc/hir/mod.rs @@ -1368,7 +1368,6 @@ impl Expr { ExprKind::Lit(_) => ExprPrecedence::Lit, ExprKind::Type(..) | ExprKind::Cast(..) => ExprPrecedence::Cast, ExprKind::DropTemps(ref expr, ..) => expr.precedence(), - ExprKind::If(..) => ExprPrecedence::If, ExprKind::While(..) => ExprPrecedence::While, ExprKind::Loop(..) => ExprPrecedence::Loop, ExprKind::Match(..) => ExprPrecedence::Match, @@ -1421,7 +1420,6 @@ impl Expr { ExprKind::MethodCall(..) | ExprKind::Struct(..) | ExprKind::Tup(..) | - ExprKind::If(..) | ExprKind::Match(..) | ExprKind::Closure(..) | ExprKind::Block(..) | @@ -1498,10 +1496,6 @@ pub enum ExprKind { /// This construct only exists to tweak the drop order in HIR lowering. /// An example of that is the desugaring of `for` loops. DropTemps(P), - /// An `if` block, with an optional else block. - /// - /// I.e., `if { } else { }`. - If(P, P, Option>), /// A while loop, with an optional label /// /// I.e., `'label: while expr { }`. @@ -1615,6 +1609,10 @@ pub enum LocalSource { pub enum MatchSource { /// A `match _ { .. }`. Normal, + /// An `if _ { .. }` (optionally with `else { .. }`). + IfDesugar { + contains_else_clause: bool, + }, /// An `if let _ = _ { .. }` (optionally with `else { .. }`). IfLetDesugar { contains_else_clause: bool, diff --git a/src/librustc/hir/print.rs b/src/librustc/hir/print.rs index 54816316f0bf5..c42d8f3cb3c36 100644 --- a/src/librustc/hir/print.rs +++ b/src/librustc/hir/print.rs @@ -1093,65 +1093,6 @@ impl<'a> State<'a> { self.ann.post(self, AnnNode::Block(blk)) } - fn print_else(&mut self, els: Option<&hir::Expr>) -> io::Result<()> { - match els { - Some(_else) => { - match _else.node { - // "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) - } - // BLEAH, constraints would be great here - _ => { - panic!("print_if saw if with weird alternative"); - } - } - } - _ => Ok(()), - } - } - - pub fn print_if(&mut self, - test: &hir::Expr, - blk: &hir::Expr, - elseopt: Option<&hir::Expr>) - -> io::Result<()> { - self.head("if")?; - self.print_expr_as_cond(test)?; - self.s.space()?; - self.print_expr(blk)?; - self.print_else(elseopt) - } - - pub fn print_if_let(&mut self, - pat: &hir::Pat, - expr: &hir::Expr, - blk: &hir::Block, - elseopt: Option<&hir::Expr>) - -> io::Result<()> { - self.head("if let")?; - self.print_pat(pat)?; - self.s.space()?; - self.word_space("=")?; - self.print_expr_as_cond(expr)?; - self.s.space()?; - self.print_block(blk)?; - self.print_else(elseopt) - } - pub fn print_anon_const(&mut self, constant: &hir::AnonConst) -> io::Result<()> { self.ann.nested(self, Nested::Body(constant.body)) } @@ -1406,9 +1347,6 @@ impl<'a> State<'a> { // Print `}`: self.bclose_maybe_open(expr.span, indent_unit, true)?; } - hir::ExprKind::If(ref test, ref blk, ref elseopt) => { - self.print_if(&test, &blk, elseopt.as_ref().map(|e| &**e))?; - } hir::ExprKind::While(ref test, ref blk, opt_label) => { if let Some(label) = opt_label { self.print_ident(label.ident)?; @@ -2414,7 +2352,6 @@ impl<'a> State<'a> { /// isn't parsed as (if true {...} else {...} | x) | 5 fn expr_requires_semi_to_be_stmt(e: &hir::Expr) -> bool { match e.node { - hir::ExprKind::If(..) | hir::ExprKind::Match(..) | hir::ExprKind::Block(..) | hir::ExprKind::While(..) | diff --git a/src/librustc/ich/impls_syntax.rs b/src/librustc/ich/impls_syntax.rs index 90dd5099cbfd6..35df43ef25efa 100644 --- a/src/librustc/ich/impls_syntax.rs +++ b/src/librustc/ich/impls_syntax.rs @@ -395,6 +395,7 @@ impl_stable_hash_for!(enum ::syntax_pos::hygiene::ExpnFormat { }); impl_stable_hash_for!(enum ::syntax_pos::hygiene::CompilerDesugaringKind { + IfTemporary, Async, Await, QuestionMark, diff --git a/src/librustc/middle/expr_use_visitor.rs b/src/librustc/middle/expr_use_visitor.rs index 93ba4241c4725..d46bba92f3fc9 100644 --- a/src/librustc/middle/expr_use_visitor.rs +++ b/src/librustc/middle/expr_use_visitor.rs @@ -424,14 +424,6 @@ impl<'a, 'gcx, 'tcx> ExprUseVisitor<'a, 'gcx, 'tcx> { self.consume_exprs(exprs); } - hir::ExprKind::If(ref cond_expr, ref then_expr, ref opt_else_expr) => { - self.consume_expr(&cond_expr); - self.walk_expr(&then_expr); - if let Some(ref else_expr) = *opt_else_expr { - self.consume_expr(&else_expr); - } - } - hir::ExprKind::Match(ref discr, ref arms, _) => { let discr_cmt = Rc::new(return_if_err!(self.mc.cat_expr(&discr))); let r = self.tcx().lifetimes.re_empty; diff --git a/src/librustc/middle/liveness.rs b/src/librustc/middle/liveness.rs index 4b458e474b299..a142b220f31af 100644 --- a/src/librustc/middle/liveness.rs +++ b/src/librustc/middle/liveness.rs @@ -500,7 +500,6 @@ fn visit_expr<'a, 'tcx>(ir: &mut IrMaps<'a, 'tcx>, expr: &'tcx Expr) { } // live nodes required for interesting control flow: - hir::ExprKind::If(..) | hir::ExprKind::Match(..) | hir::ExprKind::While(..) | hir::ExprKind::Loop(..) => { @@ -1040,28 +1039,6 @@ impl<'a, 'tcx> Liveness<'a, 'tcx> { }) } - hir::ExprKind::If(ref cond, ref then, ref els) => { - // - // (cond) - // | - // v - // (expr) - // / \ - // | | - // v v - // (then)(els) - // | | - // v v - // ( succ ) - // - let else_ln = self.propagate_through_opt_expr(els.as_ref().map(|e| &**e), succ); - let then_ln = self.propagate_through_expr(&then, succ); - let ln = self.live_node(expr.hir_id, expr.span); - self.init_from_succ(ln, else_ln); - self.merge_from_succ(ln, then_ln, false); - self.propagate_through_expr(&cond, ln) - } - hir::ExprKind::While(ref cond, ref blk, _) => { self.propagate_through_loop(expr, WhileLoop(&cond), &blk, succ) } @@ -1523,7 +1500,7 @@ fn check_expr<'a, 'tcx>(this: &mut Liveness<'a, 'tcx>, expr: &'tcx Expr) { } // no correctness conditions related to liveness - hir::ExprKind::Call(..) | hir::ExprKind::MethodCall(..) | hir::ExprKind::If(..) | + hir::ExprKind::Call(..) | hir::ExprKind::MethodCall(..) | hir::ExprKind::Match(..) | hir::ExprKind::While(..) | hir::ExprKind::Loop(..) | hir::ExprKind::Index(..) | hir::ExprKind::Field(..) | hir::ExprKind::Array(..) | hir::ExprKind::Tup(..) | hir::ExprKind::Binary(..) | diff --git a/src/librustc/middle/mem_categorization.rs b/src/librustc/middle/mem_categorization.rs index 7edd5c5a9de1b..85c7f43790336 100644 --- a/src/librustc/middle/mem_categorization.rs +++ b/src/librustc/middle/mem_categorization.rs @@ -678,7 +678,7 @@ impl<'a, 'gcx, 'tcx> MemCategorizationContext<'a, 'gcx, 'tcx> { hir::ExprKind::Closure(..) | hir::ExprKind::Ret(..) | hir::ExprKind::Unary(..) | hir::ExprKind::Yield(..) | hir::ExprKind::MethodCall(..) | hir::ExprKind::Cast(..) | hir::ExprKind::DropTemps(..) | - hir::ExprKind::Array(..) | hir::ExprKind::Tup(..) | hir::ExprKind::If(..) | + hir::ExprKind::Array(..) | hir::ExprKind::Tup(..) | hir::ExprKind::Binary(..) | hir::ExprKind::While(..) | hir::ExprKind::Block(..) | hir::ExprKind::Loop(..) | hir::ExprKind::Match(..) | hir::ExprKind::Lit(..) | hir::ExprKind::Break(..) | diff --git a/src/librustc/middle/region.rs b/src/librustc/middle/region.rs index 2b88f273adce4..8ef4e9ac8f45f 100644 --- a/src/librustc/middle/region.rs +++ b/src/librustc/middle/region.rs @@ -884,17 +884,6 @@ fn resolve_expr<'a, 'tcx>(visitor: &mut RegionResolutionVisitor<'a, 'tcx>, expr: terminating(r.hir_id.local_id); } - hir::ExprKind::If(ref expr, ref then, Some(ref otherwise)) => { - terminating(expr.hir_id.local_id); - terminating(then.hir_id.local_id); - terminating(otherwise.hir_id.local_id); - } - - hir::ExprKind::If(ref expr, ref then, None) => { - terminating(expr.hir_id.local_id); - terminating(then.hir_id.local_id); - } - hir::ExprKind::Loop(ref body, _, _) => { terminating(body.hir_id.local_id); } diff --git a/src/librustc_errors/diagnostic_builder.rs b/src/librustc_errors/diagnostic_builder.rs index f74dcd6070c70..31f697a724a03 100644 --- a/src/librustc_errors/diagnostic_builder.rs +++ b/src/librustc_errors/diagnostic_builder.rs @@ -100,6 +100,18 @@ impl<'a> DiagnosticBuilder<'a> { self.cancel(); } + /// Emit the diagnostic unless `delay` is true, + /// in which case the emission will be delayed as a bug. + /// + /// See `emit` and `delay_as_bug` for details. + pub fn emit_unless(&mut self, delay: bool) { + if delay { + self.delay_as_bug() + } else { + self.emit() + } + } + /// Buffers the diagnostic for later emission, unless handler /// has disabled such buffering. pub fn buffer(mut self, buffered_diagnostics: &mut Vec) { diff --git a/src/librustc_mir/build/expr/as_place.rs b/src/librustc_mir/build/expr/as_place.rs index 713e3fe0218ba..5f444d4ceeb88 100644 --- a/src/librustc_mir/build/expr/as_place.rs +++ b/src/librustc_mir/build/expr/as_place.rs @@ -192,7 +192,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { | ExprKind::Pointer { .. } | ExprKind::Repeat { .. } | ExprKind::Borrow { .. } - | ExprKind::If { .. } | ExprKind::Match { .. } | ExprKind::Loop { .. } | ExprKind::Block { .. } diff --git a/src/librustc_mir/build/expr/as_rvalue.rs b/src/librustc_mir/build/expr/as_rvalue.rs index e745ee0f190dd..fbc4835a6557b 100644 --- a/src/librustc_mir/build/expr/as_rvalue.rs +++ b/src/librustc_mir/build/expr/as_rvalue.rs @@ -345,7 +345,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { ExprKind::Literal { .. } | ExprKind::Block { .. } | ExprKind::Match { .. } - | ExprKind::If { .. } | ExprKind::NeverToAny { .. } | ExprKind::Use { .. } | ExprKind::Loop { .. } diff --git a/src/librustc_mir/build/expr/category.rs b/src/librustc_mir/build/expr/category.rs index 4e24b6853d6eb..222ce6d1c968e 100644 --- a/src/librustc_mir/build/expr/category.rs +++ b/src/librustc_mir/build/expr/category.rs @@ -45,7 +45,6 @@ impl Category { | ExprKind::ValueTypeAscription { .. } => Some(Category::Place), ExprKind::LogicalOp { .. } - | ExprKind::If { .. } | ExprKind::Match { .. } | ExprKind::NeverToAny { .. } | ExprKind::Use { .. } diff --git a/src/librustc_mir/build/expr/into.rs b/src/librustc_mir/build/expr/into.rs index 30ed9cef36f7b..15795a64e3b7d 100644 --- a/src/librustc_mir/build/expr/into.rs +++ b/src/librustc_mir/build/expr/into.rs @@ -76,43 +76,6 @@ impl<'a, 'gcx, 'tcx> Builder<'a, 'gcx, 'tcx> { end_block.unit() } } - ExprKind::If { - condition: cond_expr, - then: then_expr, - otherwise: else_expr, - } => { - let operand = unpack!(block = this.as_local_operand(block, cond_expr)); - - let mut then_block = this.cfg.start_new_block(); - let mut else_block = this.cfg.start_new_block(); - let term = TerminatorKind::if_(this.hir.tcx(), operand, then_block, else_block); - this.cfg.terminate(block, source_info, term); - - unpack!(then_block = this.into(destination, then_block, then_expr)); - else_block = if let Some(else_expr) = else_expr { - unpack!(this.into(destination, else_block, else_expr)) - } else { - // Body of the `if` expression without an `else` clause must return `()`, thus - // we implicitly generate a `else {}` if it is not specified. - this.cfg - .push_assign_unit(else_block, source_info, destination); - else_block - }; - - let join_block = this.cfg.start_new_block(); - this.cfg.terminate( - then_block, - source_info, - TerminatorKind::Goto { target: join_block }, - ); - this.cfg.terminate( - else_block, - source_info, - TerminatorKind::Goto { target: join_block }, - ); - - join_block.unit() - } ExprKind::LogicalOp { op, lhs, rhs } => { // And: // diff --git a/src/librustc_mir/hair/cx/expr.rs b/src/librustc_mir/hair/cx/expr.rs index 5e646a49e0e42..50140880a368d 100644 --- a/src/librustc_mir/hair/cx/expr.rs +++ b/src/librustc_mir/hair/cx/expr.rs @@ -601,13 +601,6 @@ fn make_mirror_unadjusted<'a, 'gcx, 'tcx>(cx: &mut Cx<'a, 'gcx, 'tcx>, arms: arms.iter().map(|a| convert_arm(cx, a)).collect(), } } - hir::ExprKind::If(ref cond, ref then, ref otherwise) => { - ExprKind::If { - condition: cond.to_ref(), - then: then.to_ref(), - otherwise: otherwise.to_ref(), - } - } hir::ExprKind::While(ref cond, ref body, _) => { ExprKind::Loop { condition: Some(cond.to_ref()), diff --git a/src/librustc_mir/hair/mod.rs b/src/librustc_mir/hair/mod.rs index 2e53bee3f3d7f..d4f139e103a64 100644 --- a/src/librustc_mir/hair/mod.rs +++ b/src/librustc_mir/hair/mod.rs @@ -185,11 +185,6 @@ pub enum ExprKind<'tcx> { cast: PointerCast, source: ExprRef<'tcx>, }, - If { - condition: ExprRef<'tcx>, - then: ExprRef<'tcx>, - otherwise: Option>, - }, Loop { condition: Option>, body: ExprRef<'tcx>, diff --git a/src/librustc_mir/hair/pattern/check_match.rs b/src/librustc_mir/hair/pattern/check_match.rs index 1a7266859ad9f..b58f8dd492424 100644 --- a/src/librustc_mir/hair/pattern/check_match.rs +++ b/src/librustc_mir/hair/pattern/check_match.rs @@ -365,6 +365,7 @@ fn check_arms<'a, 'tcx>( match is_useful(cx, &seen, &v, LeaveOutWitness) { NotUseful => { match source { + hir::MatchSource::IfDesugar { .. } => bug!(), hir::MatchSource::IfLetDesugar { .. } => { cx.tcx.lint_hir( lint::builtin::IRREFUTABLE_LET_PATTERNS, diff --git a/src/librustc_passes/rvalue_promotion.rs b/src/librustc_passes/rvalue_promotion.rs index 0f651fafcd2ac..2d35a0de7953f 100644 --- a/src/librustc_passes/rvalue_promotion.rs +++ b/src/librustc_passes/rvalue_promotion.rs @@ -518,15 +518,6 @@ fn check_expr_kind<'a, 'tcx>( NotPromotable } - hir::ExprKind::If(ref lhs, ref rhs, ref option_expr) => { - let _ = v.check_expr(lhs); - let _ = v.check_expr(rhs); - if let Some(ref expr) = option_expr { - let _ = v.check_expr(&expr); - } - NotPromotable - } - // Loops (not very meaningful in constants). hir::ExprKind::While(ref expr, ref box_block, ref _option_label) => { let _ = v.check_expr(expr); diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index e9e202ce37996..e8af2f40b87a7 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -2,12 +2,12 @@ use crate::check::{FnCtxt, Expectation, Diverges, Needs}; use crate::check::coercion::CoerceMany; use crate::util::nodemap::FxHashMap; use errors::{Applicability, DiagnosticBuilder}; -use rustc::hir::{self, PatKind, Pat}; +use rustc::hir::{self, PatKind, Pat, ExprKind}; use rustc::hir::def::{Res, DefKind, CtorKind}; use rustc::hir::pat_util::EnumerateAndAdjustIterator; use rustc::infer; use rustc::infer::type_variable::TypeVariableOrigin; -use rustc::traits::ObligationCauseCode; +use rustc::traits::{ObligationCause, ObligationCauseCode}; use rustc::ty::{self, Ty, TypeFoldable}; use rustc::ty::subst::Kind; use syntax::ast; @@ -15,6 +15,7 @@ use syntax::source_map::Spanned; use syntax::ptr::P; use syntax::util::lev_distance::find_best_match_for_name; use syntax_pos::Span; +use syntax_pos::hygiene::CompilerDesugaringKind; use std::collections::hash_map::Entry::{Occupied, Vacant}; use std::cmp; @@ -22,10 +23,9 @@ use std::cmp; use super::report_unexpected_variant_res; impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { - /// `match_discrim_span` argument having a `Span` indicates that this pattern is part of - /// a match expression arm guard, and it points to the match discriminant to add context - /// in type errors. In the folloowing example, `match_discrim_span` corresponds to the - /// `a + b` expression: + /// `discrim_span` argument having a `Span` indicates that this pattern is part of a match + /// expression arm guard, and it points to the match discriminant to add context in type errors. + /// In the following example, `discrim_span` corresponds to the `a + b` expression: /// /// ```text /// error[E0308]: mismatched types @@ -44,7 +44,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { pat: &'gcx hir::Pat, mut expected: Ty<'tcx>, mut def_bm: ty::BindingMode, - match_discrim_span: Option, + discrim_span: Option, ) { let tcx = self.tcx; @@ -176,7 +176,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // &'static str <: expected // // that's equivalent to there existing a LUB. - self.demand_suptype(pat.span, expected, pat_ty); + if let Some(mut err) = self.demand_suptype_diag(pat.span, expected, pat_ty) { + err.emit_unless(discrim_span + .filter(|&s| s.is_compiler_desugaring(CompilerDesugaringKind::IfTemporary)) + .is_some()); + } + pat_ty } PatKind::Range(ref begin, ref end, _) => { @@ -224,8 +229,8 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { let common_type = self.resolve_type_vars_if_possible(&lhs_ty); // subtyping doesn't matter here, as the value is some kind of scalar - self.demand_eqtype_pat(pat.span, expected, lhs_ty, match_discrim_span); - self.demand_eqtype_pat(pat.span, expected, rhs_ty, match_discrim_span); + self.demand_eqtype_pat(pat.span, expected, lhs_ty, discrim_span); + self.demand_eqtype_pat(pat.span, expected, rhs_ty, discrim_span); common_type } PatKind::Binding(ba, var_id, _, ref sub) => { @@ -254,13 +259,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // `x` is assigned a value of type `&M T`, hence `&M T <: typeof(x)` is // required. However, we use equality, which is stronger. See (*) for // an explanation. - self.demand_eqtype_pat(pat.span, region_ty, local_ty, match_discrim_span); + self.demand_eqtype_pat(pat.span, region_ty, local_ty, discrim_span); } // otherwise the type of x is the expected type T ty::BindByValue(_) => { // As above, `T <: typeof(x)` is required but we // use equality, see (*) below. - self.demand_eqtype_pat(pat.span, expected, local_ty, match_discrim_span); + self.demand_eqtype_pat(pat.span, expected, local_ty, discrim_span); } } @@ -268,11 +273,11 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // what the type of the binding `x` ought to be if var_id != pat.hir_id { let vt = self.local_ty(pat.span, var_id).decl_ty; - self.demand_eqtype_pat(pat.span, vt, local_ty, match_discrim_span); + self.demand_eqtype_pat(pat.span, vt, local_ty, discrim_span); } if let Some(ref p) = *sub { - self.check_pat_walk(&p, expected, def_bm, match_discrim_span); + self.check_pat_walk(&p, expected, def_bm, discrim_span); } local_ty @@ -285,14 +290,14 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { ddpos, expected, def_bm, - match_discrim_span, + discrim_span, ) } PatKind::Path(ref qpath) => { self.check_pat_path(pat, qpath, expected) } PatKind::Struct(ref qpath, ref fields, etc) => { - self.check_pat_struct(pat, qpath, fields, etc, expected, def_bm, match_discrim_span) + self.check_pat_struct(pat, qpath, fields, etc, expected, def_bm, discrim_span) } PatKind::Tuple(ref elements, ddpos) => { let mut expected_len = elements.len(); @@ -318,7 +323,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // further errors being emitted when using the bindings. #50333 let element_tys_iter = (0..max_len).map(|_| tcx.types.err); for (_, elem) in elements.iter().enumerate_and_adjust(max_len, ddpos) { - self.check_pat_walk(elem, &tcx.types.err, def_bm, match_discrim_span); + self.check_pat_walk(elem, &tcx.types.err, def_bm, discrim_span); } tcx.mk_tup(element_tys_iter) } else { @@ -327,7 +332,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { elem, &element_tys[i].expect_ty(), def_bm, - match_discrim_span, + discrim_span, ); } pat_ty @@ -341,11 +346,11 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { // Here, `demand::subtype` is good enough, but I don't // think any errors can be introduced by using // `demand::eqtype`. - self.demand_eqtype_pat(pat.span, expected, uniq_ty, match_discrim_span); - self.check_pat_walk(&inner, inner_ty, def_bm, match_discrim_span); + self.demand_eqtype_pat(pat.span, expected, uniq_ty, discrim_span); + self.check_pat_walk(&inner, inner_ty, def_bm, discrim_span); uniq_ty } else { - self.check_pat_walk(&inner, tcx.types.err, def_bm, match_discrim_span); + self.check_pat_walk(&inner, tcx.types.err, def_bm, discrim_span); tcx.types.err } } @@ -384,10 +389,10 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } }; - self.check_pat_walk(&inner, inner_ty, def_bm, match_discrim_span); + self.check_pat_walk(&inner, inner_ty, def_bm, discrim_span); rptr_ty } else { - self.check_pat_walk(&inner, tcx.types.err, def_bm, match_discrim_span); + self.check_pat_walk(&inner, tcx.types.err, def_bm, discrim_span); tcx.types.err } } @@ -445,13 +450,13 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { }; for elt in before { - self.check_pat_walk(&elt, inner_ty, def_bm, match_discrim_span); + self.check_pat_walk(&elt, inner_ty, def_bm, discrim_span); } if let Some(ref slice) = *slice { - self.check_pat_walk(&slice, slice_ty, def_bm, match_discrim_span); + self.check_pat_walk(&slice, slice_ty, def_bm, discrim_span); } for elt in after { - self.check_pat_walk(&elt, inner_ty, def_bm, match_discrim_span); + self.check_pat_walk(&elt, inner_ty, def_bm, discrim_span); } expected_ty } @@ -595,73 +600,24 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); ) -> Ty<'tcx> { let tcx = self.tcx; - // Not entirely obvious: if matches may create ref bindings, we want to - // use the *precise* type of the discriminant, *not* some supertype, as - // the "discriminant type" (issue #23116). - // - // arielb1 [writes here in this comment thread][c] that there - // is certainly *some* potential danger, e.g., for an example - // like: - // - // [c]: https://github.com/rust-lang/rust/pull/43399#discussion_r130223956 - // - // ``` - // let Foo(x) = f()[0]; - // ``` - // - // Then if the pattern matches by reference, we want to match - // `f()[0]` as a lexpr, so we can't allow it to be - // coerced. But if the pattern matches by value, `f()[0]` is - // still syntactically a lexpr, but we *do* want to allow - // coercions. - // - // However, *likely* we are ok with allowing coercions to - // happen if there are no explicit ref mut patterns - all - // implicit ref mut patterns must occur behind a reference, so - // they will have the "correct" variance and lifetime. - // - // This does mean that the following pattern would be legal: - // - // ``` - // struct Foo(Bar); - // struct Bar(u32); - // impl Deref for Foo { - // type Target = Bar; - // fn deref(&self) -> &Bar { &self.0 } - // } - // impl DerefMut for Foo { - // fn deref_mut(&mut self) -> &mut Bar { &mut self.0 } - // } - // fn foo(x: &mut Foo) { - // { - // let Bar(z): &mut Bar = x; - // *z = 42; - // } - // assert_eq!(foo.0.0, 42); - // } - // ``` - // - // FIXME(tschottdorf): don't call contains_explicit_ref_binding, which - // is problematic as the HIR is being scraped, but ref bindings may be - // implicit after #42640. We need to make sure that pat_adjustments - // (once introduced) is populated by the time we get here. - // - // See #44848. - let contains_ref_bindings = arms.iter() - .filter_map(|a| a.contains_explicit_ref_binding()) - .max_by_key(|m| match *m { - hir::MutMutable => 1, - hir::MutImmutable => 0, - }); - let discrim_ty; - if let Some(m) = contains_ref_bindings { - discrim_ty = self.check_expr_with_needs(discrim, Needs::maybe_mut_place(m)); + use hir::MatchSource::*; + let (source_if, if_no_else, if_desugar) = match match_src { + IfDesugar { contains_else_clause } => (true, !contains_else_clause, true), + IfLetDesugar { contains_else_clause } => (true, !contains_else_clause, false), + _ => (false, false, false), + }; + + // Type check the descriminant and get its type. + let discrim_ty = if if_desugar { + // 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 { .. }`. + self.check_expr_has_type_or_error(discrim, self.tcx.types.bool) } else { - // ...but otherwise we want to use any supertype of the - // discriminant. This is sort of a workaround, see note (*) in - // `check_pat` for some details. - discrim_ty = self.next_ty_var(TypeVariableOrigin::TypeInference(discrim.span)); - self.check_expr_has_type_or_error(discrim, discrim_ty); + self.demand_discriminant_type(arms, discrim) }; // If there are no arms, that is a diverging match; a special case. @@ -670,11 +626,7 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); return tcx.types.never; } - if self.diverges.get().always() { - for arm in arms { - self.warn_if_unreachable(arm.body.hir_id, arm.body.span, "arm"); - } - } + self.warn_arms_when_scrutinee_diverges(arms, source_if); // Otherwise, we have to union together the types that the // arms produce and so forth. @@ -687,12 +639,8 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); let mut all_pats_diverge = Diverges::WarnedAlways; for p in &arm.pats { self.diverges.set(Diverges::Maybe); - self.check_pat_walk( - &p, - discrim_ty, - ty::BindingMode::BindByValue(hir::Mutability::MutImmutable), - Some(discrim.span), - ); + let binding_mode = ty::BindingMode::BindByValue(hir::Mutability::MutImmutable); + self.check_pat_walk(&p, discrim_ty, binding_mode, Some(discrim.span)); all_pats_diverge &= self.diverges.get(); } @@ -734,7 +682,7 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); let mut other_arms = vec![]; // used only for diagnostics let mut prior_arm_ty = None; for (i, (arm, pats_diverge)) in arms.iter().zip(all_arm_pats_diverge).enumerate() { - if let Some(ref g) = arm.guard { + if let Some(g) = &arm.guard { self.diverges.set(pats_diverge); match g { hir::Guard::If(e) => self.check_expr_has_type_or_error(e, tcx.types.bool), @@ -745,43 +693,44 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); let arm_ty = self.check_expr_with_expectation(&arm.body, expected); all_arms_diverge &= self.diverges.get(); - // Handle the fallback arm of a desugared if-let like a missing else. - let is_if_let_fallback = match match_src { - hir::MatchSource::IfLetDesugar { contains_else_clause: false } => { - i == arms.len() - 1 && arm_ty.is_unit() + let span = expr.span; + + if source_if { + let then_expr = &arms[0].body; + match (i, if_no_else) { + (0, _) => coercion.coerce(self, &self.misc(span), then_expr, arm_ty), + (_, true) => self.if_fallback_coercion(span, then_expr, &mut coercion), + (_, _) => { + let then_ty = prior_arm_ty.unwrap(); + let cause = self.if_cause(span, then_expr, &arm.body, then_ty, arm_ty); + coercion.coerce(self, &cause, &arm.body, arm_ty); + } } - _ => false - }; - - let arm_span = if let hir::ExprKind::Block(ref blk, _) = arm.body.node { - // Point at the block expr instead of the entire block - blk.expr.as_ref().map(|e| e.span).unwrap_or(arm.body.span) } else { - arm.body.span - }; - if is_if_let_fallback { - let cause = self.cause(expr.span, ObligationCauseCode::IfExpressionWithNoElse); - assert!(arm_ty.is_unit()); - coercion.coerce_forced_unit(self, &cause, &mut |_| (), true); - } else { - let cause = if i == 0 { + let arm_span = if let hir::ExprKind::Block(blk, _) = &arm.body.node { + // Point at the block expr instead of the entire block + blk.expr.as_ref().map(|e| e.span).unwrap_or(arm.body.span) + } else { + arm.body.span + }; + 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. - self.cause(arm_span, ObligationCauseCode::BlockTailExpression(arm.body.hir_id)) - } else { - self.cause(expr.span, ObligationCauseCode::MatchExpressionArm { + 0 => (arm_span, ObligationCauseCode::BlockTailExpression(arm.body.hir_id)), + _ => (span, ObligationCauseCode::MatchExpressionArm { arm_span, source: match_src, prior_arms: other_arms.clone(), last_ty: prior_arm_ty.unwrap(), discrim_hir_id: discrim.hir_id, - }) + }), }; + let cause = self.cause(span, code); coercion.coerce(self, &cause, &arm.body, arm_ty); - } - 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); } @@ -792,6 +741,251 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); coercion.complete(self) } + /// 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: &'gcx [hir::Arm], source_if: bool) { + if self.diverges.get().always() { + let msg = if source_if { "block in `if` expression" } else { "arm" }; + for arm in arms { + self.warn_if_unreachable(arm.body.hir_id, arm.body.span, msg); + } + } + } + + /// Handle the fallback arm of a desugared if(-let) like a missing else. + fn if_fallback_coercion( + &self, + span: Span, + then_expr: &'gcx hir::Expr, + coercion: &mut CoerceMany<'gcx, 'tcx, '_, rustc::hir::Arm>, + ) { + // 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 = self.maybe_get_coercion_reason(then_expr.hir_id, span); + let cause = self.cause(span, ObligationCauseCode::IfExpressionWithNoElse); + coercion.coerce_forced_unit(self, &cause, &mut |err| { + if let Some((span, msg)) = &ret_reason { + err.span_label(*span, msg.as_str()); + } else if let ExprKind::Block(block, _) = &then_expr.node { + if let Some(expr) = &block.expr { + err.span_label(expr.span, "found here".to_string()); + } + } + err.note("`if` expressions without `else` evaluate to `()`"); + err.help("consider adding an `else` block that evaluates to the expected type"); + }, ret_reason.is_none()); + } + + fn maybe_get_coercion_reason(&self, hir_id: hir::HirId, span: Span) -> Option<(Span, String)> { + use hir::Node::{Block, Item, Local}; + + let node = self.tcx.hir().get_by_hir_id(self.tcx.hir().get_parent_node_by_hir_id( + self.tcx.hir().get_parent_node_by_hir_id(hir_id), + )); + if let Block(block) = node { + // check that the body's parent is an fn + let parent = self.tcx.hir().get_by_hir_id( + self.tcx.hir().get_parent_node_by_hir_id( + self.tcx.hir().get_parent_node_by_hir_id(block.hir_id), + ), + ); + if let (Some(expr), Item(hir::Item { + node: hir::ItemKind::Fn(..), .. + })) = (&block.expr, parent) { + // check that the `if` expr without `else` is the fn body's expr + if expr.span == span { + return self.get_fn_decl(hir_id).map(|(fn_decl, _)| ( + fn_decl.output.span(), + format!("expected `{}` because of this return type", fn_decl.output), + )); + } + } + } + if let Local(hir::Local { ty: Some(_), pat, .. }) = node { + return Some((pat.span, "expected because of this assignment".to_string())); + } + None + } + + fn if_cause( + &self, + span: Span, + then_expr: &'gcx hir::Expr, + else_expr: &'gcx hir::Expr, + then_ty: Ty<'tcx>, + else_ty: Ty<'tcx>, + ) -> ObligationCause<'tcx> { + let mut outer_sp = if self.tcx.sess.source_map().is_multiline(span) { + // The `if`/`else` isn't in one line in the output, include some context to make it + // clear it is an if/else expression: + // ``` + // LL | let x = if true { + // | _____________- + // LL || 10i32 + // || ----- expected because of this + // LL || } else { + // LL || 10u32 + // || ^^^^^ expected i32, found u32 + // LL || }; + // ||_____- if and else have incompatible types + // ``` + Some(span) + } else { + // The entire expression is in one line, only point at the arms + // ``` + // LL | let x = if true { 10i32 } else { 10u32 }; + // | ----- ^^^^^ expected i32, found u32 + // | | + // | expected because of this + // ``` + None + }; + + let mut remove_semicolon = None; + let error_sp = if let ExprKind::Block(block, _) = &else_expr.node { + if let Some(expr) = &block.expr { + expr.span + } else if let Some(stmt) = block.stmts.last() { + // possibly incorrect trailing `;` in the else arm + remove_semicolon = self.could_remove_semicolon(block, then_ty); + stmt.span + } else { // empty block, point at its entirety + // Avoid overlapping spans that aren't as readable: + // ``` + // 2 | let x = if true { + // | _____________- + // 3 | | 3 + // | | - expected because of this + // 4 | | } else { + // | |____________^ + // 5 | || + // 6 | || }; + // | || ^ + // | ||_____| + // | |______if and else have incompatible types + // | expected integer, found () + // ``` + // by not pointing at the entire expression: + // ``` + // 2 | let x = if true { + // | ------- if and else have incompatible types + // 3 | 3 + // | - expected because of this + // 4 | } else { + // | ____________^ + // 5 | | + // 6 | | }; + // | |_____^ expected integer, found () + // ``` + if outer_sp.is_some() { + outer_sp = Some(self.tcx.sess.source_map().def_span(span)); + } + else_expr.span + } + } else { // shouldn't happen unless the parser has done something weird + else_expr.span + }; + + // Compute `Span` of `then` part of `if`-expression: + let then_sp = if let ExprKind::Block(block, _) = &then_expr.node { + if let Some(expr) = &block.expr { + expr.span + } else if let Some(stmt) = block.stmts.last() { + // possibly incorrect trailing `;` in the else arm + remove_semicolon = remove_semicolon.or(self.could_remove_semicolon(block, else_ty)); + stmt.span + } else { // empty block, point at its entirety + outer_sp = None; // same as in `error_sp`, cleanup output + then_expr.span + } + } else { // shouldn't happen unless the parser has done something weird + then_expr.span + }; + + // Finally construct the cause: + self.cause(error_sp, ObligationCauseCode::IfExpression { + then: then_sp, + outer: outer_sp, + semicolon: remove_semicolon, + }) + } + + fn demand_discriminant_type( + &self, + arms: &'gcx [hir::Arm], + discrim: &'gcx hir::Expr, + ) -> Ty<'tcx> { + // Not entirely obvious: if matches may create ref bindings, we want to + // use the *precise* type of the discriminant, *not* some supertype, as + // the "discriminant type" (issue #23116). + // + // arielb1 [writes here in this comment thread][c] that there + // is certainly *some* potential danger, e.g., for an example + // like: + // + // [c]: https://github.com/rust-lang/rust/pull/43399#discussion_r130223956 + // + // ``` + // let Foo(x) = f()[0]; + // ``` + // + // Then if the pattern matches by reference, we want to match + // `f()[0]` as a lexpr, so we can't allow it to be + // coerced. But if the pattern matches by value, `f()[0]` is + // still syntactically a lexpr, but we *do* want to allow + // coercions. + // + // However, *likely* we are ok with allowing coercions to + // happen if there are no explicit ref mut patterns - all + // implicit ref mut patterns must occur behind a reference, so + // they will have the "correct" variance and lifetime. + // + // This does mean that the following pattern would be legal: + // + // ``` + // struct Foo(Bar); + // struct Bar(u32); + // impl Deref for Foo { + // type Target = Bar; + // fn deref(&self) -> &Bar { &self.0 } + // } + // impl DerefMut for Foo { + // fn deref_mut(&mut self) -> &mut Bar { &mut self.0 } + // } + // fn foo(x: &mut Foo) { + // { + // let Bar(z): &mut Bar = x; + // *z = 42; + // } + // assert_eq!(foo.0.0, 42); + // } + // ``` + // + // FIXME(tschottdorf): don't call contains_explicit_ref_binding, which + // is problematic as the HIR is being scraped, but ref bindings may be + // implicit after #42640. We need to make sure that pat_adjustments + // (once introduced) is populated by the time we get here. + // + // See #44848. + let contains_ref_bindings = arms.iter() + .filter_map(|a| a.contains_explicit_ref_binding()) + .max_by_key(|m| match *m { + hir::MutMutable => 1, + hir::MutImmutable => 0, + }); + + if let Some(m) = contains_ref_bindings { + self.check_expr_with_needs(discrim, Needs::maybe_mut_place(m)) + } else { + // ...but otherwise we want to use any supertype of the + // discriminant. This is sort of a workaround, see note (*) in + // `check_pat` for some details. + let discrim_ty = self.next_ty_var(TypeVariableOrigin::TypeInference(discrim.span)); + self.check_expr_has_type_or_error(discrim, discrim_ty); + discrim_ty + } + } + fn check_pat_struct( &self, pat: &'gcx hir::Pat, @@ -800,7 +994,7 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); etc: bool, expected: Ty<'tcx>, def_bm: ty::BindingMode, - match_discrim_span: Option, + discrim_span: Option, ) -> Ty<'tcx> { // Resolve the path and check the definition for errors. @@ -809,18 +1003,13 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); variant_ty } else { for field in fields { - self.check_pat_walk( - &field.node.pat, - self.tcx.types.err, - def_bm, - match_discrim_span, - ); + self.check_pat_walk(&field.node.pat, self.tcx.types.err, def_bm, discrim_span); } return self.tcx.types.err; }; // Type-check the path. - self.demand_eqtype_pat(pat.span, expected, pat_ty, match_discrim_span); + self.demand_eqtype_pat(pat.span, expected, pat_ty, discrim_span); // Type-check subpatterns. if self.check_struct_pat_fields(pat_ty, pat.hir_id, pat.span, variant, fields, etc, def_bm) diff --git a/src/librustc_typeck/check/coercion.rs b/src/librustc_typeck/check/coercion.rs index d21ceb983f8f4..008975068e581 100644 --- a/src/librustc_typeck/check/coercion.rs +++ b/src/librustc_typeck/check/coercion.rs @@ -1248,12 +1248,8 @@ impl<'gcx, 'tcx, 'exprs, E> CoerceMany<'gcx, 'tcx, 'exprs, E> augment_error(&mut db); } - if expression.filter(|e| fcx.is_assign_to_bool(e, expected)).is_some() { - // Error reported in `check_assign` so avoid emitting error again. - db.delay_as_bug(); - } else { - db.emit(); - } + // Error possibly reported in `check_assign` so avoid emitting error again. + db.emit_unless(expression.filter(|e| fcx.is_assign_to_bool(e, expected)).is_some()); self.final_ty = Some(fcx.tcx.types.err); } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 965f52be31cde..2b519731eeedb 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -117,6 +117,7 @@ use rustc::ty::subst::{UnpackedKind, Subst, InternalSubsts, SubstsRef, UserSelfT use rustc::ty::util::{Representability, IntTypeExt, Discr}; use rustc::ty::layout::VariantIdx; use syntax_pos::{self, BytePos, Span, MultiSpan}; +use syntax_pos::hygiene::CompilerDesugaringKind; use syntax::ast; use syntax::attr; use syntax::feature_gate::{GateIssue, emit_feature_err}; @@ -1086,12 +1087,8 @@ fn check_fn<'a, 'gcx, 'tcx>(inherited: &'a Inherited<'a, 'gcx, 'tcx>, // Add formal parameters. for (arg_ty, arg) in fn_sig.inputs().iter().zip(&body.arguments) { // Check the pattern. - fcx.check_pat_walk( - &arg.pat, - arg_ty, - ty::BindingMode::BindByValue(hir::Mutability::MutImmutable), - None, - ); + let binding_mode = ty::BindingMode::BindByValue(hir::Mutability::MutImmutable); + fcx.check_pat_walk(&arg.pat, arg_ty, binding_mode, None); // Check that argument is Sized. // The check for a non-trivial pattern is a hack to avoid duplicate warnings @@ -2045,15 +2042,17 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { /// Produces warning on the given node, if the current point in the /// function is unreachable, and there hasn't been another warning. fn warn_if_unreachable(&self, id: hir::HirId, span: Span, kind: &str) { - if self.diverges.get() == Diverges::Always { + if self.diverges.get() == Diverges::Always && + // If span arose from a desugaring of `if` then it is the condition itself, + // which diverges, that we are about to lint on. This gives suboptimal diagnostics + // and so we stop here and allow the block of the `if`-expression to be linted instead. + !span.is_compiler_desugaring(CompilerDesugaringKind::IfTemporary) { self.diverges.set(Diverges::WarnedAlways); debug!("warn_if_unreachable: id={:?} span={:?} kind={}", id, span, kind); - self.tcx().lint_hir( - lint::builtin::UNREACHABLE_CODE, - id, span, - &format!("unreachable {}", kind)); + let msg = format!("unreachable {}", kind); + self.tcx().lint_hir(lint::builtin::UNREACHABLE_CODE, id, span, &msg); } } @@ -3161,13 +3160,12 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { } if let Some(mut err) = self.demand_suptype_diag(expr.span, expected_ty, ty) { - if self.is_assign_to_bool(expr, expected_ty) { - // Error reported in `check_assign` so avoid emitting error again. - // FIXME(centril): Consider removing if/when `if` desugars to `match`. - err.delay_as_bug(); - } else { - err.emit(); - } + let expr = match &expr.node { + ExprKind::DropTemps(expr) => expr, + _ => expr, + }; + // Error possibly reported in `check_assign` so avoid emitting error again. + err.emit_unless(self.is_assign_to_bool(expr, expected_ty)); } ty } @@ -3330,194 +3328,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { return_expr_ty); } - // A generic function for checking the 'then' and 'else' clauses in an 'if' - // or 'if-else' expression. - fn check_then_else(&self, - cond_expr: &'gcx hir::Expr, - then_expr: &'gcx hir::Expr, - opt_else_expr: Option<&'gcx hir::Expr>, - sp: Span, - expected: Expectation<'tcx>) -> Ty<'tcx> { - let cond_ty = self.check_expr_has_type_or_error(cond_expr, self.tcx.types.bool); - let cond_diverges = self.diverges.get(); - self.diverges.set(Diverges::Maybe); - - let expected = expected.adjust_for_branches(self); - let then_ty = self.check_expr_with_expectation(then_expr, expected); - let then_diverges = self.diverges.get(); - self.diverges.set(Diverges::Maybe); - - // We've already taken the expected type's preferences - // into account when typing the `then` branch. To figure - // out the initial shot at a LUB, we thus only consider - // `expected` if it represents a *hard* constraint - // (`only_has_type`); otherwise, we just go with a - // fresh type variable. - let coerce_to_ty = expected.coercion_target_type(self, sp); - let mut coerce: DynamicCoerceMany<'_, '_> = CoerceMany::new(coerce_to_ty); - - coerce.coerce(self, &self.misc(sp), then_expr, then_ty); - - if let Some(else_expr) = opt_else_expr { - let else_ty = self.check_expr_with_expectation(else_expr, expected); - let else_diverges = self.diverges.get(); - - let mut outer_sp = if self.tcx.sess.source_map().is_multiline(sp) { - // The `if`/`else` isn't in one line in the output, include some context to make it - // clear it is an if/else expression: - // ``` - // LL | let x = if true { - // | _____________- - // LL || 10i32 - // || ----- expected because of this - // LL || } else { - // LL || 10u32 - // || ^^^^^ expected i32, found u32 - // LL || }; - // ||_____- if and else have incompatible types - // ``` - Some(sp) - } else { - // The entire expression is in one line, only point at the arms - // ``` - // LL | let x = if true { 10i32 } else { 10u32 }; - // | ----- ^^^^^ expected i32, found u32 - // | | - // | expected because of this - // ``` - None - }; - let mut remove_semicolon = None; - let error_sp = if let ExprKind::Block(block, _) = &else_expr.node { - if let Some(expr) = &block.expr { - expr.span - } else if let Some(stmt) = block.stmts.last() { - // possibly incorrect trailing `;` in the else arm - remove_semicolon = self.could_remove_semicolon(block, then_ty); - stmt.span - } else { // empty block, point at its entirety - // Avoid overlapping spans that aren't as readable: - // ``` - // 2 | let x = if true { - // | _____________- - // 3 | | 3 - // | | - expected because of this - // 4 | | } else { - // | |____________^ - // 5 | || - // 6 | || }; - // | || ^ - // | ||_____| - // | |______if and else have incompatible types - // | expected integer, found () - // ``` - // by not pointing at the entire expression: - // ``` - // 2 | let x = if true { - // | ------- if and else have incompatible types - // 3 | 3 - // | - expected because of this - // 4 | } else { - // | ____________^ - // 5 | | - // 6 | | }; - // | |_____^ expected integer, found () - // ``` - if outer_sp.is_some() { - outer_sp = Some(self.tcx.sess.source_map().def_span(sp)); - } - else_expr.span - } - } else { // shouldn't happen unless the parser has done something weird - else_expr.span - }; - let then_sp = if let ExprKind::Block(block, _) = &then_expr.node { - if let Some(expr) = &block.expr { - expr.span - } else if let Some(stmt) = block.stmts.last() { - // possibly incorrect trailing `;` in the else arm - remove_semicolon = remove_semicolon.or( - self.could_remove_semicolon(block, else_ty)); - stmt.span - } else { // empty block, point at its entirety - outer_sp = None; // same as in `error_sp`, cleanup output - then_expr.span - } - } else { // shouldn't happen unless the parser has done something weird - then_expr.span - }; - - let if_cause = self.cause(error_sp, ObligationCauseCode::IfExpression { - then: then_sp, - outer: outer_sp, - semicolon: remove_semicolon, - }); - - coerce.coerce(self, &if_cause, else_expr, else_ty); - - // We won't diverge unless both branches do (or the condition does). - self.diverges.set(cond_diverges | then_diverges & else_diverges); - } else { - // 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 = self.maybe_get_coercion_reason(then_expr.hir_id, sp); - - let else_cause = self.cause(sp, ObligationCauseCode::IfExpressionWithNoElse); - coerce.coerce_forced_unit(self, &else_cause, &mut |err| { - if let Some((sp, msg)) = &ret_reason { - err.span_label(*sp, msg.as_str()); - } else if let ExprKind::Block(block, _) = &then_expr.node { - if let Some(expr) = &block.expr { - err.span_label(expr.span, "found here".to_string()); - } - } - err.note("`if` expressions without `else` evaluate to `()`"); - err.help("consider adding an `else` block that evaluates to the expected type"); - }, ret_reason.is_none()); - - // If the condition is false we can't diverge. - self.diverges.set(cond_diverges); - } - - let result_ty = coerce.complete(self); - if cond_ty.references_error() { - self.tcx.types.err - } else { - result_ty - } - } - - fn maybe_get_coercion_reason(&self, hir_id: hir::HirId, sp: Span) -> Option<(Span, String)> { - let node = self.tcx.hir().get_by_hir_id(self.tcx.hir().get_parent_node_by_hir_id( - self.tcx.hir().get_parent_node_by_hir_id(hir_id), - )); - if let Node::Block(block) = node { - // check that the body's parent is an fn - let parent = self.tcx.hir().get_by_hir_id( - self.tcx.hir().get_parent_node_by_hir_id( - self.tcx.hir().get_parent_node_by_hir_id(block.hir_id), - ), - ); - if let (Some(expr), Node::Item(hir::Item { - node: 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).map(|(fn_decl, _)| ( - fn_decl.output.span(), - format!("expected `{}` because of this return type", fn_decl.output), - )); - } - } - } - if let Node::Local(hir::Local { - ty: Some(_), pat, .. - }) = node { - return Some((pat.span, "expected because of this assignment".to_string())); - } - None - } - // Check field access expressions fn check_field(&self, expr: &'gcx hir::Expr, @@ -4062,7 +3872,7 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { match expr.node { ExprKind::Block(..) | ExprKind::Loop(..) | ExprKind::While(..) | - ExprKind::If(..) | ExprKind::Match(..) => {} + ExprKind::Match(..) => {} _ => self.warn_if_unreachable(expr.hir_id, expr.span, "expression") } @@ -4444,10 +4254,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { ExprKind::Assign(ref lhs, ref rhs) => { self.check_assign(expr, expected, lhs, rhs) } - ExprKind::If(ref cond, ref then_expr, ref opt_else_expr) => { - self.check_then_else(&cond, then_expr, opt_else_expr.as_ref().map(|e| &**e), - expr.span, expected) - } ExprKind::While(ref cond, ref body, _) => { let ctxt = BreakableCtxt { // cannot use break with a value from a while loop @@ -5261,7 +5067,6 @@ impl<'a, 'gcx, 'tcx> FnCtxt<'a, 'gcx, 'tcx> { match expression.node { ExprKind::Call(..) | ExprKind::MethodCall(..) | - ExprKind::If(..) | ExprKind::While(..) | ExprKind::Loop(..) | ExprKind::Match(..) | diff --git a/src/libsyntax_pos/hygiene.rs b/src/libsyntax_pos/hygiene.rs index a901afdff43e6..1d9dc26bf6092 100644 --- a/src/libsyntax_pos/hygiene.rs +++ b/src/libsyntax_pos/hygiene.rs @@ -591,6 +591,10 @@ impl ExpnFormat { /// The kind of compiler desugaring. #[derive(Clone, Copy, Hash, Debug, PartialEq, Eq, RustcEncodable, RustcDecodable)] pub enum CompilerDesugaringKind { + /// We desugar `if c { i } else { e }` to `match $ExprKind::Use(c) { true => i, _ => e }`. + /// However, we do not want to blame `c` for unreachability but rather say that `i` + /// is unreachable. This desugaring kind allows us to avoid blaming `c`. + IfTemporary, QuestionMark, TryBlock, /// Desugaring of an `impl Trait` in return type position @@ -605,6 +609,7 @@ pub enum CompilerDesugaringKind { impl CompilerDesugaringKind { pub fn name(self) -> Symbol { Symbol::intern(match self { + CompilerDesugaringKind::IfTemporary => "if", CompilerDesugaringKind::Async => "async", CompilerDesugaringKind::Await => "await", CompilerDesugaringKind::QuestionMark => "?", From 99039689f0c097703c7258c339b05d1c88e54ff8 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Wed, 24 Apr 2019 12:50:05 +0200 Subject: [PATCH 2/5] Adjust mir-opt tests for new HIR without If --- src/test/mir-opt/issue-38669.rs | 8 ++++--- src/test/mir-opt/loop_test.rs | 21 ++++++++++--------- .../mir-opt/nll/region-subtyping-basic.rs | 6 +++--- src/test/mir-opt/simplify_if.rs | 12 ++++++----- 4 files changed, 26 insertions(+), 21 deletions(-) diff --git a/src/test/mir-opt/issue-38669.rs b/src/test/mir-opt/issue-38669.rs index 047e623941b71..e8ab690bb4648 100644 --- a/src/test/mir-opt/issue-38669.rs +++ b/src/test/mir-opt/issue-38669.rs @@ -27,15 +27,17 @@ fn main() { // bb3: { // StorageLive(_4); // _4 = _1; -// switchInt(move _4) -> [false: bb5, otherwise: bb4]; +// FakeRead(ForMatchedPlace, _4); +// switchInt(_4) -> [false: bb5, otherwise: bb4]; // } -// bb4: { +// ... +// bb7: { // _0 = (); // StorageDead(_4); // StorageDead(_1); // return; // } -// bb5: { +// bb8: { // _3 = (); // StorageDead(_4); // _1 = const true; diff --git a/src/test/mir-opt/loop_test.rs b/src/test/mir-opt/loop_test.rs index 34891ee70b5c6..e75955b9b2440 100644 --- a/src/test/mir-opt/loop_test.rs +++ b/src/test/mir-opt/loop_test.rs @@ -22,19 +22,20 @@ fn main() { // resume; // } // ... -// bb3: { // Entry into the loop +// bb6: { // Entry into the loop // _1 = (); -// goto -> bb4; +// StorageDead(_2); +// goto -> bb7; // } -// bb4: { // The loop_block -// falseUnwind -> [real: bb5, cleanup: bb1]; +// bb7: { // The loop_block +// falseUnwind -> [real: bb8, cleanup: bb1]; // } -// bb5: { // The loop body (body_block) -// StorageLive(_5); -// _5 = const 1i32; -// FakeRead(ForLet, _5); -// StorageDead(_5); -// goto -> bb4; +// bb8: { // The loop body (body_block) +// StorageLive(_6); +// _6 = const 1i32; +// FakeRead(ForLet, _6); +// StorageDead(_6); +// goto -> bb7; // } // ... // END rustc.main.SimplifyCfg-qualify-consts.after.mir diff --git a/src/test/mir-opt/nll/region-subtyping-basic.rs b/src/test/mir-opt/nll/region-subtyping-basic.rs index bb27461bb1e0f..622cc99983002 100644 --- a/src/test/mir-opt/nll/region-subtyping-basic.rs +++ b/src/test/mir-opt/nll/region-subtyping-basic.rs @@ -22,9 +22,9 @@ fn main() { // END RUST SOURCE // START rustc.main.nll.0.mir -// | '_#2r | U0 | {bb2[0..=5], bb3[0..=1]} -// | '_#3r | U0 | {bb2[1..=5], bb3[0..=1]} -// | '_#4r | U0 | {bb2[4..=5], bb3[0..=1]} +// | '_#2r | U0 | {bb2[0..=8], bb3[0], bb6[0..=1]} +// | '_#3r | U0 | {bb2[1..=8], bb3[0], bb6[0..=1]} +// | '_#4r | U0 | {bb2[4..=8], bb3[0], bb6[0..=1]} // END rustc.main.nll.0.mir // START rustc.main.nll.0.mir // let _2: &'_#3r usize; diff --git a/src/test/mir-opt/simplify_if.rs b/src/test/mir-opt/simplify_if.rs index f6e6c6baf7a46..b2a99a6d446bb 100644 --- a/src/test/mir-opt/simplify_if.rs +++ b/src/test/mir-opt/simplify_if.rs @@ -5,13 +5,15 @@ fn main() { } // END RUST SOURCE -// START rustc.main.SimplifyBranches-initial.before.mir +// START rustc.main.SimplifyBranches-after-copy-prop.before.mir // bb0: { -// switchInt(const false) -> [false: bb3, otherwise: bb2]; +// ... +// switchInt(const false) -> [false: bb3, otherwise: bb1]; // } -// END rustc.main.SimplifyBranches-initial.before.mir -// START rustc.main.SimplifyBranches-initial.after.mir +// END rustc.main.SimplifyBranches-after-copy-prop.before.mir +// START rustc.main.SimplifyBranches-after-copy-prop.after.mir // bb0: { +// ... // goto -> bb3; // } -// END rustc.main.SimplifyBranches-initial.after.mir +// END rustc.main.SimplifyBranches-after-copy-prop.after.mir From 8d1e5b8b396e06778583ad62177430d2b95015f5 Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Tue, 19 Mar 2019 04:47:07 +0100 Subject: [PATCH 3/5] Various test changes --- src/test/incremental/hashes/if_expressions.rs | 4 +- src/test/run-pass/if-ret.stderr | 8 ++ src/test/ui/if/if-let-arm-types.rs | 5 +- src/test/ui/if/if-let-arm-types.stderr | 8 +- src/test/ui/if/if-without-else-as-fn-expr.rs | 30 +++++ .../ui/if/if-without-else-as-fn-expr.stderr | 123 +++++++++++++++++- src/test/ui/issues/issue-19991.stderr | 3 + src/test/ui/issues/issue-50577.rs | 1 + src/test/ui/issues/issue-50577.stderr | 15 ++- src/test/ui/reachable/expr_if.rs | 2 +- src/test/ui/reachable/expr_if.stderr | 20 ++- 11 files changed, 200 insertions(+), 19 deletions(-) create mode 100644 src/test/run-pass/if-ret.stderr diff --git a/src/test/incremental/hashes/if_expressions.rs b/src/test/incremental/hashes/if_expressions.rs index fba7869af42f2..6bc7d286e3adb 100644 --- a/src/test/incremental/hashes/if_expressions.rs +++ b/src/test/incremental/hashes/if_expressions.rs @@ -94,7 +94,7 @@ pub fn add_else_branch(x: bool) -> u32 { } #[cfg(not(cfail1))] -#[rustc_clean(cfg="cfail2", except="HirBody,typeck_tables_of")] +#[rustc_clean(cfg="cfail2", except="HirBody")] #[rustc_clean(cfg="cfail3")] pub fn add_else_branch(x: bool) -> u32 { let mut ret = 1; @@ -191,7 +191,7 @@ pub fn add_else_branch_if_let(x: Option) -> u32 { } #[cfg(not(cfail1))] -#[rustc_clean(cfg="cfail2", except="HirBody,typeck_tables_of")] +#[rustc_clean(cfg="cfail2", except="HirBody")] #[rustc_clean(cfg="cfail3")] pub fn add_else_branch_if_let(x: Option) -> u32 { let mut ret = 1; diff --git a/src/test/run-pass/if-ret.stderr b/src/test/run-pass/if-ret.stderr new file mode 100644 index 0000000000000..a64281833e5cb --- /dev/null +++ b/src/test/run-pass/if-ret.stderr @@ -0,0 +1,8 @@ +warning: unreachable block in `if` expression + --> $DIR/if-ret.rs:4:24 + | +LL | fn foo() { if (return) { } } + | ^^^ + | + = note: #[warn(unreachable_code)] on by default + diff --git a/src/test/ui/if/if-let-arm-types.rs b/src/test/ui/if/if-let-arm-types.rs index 819f5dd1cfc35..0f8815f0479f5 100644 --- a/src/test/ui/if/if-let-arm-types.rs +++ b/src/test/ui/if/if-let-arm-types.rs @@ -1,11 +1,12 @@ fn main() { if let Some(b) = None { - //~^ NOTE if let` arms have incompatible types + //~^ NOTE if and else have incompatible types () + //~^ NOTE expected because of this } else { 1 }; - //~^^ ERROR: `if let` arms have incompatible types + //~^^ ERROR: if and else have incompatible types //~| NOTE expected (), found integer //~| NOTE expected type `()` } diff --git a/src/test/ui/if/if-let-arm-types.stderr b/src/test/ui/if/if-let-arm-types.stderr index b986973fe91f4..ff88de20f76cc 100644 --- a/src/test/ui/if/if-let-arm-types.stderr +++ b/src/test/ui/if/if-let-arm-types.stderr @@ -1,14 +1,16 @@ -error[E0308]: `if let` arms have incompatible types - --> $DIR/if-let-arm-types.rs:6:9 +error[E0308]: if and else have incompatible types + --> $DIR/if-let-arm-types.rs:7:9 | LL | / if let Some(b) = None { LL | | LL | | () + | | -- expected because of this +LL | | LL | | } else { LL | | 1 | | ^ expected (), found integer LL | | }; - | |_____- `if let` arms have incompatible types + | |_____- if and else have incompatible types | = note: expected type `()` found type `{integer}` diff --git a/src/test/ui/if/if-without-else-as-fn-expr.rs b/src/test/ui/if/if-without-else-as-fn-expr.rs index 67e4445629f8c..15892de83854c 100644 --- a/src/test/ui/if/if-without-else-as-fn-expr.rs +++ b/src/test/ui/if/if-without-else-as-fn-expr.rs @@ -3,6 +3,7 @@ fn foo(bar: usize) -> usize { return 3; } //~^^^ ERROR if may be missing an else clause + //~| ERROR mismatched types [E0308] } fn foo2(bar: usize) -> usize { @@ -10,6 +11,7 @@ fn foo2(bar: usize) -> usize { return 3; }; //~^^^ ERROR if may be missing an else clause + //~| ERROR mismatched types [E0308] x } @@ -18,8 +20,36 @@ fn foo3(bar: usize) -> usize { 3 } //~^^^ ERROR if may be missing an else clause + //~| ERROR mismatched types [E0308] } +fn foo_let(bar: usize) -> usize { + if let 0 = 1 { + return 3; + } + //~^^^ ERROR if may be missing an else clause + //~| ERROR mismatched types [E0308] +} + +fn foo2_let(bar: usize) -> usize { + let x: usize = if let 0 = 1 { + return 3; + }; + //~^^^ ERROR if may be missing an else clause + //~| ERROR mismatched types [E0308] + x +} + +fn foo3_let(bar: usize) -> usize { + if let 0 = 1 { + 3 + } + //~^^^ ERROR if may be missing an else clause + //~| ERROR mismatched types [E0308] +} + +// FIXME(60254): deduplicate first error in favor of second. + fn main() { let _ = foo(1); } diff --git a/src/test/ui/if/if-without-else-as-fn-expr.stderr b/src/test/ui/if/if-without-else-as-fn-expr.stderr index 0ba72726ca787..06600b1cb9aea 100644 --- a/src/test/ui/if/if-without-else-as-fn-expr.stderr +++ b/src/test/ui/if/if-without-else-as-fn-expr.stderr @@ -1,3 +1,14 @@ +error[E0308]: mismatched types + --> $DIR/if-without-else-as-fn-expr.rs:2:5 + | +LL | / if bar % 5 == 0 { +LL | | return 3; +LL | | } + | |_____^ expected usize, found () + | + = note: expected type `usize` + found type `()` + error[E0317]: if may be missing an else clause --> $DIR/if-without-else-as-fn-expr.rs:2:5 | @@ -13,8 +24,20 @@ LL | | } = note: `if` expressions without `else` evaluate to `()` = help: consider adding an `else` block that evaluates to the expected type +error[E0308]: mismatched types + --> $DIR/if-without-else-as-fn-expr.rs:10:20 + | +LL | let x: usize = if bar % 5 == 0 { + | ____________________^ +LL | | return 3; +LL | | }; + | |_____^ expected usize, found () + | + = note: expected type `usize` + found type `()` + error[E0317]: if may be missing an else clause - --> $DIR/if-without-else-as-fn-expr.rs:9:20 + --> $DIR/if-without-else-as-fn-expr.rs:10:20 | LL | let x: usize = if bar % 5 == 0 { | _________-__________^ @@ -29,8 +52,19 @@ LL | | }; = note: `if` expressions without `else` evaluate to `()` = help: consider adding an `else` block that evaluates to the expected type +error[E0308]: mismatched types + --> $DIR/if-without-else-as-fn-expr.rs:19:5 + | +LL | / if bar % 5 == 0 { +LL | | 3 +LL | | } + | |_____^ expected usize, found () + | + = note: expected type `usize` + found type `()` + error[E0317]: if may be missing an else clause - --> $DIR/if-without-else-as-fn-expr.rs:17:5 + --> $DIR/if-without-else-as-fn-expr.rs:19:5 | LL | fn foo3(bar: usize) -> usize { | ----- expected `usize` because of this return type @@ -44,6 +78,87 @@ LL | | } = note: `if` expressions without `else` evaluate to `()` = help: consider adding an `else` block that evaluates to the expected type -error: aborting due to 3 previous errors +error[E0308]: mismatched types + --> $DIR/if-without-else-as-fn-expr.rs:27:5 + | +LL | / if let 0 = 1 { +LL | | return 3; +LL | | } + | |_____^ expected usize, found () + | + = note: expected type `usize` + found type `()` + +error[E0317]: if may be missing an else clause + --> $DIR/if-without-else-as-fn-expr.rs:27:5 + | +LL | fn foo_let(bar: usize) -> usize { + | ----- expected `usize` because of this return type +LL | / if let 0 = 1 { +LL | | return 3; +LL | | } + | |_____^ expected usize, found () + | + = note: expected type `usize` + found type `()` + = note: `if` expressions without `else` evaluate to `()` + = help: consider adding an `else` block that evaluates to the expected type + +error[E0308]: mismatched types + --> $DIR/if-without-else-as-fn-expr.rs:35:20 + | +LL | let x: usize = if let 0 = 1 { + | ____________________^ +LL | | return 3; +LL | | }; + | |_____^ expected usize, found () + | + = note: expected type `usize` + found type `()` + +error[E0317]: if may be missing an else clause + --> $DIR/if-without-else-as-fn-expr.rs:35:20 + | +LL | let x: usize = if let 0 = 1 { + | _________-__________^ + | | | + | | expected because of this assignment +LL | | return 3; +LL | | }; + | |_____^ expected usize, found () + | + = note: expected type `usize` + found type `()` + = note: `if` expressions without `else` evaluate to `()` + = help: consider adding an `else` block that evaluates to the expected type + +error[E0308]: mismatched types + --> $DIR/if-without-else-as-fn-expr.rs:44:5 + | +LL | / if let 0 = 1 { +LL | | 3 +LL | | } + | |_____^ expected usize, found () + | + = note: expected type `usize` + found type `()` + +error[E0317]: if may be missing an else clause + --> $DIR/if-without-else-as-fn-expr.rs:44:5 + | +LL | fn foo3_let(bar: usize) -> usize { + | ----- expected `usize` because of this return type +LL | / if let 0 = 1 { +LL | | 3 +LL | | } + | |_____^ expected usize, found () + | + = note: expected type `usize` + found type `()` + = note: `if` expressions without `else` evaluate to `()` + = help: consider adding an `else` block that evaluates to the expected type + +error: aborting due to 12 previous errors -For more information about this error, try `rustc --explain E0317`. +Some errors have detailed explanations: E0308, E0317. +For more information about an error, try `rustc --explain E0308`. diff --git a/src/test/ui/issues/issue-19991.stderr b/src/test/ui/issues/issue-19991.stderr index 4c6d150229a3e..d9ea910adef50 100644 --- a/src/test/ui/issues/issue-19991.stderr +++ b/src/test/ui/issues/issue-19991.stderr @@ -6,11 +6,14 @@ LL | | LL | | LL | | LL | | 765 + | | --- found here LL | | }; | |_____^ expected (), found integer | = note: expected type `()` found type `{integer}` + = note: `if` expressions without `else` evaluate to `()` + = help: consider adding an `else` block that evaluates to the expected type error: aborting due to previous error diff --git a/src/test/ui/issues/issue-50577.rs b/src/test/ui/issues/issue-50577.rs index f0f1dc6c28667..bf892a8daa27f 100644 --- a/src/test/ui/issues/issue-50577.rs +++ b/src/test/ui/issues/issue-50577.rs @@ -2,5 +2,6 @@ fn main() { enum Foo { Drop = assert_eq!(1, 1) //~^ ERROR if may be missing an else clause + //~| ERROR mismatched types [E0308] } } diff --git a/src/test/ui/issues/issue-50577.stderr b/src/test/ui/issues/issue-50577.stderr index 0c3ba2ea4f94d..413c8c5c80b52 100644 --- a/src/test/ui/issues/issue-50577.stderr +++ b/src/test/ui/issues/issue-50577.stderr @@ -1,3 +1,13 @@ +error[E0308]: mismatched types + --> $DIR/issue-50577.rs:3:16 + | +LL | Drop = assert_eq!(1, 1) + | ^^^^^^^^^^^^^^^^ expected isize, found () + | + = note: expected type `isize` + found type `()` + = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) + error[E0317]: if may be missing an else clause --> $DIR/issue-50577.rs:3:16 | @@ -13,6 +23,7 @@ LL | Drop = assert_eq!(1, 1) = help: consider adding an `else` block that evaluates to the expected type = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) -error: aborting due to previous error +error: aborting due to 2 previous errors -For more information about this error, try `rustc --explain E0317`. +Some errors have detailed explanations: E0308, E0317. +For more information about an error, try `rustc --explain E0308`. diff --git a/src/test/ui/reachable/expr_if.rs b/src/test/ui/reachable/expr_if.rs index ed43bd8c6895e..a3d54892de5bb 100644 --- a/src/test/ui/reachable/expr_if.rs +++ b/src/test/ui/reachable/expr_if.rs @@ -4,7 +4,7 @@ #![deny(unreachable_code)] fn foo() { - if {return} { + if {return} { //~ ERROR unreachable block in `if` expression println!("Hello, world!"); } } diff --git a/src/test/ui/reachable/expr_if.stderr b/src/test/ui/reachable/expr_if.stderr index d11471da1a6a3..f1690e595e5d1 100644 --- a/src/test/ui/reachable/expr_if.stderr +++ b/src/test/ui/reachable/expr_if.stderr @@ -1,15 +1,25 @@ -error: unreachable statement - --> $DIR/expr_if.rs:27:5 +error: unreachable block in `if` expression + --> $DIR/expr_if.rs:7:17 | -LL | println!("But I am."); - | ^^^^^^^^^^^^^^^^^^^^^^ +LL | if {return} { + | _________________^ +LL | | println!("Hello, world!"); +LL | | } + | |_____^ | note: lint level defined here --> $DIR/expr_if.rs:4:9 | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ + +error: unreachable statement + --> $DIR/expr_if.rs:27:5 + | +LL | println!("But I am."); + | ^^^^^^^^^^^^^^^^^^^^^^ + | = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) -error: aborting due to previous error +error: aborting due to 2 previous errors From efd3733f9d33e814629c6d14ddee92d44f62205c Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Thu, 25 Apr 2019 05:48:26 +0200 Subject: [PATCH 4/5] add test checking that 'if cond { .. }' where 'cond: &mut? bool' isn't accepted. --- src/test/ui/if/if-no-match-bindings.rs | 22 ++++++++++++ src/test/ui/if/if-no-match-bindings.stderr | 39 ++++++++++++++++++++++ 2 files changed, 61 insertions(+) create mode 100644 src/test/ui/if/if-no-match-bindings.rs create mode 100644 src/test/ui/if/if-no-match-bindings.stderr diff --git a/src/test/ui/if/if-no-match-bindings.rs b/src/test/ui/if/if-no-match-bindings.rs new file mode 100644 index 0000000000000..581ce18c1d614 --- /dev/null +++ b/src/test/ui/if/if-no-match-bindings.rs @@ -0,0 +1,22 @@ +// Checks for `if` expressions with respect to default match bindings. +// Specifically, we do not accept `if cond { ... }` where `cond: &mut? bool`. +// Meanwhile, `match cond { true => ..., _ => ... }` does accept that. + +// FIXME(@rust-lang/lang-team): consider relaxing this? + +fn b_ref<'a>() -> &'a bool { &true } +fn b_mut_ref<'a>() -> &'a mut bool { &mut true } + +fn main() { + // This is OK: + match b_ref() { true => {}, _ => {} } + match b_mut_ref() { true => {}, _ => {} } + match &true { true => {}, _ => {} } + match &mut true { true => {}, _ => {} } + + // This is NOT: + if b_ref() {} //~ ERROR mismatched types [E0308] + if b_mut_ref() {} //~ ERROR mismatched types [E0308] + if &true {} //~ ERROR mismatched types [E0308] + if &mut true {} //~ ERROR mismatched types [E0308] +} diff --git a/src/test/ui/if/if-no-match-bindings.stderr b/src/test/ui/if/if-no-match-bindings.stderr new file mode 100644 index 0000000000000..7b0b472121fce --- /dev/null +++ b/src/test/ui/if/if-no-match-bindings.stderr @@ -0,0 +1,39 @@ +error[E0308]: mismatched types + --> $DIR/if-no-match-bindings.rs:18:8 + | +LL | if b_ref() {} + | ^^^^^^^ expected bool, found &bool + | + = note: expected type `bool` + found type `&bool` + +error[E0308]: mismatched types + --> $DIR/if-no-match-bindings.rs:19:8 + | +LL | if b_mut_ref() {} + | ^^^^^^^^^^^ expected bool, found &mut bool + | + = note: expected type `bool` + found type `&mut bool` + +error[E0308]: mismatched types + --> $DIR/if-no-match-bindings.rs:20:8 + | +LL | if &true {} + | ^^^^^ expected bool, found &bool + | + = note: expected type `bool` + found type `&bool` + +error[E0308]: mismatched types + --> $DIR/if-no-match-bindings.rs:21:8 + | +LL | if &mut true {} + | ^^^^^^^^^ expected bool, found &mut bool + | + = note: expected type `bool` + found type `&mut bool` + +error: aborting due to 4 previous errors + +For more information about this error, try `rustc --explain E0308`. From f9cc5a65d24270fac44a7ccf709d7a7efd7b525d Mon Sep 17 00:00:00 2001 From: Mazdak Farrokhzad Date: Fri, 10 May 2019 19:55:52 +0200 Subject: [PATCH 5/5] check_match: add FIXME for removing of hack. --- src/librustc_typeck/check/_match.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index e8af2f40b87a7..a69f639e8941f 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -615,6 +615,8 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); // `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(discrim, self.tcx.types.bool) } else { self.demand_discriminant_type(arms, discrim)