Skip to content

Commit 8c60295

Browse files
committed
Auto merge of #114937 - flip1995:clippy_beta_backport, r=Mark-Simulacrum
[beta] Clippy backports for ICE fixes This backports PRs to beta, that fix ICEs, some lint grouping and FP fixes. Namely: - rust-lang/rust-clippy#11191 - rust-lang/rust-clippy#11172 - rust-lang/rust-clippy#11130 - rust-lang/rust-clippy#11106 - rust-lang/rust-clippy#11104 - rust-lang/rust-clippy#11077 - rust-lang/rust-clippy#11070 (This PR is not synced to the Rust repo yet, but I will open a separate PR to get it into `master`, before beta is branched: #114938) - rust-lang/rust-clippy#11069 Kind of a big backport, but most of it is tests. r? `@Mark-Simulacrum` cc `@Manishearth`
2 parents ab197f4 + 48fca12 commit 8c60295

23 files changed

+528
-212
lines changed
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,12 @@
1-
use clippy_utils::diagnostics::span_lint_and_help;
1+
use clippy_utils::diagnostics::span_lint_and_then;
22
use clippy_utils::last_path_segment;
33
use clippy_utils::ty::{implements_trait, is_type_diagnostic_item};
4-
use if_chain::if_chain;
5-
64
use rustc_hir::{Expr, ExprKind};
75
use rustc_lint::LateContext;
86
use rustc_lint::LateLintPass;
97
use rustc_middle::ty;
8+
use rustc_middle::ty::print::with_forced_trimmed_paths;
9+
use rustc_middle::ty::GenericArgKind;
1010
use rustc_session::{declare_lint_pass, declare_tool_lint};
1111
use rustc_span::symbol::sym;
1212

@@ -15,58 +15,65 @@ declare_clippy_lint! {
1515
/// This lint warns when you use `Arc` with a type that does not implement `Send` or `Sync`.
1616
///
1717
/// ### Why is this bad?
18-
/// Wrapping a type in Arc doesn't add thread safety to the underlying data, so data races
19-
/// could occur when touching the underlying data.
18+
/// `Arc<T>` is only `Send`/`Sync` when `T` is [both `Send` and `Sync`](https://doc.rust-lang.org/std/sync/struct.Arc.html#impl-Send-for-Arc%3CT%3E),
19+
/// either `T` should be made `Send + Sync` or an `Rc` should be used instead of an `Arc`
2020
///
2121
/// ### Example
2222
/// ```rust
2323
/// # use std::cell::RefCell;
2424
/// # use std::sync::Arc;
2525
///
2626
/// fn main() {
27-
/// // This is safe, as `i32` implements `Send` and `Sync`.
27+
/// // This is fine, as `i32` implements `Send` and `Sync`.
2828
/// let a = Arc::new(42);
2929
///
30-
/// // This is not safe, as `RefCell` does not implement `Sync`.
30+
/// // `RefCell` is `!Sync`, so either the `Arc` should be replaced with an `Rc`
31+
/// // or the `RefCell` replaced with something like a `RwLock`
3132
/// let b = Arc::new(RefCell::new(42));
3233
/// }
3334
/// ```
3435
#[clippy::version = "1.72.0"]
3536
pub ARC_WITH_NON_SEND_SYNC,
36-
correctness,
37+
suspicious,
3738
"using `Arc` with a type that does not implement `Send` or `Sync`"
3839
}
3940
declare_lint_pass!(ArcWithNonSendSync => [ARC_WITH_NON_SEND_SYNC]);
4041

