diff --git a/src/librustc/hir/lowering.rs b/src/librustc/hir/lowering.rs index 011808a7ff9..198c6797cca 100644 --- a/src/librustc/hir/lowering.rs +++ b/src/librustc/hir/lowering.rs @@ -63,7 +63,7 @@ use syntax::errors; use syntax::ext::hygiene::{Mark, SyntaxContext}; use syntax::print::pprust; use syntax::source_map::{self, respan, ExpnInfo, CompilerDesugaringKind, Spanned}; -use syntax::source_map::CompilerDesugaringKind::IfTemporary; +use syntax::source_map::CompilerDesugaringKind::CondTemporary; use syntax::std_inject; use syntax::symbol::{kw, sym, Symbol}; use syntax::tokenstream::{TokenStream, TokenTree}; @@ -4408,7 +4408,7 @@ impl<'a> LoweringContext<'a> { // 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 span_block = self.mark_span_with_reason(IfTemporary, cond.span, None); + let span_block = self.mark_span_with_reason(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`. @@ -4484,7 +4484,7 @@ impl<'a> LoweringContext<'a> { // ``` // 'label: loop { // match DropTemps($cond) { - // true => $block, + // true => $body, // _ => break, // } // } @@ -4502,11 +4502,12 @@ impl<'a> LoweringContext<'a> { let else_arm = this.arm(hir_vec![else_pat], else_expr); // Lower condition: + let span_block = this.mark_span_with_reason(CondTemporary, cond.span, None); let cond = this.with_loop_condition_scope(|this| this.lower_expr(cond)); // 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 = this.expr_drop_temps(cond.span, P(cond), ThinVec::new()); + let cond = this.expr_drop_temps(span_block, P(cond), ThinVec::new()); let match_expr = this.expr_match( cond.span, diff --git a/src/librustc/ich/impls_syntax.rs b/src/librustc/ich/impls_syntax.rs index 9430661f75a..1db18d30282 100644 --- a/src/librustc/ich/impls_syntax.rs +++ b/src/librustc/ich/impls_syntax.rs @@ -415,7 +415,7 @@ impl_stable_hash_for!(enum ::syntax_pos::hygiene::ExpnFormat { }); impl_stable_hash_for!(enum ::syntax_pos::hygiene::CompilerDesugaringKind { - IfTemporary, + CondTemporary, Async, Await, QuestionMark, diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs index eeed5be8678..d24f92a6faf 100644 --- a/src/librustc_typeck/check/_match.rs +++ b/src/librustc_typeck/check/_match.rs @@ -180,7 +180,12 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { // then that's equivalent to there existing a LUB. 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)) + .filter(|&s| { + // In the case of `if`- and `while`-expressions we've already checked + // that `scrutinee: bool`. We know that the pattern is `true`, + // so an error here would be a duplicate and from the wrong POV. + s.is_compiler_desugaring(CompilerDesugaringKind::CondTemporary) + }) .is_some()); } @@ -624,14 +629,15 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); let tcx = self.tcx; use hir::MatchSource::*; - let (source_if, if_no_else, if_desugar) = match match_src { + let (source_if, if_no_else, force_scrutinee_bool) = match match_src { IfDesugar { contains_else_clause } => (true, !contains_else_clause, true), IfLetDesugar { contains_else_clause } => (true, !contains_else_clause, false), + WhileDesugar => (false, false, true), _ => (false, false, false), }; // Type check the descriminant and get its type. - let discrim_ty = if if_desugar { + let discrim_ty = if force_scrutinee_bool { // Here we want to ensure: // // 1. That default match bindings are *not* accepted in the condition of an @@ -651,7 +657,7 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); return tcx.types.never; } - self.warn_arms_when_scrutinee_diverges(arms, source_if); + self.warn_arms_when_scrutinee_diverges(arms, match_src); // Otherwise, we have to union together the types that the // arms produce and so forth. @@ -726,7 +732,7 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); 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), + (0, _) => coercion.coerce(self, &self.misc(span), &arm.body, arm_ty), (_, true) => self.if_fallback_coercion(span, then_expr, &mut coercion), (_, _) => { let then_ty = prior_arm_ty.unwrap(); @@ -771,9 +777,14 @@ https://doc.rust-lang.org/reference/types.html#trait-objects"); /// When the previously checked expression (the scrutinee) diverges, /// warn the user about the match arms being unreachable. - fn warn_arms_when_scrutinee_diverges(&self, arms: &'tcx [hir::Arm], source_if: bool) { + fn warn_arms_when_scrutinee_diverges(&self, arms: &'tcx [hir::Arm], source: hir::MatchSource) { if self.diverges.get().always() { - let msg = if source_if { "block in `if` expression" } else { "arm" }; + use hir::MatchSource::*; + let msg = match source { + IfDesugar { .. } | IfLetDesugar { .. } => "block in `if` expression", + WhileDesugar { .. } | WhileLetDesugar { .. } => "block in `while` expression", + _ => "arm", + }; for arm in arms { self.warn_if_unreachable(arm.body.hir_id, arm.body.span, msg); } diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs index 5b19614a5b3..3bfb3477d47 100644 --- a/src/librustc_typeck/check/mod.rs +++ b/src/librustc_typeck/check/mod.rs @@ -2161,10 +2161,10 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> { /// 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 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) { + // If span arose from a desugaring of `if` or `while`, then it is the condition itself, + // which diverges, that we are about to lint on. This gives suboptimal diagnostics. + // Instead, stop here so that the `if`- or `while`-expression's block is linted instead. + !span.is_compiler_desugaring(CompilerDesugaringKind::CondTemporary) { self.diverges.set(Diverges::WarnedAlways); debug!("warn_if_unreachable: id={:?} span={:?} kind={}", id, span, kind); diff --git a/src/libsyntax_pos/hygiene.rs b/src/libsyntax_pos/hygiene.rs index 4dbd4ccda91..a6c8c76cf23 100644 --- a/src/libsyntax_pos/hygiene.rs +++ b/src/libsyntax_pos/hygiene.rs @@ -723,7 +723,8 @@ 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, + /// This also applies to `while` loops. + CondTemporary, QuestionMark, TryBlock, /// Desugaring of an `impl Trait` in return type position @@ -738,7 +739,7 @@ pub enum CompilerDesugaringKind { impl CompilerDesugaringKind { pub fn name(self) -> Symbol { Symbol::intern(match self { - CompilerDesugaringKind::IfTemporary => "if", + CompilerDesugaringKind::CondTemporary => "if and while condition", CompilerDesugaringKind::Async => "async", CompilerDesugaringKind::Await => "await", CompilerDesugaringKind::QuestionMark => "?", diff --git a/src/test/ui/reachable/expr_while.rs b/src/test/ui/reachable/expr_while.rs index 36a3e3dd961..10a4b69939f 100644 --- a/src/test/ui/reachable/expr_while.rs +++ b/src/test/ui/reachable/expr_while.rs @@ -5,8 +5,8 @@ fn foo() { while {return} { + //~^ ERROR unreachable block in `while` expression println!("Hello, world!"); - //~^ ERROR unreachable } } @@ -20,11 +20,10 @@ fn bar() { fn baz() { // Here, we cite the `while` loop as dead. while {return} { + //~^ ERROR unreachable block in `while` expression println!("I am dead."); - //~^ ERROR unreachable } println!("I am, too."); - //~^ ERROR unreachable } fn main() { } diff --git a/src/test/ui/reachable/expr_while.stderr b/src/test/ui/reachable/expr_while.stderr index d2f5588568a..fc528926b4c 100644 --- a/src/test/ui/reachable/expr_while.stderr +++ b/src/test/ui/reachable/expr_while.stderr @@ -1,31 +1,28 @@ -error: unreachable statement - --> $DIR/expr_while.rs:8:9 +error: unreachable block in `while` expression + --> $DIR/expr_while.rs:7:20 | -LL | println!("Hello, world!"); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^ +LL | while {return} { + | ____________________^ +LL | | +LL | | println!("Hello, world!"); +LL | | } + | |_____^ | note: lint level defined here --> $DIR/expr_while.rs:4:9 | LL | #![deny(unreachable_code)] | ^^^^^^^^^^^^^^^^ - = 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: unreachable statement - --> $DIR/expr_while.rs:23:9 +error: unreachable block in `while` expression + --> $DIR/expr_while.rs:22:20 | -LL | println!("I am dead."); - | ^^^^^^^^^^^^^^^^^^^^^^^ - | - = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info) +LL | while {return} { + | ____________________^ +LL | | +LL | | println!("I am dead."); +LL | | } + | |_____^ -error: unreachable statement - --> $DIR/expr_while.rs:26:5 - | -LL | println!("I am, too."); - | ^^^^^^^^^^^^^^^^^^^^^^^ - | - = 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 3 previous errors +error: aborting due to 2 previous errors