Tidy comments + tests; revert 'size-is-zero' detection

nhamovitz · nhamovitz · commit a3420f70043c · 2021-10-18T03:05:19.000-07:00
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
@@ -36,16 +36,15 @@ declare_clippy_lint! {
 }
 declare_lint_pass!(TrailingZeroSizedArrayWithoutReprC => [TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C]);
 
-// 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) {
+            // NOTE: This is to include attributes on the definition when we print the lint. If the convention
+            // is to not do that with struct definitions (I'm not sure), then this isn't necessary.
             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 first_attr = attrs.iter().min_by_key(|attr| attr.span.lo());
             let lint_span = if let Some(first_attr) = first_attr {
-                first_attr.span.until(item.span)
+                first_attr.span.to(item.span)
             } else {
                 item.span
             };
@@ -66,18 +65,29 @@ impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutReprC {
 
 fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) -> bool {
     if_chain! {
-        // Check if last field is an array
+        // First 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(_, 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 check if that that array zero-sized
 
+        // This is pretty much copied from `enum_clike.rs` and I don't fully understand it, so let me know
+        // if there's a better way. I tried `Const::from_anon_const` but it didn't fold in the values
+        // on the `ZeroSizedWithConst` and `ZeroSizedWithConstFunction` tests.
+
+        // This line in particular seems convoluted.
+        let length_did = cx.tcx.hir().body_owner_def_id(length.body).to_def_id();
+        let length_ty = cx.tcx.type_of(length_did);
+        let length = cx
+            .tcx
+            .const_eval_poly(length_did)
+            .ok()
+            .map(|val| Const::from_value(cx.tcx, val, length_ty))
+            .and_then(miri_to_const);
+        if let Some(Constant::Int(length)) = length;
+        if length == 0;
         then {
             true
         } else {
@@ -88,7 +98,8 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx
 
 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.
+    // on all testcases.) Happy to use another; they're in the commit history if you want to look (or I
+    // can go find them).
     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
@@ -1,5 +1,4 @@
 #![warn(clippy::trailing_zero_sized_array_without_repr_c)]
-#![feature(const_generics_defaults)] // see below
 
 // Do lint:
 
@@ -17,14 +16,16 @@ struct GenericArrayType<T> {
     last: [T; 0],
 }
 
-#[derive(Debug)]
-struct OnlyAnotherAttributeDerive {
+#[must_use]
+struct OnlyAnotherAttributeMustUse {
     field: i32,
     last: [usize; 0],
 }
 
-#[must_use]
-struct OnlyAnotherAttributeMustUse {
+// NOTE: Unfortunately the attribute isn't included in the lint output. I'm not sure how to make it
+// show up.
+#[derive(Debug)]
+struct OnlyAnotherAttributeDerive {
     field: i32,
     last: [usize; 0],
 }
@@ -82,6 +83,12 @@ struct NonZeroSizedArray {
     last: [usize; 1],
 }
 
+struct NotLastField {
+    f1: u32,
+    zero_sized: [usize; 0],
+    last: i32,
+}
+
 const ONE: usize = 1;
 struct NonZeroSizedWithConst {
     field: i32,
@@ -133,21 +140,4 @@ enum DontLintAnonymousStructsFromDesuraging {
     C { x: u32, y: [u64; 0] },
 }
 
-// NOTE: including these (along with the required feature) triggers an ICE. Not sure why. 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]
-// }
-
-// struct ConstParamLint<const N: usize = 0> {
-//     field: i32,
-//     last: [usize; N]
-// }
-
-fn main() {
-    let _ = OnlyAnotherAttributeMustUse { field: 0, last: [] };
-    let _ = OtherAttributesMustUse { field: 0, last: [] };
-}
+fn main() {}
diff --git a/tests/ui/trailing_zero_sized_array_without_repr_c.stderr b/tests/ui/trailing_zero_sized_array_without_repr_c.stderr
@@ -1,5 +1,5 @@
 error: trailing zero-sized array in a struct which is not marked `#[repr(C)]`
-  --> $DIR/trailing_zero_sized_array_without_repr_c.rs:4:1
+  --> $DIR/trailing_zero_sized_array_without_repr_c.rs:5:1
    |
 LL | / struct RarelyUseful {
 LL | |     field: i32,
@@ -8,87 +8,76 @@ LL | | }
    | |_^
    |
    = note: `-D clippy::trailing-zero-sized-array-without-repr-c` implied by `-D warnings`
-help: try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute):
-   |
-LL + #[repr(C)]
-LL + struct RarelyUseful {
-LL +     field: i32,
-LL +     last: [usize; 0],
-LL + }
-   |
+   = help: consider annotating the struct definition with `#[repr(C)]` (or another `repr` attribute)
 
 error: trailing zero-sized array in a struct which is not marked `#[repr(C)]`
-  --> $DIR/trailing_zero_sized_array_without_repr_c.rs:15:1
+  --> $DIR/trailing_zero_sized_array_without_repr_c.rs:10:1
    |
-LL | / struct OnlyFieldIsZeroSizeArray {
+LL | / struct OnlyField {
 LL | |     first_and_last: [usize; 0],
 LL | | }
    | |_^
    |
-help: try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute):
-   |
-LL + #[repr(C)]
-LL + struct OnlyFieldIsZeroSizeArray {
-LL +     first_and_last: [usize; 0],
-LL + }
-   |
+   = help: consider annotating the struct definition with `#[repr(C)]` (or another `repr` attribute)
 
 error: trailing zero-sized array in a struct which is not marked `#[repr(C)]`
-  --> $DIR/trailing_zero_sized_array_without_repr_c.rs:19:1
+  --> $DIR/trailing_zero_sized_array_without_repr_c.rs:14:1
    |
 LL | / struct GenericArrayType<T> {
 LL | |     field: i32,
 LL | |     last: [T; 0],
 LL | | }
    | |_^
    |
-help: try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute):
+   = help: consider annotating the struct definition with `#[repr(C)]` (or another `repr` attribute)
+
+error: trailing zero-sized array in a struct which is not marked `#[repr(C)]`
+  --> $DIR/trailing_zero_sized_array_without_repr_c.rs:19:1
    |
-LL + #[repr(C)]
-LL + struct GenericArrayType<T> {
-LL +     field: i32,
-LL +     last: [T; 0],
-LL + }
+LL | / #[must_use]
+LL | | struct OnlyAnotherAttributeMustUse {
+LL | |     field: i32,
+LL | |     last: [usize; 0],
+LL | | }
+   | |_^
    |
+   = help: consider annotating the struct definition with `#[repr(C)]` (or another `repr` attribute)
 
 error: trailing zero-sized array in a struct which is not marked `#[repr(C)]`
-  --> $DIR/trailing_zero_sized_array_without_repr_c.rs:30:1
+  --> $DIR/trailing_zero_sized_array_without_repr_c.rs:28:1
    |
-LL | / struct ZeroSizedFromExternalConst {
+LL | / struct OnlyAnotherAttributeDerive {
 LL | |     field: i32,
-LL | |     last: [usize; ZERO],
+LL | |     last: [usize; 0],
 LL | | }
    | |_^
    |
-help: try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute):
+   = help: consider annotating the struct definition with `#[repr(C)]` (or another `repr` attribute)
+
+error: trailing zero-sized array in a struct which is not marked `#[repr(C)]`
+  --> $DIR/trailing_zero_sized_array_without_repr_c.rs:34:1
    |
-LL + #[repr(C)]
-LL + struct ZeroSizedFromExternalConst {
-LL +     field: i32,
-LL +     last: [usize; ZERO],
-LL + }
+LL | / struct ZeroSizedWithConst {
+LL | |     field: i32,
+LL | |     last: [usize; ZERO],
+LL | | }
+   | |_^
    |
+   = help: consider annotating the struct definition with `#[repr(C)]` (or another `repr` attribute)
 
 error: trailing zero-sized array in a struct which is not marked `#[repr(C)]`
-  --> $DIR/trailing_zero_sized_array_without_repr_c.rs:45:1
+  --> $DIR/trailing_zero_sized_array_without_repr_c.rs:43:1
    |
-LL | / struct UsingFunction {
+LL | / struct ZeroSizedWithConstFunction {
 LL | |     field: i32,
 LL | |     last: [usize; compute_zero()],
 LL | | }
    | |_^
    |
-help: try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute):
-   |
-LL + #[repr(C)]
-LL + struct UsingFunction {
-LL +     field: i32,
-LL +     last: [usize; compute_zero()],
-LL + }
-   |
+   = help: consider annotating the struct definition with `#[repr(C)]` (or another `repr` attribute)
 
 error: trailing zero-sized array in a struct which is not marked `#[repr(C)]`
-  --> $DIR/trailing_zero_sized_array_without_repr_c.rs:94:1
+  --> $DIR/trailing_zero_sized_array_without_repr_c.rs:48:1
    |
 LL | / struct LotsOfFields {
 LL | |     f1: u32,
@@ -99,15 +88,7 @@ LL | |     last: [usize; 0],
 LL | | }
    | |_^
    |
-help: try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute):
-   |
-LL + #[repr(C)]
-LL + struct LotsOfFields {
-LL +     f1: u32,
-LL +     f2: u32,
-LL +     f3: u32,
-LL +     f4: u32,
- ...
+   = help: consider annotating the struct definition with `#[repr(C)]` (or another `repr` attribute)
 
-error: aborting due to 6 previous errors
+error: aborting due to 8 previous errors
 
