Auto merge of rust-lang#7838 - nhamovitz:trailing_zs_arr_wo_repr, r=Manishearth

Warn on structs with a trailing zero-sized array but no `repr` attribute

Closes rust-lang#2868

changelog: Implement ``[`trailing_empty_array`]``, which warns if a struct is defined where the last field is a zero-sized array but there are no `repr` attributes. 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. Either way, a `repr` attribute is needed.

Author: bors

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

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,

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),
 ])

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]

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))
+}

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<T> {
+    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<const N: usize = 0> {
+    field: i32,
+    last: [usize; N],
+}
+
+struct ConstParamNoDefault<const N: usize> {
+    field: i32,
+    last: [usize; N],
+}
+
+#[rustfmt::skip] 
+struct ConstParamNonZeroDefault<const N: usize = 1> {
+    field: i32,
+    last: [usize; N],
+}
+
+struct TwoGenericParams<T, const N: usize> {
+    field: i32,
+    last: [T; N],
+}
+
+type A = ConstParamZeroDefault;
+type B = ConstParamZeroDefault<0>;
+type C = ConstParamNoDefault<0>;
+type D = ConstParamNonZeroDefault<0>;
+
+fn main() {}