Skip to content

Commit 2e33bf6

Browse files
committed
Auto merge of #6938 - Y-Nak:refactor-types, r=flip1995
Refactor types r? `@flip1995` This is the last PR to close #6724 🎉 Also, this fixes #6936. changelog: `vec_box`: Fix FN in `const` or `static` changelog: `linkedlist`: Fix FN in `const` or `static` changelog: `option_option`: Fix FN in `const` or `static`
2 parents 0b76719 + 818f832 commit 2e33bf6

15 files changed

+1077
-1000
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,173 @@
1+
use rustc_hir::{BinOpKind, Expr, ExprKind};
2+
use rustc_lint::{LateContext, LateLintPass};
3+
use rustc_middle::ty;
4+
use rustc_session::{declare_lint_pass, declare_tool_lint};
5+
6+
use crate::consts::{constant, Constant};
7+
8+
use clippy_utils::comparisons::{normalize_comparison, Rel};
9+
use clippy_utils::diagnostics::span_lint_and_help;
10+
use clippy_utils::source::snippet;
11+
use clippy_utils::ty::is_isize_or_usize;
12+
use clippy_utils::{clip, int_bits, unsext};
13+
14+
declare_clippy_lint! {
15+
/// **What it does:** Checks for comparisons where one side of the relation is
16+
/// either the minimum or maximum value for its type and warns if it involves a
17+
/// case that is always true or always false. Only integer and boolean types are
18+
/// checked.
19+
///
20+
/// **Why is this bad?** An expression like `min <= x` may misleadingly imply
21+
/// that it is possible for `x` to be less than the minimum. Expressions like
22+
/// `max < x` are probably mistakes.
23+
///
24+
/// **Known problems:** For `usize` the size of the current compile target will
25+
/// be assumed (e.g., 64 bits on 64 bit systems). This means code that uses such
26+
/// a comparison to detect target pointer width will trigger this lint. One can
27+
/// use `mem::sizeof` and compare its value or conditional compilation
28+
/// attributes
29+
/// like `#[cfg(target_pointer_width = "64")] ..` instead.
30+
///
31+
/// **Example:**
32+
///
33+
/// ```rust
34+
/// let vec: Vec<isize> = Vec::new();
35+
/// if vec.len() <= 0 {}
36+
/// if 100 > i32::MAX {}
37+
/// ```
38+
pub ABSURD_EXTREME_COMPARISONS,
39+
correctness,
40+
"a comparison with a maximum or minimum value that is always true or false"
41+
}
42+
43+
declare_lint_pass!(AbsurdExtremeComparisons => [ABSURD_EXTREME_COMPARISONS]);
44+
45+
impl<'tcx> LateLintPass<'tcx> for AbsurdExtremeComparisons {
46+
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) {
47+
if let ExprKind::Binary(ref cmp, ref lhs, ref rhs) = expr.kind {
48+
if let Some((culprit, result)) = detect_absurd_comparison(cx, cmp.node, lhs, rhs) {
49+
if !expr.span.from_expansion() {
50+
let msg = "this comparison involving the minimum or maximum element for this \
51+
type contains a case that is always true or always false";
52+
53+
let conclusion = match result {
54+
AbsurdComparisonResult::AlwaysFalse => "this comparison is always false".to_owned(),
55+
AbsurdComparisonResult::AlwaysTrue => "this comparison is always true".to_owned(),
56+
AbsurdComparisonResult::InequalityImpossible => format!(
57+
"the case where the two sides are not equal never occurs, consider using `{} == {}` \
58+
instead",
59+
snippet(cx, lhs.span, "lhs"),
60+
snippet(cx, rhs.span, "rhs")
61+
),
62+
};
63+
64+
let help = format!(
65+
"because `{}` is the {} value for this type, {}",
66+
snippet(cx, culprit.expr.span, "x"),
67+
match culprit.which {
68+
ExtremeType::Minimum => "minimum",
69+
ExtremeType::Maximum => "maximum",
70+
},
71+
conclusion
72+
);
73+
74+
span_lint_and_help(cx, ABSURD_EXTREME_COMPARISONS, expr.span, msg, None, &help);
75+
}
76+
}
77+
}
78+
}
79+
}
80+
81+
enum ExtremeType {
82+
Minimum,
83+
Maximum,
84+
}
85+
86+
struct ExtremeExpr<'a> {
87+
which: ExtremeType,
88+
expr: &'a Expr<'a>,
89+
}
90+
91+
enum AbsurdComparisonResult {
92+
AlwaysFalse,
93+
AlwaysTrue,
94+
InequalityImpossible,
95+
}
96+
97+
fn is_cast_between_fixed_and_target<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> bool {
98+
if let ExprKind::Cast(ref cast_exp, _) = expr.kind {
99+
let precast_ty = cx.typeck_results().expr_ty(cast_exp);
100+
let cast_ty = cx.typeck_results().expr_ty(expr);
101+
102+
return is_isize_or_usize(precast_ty) != is_isize_or_usize(cast_ty);
103+
}
104+
105+
false
106+
}
107+
108+
fn detect_absurd_comparison<'tcx>(
109+
cx: &LateContext<'tcx>,
110+
op: BinOpKind,
111+
lhs: &'tcx Expr<'_>,
112+
rhs: &'tcx Expr<'_>,
113+
) -> Option<(ExtremeExpr<'tcx>, AbsurdComparisonResult)> {
114+
use AbsurdComparisonResult::{AlwaysFalse, AlwaysTrue, InequalityImpossible};
115+
use ExtremeType::{Maximum, Minimum};
116+
// absurd comparison only makes sense on primitive types
117+
// primitive types don't implement comparison operators with each other
118+
if cx.typeck_results().expr_ty(lhs) != cx.typeck_results().expr_ty(rhs) {
119+
return None;
120+
}
121+
122+
// comparisons between fix sized types and target sized types are considered unanalyzable
123+
if is_cast_between_fixed_and_target(cx, lhs) || is_cast_between_fixed_and_target(cx, rhs) {
124+
return None;
125+
}
126+
127+
let (rel, normalized_lhs, normalized_rhs) = normalize_comparison(op, lhs, rhs)?;
128+
129+
let lx = detect_extreme_expr(cx, normalized_lhs);
130+
let rx = detect_extreme_expr(cx, normalized_rhs);
131+
132+
Some(match rel {
133+
Rel::Lt => {
134+
match (lx, rx) {
135+
(Some(l @ ExtremeExpr { which: Maximum, .. }), _) => (l, AlwaysFalse), // max < x
136+
(_, Some(r @ ExtremeExpr { which: Minimum, .. })) => (r, AlwaysFalse), // x < min
137+
_ => return None,
138+
}
139+
},
140+
Rel::Le => {
141+
match (lx, rx) {
142+
(Some(l @ ExtremeExpr { which: Minimum, .. }), _) => (l, AlwaysTrue), // min <= x
143+
(Some(l @ ExtremeExpr { which: Maximum, .. }), _) => (l, InequalityImpossible), // max <= x
144+
(_, Some(r @ ExtremeExpr { which: Minimum, .. })) => (r, InequalityImpossible), // x <= min
145+
(_, Some(r @ ExtremeExpr { which: Maximum, .. })) => (r, AlwaysTrue), // x <= max
146+
_ => return None,
147+
}
148+
},
149+
Rel::Ne | Rel::Eq => return None,
150+
})
151+
}
152+
153+
fn detect_extreme_expr<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) -> Option<ExtremeExpr<'tcx>> {
154+
let ty = cx.typeck_results().expr_ty(expr);
155+
156+
let cv = constant(cx, cx.typeck_results(), expr)?.0;
157+
158+
let which = match (ty.kind(), cv) {
159+
(&ty::Bool, Constant::Bool(false)) | (&ty::Uint(_), Constant::Int(0)) => ExtremeType::Minimum,
160+
(&ty::Int(ity), Constant::Int(i)) if i == unsext(cx.tcx, i128::MIN >> (128 - int_bits(cx.tcx, ity)), ity) => {
161+
ExtremeType::Minimum
162+
},
163+
164+
(&ty::Bool, Constant::Bool(true)) => ExtremeType::Maximum,
165+
(&ty::Int(ity), Constant::Int(i)) if i == unsext(cx.tcx, i128::MAX >> (128 - int_bits(cx.tcx, ity)), ity) => {
166+
ExtremeType::Maximum
167+
},
168+
(&ty::Uint(uty), Constant::Int(i)) if clip(cx.tcx, u128::MAX, uty) == i => ExtremeType::Maximum,
169+
170+
_ => return None,
171+
};
172+
Some(ExtremeExpr { which, expr })
173+
}

0 commit comments

Comments
 (0)