From 756caf79e64598886551325dbf9ab7eccee03328 Mon Sep 17 00:00:00 2001
From: Andre Bogus <bogusandre@gmail.com>
Date: Tue, 31 May 2022 09:23:17 +0200
Subject: [PATCH] account for generics

---
 clippy_lints/src/large_enum_variant.rs | 24 ++++++++++++++++++++++--
 tests/ui/large_enum_variant.rs         | 14 ++++++++++++++
 tests/ui/large_enum_variant.stderr     | 24 +++++++++++++++++++++++-
 3 files changed, 59 insertions(+), 3 deletions(-)

diff --git a/clippy_lints/src/large_enum_variant.rs b/clippy_lints/src/large_enum_variant.rs
index 34b7cf67db1..63ac092dfaf 100644
--- a/clippy_lints/src/large_enum_variant.rs
+++ b/clippy_lints/src/large_enum_variant.rs
@@ -7,6 +7,7 @@ use rustc_hir::{Item, ItemKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::lint::in_external_macro;
 use rustc_middle::ty::layout::LayoutOf;
+use rustc_middle::ty::{Adt, Ty};
 use rustc_session::{declare_tool_lint, impl_lint_pass};
 use rustc_span::source_map::Span;
 
@@ -26,6 +27,15 @@ declare_clippy_lint! {
     /// the overhead is negligible and the boxing is counter-productive. Always
     /// measure the change this lint suggests.
     ///
+    /// For types that implement `Copy`, the suggestion to `Box` a variant's
+    /// data would require removing the trait impl. The types can of course
+    /// still be `Clone`, but that is worse ergonomically. Depending on the
+    /// use case it may be possible to store the large data in an auxillary
+    /// structure (e.g. Arena or ECS).
+    ///
+    /// The lint will ignore generic types if the layout depends on the
+    /// generics, even if the size difference will be large anyway.
+    ///
     /// ### Example
     /// ```rust
     /// // Bad
@@ -74,7 +84,7 @@ struct VariantInfo {
 impl_lint_pass!(LargeEnumVariant => [LARGE_ENUM_VARIANT]);
 
 impl<'tcx> LateLintPass<'tcx> for LargeEnumVariant {
-    fn check_item(&mut self, cx: &LateContext<'_>, item: &Item<'_>) {
+    fn check_item(&mut self, cx: &LateContext<'tcx>, item: &Item<'tcx>) {
         if in_external_macro(cx.tcx.sess, item.span) {
             return;
         }
@@ -132,7 +142,7 @@ impl<'tcx> LateLintPass<'tcx> for LargeEnumVariant {
                         let fields = def.variants[variants_size[0].ind].data.fields();
                         variants_size[0].fields_size.sort_by(|a, b| (a.size.cmp(&b.size)));
                         let mut applicability = Applicability::MaybeIncorrect;
-                        if is_copy(cx, ty) {
+                        if is_copy(cx, ty) || maybe_copy(cx, ty) {
                             diag.span_note(
                                 item.ident.span,
                                 "boxing a variant would require the type no longer be `Copy`",
@@ -176,3 +186,13 @@ impl<'tcx> LateLintPass<'tcx> for LargeEnumVariant {
         }
     }
 }
+
+fn maybe_copy<'tcx>(cx: &LateContext<'tcx>, ty: Ty<'tcx>) -> bool {
+    if let Adt(_def, substs) = ty.kind()
+        && substs.types().next().is_some()
+        && let Some(copy_trait) = cx.tcx.lang_items().copy_trait()
+    {
+        return cx.tcx.non_blanket_impls_for_ty(copy_trait, ty).next().is_some();
+    }
+    false
+}
diff --git a/tests/ui/large_enum_variant.rs b/tests/ui/large_enum_variant.rs
index 42563383fed..23152a13322 100644
--- a/tests/ui/large_enum_variant.rs
+++ b/tests/ui/large_enum_variant.rs
@@ -114,8 +114,22 @@ impl Clone for ManuallyCopyLargeEnum {
         *self
     }
 }
+
 impl Copy for ManuallyCopyLargeEnum {}
 
+enum SomeGenericPossiblyCopyEnum<T> {
+    A(bool, std::marker::PhantomData<T>),
+    B([u64; 4000]),
+}
+
+impl<T: Copy> Clone for SomeGenericPossiblyCopyEnum<T> {
+    fn clone(&self) -> Self {
+        *self
+    }
+}
+
+impl<T: Copy> Copy for SomeGenericPossiblyCopyEnum<T> {}
+
 fn main() {
     large_enum_variant!();
 }
diff --git a/tests/ui/large_enum_variant.stderr b/tests/ui/large_enum_variant.stderr
index 397d263ae9f..0248327262d 100644
--- a/tests/ui/large_enum_variant.stderr
+++ b/tests/ui/large_enum_variant.stderr
@@ -171,5 +171,27 @@ help: consider boxing the large fields to reduce the total size of the enum
 LL |     B([u128; 4000]),
    |     ^^^^^^^^^^^^^^^
 
-error: aborting due to 10 previous errors
+error: large size difference between variants
+  --> $DIR/large_enum_variant.rs:122:5
+   |
+LL |     B([u64; 4000]),
+   |     ^^^^^^^^^^^^^^ this variant is 32000 bytes
+   |
+note: and the second-largest variant is 1 bytes:
+  --> $DIR/large_enum_variant.rs:121:5
+   |
+LL |     A(bool, std::marker::PhantomData<T>),
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+note: boxing a variant would require the type no longer be `Copy`
+  --> $DIR/large_enum_variant.rs:120:6
+   |
+LL | enum SomeGenericPossiblyCopyEnum<T> {
+   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^
+help: consider boxing the large fields to reduce the total size of the enum
+  --> $DIR/large_enum_variant.rs:122:5
+   |
+LL |     B([u64; 4000]),
+   |     ^^^^^^^^^^^^^^
+
+error: aborting due to 11 previous errors