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

3 files changed, 74 insertions, 7 deletions

diff --git a/clippy_lints/src/derivable_impls.rs b/clippy_lints/src/derivable_impls.rs
index 8f68f90a2a1..ec0ca50cfec 100644
--- a/clippy_lints/src/derivable_impls.rs
+++ b/clippy_lints/src/derivable_impls.rs
@@ -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);
                 }
diff --git a/tests/ui/derivable_impls.fixed b/tests/ui/derivable_impls.fixed
index aa0efb85c29..a10f3d01070 100644
--- a/tests/ui/derivable_impls.fixed
+++ b/tests/ui/derivable_impls.fixed
@@ -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() {}
diff --git a/tests/ui/derivable_impls.rs b/tests/ui/derivable_impls.rs
index 8dc999ad586..18cef1c5be8 100644
--- a/tests/ui/derivable_impls.rs
+++ b/tests/ui/derivable_impls.rs
@@ -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() {}