Skip to content

Commit 971e435

Browse files
committed
Auto merge of #12543 - Xaeroxe:manual-clamp-const, r=xFrednet,GuillaumeGomez
restrict manual_clamp to const case, bring it out of nursery Implements the plan that I described in #9484 (comment) This does two things primarily 1. Restrict `manual_clamp` such that it will only trigger if we are able to guarantee that `clamp` won't panic at runtime. 2. Bring `manual_clamp` out of nursery status and move it into the complexity group. changelog: [`manual_clamp`]: Restrict this lint such that it only triggers if max and min are const, and max is greater than or equal to min. Then bring it out of the nursery group.
2 parents d928657 + 89588f4 commit 971e435

File tree

4 files changed

+577
-261
lines changed

4 files changed

+577
-261
lines changed

clippy_lints/src/manual_clamp.rs

+34-4
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use clippy_config::msrvs::{self, Msrv};
2+
use clippy_utils::consts::{constant, Constant};
23
use clippy_utils::diagnostics::{span_lint_and_then, span_lint_hir_and_then};
34
use clippy_utils::higher::If;
45
use clippy_utils::sugg::Sugg;
@@ -17,6 +18,7 @@ use rustc_middle::ty::Ty;
1718
use rustc_session::impl_lint_pass;
1819
use rustc_span::symbol::sym;
1920
use rustc_span::Span;
21+
use std::cmp::Ordering;
2022
use std::ops::Deref;
2123

2224
declare_clippy_lint! {
@@ -26,6 +28,11 @@ declare_clippy_lint! {
2628
/// ### Why is this bad?
2729
/// clamp is much shorter, easier to read, and doesn't use any control flow.
2830
///
31+
/// ### Limitations
32+
///
33+
/// This lint will only trigger if max and min are known at compile time, and max is
34+
/// greater than min.
35+
///
2936
/// ### Known issue(s)
3037
/// If the clamped variable is NaN this suggestion will cause the code to propagate NaN
3138
/// rather than returning either `max` or `min`.
@@ -80,7 +87,7 @@ declare_clippy_lint! {
8087
/// ```
8188
#[clippy::version = "1.66.0"]
8289
pub MANUAL_CLAMP,
83-
nursery,
90+
complexity,
8491
"using a clamp pattern instead of the clamp function"
8592
}
8693
impl_lint_pass!(ManualClamp => [MANUAL_CLAMP]);
@@ -103,6 +110,26 @@ struct ClampSuggestion<'tcx> {
103110
hir_with_ignore_attr: Option<HirId>,
104111
}
105112

113+
impl<'tcx> ClampSuggestion<'tcx> {
114+
/// This function will return true if and only if you can demonstrate at compile time that min
115+
/// is less than max.
116+
fn min_less_than_max(&self, cx: &LateContext<'tcx>) -> bool {
117+
let max_type = cx.typeck_results().expr_ty(self.params.max);
118+
let min_type = cx.typeck_results().expr_ty(self.params.min);
119+
if max_type != min_type {
120+
return false;
121+
}
122+
if let Some(max) = constant(cx, cx.typeck_results(), self.params.max)
123+
&& let Some(min) = constant(cx, cx.typeck_results(), self.params.min)
124+
&& let Some(ord) = Constant::partial_cmp(cx.tcx, max_type, &min, &max)
125+
{
126+
ord != Ordering::Greater
127+
} else {
128+
false
129+
}
130+
}
131+
}
132+
106133
#[derive(Debug)]
107134
struct InputMinMax<'tcx> {
108135
input: &'tcx Expr<'tcx>,
@@ -123,7 +150,7 @@ impl<'tcx> LateLintPass<'tcx> for ManualClamp {
123150
.or_else(|| is_match_pattern(cx, expr))
124151
.or_else(|| is_if_elseif_pattern(cx, expr));
125152
if let Some(suggestion) = suggestion {
126-
emit_suggestion(cx, &suggestion);
153+
maybe_emit_suggestion(cx, &suggestion);
127154
}
128155
}
129156
}
@@ -133,13 +160,16 @@ impl<'tcx> LateLintPass<'tcx> for ManualClamp {
133160
return;
134161
}
135162
for suggestion in is_two_if_pattern(cx, block) {
136-
emit_suggestion(cx, &suggestion);
163+
maybe_emit_suggestion(cx, &suggestion);
137164
}
138165
}
139166
extract_msrv_attr!(LateContext);
140167
}
141168

142-
fn emit_suggestion<'tcx>(cx: &LateContext<'tcx>, suggestion: &ClampSuggestion<'tcx>) {
169+
fn maybe_emit_suggestion<'tcx>(cx: &LateContext<'tcx>, suggestion: &ClampSuggestion<'tcx>) {
170+
if !suggestion.min_less_than_max(cx) {
171+
return;
172+
}
143173
let ClampSuggestion {
144174
params: InputMinMax {
145175
input,

0 commit comments

Comments
 (0)