Auto merge of #10954 - y21:issue10158, r=llogiq

[`derivable_impls`]: don't lint if `default()` call expr unsize-coerces to trait object

Fixes #10158.

This fixes a FP where the derive-generated Default impl would have different behavior because of unsize coercion from `Box<T>` to `Box<dyn Trait>`:
```rs
struct S {
  x: Box<dyn std::fmt::Debug>
}
impl Default for S {
  fn default() -> Self {
    Self {
      x: Box::<()>::default()
     // ^~ Box<()> coerces to Box<dyn Debug>
     // #[derive(Default)] would call Box::<dyn Debug>::default()
    }
  }
}
```
(this intentionally only looks for trait objects `dyn` specifically, and not any unsize coercion, e.g. `&[i32; 5]` to `&[i32]`, because that breaks existing tests and isn't actually problematic, as far as I can tell)

changelog: [`derivable_impls`]: don't lint if `default()` call expression unsize-coerces to trait object
This commit is contained in:
bors 2023-06-14 16:59:26 +00:00
commit ffe95252bd
3 changed files with 74 additions and 7 deletions

View File

@ -4,11 +4,13 @@ use clippy_utils::source::indent_of;
use clippy_utils::{is_default_equivalent, peel_blocks};
use rustc_errors::Applicability;
use rustc_hir::{
self as hir,
def::{CtorKind, CtorOf, DefKind, Res},
Body, Expr, ExprKind, GenericArg, Impl, ImplItemKind, Item, ItemKind, Node, PathSegment, QPath, Ty, TyKind,
Body, Expr, ExprKind, GenericArg, Impl, ImplItemKind, Item, ItemKind, Node, PathSegment, QPath, TyKind,
};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty::{Adt, AdtDef, SubstsRef};
use rustc_middle::ty::adjustment::{Adjust, PointerCast};
use rustc_middle::ty::{self, Adt, AdtDef, SubstsRef, Ty, TypeckResults};
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::sym;
@ -75,13 +77,23 @@ fn is_path_self(e: &Expr<'_>) -> bool {
}
}
fn contains_trait_object(ty: Ty<'_>) -> bool {
match ty.kind() {
ty::Ref(_, ty, _) => contains_trait_object(*ty),
ty::Adt(def, substs) => def.is_box() && substs[0].as_type().map_or(false, contains_trait_object),
ty::Dynamic(..) => true,
_ => false,
}
}
fn check_struct<'tcx>(
cx: &LateContext<'tcx>,
item: &'tcx Item<'_>,
self_ty: &Ty<'_>,
self_ty: &hir::Ty<'_>,
func_expr: &Expr<'_>,
adt_def: AdtDef<'_>,
substs: SubstsRef<'_>,
typeck_results: &'tcx TypeckResults<'tcx>,
) {
if let TyKind::Path(QPath::Resolved(_, p)) = self_ty.kind {
if let Some(PathSegment { args, .. }) = p.segments.last() {
@ -96,10 +108,23 @@ fn check_struct<'tcx>(
}
}
}
// the default() call might unsize coerce to a trait object (e.g. Box<T> to Box<dyn Trait>),
// which would not be the same if derived (see #10158).
// this closure checks both if the expr is equivalent to a `default()` call and does not
// have such coercions.
let is_default_without_adjusts = |expr| {
is_default_equivalent(cx, expr)
&& typeck_results.expr_adjustments(expr).iter().all(|adj| {
!matches!(adj.kind, Adjust::Pointer(PointerCast::Unsize)
if contains_trait_object(adj.target))
})
};
let should_emit = match peel_blocks(func_expr).kind {
ExprKind::Tup(fields) => fields.iter().all(|e| is_default_equivalent(cx, e)),
ExprKind::Call(callee, args) if is_path_self(callee) => args.iter().all(|e| is_default_equivalent(cx, e)),
ExprKind::Struct(_, fields, _) => fields.iter().all(|ef| is_default_equivalent(cx, ef.expr)),
ExprKind::Tup(fields) => fields.iter().all(is_default_without_adjusts),
ExprKind::Call(callee, args) if is_path_self(callee) => args.iter().all(is_default_without_adjusts),
ExprKind::Struct(_, fields, _) => fields.iter().all(|ef| is_default_without_adjusts(ef.expr)),
_ => false,
};
@ -197,7 +222,7 @@ impl<'tcx> LateLintPass<'tcx> for DerivableImpls {
then {
if adt_def.is_struct() {
check_struct(cx, item, self_ty, func_expr, adt_def, substs);
check_struct(cx, item, self_ty, func_expr, adt_def, substs, cx.tcx.typeck_body(*b));
} else if adt_def.is_enum() && self.msrv.meets(msrvs::DEFAULT_ENUM_ATTRIBUTE) {
check_enum(cx, item, func_expr, adt_def);
}

View File

@ -268,4 +268,25 @@ impl Default for OtherGenericType {
}
}
mod issue10158 {
pub trait T {}
#[derive(Default)]
pub struct S {}
impl T for S {}
pub struct Outer {
pub inner: Box<dyn T>,
}
impl Default for Outer {
fn default() -> Self {
Outer {
// Box::<S>::default() adjusts to Box<dyn T>
inner: Box::<S>::default(),
}
}
}
}
fn main() {}

View File

@ -304,4 +304,25 @@ impl Default for OtherGenericType {
}
}
mod issue10158 {
pub trait T {}
#[derive(Default)]
pub struct S {}
impl T for S {}
pub struct Outer {
pub inner: Box<dyn T>,
}
impl Default for Outer {
fn default() -> Self {
Outer {
// Box::<S>::default() adjusts to Box<dyn T>
inner: Box::<S>::default(),
}
}
}
}
fn main() {}