Skip to content

Commit 2ba1926

Browse files
committed
[derivable_impls]: don't lint if expr unsize-coerces
1 parent a3b185b commit 2ba1926

File tree

3 files changed

+75
-5
lines changed

3 files changed

+75
-5
lines changed

clippy_lints/src/derivable_impls.rs

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,8 @@ use rustc_hir::{
88
Body, Expr, ExprKind, GenericArg, Impl, ImplItemKind, Item, ItemKind, Node, PathSegment, QPath, Ty, TyKind,
99
};
1010
use rustc_lint::{LateContext, LateLintPass};
11-
use rustc_middle::ty::{Adt, AdtDef, SubstsRef};
11+
use rustc_middle::ty::adjustment::{Adjust, PointerCast};
12+
use rustc_middle::ty::{self, Adt, AdtDef, SubstsRef, TypeckResults};
1213
use rustc_session::{declare_tool_lint, impl_lint_pass};
1314
use rustc_span::sym;
1415

@@ -75,13 +76,27 @@ fn is_path_self(e: &Expr<'_>) -> bool {
7576
}
7677
}
7778

79+
fn contains_trait_object(cx: &LateContext<'_>, ty: ty::Ty<'_>) -> bool {
80+
match ty.kind() {
81+
ty::TyKind::Ref(_, ty, _) => {
82+
contains_trait_object(cx, *ty)
83+
},
84+
ty::TyKind::Adt(def, substs) => {
85+
def.is_box() && substs[0].as_type().map_or(false, |ty| contains_trait_object(cx, ty))
86+
}
87+
ty::TyKind::Dynamic(..) => true,
88+
_ => false,
89+
}
90+
}
91+
7892
fn check_struct<'tcx>(
7993
cx: &LateContext<'tcx>,
8094
item: &'tcx Item<'_>,
8195
self_ty: &Ty<'_>,
8296
func_expr: &Expr<'_>,
8397
adt_def: AdtDef<'_>,
8498
substs: SubstsRef<'_>,
99+
typeck_results: &'tcx TypeckResults<'tcx>,
85100
) {
86101
if let TyKind::Path(QPath::Resolved(_, p)) = self_ty.kind {
87102
if let Some(PathSegment { args, .. }) = p.segments.last() {
@@ -96,10 +111,23 @@ fn check_struct<'tcx>(
96111
}
97112
}
98113
}
114+
115+
// the default() call might unsize coerce to a trait object (e.g. Box<T> to Box<dyn Trait>),
116+
// which would not be the same if derived (see #10158).
117+
// this closure checks both if the expr is equivalent to a `default()` call and does not
118+
// have such coercions.
119+
let is_default_without_adjusts = |expr| {
120+
is_default_equivalent(cx, expr)
121+
&& typeck_results.expr_adjustments(expr).iter().all(|adj| {
122+
!matches!(adj.kind, Adjust::Pointer(PointerCast::Unsize)
123+
if contains_trait_object(cx, adj.target))
124+
})
125+
};
126+
99127
let should_emit = match peel_blocks(func_expr).kind {
100-
ExprKind::Tup(fields) => fields.iter().all(|e| is_default_equivalent(cx, e)),
101-
ExprKind::Call(callee, args) if is_path_self(callee) => args.iter().all(|e| is_default_equivalent(cx, e)),
102-
ExprKind::Struct(_, fields, _) => fields.iter().all(|ef| is_default_equivalent(cx, ef.expr)),
128+
ExprKind::Tup(fields) => fields.iter().all(|e| is_default_without_adjusts(e)),
129+
ExprKind::Call(callee, args) if is_path_self(callee) => args.iter().all(|e| is_default_without_adjusts(e)),
130+
ExprKind::Struct(_, fields, _) => fields.iter().all(|ef| is_default_without_adjusts(ef.expr)),
103131
_ => false,
104132
};
105133

@@ -197,7 +225,7 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
197225

198226
then {
199227
if adt_def.is_struct() {
200-
check_struct(cx, item, self_ty, func_expr, adt_def, substs);
228+
check_struct(cx, item, self_ty, func_expr, adt_def, substs, cx.tcx.typeck_body(*b));
201229
} else if adt_def.is_enum() && self.msrv.meets(msrvs::DEFAULT_ENUM_ATTRIBUTE) {
202230
check_enum(cx, item, func_expr, adt_def);
203231
}

tests/ui/derivable_impls.fixed

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,4 +268,25 @@ impl Default for OtherGenericType {
268268
}
269269
}
270270

271+
mod issue10158 {
272+
pub trait T {}
273+
274+
#[derive(Default)]
275+
pub struct S {}
276+
impl T for S {}
277+
278+
pub struct Outer {
279+
pub inner: Box<dyn T>,
280+
}
281+
282+
impl Default for Outer {
283+
fn default() -> Self {
284+
Outer {
285+
// Box::<S>::default() adjusts to Box<dyn T>
286+
inner: Box::<S>::default(),
287+
}
288+
}
289+
}
290+
}
291+
271292
fn main() {}

tests/ui/derivable_impls.rs

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -304,4 +304,25 @@ impl Default for OtherGenericType {
304304
}
305305
}
306306

307+
mod issue10158 {
308+
pub trait T {}
309+
310+
#[derive(Default)]
311+
pub struct S {}
312+
impl T for S {}
313+
314+
pub struct Outer {
315+
pub inner: Box<dyn T>,
316+
}
317+
318+
impl Default for Outer {
319+
fn default() -> Self {
320+
Outer {
321+
// Box::<S>::default() adjusts to Box<dyn T>
322+
inner: Box::<S>::default(),
323+
}
324+
}
325+
}
326+
}
327+
307328
fn main() {}

0 commit comments

Comments
 (0)