Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable empty_loop lint for no_std crates #6205

Merged
merged 3 commits into from
Nov 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
7 changes: 7 additions & 0 deletions clippy_lints/src/utils/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -468,6 +468,13 @@ 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);
let def_id = cx.tcx.hir().local_def_id(parent).to_def_id();
Some(def_id) == cx.tcx.lang_items().panic_impl()
}

/// 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
2 changes: 1 addition & 1 deletion tests/ui/crashes/ice-360.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ 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.
= help: you should either use `panic!()` or add `std::thread::sleep(..);` to the loop body

error: aborting due to 2 previous errors

6 changes: 3 additions & 3 deletions tests/ui/empty_loop.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -5,23 +5,23 @@ 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.
= help: you should either use `panic!()` or add `std::thread::sleep(..);` to the loop body

error: empty `loop {}` 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.
= help: you should either use `panic!()` or add `std::thread::sleep(..);` to the loop body

error: empty `loop {}` 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.
= help: you should either use `panic!()` or add `std::thread::sleep(..);` to the loop body

error: aborting due to 3 previous errors

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