diff options
3 files changed, 147 insertions, 41 deletions
diff --git a/clippy_lints/src/trailing_zero_sized_array_without_repr_c.rs b/clippy_lints/src/trailing_zero_sized_array_without_repr_c.rs index 7eeb2914c40..7f579835512 100644 --- a/clippy_lints/src/trailing_zero_sized_array_without_repr_c.rs +++ b/clippy_lints/src/trailing_zero_sized_array_without_repr_c.rs @@ -26,7 +26,7 @@ declare_clippy_lint! { /// Use instead: /// ```rust /// #[repr(C)] - /// struct MakesSense { + /// struct MoreOftenUseful { /// some_field: usize, /// last: [SomeType; 0], /// } @@ -45,15 +45,13 @@ declare_lint_pass!(TrailingZeroSizedArrayWithoutReprC => [TRAILING_ZERO_SIZED_AR impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutReprC { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { - dbg!(item.ident); - if is_struct_with_trailing_zero_sized_array(cx, item) && !has_repr_c(cx, item.def_id) { span_lint_and_sugg( cx, TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C, item.span, "trailing zero-sized array in a struct which is not marked `#[repr(C)]`", - "try", + "try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute):", format!("#[repr(C)]\n{}", snippet(cx, item.span, "..")), Applicability::MaybeIncorrect, ); @@ -62,46 +60,40 @@ impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutReprC { } fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) -> bool { - if let ItemKind::Struct(data, _generics) = &item.kind { - if let VariantData::Struct(field_defs, _) = data { - if let Some(last_field) = field_defs.last() { - if let TyKind::Array(_, aconst) = last_field.ty.kind { - let aconst_def_id = cx.tcx.hir().body_owner_def_id(aconst.body).to_def_id(); - let ty = cx.tcx.type_of(aconst_def_id); - let constant = cx - .tcx - // NOTE: maybe const_eval_resolve? seems especially cursed to be using a const expr which - // resolves to 0 to create a zero-sized array, tho - .const_eval_poly(aconst_def_id) - .ok() - .map(|val| rustc_middle::ty::Const::from_value(cx.tcx, val, ty)); - if let Some(Constant::Int(val)) = constant.and_then(miri_to_const) { - if val == 0 { - // eprintln!("trailing: true"); - return true; - } - } - } - } + if_chain! { + if let ItemKind::Struct(data, _generics) = &item.kind; + if let VariantData::Struct(field_defs, _) = data; + if let Some(last_field) = field_defs.last(); + if let TyKind::Array(_, aconst) = last_field.ty.kind; + let aconst_def_id = cx.tcx.hir().body_owner_def_id(aconst.body).to_def_id(); + let ty = cx.tcx.type_of(aconst_def_id); + let constant = cx + .tcx + // NOTE: maybe const_eval_resolve? + .const_eval_poly(aconst_def_id) + .ok() + .map(|val| rustc_middle::ty::Const::from_value(cx.tcx, val, ty)); + if let Some(Constant::Int(val)) = constant.and_then(miri_to_const); + if val == 0; + then { + true + } else { + false } } - // dbg!(aconst); - // eprintln!("trailing: false"); - false } fn has_repr_c(cx: &LateContext<'tcx>, def_id: LocalDefId) -> bool { - let hir_id = cx.tcx.hir().local_def_id_to_hir_id(def_id); - let attrs = cx.tcx.hir().attrs(hir_id); + let hir_map = cx.tcx.hir(); + let hir_id = hir_map.local_def_id_to_hir_id(def_id); + let attrs = hir_map.attrs(hir_id); // NOTE: Can there ever be more than one `repr` attribute? // other `repr` syms: repr, repr128, repr_align, repr_align_enum, repr_no_niche, repr_packed, // repr_simd, repr_transparent - if let Some(_repr_attr) = attrs.iter().find(|attr| attr.has_name(sym::repr)) { - // eprintln!("repr: true"); + if let Some(_attr) = attrs.iter().find(|attr| attr.has_name(sym::repr)) { true } else { - // eprintln!("repr: false"); false } } diff --git a/tests/ui/trailing_zero_sized_array_without_repr_c.rs b/tests/ui/trailing_zero_sized_array_without_repr_c.rs index 311193fb4a1..62fe94d7abf 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr_c.rs +++ b/tests/ui/trailing_zero_sized_array_without_repr_c.rs @@ -1,6 +1,5 @@ #![warn(clippy::trailing_zero_sized_array_without_repr_c)] - -// #![feature(const_generics_defaults)] +// #![feature(const_generics_defaults)] // see below struct RarelyUseful { field: i32, @@ -48,6 +47,9 @@ struct UsingFunction { last: [usize; compute_zero()], } +// NOTE: including these (along with the required feature) triggers an ICE. Should make sure the +// const generics people are aware of that if they weren't already. + // #[repr(C)] // struct ConstParamOk<const N: usize = 0> { // field: i32, @@ -59,8 +61,7 @@ struct UsingFunction { // last: [usize; N] // } - -// TODO: actually, uh,, +// TODO: actually, uh,, no idea what behavior here would be #[repr(packed)] struct ReprPacked { small: u8, @@ -68,20 +69,21 @@ struct ReprPacked { weird: [u64; 0], } -// same +// TODO: clarify expected behavior #[repr(align(64))] struct ReprAlign { field: i32, last: [usize; 0], } -// same +// TODO: clarify expected behavior #[repr(C, align(64))] struct ReprCAlign { field: i32, last: [usize; 0], } +// NOTE: because of https://doc.rust-lang.org/stable/reference/type-layout.html#primitive-representation-of-enums-with-fields and I'm not sure when in the compilation pipeline that would happen #[repr(C)] enum DontLintAnonymousStructsFromDesuraging { A(u32), @@ -109,5 +111,4 @@ struct LotsOfFields { last: [usize; 0], } -fn main() { -} +fn main() {} diff --git a/tests/ui/trailing_zero_sized_array_without_repr_c.stderr b/tests/ui/trailing_zero_sized_array_without_repr_c.stderr new file mode 100644 index 00000000000..84606ed6185 --- /dev/null +++ b/tests/ui/trailing_zero_sized_array_without_repr_c.stderr @@ -0,0 +1,113 @@ +error: trailing zero-sized array in a struct which is not marked `#[repr(C)]` + --> $DIR/trailing_zero_sized_array_without_repr_c.rs:4:1 + | +LL | / struct RarelyUseful { +LL | | field: i32, +LL | | last: [usize; 0], +LL | | } + | |_^ + | + = note: `-D clippy::trailing-zero-sized-array-without-repr-c` implied by `-D warnings` +help: try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute): + | +LL + #[repr(C)] +LL + struct RarelyUseful { +LL + field: i32, +LL + last: [usize; 0], +LL + } + | + +error: trailing zero-sized array in a struct which is not marked `#[repr(C)]` + --> $DIR/trailing_zero_sized_array_without_repr_c.rs:15:1 + | +LL | / struct OnlyFieldIsZeroSizeArray { +LL | | first_and_last: [usize; 0], +LL | | } + | |_^ + | +help: try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute): + | +LL + #[repr(C)] +LL + struct OnlyFieldIsZeroSizeArray { +LL + first_and_last: [usize; 0], +LL + } + | + +error: trailing zero-sized array in a struct which is not marked `#[repr(C)]` + --> $DIR/trailing_zero_sized_array_without_repr_c.rs:19:1 + | +LL | / struct GenericArrayType<T> { +LL | | field: i32, +LL | | last: [T; 0], +LL | | } + | |_^ + | +help: try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute): + | +LL + #[repr(C)] +LL + struct GenericArrayType<T> { +LL + field: i32, +LL + last: [T; 0], +LL + } + | + +error: trailing zero-sized array in a struct which is not marked `#[repr(C)]` + --> $DIR/trailing_zero_sized_array_without_repr_c.rs:30:1 + | +LL | / struct ZeroSizedFromExternalConst { +LL | | field: i32, +LL | | last: [usize; ZERO], +LL | | } + | |_^ + | +help: try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute): + | +LL + #[repr(C)] +LL + struct ZeroSizedFromExternalConst { +LL + field: i32, +LL + last: [usize; ZERO], +LL + } + | + +error: trailing zero-sized array in a struct which is not marked `#[repr(C)]` + --> $DIR/trailing_zero_sized_array_without_repr_c.rs:45:1 + | +LL | / struct UsingFunction { +LL | | field: i32, +LL | | last: [usize; compute_zero()], +LL | | } + | |_^ + | +help: try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute): + | +LL + #[repr(C)] +LL + struct UsingFunction { +LL + field: i32, +LL + last: [usize; compute_zero()], +LL + } + | + +error: trailing zero-sized array in a struct which is not marked `#[repr(C)]` + --> $DIR/trailing_zero_sized_array_without_repr_c.rs:94:1 + | +LL | / struct LotsOfFields { +LL | | f1: u32, +LL | | f2: u32, +LL | | f3: u32, +... | +LL | | last: [usize; 0], +LL | | } + | |_^ + | +help: try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute): + | +LL + #[repr(C)] +LL + struct LotsOfFields { +LL + f1: u32, +LL + f2: u32, +LL + f3: u32, +LL + f4: u32, + ... + +error: aborting due to 6 previous errors + |