Point at original span when emitting unreachable lint

Fixes #64590 When we emit an 'unreachable' lint, we now add a note pointing at the expression that actually causes the code to be unreachable (e.g. `return`, `break`, `panic`). This is especially useful when macros are involved, since a diverging expression might be hidden inside of a macro invocation.
rust-lang · Sep 18, 2019 · 822393d · 822393d
1 parent eceec57
commit 822393d
Show file tree

Hide file tree

Showing 37 changed files with 328 additions and 11 deletions.
diff --git a/src/librustc_typeck/check/_match.rs b/src/librustc_typeck/check/_match.rs
@@ -43,7 +43,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         // If there are no arms, that is a diverging match; a special case.
         if arms.is_empty() {
-            self.diverges.set(self.diverges.get() | Diverges::Always);
+            self.diverges.set(self.diverges.get() | Diverges::Always(expr.span));
             return tcx.types.never;
         }
 
@@ -69,7 +69,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
             // warnings).
             match all_pats_diverge {
                 Diverges::Maybe => Diverges::Maybe,
-                Diverges::Always | Diverges::WarnedAlways => Diverges::WarnedAlways,
+                Diverges::Always(..) | Diverges::WarnedAlways => Diverges::WarnedAlways,
             }
         }).collect();
 

diff --git a/src/librustc_typeck/check/expr.rs b/src/librustc_typeck/check/expr.rs
@@ -170,7 +170,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
 
         // Any expression that produces a value of type `!` must have diverged
         if ty.is_never() {
-            self.diverges.set(self.diverges.get() | Diverges::Always);
+            self.diverges.set(self.diverges.get() | Diverges::Always(expr.span));
         }
 
         // Record the type, which applies it effects.

