diff --git a/CHANGELOG.md b/CHANGELOG.md index 7ffb2be4a76..dacf552d606 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3022,6 +3022,7 @@ Released 2018-09-13 [`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments [`too_many_lines`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines [`toplevel_ref_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#toplevel_ref_arg +[`trailing_empty_array`]: https://rust-lang.github.io/rust-clippy/master/index.html#trailing_empty_array [`trait_duplication_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#trait_duplication_in_bounds [`transmute_bytes_to_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_bytes_to_str [`transmute_float_to_int`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_float_to_int diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 83e51e7886a..b1aa2aa930e 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -444,6 +444,7 @@ store.register_lints(&[ temporary_assignment::TEMPORARY_ASSIGNMENT, to_digit_is_some::TO_DIGIT_IS_SOME, to_string_in_display::TO_STRING_IN_DISPLAY, + trailing_empty_array::TRAILING_EMPTY_ARRAY, trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS, trait_bounds::TYPE_REPETITION_IN_BOUNDS, transmute::CROSSPOINTER_TRANSMUTE, diff --git a/clippy_lints/src/lib.register_nursery.rs b/clippy_lints/src/lib.register_nursery.rs index 96e0b421094..1e54482a8da 100644 --- a/clippy_lints/src/lib.register_nursery.rs +++ b/clippy_lints/src/lib.register_nursery.rs @@ -25,6 +25,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![ LintId::of(regex::TRIVIAL_REGEX), LintId::of(strings::STRING_LIT_AS_BYTES), LintId::of(suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS), + LintId::of(trailing_empty_array::TRAILING_EMPTY_ARRAY), LintId::of(transmute::USELESS_TRANSMUTE), LintId::of(use_self::USE_SELF), ]) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 9794d74e198..58e4c061892 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -355,6 +355,7 @@ mod tabs_in_doc_comments; mod temporary_assignment; mod to_digit_is_some; mod to_string_in_display; +mod trailing_empty_array; mod trait_bounds; mod transmute; mod transmuting_null; @@ -777,6 +778,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || Box::new(undocumented_unsafe_blocks::UndocumentedUnsafeBlocks::default())); store.register_late_pass(|| Box::new(match_str_case_mismatch::MatchStrCaseMismatch)); store.register_late_pass(move || Box::new(format_args::FormatArgs)); + store.register_late_pass(|| Box::new(trailing_empty_array::TrailingEmptyArray)); + } #[rustfmt::skip] diff --git a/clippy_lints/src/trailing_empty_array.rs b/clippy_lints/src/trailing_empty_array.rs new file mode 100644 index 00000000000..c216a1f81ea --- /dev/null +++ b/clippy_lints/src/trailing_empty_array.rs @@ -0,0 +1,77 @@ +use clippy_utils::diagnostics::span_lint_and_help; +use rustc_hir::{HirId, Item, ItemKind}; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::Const; +use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::sym; + +declare_clippy_lint! { + /// ### What it does + /// Displays a warning when a struct with a trailing zero-sized array is declared without a `repr` attribute. + /// + /// ### Why is this bad? + /// Zero-sized arrays aren't very useful in Rust itself, so such a struct is likely being created to pass to C code or in some other situation where control over memory layout matters (for example, in conjuction with manual allocation to make it easy to compute the offset of the array). Either way, `#[repr(C)]` (or another `repr` attribute) is needed. + /// + /// ### Example + /// ```rust + /// struct RarelyUseful { + /// some_field: u32, + /// last: [u32; 0], + /// } + /// ``` + /// + /// Use instead: + /// ```rust + /// #[repr(C)] + /// struct MoreOftenUseful { + /// some_field: usize, + /// last: [u32; 0], + /// } + /// ``` + pub TRAILING_EMPTY_ARRAY, + nursery, + "struct with a trailing zero-sized array but without `#[repr(C)]` or another `repr` attribute" +} +declare_lint_pass!(TrailingEmptyArray => [TRAILING_EMPTY_ARRAY]); + +impl<'tcx> LateLintPass<'tcx> for TrailingEmptyArray { + fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { + if is_struct_with_trailing_zero_sized_array(cx, item) && !has_repr_attr(cx, item.hir_id()) { + span_lint_and_help( + cx, + TRAILING_EMPTY_ARRAY, + item.span, + "trailing zero-sized array in a struct which is not marked with a `repr` attribute", + None, + &format!( + "consider annotating `{}` with `#[repr(C)]` or another `repr` attribute", + cx.tcx.def_path_str(item.def_id.to_def_id()) + ), + ); + } + } +} + +fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) -> bool { + if_chain! { + // First check if last field is an array + if let ItemKind::Struct(data, _) = &item.kind; + if let Some(last_field) = data.fields().last(); + if let rustc_hir::TyKind::Array(_, length) = last_field.ty.kind; + + // Then check if that that array zero-sized + let length_ldid = cx.tcx.hir().local_def_id(length.hir_id); + let length = Const::from_anon_const(cx.tcx, length_ldid); + let length = length.try_eval_usize(cx.tcx, cx.param_env); + if let Some(length) = length; + then { + length == 0 + } else { + false + } + } +} + +fn has_repr_attr(cx: &LateContext<'_>, hir_id: HirId) -> bool { + cx.tcx.hir().attrs(hir_id).iter().any(|attr| attr.has_name(sym::repr)) +} diff --git a/tests/ui/trailing_empty_array.rs b/tests/ui/trailing_empty_array.rs new file mode 100644 index 00000000000..501c9eb7651 --- /dev/null +++ b/tests/ui/trailing_empty_array.rs @@ -0,0 +1,186 @@ +#![warn(clippy::trailing_empty_array)] +#![feature(const_generics_defaults)] + +// Do lint: + +struct RarelyUseful { + field: i32, + last: [usize; 0], +} + +struct OnlyField { + first_and_last: [usize; 0], +} + +struct GenericArrayType { + field: i32, + last: [T; 0], +} + +#[must_use] +struct OnlyAnotherAttribute { + field: i32, + last: [usize; 0], +} + +#[derive(Debug)] +struct OnlyADeriveAttribute { + field: i32, + last: [usize; 0], +} + +const ZERO: usize = 0; +struct ZeroSizedWithConst { + field: i32, + last: [usize; ZERO], +} + +#[allow(clippy::eq_op)] +const fn compute_zero() -> usize { + (4 + 6) - (2 * 5) +} +struct ZeroSizedWithConstFunction { + field: i32, + last: [usize; compute_zero()], +} + +const fn compute_zero_from_arg(x: usize) -> usize { + x - 1 +} +struct ZeroSizedWithConstFunction2 { + field: i32, + last: [usize; compute_zero_from_arg(1)], +} + +struct ZeroSizedArrayWrapper([usize; 0]); + +struct TupleStruct(i32, [usize; 0]); + +struct LotsOfFields { + f1: u32, + f2: u32, + f3: u32, + f4: u32, + f5: u32, + f6: u32, + f7: u32, + f8: u32, + f9: u32, + f10: u32, + f11: u32, + f12: u32, + f13: u32, + f14: u32, + f15: u32, + f16: u32, + last: [usize; 0], +} + +// Don't lint + +#[repr(C)] +struct GoodReason { + field: i32, + last: [usize; 0], +} + +#[repr(C)] +struct OnlyFieldWithReprC { + first_and_last: [usize; 0], +} + +struct NonZeroSizedArray { + field: i32, + last: [usize; 1], +} + +struct NotLastField { + f1: u32, + zero_sized: [usize; 0], + last: i32, +} + +const ONE: usize = 1; +struct NonZeroSizedWithConst { + field: i32, + last: [usize; ONE], +} + +#[derive(Debug)] +#[repr(C)] +struct AlsoADeriveAttribute { + field: i32, + last: [usize; 0], +} + +#[must_use] +#[repr(C)] +struct AlsoAnotherAttribute { + field: i32, + last: [usize; 0], +} + +#[repr(packed)] +struct ReprPacked { + field: i32, + last: [usize; 0], +} + +#[repr(C, packed)] +struct ReprCPacked { + field: i32, + last: [usize; 0], +} + +#[repr(align(64))] +struct ReprAlign { + field: i32, + last: [usize; 0], +} +#[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), + B(f32, [u64; 0]), + C { x: u32, y: [u64; 0] }, +} + +#[repr(C)] +struct TupleStructReprC(i32, [usize; 0]); + +type NamedTuple = (i32, [usize; 0]); + +#[rustfmt::skip] // [rustfmt#4995](https://github.com/rust-lang/rustfmt/issues/4995) +struct ConstParamZeroDefault { + field: i32, + last: [usize; N], +} + +struct ConstParamNoDefault { + field: i32, + last: [usize; N], +} + +#[rustfmt::skip] +struct ConstParamNonZeroDefault { + field: i32, + last: [usize; N], +} + +struct TwoGenericParams { + field: i32, + last: [T; N], +} + +type A = ConstParamZeroDefault; +type B = ConstParamZeroDefault<0>; +type C = ConstParamNoDefault<0>; +type D = ConstParamNonZeroDefault<0>; + +fn main() {} diff --git a/tests/ui/trailing_empty_array.stderr b/tests/ui/trailing_empty_array.stderr new file mode 100644 index 00000000000..d88aa0504b5 --- /dev/null +++ b/tests/ui/trailing_empty_array.stderr @@ -0,0 +1,120 @@ +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:6:1 + | +LL | / struct RarelyUseful { +LL | | field: i32, +LL | | last: [usize; 0], +LL | | } + | |_^ + | + = note: `-D clippy::trailing-empty-array` implied by `-D warnings` + = help: consider annotating `RarelyUseful` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:11:1 + | +LL | / struct OnlyField { +LL | | first_and_last: [usize; 0], +LL | | } + | |_^ + | + = help: consider annotating `OnlyField` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:15:1 + | +LL | / struct GenericArrayType { +LL | | field: i32, +LL | | last: [T; 0], +LL | | } + | |_^ + | + = help: consider annotating `GenericArrayType` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:21:1 + | +LL | / struct OnlyAnotherAttribute { +LL | | field: i32, +LL | | last: [usize; 0], +LL | | } + | |_^ + | + = help: consider annotating `OnlyAnotherAttribute` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:27:1 + | +LL | / struct OnlyADeriveAttribute { +LL | | field: i32, +LL | | last: [usize; 0], +LL | | } + | |_^ + | + = help: consider annotating `OnlyADeriveAttribute` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:33:1 + | +LL | / struct ZeroSizedWithConst { +LL | | field: i32, +LL | | last: [usize; ZERO], +LL | | } + | |_^ + | + = help: consider annotating `ZeroSizedWithConst` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:42:1 + | +LL | / struct ZeroSizedWithConstFunction { +LL | | field: i32, +LL | | last: [usize; compute_zero()], +LL | | } + | |_^ + | + = help: consider annotating `ZeroSizedWithConstFunction` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:50:1 + | +LL | / struct ZeroSizedWithConstFunction2 { +LL | | field: i32, +LL | | last: [usize; compute_zero_from_arg(1)], +LL | | } + | |_^ + | + = help: consider annotating `ZeroSizedWithConstFunction2` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:55:1 + | +LL | struct ZeroSizedArrayWrapper([usize; 0]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider annotating `ZeroSizedArrayWrapper` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:57:1 + | +LL | struct TupleStruct(i32, [usize; 0]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = help: consider annotating `TupleStruct` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_empty_array.rs:59:1 + | +LL | / struct LotsOfFields { +LL | | f1: u32, +LL | | f2: u32, +LL | | f3: u32, +... | +LL | | last: [usize; 0], +LL | | } + | |_^ + | + = help: consider annotating `LotsOfFields` with `#[repr(C)]` or another `repr` attribute + +error: aborting due to 11 previous errors +