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 @@ /// ``` 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,9 +17,16 @@ struct GenericArrayType { last: [T; 0], } -struct SizedArray { +#[derive(Debug)] +struct PlayNiceWithOtherAttributesDerive { field: i32, - last: [usize; 1], + last: [usize; 0] +} + +#[must_use] +struct PlayNiceWithOtherAttributesMustUse { + field: i32, + last: [usize; 0] } const ZERO: usize = 0; @@ -32,13 +35,7 @@ struct ZeroSizedFromExternalConst { last: [usize; ZERO], } -const ONE: usize = 1; -struct NonZeroSizedFromExternalConst { - field: i32, - last: [usize; ONE], -} - -#[allow(clippy::eq_op)] // lmao im impressed +#[allow(clippy::eq_op)] const fn compute_zero() -> usize { (4 + 6) - (2 * 5) } @@ -47,50 +44,6 @@ 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 { -// field: i32, -// last: [usize; N] -// } - -// struct ConstParamLint { -// field: i32, -// last: [usize; N] -// } - -// TODO: actually, uh,, no idea what behavior here would be -#[repr(packed)] -struct ReprPacked { - small: u8, - medium: i32, - weird: [u64; 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, - 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] }, -} - struct LotsOfFields { f1: u32, f2: u32, @@ -111,4 +64,70 @@ struct LotsOfFields { last: [usize; 0], } -fn main() {} +// Don't lint + +#[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], +} + +#[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] }, +} + +// 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 { +// field: i32, +// last: [usize; N] +// } + +// struct ConstParamLint { +// field: i32, +// last: [usize; N] +// } + +fn main() { + let _ = PlayNiceWithOtherAttributesMustUse {field: 0, last: []}; +}