Skip to content

Commit 7b1d6e9

Browse files
committed
Consider labels of inline asm as conditionally executed
Since an `asm!()` statement is mostly unknown, as we do not know what it does, consider all labels' `break` as being conditionally executed only.
1 parent 544e24a commit 7b1d6e9

File tree

3 files changed

+94
-43
lines changed

3 files changed

+94
-43
lines changed

clippy_lints/src/loops/never_loop.rs

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use clippy_utils::source::snippet;
77
use clippy_utils::visitors::{Descend, for_each_expr_without_closures};
88
use rustc_errors::Applicability;
99
use rustc_hir::{
10-
Block, Destination, Expr, ExprKind, HirId, InlineAsmOperand, Node, Pat, Stmt, StmtKind, StructTailExpr,
10+
Block, Destination, Expr, ExprKind, HirId, InlineAsm, InlineAsmOperand, Node, Pat, Stmt, StmtKind, StructTailExpr,
1111
};
1212
use rustc_lint::LateContext;
1313
use rustc_span::{BytePos, Span, sym};
@@ -75,12 +75,19 @@ pub(super) fn check<'tcx>(
7575
fn contains_any_break_or_continue(block: &Block<'_>) -> bool {
7676
for_each_expr_without_closures(block, |e| match e.kind {
7777
ExprKind::Break(..) | ExprKind::Continue(..) => ControlFlow::Break(()),
78+
ExprKind::InlineAsm(asm) if contains_label(asm) => ControlFlow::Break(()),
7879
ExprKind::Loop(..) => ControlFlow::Continue(Descend::No),
7980
_ => ControlFlow::Continue(Descend::Yes),
8081
})
8182
.is_some()
8283
}
8384

85+
fn contains_label(asm: &InlineAsm<'_>) -> bool {
86+
asm.operands
87+
.iter()
88+
.any(|(op, _span)| matches!(op, InlineAsmOperand::Label { .. }))
89+
}
90+
8491
/// The `never_loop` analysis keeps track of three things:
8592
///
8693
/// * Has any (reachable) code path hit a `continue` of the main loop?
@@ -378,7 +385,15 @@ fn never_loop_expr<'tcx>(
378385
InlineAsmOperand::Const { .. } | InlineAsmOperand::SymFn { .. } | InlineAsmOperand::SymStatic { .. } => {
379386
NeverLoopResult::Normal
380387
},
381-
InlineAsmOperand::Label { block } => never_loop_block(cx, block, local_labels, main_loop_id),
388+
InlineAsmOperand::Label { block } =>
389+
// We do not know whether the label will be executed or not, so `Diverging` must be
390+
// downgraded to `Normal`.
391+
{
392+
match never_loop_block(cx, block, local_labels, main_loop_id) {
393+
NeverLoopResult::Diverging { .. } => NeverLoopResult::Normal,
394+
result => result,
395+
}
396+
},
382397
})),
383398
ExprKind::OffsetOf(_, _)
384399
| ExprKind::Yield(_, _)

tests/ui/never_loop.rs

Lines changed: 34 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
11
#![feature(try_blocks)]
2-
#![allow(
3-
clippy::eq_op,
4-
clippy::single_match,
5-
unused_assignments,
6-
unused_variables,
7-
clippy::while_immutable_condition
8-
)]
2+
#![expect(clippy::eq_op, clippy::single_match, clippy::while_immutable_condition)]
93
//@no-rustfix
4+
5+
use std::arch::asm;
6+
107
fn test1() {
118
let mut x = 0;
129
loop {
@@ -522,3 +519,33 @@ fn issue15350() {
522519
}
523520
}
524521
}
522+
523+
fn issue15673() {
524+
loop {
525+
unsafe {
526+
// No lint since we don't analyze the inside of the asm
527+
asm! {
528+
"/* {} */",
529+
label {
530+
break;
531+
}
532+
}
533+
}
534+
}
535+
536+
//~v never_loop
537+
loop {
538+
// We don't check the applicability in tests, but this should
539+
// be `Applicability::MaybeIncorrect` because of the `break`
540+
// in the `asm!()` labels.
541+
unsafe {
542+
asm! {
543+
"/* {} */",
544+
label {
545+
break;
546+
}
547+
}
548+
}
549+
return;
550+
}
551+
}

0 commit comments

Comments
 (0)