4142
impl LateLintPass<'_> for ArcWithNonSendSync {
4243
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &Expr<'_>) {
4344
let ty = cx.typeck_results().expr_ty(expr);
44-
if_chain! {
45-
if is_type_diagnostic_item(cx, ty, sym::Arc);
46-
if let ExprKind::Call(func, [arg]) = expr.kind;
47-
if let ExprKind::Path(func_path) = func.kind;
48-
if last_path_segment(&func_path).ident.name == sym::new;
49-
if let arg_ty = cx.typeck_results().expr_ty(arg);
50-
if !matches!(arg_ty.kind(), ty::Param(_));
51-
if !cx.tcx
52-
.lang_items()
53-
.sync_trait()
54-
.map_or(false, |id| implements_trait(cx, arg_ty, id, &[])) ||
55-
!cx.tcx
56-
.get_diagnostic_item(sym::Send)
57-
.map_or(false, |id| implements_trait(cx, arg_ty, id, &[]));
45+
if is_type_diagnostic_item(cx, ty, sym::Arc)
46+
&& let ExprKind::Call(func, [arg]) = expr.kind
47+
&& let ExprKind::Path(func_path) = func.kind
48+
&& last_path_segment(&func_path).ident.name == sym::new
49+
&& let arg_ty = cx.typeck_results().expr_ty(arg)
50+
// make sure that the type is not and does not contain any type parameters
51+
&& arg_ty.walk().all(|arg| {
52+
!matches!(arg.unpack(), GenericArgKind::Type(ty) if matches!(ty.kind(), ty::Param(_)))
53+
})
54+
&& let Some(send) = cx.tcx.get_diagnostic_item(sym::Send)
55+
&& let Some(sync) = cx.tcx.lang_items().sync_trait()
56+
&& let [is_send, is_sync] = [send, sync].map(|id| implements_trait(cx, arg_ty, id, &[]))
57+
&& !(is_send && is_sync)
58+
{
59+
span_lint_and_then(
60+
cx,
61+
ARC_WITH_NON_SEND_SYNC,
62+
expr.span,
63+
"usage of an `Arc` that is not `Send` or `Sync`",
64+
|diag| with_forced_trimmed_paths!({
65+
if !is_send {
66+
diag.note(format!("the trait `Send` is not implemented for `{arg_ty}`"));
67+
}
68+
if !is_sync {
69+
diag.note(format!("the trait `Sync` is not implemented for `{arg_ty}`"));
70+
}
71+
72+
diag.note(format!("required for `{ty}` to implement `Send` and `Sync`"));
5873

59-
then {
60-
span_lint_and_help(
61-
cx,
62-
ARC_WITH_NON_SEND_SYNC,
63-
expr.span,
64-
"usage of `Arc<T>` where `T` is not `Send` or `Sync`",
65-
None,
66-
"consider using `Rc<T>` instead or wrapping `T` in a std::sync type like \
67-
`Mutex<T>`",
68-
);
69-
}
74+
diag.help("consider using an `Rc` instead or wrapping the inner type with a `Mutex`");
75+
}
76+
));
7077
}
7178
}
7279
}

