clippy_lints: Enable empty_loop lint for no_std crates

We use a different message for no_std crates, so that we avoid
recomending functions the user cannot use. We also update the
documentation to note that the remediations are different for `std`
and `no_std` crates.

Furthermore, we move the `empty_loop` lint to a "Correctness" lint,
as writing `loop {}` can cause miscompilations w/ LLVM.

Signed-off-by: Joe Richey <joerichey@google.com>

Author: josephlr

clippy_lints/src/lib.rs

@@ -1566,7 +1566,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&len_zero::LEN_WITHOUT_IS_EMPTY),
         LintId::of(&len_zero::LEN_ZERO),
         LintId::of(&literal_representation::INCONSISTENT_DIGIT_GROUPING),
-        LintId::of(&loops::EMPTY_LOOP),
         LintId::of(&loops::FOR_KV_MAP),
         LintId::of(&loops::NEEDLESS_RANGE_LOOP),
         LintId::of(&loops::SAME_ITEM_PUSH),
@@ -1752,6 +1751,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf:
         LintId::of(&inline_fn_without_body::INLINE_FN_WITHOUT_BODY),
         LintId::of(&let_underscore::LET_UNDERSCORE_LOCK),
         LintId::of(&literal_representation::MISTYPED_LITERAL_SUFFIXES),
+        LintId::of(&loops::EMPTY_LOOP),
         LintId::of(&loops::FOR_LOOPS_OVER_FALLIBLES),
         LintId::of(&loops::ITER_NEXT_LOOP),
         LintId::of(&loops::NEVER_LOOP),

clippy_lints/src/loops.rs

