diff options
| -rw-r--r-- | clippy_lints/src/lib.rs | 1 | ||||
| -rw-r--r-- | clippy_lints/src/trailing_zero_sized_array_without_repr_c.rs | 66 | ||||
| -rw-r--r-- | tests/ui/trailing_zero_sized_array_without_repr_c.rs | 125 |
3 files changed, 123 insertions, 69 deletions
diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index d494892c3b4..72636146d7c 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -21,6 +21,7 @@ // (Currently there is no way to opt into sysroot crates without `extern crate`.) extern crate rustc_ast; extern crate rustc_ast_pretty; +extern crate rustc_attr; extern crate rustc_data_structures; extern crate rustc_driver; extern crate rustc_errors; 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 7f579835512..4c3c5191d28 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,10 +1,14 @@ use clippy_utils::consts::{miri_to_const, Constant}; use clippy_utils::diagnostics::span_lint_and_sugg; use clippy_utils::source::snippet; +use rustc_ast::Attribute; use rustc_errors::Applicability; use rustc_hir::def_id::LocalDefId; -use rustc_hir::{Item, ItemKind, TyKind, VariantData}; +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_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; @@ -33,19 +37,16 @@ declare_clippy_lint! { /// ``` pub TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C, nursery, - "struct with a trailing zero-sized array but without `repr(C)`" + "struct with a trailing zero-sized array but without `repr(C)` or another `repr` attribute" } declare_lint_pass!(TrailingZeroSizedArrayWithoutReprC => [TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C]); -// -// TODO: Register the lint pass in `clippy_lints/src/lib.rs`, -// e.g. store.register_early_pass(|| -// Box::new(trailing_zero_sized_array_without_repr_c::TrailingZeroSizedArrayWithoutReprC)); -// DONE! +// TESTNAME=trailing_zero_sized_array_without_repr_c cargo uitest impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutReprC { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { - if is_struct_with_trailing_zero_sized_array(cx, item) && !has_repr_c(cx, item.def_id) { + dbg!(item.ident); + if is_struct_with_trailing_zero_sized_array(cx, item) && !has_repr_attr(cx, item.def_id) { span_lint_and_sugg( cx, TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C, @@ -64,7 +65,7 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx 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; + 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 @@ -83,17 +84,50 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx } } -fn has_repr_c(cx: &LateContext<'tcx>, def_id: LocalDefId) -> bool { +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 = dbg!(includes_repr_attr_using_sym(attrs_get_attrs)); + let b12 = dbg!(includes_repr_attr_using_sym(attrs_hir_map)); + let b21 = dbg!(includes_repr_attr_using_helper(cx, attrs_get_attrs)); + let b22 = dbg!(includes_repr_attr_using_helper(cx, attrs_hir_map)); + let b3 = dbg!(has_repr_attr_using_adt(cx, def_id)); + let all_same = b11 && b12 && b21 && b22 && b3; + dbg!(all_same); + + b11 +} + +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); - let attrs = hir_map.attrs(hir_id); + 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(_attr) = attrs.iter().find(|attr| attr.has_name(sym::repr)) { - true +// 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 { + 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 62fe94d7abf..8e8c84fe9c5 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr_c.rs +++ b/tests/ui/trailing_zero_sized_array_without_repr_c.rs @@ -1,13 +1,9 @@ #![warn(clippy::trailing_zero_sized_array_without_repr_c)] // #![feature(const_generics_defaults)] // see below -struct RarelyUseful { - field: i32, - last: [usize; 0], -} +// Do lint: -#[repr(C)] -struct GoodReason { +struct RarelyUseful { field: i32, last: [usize; 0], } @@ -21,24 +17,25 @@ struct GenericArrayType<T> { last: [T; 0], } -struct SizedArray { +#[derive(Debug)] +struct PlayNiceWithOtherAttributesDerive { field: i32, - last: [usize; 1], + last: [usize; 0] } -const ZERO: usize = 0; -struct ZeroSizedFromExternalConst { +#[must_use] +struct PlayNiceWithOtherAttributesMustUse { field: i32, - last: [usize; ZERO], + last: [usize; 0] } -const ONE: usize = 1; -struct NonZeroSizedFromExternalConst { +const ZERO: usize = 0; +struct ZeroSizedFromExternalConst { field: i32, - last: [usize; ONE], + last: [usize; ZERO], } -#[allow(clippy::eq_op)] // lmao im impressed +#[allow(clippy::eq_op)] const fn compute_zero() -> usize { (4 + 6) - (2 * 5) } @@ -47,36 +44,62 @@ 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. +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], +} -// #[repr(C)] -// struct ConstParamOk<const N: usize = 0> { -// field: i32, -// last: [usize; N] -// } +// Don't lint -// struct ConstParamLint<const N: usize = 0> { -// field: i32, -// last: [usize; N] -// } +#[repr(C)] +struct GoodReason { + field: i32, + last: [usize; 0], +} + +struct SizedArray { + field: i32, + last: [usize; 1], +} + +const ONE: usize = 1; +struct NonZeroSizedFromExternalConst { + field: i32, + last: [usize; ONE], +} -// TODO: actually, uh,, no idea what behavior here would be #[repr(packed)] struct ReprPacked { - small: u8, - medium: i32, - weird: [u64; 0], + field: i32, + last: [usize; 0], +} + +#[repr(C, packed)] +struct ReprCPacked { + field: i32, + last: [usize; 0], } -// TODO: clarify expected behavior #[repr(align(64))] struct ReprAlign { field: i32, last: [usize; 0], } - -// TODO: clarify expected behavior #[repr(C, align(64))] struct ReprCAlign { field: i32, @@ -91,24 +114,20 @@ enum DontLintAnonymousStructsFromDesuraging { C { x: u32, y: [u64; 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], -} +// 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, +// last: [usize; N] +// } -fn main() {} +// struct ConstParamLint<const N: usize = 0> { +// field: i32, +// last: [usize; N] +// } + +fn main() { + let _ = PlayNiceWithOtherAttributesMustUse {field: 0, last: []}; +} |