Skip to content

Commit 5d57ba8

Browse files
committed
Auto merge of #12097 - y21:issue9427-3, r=llogiq
don't change eagerness for struct literal syntax with significant drop Fixes the bug reported by `@ju1ius` in #9427 (comment). `eager_or_lazy` already understands to suppress eagerness changes when the expression type has a significant drop impl, but only for initialization of tuple structs or unit structs. This changes it to also avoid changing it for `Self { .. }` and `TypeWithDrop { .. }` changelog: [`unnecessary_lazy_eval`]: don't suggest changing eagerness for struct literal syntax when type has a significant drop impl
2 parents 7bb0e9c + 890c070 commit 5d57ba8

File tree

4 files changed

+100
-65
lines changed

4 files changed

+100
-65
lines changed

clippy_utils/src/eager_or_lazy.rs

+11-2
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,10 @@ fn fn_eagerness(cx: &LateContext<'_>, fn_id: DefId, name: Symbol, have_one_arg:
9999
}
100100

101101
fn res_has_significant_drop(res: Res, cx: &LateContext<'_>, e: &Expr<'_>) -> bool {
102-
if let Res::Def(DefKind::Ctor(..) | DefKind::Variant, _) | Res::SelfCtor(_) = res {
102+
if let Res::Def(DefKind::Ctor(..) | DefKind::Variant | DefKind::Enum | DefKind::Struct, _)
103+
| Res::SelfCtor(_)
104+
| Res::SelfTyAlias { .. } = res
105+
{
103106
cx.typeck_results()
104107
.expr_ty(e)
105108
.has_significant_drop(cx.tcx, cx.param_env)
@@ -173,6 +176,13 @@ fn expr_eagerness<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> EagernessS
173176
self.eagerness |= NoChange;
174177
return;
175178
},
179+
#[expect(clippy::match_same_arms)] // arm pattern can't be merged due to `ref`, see rust#105778
180+
ExprKind::Struct(path, ..) => {
181+
if res_has_significant_drop(self.cx.qpath_res(path, e.hir_id), self.cx, e) {
182+
self.eagerness = ForceNoChange;
183+
return;
184+
}
185+
},
176186
ExprKind::Path(ref path) => {
177187
if res_has_significant_drop(self.cx.qpath_res(path, e.hir_id), self.cx, e) {
178188
self.eagerness = ForceNoChange;
@@ -291,7 +301,6 @@ fn expr_eagerness<'tcx>(cx: &LateContext<'tcx>, e: &'tcx Expr<'_>) -> EagernessS
291301
| ExprKind::Closure { .. }
292302
| ExprKind::Field(..)
293303
| ExprKind::AddrOf(..)
294-
| ExprKind::Struct(..)
295304
| ExprKind::Repeat(..)
296305
| ExprKind::Block(Block { stmts: [], .. }, _)
297306
| ExprKind::OffsetOf(..) => (),

tests/ui/unnecessary_lazy_eval.fixed

+13
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,18 @@ impl Drop for Issue9427FollowUp {
4747
}
4848
}
4949

50+
struct Issue9427Followup2 {
51+
ptr: *const (),
52+
}
53+
impl Issue9427Followup2 {
54+
fn from_owned(ptr: *const ()) -> Option<Self> {
55+
(!ptr.is_null()).then(|| Self { ptr })
56+
}
57+
}
58+
impl Drop for Issue9427Followup2 {
59+
fn drop(&mut self) {}
60+
}
61+
5062
struct Issue10437;
5163
impl Deref for Issue10437 {
5264
type Target = u32;
@@ -128,6 +140,7 @@ fn main() {
128140
// Should not lint - bool
129141
let _ = (0 == 1).then(|| Issue9427(0)); // Issue9427 has a significant drop
130142
let _ = false.then(|| Issue9427FollowUp); // Issue9427FollowUp has a significant drop
143+
let _ = false.then(|| Issue9427Followup2 { ptr: std::ptr::null() });
131144

132145
// should not lint, bind_instead_of_map takes priority
133146
let _ = Some(10).and_then(|idx| Some(ext_arr[idx]));

tests/ui/unnecessary_lazy_eval.rs

+13
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,18 @@ impl Drop for Issue9427FollowUp {
4747
}
4848
}
4949

50+
struct Issue9427Followup2 {
51+
ptr: *const (),
52+
}
53+
impl Issue9427Followup2 {
54+
fn from_owned(ptr: *const ()) -> Option<Self> {
55+
(!ptr.is_null()).then(|| Self { ptr })
56+
}
57+
}
58+
impl Drop for Issue9427Followup2 {
59+
fn drop(&mut self) {}
60+
}
61+
5062
struct Issue10437;
5163
impl Deref for Issue10437 {
5264
type Target = u32;
@@ -128,6 +140,7 @@ fn main() {
128140
// Should not lint - bool
129141
let _ = (0 == 1).then(|| Issue9427(0)); // Issue9427 has a significant drop
130142
let _ = false.then(|| Issue9427FollowUp); // Issue9427FollowUp has a significant drop
143+
let _ = false.then(|| Issue9427Followup2 { ptr: std::ptr::null() });
131144

132145
// should not lint, bind_instead_of_map takes priority
133146
let _ = Some(10).and_then(|idx| Some(ext_arr[idx]));

0 commit comments

Comments
 (0)