Skip to content

Commit 112a4af

Browse files
committed
Auto merge of #149656 - flip1995:clippy-beta-backport, r=Mark-Simulacrum
[beta] Clippy beta backport All 3 PRs are fixing bugs that were introduced in 1.92 (current beta) and would land on the next stable. - rust-lang/rust-clippy#16079 - rust-lang/rust-clippy#15984 - rust-lang/rust-clippy#15939 I verified that all of those commits are already synced to `main`. r? `@Mark-Simulacrum`
2 parents 5cd7b31 + f5e067b commit 112a4af

14 files changed

+822
-82
lines changed

src/tools/clippy/clippy_lints/src/double_parens.rs

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_utils::diagnostics::span_lint_and_sugg;
2-
use clippy_utils::source::{HasSession, snippet_with_applicability, snippet_with_context};
2+
use clippy_utils::source::{HasSession, SpanRangeExt, snippet_with_applicability, snippet_with_context};
33
use rustc_ast::ast::{Expr, ExprKind, MethodCall};
44
use rustc_errors::Applicability;
55
use rustc_lint::{EarlyContext, EarlyLintPass};
@@ -47,8 +47,12 @@ impl EarlyLintPass for DoubleParens {
4747
// ^^^^^^ expr
4848
// ^^^^ inner
4949
ExprKind::Paren(inner) if matches!(inner.kind, ExprKind::Paren(_) | ExprKind::Tup(_)) => {
50-
// suggest removing the outer parens
51-
if expr.span.eq_ctxt(inner.span) {
50+
if expr.span.eq_ctxt(inner.span)
51+
&& !expr.span.in_external_macro(cx.sess().source_map())
52+
&& check_source(cx, inner)
53+
{
54+
// suggest removing the outer parens
55+
5256
let mut applicability = Applicability::MachineApplicable;
5357
// We don't need to use `snippet_with_context` here, because:
5458
// - if `inner`'s `ctxt` is from macro, we don't lint in the first place (see the check above)
@@ -74,8 +78,12 @@ impl EarlyLintPass for DoubleParens {
7478
if let [arg] = &**args
7579
&& let ExprKind::Paren(inner) = &arg.kind =>
7680
{
77-
// suggest removing the inner parens
78-
if expr.span.eq_ctxt(arg.span) {
81+
if expr.span.eq_ctxt(arg.span)
82+
&& !arg.span.in_external_macro(cx.sess().source_map())
83+
&& check_source(cx, arg)
84+
{
85+
// suggest removing the inner parens
86+
7987
let mut applicability = Applicability::MachineApplicable;
8088
let sugg = snippet_with_context(cx.sess(), inner.span, arg.span.ctxt(), "_", &mut applicability).0;
8189
span_lint_and_sugg(
@@ -93,3 +101,22 @@ impl EarlyLintPass for DoubleParens {
93101
}
94102
}
95103
}
104+
105+
/// Check that the span does indeed look like `( (..) )`
106+
fn check_source(cx: &EarlyContext<'_>, inner: &Expr) -> bool {
107+
if let Some(sfr) = inner.span.get_source_range(cx)
108+
// this is the same as `SourceFileRange::as_str`, but doesn't apply the range right away, because
109+
// we're interested in the source code outside it
110+
&& let Some(src) = sfr.sf.src.as_ref().map(|src| src.as_str())
111+
&& let Some((start, outer_after_inner)) = src.split_at_checked(sfr.range.end)
112+
&& let Some((outer_before_inner, inner)) = start.split_at_checked(sfr.range.start)
113+
&& outer_before_inner.trim_end().ends_with('(')
114+
&& inner.starts_with('(')
115+
&& inner.ends_with(')')
116+
&& outer_after_inner.trim_start().starts_with(')')
117+
{
118+
true
119+
} else {
120+
false
121+
}
122+
}

src/tools/clippy/clippy_lints/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -828,6 +828,6 @@ pub fn register_lint_passes(store: &mut rustc_lint::LintStore, conf: &'static Co
828828
store.register_late_pass(|_| Box::new(coerce_container_to_any::CoerceContainerToAny));
829829
store.register_late_pass(|_| Box::new(toplevel_ref_arg::ToplevelRefArg));
830830
store.register_late_pass(|_| Box::new(volatile_composites::VolatileComposites));
831-
store.register_late_pass(|_| Box::new(replace_box::ReplaceBox));
831+
store.register_late_pass(|_| Box::new(replace_box::ReplaceBox::default()));
832832
// add lints here, do not remove this comment, it's used in `new_lint`
833833
}

src/tools/clippy/clippy_lints/src/multiple_unsafe_ops_per_block.rs

Lines changed: 78 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,13 @@
11
use clippy_utils::desugar_await;
22
use clippy_utils::diagnostics::span_lint_and_then;
3-
use clippy_utils::visitors::{Descend, Visitable, for_each_expr};
4-
use core::ops::ControlFlow::Continue;
53
use hir::def::{DefKind, Res};
64
use hir::{BlockCheckMode, ExprKind, QPath, UnOp};
7-
use rustc_ast::Mutability;
5+
use rustc_ast::{BorrowKind, Mutability};
86
use rustc_hir as hir;
7+
use rustc_hir::intravisit::{Visitor, walk_body, walk_expr};
98
use rustc_lint::{LateContext, LateLintPass};
10-
use rustc_middle::ty;
9+
use rustc_middle::hir::nested_filter;
10+
use rustc_middle::ty::{self, TyCtxt, TypeckResults};
1111
use rustc_session::declare_lint_pass;
1212
use rustc_span::{DesugaringKind, Span};
1313

@@ -55,6 +55,13 @@ declare_clippy_lint! {
5555
/// unsafe { char::from_u32_unchecked(int_value) }
5656
/// }
5757
/// ```
58+
///
59+
/// ### Note
60+
///
61+
/// Taking a raw pointer to a union field is always safe and will
62+
/// not be considered unsafe by this lint, even when linting code written
63+
/// with a specified Rust version of 1.91 or earlier (which required
64+
/// using an `unsafe` block).
5865
#[clippy::version = "1.69.0"]
5966
pub MULTIPLE_UNSAFE_OPS_PER_BLOCK,
6067
restriction,
@@ -70,8 +77,7 @@ impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock {
7077
{
7178
return;
7279
}
73-
let mut unsafe_ops = vec![];
74-
collect_unsafe_exprs(cx, block, &mut unsafe_ops);
80+
let unsafe_ops = UnsafeExprCollector::collect_unsafe_exprs(cx, block);
7581
if unsafe_ops.len() > 1 {
7682
span_lint_and_then(
7783
cx,
@@ -91,25 +97,49 @@ impl<'tcx> LateLintPass<'tcx> for MultipleUnsafeOpsPerBlock {
9197
}
9298
}
9399

94-
fn collect_unsafe_exprs<'tcx>(
95-
cx: &LateContext<'tcx>,
96-
node: impl Visitable<'tcx>,
97-
unsafe_ops: &mut Vec<(&'static str, Span)>,
98-
) {
99-
for_each_expr(cx, node, |expr| {
100+
struct UnsafeExprCollector<'tcx> {
101+
tcx: TyCtxt<'tcx>,
102+
typeck_results: &'tcx TypeckResults<'tcx>,
103+
unsafe_ops: Vec<(&'static str, Span)>,
104+
}
105+
106+
impl<'tcx> UnsafeExprCollector<'tcx> {
107+
fn collect_unsafe_exprs(cx: &LateContext<'tcx>, block: &'tcx hir::Block<'tcx>) -> Vec<(&'static str, Span)> {
108+
let mut collector = Self {
109+
tcx: cx.tcx,
110+
typeck_results: cx.typeck_results(),
111+
unsafe_ops: vec![],
112+
};
113+
collector.visit_block(block);
114+
collector.unsafe_ops
115+
}
116+
}
117+
118+
impl<'tcx> Visitor<'tcx> for UnsafeExprCollector<'tcx> {
119+
type NestedFilter = nested_filter::OnlyBodies;
120+
121+
fn visit_expr(&mut self, expr: &'tcx hir::Expr<'tcx>) {
100122
match expr.kind {
101123
// The `await` itself will desugar to two unsafe calls, but we should ignore those.
102124
// Instead, check the expression that is `await`ed
103125
_ if let Some(e) = desugar_await(expr) => {
104-
collect_unsafe_exprs(cx, e, unsafe_ops);
105-
return Continue(Descend::No);
126+
return self.visit_expr(e);
106127
},
107128

108-
ExprKind::InlineAsm(_) => unsafe_ops.push(("inline assembly used here", expr.span)),
129+
ExprKind::InlineAsm(_) => self.unsafe_ops.push(("inline assembly used here", expr.span)),
130+
131+
ExprKind::AddrOf(BorrowKind::Raw, _, mut inner) => {
132+
while let ExprKind::Field(prefix, _) = inner.kind
133+
&& self.typeck_results.expr_adjustments(prefix).is_empty()
134+
{
135+
inner = prefix;
136+
}
137+
return self.visit_expr(inner);
138+
},
109139

110140
ExprKind::Field(e, _) => {
111-
if cx.typeck_results().expr_ty(e).is_union() {
112-
unsafe_ops.push(("union field access occurs here", expr.span));
141+
if self.typeck_results.expr_ty(e).is_union() {
142+
self.unsafe_ops.push(("union field access occurs here", expr.span));
113143
}
114144
},
115145

@@ -127,32 +157,32 @@ fn collect_unsafe_exprs<'tcx>(
127157
..
128158
},
129159
)) => {
130-
unsafe_ops.push(("access of a mutable static occurs here", expr.span));
160+
self.unsafe_ops
161+
.push(("access of a mutable static occurs here", expr.span));
131162
},
132163

133-
ExprKind::Unary(UnOp::Deref, e) if cx.typeck_results().expr_ty_adjusted(e).is_raw_ptr() => {
134-
unsafe_ops.push(("raw pointer dereference occurs here", expr.span));
164+
ExprKind::Unary(UnOp::Deref, e) if self.typeck_results.expr_ty(e).is_raw_ptr() => {
165+
self.unsafe_ops.push(("raw pointer dereference occurs here", expr.span));
135166
},
136167

137168
ExprKind::Call(path_expr, _) => {
138-
let sig = match *cx.typeck_results().expr_ty(path_expr).kind() {
139-
ty::FnDef(id, _) => cx.tcx.fn_sig(id).skip_binder(),
140-
ty::FnPtr(sig_tys, hdr) => sig_tys.with(hdr),
141-
_ => return Continue(Descend::Yes),
169+
let opt_sig = match *self.typeck_results.expr_ty_adjusted(path_expr).kind() {
170+
ty::FnDef(id, _) => Some(self.tcx.fn_sig(id).skip_binder()),
171+
ty::FnPtr(sig_tys, hdr) => Some(sig_tys.with(hdr)),
172+
_ => None,
142173
};
143-
if sig.safety().is_unsafe() {
144-
unsafe_ops.push(("unsafe function call occurs here", expr.span));
174+
if opt_sig.is_some_and(|sig| sig.safety().is_unsafe()) {
175+
self.unsafe_ops.push(("unsafe function call occurs here", expr.span));
145176
}
146177
},
147178

148179
ExprKind::MethodCall(..) => {
149-
if let Some(sig) = cx
150-
.typeck_results()
180+
let opt_sig = self
181+
.typeck_results
151182
.type_dependent_def_id(expr.hir_id)
152-
.map(|def_id| cx.tcx.fn_sig(def_id))
153-
&& sig.skip_binder().safety().is_unsafe()
154-
{
155-
unsafe_ops.push(("unsafe method call occurs here", expr.span));
183+
.map(|def_id| self.tcx.fn_sig(def_id));
184+
if opt_sig.is_some_and(|sig| sig.skip_binder().safety().is_unsafe()) {
185+
self.unsafe_ops.push(("unsafe method call occurs here", expr.span));
156186
}
157187
},
158188

@@ -173,15 +203,26 @@ fn collect_unsafe_exprs<'tcx>(
173203
}
174204
))
175205
) {
176-
unsafe_ops.push(("modification of a mutable static occurs here", expr.span));
177-
collect_unsafe_exprs(cx, rhs, unsafe_ops);
178-
return Continue(Descend::No);
206+
self.unsafe_ops
207+
.push(("modification of a mutable static occurs here", expr.span));
208+
return self.visit_expr(rhs);
179209
}
180210
},
181211

182212
_ => {},
183213
}
184214

185-
Continue::<(), _>(Descend::Yes)
186-
});
215+
walk_expr(self, expr);
216+
}
217+
218+
fn visit_body(&mut self, body: &hir::Body<'tcx>) {
219+
let saved_typeck_results = self.typeck_results;
220+
self.typeck_results = self.tcx.typeck_body(body.id());
221+
walk_body(self, body);
222+
self.typeck_results = saved_typeck_results;
223+
}
224+
225+
fn maybe_tcx(&mut self) -> Self::MaybeTyCtxt {
226+
self.tcx
227+
}
187228
}

src/tools/clippy/clippy_lints/src/replace_box.rs

Lines changed: 94 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,17 @@ use clippy_utils::res::{MaybeDef, MaybeResPath};
33
use clippy_utils::sugg::Sugg;
44
use clippy_utils::ty::implements_trait;
55
use clippy_utils::{is_default_equivalent_call, local_is_initialized};
6+
use rustc_data_structures::fx::FxHashSet;
7+
use rustc_data_structures::smallvec::SmallVec;
68
use rustc_errors::Applicability;
7-
use rustc_hir::{Expr, ExprKind, LangItem, QPath};
9+
use rustc_hir::{Body, BodyId, Expr, ExprKind, HirId, LangItem, QPath};
10+
use rustc_hir_typeck::expr_use_visitor::{Delegate, ExprUseVisitor, PlaceBase, PlaceWithHirId};
811
use rustc_lint::{LateContext, LateLintPass};
9-
use rustc_session::declare_lint_pass;
10-
use rustc_span::sym;
12+
use rustc_middle::hir::place::ProjectionKind;
13+
use rustc_middle::mir::FakeReadCause;
14+
use rustc_middle::ty;
15+
use rustc_session::impl_lint_pass;
16+
use rustc_span::{Symbol, sym};
1117

1218
declare_clippy_lint! {
1319
/// ### What it does
@@ -33,17 +39,57 @@ declare_clippy_lint! {
3339
perf,
3440
"assigning a newly created box to `Box<T>` is inefficient"
3541
}
36-
declare_lint_pass!(ReplaceBox => [REPLACE_BOX]);
42+
43+
#[derive(Default)]
44+
pub struct ReplaceBox {
45+
consumed_locals: FxHashSet<HirId>,
46+
loaded_bodies: SmallVec<[BodyId; 2]>,
47+
}
48+
49+
impl ReplaceBox {
50+
fn get_consumed_locals(&mut self, cx: &LateContext<'_>) -> &FxHashSet<HirId> {
51+
if let Some(body_id) = cx.enclosing_body
52+
&& !self.loaded_bodies.contains(&body_id)
53+
{
54+
self.loaded_bodies.push(body_id);
55+
ExprUseVisitor::for_clippy(
56+
cx,
57+
cx.tcx.hir_body_owner_def_id(body_id),
58+
MovedVariablesCtxt {
59+
consumed_locals: &mut self.consumed_locals,
60+
},
61+
)
62+
.consume_body(cx.tcx.hir_body(body_id))
63+
.into_ok();
64+
}
65+
66+
&self.consumed_locals
67+
}
68+
}
69+
70+
impl_lint_pass!(ReplaceBox => [REPLACE_BOX]);
3771

3872
impl LateLintPass<'_> for ReplaceBox {
73+
fn check_body_post(&mut self, _: &LateContext<'_>, body: &Body<'_>) {
74+
if self.loaded_bodies.first().is_some_and(|&x| x == body.id()) {
75+
self.consumed_locals.clear();
76+
self.loaded_bodies.clear();
77+
}
78+
}
79+
3980
fn check_expr(&mut self, cx: &LateContext<'_>, expr: &'_ Expr<'_>) {
4081
if let ExprKind::Assign(lhs, rhs, _) = &expr.kind
4182
&& !lhs.span.from_expansion()
4283
&& !rhs.span.from_expansion()
4384
&& let lhs_ty = cx.typeck_results().expr_ty(lhs)
85+
&& let Some(inner_ty) = lhs_ty.boxed_ty()
4486
// No diagnostic for late-initialized locals
4587
&& lhs.res_local_id().is_none_or(|local| local_is_initialized(cx, local))
46-
&& let Some(inner_ty) = lhs_ty.boxed_ty()
88+
// No diagnostic if this is a local that has been moved, or the field
89+
// of a local that has been moved, or several chained field accesses of a local
90+
&& local_base(lhs).is_none_or(|(base_id, _)| {
91+
!self.get_consumed_locals(cx).contains(&base_id)
92+
})
4793
{
4894
if let Some(default_trait_id) = cx.tcx.get_diagnostic_item(sym::Default)
4995
&& implements_trait(cx, inner_ty, default_trait_id, &[])
@@ -109,3 +155,46 @@ fn get_box_new_payload<'tcx>(cx: &LateContext<'_>, expr: &Expr<'tcx>) -> Option<
109155
None
110156
}
111157
}
158+
159+
struct MovedVariablesCtxt<'a> {
160+
consumed_locals: &'a mut FxHashSet<HirId>,
161+
}
162+
163+
impl<'tcx> Delegate<'tcx> for MovedVariablesCtxt<'_> {
164+
fn consume(&mut self, cmt: &PlaceWithHirId<'tcx>, _: HirId) {
165+
if let PlaceBase::Local(id) = cmt.place.base
166+
&& let mut projections = cmt
167+
.place
168+
.projections
169+
.iter()
170+
.filter(|x| matches!(x.kind, ProjectionKind::Deref))
171+
// Either no deref or multiple derefs
172+
&& (projections.next().is_none() || projections.next().is_some())
173+
{
174+
self.consumed_locals.insert(id);
175+
}
176+
}
177+
178+
fn use_cloned(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {}
179+
180+
fn borrow(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId, _: ty::BorrowKind) {}
181+
182+
fn mutate(&mut self, _: &PlaceWithHirId<'tcx>, _: HirId) {}
183+
184+
fn fake_read(&mut self, _: &PlaceWithHirId<'tcx>, _: FakeReadCause, _: HirId) {}
185+
}
186+
187+
/// A local place followed by optional fields
188+
type IdFields = (HirId, Vec<Symbol>);
189+
190+
/// If `expr` is a local variable with optional field accesses, return it.
191+
fn local_base(expr: &Expr<'_>) -> Option<IdFields> {
192+
match expr.kind {
193+
ExprKind::Path(qpath) => qpath.res_local_id().map(|id| (id, Vec::new())),
194+
ExprKind::Field(expr, field) => local_base(expr).map(|(id, mut fields)| {
195+
fields.push(field.name);
196+
(id, fields)
197+
}),
198+
_ => None,
199+
}
200+
}

src/tools/clippy/tests/ui/auxiliary/macro_rules.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,3 +57,15 @@ macro_rules! bad_transmute {
5757
std::mem::transmute($e)
5858
};
5959
}
60+
61+
#[macro_export]
62+
#[rustfmt::skip]
63+
macro_rules! double_parens {
64+
($a:expr, $b:expr, $c:expr, $d:expr) => {{
65+
let a = ($a);
66+
let a = (());
67+
let b = ((5));
68+
let c = std::convert::identity((5));
69+
InterruptMask((($a.union($b).union($c).union($d)).into_bits()) as u32)
70+
}};
71+
}

0 commit comments

Comments
 (0)