src/tools/clippy/clippy_lints/src/dereference.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1296,8 +1296,8 @@ fn referent_used_exactly_once<'tcx>(
12961296
possible_borrowers: &mut Vec<(LocalDefId, PossibleBorrowerMap<'tcx, 'tcx>)>,
12971297
reference: &Expr<'tcx>,
12981298
) -> bool {
1299-
let mir = enclosing_mir(cx.tcx, reference.hir_id);
1300-
if let Some(local) = expr_local(cx.tcx, reference)
1299+
if let Some(mir) = enclosing_mir(cx.tcx, reference.hir_id)
1300+
&& let Some(local) = expr_local(cx.tcx, reference)
13011301
&& let [location] = *local_assignments(mir, local).as_slice()
13021302
&& let Some(statement) = mir.basic_blocks[location.block].statements.get(location.statement_index)
13031303
&& let StatementKind::Assign(box (_, Rvalue::Ref(_, _, place))) = statement.kind

src/tools/clippy/clippy_lints/src/methods/unnecessary_literal_unwrap.rs

+31
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ use clippy_utils::{diagnostics::span_lint_and_then, is_res_lang_ctor, last_path_
22
use rustc_errors::Applicability;
33
use rustc_hir as hir;
44
use rustc_lint::LateContext;
5+
use rustc_middle::ty;
6+
use rustc_middle::ty::print::with_forced_trimmed_paths;
57

68
use super::UNNECESSARY_LITERAL_UNWRAP;
79

@@ -21,6 +23,7 @@ fn get_ty_from_args<'a>(args: Option<&'a [hir::GenericArg<'a>]>, index: usize) -
2123
}
2224
}
2325

26+
#[expect(clippy::too_many_lines)]
2427
pub(super) fn check(
2528
cx: &LateContext<'_>,
2629
expr: &hir::Expr<'_>,
@@ -62,6 +65,34 @@ pub(super) fn check(
6265
(expr.span.with_hi(args[0].span.lo()), "panic!(".to_string()),
6366
(expr.span.with_lo(args[0].span.hi()), ")".to_string()),
6467
]),
68+
("None", "unwrap_or_default", _) => {
69+
let ty = cx.typeck_results().expr_ty(expr);
70+
let default_ty_string = if let ty::Adt(def, ..) = ty.kind() {
71+
with_forced_trimmed_paths!(format!("{}", cx.tcx.def_path_str(def.did())))
72+
} else {
73+
"Default".to_string()
74+
};
75+
Some(vec![(expr.span, format!("{default_ty_string}::default()"))])
76+
},
77+
("None", "unwrap_or", _) => Some(vec![
78+
(expr.span.with_hi(args[0].span.lo()), String::new()),
79+
(expr.span.with_lo(args[0].span.hi()), String::new()),
80+
]),
81+
("None", "unwrap_or_else", _) => match args[0].kind {
82+
hir::ExprKind::Closure(hir::Closure {
83+
fn_decl:
84+
hir::FnDecl {
85+
output: hir::FnRetTy::DefaultReturn(span) | hir::FnRetTy::Return(hir::Ty { span, .. }),
86+
..
87+
},
88+
..
89+
}) => Some(vec![
90+
(expr.span.with_hi(span.hi()), String::new()),
91+
(expr.span.with_lo(args[0].span.hi()), String::new()),
92+
]),
93+
_ => None,
94+
},
95+
_ if call_args.is_empty() => None,
6596
(_, _, Some(_)) => None,
6697
("Ok", "unwrap_err", None) | ("Err", "unwrap", None) => Some(vec![
6798
(

src/tools/clippy/clippy_lints/src/missing_fields_in_debug.rs

+5-6
Original file line numberDiff line numberDiff line change
@@ -207,11 +207,10 @@ impl<'tcx> LateLintPass<'tcx> for MissingFieldsInDebug {
207207
if let ItemKind::Impl(Impl { of_trait: Some(trait_ref), self_ty, items, .. }) = item.kind
208208
&& let Res::Def(DefKind::Trait, trait_def_id) = trait_ref.path.res
209209
&& let TyKind::Path(QPath::Resolved(_, self_path)) = &self_ty.kind
210-
// don't trigger if self is a generic parameter, e.g. `impl<T> Debug for T`
211-
// this can only happen in core itself, where the trait is defined,
212-
// but it caused ICEs in the past:
213-
// https://github.com/rust-lang/rust-clippy/issues/10887
214-
&& !matches!(self_path.res, Res::Def(DefKind::TyParam, _))
210+
// make sure that the self type is either a struct, an enum or a union
211+
// this prevents ICEs such as when self is a type parameter or a primitive type
212+
// (see #10887, #11063)
213+
&& let Res::Def(DefKind::Struct | DefKind::Enum | DefKind::Union, self_path_did) = self_path.res
215214
&& cx.match_def_path(trait_def_id, &[sym::core, sym::fmt, sym::Debug])
216215
// don't trigger if this impl was derived
217216
&& !cx.tcx.has_attr(item.owner_id, sym::automatically_derived)
@@ -222,7 +221,7 @@ impl<'tcx> LateLintPass<'tcx> for MissingFieldsInDebug {
222221
&& let body = cx.tcx.hir().body(*body_id)
223222
&& let ExprKind::Block(block, _) = body.value.kind
224223
// inspect `self`
225-
&& let self_ty = cx.tcx.type_of(self_path.res.def_id()).skip_binder().peel_refs()
224+
&& let self_ty = cx.tcx.type_of(self_path_did).skip_binder().peel_refs()
226225
&& let Some(self_adt) = self_ty.ty_adt_def()
227226
&& let Some(self_def_id) = self_adt.did().as_local()
228227
&& let Some(Node::Item(self_item)) = cx.tcx.hir().find_by_def_id(self_def_id)

src/tools/clippy/clippy_lints/src/redundant_type_annotations.rs

+7-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
use clippy_utils::diagnostics::span_lint;
2+
use clippy_utils::is_lint_allowed;
23
use rustc_ast::LitKind;
34
use rustc_hir as hir;
5+
use rustc_hir::def::DefKind;
46
use rustc_lint::{LateContext, LateLintPass};
57
use rustc_middle::ty::Ty;
68
use rustc_session::{declare_lint_pass, declare_tool_lint};
@@ -45,8 +47,8 @@ fn is_same_type<'tcx>(cx: &LateContext<'tcx>, ty_resolved_path: hir::def::Res, f
4547
return primty.name() == func_return_type_sym;
4648
}
4749

48-
// type annotation is any other non generic type
49-
if let hir::def::Res::Def(_, defid) = ty_resolved_path
50+
// type annotation is a non generic type
51+
if let hir::def::Res::Def(DefKind::Struct | DefKind::Union | DefKind::Enum, defid) = ty_resolved_path
5052
&& let Some(annotation_ty) = cx.tcx.type_of(defid).no_bound_vars()
5153
{
5254
return annotation_ty == func_return_type;
@@ -130,8 +132,9 @@ fn extract_primty(ty_kind: &hir::TyKind<'_>) -> Option<hir::PrimTy> {
130132

131133
impl LateLintPass<'_> for RedundantTypeAnnotations {
132134
fn check_local<'tcx>(&mut self, cx: &LateContext<'tcx>, local: &'tcx rustc_hir::Local<'tcx>) {
133-
// type annotation part
134-
if !local.span.from_expansion()
135+
if !is_lint_allowed(cx, REDUNDANT_TYPE_ANNOTATIONS, local.hir_id)
136+
// type annotation part
137+
&& !local.span.from_expansion()
135138
&& let Some(ty) = &local.ty
136139

137140
// initialization part

src/tools/clippy/clippy_lints/src/tuple_array_conversions.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ declare_clippy_lint! {
3131
/// ```
3232
#[clippy::version = "1.72.0"]
3333
pub TUPLE_ARRAY_CONVERSIONS,
34-
complexity,
34+
nursery,
3535
"checks for tuple<=>array conversions that are not done with `.into()`"
3636
}
3737
impl_lint_pass!(TupleArrayConversions => [TUPLE_ARRAY_CONVERSIONS]);

src/tools/clippy/clippy_lints/src/useless_conversion.rs

+15-3
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use clippy_utils::ty::{is_copy, is_type_diagnostic_item, same_type_and_consts};
66
use clippy_utils::{get_parent_expr, is_trait_method, match_def_path, path_to_local, paths};
77
use if_chain::if_chain;
88
use rustc_errors::Applicability;
9+
use rustc_hir::def::DefKind;
910
use rustc_hir::def_id::DefId;
1011
use rustc_hir::{BindingAnnotation, Expr, ExprKind, HirId, MatchSource, Node, PatKind};
1112
use rustc_lint::{LateContext, LateLintPass};
@@ -40,6 +41,7 @@ declare_clippy_lint! {
4041
#[derive(Default)]
4142
pub struct UselessConversion {
4243
try_desugar_arm: Vec<HirId>,
44+
expn_depth: u32,
4345
}
4446

4547
impl_lint_pass!(UselessConversion => [USELESS_CONVERSION]);
@@ -106,6 +108,7 @@ fn into_iter_deep_call<'hir>(cx: &LateContext<'_>, mut expr: &'hir Expr<'hir>) -
106108
impl<'tcx> LateLintPass<'tcx> for UselessConversion {
107109
fn check_expr(&mut self, cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) {
108110
if e.span.from_expansion() {
111+
self.expn_depth += 1;
109112
return;
110113
}
111114

@@ -151,9 +154,14 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
151154
{
152155
if let Some(parent) = get_parent_expr(cx, e) {
153156
let parent_fn = match parent.kind {
154-
ExprKind::Call(recv, args) if let ExprKind::Path(ref qpath) = recv.kind => {
155-
cx.qpath_res(qpath, recv.hir_id).opt_def_id()
156-
.map(|did| (did, args, MethodOrFunction::Function))
157+
ExprKind::Call(recv, args)
158+
if let ExprKind::Path(ref qpath) = recv.kind
159+
&& let Some(did) = cx.qpath_res(qpath, recv.hir_id).opt_def_id()
160+
// make sure that the path indeed points to a fn-like item, so that
161+
// `fn_sig` does not ICE. (see #11065)
162+
&& cx.tcx.opt_def_kind(did).is_some_and(DefKind::is_fn_like) =>
163+
{
164+
Some((did, args, MethodOrFunction::Function))
157165
}
158166
ExprKind::MethodCall(.., args, _) => {
159167
cx.typeck_results().type_dependent_def_id(parent.hir_id)
@@ -169,6 +177,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
169177
&& let Some(&into_iter_param) = sig.inputs().get(kind.param_pos(arg_pos))
170178
&& let ty::Param(param) = into_iter_param.kind()
171179
&& let Some(span) = into_iter_bound(cx, parent_fn_did, into_iter_did, param.index)
180+
&& self.expn_depth == 0
172181
{
173182
// Get the "innermost" `.into_iter()` call, e.g. given this expression:
174183
// `foo.into_iter().into_iter()`
@@ -304,5 +313,8 @@ impl<'tcx> LateLintPass<'tcx> for UselessConversion {
304313
if Some(&e.hir_id) == self.try_desugar_arm.last() {
305314
self.try_desugar_arm.pop();
306315
}
316+
if e.span.from_expansion() {
317+
self.expn_depth -= 1;
318+
}
307319
}
308320
}

src/tools/clippy/clippy_utils/src/mir/mod.rs

+14-9
Original file line numberDiff line numberDiff line change
@@ -101,21 +101,26 @@ pub fn used_exactly_once(mir: &rustc_middle::mir::Body<'_>, local: rustc_middle:
101101

102102
/// Returns the `mir::Body` containing the node associated with `hir_id`.
103103
#[allow(clippy::module_name_repetitions)]
104-
pub fn enclosing_mir(tcx: TyCtxt<'_>, hir_id: HirId) -> &Body<'_> {
104+
pub fn enclosing_mir(tcx: TyCtxt<'_>, hir_id: HirId) -> Option<&Body<'_>> {
105105
let body_owner_local_def_id = tcx.hir().enclosing_body_owner(hir_id);
106-
tcx.optimized_mir(body_owner_local_def_id.to_def_id())
106+
if tcx.hir().body_owner_kind(body_owner_local_def_id).is_fn_or_closure() {
107+
Some(tcx.optimized_mir(body_owner_local_def_id.to_def_id()))
108+
} else {
109+
None
110+
}
107111
}
108112

109113
/// Tries to determine the `Local` corresponding to `expr`, if any.
110114
/// This function is expensive and should be used sparingly.
111115
pub fn expr_local(tcx: TyCtxt<'_>, expr: &Expr<'_>) -> Option<Local> {
112-
let mir = enclosing_mir(tcx, expr.hir_id);
113-
mir.local_decls.iter_enumerated().find_map(|(local, local_decl)| {
114-
if local_decl.source_info.span == expr.span {
115-
Some(local)
116-
} else {
117-
None
118-
}
116+
enclosing_mir(tcx, expr.hir_id).and_then(|mir| {
117+
mir.local_decls.iter_enumerated().find_map(|(local, local_decl)| {
118+
if local_decl.source_info.span == expr.span {
119+
Some(local)
120+
} else {
121+
None
122+
}
123+
})
119124
})
120125
}
121126

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

+11-4
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,18 @@ fn foo<T>(x: T) {
77
// Should not lint - purposefully ignoring generic args.
88
let a = Arc::new(x);
99
}
10+
fn issue11076<T>() {
11+
let a: Arc<Vec<T>> = Arc::new(Vec::new());
12+
}
1013

1114
fn main() {
12-
// This is safe, as `i32` implements `Send` and `Sync`.
13-
let a = Arc::new(42);
15+
let _ = Arc::new(42);
1416

15-
// This is not safe, as `RefCell` does not implement `Sync`.
16-
let b = Arc::new(RefCell::new(42));
17+
// !Sync
18+
let _ = Arc::new(RefCell::new(42));
19+
let mutex = Mutex::new(1);
20+
// !Send
21+
let _ = Arc::new(mutex.lock().unwrap());
22+
// !Send + !Sync
23+
let _ = Arc::new(&42 as *const i32);
1724
}
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,34 @@
1-
error: usage of `Arc<T>` where `T` is not `Send` or `Sync`
2-
--> $DIR/arc_with_non_send_sync.rs:16:13
1+
error: usage of an `Arc` that is not `Send` or `Sync`
2+
--> $DIR/arc_with_non_send_sync.rs:18:13
33
|
4-
LL | let b = Arc::new(RefCell::new(42));
4+
LL | let _ = Arc::new(RefCell::new(42));
55
| ^^^^^^^^^^^^^^^^^^^^^^^^^^
66
|
7-
= help: consider using `Rc<T>` instead or wrapping `T` in a std::sync type like `Mutex<T>`
7+
= note: the trait `Sync` is not implemented for `RefCell<i32>`
8+
= note: required for `Arc<RefCell<i32>>` to implement `Send` and `Sync`
9+
= help: consider using an `Rc` instead or wrapping the inner type with a `Mutex`
810
= note: `-D clippy::arc-with-non-send-sync` implied by `-D warnings`
911

10-
error: aborting due to previous error
12+
error: usage of an `Arc` that is not `Send` or `Sync`
13+
--> $DIR/arc_with_non_send_sync.rs:21:13
14+
|
15+
LL | let _ = Arc::new(mutex.lock().unwrap());
16+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
17+
|
18+
= note: the trait `Send` is not implemented for `MutexGuard<'_, i32>`
19+
= note: required for `Arc<MutexGuard<'_, i32>>` to implement `Send` and `Sync`
20+
= help: consider using an `Rc` instead or wrapping the inner type with a `Mutex`
21+
22+
error: usage of an `Arc` that is not `Send` or `Sync`
23+
--> $DIR/arc_with_non_send_sync.rs:23:13
24+
|
25+
LL | let _ = Arc::new(&42 as *const i32);
26+
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^
27+
|
28+
= note: the trait `Send` is not implemented for `*const i32`
29+
= note: the trait `Sync` is not implemented for `*const i32`
30+
= note: required for `Arc<*const i32>` to implement `Send` and `Sync`
31+
= help: consider using an `Rc` instead or wrapping the inner type with a `Mutex`
32+
33+
error: aborting due to 3 previous errors
1134

0 commit comments

Comments
 (0)