Skip to content

Commit

Permalink
new lint: missing-spin-loop
Browse files Browse the repository at this point in the history
  • Loading branch information
llogiq committed Jan 14, 2022
1 parent 7a4acf9 commit 2b03d5e
Show file tree
Hide file tree
Showing 12 changed files with 171 additions and 0 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3122,6 +3122,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
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,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),
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,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,
Expand Down
1 change: 1 addition & 0 deletions clippy_lints/src/lib.register_perf.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
33 changes: 33 additions & 0 deletions clippy_lints/src/loops/missing_spin_loop.rs
Original file line number Diff line number Diff line change
@@ -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
);
}
}
}
34 changes: 34 additions & 0 deletions clippy_lints/src/loops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
///
/// ```ignore
/// 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,no_run
///# 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,
Expand All @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
17 changes: 17 additions & 0 deletions tests/ui/missing_spin_loop.fixed
Original file line number Diff line number Diff line change
@@ -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
}
17 changes: 17 additions & 0 deletions tests/ui/missing_spin_loop.rs
Original file line number Diff line number Diff line change
@@ -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
}
10 changes: 10 additions & 0 deletions tests/ui/missing_spin_loop.stderr
Original file line number Diff line number Diff line change
@@ -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

23 changes: 23 additions & 0 deletions tests/ui/missing_spin_loop_no_std.fixed
Original file line number Diff line number Diff line change
@@ -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() {}
23 changes: 23 additions & 0 deletions tests/ui/missing_spin_loop_no_std.rs
Original file line number Diff line number Diff line change
@@ -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) {}
0
}

#[panic_handler]
fn panic(_info: &core::panic::PanicInfo) -> ! {
loop {}
}

#[lang = "eh_personality"]
extern "C" fn eh_personality() {}
10 changes: 10 additions & 0 deletions tests/ui/missing_spin_loop_no_std.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
error: busy-waiting loop should at least have a spin loop hint
--> $DIR/missing_spin_loop_no_std.rs:13:37
|
LL | while b.load(Ordering::Acquire) {}
| ^^ help: try this: `{ core::hint::spin_loop() }`
|
= note: `-D clippy::missing-spin-loop` implied by `-D warnings`

error: aborting due to previous error

0 comments on commit 2b03d5e

Please sign in to comment.