new lint: missing-spin-loop

Author: llogiq

CHANGELOG.md

@@ -3120,6 +3120,7 @@ Released 2018-09-13
 [`missing_inline_in_public_items`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_inline_in_public_items
 [`missing_panics_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_panics_doc
 [`missing_safety_doc`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_safety_doc
+[`missing_spin_loop`]: https://rust-lang.github.io/rust-clippy/master/index.html#missing_spin_loop
 [`mistyped_literal_suffixes`]: https://rust-lang.github.io/rust-clippy/master/index.html#mistyped_literal_suffixes
 [`mixed_case_hex_literals`]: https://rust-lang.github.io/rust-clippy/master/index.html#mixed_case_hex_literals
 [`mod_module_files`]: https://rust-lang.github.io/rust-clippy/master/index.html#mod_module_files

clippy_lints/src/lib.register_all.rs

@@ -102,6 +102,7 @@ store.register_group(true, "clippy::all", Some("clippy_all"), vec![
     LintId::of(loops::ITER_NEXT_LOOP),
     LintId::of(loops::MANUAL_FLATTEN),
     LintId::of(loops::MANUAL_MEMCPY),
+    LintId::of(loops::MISSING_SPIN_LOOP),
     LintId::of(loops::MUT_RANGE_BOUND),
     LintId::of(loops::NEEDLESS_COLLECT),
     LintId::of(loops::NEEDLESS_RANGE_LOOP),

clippy_lints/src/lib.register_lints.rs

@@ -212,6 +212,7 @@ store.register_lints(&[
     loops::ITER_NEXT_LOOP,
     loops::MANUAL_FLATTEN,
     loops::MANUAL_MEMCPY,
+    loops::MISSING_SPIN_LOOP,
     loops::MUT_RANGE_BOUND,
     loops::NEEDLESS_COLLECT,
     loops::NEEDLESS_RANGE_LOOP,

clippy_lints/src/lib.register_perf.rs

@@ -10,6 +10,7 @@ store.register_group(true, "clippy::perf", Some("clippy_perf"), vec![
     LintId::of(large_const_arrays::LARGE_CONST_ARRAYS),
     LintId::of(large_enum_variant::LARGE_ENUM_VARIANT),
     LintId::of(loops::MANUAL_MEMCPY),
+    LintId::of(loops::MISSING_SPIN_LOOP),
     LintId::of(loops::NEEDLESS_COLLECT),
     LintId::of(methods::EXPECT_FUN_CALL),
     LintId::of(methods::EXTEND_WITH_DRAIN),

clippy_lints/src/loops/missing_spin_loop.rs

@@ -0,0 +1,33 @@
+use super::MISSING_SPIN_LOOP;
+use clippy_utils::diagnostics::span_lint_and_sugg;
+use clippy_utils::is_no_std_crate;
+use rustc_errors::Applicability;
+use rustc_hir::{Block, Expr, ExprKind};
+use rustc_lint::LateContext;
+//use rustc_middle::ty;
+use rustc_span::sym;
+
+pub(super) fn check<'tcx>(cx: &LateContext<'tcx>, cond: &'tcx Expr<'_>, body: &'tcx Expr<'_>) {
+    if_chain! {
+        if let ExprKind::Block(Block { stmts: [], expr: None, ..}, _) = body.kind;
+        if let ExprKind::MethodCall(method, _, [_callee, _ordering], _) = cond.kind;
+        if method.ident.name == sym::load;
+        //if let ty::Adt(def, _substs) = cx.typeck_results().expr_ty(callee).kind();
+        //if cx.tcx.is_diagnostic_item(sym::AtomicBool, def.did);
+        then {
+            span_lint_and_sugg(
+                cx,
+                MISSING_SPIN_LOOP,
+                body.span,
+                "busy-waiting loop should at least have a spin loop hint",
+                "try this",
+                (if is_no_std_crate(cx) {
+                    "{ core::hint::spin_loop() }"
+                }else {
+                    "{ std::hint::spin_loop() }"
+                }).into(),
+                Applicability::MachineApplicable
+            );
+        }
+    }
+}

clippy_lints/src/loops/mod.rs

@@ -7,6 +7,7 @@ mod for_loops_over_fallibles;
 mod iter_next_loop;
 mod manual_flatten;
 mod manual_memcpy;
+mod missing_spin_loop;
 mod mut_range_bound;
 mod needless_collect;
 mod needless_range_loop;
@@ -560,6 +561,37 @@ declare_clippy_lint! {
     "for loops over `Option`s or `Result`s with a single expression can be simplified"
 }
 
+declare_clippy_lint! {
+    /// ### What it does
+    /// Check for empty spin loops
+    ///
+    /// ### Why is this bad?
+    /// The loop body should have something like `thread::park()` or at least
+    /// `std::hint::spin_loop()` to avoid needlessly burning cycles and conserve
+    /// energy. Perhaps even better use an actual lock, if possible.
+    ///
+    /// ### Example
+    ///
+    /// ```rust
+    /// use core::sync::atomic::{AtomicBool, Ordering};
+    /// let b = AtomicBool::new(true);
+    /// // give a ref to `b` to another thread,wait for it to become false
+    /// while b.load(Ordering::Acquire) {};
+    /// ```
+    /// Use instead:
+    /// ```rust
+    ///# use core::sync::atomic::{AtomicBool, Ordering};
+    ///# let b = AtomicBool::new(true);
+    /// while b.load(Ordering::Acquire) {
+    ///     std::hint::spin_loop()
+    /// }
+    /// ```
+    #[clippy::version = "1.59.0"]
+    pub MISSING_SPIN_LOOP,
+    perf,
+    "An empty busy waiting loop"
+}
+
 declare_lint_pass!(Loops => [
     MANUAL_MEMCPY,
     MANUAL_FLATTEN,
@@ -579,6 +611,7 @@ declare_lint_pass!(Loops => [
     WHILE_IMMUTABLE_CONDITION,
     SAME_ITEM_PUSH,
     SINGLE_ELEMENT_LOOP,
+    MISSING_SPIN_LOOP,
 ]);
 
 impl<'tcx> LateLintPass<'tcx> for Loops {
@@ -628,6 +661,7 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
 
         if let Some(higher::While { condition, body }) = higher::While::hir(expr) {
             while_immutable_condition::check(cx, condition, body);
+            missing_spin_loop::check(cx, condition, body);
         }
 
         needless_collect::check(expr, cx);

tests/ui/missing_spin_loop.fixed

@@ -0,0 +1,17 @@
+// run-rustfix
+#![warn(clippy::missing_spin_loop)]
+
+use core::sync::atomic::{AtomicBool, Ordering};
+
+fn main() {
+    let b = AtomicBool::new(true);
+    // This should lint
+    while b.load(Ordering::Acquire) { std::hint::spin_loop() }
+
+    // This is OK, as the body is not empty
+    while b.load(Ordering::Acquire) {
+        std::hint::spin_loop()
+    }
+    // TODO: also match on compare_exchange(_weak), but that could be
+    // loop+match or while let
+}

tests/ui/missing_spin_loop.rs

@@ -0,0 +1,17 @@
+// run-rustfix
+#![warn(clippy::missing_spin_loop)]
+
+use core::sync::atomic::{AtomicBool, Ordering};
+
+fn main() {
+    let b = AtomicBool::new(true);
+    // This should lint
+    while b.load(Ordering::Acquire) {}
+
+    // This is OK, as the body is not empty
+    while b.load(Ordering::Acquire) {
+        std::hint::spin_loop()
+    }
+    // TODO: also match on compare_exchange(_weak), but that could be
+    // loop+match or while let
+}

tests/ui/missing_spin_loop.stderr

@@ -0,0 +1,10 @@
+error: busy-waiting loop should at least have a spin loop hint
+  --> $DIR/missing_spin_loop.rs:9:37
+   |
+LL |     while b.load(Ordering::Acquire) {}
+   |                                     ^^ help: try this: `{ std::hint::spin_loop() }`
+   |
+   = note: `-D clippy::missing-spin-loop` implied by `-D warnings`
+
+error: aborting due to previous error
+

tests/ui/missing_spin_loop_no_std.fixed

@@ -0,0 +1,23 @@
+// run-rustfix
+#![warn(clippy::missing_spin_loop)]
+#![feature(lang_items, start, libc)]
+#![no_std]
+
+use core::sync::atomic::{AtomicBool, Ordering};
+
+#[start]
+fn main(_argc: isize, _argv: *const *const u8) -> isize {
+    // This should trigger the lint
+    let b = AtomicBool::new(true);
+    // This should lint with `core::hint::spin_loop()`
+    while b.load(Ordering::Acquire) { core::hint::spin_loop() }
+    0
+}
+
+#[panic_handler]
+fn panic(_info: &core::panic::PanicInfo) -> ! {
+    loop {}
+}
+
+#[lang = "eh_personality"]
+extern "C" fn eh_personality() {}