about summary refs log tree commit diff
diff options
context:
space:
mode:
authorNathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com>2021-10-16 17:49:13 -0700
committerNathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com>2021-10-18 03:05:19 -0700
commit9b3f55ee61e781ef3360ddfaa436746bb7e40df5 (patch)
treec10ad5fff2f7dfdb3dbd5c9593671048b68eb2f3
parent5fdf93415bfd59a7cd61ebec20f0a4e4fec78164 (diff)
downloadrust-9b3f55ee61e781ef3360ddfaa436746bb7e40df5.tar.gz
rust-9b3f55ee61e781ef3360ddfaa436746bb7e40df5.zip
tried to simplify but it doesn't work :/
-rw-r--r--clippy_lints/src/trailing_zero_sized_array_without_repr_c.rs109
-rw-r--r--tests/ui/trailing_zero_sized_array_without_repr_c.rs34
2 files changed, 61 insertions, 82 deletions
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,93 +40,55 @@ declare_lint_pass!(TrailingZeroSizedArrayWithoutReprC => [TRAILING_ZERO_SIZED_AR
 
 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;
-        then {
-            true
-        } else {
-            false
-        }
-    }
-}
-
-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);
+        if let rustc_hir::TyKind::Array(_, length) = last_field.ty.kind;
 
-    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);
+        // 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;
 
-    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)
+        then {
+            true
         } 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<T> {
 }
 
 #[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: [] };
 }