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 a9c6e24918e..913812126a9 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 @@ -1,14 +1,11 @@ use clippy_utils::consts::{miri_to_const, Constant}; use clippy_utils::diagnostics::span_lint_and_help; use rustc_ast::Attribute; -use rustc_hir::def_id::LocalDefId; use rustc_hir::{Item, ItemKind, VariantData}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::dep_graph::DepContext; -use rustc_middle::ty as ty_mod; -use rustc_middle::ty::ReprFlags; +use rustc_middle::ty::Const; use rustc_session::{declare_lint_pass, declare_tool_lint}; -use rustc_span::sym; declare_clippy_lint! { /// ### What it does @@ -43,37 +40,44 @@ 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_attr(cx, item.def_id) { - // span_lint_and_help( - // cx, - // TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C, - // item.span, - // "trailing zero-sized array in a struct which is not marked `#[repr(C)]`", - // None, - // "consider annotating the struct definition with `#[repr(C)]` (or another `repr` attribute)", - // ); - eprintln!("— consider yourself linted — 🦀") + if is_struct_with_trailing_zero_sized_array(cx, item) { + let attrs = cx.tcx.get_attrs(item.def_id.to_def_id()); + let first_attr = attrs.first(); // Actually, I've no idea if this is guaranteed to be the first one in the source code. + + let lint_span = if let Some(first_attr) = first_attr { + first_attr.span.until(item.span) + } else { + item.span + }; + + if !has_repr_attr(cx, attrs) { + span_lint_and_help( + cx, + TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C, + lint_span, + "trailing zero-sized array in a struct which is not marked `#[repr(C)]`", + None, + "consider annotating the struct definition with `#[repr(C)]` (or another `repr` attribute)", + ); + } } } } fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) -> bool { if_chain! { - if let ItemKind::Struct(data, _generics) = &item.kind; + // Check if last field is an array + if let ItemKind::Struct(data, _) = &item.kind; if let VariantData::Struct(field_defs, _) = data; if let Some(last_field) = field_defs.last(); - if let rustc_hir::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; + if let rustc_hir::TyKind::Array(_, length) = last_field.ty.kind; + + // 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); + if let Some(Constant::Int(length)) = miri_to_const(length); + if length == 0; + then { true } else { @@ -82,54 +86,9 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx } } -fn has_repr_attr(cx: &LateContext<'tcx>, def_id: LocalDefId) -> bool { - let attrs_get_attrs = get_attrs_get_attrs(cx, def_id); - let attrs_hir_map = get_attrs_hir_map(cx, def_id); - - let b11 = includes_repr_attr_using_sym(attrs_get_attrs); - let b12 = includes_repr_attr_using_sym(attrs_hir_map); - let b21 = includes_repr_attr_using_helper(cx, attrs_get_attrs); - let b22 = includes_repr_attr_using_helper(cx, attrs_hir_map); - let b3 = has_repr_attr_using_adt(cx, def_id); - let all_same = (b11 && b12 && b21 && b22 && b3) || (!b11 && !b12 && !b21 && !b22 && !b3); - dbg!(all_same); - - b21 -} - -fn get_attrs_get_attrs(cx: &LateContext<'tcx>, def_id: LocalDefId) -> &'tcx [Attribute] { - cx.tcx.get_attrs(def_id.to_def_id()) -} - -fn get_attrs_hir_map(cx: &LateContext<'tcx>, def_id: LocalDefId) -> &'tcx [Attribute] { - let hir_map = cx.tcx.hir(); - let hir_id = hir_map.local_def_id_to_hir_id(def_id); - hir_map.attrs(hir_id) -} - -// Don't like this because it's so dependent on the current list of `repr` flags and it would have -// to be manually updated if that ever expanded. idk if there's any mechanism in `bitflag!` or -// elsewhere for requiring that sort of exhaustiveness -fn has_repr_attr_using_adt(cx: &LateContext<'tcx>, def_id: LocalDefId) -> bool { - let ty = cx.tcx.type_of(def_id.to_def_id()); - if let ty_mod::Adt(adt, _) = ty.kind() { - if adt.is_struct() { - let repr = adt.repr; - let repr_attr = ReprFlags::IS_C | ReprFlags::IS_TRANSPARENT | ReprFlags::IS_SIMD | ReprFlags::IS_LINEAR; - repr.int.is_some() || repr.align.is_some() || repr.pack.is_some() || repr.flags.intersects(repr_attr) - } else { - false - } - } else { - false - } -} - -fn includes_repr_attr_using_sym(attrs: &'tcx [Attribute]) -> bool { - attrs.iter().any(|attr| attr.has_name(sym::repr)) -} - -fn includes_repr_attr_using_helper(cx: &LateContext<'tcx>, attrs: &'tcx [Attribute]) -> bool { +fn has_repr_attr(cx: &LateContext<'tcx>, attrs: &[Attribute]) -> bool { + // NOTE: there's at least four other ways to do this but I liked this one the best. (All five agreed + // on all testcases.) Happy to use another; they're in the commit history. attrs .iter() .any(|attr| !rustc_attr::find_repr_attrs(cx.tcx.sess(), attr).is_empty()) 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 07cba5774a5..6ab96c2ebf6 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr_c.rs +++ b/tests/ui/trailing_zero_sized_array_without_repr_c.rs @@ -8,7 +8,7 @@ struct RarelyUseful { last: [usize; 0], } -struct OnlyFieldIsZeroSizeArray { +struct OnlyField { first_and_last: [usize; 0], } @@ -18,19 +18,19 @@ struct GenericArrayType { } #[derive(Debug)] -struct PlayNiceWithOtherAttributesDerive { +struct OnlyAnotherAttributeDerive { field: i32, last: [usize; 0], } #[must_use] -struct PlayNiceWithOtherAttributesMustUse { +struct OnlyAnotherAttributeMustUse { field: i32, last: [usize; 0], } const ZERO: usize = 0; -struct ZeroSizedFromExternalConst { +struct ZeroSizedWithConst { field: i32, last: [usize; ZERO], } @@ -39,7 +39,7 @@ struct ZeroSizedFromExternalConst { const fn compute_zero() -> usize { (4 + 6) - (2 * 5) } -struct UsingFunction { +struct ZeroSizedWithConstFunction { field: i32, last: [usize; compute_zero()], } @@ -72,17 +72,36 @@ struct GoodReason { last: [usize; 0], } +#[repr(C)] +struct OnlyFieldWithReprC { + first_and_last: [usize; 0], +} + struct NonZeroSizedArray { field: i32, last: [usize; 1], } const ONE: usize = 1; -struct NonZeroSizedFromExternalConst { +struct NonZeroSizedWithConst { field: i32, last: [usize; ONE], } +#[derive(Debug)] +#[repr(C)] +struct OtherAttributesDerive { + field: i32, + last: [usize; 0], +} + +#[must_use] +#[repr(C)] +struct OtherAttributesMustUse { + field: i32, + last: [usize; 0], +} + #[repr(packed)] struct ReprPacked { field: i32, @@ -129,5 +148,6 @@ enum DontLintAnonymousStructsFromDesuraging { // } fn main() { - let _ = PlayNiceWithOtherAttributesMustUse { field: 0, last: [] }; + let _ = OnlyAnotherAttributeMustUse { field: 0, last: [] }; + let _ = OtherAttributesMustUse { field: 0, last: [] }; }