Skip to content

Commit

Permalink
Enable empty_loop lint for no_std crates
Browse files Browse the repository at this point in the history
We skip the lint if the `loop {}` is in the `#[panic_handler]` as the
main recommendation we give is to panic, which obviously isn't
possible in a panic handler.

Signed-off-by: Joe Richey <[email protected]>
  • Loading branch information
josephlr committed Oct 26, 2020
1 parent afbac89 commit 38087a8
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 16 deletions.
28 changes: 13 additions & 15 deletions clippy_lints/src/loops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@ use crate::utils::sugg::Sugg;
use crate::utils::usage::{is_unused, mutated_variables};
use crate::utils::{
contains_name, get_enclosing_block, get_parent_expr, get_trait_def_id, has_iter_method, higher, implements_trait,
indent_of, is_integer_const, is_no_std_crate, is_refutable, is_type_diagnostic_item, last_path_segment,
match_trait_method, match_type, match_var, multispan_sugg, qpath_res, single_segment_path, snippet,
snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_help, span_lint_and_sugg,
span_lint_and_then, sugg, SpanlessEq,
indent_of, is_in_panic_handler, is_integer_const, is_no_std_crate, is_refutable, is_type_diagnostic_item,
last_path_segment, match_trait_method, match_type, match_var, multispan_sugg, qpath_res, single_segment_path,
snippet, snippet_with_applicability, snippet_with_macro_callsite, span_lint, span_lint_and_help,
span_lint_and_sugg, span_lint_and_then, sugg, SpanlessEq,
};
use if_chain::if_chain;
use rustc_ast::ast;
Expand Down Expand Up @@ -543,17 +543,15 @@ impl<'tcx> LateLintPass<'tcx> for Loops {
// (also matches an explicit "match" instead of "if let")
// (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
// TODO(issue #6161): Enable for no_std crates (outside of #[panic_handler])
if block.stmts.is_empty() && block.expr.is_none() && !is_no_std_crate(cx.tcx.hir().krate()) {
span_lint_and_help(
cx,
EMPTY_LOOP,
expr.span,
"empty `loop {}` wastes CPU cycles",
None,
"You should either use `panic!()` or add `std::thread::sleep(..);` to the loop body.",
);
// also check for empty `loop {}` statements, skipping those in #[panic_handler]
if block.stmts.is_empty() && block.expr.is_none() && !is_in_panic_handler(cx, expr) {
let msg = "empty `loop {}` wastes CPU cycles";
let help = if is_no_std_crate(cx.tcx.hir().krate()) {
"You should either use `panic!()` or add a call pausing or sleeping 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
Expand Down
14 changes: 14 additions & 0 deletions clippy_lints/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,6 +439,20 @@ pub fn is_entrypoint_fn(cx: &LateContext<'_>, def_id: DefId) -> bool {
.map_or(false, |(entry_fn_def_id, _)| def_id == entry_fn_def_id.to_def_id())
}

/// Returns `true` if the expression is in the program's `#[panic_handler]`.
pub fn is_in_panic_handler(cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
let parent = cx.tcx.hir().get_parent_item(e.hir_id);
if let Some(Node::Item(Item {
kind: ItemKind::Fn(..), ..
})) = cx.tcx.hir().find(parent)
{
let def_id = cx.tcx.hir().local_def_id(parent).to_def_id();
Some(def_id) == cx.tcx.lang_items().panic_impl()
} else {
false
}
}

/// Gets the name of the item the expression is in, if available.
pub fn get_item_name(cx: &LateContext<'_>, expr: &Expr<'_>) -> Option<Symbol> {
let parent_id = cx.tcx.hir().get_parent_item(expr.hir_id);
Expand Down
7 changes: 6 additions & 1 deletion tests/ui/empty_loop_no_std.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,18 @@ use core::panic::PanicInfo;

#[start]
fn main(argc: isize, argv: *const *const u8) -> isize {
// This should trigger the lint
loop {}
}

#[panic_handler]
fn panic(_info: &PanicInfo) -> ! {
// This should NOT trigger the lint
loop {}
}

#[lang = "eh_personality"]
extern "C" fn eh_personality() {}
extern "C" fn eh_personality() {
// This should also trigger the lint
loop {}
}
19 changes: 19 additions & 0 deletions tests/ui/empty_loop_no_std.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
error: empty `loop {}` wastes CPU cycles
--> $DIR/empty_loop_no_std.rs:14:5
|
LL | loop {}
| ^^^^^^^
|
= note: `-D clippy::empty-loop` implied by `-D warnings`
= help: You should either use `panic!()` or add a call pausing or sleeping the thread to the loop body.

error: empty `loop {}` wastes CPU cycles
--> $DIR/empty_loop_no_std.rs:26:5
|
LL | loop {}
| ^^^^^^^
|
= help: You should either use `panic!()` or add a call pausing or sleeping the thread to the loop body.

error: aborting due to 2 previous errors

0 comments on commit 38087a8

Please sign in to comment.