Enforce 'cond: bool' in while-expr + improve reachability diags.

Author: Centril

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,

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,

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);
             }

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);

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 => "?",

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() { }

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)
-
-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)
+LL |       while {return} {
+   |  ____________________^
+LL | |
+LL | |         println!("I am dead.");
+LL | |     }
+   | |_____^
 
-error: aborting due to 3 previous errors
+error: aborting due to 2 previous errors