add multiple get_attrs and includes_repr and they all work!

Author: nhamovitz

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;

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

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: []};
+}