@@ -293,18 +293,55 @@ declare_clippy_lint! {
 declare_clippy_lint! {
     /// **What it does:** Checks for empty `loop` expressions.
     ///
-    /// **Why is this bad?** Those busy loops burn CPU cycles without doing
-    /// anything. Think of the environment and either block on something or at least
-    /// make the thread sleep for some microseconds.
+    /// **Why is this bad?** Due to [an LLVM codegen bug](https://github.com/rust-lang/rust/issues/28728)
+    /// these expressions can be miscompiled or 'optimized' out. Also, these busy loops
+    /// burn CPU cycles without doing anything.
     ///
-    /// **Known problems:** None.
+    /// It is _almost always_ a better idea to `panic!` than to have a busy loop.
+    ///
+    /// If panicking isn't possible, think of the environment and either:
+    ///   - block on something
+    ///   - sleep the thread for some microseconds
+    ///   - yield or pause the thread
+    ///
+    /// For `std` targets, this can be done with
+    /// [`std::thread::sleep`](https://doc.rust-lang.org/std/thread/fn.sleep.html)
+    /// or [`std::thread::yield_now`](https://doc.rust-lang.org/std/thread/fn.yield_now.html).
+    ///
+    /// For `no_std` targets, doing this is more complicated, especially because
+    /// `#[panic_handler]`s can't panic. To stop/pause the thread, you will
+    /// probably need to invoke some target-specific intrinsic. Examples include:
+    ///   - [`x86_64::instructions::hlt`](https://docs.rs/x86_64/0.12.2/x86_64/instructions/fn.hlt.html)
+    ///   - [`cortex_m::asm::wfi`](https://docs.rs/cortex-m/0.6.3/cortex_m/asm/fn.wfi.html)
+    ///
+    /// If you just care about fixing the LLVM bug (and don't care about burning
+    /// CPU), you can:
+    ///   - On nightly, insert a non-`pure` `asm!` statement. For example:
+    ///     ```rust
+    ///     loop {
+    ///         unsafe { asm!("") };
+    ///     }
+    ///     ```
+    ///     Alternatively, you can compile your code with `-Zinsert-sideeffect`,
+    ///     which will prevent the LLVM from miscompiling your code.
+    ///   - On stable, insert a [`read_volatile`](https://doc.rust-lang.org/core/ptr/fn.read_volatile.html)
+    ///     operation in the loop body. For example:
+    ///     ```rust
+    ///     let dummy = 0u8;
+    ///     loop {
+    ///         unsafe { core::ptr::read_volatile(&dummy) };
+    ///     }
+    ///     ```
+    ///
+    /// **Known problems:** This is a correctness lint due to the LLVM codegen
+    /// bug. Once the bug is fixed, this will go back to being a style lint.
     ///
     /// **Example:**
     /// ```no_run
     /// loop {}
     /// ```
     pub EMPTY_LOOP,
-    style,
+    correctness,
     "empty `loop {}`, which should block or sleep"
 }
 
@@ -502,14 +539,16 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
         // (even if the "match" or "if let" is used for declaration)
         if let ExprKind::Loop(ref block, _, LoopSource::Loop) = expr.kind {
             // also check for empty `loop {}` statements
-            if block.stmts.is_empty() && block.expr.is_none() && !is_no_std_crate(cx.tcx.hir().krate()) {
-                span_lint(
-                    cx,
-                    EMPTY_LOOP,
-                    expr.span,
-                    "empty `loop {}` detected. You may want to either use `panic!()` or add \
-                     `std::thread::sleep(..);` to the loop body.",
-                );
+            if block.stmts.is_empty() && block.expr.is_none() {
+                let msg = "empty `loop {}` is currently miscompiled and wastes CPU cycles";
+                let help = if is_no_std_crate(cx.tcx.hir().krate()) {
+                    "You should either use `panic!()` or add some call \
+                    to sleep or pause the thread to the loop body."
+                } else {
+                    "You should either use `panic!()` or add \
+                    `std::thread::sleep(..);` to the loop body."
+                };
+                span_lint_and_help(cx, EMPTY_LOOP, expr.span, msg, None, help);
             }
 
             // extract the expression from the first statement (if any) in a block

src/lintlist/mod.rs

@@ -482,7 +482,7 @@ vec![
     },
     Lint {
         name: "empty_loop",
-        group: "style",
+        group: "correctness",
         desc: "empty `loop {}`, which should block or sleep",
         deprecation: None,
         module: "loops",

tests/ui/crashes/ice-360.stderr

@@ -12,13 +12,14 @@ LL | |     }
    |
    = note: `-D clippy::while-let-loop` implied by `-D warnings`
 
-error: empty `loop {}` detected. You may want to either use `panic!()` or add `std::thread::sleep(..);` to the loop body.
+error: empty `loop {}` is currently miscompiled and wastes CPU cycles
   --> $DIR/ice-360.rs:10:9
    |
 LL |         loop {}
    |         ^^^^^^^
    |
-   = note: `-D clippy::empty-loop` implied by `-D warnings`
+   = note: `#[deny(clippy::empty_loop)]` on by default
+   = help: You should either use `panic!()` or add `std::thread::sleep(..);` to the loop body.
 
 error: aborting due to 2 previous errors
 

tests/ui/empty_loop.stderr

@@ -1,22 +1,27 @@
-error: empty `loop {}` detected. You may want to either use `panic!()` or add `std::thread::sleep(..);` to the loop body.
+error: empty `loop {}` is currently miscompiled and wastes CPU cycles
   --> $DIR/empty_loop.rs:9:5
    |
 LL |     loop {}
    |     ^^^^^^^
    |
    = note: `-D clippy::empty-loop` implied by `-D warnings`
+   = help: You should either use `panic!()` or add `std::thread::sleep(..);` to the loop body.
 
-error: empty `loop {}` detected. You may want to either use `panic!()` or add `std::thread::sleep(..);` to the loop body.
+error: empty `loop {}` is currently miscompiled and wastes CPU cycles
   --> $DIR/empty_loop.rs:11:9
    |
 LL |         loop {}
    |         ^^^^^^^
+   |
+   = help: You should either use `panic!()` or add `std::thread::sleep(..);` to the loop body.
 
-error: empty `loop {}` detected. You may want to either use `panic!()` or add `std::thread::sleep(..);` to the loop body.
+error: empty `loop {}` is currently miscompiled and wastes CPU cycles
   --> $DIR/empty_loop.rs:15:9
    |
 LL |         'inner: loop {}
    |         ^^^^^^^^^^^^^^^
+   |
+   = help: You should either use `panic!()` or add `std::thread::sleep(..);` to the loop body.
 
 error: aborting due to 3 previous errors
 

tests/ui/issue-3746.rs renamed to tests/ui/empty_loop_no_std.rs

@@ -10,11 +10,16 @@ use core::panic::PanicInfo;
 
 #[start]
 fn main(argc: isize, argv: *const *const u8) -> isize {
-    loop {}
+    // This loop should not cause a warning.
+    let dummy = 0u8;
+    loop {
+        unsafe { core::ptr::read_volatile(&dummy) };
+    }
 }
 
 #[panic_handler]
 fn panic(_info: &PanicInfo) -> ! {
+    // But this loop will cause a warning.
     loop {}
 }
 

tests/ui/empty_loop_no_std.stderr

@@ -0,0 +1,11 @@
+error: empty `loop {}` is currently miscompiled and wastes CPU cycles
+  --> $DIR/empty_loop_no_std.rs:23:5
+   |
+LL |     loop {}
+   |     ^^^^^^^
+   |
+   = note: `-D clippy::empty-loop` implied by `-D warnings`
+   = help: You should either use `panic!()` or add some call to sleep or pause the thread to the loop body.
+
+error: aborting due to previous error
+