diff --git a/src/librustc_typeck/check/mod.rs b/src/librustc_typeck/check/mod.rs
@@ -450,7 +450,10 @@ pub enum Diverges {
 
     /// Definitely known to diverge and therefore
     /// not reach the next sibling or its parent.
-    Always,
+    /// The `Span` points to the expression
+    /// that caused us to diverge
+    /// (e.g. `return`, `break`, etc)
+    Always(Span),
 
     /// Same as `Always` but with a reachability
     /// warning already emitted.
@@ -487,7 +490,7 @@ impl ops::BitOrAssign for Diverges {
 
 impl Diverges {
     fn always(self) -> bool {
-        self >= Diverges::Always
+        self >= Diverges::Always(DUMMY_SP)
     }
 }
 
@@ -2307,17 +2310,23 @@ impl<'a, 'tcx> FnCtxt<'a, '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 &&
+        // FIXME: Combine these two 'if' expressions into one once
+        // let chains are implemented
+        if let Diverges::Always(orig_span) = self.diverges.get() {
             // 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_desugaring(DesugaringKind::CondTemporary) {
-            self.diverges.set(Diverges::WarnedAlways);
+            if !span.is_desugaring(DesugaringKind::CondTemporary) {
+                self.diverges.set(Diverges::WarnedAlways);
 
-            debug!("warn_if_unreachable: id={:?} span={:?} kind={}", id, span, kind);
+                debug!("warn_if_unreachable: id={:?} span={:?} kind={}", id, span, kind);
 
-            let msg = format!("unreachable {}", kind);
-            self.tcx().lint_hir(lint::builtin::UNREACHABLE_CODE, id, span, &msg);
+                let msg = format!("unreachable {}", kind);
+                let mut err = self.tcx().struct_span_lint_hir(lint::builtin::UNREACHABLE_CODE,
+                                                              id, span, &msg);
+                err.span_note(orig_span, "any code following this expression is unreachable");
+                err.emit();
+            }
         }
     }
 

diff --git a/src/test/ui/dead-code-ret.stderr b/src/test/ui/dead-code-ret.stderr
@@ -9,6 +9,11 @@ note: lint level defined here
    |
 LL | #![deny(unreachable_code)]
    |         ^^^^^^^^^^^^^^^^
+note: any code following this expression is unreachable
+  --> $DIR/dead-code-ret.rs:6:5
+   |
+LL |     return;
+   |     ^^^^^^
    = 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

diff --git a/src/test/ui/if-ret.stderr b/src/test/ui/if-ret.stderr
@@ -5,4 +5,9 @@ LL | fn foo() { if (return) { } }
    |                        ^^^
    |
    = note: `#[warn(unreachable_code)]` on by default
+note: any code following this expression is unreachable
+  --> $DIR/if-ret.rs:6:15
+   |
+LL | fn foo() { if (return) { } }
+   |               ^^^^^^^^
 
diff --git a/src/test/ui/issues/issue-2150.stderr b/src/test/ui/issues/issue-2150.stderr
@@ -9,6 +9,12 @@ note: lint level defined here
    |
 LL | #![deny(unreachable_code)]
    |         ^^^^^^^^^^^^^^^^
+note: any code following this expression is unreachable
+  --> $DIR/issue-2150.rs:7:5
+   |
+LL |     panic!();
+   |     ^^^^^^^^^
+   = 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
 
diff --git a/src/test/ui/issues/issue-7246.stderr b/src/test/ui/issues/issue-7246.stderr
@@ -9,6 +9,11 @@ note: lint level defined here
    |
 LL | #![deny(unreachable_code)]
    |         ^^^^^^^^^^^^^^^^
+note: any code following this expression is unreachable
+  --> $DIR/issue-7246.rs:6:5
+   |
+LL |     return;
+   |     ^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/lint/lint-attr-non-item-node.stderr b/src/test/ui/lint/lint-attr-non-item-node.stderr
@@ -9,6 +9,11 @@ note: lint level defined here
    |
 LL |     #[deny(unreachable_code)]
    |            ^^^^^^^^^^^^^^^^
+note: any code following this expression is unreachable
+  --> $DIR/lint-attr-non-item-node.rs:6:9
+   |
+LL |         break;
+   |         ^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/liveness/liveness-unused.stderr b/src/test/ui/liveness/liveness-unused.stderr
@@ -10,6 +10,11 @@ note: lint level defined here
 LL | #![warn(unused)]
    |         ^^^^^^
    = note: `#[warn(unreachable_code)]` implied by `#[warn(unused)]`
+note: any code following this expression is unreachable
+  --> $DIR/liveness-unused.rs:91:9
+   |
+LL |         continue;
+   |         ^^^^^^^^
 
 error: unused variable: `x`
   --> $DIR/liveness-unused.rs:8:7

diff --git a/src/test/ui/match/match-no-arms-unreachable-after.stderr b/src/test/ui/match/match-no-arms-unreachable-after.stderr
@@ -9,6 +9,11 @@ note: lint level defined here
    |
 LL | #![deny(unreachable_code)]
    |         ^^^^^^^^^^^^^^^^
+note: any code following this expression is unreachable
+  --> $DIR/match-no-arms-unreachable-after.rs:7:5
+   |
+LL |     match v { }
+   |     ^^^^^^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/never-assign-dead-code.stderr b/src/test/ui/never-assign-dead-code.stderr
@@ -10,12 +10,24 @@ note: lint level defined here
 LL | #![warn(unused)]
    |         ^^^^^^
    = note: `#[warn(unreachable_code)]` implied by `#[warn(unused)]`
+note: any code following this expression is unreachable
+  --> $DIR/never-assign-dead-code.rs:9:16
+   |
+LL |     let x: ! = panic!("aah");
+   |                ^^^^^^^^^^^^^
+   = note: this warning originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
 
 warning: unreachable call
   --> $DIR/never-assign-dead-code.rs:10:5
    |
 LL |     drop(x);
    |     ^^^^
+   |
+note: any code following this expression is unreachable
+  --> $DIR/never-assign-dead-code.rs:10:10
+   |
+LL |     drop(x);
+   |          ^
 
 warning: unused variable: `x`
   --> $DIR/never-assign-dead-code.rs:9:9

diff --git a/src/test/ui/reachable/expr_add.stderr b/src/test/ui/reachable/expr_add.stderr
@@ -9,6 +9,11 @@ note: lint level defined here
    |
 LL | #![deny(unreachable_code)]
    |         ^^^^^^^^^^^^^^^^
+note: any code following this expression is unreachable
+  --> $DIR/expr_add.rs:17:19
+   |
+LL |     let x = Foo + return;
+   |                   ^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/reachable/expr_again.stderr b/src/test/ui/reachable/expr_again.stderr
@@ -9,6 +9,11 @@ note: lint level defined here
    |
 LL | #![deny(unreachable_code)]
    |         ^^^^^^^^^^^^^^^^
+note: any code following this expression is unreachable
+  --> $DIR/expr_again.rs:7:9
+   |
+LL |         continue;
+   |         ^^^^^^^^
    = 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

diff --git a/src/test/ui/reachable/expr_array.stderr b/src/test/ui/reachable/expr_array.stderr
@@ -9,12 +9,23 @@ note: lint level defined here
    |
 LL | #![deny(unreachable_code)]
    |         ^^^^^^^^^^^^^^^^
+note: any code following this expression is unreachable
+  --> $DIR/expr_array.rs:9:26
+   |
+LL |     let x: [usize; 2] = [return, 22];
+   |                          ^^^^^^
 
 error: unreachable expression
   --> $DIR/expr_array.rs:14:25
    |
 LL |     let x: [usize; 2] = [22, return];
    |                         ^^^^^^^^^^^^
+   |
+note: any code following this expression is unreachable
+  --> $DIR/expr_array.rs:14:30
+   |
+LL |     let x: [usize; 2] = [22, return];
+   |                              ^^^^^^
 
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/reachable/expr_assign.stderr b/src/test/ui/reachable/expr_assign.stderr
@@ -9,18 +9,35 @@ note: lint level defined here
    |
 LL | #![deny(unreachable_code)]
    |         ^^^^^^^^^^^^^^^^
+note: any code following this expression is unreachable
+  --> $DIR/expr_assign.rs:10:9
+   |
+LL |     x = return;
+   |         ^^^^^^
 
 error: unreachable expression
   --> $DIR/expr_assign.rs:20:14
    |
 LL |         *p = return;
    |              ^^^^^^
+   |
+note: any code following this expression is unreachable
+  --> $DIR/expr_assign.rs:20:9
+   |
+LL |         *p = return;
+   |         ^^
 
 error: unreachable expression
   --> $DIR/expr_assign.rs:26:15
    |
 LL |     *{return; &mut i} = 22;
    |               ^^^^^^
+   |
+note: any code following this expression is unreachable
+  --> $DIR/expr_assign.rs:26:7
+   |
+LL |     *{return; &mut i} = 22;
+   |       ^^^^^^
 
 error: aborting due to 3 previous errors
 
diff --git a/src/test/ui/reachable/expr_block.stderr b/src/test/ui/reachable/expr_block.stderr
@@ -9,13 +9,23 @@ note: lint level defined here
    |
 LL | #![deny(unreachable_code)]
    |         ^^^^^^^^^^^^^^^^
+note: any code following this expression is unreachable
+  --> $DIR/expr_block.rs:9:9
+   |
+LL |         return;
+   |         ^^^^^^
 
 error: unreachable statement
   --> $DIR/expr_block.rs:25:9
    |
 LL |         println!("foo");
    |         ^^^^^^^^^^^^^^^^
    |
+note: any code following this expression is unreachable
+  --> $DIR/expr_block.rs:24:9
+   |
+LL |         return;
+   |         ^^^^^^
    = 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 2 previous errors

diff --git a/src/test/ui/reachable/expr_box.stderr b/src/test/ui/reachable/expr_box.stderr
@@ -9,6 +9,11 @@ note: lint level defined here
    |
 LL | #![deny(unreachable_code)]
    |         ^^^^^^^^^^^^^^^^
+note: any code following this expression is unreachable
+  --> $DIR/expr_box.rs:6:17
+   |
+LL |     let x = box return;
+   |                 ^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/reachable/expr_call.stderr b/src/test/ui/reachable/expr_call.stderr
@@ -9,12 +9,23 @@ note: lint level defined here
    |
 LL | #![deny(unreachable_code)]
    |         ^^^^^^^^^^^^^^^^
+note: any code following this expression is unreachable
+  --> $DIR/expr_call.rs:13:9
+   |
+LL |     foo(return, 22);
+   |         ^^^^^^
 
 error: unreachable call
   --> $DIR/expr_call.rs:18:5
    |
 LL |     bar(return);
    |     ^^^
+   |
+note: any code following this expression is unreachable
+  --> $DIR/expr_call.rs:18:9
+   |
+LL |     bar(return);
+   |         ^^^^^^
 
 error: aborting due to 2 previous errors
 
diff --git a/src/test/ui/reachable/expr_cast.stderr b/src/test/ui/reachable/expr_cast.stderr
@@ -9,6 +9,11 @@ note: lint level defined here
    |
 LL | #![deny(unreachable_code)]
    |         ^^^^^^^^^^^^^^^^
+note: any code following this expression is unreachable
+  --> $DIR/expr_cast.rs:9:14
+   |
+LL |     let x = {return} as !;
+   |              ^^^^^^
 
 error: aborting due to previous error
 
diff --git a/src/test/ui/reachable/expr_if.stderr b/src/test/ui/reachable/expr_if.stderr
@@ -12,13 +12,23 @@ note: lint level defined here
    |
 LL | #![deny(unreachable_code)]
    |         ^^^^^^^^^^^^^^^^
+note: any code following this expression is unreachable
+  --> $DIR/expr_if.rs:7:9
+   |
+LL |     if {return} {
+   |         ^^^^^^
 
 error: unreachable statement
   --> $DIR/expr_if.rs:27:5
    |
 LL |     println!("But I am.");
    |     ^^^^^^^^^^^^^^^^^^^^^^
    |
+note: any code following this expression is unreachable
+  --> $DIR/expr_if.rs:21:9
+   |
+LL |         return;
+   |         ^^^^^^
    = 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 2 previous errors