Skip to content

Commit 79499c8

Browse files
perf: run structural checks before const context queries in question_mark, manual_clamp and ranges (#17275)
Gate the const context, lint level and MSRV queries in the `question_mark`, `manual_clamp` and `ranges` passes behind a cheap expression/statement kind check, match method names before the type queries in `is_max_min_pattern`, and parse each range expression once instead of up to three times. | crate | instructions base | instructions new | delta | |---|---|---|---| | serde-1.0.204 | 7,203,361,007 | 7,134,072,760 | -0.96% | | syn-2.0.71 | 3,397,540,719 | 3,354,632,677 | -1.26% | | cargo-0.80.0 (lib) | 29,891,566,078 | 29,339,576,071 | -1.85% | | ryu-1.0.18 | 271,040,916 | 265,667,969 | -1.98% | changelog: none
2 parents 0354717 + dbfa2c3 commit 79499c8

3 files changed

Lines changed: 61 additions & 31 deletions

File tree

clippy_lints/src/manual_clamp.rs

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,13 @@ struct InputMinMax<'tcx> {
140140

141141
impl<'tcx> LateLintPass<'tcx> for ManualClamp {
142142
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
143-
if !expr.span.from_expansion() && !is_in_const_context(cx) {
143+
// Cheap kind check before the costlier const context query.
144+
if matches!(
145+
expr.kind,
146+
ExprKind::If(..) | ExprKind::Match(..) | ExprKind::MethodCall(..) | ExprKind::Call(..)
147+
) && !expr.span.from_expansion()
148+
&& !is_in_const_context(cx)
149+
{
144150
let suggestion = is_if_elseif_else_pattern(cx, expr)
145151
.or_else(|| is_max_min_pattern(cx, expr))
146152
.or_else(|| is_call_max_min_pattern(cx, expr))
@@ -155,6 +161,15 @@ impl<'tcx> LateLintPass<'tcx> for ManualClamp {
155161
}
156162

157163
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'tcx>) {
164+
// Cheap `if`-statement check before the costlier const context query.
165+
if !block
166+
.stmts
167+
.iter()
168+
.any(|stmt| matches!(stmt.kind, StmtKind::Expr(e) if matches!(e.kind, ExprKind::If(..))))
169+
{
170+
return;
171+
}
172+
158173
if is_in_const_context(cx) || !self.msrv.meets(cx, msrvs::CLAMP) {
159174
return;
160175
}
@@ -293,18 +308,19 @@ fn is_if_elseif_else_pattern<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx
293308
/// ```
294309
fn is_max_min_pattern<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<ClampSuggestion<'tcx>> {
295310
if let ExprKind::MethodCall(seg_second, receiver, [arg_second], _) = expr.kind
311+
&& let ExprKind::MethodCall(seg_first, input, [arg_first], _) = &receiver.kind
312+
// Match method names before the costlier type queries.
313+
&& let Some((min, max)) = match (seg_first.ident.name, seg_second.ident.name) {
314+
(sym::min, sym::max) => Some((arg_second, arg_first)),
315+
(sym::max, sym::min) => Some((arg_first, arg_second)),
316+
_ => None,
317+
}
296318
&& (cx.typeck_results().expr_ty_adjusted(receiver).is_floating_point()
297319
|| cx.ty_based_def(expr).assoc_fn_parent(cx).is_diag_item(cx, sym::Ord))
298-
&& let ExprKind::MethodCall(seg_first, input, [arg_first], _) = &receiver.kind
299320
&& (cx.typeck_results().expr_ty_adjusted(input).is_floating_point()
300321
|| cx.ty_based_def(receiver).assoc_fn_parent(cx).is_diag_item(cx, sym::Ord))
301322
{
302323
let is_float = cx.typeck_results().expr_ty_adjusted(input).is_floating_point();
303-
let (min, max) = match (seg_first.ident.name, seg_second.ident.name) {
304-
(sym::min, sym::max) => (arg_second, arg_first),
305-
(sym::max, sym::min) => (arg_first, arg_second),
306-
_ => return None,
307-
};
308324
Some(ClampSuggestion {
309325
params: InputMinMax {
310326
input,

clippy_lints/src/question_mark.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -629,6 +629,11 @@ fn is_inferred_ret_closure(expr: &Expr<'_>) -> bool {
629629

630630
impl<'tcx> LateLintPass<'tcx> for QuestionMark {
631631
fn check_stmt(&mut self, cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'_>) {
632+
// Cheap `let` check before the costlier lint level and const context queries.
633+
if !matches!(stmt.kind, StmtKind::Let(..)) {
634+
return;
635+
}
636+
632637
if !is_lint_allowed(cx, QUESTION_MARK_USED, stmt.hir_id) || !self.msrv.meets(cx, msrvs::QUESTION_MARK_OPERATOR)
633638
{
634639
return;
@@ -646,7 +651,9 @@ impl<'tcx> LateLintPass<'tcx> for QuestionMark {
646651
return;
647652
}
648653

649-
if !self.inside_try_block()
654+
// Cheap `if`/`match` check before the costlier lint level and const context queries.
655+
if matches!(expr.kind, ExprKind::If(..) | ExprKind::Match(..))
656+
&& !self.inside_try_block()
650657
&& !is_in_const_context(cx)
651658
&& is_lint_allowed(cx, QUESTION_MARK_USED, expr.hir_id)
652659
&& self.msrv.meets(cx, msrvs::QUESTION_MARK_OPERATOR)

clippy_lints/src/ranges.rs

Lines changed: 30 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -191,14 +191,21 @@ impl Ranges {
191191
impl<'tcx> LateLintPass<'tcx> for Ranges {
192192
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
193193
if let ExprKind::Binary(ref op, l, r) = expr.kind
194+
&& matches!(
195+
op.node,
196+
BinOpKind::And | BinOpKind::BitAnd | BinOpKind::Or | BinOpKind::BitOr
197+
)
194198
&& self.msrv.meets(cx, msrvs::RANGE_CONTAINS)
199+
&& !is_in_const_context(cx)
195200
{
196201
check_possible_range_contains(cx, op.node, l, r, expr, expr.span);
197202
}
198203

199-
check_exclusive_range_plus_one(cx, expr);
200-
check_inclusive_range_minus_one(cx, expr);
201-
check_reversed_empty_range(cx, expr);
204+
if let Some(range) = higher::Range::hir(cx, expr) {
205+
check_exclusive_range_plus_one(cx, expr, &range);
206+
check_inclusive_range_minus_one(cx, expr, &range);
207+
check_reversed_empty_range(cx, expr, &range);
208+
}
202209
}
203210
}
204211

@@ -210,10 +217,6 @@ fn check_possible_range_contains(
210217
expr: &Expr<'_>,
211218
span: Span,
212219
) {
213-
if is_in_const_context(cx) {
214-
return;
215-
}
216-
217220
let combine_and = match op {
218221
BinOpKind::And | BinOpKind::BitAnd => true,
219222
BinOpKind::Or | BinOpKind::BitOr => false,
@@ -481,54 +484,58 @@ fn can_switch_ranges<'tcx>(
481484
}
482485

483486
// exclusive range plus one: `x..(y+1)`
484-
fn check_exclusive_range_plus_one<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
487+
fn check_exclusive_range_plus_one<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, range: &higher::Range<'tcx>) {
485488
check_range_switch(
486489
cx,
487490
expr,
491+
range,
488492
RangeLimits::HalfOpen,
489493
y_plus_one,
490494
RANGE_PLUS_ONE,
491495
"an inclusive range would be more readable",
492-
"..=",
493496
);
494497
}
495498

496499
// inclusive range minus one: `x..=(y-1)`
497-
fn check_inclusive_range_minus_one<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
500+
fn check_inclusive_range_minus_one<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>, range: &higher::Range<'tcx>) {
498501
check_range_switch(
499502
cx,
500503
expr,
504+
range,
501505
RangeLimits::Closed,
502506
y_minus_one,
503507
RANGE_MINUS_ONE,
504508
"an exclusive range would be more readable",
505-
"..",
506509
);
507510
}
508511

509512
/// Check for a `kind` of range in `expr`, check for `predicate` on the end,
510-
/// and emit the `lint` with `msg` and the `operator`.
513+
/// and emit the `lint` with `msg`, suggesting the opposite range limits.
511514
fn check_range_switch<'tcx>(
512515
cx: &LateContext<'tcx>,
513516
expr: &'tcx Expr<'_>,
517+
range: &higher::Range<'tcx>,
514518
kind: RangeLimits,
515519
predicate: impl for<'hir> FnOnce(&Expr<'hir>) -> Option<&'hir Expr<'hir>>,
516520
lint: &'static Lint,
517521
msg: &'static str,
518-
operator: &str,
519522
) {
520-
if let Some(range) = higher::Range::hir(cx, expr)
521-
&& let higher::Range {
522-
start,
523-
end: Some(end),
524-
limits,
525-
span,
526-
} = range
523+
if let higher::Range {
524+
start,
525+
end: Some(end),
526+
limits,
527+
span,
528+
} = *range
527529
&& span.can_be_used_for_suggestions()
528530
&& limits == kind
529531
&& let Some(y) = predicate(end)
530532
&& can_switch_ranges(cx, span.ctxt(), expr, kind, cx.typeck_results().expr_ty(y))
531533
{
534+
// Suggest the opposite range limits to the ones being checked.
535+
let operator = match kind {
536+
RangeLimits::HalfOpen => "..=",
537+
RangeLimits::Closed => "..",
538+
};
532539
span_lint_and_then(cx, lint, span, msg, |diag| {
533540
let mut app = Applicability::MachineApplicable;
534541
let start = start.map_or(String::new(), |x| {
@@ -550,7 +557,7 @@ fn check_range_switch<'tcx>(
550557
}
551558
}
552559

553-
fn check_reversed_empty_range(cx: &LateContext<'_>, expr: &Expr<'_>) {
560+
fn check_reversed_empty_range(cx: &LateContext<'_>, expr: &Expr<'_>, range: &higher::Range<'_>) {
554561
fn inside_indexing_expr(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool {
555562
matches!(
556563
get_parent_expr(cx, expr),
@@ -580,12 +587,12 @@ fn check_reversed_empty_range(cx: &LateContext<'_>, expr: &Expr<'_>) {
580587
}
581588
}
582589

583-
if let Some(higher::Range {
590+
if let higher::Range {
584591
start: Some(start),
585592
end: Some(end),
586593
limits,
587594
span,
588-
}) = higher::Range::hir(cx, expr)
595+
} = *range
589596
&& let ty = cx.typeck_results().expr_ty(start)
590597
&& let ty::Int(_) | ty::Uint(_) = ty.kind()
591598
&& let ecx = ConstEvalCtxt::new(cx)

0 commit comments

Comments
 (0)