mirror of
https://github.com/rust-lang/rust.git
synced 2024-12-14 09:36:06 +00:00
Auto merge of #7677 - surechen:edit_large_enum_variant, r=camsteffen
fix bug for large_enum_variants Fix the discussion problem in the issue of https://github.com/rust-lang/rust-clippy/issues/7666#issuecomment-919654291 About the false positive problem of case: ```rust enum LargeEnum6 { A, B([u8;255]), C([u8;200]), } ``` changelog: Fix largest_enum_variant wrongly identifying the second largest variant.
This commit is contained in:
commit
a893eb993b
@ -1,13 +1,14 @@
|
||||
//! lint when there is a large size difference between variants on an enum
|
||||
|
||||
use clippy_utils::diagnostics::span_lint_and_then;
|
||||
use clippy_utils::source::snippet_opt;
|
||||
use clippy_utils::source::snippet_with_applicability;
|
||||
use rustc_errors::Applicability;
|
||||
use rustc_hir::{Item, ItemKind, VariantData};
|
||||
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_session::{declare_tool_lint, impl_lint_pass};
|
||||
use rustc_span::source_map::Span;
|
||||
|
||||
declare_clippy_lint! {
|
||||
/// ### What it does
|
||||
@ -58,6 +59,17 @@ impl LargeEnumVariant {
|
||||
}
|
||||
}
|
||||
|
||||
struct FieldInfo {
|
||||
ind: usize,
|
||||
size: u64,
|
||||
}
|
||||
|
||||
struct VariantInfo {
|
||||
ind: usize,
|
||||
size: u64,
|
||||
fields_size: Vec<FieldInfo>,
|
||||
}
|
||||
|
||||
impl_lint_pass!(LargeEnumVariant => [LARGE_ENUM_VARIANT]);
|
||||
|
||||
impl<'tcx> LateLintPass<'tcx> for LargeEnumVariant {
|
||||
@ -68,72 +80,95 @@ impl<'tcx> LateLintPass<'tcx> for LargeEnumVariant {
|
||||
if let ItemKind::Enum(ref def, _) = item.kind {
|
||||
let ty = cx.tcx.type_of(item.def_id);
|
||||
let adt = ty.ty_adt_def().expect("already checked whether this is an enum");
|
||||
|
||||
let mut largest_variant: Option<(_, _)> = None;
|
||||
let mut second_variant: Option<(_, _)> = None;
|
||||
|
||||
for (i, variant) in adt.variants.iter().enumerate() {
|
||||
let size: u64 = variant
|
||||
.fields
|
||||
.iter()
|
||||
.filter_map(|f| {
|
||||
let ty = cx.tcx.type_of(f.did);
|
||||
// don't count generics by filtering out everything
|
||||
// that does not have a layout
|
||||
cx.layout_of(ty).ok().map(|l| l.size.bytes())
|
||||
})
|
||||
.sum();
|
||||
|
||||
let grouped = (size, (i, variant));
|
||||
|
||||
if grouped.0 >= largest_variant.map_or(0, |x| x.0) {
|
||||
second_variant = largest_variant;
|
||||
largest_variant = Some(grouped);
|
||||
}
|
||||
if adt.variants.len() <= 1 {
|
||||
return;
|
||||
}
|
||||
let mut variants_size: Vec<VariantInfo> = adt
|
||||
.variants
|
||||
.iter()
|
||||
.enumerate()
|
||||
.map(|(i, variant)| {
|
||||
let mut fields_size = Vec::new();
|
||||
let size: u64 = variant
|
||||
.fields
|
||||
.iter()
|
||||
.enumerate()
|
||||
.filter_map(|(i, f)| {
|
||||
let ty = cx.tcx.type_of(f.did);
|
||||
// don't count generics by filtering out everything
|
||||
// that does not have a layout
|
||||
cx.layout_of(ty).ok().map(|l| {
|
||||
let size = l.size.bytes();
|
||||
fields_size.push(FieldInfo { ind: i, size });
|
||||
size
|
||||
})
|
||||
})
|
||||
.sum();
|
||||
VariantInfo {
|
||||
ind: i,
|
||||
size,
|
||||
fields_size,
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
|
||||
if let (Some(largest), Some(second)) = (largest_variant, second_variant) {
|
||||
let difference = largest.0 - second.0;
|
||||
variants_size.sort_by(|a, b| (b.size.cmp(&a.size)));
|
||||
|
||||
if difference > self.maximum_size_difference_allowed {
|
||||
let (i, variant) = largest.1;
|
||||
let mut difference = variants_size[0].size - variants_size[1].size;
|
||||
if difference > self.maximum_size_difference_allowed {
|
||||
let help_text = "consider boxing the large fields to reduce the total size of the enum";
|
||||
span_lint_and_then(
|
||||
cx,
|
||||
LARGE_ENUM_VARIANT,
|
||||
def.variants[variants_size[0].ind].span,
|
||||
"large size difference between variants",
|
||||
|diag| {
|
||||
diag.span_label(
|
||||
def.variants[variants_size[0].ind].span,
|
||||
&format!("this variant is {} bytes", variants_size[0].size),
|
||||
);
|
||||
diag.span_note(
|
||||
def.variants[variants_size[1].ind].span,
|
||||
&format!("and the second-largest variant is {} bytes:", variants_size[1].size),
|
||||
);
|
||||
|
||||
let help_text = "consider boxing the large fields to reduce the total size of the enum";
|
||||
span_lint_and_then(
|
||||
cx,
|
||||
LARGE_ENUM_VARIANT,
|
||||
def.variants[i].span,
|
||||
"large size difference between variants",
|
||||
|diag| {
|
||||
diag.span_label(
|
||||
def.variants[(largest.1).0].span,
|
||||
&format!("this variant is {} bytes", largest.0),
|
||||
);
|
||||
diag.span_note(
|
||||
def.variants[(second.1).0].span,
|
||||
&format!("and the second-largest variant is {} bytes:", second.0),
|
||||
);
|
||||
if variant.fields.len() == 1 {
|
||||
let span = match def.variants[i].data {
|
||||
VariantData::Struct(fields, ..) | VariantData::Tuple(fields, ..) => {
|
||||
fields[0].ty.span
|
||||
},
|
||||
VariantData::Unit(..) => unreachable!(),
|
||||
};
|
||||
if let Some(snip) = snippet_opt(cx, span) {
|
||||
diag.span_suggestion(
|
||||
span,
|
||||
help_text,
|
||||
format!("Box<{}>", snip),
|
||||
Applicability::MaybeIncorrect,
|
||||
);
|
||||
return;
|
||||
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;
|
||||
let sugg: Vec<(Span, String)> = variants_size[0]
|
||||
.fields_size
|
||||
.iter()
|
||||
.rev()
|
||||
.map_while(|val| {
|
||||
if difference > self.maximum_size_difference_allowed {
|
||||
difference = difference.saturating_sub(val.size);
|
||||
Some((
|
||||
fields[val.ind].ty.span,
|
||||
format!(
|
||||
"Box<{}>",
|
||||
snippet_with_applicability(
|
||||
cx,
|
||||
fields[val.ind].ty.span,
|
||||
"..",
|
||||
&mut applicability
|
||||
)
|
||||
.into_owned()
|
||||
),
|
||||
))
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
diag.span_help(def.variants[i].span, help_text);
|
||||
},
|
||||
);
|
||||
}
|
||||
})
|
||||
.collect();
|
||||
|
||||
if !sugg.is_empty() {
|
||||
diag.multipart_suggestion(help_text, sugg, Applicability::MaybeIncorrect);
|
||||
return;
|
||||
}
|
||||
|
||||
diag.span_help(def.variants[variants_size[0].ind].span, help_text);
|
||||
},
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
@ -35,6 +35,7 @@ enum LargeEnum2 {
|
||||
VariantOk(i32, u32),
|
||||
ContainingLargeEnum(LargeEnum),
|
||||
}
|
||||
|
||||
enum LargeEnum3 {
|
||||
ContainingMoreThanOneField(i32, [i32; 8000], [i32; 9500]),
|
||||
VoidVariant,
|
||||
@ -56,6 +57,23 @@ enum LargeEnumOk {
|
||||
LargeB([i32; 8001]),
|
||||
}
|
||||
|
||||
enum LargeEnum6 {
|
||||
A,
|
||||
B([u8; 255]),
|
||||
C([u8; 200]),
|
||||
}
|
||||
|
||||
enum LargeEnum7 {
|
||||
A,
|
||||
B([u8; 1255]),
|
||||
C([u8; 200]),
|
||||
}
|
||||
|
||||
enum LargeEnum8 {
|
||||
VariantOk(i32, u32),
|
||||
ContainingMoreThanOneField([i32; 8000], [i32; 2], [i32; 9500], [i32; 30]),
|
||||
}
|
||||
|
||||
fn main() {
|
||||
large_enum_variant!();
|
||||
}
|
||||
|
@ -32,30 +32,45 @@ LL | ContainingLargeEnum(Box<LargeEnum>),
|
||||
| ~~~~~~~~~~~~~~
|
||||
|
||||
error: large size difference between variants
|
||||
--> $DIR/large_enum_variant.rs:46:5
|
||||
--> $DIR/large_enum_variant.rs:40:5
|
||||
|
|
||||
LL | ContainingMoreThanOneField(i32, [i32; 8000], [i32; 9500]),
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this variant is 70004 bytes
|
||||
|
|
||||
note: and the second-largest variant is 8 bytes:
|
||||
--> $DIR/large_enum_variant.rs:42:5
|
||||
|
|
||||
LL | StructLikeLittle { x: i32, y: i32 },
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
help: consider boxing the large fields to reduce the total size of the enum
|
||||
|
|
||||
LL | ContainingMoreThanOneField(i32, Box<[i32; 8000]>, Box<[i32; 9500]>),
|
||||
| ~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~
|
||||
|
||||
error: large size difference between variants
|
||||
--> $DIR/large_enum_variant.rs:47:5
|
||||
|
|
||||
LL | StructLikeLarge { x: [i32; 8000], y: i32 },
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this variant is 32004 bytes
|
||||
|
|
||||
note: and the second-largest variant is 8 bytes:
|
||||
--> $DIR/large_enum_variant.rs:45:5
|
||||
--> $DIR/large_enum_variant.rs:46:5
|
||||
|
|
||||
LL | VariantOk(i32, u32),
|
||||
| ^^^^^^^^^^^^^^^^^^^
|
||||
help: consider boxing the large fields to reduce the total size of the enum
|
||||
--> $DIR/large_enum_variant.rs:46:5
|
||||
|
|
||||
LL | StructLikeLarge { x: [i32; 8000], y: i32 },
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
||||
LL | StructLikeLarge { x: Box<[i32; 8000]>, y: i32 },
|
||||
| ~~~~~~~~~~~~~~~~
|
||||
|
||||
error: large size difference between variants
|
||||
--> $DIR/large_enum_variant.rs:51:5
|
||||
--> $DIR/large_enum_variant.rs:52:5
|
||||
|
|
||||
LL | StructLikeLarge2 { x: [i32; 8000] },
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this variant is 32000 bytes
|
||||
|
|
||||
note: and the second-largest variant is 8 bytes:
|
||||
--> $DIR/large_enum_variant.rs:50:5
|
||||
--> $DIR/large_enum_variant.rs:51:5
|
||||
|
|
||||
LL | VariantOk(i32, u32),
|
||||
| ^^^^^^^^^^^^^^^^^^^
|
||||
@ -64,5 +79,37 @@ help: consider boxing the large fields to reduce the total size of the enum
|
||||
LL | StructLikeLarge2 { x: Box<[i32; 8000]> },
|
||||
| ~~~~~~~~~~~~~~~~
|
||||
|
||||
error: aborting due to 4 previous errors
|
||||
error: large size difference between variants
|
||||
--> $DIR/large_enum_variant.rs:68:5
|
||||
|
|
||||
LL | B([u8; 1255]),
|
||||
| ^^^^^^^^^^^^^ this variant is 1255 bytes
|
||||
|
|
||||
note: and the second-largest variant is 200 bytes:
|
||||
--> $DIR/large_enum_variant.rs:69:5
|
||||
|
|
||||
LL | C([u8; 200]),
|
||||
| ^^^^^^^^^^^^
|
||||
help: consider boxing the large fields to reduce the total size of the enum
|
||||
|
|
||||
LL | B(Box<[u8; 1255]>),
|
||||
| ~~~~~~~~~~~~~~~
|
||||
|
||||
error: large size difference between variants
|
||||
--> $DIR/large_enum_variant.rs:74:5
|
||||
|
|
||||
LL | ContainingMoreThanOneField([i32; 8000], [i32; 2], [i32; 9500], [i32; 30]),
|
||||
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this variant is 70128 bytes
|
||||
|
|
||||
note: and the second-largest variant is 8 bytes:
|
||||
--> $DIR/large_enum_variant.rs:73:5
|
||||
|
|
||||
LL | VariantOk(i32, u32),
|
||||
| ^^^^^^^^^^^^^^^^^^^
|
||||
help: consider boxing the large fields to reduce the total size of the enum
|
||||
|
|
||||
LL | ContainingMoreThanOneField(Box<[i32; 8000]>, [i32; 2], Box<[i32; 9500]>, [i32; 30]),
|
||||
| ~~~~~~~~~~~~~~~~ ~~~~~~~~~~~~~~~~
|
||||
|
||||
error: aborting due to 7 previous errors
|
||||
|
||||
|
Loading…
Reference in New Issue
Block a user