Skip to content

Commit 28fd269

Browse files
committed
Auto merge of rust-lang#122510 - flip1995:clippy_backport, r=Mark-Simulacrum
[beta] Clippy backports Backports included in this PR: - rust-lang/rust-clippy#12276 Fixing the lint on some platforms before hitting stable - rust-lang/rust-clippy#12405 Respect MSRV before hitting stable - rust-lang/rust-clippy#12266 Fixing an (unlikely) ICE - rust-lang/rust-clippy#12177 Fixing FPs before hitting stable Each backport on its own might not warrant a backport, but the collection of those are nice QoL fixes for Clippy users, before those bugs hit stable. All of those commits are already on `master`. r? `@Mark-Simulacrum`
2 parents 2bcca41 + 046eb84 commit 28fd269

9 files changed

+200
-61
lines changed

src/tools/clippy/clippy_config/src/msrvs.rs

+1
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ msrv_aliases! {
2121
1,68,0 { PATH_MAIN_SEPARATOR_STR }
2222
1,65,0 { LET_ELSE, POINTER_CAST_CONSTNESS }
2323
1,62,0 { BOOL_THEN_SOME, DEFAULT_ENUM_ATTRIBUTE }
24+
1,59,0 { THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST }
2425
1,58,0 { FORMAT_ARGS_CAPTURE, PATTERN_TRAIT_CHAR_ARRAY }
2526
1,55,0 { SEEK_REWIND }
2627
1,54,0 { INTO_KEYS }

src/tools/clippy/clippy_lints/src/indexing_slicing.rs

+1
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,7 @@ impl<'tcx> LateLintPass<'tcx> for IndexingSlicing {
174174
// only `usize` index is legal in rust array index
175175
// leave other type to rustc
176176
if let Constant::Int(off) = constant
177+
&& off <= usize::MAX as u128
177178
&& let ty::Uint(utype) = cx.typeck_results().expr_ty(index).kind()
178179
&& *utype == ty::UintTy::Usize
179180
&& let ty::Array(_, s) = ty.kind()

src/tools/clippy/clippy_lints/src/thread_local_initializer_can_be_made_const.rs

+46-21
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,11 @@
1-
use clippy_config::msrvs::Msrv;
1+
use clippy_config::msrvs::{self, Msrv};
22
use clippy_utils::diagnostics::span_lint_and_sugg;
3-
use clippy_utils::fn_has_unsatisfiable_preds;
43
use clippy_utils::qualify_min_const_fn::is_min_const_fn;
54
use clippy_utils::source::snippet;
5+
use clippy_utils::{fn_has_unsatisfiable_preds, peel_blocks};
66
use rustc_errors::Applicability;
77
use rustc_hir::{intravisit, ExprKind};
8-
use rustc_lint::{LateContext, LateLintPass, LintContext};
9-
use rustc_middle::lint::in_external_macro;
8+
use rustc_lint::{LateContext, LateLintPass};
109
use rustc_session::impl_lint_pass;
1110
use rustc_span::sym::thread_local_macro;
1211

@@ -57,6 +56,31 @@ impl ThreadLocalInitializerCanBeMadeConst {
5756

5857
impl_lint_pass!(ThreadLocalInitializerCanBeMadeConst => [THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST]);
5958

59+
#[inline]
60+
fn is_thread_local_initializer(
61+
cx: &LateContext<'_>,
62+
fn_kind: rustc_hir::intravisit::FnKind<'_>,
63+
span: rustc_span::Span,
64+
) -> Option<bool> {
65+
let macro_def_id = span.source_callee()?.macro_def_id?;
66+
Some(
67+
cx.tcx.is_diagnostic_item(thread_local_macro, macro_def_id)
68+
&& matches!(fn_kind, intravisit::FnKind::ItemFn(..)),
69+
)
70+
}
71+
72+
#[inline]
73+
fn initializer_can_be_made_const(cx: &LateContext<'_>, defid: rustc_span::def_id::DefId, msrv: &Msrv) -> bool {
74+
// Building MIR for `fn`s with unsatisfiable preds results in ICE.
75+
if !fn_has_unsatisfiable_preds(cx, defid)
76+
&& let mir = cx.tcx.optimized_mir(defid)
77+
&& let Ok(()) = is_min_const_fn(cx.tcx, mir, msrv)
78+
{
79+
return true;
80+
}
81+
false
82+
}
83+
6084
impl<'tcx> LateLintPass<'tcx> for ThreadLocalInitializerCanBeMadeConst {
6185
fn check_fn(
6286
&mut self,
@@ -65,31 +89,32 @@ impl<'tcx> LateLintPass<'tcx> for ThreadLocalInitializerCanBeMadeConst {
6589
_: &'tcx rustc_hir::FnDecl<'tcx>,
6690
body: &'tcx rustc_hir::Body<'tcx>,
6791
span: rustc_span::Span,
68-
defid: rustc_span::def_id::LocalDefId,
92+
local_defid: rustc_span::def_id::LocalDefId,
6993
) {
70-
if in_external_macro(cx.sess(), span)
71-
&& let Some(callee) = span.source_callee()
72-
&& let Some(macro_def_id) = callee.macro_def_id
73-
&& cx.tcx.is_diagnostic_item(thread_local_macro, macro_def_id)
74-
&& let intravisit::FnKind::ItemFn(..) = fn_kind
75-
// Building MIR for `fn`s with unsatisfiable preds results in ICE.
76-
&& !fn_has_unsatisfiable_preds(cx, defid.to_def_id())
77-
&& let mir = cx.tcx.optimized_mir(defid.to_def_id())
78-
&& let Ok(()) = is_min_const_fn(cx.tcx, mir, &self.msrv)
79-
// this is the `__init` function emitted by the `thread_local!` macro
80-
// when the `const` keyword is not used. We avoid checking the `__init` directly
81-
// as that is not a public API.
82-
// we know that the function is const-qualifiable, so now we need only to get the
83-
// initializer expression to span-lint it.
94+
let defid = local_defid.to_def_id();
95+
if self.msrv.meets(msrvs::THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST)
96+
&& is_thread_local_initializer(cx, fn_kind, span).unwrap_or(false)
97+
// Some implementations of `thread_local!` include an initializer fn.
98+
// In the case of a const initializer, the init fn is also const,
99+
// so we can skip the lint in that case. This occurs only on some
100+
// backends due to conditional compilation:
101+
// https://doc.rust-lang.org/src/std/sys/common/thread_local/mod.rs.html
102+
// for details on this issue, see:
103+
// https://github.com/rust-lang/rust-clippy/pull/12276
104+
&& !cx.tcx.is_const_fn(defid)
105+
&& initializer_can_be_made_const(cx, defid, &self.msrv)
106+
// we know that the function is const-qualifiable, so now
107+
// we need only to get the initializer expression to span-lint it.
84108
&& let ExprKind::Block(block, _) = body.value.kind
85-
&& let Some(ret_expr) = block.expr
109+
&& let Some(unpeeled) = block.expr
110+
&& let ret_expr = peel_blocks(unpeeled)
86111
&& let initializer_snippet = snippet(cx, ret_expr.span, "thread_local! { ... }")
87112
&& initializer_snippet != "thread_local! { ... }"
88113
{
89114
span_lint_and_sugg(
90115
cx,
91116
THREAD_LOCAL_INITIALIZER_CAN_BE_MADE_CONST,
92-
ret_expr.span,
117+
unpeeled.span,
93118
"initializer for `thread_local` value can be made `const`",
94119
"replace with",
95120
format!("const {{ {initializer_snippet} }}"),

src/tools/clippy/clippy_lints/src/unconditional_recursion.rs

+47-39
Original file line numberDiff line numberDiff line change
@@ -69,14 +69,6 @@ fn span_error(cx: &LateContext<'_>, method_span: Span, expr: &Expr<'_>) {
6969
);
7070
}
7171

72-
fn get_ty_def_id(ty: Ty<'_>) -> Option<DefId> {
73-
match ty.peel_refs().kind() {
74-
ty::Adt(adt, _) => Some(adt.did()),
75-
ty::Foreign(def_id) => Some(*def_id),
76-
_ => None,
77-
}
78-
}
79-
8072
fn get_hir_ty_def_id<'tcx>(tcx: TyCtxt<'tcx>, hir_ty: rustc_hir::Ty<'tcx>) -> Option<DefId> {
8173
let TyKind::Path(qpath) = hir_ty.kind else { return None };
8274
match qpath {
@@ -131,21 +123,49 @@ fn get_impl_trait_def_id(cx: &LateContext<'_>, method_def_id: LocalDefId) -> Opt
131123
}
132124
}
133125

134-
#[allow(clippy::unnecessary_def_path)]
126+
/// When we have `x == y` where `x = &T` and `y = &T`, then that resolves to
127+
/// `<&T as PartialEq<&T>>::eq`, which is not the same as `<T as PartialEq<T>>::eq`,
128+
/// however we still would want to treat it the same, because we know that it's a blanket impl
129+
/// that simply delegates to the `PartialEq` impl with one reference removed.
130+
///
131+
/// Still, we can't just do `lty.peel_refs() == rty.peel_refs()` because when we have `x = &T` and
132+
/// `y = &&T`, this is not necessarily the same as `<T as PartialEq<T>>::eq`
133+
///
134+
/// So to avoid these FNs and FPs, we keep removing a layer of references from *both* sides
135+
/// until both sides match the expected LHS and RHS type (or they don't).
136+
fn matches_ty<'tcx>(
137+
mut left: Ty<'tcx>,
138+
mut right: Ty<'tcx>,
139+
expected_left: Ty<'tcx>,
140+
expected_right: Ty<'tcx>,
141+
) -> bool {
142+
while let (&ty::Ref(_, lty, _), &ty::Ref(_, rty, _)) = (left.kind(), right.kind()) {
143+
if lty == expected_left && rty == expected_right {
144+
return true;
145+
}
146+
left = lty;
147+
right = rty;
148+
}
149+
false
150+
}
151+
135152
fn check_partial_eq(cx: &LateContext<'_>, method_span: Span, method_def_id: LocalDefId, name: Ident, expr: &Expr<'_>) {
136-
let args = cx
137-
.tcx
138-
.instantiate_bound_regions_with_erased(cx.tcx.fn_sig(method_def_id).skip_binder())
139-
.inputs();
153+
let Some(sig) = cx
154+
.typeck_results()
155+
.liberated_fn_sigs()
156+
.get(cx.tcx.local_def_id_to_hir_id(method_def_id))
157+
else {
158+
return;
159+
};
160+
140161
// That has two arguments.
141-
if let [self_arg, other_arg] = args
142-
&& let Some(self_arg) = get_ty_def_id(*self_arg)
143-
&& let Some(other_arg) = get_ty_def_id(*other_arg)
162+
if let [self_arg, other_arg] = sig.inputs()
163+
&& let &ty::Ref(_, self_arg, _) = self_arg.kind()
164+
&& let &ty::Ref(_, other_arg, _) = other_arg.kind()
144165
// The two arguments are of the same type.
145-
&& self_arg == other_arg
146166
&& let Some(trait_def_id) = get_impl_trait_def_id(cx, method_def_id)
147167
// The trait is `PartialEq`.
148-
&& Some(trait_def_id) == get_trait_def_id(cx, &["core", "cmp", "PartialEq"])
168+
&& cx.tcx.is_diagnostic_item(sym::PartialEq, trait_def_id)
149169
{
150170
let to_check_op = if name.name == sym::eq {
151171
BinOpKind::Eq
@@ -154,31 +174,19 @@ fn check_partial_eq(cx: &LateContext<'_>, method_span: Span, method_def_id: Loca
154174
};
155175
let is_bad = match expr.kind {
156176
ExprKind::Binary(op, left, right) if op.node == to_check_op => {
157-
// Then we check if the left-hand element is of the same type as `self`.
158-
if let Some(left_ty) = cx.typeck_results().expr_ty_opt(left)
159-
&& let Some(left_id) = get_ty_def_id(left_ty)
160-
&& self_arg == left_id
161-
&& let Some(right_ty) = cx.typeck_results().expr_ty_opt(right)
162-
&& let Some(right_id) = get_ty_def_id(right_ty)
163-
&& other_arg == right_id
164-
{
165-
true
166-
} else {
167-
false
168-
}
177+
// Then we check if the LHS matches self_arg and RHS matches other_arg
178+
let left_ty = cx.typeck_results().expr_ty_adjusted(left);
179+
let right_ty = cx.typeck_results().expr_ty_adjusted(right);
180+
matches_ty(left_ty, right_ty, self_arg, other_arg)
169181
},
170-
ExprKind::MethodCall(segment, receiver, &[_arg], _) if segment.ident.name == name.name => {
171-
if let Some(ty) = cx.typeck_results().expr_ty_opt(receiver)
172-
&& let Some(ty_id) = get_ty_def_id(ty)
173-
&& self_arg != ty_id
174-
{
175-
// Since this called on a different type, the lint should not be
176-
// triggered here.
177-
return;
178-
}
182+
ExprKind::MethodCall(segment, receiver, [arg], _) if segment.ident.name == name.name => {
183+
let receiver_ty = cx.typeck_results().expr_ty_adjusted(receiver);
184+
let arg_ty = cx.typeck_results().expr_ty_adjusted(arg);
185+
179186
if let Some(fn_id) = cx.typeck_results().type_dependent_def_id(expr.hir_id)
180187
&& let Some(trait_id) = cx.tcx.trait_of_item(fn_id)
181188
&& trait_id == trait_def_id
189+
&& matches_ty(receiver_ty, arg_ty, self_arg, other_arg)
182190
{
183191
true
184192
} else {
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
#[allow(overflowing_literals, unconditional_panic, clippy::no_effect)]
2+
fn main() {
3+
let arr: [i32; 5] = [0; 5];
4+
arr[0xfffffe7ffffffffffffffffffffffff];
5+
}

src/tools/clippy/tests/ui/thread_local_initializer_can_be_made_const.fixed

+14
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,18 @@ fn main() {
2727
}
2828
//~^^^^ ERROR: initializer for `thread_local` value can be made `const`
2929
//~^^^ ERROR: initializer for `thread_local` value can be made `const`
30+
31+
thread_local! {
32+
static PEEL_ME: i32 = const { 1 };
33+
//~^ ERROR: initializer for `thread_local` value can be made `const`
34+
static PEEL_ME_MANY: i32 = const { { let x = 1; x * x } };
35+
//~^ ERROR: initializer for `thread_local` value can be made `const`
36+
}
37+
}
38+
39+
#[clippy::msrv = "1.58"]
40+
fn f() {
41+
thread_local! {
42+
static TLS: i32 = 1;
43+
}
3044
}

src/tools/clippy/tests/ui/thread_local_initializer_can_be_made_const.rs

+14
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,18 @@ fn main() {
2727
}
2828
//~^^^^ ERROR: initializer for `thread_local` value can be made `const`
2929
//~^^^ ERROR: initializer for `thread_local` value can be made `const`
30+
31+
thread_local! {
32+
static PEEL_ME: i32 = { 1 };
33+
//~^ ERROR: initializer for `thread_local` value can be made `const`
34+
static PEEL_ME_MANY: i32 = { let x = 1; x * x };
35+
//~^ ERROR: initializer for `thread_local` value can be made `const`
36+
}
37+
}
38+
39+
#[clippy::msrv = "1.58"]
40+
fn f() {
41+
thread_local! {
42+
static TLS: i32 = 1;
43+
}
3044
}

src/tools/clippy/tests/ui/thread_local_initializer_can_be_made_const.stderr

+13-1
Original file line numberDiff line numberDiff line change
@@ -25,5 +25,17 @@ error: initializer for `thread_local` value can be made `const`
2525
LL | static BUF_4_CAN_BE_MADE_CONST: RefCell<String> = RefCell::new(String::new());
2626
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: replace with: `const { RefCell::new(String::new()) }`
2727

28-
error: aborting due to 4 previous errors
28+
error: initializer for `thread_local` value can be made `const`
29+
--> $DIR/thread_local_initializer_can_be_made_const.rs:32:31
30+
|
31+
LL | static PEEL_ME: i32 = { 1 };
32+
| ^^^^^ help: replace with: `const { 1 }`
33+
34+
error: initializer for `thread_local` value can be made `const`
35+
--> $DIR/thread_local_initializer_can_be_made_const.rs:34:36
36+
|
37+
LL | static PEEL_ME_MANY: i32 = { let x = 1; x * x };
38+
| ^^^^^^^^^^^^^^^^^^^^ help: replace with: `const { { let x = 1; x * x } }`
39+
40+
error: aborting due to 6 previous errors
2941

src/tools/clippy/tests/ui/unconditional_recursion.rs

+59
Original file line numberDiff line numberDiff line change
@@ -288,4 +288,63 @@ impl PartialEq for S15<'_> {
288288
}
289289
}
290290

291+
mod issue12154 {
292+
struct MyBox<T>(T);
293+
294+
impl<T> std::ops::Deref for MyBox<T> {
295+
type Target = T;
296+
fn deref(&self) -> &T {
297+
&self.0
298+
}
299+
}
300+
301+
impl<T: PartialEq> PartialEq for MyBox<T> {
302+
fn eq(&self, other: &Self) -> bool {
303+
(**self).eq(&**other)
304+
}
305+
}
306+
307+
// Not necessarily related to the issue but another FP from the http crate that was fixed with it:
308+
// https://docs.rs/http/latest/src/http/header/name.rs.html#1424
309+
// We used to simply peel refs from the LHS and RHS, so we couldn't differentiate
310+
// between `PartialEq<T> for &T` and `PartialEq<&T> for T` impls.
311+
#[derive(PartialEq)]
312+
struct HeaderName;
313+
impl<'a> PartialEq<&'a HeaderName> for HeaderName {
314+
fn eq(&self, other: &&'a HeaderName) -> bool {
315+
*self == **other
316+
}
317+
}
318+
319+
impl<'a> PartialEq<HeaderName> for &'a HeaderName {
320+
fn eq(&self, other: &HeaderName) -> bool {
321+
*other == *self
322+
}
323+
}
324+
325+
// Issue #12181 but also fixed by the same PR
326+
struct Foo;
327+
328+
impl Foo {
329+
fn as_str(&self) -> &str {
330+
"Foo"
331+
}
332+
}
333+
334+
impl PartialEq for Foo {
335+
fn eq(&self, other: &Self) -> bool {
336+
self.as_str().eq(other.as_str())
337+
}
338+
}
339+
340+
impl<T> PartialEq<T> for Foo
341+
where
342+
for<'a> &'a str: PartialEq<T>,
343+
{
344+
fn eq(&self, other: &T) -> bool {
345+
(&self.as_str()).eq(other)
346+
}
347+
}
348+
}
349+
291350
fn main() {}

0 commit comments

Comments
 (0)