From 797507c58390662098f0d20a9ee89d200f93f11c Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Thu, 14 Oct 2021 20:08:38 -0700 Subject: [PATCH 01/37] Add boilerplate and basic tests --- CHANGELOG.md | 1 + clippy_lints/src/lib.register_lints.rs | 1 + clippy_lints/src/lib.register_nursery.rs | 1 + clippy_lints/src/lib.rs | 1 + ...railing_zero_sized_array_without_repr_c.rs | 58 +++++++++++++++++++ ...railing_zero_sized_array_without_repr_c.rs | 23 ++++++++ 6 files changed, 85 insertions(+) create mode 100644 clippy_lints/src/trailing_zero_sized_array_without_repr_c.rs create mode 100644 tests/ui/trailing_zero_sized_array_without_repr_c.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index f6a883311c9..a124bebe924 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3021,6 +3021,7 @@ Released 2018-09-13 [`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments [`too_many_lines`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines [`toplevel_ref_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#toplevel_ref_arg +[`trailing_zero_sized_array_without_repr_c`]: https://rust-lang.github.io/rust-clippy/master/index.html#trailing_zero_sized_array_without_repr_c [`trait_duplication_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#trait_duplication_in_bounds [`transmute_bytes_to_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_bytes_to_str [`transmute_float_to_int`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_float_to_int diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index f334f723f6b..34b87002fa3 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -443,6 +443,7 @@ store.register_lints(&[ temporary_assignment::TEMPORARY_ASSIGNMENT, to_digit_is_some::TO_DIGIT_IS_SOME, to_string_in_display::TO_STRING_IN_DISPLAY, + trailing_zero_sized_array_without_repr_c::TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C, trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS, trait_bounds::TYPE_REPETITION_IN_BOUNDS, transmute::CROSSPOINTER_TRANSMUTE, diff --git a/clippy_lints/src/lib.register_nursery.rs b/clippy_lints/src/lib.register_nursery.rs index 96e0b421094..325706746db 100644 --- a/clippy_lints/src/lib.register_nursery.rs +++ b/clippy_lints/src/lib.register_nursery.rs @@ -25,6 +25,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![ LintId::of(regex::TRIVIAL_REGEX), LintId::of(strings::STRING_LIT_AS_BYTES), LintId::of(suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS), + LintId::of(trailing_zero_sized_array_without_repr_c::TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C), LintId::of(transmute::USELESS_TRANSMUTE), LintId::of(use_self::USE_SELF), ]) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 9794d74e198..17bffc08309 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -355,6 +355,7 @@ mod tabs_in_doc_comments; mod temporary_assignment; mod to_digit_is_some; mod to_string_in_display; +mod trailing_zero_sized_array_without_repr_c; mod trait_bounds; mod transmute; mod transmuting_null; 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 new file mode 100644 index 00000000000..9aea22af274 --- /dev/null +++ b/clippy_lints/src/trailing_zero_sized_array_without_repr_c.rs @@ -0,0 +1,58 @@ +use clippy_utils::diagnostics::span_lint_and_sugg; +use rustc_hir::*; +use rustc_lint::{LateContext, LateLintPass}; +use rustc_session::{declare_lint_pass, declare_tool_lint}; + +declare_clippy_lint! { + /// ### What it does + /// Displays a warning when a struct with a trailing zero-sized array is declared without the `repr(C)` attribute. + /// + /// ### Why is this bad? + /// Zero-sized arrays aren't very useful in Rust itself, so such a struct is likely being created to pass to C code (or in conjuction with manual allocation to make it easy to compute the offset of the array). Either way, `#[repr(C)]` is needed. + /// + /// ### Example + /// ```rust + /// struct RarelyUseful { + /// some_field: usize, + /// last: [SomeType; 0], + /// } + /// ``` + /// + /// Use instead: + /// ```rust + /// #[repr(C)] + /// struct MakesSense { + /// some_field: usize, + /// last: [SomeType; 0], + /// } + /// ``` + pub TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C, + nursery, + "struct with a trailing zero-sized array but without `repr(C)`" +} +declare_lint_pass!(TrailingZeroSizedArrayWithoutReprC => [TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C]); + +impl LateLintPass<'_> for TrailingZeroSizedArrayWithoutReprC { + fn check_struct_def(&mut self, _: &LateContext<'tcx>, _: &'tcx rustc_hir::VariantData<'tcx>) {} + + fn check_struct_def_post(&mut self, _: &LateContext<'tcx>, _: &'tcx rustc_hir::VariantData<'tcx>) {} + // https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/sty/enum.TyKind.html#variant.Array in latepass + // or https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/ast/enum.TyKind.html#variant.Array in early pass + + fn check_field_def(&mut self, _: &LateContext<'tcx>, _: &'tcx rustc_hir::FieldDef<'tcx>) {} + + fn check_attribute(&mut self, _: &LateContext<'tcx>, _: &'tcx rustc_ast::Attribute) {} + + fn enter_lint_attrs(&mut self, _: &LateContext<'tcx>, _: &'tcx [rustc_ast::Attribute]) {} + + fn exit_lint_attrs(&mut self, _: &LateContext<'tcx>, _: &'tcx [rustc_ast::Attribute]) {} +} +// +// TODO: Register the lint pass in `clippy_lints/src/lib.rs`, +// e.g. store.register_late_pass(|| +// Box::new(trailing_zero_sized_array_without_repr_c::TrailingZeroSizedArrayWithoutReprC)); + + +fn temp_alert() { + span_lint_and_sugg(cx, lint, sp, msg, help, sugg, applicability) +} \ No newline at end of file diff --git a/tests/ui/trailing_zero_sized_array_without_repr_c.rs b/tests/ui/trailing_zero_sized_array_without_repr_c.rs new file mode 100644 index 00000000000..2a0f432bc2d --- /dev/null +++ b/tests/ui/trailing_zero_sized_array_without_repr_c.rs @@ -0,0 +1,23 @@ +#![warn(clippy::trailing_zero_sized_array_without_repr_c)] + +struct RarelyUseful { + field: i32, + last: [SomeType; 0], +} + +#[repr(C)] +struct GoodReason { + field: i32, + last: [SomeType; 0], +} + +struct OnlyFieldIsZeroSizeArray { + first_and_last: [SomeType; 0], +} + +struct GenericArrayType { + field: i32, + last: [T; 0], +} + +fn main() {} From c69387a0d530dccb56ef11d4cd148f9c38904893 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Fri, 15 Oct 2021 00:13:42 -0700 Subject: [PATCH 02/37] Well it builds --- ...railing_zero_sized_array_without_repr_c.rs | 47 +++++++++++++++---- ...railing_zero_sized_array_without_repr_c.rs | 6 +-- 2 files changed, 41 insertions(+), 12 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 9aea22af274..507d9b40412 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,5 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; use rustc_hir::*; +use rustc_lint::{EarlyContext, EarlyLintPass}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; @@ -33,26 +34,54 @@ declare_clippy_lint! { declare_lint_pass!(TrailingZeroSizedArrayWithoutReprC => [TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C]); impl LateLintPass<'_> for TrailingZeroSizedArrayWithoutReprC { - fn check_struct_def(&mut self, _: &LateContext<'tcx>, _: &'tcx rustc_hir::VariantData<'tcx>) {} + fn check_struct_def(&mut self, cx: &LateContext<'tcx>, data: &'tcx rustc_hir::VariantData<'tcx>) { + dbg!("in check_struct_def"); + if_chain! { + if let Some(def) = data.fields().last(); + if let rustc_hir::TyKind::Array(ty, acost) = def.ty.kind; + then { + // is the AnonConst `0` + } + } - fn check_struct_def_post(&mut self, _: &LateContext<'tcx>, _: &'tcx rustc_hir::VariantData<'tcx>) {} + // span_lint_and_sugg( + // cx, + // todo!(), + // todo!(), + // todo!(), + // todo!(), + // todo!(), + // rustc_errors::Applicability::MaybeIncorrect, + // ) + } // https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/sty/enum.TyKind.html#variant.Array in latepass // or https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/ast/enum.TyKind.html#variant.Array in early pass - fn check_field_def(&mut self, _: &LateContext<'tcx>, _: &'tcx rustc_hir::FieldDef<'tcx>) {} + // fn check_struct_def_post(&mut self, _: &LateContext<'tcx>, _: &'tcx rustc_hir::VariantData<'tcx>) + // {} - fn check_attribute(&mut self, _: &LateContext<'tcx>, _: &'tcx rustc_ast::Attribute) {} + // fn check_field_def(&mut self, _: &LateContext<'tcx>, _: &'tcx rustc_hir::FieldDef<'tcx>) {} - fn enter_lint_attrs(&mut self, _: &LateContext<'tcx>, _: &'tcx [rustc_ast::Attribute]) {} + // fn check_attribute(&mut self, _: &LateContext<'tcx>, _: &'tcx rustc_ast::Attribute) {} - fn exit_lint_attrs(&mut self, _: &LateContext<'tcx>, _: &'tcx [rustc_ast::Attribute]) {} + // fn enter_lint_attrs(&mut self, _: &LateContext<'tcx>, _: &'tcx [rustc_ast::Attribute]) {} + + // fn exit_lint_attrs(&mut self, _: &LateContext<'tcx>, _: &'tcx [rustc_ast::Attribute]) {} } // // TODO: Register the lint pass in `clippy_lints/src/lib.rs`, // e.g. store.register_late_pass(|| // Box::new(trailing_zero_sized_array_without_repr_c::TrailingZeroSizedArrayWithoutReprC)); +// fn temp_alert() {} -fn temp_alert() { - span_lint_and_sugg(cx, lint, sp, msg, help, sugg, applicability) -} \ No newline at end of file +impl EarlyLintPass for TrailingZeroSizedArrayWithoutReprC { + fn check_struct_def(&mut self, cx: &EarlyContext<'_>, data: &rustc_ast::VariantData) { + if_chain! { + if let rustc_ast::ast::VariantData::Struct(field_defs, some_bool_huh) = data; + if let Some(last_field) = field_defs.last(); + if let rustc_ast::ast::TyKind::Array(_, aconst) = &last_field.ty.kind; + then {dbg!(aconst); return ();} + } + } +} 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 2a0f432bc2d..771622178e7 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr_c.rs +++ b/tests/ui/trailing_zero_sized_array_without_repr_c.rs @@ -2,17 +2,17 @@ struct RarelyUseful { field: i32, - last: [SomeType; 0], + last: [usize; 0], } #[repr(C)] struct GoodReason { field: i32, - last: [SomeType; 0], + last: [usize; 0], } struct OnlyFieldIsZeroSizeArray { - first_and_last: [SomeType; 0], + first_and_last: [usize; 0], } struct GenericArrayType { From 92d3b775bd05c363b69b91ed66c20904c7fbfbe7 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Fri, 15 Oct 2021 01:31:26 -0700 Subject: [PATCH 03/37] =?UTF-8?q?ayy=20it=20compiles!=20ship=20it,=20right?= =?UTF-8?q?=3F=20=F0=9F=98=8E=20/s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit why was `rustc_lint_defs` not already externed in `lib.rs`? and how was r-a able to find it but cargo wasn't? 🤔 --- clippy_lints/src/lib.rs | 2 + ...railing_zero_sized_array_without_repr_c.rs | 76 ++++++++----------- 2 files changed, 33 insertions(+), 45 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 17bffc08309..6624ee93101 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -30,6 +30,7 @@ extern crate rustc_index; extern crate rustc_infer; extern crate rustc_lexer; extern crate rustc_lint; +extern crate rustc_lint_defs; extern crate rustc_middle; extern crate rustc_mir_dataflow; extern crate rustc_parse; @@ -487,6 +488,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(utils::internal_lints::OuterExpnDataPass)); } + store.register_early_pass(|| Box::new(trailing_zero_sized_array_without_repr_c::TrailingZeroSizedArrayWithoutReprC)); store.register_late_pass(|| Box::new(utils::author::Author)); store.register_late_pass(|| Box::new(await_holding_invalid::AwaitHolding)); store.register_late_pass(|| Box::new(serde_api::SerdeApi)); 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 507d9b40412..a96b8792295 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,7 +1,6 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use rustc_hir::*; use rustc_lint::{EarlyContext, EarlyLintPass}; -use rustc_lint::{LateContext, LateLintPass}; +use rustc_lint_defs::Applicability; use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { @@ -33,55 +32,42 @@ declare_clippy_lint! { } declare_lint_pass!(TrailingZeroSizedArrayWithoutReprC => [TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C]); -impl LateLintPass<'_> for TrailingZeroSizedArrayWithoutReprC { - fn check_struct_def(&mut self, cx: &LateContext<'tcx>, data: &'tcx rustc_hir::VariantData<'tcx>) { - dbg!("in check_struct_def"); - if_chain! { - if let Some(def) = data.fields().last(); - if let rustc_hir::TyKind::Array(ty, acost) = def.ty.kind; - then { - // is the AnonConst `0` - } - } - - // span_lint_and_sugg( - // cx, - // todo!(), - // todo!(), - // todo!(), - // todo!(), - // todo!(), - // rustc_errors::Applicability::MaybeIncorrect, - // ) - } - // https://doc.rust-lang.org/nightly/nightly-rustc/rustc_middle/ty/sty/enum.TyKind.html#variant.Array in latepass - // or https://doc.rust-lang.org/nightly/nightly-rustc/rustc_ast/ast/enum.TyKind.html#variant.Array in early pass - - // fn check_struct_def_post(&mut self, _: &LateContext<'tcx>, _: &'tcx rustc_hir::VariantData<'tcx>) - // {} - - // fn check_field_def(&mut self, _: &LateContext<'tcx>, _: &'tcx rustc_hir::FieldDef<'tcx>) {} - - // fn check_attribute(&mut self, _: &LateContext<'tcx>, _: &'tcx rustc_ast::Attribute) {} - - // fn enter_lint_attrs(&mut self, _: &LateContext<'tcx>, _: &'tcx [rustc_ast::Attribute]) {} - - // fn exit_lint_attrs(&mut self, _: &LateContext<'tcx>, _: &'tcx [rustc_ast::Attribute]) {} -} // // TODO: Register the lint pass in `clippy_lints/src/lib.rs`, -// e.g. store.register_late_pass(|| +// e.g. store.register_early_pass(|| // Box::new(trailing_zero_sized_array_without_repr_c::TrailingZeroSizedArrayWithoutReprC)); -// fn temp_alert() {} - impl EarlyLintPass for TrailingZeroSizedArrayWithoutReprC { fn check_struct_def(&mut self, cx: &EarlyContext<'_>, data: &rustc_ast::VariantData) { - if_chain! { - if let rustc_ast::ast::VariantData::Struct(field_defs, some_bool_huh) = data; - if let Some(last_field) = field_defs.last(); - if let rustc_ast::ast::TyKind::Array(_, aconst) = &last_field.ty.kind; - then {dbg!(aconst); return ();} + if is_struct_with_trailing_zero_sized_array(cx, data) && !has_repr_c(cx, data) { + span_lint_and_sugg( + cx, + todo!(), + todo!(), + todo!(), + "try", + "`#[repr(C)]`".to_string(), + Applicability::MachineApplicable, + ) } } } + +fn is_struct_with_trailing_zero_sized_array(cx: &EarlyContext<'_>, data: &rustc_ast::VariantData) -> bool { + if_chain! { + if let rustc_ast::ast::VariantData::Struct(field_defs, some_bool_huh) = data; + if let Some(last_field) = field_defs.last(); + if let rustc_ast::ast::TyKind::Array(_, aconst) = &last_field.ty.kind; + // TODO: if array is zero-sized; + then { + dbg!(aconst); + true + } else { + false + } + } +} + +fn has_repr_c(cx: &EarlyContext<'_>, data: &rustc_ast::VariantData) -> bool { + todo!() +} From 7ee8e7a9b846686148b8797ecc5ae724a6c5a1a9 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Fri, 15 Oct 2021 16:16:27 -0700 Subject: [PATCH 04/37] Implement detecting trailing zero-sized array --- clippy_lints/src/lib.rs | 4 +- ...railing_zero_sized_array_without_repr_c.rs | 62 ++++++++++++------- ...railing_zero_sized_array_without_repr_c.rs | 26 ++++++++ 3 files changed, 69 insertions(+), 23 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 6624ee93101..d494892c3b4 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -30,7 +30,6 @@ extern crate rustc_index; extern crate rustc_infer; extern crate rustc_lexer; extern crate rustc_lint; -extern crate rustc_lint_defs; extern crate rustc_middle; extern crate rustc_mir_dataflow; extern crate rustc_parse; @@ -488,7 +487,6 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(|| Box::new(utils::internal_lints::OuterExpnDataPass)); } - store.register_early_pass(|| Box::new(trailing_zero_sized_array_without_repr_c::TrailingZeroSizedArrayWithoutReprC)); store.register_late_pass(|| Box::new(utils::author::Author)); store.register_late_pass(|| Box::new(await_holding_invalid::AwaitHolding)); store.register_late_pass(|| Box::new(serde_api::SerdeApi)); @@ -780,6 +778,8 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || Box::new(undocumented_unsafe_blocks::UndocumentedUnsafeBlocks::default())); store.register_late_pass(|| Box::new(match_str_case_mismatch::MatchStrCaseMismatch)); store.register_late_pass(move || Box::new(format_args::FormatArgs)); + store.register_late_pass(|| Box::new(trailing_zero_sized_array_without_repr_c::TrailingZeroSizedArrayWithoutReprC)); + } #[rustfmt::skip] 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 a96b8792295..6ca382d1679 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,6 +1,9 @@ use clippy_utils::diagnostics::span_lint_and_sugg; -use rustc_lint::{EarlyContext, EarlyLintPass}; -use rustc_lint_defs::Applicability; +// use clippy_utils::is_integer_const; +use clippy_utils::consts::{miri_to_const, Constant}; +use rustc_errors::Applicability; +use rustc_hir::{Item, ItemKind, TyKind, VariantData}; +use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { @@ -36,38 +39,55 @@ declare_lint_pass!(TrailingZeroSizedArrayWithoutReprC => [TRAILING_ZERO_SIZED_AR // 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! -impl EarlyLintPass for TrailingZeroSizedArrayWithoutReprC { - fn check_struct_def(&mut self, cx: &EarlyContext<'_>, data: &rustc_ast::VariantData) { - if is_struct_with_trailing_zero_sized_array(cx, data) && !has_repr_c(cx, data) { - span_lint_and_sugg( - cx, - todo!(), - todo!(), - todo!(), - "try", - "`#[repr(C)]`".to_string(), - Applicability::MachineApplicable, - ) +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) */ + { + // span_lint_and_sugg( + // cx, + // todo!(), + // todo!(), + // todo!(), + // "try", + // "`#[repr(C)]`".to_string(), + // Applicability::MachineApplicable, + // ); + // println!("consider yourself linted 😎"); } } } -fn is_struct_with_trailing_zero_sized_array(cx: &EarlyContext<'_>, data: &rustc_ast::VariantData) -> bool { +fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) -> bool { + dbg!(item.ident); if_chain! { - if let rustc_ast::ast::VariantData::Struct(field_defs, some_bool_huh) = data; + 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 rustc_ast::ast::TyKind::Array(_, aconst) = &last_field.ty.kind; - // TODO: if array is zero-sized; + if let 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 + .const_eval_poly(aconst_def_id) // NOTE: maybe const_eval_resolve? seems especially cursed to be using a const expr which resolves to 0 to create a zero-sized array, tho + .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 { - dbg!(aconst); + eprintln!("true"); true } else { + // dbg!(aconst); + eprintln!("false"); false } } } -fn has_repr_c(cx: &EarlyContext<'_>, data: &rustc_ast::VariantData) -> bool { - todo!() +fn has_repr_c(cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) -> bool { + // todo!() + true } 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 771622178e7..5f844f16ba1 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr_c.rs +++ b/tests/ui/trailing_zero_sized_array_without_repr_c.rs @@ -20,4 +20,30 @@ struct GenericArrayType { last: [T; 0], } +struct SizedArray { + field: i32, + last: [usize; 1], +} + +const ZERO: usize = 0; +struct ZeroSizedFromExternalConst { + field: i32, + last: [usize; ZERO], +} + +const ONE: usize = 1; +struct NonZeroSizedFromExternalConst { + field: i32, + last: [usize; ONE], +} + +#[allow(clippy::eq_op)] // lmao im impressed +const fn compute_zero() -> usize { + (4 + 6) - (2 * 5) +} +struct UsingFunction { + field: i32, + last: [usize; compute_zero()], +} + fn main() {} From 523b0131618d8e4437a788eb1e3ddca743b3789f Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Fri, 15 Oct 2021 23:44:39 -0700 Subject: [PATCH 05/37] Implement getting an array of attributes! --- ...railing_zero_sized_array_without_repr_c.rs | 73 ++++++++++++------- 1 file changed, 46 insertions(+), 27 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 6ca382d1679..063dfb874cf 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 @@ -5,6 +5,7 @@ use rustc_errors::Applicability; use rustc_hir::{Item, ItemKind, TyKind, VariantData}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::sym; declare_clippy_lint! { /// ### What it does @@ -43,9 +44,8 @@ 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>) { - if is_struct_with_trailing_zero_sized_array(cx, item) - /* && !has_repr_c(cx, item) */ - { + dbg!(item.ident); + if is_struct_with_trailing_zero_sized_array(cx, item) && !has_repr_c(cx, item) { // span_lint_and_sugg( // cx, // todo!(), @@ -61,33 +61,52 @@ impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutReprC { } fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) -> bool { - dbg!(item.ident); - if_chain! { - 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; - 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 - .const_eval_poly(aconst_def_id) // NOTE: maybe const_eval_resolve? seems especially cursed to be using a const expr which resolves to 0 to create a zero-sized array, tho - .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 { - eprintln!("true"); - true - } else { - // dbg!(aconst); - eprintln!("false"); - false + 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 { + 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? seems especially cursed to be using a const expr which + // resolves to 0 to create a zero-sized array, tho + .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 { + eprintln!("trailing: true"); + return true; + } + } + } + } } } + // dbg!(aconst); + eprintln!("trailing: false"); + false } fn has_repr_c(cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) -> bool { - // todo!() - true + // let hir_id2 = if let Some(body) = cx.enclosing_body { + // body.hir_id + // } else { + // todo!(); + // }; + + let hir_id = cx.tcx.hir().local_def_id_to_hir_id(item.def_id); + let attrs = cx.tcx.hir().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(repr_attr) = attrs.iter().find(|attr| attr.has_name(sym::repr)) { + eprintln!("repr: true"); + true + } else { + eprintln!("repr: false"); + false + } } From e53a4da4a142d3e90f8eb658a19466eca54901ea Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Sat, 16 Oct 2021 00:51:09 -0700 Subject: [PATCH 06/37] it works i think (incl some `dbg`s) --- ...railing_zero_sized_array_without_repr_c.rs | 53 +++++++------ ...railing_zero_sized_array_without_repr_c.rs | 79 +++++++++++++------ 2 files changed, 82 insertions(+), 50 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 063dfb874cf..62f8ecdc216 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 @@ -2,7 +2,7 @@ use clippy_utils::diagnostics::span_lint_and_sugg; // use clippy_utils::is_integer_const; use clippy_utils::consts::{miri_to_const, Constant}; use rustc_errors::Applicability; -use rustc_hir::{Item, ItemKind, TyKind, VariantData}; +use rustc_hir::{HirId, Item, ItemKind, TyKind, VariantData}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; @@ -45,17 +45,29 @@ 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_c(cx, item) { - // span_lint_and_sugg( - // cx, - // todo!(), - // todo!(), - // todo!(), - // "try", - // "`#[repr(C)]`".to_string(), - // Applicability::MachineApplicable, - // ); - // println!("consider yourself linted 😎"); + + let hir_id = cx.tcx.hir().local_def_id_to_hir_id(item.def_id); + let hir_id2 = item.hir_id(); + dbg!(hir_id); + dbg!(hir_id2); + dbg!(hir_id == hir_id2); + + let span1 = cx.tcx.hir().span(hir_id); + let span2 = item.span; + dbg!(span1); + dbg!(span2); + dbg!(span1 == span2); + + if is_struct_with_trailing_zero_sized_array(cx, item) && !has_repr_c(cx, hir_id) { + span_lint_and_sugg( + cx, + TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C, + span2, + "trailing zero-sized array in a struct which isn't marked `#[repr(C)]`", + "try", + "#[repr(C)]".to_string(), + Applicability::MaybeIncorrect, + ); } } } @@ -76,7 +88,7 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx .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 { - eprintln!("trailing: true"); + // eprintln!("trailing: true"); return true; } } @@ -85,28 +97,21 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx } } // dbg!(aconst); - eprintln!("trailing: false"); + // eprintln!("trailing: false"); false } -fn has_repr_c(cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) -> bool { - // let hir_id2 = if let Some(body) = cx.enclosing_body { - // body.hir_id - // } else { - // todo!(); - // }; - - let hir_id = cx.tcx.hir().local_def_id_to_hir_id(item.def_id); +fn has_repr_c(cx: &LateContext<'tcx>, hir_id: HirId) -> bool { let attrs = cx.tcx.hir().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(repr_attr) = attrs.iter().find(|attr| attr.has_name(sym::repr)) { - eprintln!("repr: true"); + // eprintln!("repr: true"); true } else { - eprintln!("repr: false"); + // eprintln!("repr: false"); false } } 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 5f844f16ba1..1e3683b2c25 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr_c.rs +++ b/tests/ui/trailing_zero_sized_array_without_repr_c.rs @@ -15,35 +15,62 @@ struct OnlyFieldIsZeroSizeArray { first_and_last: [usize; 0], } -struct GenericArrayType { - field: i32, - last: [T; 0], -} +// struct GenericArrayType { +// field: i32, +// last: [T; 0], +// } -struct SizedArray { - field: i32, - last: [usize; 1], -} +// struct SizedArray { +// field: i32, +// last: [usize; 1], +// } -const ZERO: usize = 0; -struct ZeroSizedFromExternalConst { - field: i32, - last: [usize; ZERO], -} +// const ZERO: usize = 0; +// struct ZeroSizedFromExternalConst { +// field: i32, +// last: [usize; ZERO], +// } -const ONE: usize = 1; -struct NonZeroSizedFromExternalConst { - field: i32, - last: [usize; ONE], -} +// const ONE: usize = 1; +// struct NonZeroSizedFromExternalConst { +// field: i32, +// last: [usize; ONE], +// } -#[allow(clippy::eq_op)] // lmao im impressed -const fn compute_zero() -> usize { - (4 + 6) - (2 * 5) -} -struct UsingFunction { - field: i32, - last: [usize; compute_zero()], -} +// #[allow(clippy::eq_op)] // lmao im impressed +// const fn compute_zero() -> usize { +// (4 + 6) - (2 * 5) +// } +// struct UsingFunction { +// field: i32, +// last: [usize; compute_zero()], +// } + +// // TODO: same +// #[repr(packed)] +// struct ReprPacked { +// small: u8, +// medium: i32, +// weird: [u64; 0], +// } + +// // TODO: actually, uh,, +// #[repr(align(64))] +// struct ReprAlign { +// field: i32, +// last: [usize; 0], +// } +// #[repr(C, align(64))] +// struct ReprCAlign { +// field: i32, +// last: [usize; 0], +// } + +// #[repr(C)] +// enum DontLintAnonymousStructsFromDesuraging { +// A(u32), +// B(f32, [u64; 0]), +// C { x: u32, y: [u64; 0] }, +// } fn main() {} From 4b4db597722ff561515812e2e0cab255a678a41c Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Sat, 16 Oct 2021 02:01:17 -0700 Subject: [PATCH 07/37] output looks fantastic --- ...railing_zero_sized_array_without_repr_c.rs | 34 ++--- ...railing_zero_sized_array_without_repr_c.rs | 139 +++++++++++------- 2 files changed, 100 insertions(+), 73 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 62f8ecdc216..7eeb2914c40 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,8 +1,9 @@ -use clippy_utils::diagnostics::span_lint_and_sugg; -// use clippy_utils::is_integer_const; use clippy_utils::consts::{miri_to_const, Constant}; +use clippy_utils::diagnostics::span_lint_and_sugg; +use clippy_utils::source::snippet; use rustc_errors::Applicability; -use rustc_hir::{HirId, Item, ItemKind, TyKind, VariantData}; +use rustc_hir::def_id::LocalDefId; +use rustc_hir::{Item, ItemKind, TyKind, VariantData}; use rustc_lint::{LateContext, LateLintPass}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; @@ -46,26 +47,14 @@ impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutReprC { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { dbg!(item.ident); - let hir_id = cx.tcx.hir().local_def_id_to_hir_id(item.def_id); - let hir_id2 = item.hir_id(); - dbg!(hir_id); - dbg!(hir_id2); - dbg!(hir_id == hir_id2); - - let span1 = cx.tcx.hir().span(hir_id); - let span2 = item.span; - dbg!(span1); - dbg!(span2); - dbg!(span1 == span2); - - if is_struct_with_trailing_zero_sized_array(cx, item) && !has_repr_c(cx, hir_id) { + if is_struct_with_trailing_zero_sized_array(cx, item) && !has_repr_c(cx, item.def_id) { span_lint_and_sugg( cx, TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C, - span2, - "trailing zero-sized array in a struct which isn't marked `#[repr(C)]`", + item.span, + "trailing zero-sized array in a struct which is not marked `#[repr(C)]`", "try", - "#[repr(C)]".to_string(), + format!("#[repr(C)]\n{}", snippet(cx, item.span, "..")), Applicability::MaybeIncorrect, ); } @@ -101,13 +90,14 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx false } -fn has_repr_c(cx: &LateContext<'tcx>, hir_id: HirId) -> bool { +fn has_repr_c(cx: &LateContext<'tcx>, def_id: LocalDefId) -> bool { + let hir_id = cx.tcx.hir().local_def_id_to_hir_id(def_id); let attrs = cx.tcx.hir().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(repr_attr) = attrs.iter().find(|attr| attr.has_name(sym::repr)) { + if let Some(_repr_attr) = attrs.iter().find(|attr| attr.has_name(sym::repr)) { // eprintln!("repr: true"); true } else { 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 1e3683b2c25..311193fb4a1 100644 --- 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,7 @@ #![warn(clippy::trailing_zero_sized_array_without_repr_c)] +// #![feature(const_generics_defaults)] + struct RarelyUseful { field: i32, last: [usize; 0], @@ -15,62 +17,97 @@ struct OnlyFieldIsZeroSizeArray { first_and_last: [usize; 0], } -// struct GenericArrayType { -// field: i32, -// last: [T; 0], -// } +struct GenericArrayType { + field: i32, + last: [T; 0], +} -// struct SizedArray { -// field: i32, -// last: [usize; 1], -// } +struct SizedArray { + field: i32, + last: [usize; 1], +} -// const ZERO: usize = 0; -// struct ZeroSizedFromExternalConst { -// field: i32, -// last: [usize; ZERO], -// } +const ZERO: usize = 0; +struct ZeroSizedFromExternalConst { + field: i32, + last: [usize; ZERO], +} -// const ONE: usize = 1; -// struct NonZeroSizedFromExternalConst { -// field: i32, -// last: [usize; ONE], -// } +const ONE: usize = 1; +struct NonZeroSizedFromExternalConst { + field: i32, + last: [usize; ONE], +} -// #[allow(clippy::eq_op)] // lmao im impressed -// const fn compute_zero() -> usize { -// (4 + 6) - (2 * 5) -// } -// struct UsingFunction { -// field: i32, -// last: [usize; compute_zero()], -// } - -// // TODO: same -// #[repr(packed)] -// struct ReprPacked { -// small: u8, -// medium: i32, -// weird: [u64; 0], -// } - -// // TODO: actually, uh,, -// #[repr(align(64))] -// struct ReprAlign { -// field: i32, -// last: [usize; 0], -// } -// #[repr(C, align(64))] -// struct ReprCAlign { -// field: i32, -// last: [usize; 0], -// } +#[allow(clippy::eq_op)] // lmao im impressed +const fn compute_zero() -> usize { + (4 + 6) - (2 * 5) +} +struct UsingFunction { + field: i32, + last: [usize; compute_zero()], +} // #[repr(C)] -// enum DontLintAnonymousStructsFromDesuraging { -// A(u32), -// B(f32, [u64; 0]), -// C { x: u32, y: [u64; 0] }, +// struct ConstParamOk { +// field: i32, +// last: [usize; N] // } -fn main() {} +// struct ConstParamLint { +// field: i32, +// last: [usize; N] +// } + + +// TODO: actually, uh,, +#[repr(packed)] +struct ReprPacked { + small: u8, + medium: i32, + weird: [u64; 0], +} + +// same +#[repr(align(64))] +struct ReprAlign { + field: i32, + last: [usize; 0], +} + +// same +#[repr(C, align(64))] +struct ReprCAlign { + field: i32, + last: [usize; 0], +} + +#[repr(C)] +enum DontLintAnonymousStructsFromDesuraging { + A(u32), + B(f32, [u64; 0]), + 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], +} + +fn main() { +} From b9948c4be6741ae916b838df29e9777467d4c84c Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Sat, 16 Oct 2021 02:26:08 -0700 Subject: [PATCH 08/37] Ran `dev bless`! --- ...railing_zero_sized_array_without_repr_c.rs | 58 ++++----- ...railing_zero_sized_array_without_repr_c.rs | 17 +-- ...ing_zero_sized_array_without_repr_c.stderr | 113 ++++++++++++++++++ 3 files changed, 147 insertions(+), 41 deletions(-) create mode 100644 tests/ui/trailing_zero_sized_array_without_repr_c.stderr 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 7eeb2914c40..7f579835512 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 @@ -26,7 +26,7 @@ declare_clippy_lint! { /// Use instead: /// ```rust /// #[repr(C)] - /// struct MakesSense { + /// struct MoreOftenUseful { /// some_field: usize, /// last: [SomeType; 0], /// } @@ -45,15 +45,13 @@ 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_c(cx, item.def_id) { span_lint_and_sugg( cx, TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C, item.span, "trailing zero-sized array in a struct which is not marked `#[repr(C)]`", - "try", + "try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute):", format!("#[repr(C)]\n{}", snippet(cx, item.span, "..")), Applicability::MaybeIncorrect, ); @@ -62,46 +60,40 @@ impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutReprC { } fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) -> bool { - 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 { - 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? seems especially cursed to be using a const expr which - // resolves to 0 to create a zero-sized array, tho - .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 { - // eprintln!("trailing: true"); - return true; - } - } - } - } + if_chain! { + 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; + 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 } } - // dbg!(aconst); - // eprintln!("trailing: false"); - false } fn has_repr_c(cx: &LateContext<'tcx>, def_id: LocalDefId) -> bool { - let hir_id = cx.tcx.hir().local_def_id_to_hir_id(def_id); - let attrs = cx.tcx.hir().attrs(hir_id); + 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); // 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(_repr_attr) = attrs.iter().find(|attr| attr.has_name(sym::repr)) { - // eprintln!("repr: true"); + if let Some(_attr) = attrs.iter().find(|attr| attr.has_name(sym::repr)) { true } else { - // eprintln!("repr: false"); false } } 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 311193fb4a1..62fe94d7abf 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr_c.rs +++ b/tests/ui/trailing_zero_sized_array_without_repr_c.rs @@ -1,6 +1,5 @@ #![warn(clippy::trailing_zero_sized_array_without_repr_c)] - -// #![feature(const_generics_defaults)] +// #![feature(const_generics_defaults)] // see below struct RarelyUseful { field: i32, @@ -48,6 +47,9 @@ 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, @@ -59,8 +61,7 @@ struct UsingFunction { // last: [usize; N] // } - -// TODO: actually, uh,, +// TODO: actually, uh,, no idea what behavior here would be #[repr(packed)] struct ReprPacked { small: u8, @@ -68,20 +69,21 @@ struct ReprPacked { weird: [u64; 0], } -// same +// TODO: clarify expected behavior #[repr(align(64))] struct ReprAlign { field: i32, last: [usize; 0], } -// same +// 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), @@ -109,5 +111,4 @@ struct LotsOfFields { last: [usize; 0], } -fn main() { -} +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 new file mode 100644 index 00000000000..84606ed6185 --- /dev/null +++ b/tests/ui/trailing_zero_sized_array_without_repr_c.stderr @@ -0,0 +1,113 @@ +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 + | +LL | / struct RarelyUseful { +LL | | field: i32, +LL | | last: [usize; 0], +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 + } + | + +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 + | +LL | / struct OnlyFieldIsZeroSizeArray { +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 + } + | + +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 | / struct GenericArrayType { +LL | | field: i32, +LL | | last: [T; 0], +LL | | } + | |_^ + | +help: try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute): + | +LL + #[repr(C)] +LL + struct GenericArrayType { +LL + field: i32, +LL + last: [T; 0], +LL + } + | + +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 + | +LL | / struct ZeroSizedFromExternalConst { +LL | | field: i32, +LL | | last: [usize; ZERO], +LL | | } + | |_^ + | +help: try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute): + | +LL + #[repr(C)] +LL + struct ZeroSizedFromExternalConst { +LL + field: i32, +LL + last: [usize; ZERO], +LL + } + | + +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 + | +LL | / struct UsingFunction { +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 + } + | + +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 + | +LL | / struct LotsOfFields { +LL | | f1: u32, +LL | | f2: u32, +LL | | f3: u32, +... | +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, + ... + +error: aborting due to 6 previous errors + From 003972f4281ab83fb56ce9a898efd93b6ca3740e Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Sat, 16 Oct 2021 15:26:10 -0700 Subject: [PATCH 09/37] add multiple `get_attrs` and `includes_repr` and they all work! --- clippy_lints/src/lib.rs | 1 + ...railing_zero_sized_array_without_repr_c.rs | 66 +++++++-- ...railing_zero_sized_array_without_repr_c.rs | 139 ++++++++++-------- 3 files changed, 130 insertions(+), 76 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,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: []}; +} From 5fdf93415bfd59a7cd61ebec20f0a4e4fec78164 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Sat, 16 Oct 2021 16:13:14 -0700 Subject: [PATCH 10/37] intermediate step --- ...railing_zero_sized_array_without_repr_c.rs | 45 ++++++++++--------- ...railing_zero_sized_array_without_repr_c.rs | 14 +++--- 2 files changed, 31 insertions(+), 28 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 4c3c5191d28..a9c6e24918e 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,8 +1,6 @@ use clippy_utils::consts::{miri_to_const, Constant}; -use clippy_utils::diagnostics::span_lint_and_sugg; -use clippy_utils::source::snippet; +use clippy_utils::diagnostics::span_lint_and_help; use rustc_ast::Attribute; -use rustc_errors::Applicability; use rustc_hir::def_id::LocalDefId; use rustc_hir::{Item, ItemKind, VariantData}; use rustc_lint::{LateContext, LateLintPass}; @@ -47,15 +45,15 @@ 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_sugg( - cx, - TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C, - item.span, - "trailing zero-sized array in a struct which is not marked `#[repr(C)]`", - "try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute):", - format!("#[repr(C)]\n{}", snippet(cx, item.span, "..")), - Applicability::MaybeIncorrect, - ); + // 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 — 🦀") } } } @@ -87,15 +85,16 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx 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; + + 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); - b11 + b21 } fn get_attrs_get_attrs(cx: &LateContext<'tcx>, def_id: LocalDefId) -> &'tcx [Attribute] { @@ -108,7 +107,9 @@ fn get_attrs_hir_map(cx: &LateContext<'tcx>, def_id: LocalDefId) -> &'tcx [Attri 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 +// 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() { @@ -129,5 +130,7 @@ fn includes_repr_attr_using_sym(attrs: &'tcx [Attribute]) -> bool { } 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()) + 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 8e8c84fe9c5..07cba5774a5 100644 --- 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,5 @@ #![warn(clippy::trailing_zero_sized_array_without_repr_c)] -// #![feature(const_generics_defaults)] // see below +#![feature(const_generics_defaults)] // see below // Do lint: @@ -20,13 +20,13 @@ struct GenericArrayType { #[derive(Debug)] struct PlayNiceWithOtherAttributesDerive { field: i32, - last: [usize; 0] + last: [usize; 0], } #[must_use] struct PlayNiceWithOtherAttributesMustUse { field: i32, - last: [usize; 0] + last: [usize; 0], } const ZERO: usize = 0; @@ -72,7 +72,7 @@ struct GoodReason { last: [usize; 0], } -struct SizedArray { +struct NonZeroSizedArray { field: i32, last: [usize; 1], } @@ -114,8 +114,8 @@ enum DontLintAnonymousStructsFromDesuraging { 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. +// 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 { @@ -129,5 +129,5 @@ enum DontLintAnonymousStructsFromDesuraging { // } fn main() { - let _ = PlayNiceWithOtherAttributesMustUse {field: 0, last: []}; + let _ = PlayNiceWithOtherAttributesMustUse { field: 0, last: [] }; } From 9b3f55ee61e781ef3360ddfaa436746bb7e40df5 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Sat, 16 Oct 2021 17:49:13 -0700 Subject: [PATCH 11/37] tried to simplify but it doesn't work :/ --- ...railing_zero_sized_array_without_repr_c.rs | 109 ++++++------------ ...railing_zero_sized_array_without_repr_c.rs | 34 ++++-- 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,37 +40,44 @@ 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; + 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 { true } else { @@ -82,54 +86,9 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx } } -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 = 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); - - 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) - } 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 { } #[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: [] }; } From a3420f70043c19165a4e44639ef4e6f3c156a174 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Sat, 16 Oct 2021 22:03:08 -0700 Subject: [PATCH 12/37] Tidy comments + tests; revert 'size-is-zero' detection --- ...railing_zero_sized_array_without_repr_c.rs | 35 ++++--- ...railing_zero_sized_array_without_repr_c.rs | 36 +++---- ...ing_zero_sized_array_without_repr_c.stderr | 97 ++++++++----------- 3 files changed, 75 insertions(+), 93 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 913812126a9..bc055307d5e 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 @@ -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 index 6ab96c2ebf6..77b2c29b275 100644 --- 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 { 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 { -// field: i32, -// last: [usize; N] -// } - -// struct ConstParamLint { -// 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 index 84606ed6185..ee8182cdc38 100644 --- 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,33 +8,20 @@ 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 { LL | | field: i32, @@ -42,53 +29,55 @@ LL | | last: [T; 0], LL | | } | |_^ | -help: try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute): - | -LL + #[repr(C)] -LL + struct GenericArrayType { -LL + field: i32, -LL + last: [T; 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:19:1 | -LL | / struct ZeroSizedFromExternalConst { +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:28:1 + | +LL | / struct OnlyAnotherAttributeDerive { +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:34:1 + | +LL | / struct ZeroSizedWithConst { LL | | field: i32, LL | | last: [usize; ZERO], LL | | } | |_^ | -help: try annotating the struct definition with `#[repr(C)]` (or another `repr` attribute): - | -LL + #[repr(C)] -LL + struct ZeroSizedFromExternalConst { -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 From c5d3167a23e7f1f6515a28ff15a6698b6712ae54 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Sun, 17 Oct 2021 17:28:45 -0700 Subject: [PATCH 13/37] update testsuite and expand `if_chain` --- ...railing_zero_sized_array_without_repr_c.rs | 55 +++++++++++-------- ...railing_zero_sized_array_without_repr_c.rs | 32 +++++++++++ 2 files changed, 63 insertions(+), 24 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 bc055307d5e..de2513244da 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 @@ -64,35 +64,42 @@ impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutReprC { } fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) -> bool { - if_chain! { - // 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; + // 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 { + // Then check if that that array zero-sized - // 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 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 + // 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 { + length == 0 + } else { + false + } + } else { + false + } + } else { + false + } } else { false } + } else { + false } } 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 77b2c29b275..c6ee36e9685 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr_c.rs +++ b/tests/ui/trailing_zero_sized_array_without_repr_c.rs @@ -1,4 +1,5 @@ #![warn(clippy::trailing_zero_sized_array_without_repr_c)] +#![feature(const_generics_defaults)] // Do lint: @@ -45,6 +46,10 @@ struct ZeroSizedWithConstFunction { last: [usize; compute_zero()], } +struct ZeroSizedArrayWrapper([usize; 0]); + +struct TupleStruct(i32, [usize; 0]); + struct LotsOfFields { f1: u32, f2: u32, @@ -140,4 +145,31 @@ enum DontLintAnonymousStructsFromDesuraging { C { x: u32, y: [u64; 0] }, } +#[repr(C)] +struct TupleStructReprC(i32, [usize; 0]); + +type NamedTuple = (i32, [usize; 0]); + +#[rustfmt::skip] // [rustfmt#4995](https://github.com/rust-lang/rustfmt/issues/4995) +struct ConstParamZeroDefault { + field: i32, + last: [usize; N], +} + +struct ConstParamNoDefault { + field: i32, + last: [usize; N], +} + +#[rustfmt::skip] +struct ConstParamNonZeroDefault { + field: i32, + last: [usize; N], +} + +type A = ConstParamZeroDefault; +type B = ConstParamZeroDefault<0>; +type C = ConstParamNoDefault<0>; +type D = ConstParamNonZeroDefault<0>; + fn main() {} From 2a5a4f07cf34895cf6d3c6f8843169e3d6d9c46d Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 00:51:30 -0700 Subject: [PATCH 14/37] =?UTF-8?q?Refactor=20ZS=20array=20detection=20again?= =?UTF-8?q?=20and=20this=20one=20seems=20great=20=F0=9F=91=8D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- ...railing_zero_sized_array_without_repr_c.rs | 26 +++++++------------ 1 file changed, 9 insertions(+), 17 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 de2513244da..d68ad901704 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,4 +1,4 @@ -use clippy_utils::consts::{miri_to_const, Constant}; +use clippy_utils::consts::{constant, miri_to_const, ConstEvalLateContext, Constant}; use clippy_utils::diagnostics::span_lint_and_help; use rustc_ast::Attribute; use rustc_hir::{Item, ItemKind, VariantData}; @@ -38,9 +38,11 @@ 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) { // 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. + // is to not do that with struct definitions (I'm not sure), then this isn't necessary. (note: if + // you don't get rid of this, change `has_repr_attr` to `includes_repr_attr`). let attrs = cx.tcx.get_attrs(item.def_id.to_def_id()); let first_attr = attrs.iter().min_by_key(|attr| attr.span.lo()); let lint_span = if let Some(first_attr) = first_attr { @@ -70,21 +72,11 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx if let Some(last_field) = field_defs.last() { if let rustc_hir::TyKind::Array(_, length) = last_field.ty.kind { // 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 { + let length_ldid = cx.tcx.hir().local_def_id(length.hir_id); + let length = Const::from_anon_const(cx.tcx, length_ldid); + let length = length.try_eval_usize(cx.tcx, cx.param_env); + // if let Some((Constant::Int(length), _)) = length { + if let Some(length) = length { length == 0 } else { false From 9f402b370c43f8cbad15d477d29aa96ff9746de5 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 03:03:48 -0700 Subject: [PATCH 15/37] Check for tuple structs --- ...railing_zero_sized_array_without_repr_c.rs | 27 +++++++------------ 1 file changed, 10 insertions(+), 17 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 d68ad901704..9373551db15 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 @@ -38,7 +38,6 @@ 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) { // 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. (note: if @@ -66,24 +65,18 @@ impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutReprC { } fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) -> bool { + // TODO: when finalized, replace with an `if_chain`. I have it like this because my rust-analyzer doesn't work when it's an `if_chain` // 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 { - // Then 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); - let length = length.try_eval_usize(cx.tcx, cx.param_env); - // if let Some((Constant::Int(length), _)) = length { - if let Some(length) = length { - length == 0 - } else { - false - } - } else { - false - } + let field_defs = data.fields(); + if let Some(last_field) = field_defs.last() { + if let rustc_hir::TyKind::Array(_, length) = last_field.ty.kind { + // Then 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); + let length = length.try_eval_usize(cx.tcx, cx.param_env); + // if let Some((Constant::Int(length), _)) = length { + if let Some(length) = length { length == 0 } else { false } } else { false } From cd6862283ee603432f509510c53f35bda12e3a5a Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 03:04:50 -0700 Subject: [PATCH 16/37] Tidy imports --- .../src/trailing_zero_sized_array_without_repr_c.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 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 9373551db15..a759fff9cfc 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,7 +1,7 @@ -use clippy_utils::consts::{constant, miri_to_const, ConstEvalLateContext, Constant}; use clippy_utils::diagnostics::span_lint_and_help; use rustc_ast::Attribute; -use rustc_hir::{Item, ItemKind, VariantData}; +use rustc_hir::VariantData; +use rustc_hir::{Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::dep_graph::DepContext; use rustc_middle::ty::Const; @@ -65,8 +65,8 @@ impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutReprC { } fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) -> bool { - // TODO: when finalized, replace with an `if_chain`. I have it like this because my rust-analyzer doesn't work when it's an `if_chain` - // First check if last field is an array + // TODO: when finalized, replace with an `if_chain`. I have it like this because my rust-analyzer + // doesn't work when it's an `if_chain` First check if last field is an array if let ItemKind::Struct(data, _) = &item.kind { let field_defs = data.fields(); if let Some(last_field) = field_defs.last() { From 6377fb2fe71a8bde91a20fbadc93242e19fd0dee Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 03:13:48 -0700 Subject: [PATCH 17/37] Tidy import + update expected stderr --- ...railing_zero_sized_array_without_repr_c.rs | 3 +- ...ing_zero_sized_array_without_repr_c.stderr | 34 ++++++++++++++----- 2 files changed, 26 insertions(+), 11 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 a759fff9cfc..aea2fb208df 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,6 +1,5 @@ use clippy_utils::diagnostics::span_lint_and_help; use rustc_ast::Attribute; -use rustc_hir::VariantData; use rustc_hir::{Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::dep_graph::DepContext; @@ -90,7 +89,7 @@ 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 if you want to look (or I + // on all testcases (when i wrote this comment. I added a few since then).) Happy to use another; they're in the commit history if you want to look (or I // can go find them). attrs .iter() diff --git a/tests/ui/trailing_zero_sized_array_without_repr_c.stderr b/tests/ui/trailing_zero_sized_array_without_repr_c.stderr index ee8182cdc38..5ed91e937d5 100644 --- 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:5:1 + --> $DIR/trailing_zero_sized_array_without_repr_c.rs:6:1 | LL | / struct RarelyUseful { LL | | field: i32, @@ -11,7 +11,7 @@ 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:10:1 + --> $DIR/trailing_zero_sized_array_without_repr_c.rs:11:1 | LL | / struct OnlyField { LL | | first_and_last: [usize; 0], @@ -21,7 +21,7 @@ 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:14:1 + --> $DIR/trailing_zero_sized_array_without_repr_c.rs:15:1 | LL | / struct GenericArrayType { LL | | field: i32, @@ -32,7 +32,7 @@ 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:20:1 | LL | / #[must_use] LL | | struct OnlyAnotherAttributeMustUse { @@ -44,7 +44,7 @@ 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:28:1 + --> $DIR/trailing_zero_sized_array_without_repr_c.rs:29:1 | LL | / struct OnlyAnotherAttributeDerive { LL | | field: i32, @@ -55,7 +55,7 @@ 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:34:1 + --> $DIR/trailing_zero_sized_array_without_repr_c.rs:35:1 | LL | / struct ZeroSizedWithConst { LL | | field: i32, @@ -66,7 +66,7 @@ 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:43:1 + --> $DIR/trailing_zero_sized_array_without_repr_c.rs:44:1 | LL | / struct ZeroSizedWithConstFunction { LL | | field: i32, @@ -77,7 +77,23 @@ 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:48:1 + --> $DIR/trailing_zero_sized_array_without_repr_c.rs:49:1 + | +LL | struct ZeroSizedArrayWrapper([usize; 0]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = 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:51:1 + | +LL | struct TupleStruct(i32, [usize; 0]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = 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:53:1 | LL | / struct LotsOfFields { LL | | f1: u32, @@ -90,5 +106,5 @@ LL | | } | = help: consider annotating the struct definition with `#[repr(C)]` (or another `repr` attribute) -error: aborting due to 8 previous errors +error: aborting due to 10 previous errors From 149b3728732169cbd6684dfb5e17242924a506a7 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 03:16:10 -0700 Subject: [PATCH 18/37] run rustfmt --- clippy_lints/src/trailing_zero_sized_array_without_repr_c.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 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 aea2fb208df..6cba18146b1 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 @@ -89,8 +89,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 (when i wrote this comment. I added a few since then).) Happy to use another; they're in the commit history if you want to look (or I - // can go find them). + // on all testcases (when i wrote this comment. I added a few since then).) 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()) From 7f84e3d791846fb462eb39d05bbec029b68d97af Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 03:45:08 -0700 Subject: [PATCH 19/37] Rename lint --- CHANGELOG.md | 2 +- clippy_lints/src/lib.register_lints.rs | 2 +- clippy_lints/src/lib.register_nursery.rs | 2 +- clippy_lints/src/lib.rs | 4 ++-- ...trailing_zero_sized_array_without_repr.rs} | 22 +++++++++---------- ...trailing_zero_sized_array_without_repr.rs} | 2 +- 6 files changed, 17 insertions(+), 17 deletions(-) rename clippy_lints/src/{trailing_zero_sized_array_without_repr_c.rs => trailing_zero_sized_array_without_repr.rs} (81%) rename tests/ui/{trailing_zero_sized_array_without_repr_c.rs => trailing_zero_sized_array_without_repr.rs} (98%) diff --git a/CHANGELOG.md b/CHANGELOG.md index a124bebe924..0cf19a6f0f1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3021,7 +3021,7 @@ Released 2018-09-13 [`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments [`too_many_lines`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines [`toplevel_ref_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#toplevel_ref_arg -[`trailing_zero_sized_array_without_repr_c`]: https://rust-lang.github.io/rust-clippy/master/index.html#trailing_zero_sized_array_without_repr_c +[`trailing_zero_sized_array_without_repr`]: https://rust-lang.github.io/rust-clippy/master/index.html#trailing_zero_sized_array_without_repr [`trait_duplication_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#trait_duplication_in_bounds [`transmute_bytes_to_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_bytes_to_str [`transmute_float_to_int`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_float_to_int diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index 34b87002fa3..af0629beb36 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -443,7 +443,7 @@ store.register_lints(&[ temporary_assignment::TEMPORARY_ASSIGNMENT, to_digit_is_some::TO_DIGIT_IS_SOME, to_string_in_display::TO_STRING_IN_DISPLAY, - trailing_zero_sized_array_without_repr_c::TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C, + trailing_zero_sized_array_without_repr::TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS, trait_bounds::TYPE_REPETITION_IN_BOUNDS, transmute::CROSSPOINTER_TRANSMUTE, diff --git a/clippy_lints/src/lib.register_nursery.rs b/clippy_lints/src/lib.register_nursery.rs index 325706746db..eab0e5b90f1 100644 --- a/clippy_lints/src/lib.register_nursery.rs +++ b/clippy_lints/src/lib.register_nursery.rs @@ -25,7 +25,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![ LintId::of(regex::TRIVIAL_REGEX), LintId::of(strings::STRING_LIT_AS_BYTES), LintId::of(suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS), - LintId::of(trailing_zero_sized_array_without_repr_c::TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C), + LintId::of(trailing_zero_sized_array_without_repr::TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR), LintId::of(transmute::USELESS_TRANSMUTE), LintId::of(use_self::USE_SELF), ]) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 72636146d7c..1473a2f3095 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -356,7 +356,7 @@ mod tabs_in_doc_comments; mod temporary_assignment; mod to_digit_is_some; mod to_string_in_display; -mod trailing_zero_sized_array_without_repr_c; +mod trailing_zero_sized_array_without_repr; mod trait_bounds; mod transmute; mod transmuting_null; @@ -779,7 +779,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || Box::new(undocumented_unsafe_blocks::UndocumentedUnsafeBlocks::default())); store.register_late_pass(|| Box::new(match_str_case_mismatch::MatchStrCaseMismatch)); store.register_late_pass(move || Box::new(format_args::FormatArgs)); - store.register_late_pass(|| Box::new(trailing_zero_sized_array_without_repr_c::TrailingZeroSizedArrayWithoutReprC)); + store.register_late_pass(|| Box::new(trailing_zero_sized_array_without_repr::TrailingZeroSizedArrayWithoutRepr)); } diff --git a/clippy_lints/src/trailing_zero_sized_array_without_repr_c.rs b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs similarity index 81% rename from clippy_lints/src/trailing_zero_sized_array_without_repr_c.rs rename to clippy_lints/src/trailing_zero_sized_array_without_repr.rs index 6cba18146b1..0267f7bdf0e 100644 --- a/clippy_lints/src/trailing_zero_sized_array_without_repr_c.rs +++ b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs @@ -8,10 +8,10 @@ use rustc_session::{declare_lint_pass, declare_tool_lint}; declare_clippy_lint! { /// ### What it does - /// Displays a warning when a struct with a trailing zero-sized array is declared without the `repr(C)` attribute. + /// Displays a warning when a struct with a trailing zero-sized array is declared without a `repr` attribute. /// /// ### Why is this bad? - /// Zero-sized arrays aren't very useful in Rust itself, so such a struct is likely being created to pass to C code (or in conjuction with manual allocation to make it easy to compute the offset of the array). Either way, `#[repr(C)]` is needed. + /// Zero-sized arrays aren't very useful in Rust itself, so such a struct is likely being created to pass to C code or in some other situation where control over memory layout matters (for example, in conjuction with manual allocation to make it easy to compute the offset of the array). Either way, `#[repr(C)]` (or another `repr` attribute) is needed. /// /// ### Example /// ```rust @@ -29,13 +29,13 @@ declare_clippy_lint! { /// last: [SomeType; 0], /// } /// ``` - pub TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C, + pub TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, nursery, "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]); +declare_lint_pass!(TrailingZeroSizedArrayWithoutRepr => [TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR]); -impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutReprC { +impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutRepr { 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 @@ -52,7 +52,7 @@ impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutReprC { if !has_repr_attr(cx, attrs) { span_lint_and_help( cx, - TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR_C, + TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, lint_span, "trailing zero-sized array in a struct which is not marked `#[repr(C)]`", None, @@ -65,17 +65,17 @@ impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutReprC { fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) -> bool { // TODO: when finalized, replace with an `if_chain`. I have it like this because my rust-analyzer - // doesn't work when it's an `if_chain` First check if last field is an array + // doesn't work when it's an `if_chain`. + + // First check if last field is an array if let ItemKind::Struct(data, _) = &item.kind { - let field_defs = data.fields(); - if let Some(last_field) = field_defs.last() { + if let Some(last_field) = data.fields().last() { if let rustc_hir::TyKind::Array(_, length) = last_field.ty.kind { // Then 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); let length = length.try_eval_usize(cx.tcx, cx.param_env); - // if let Some((Constant::Int(length), _)) = length { - if let Some(length) = length { length == 0 } else { false } + length == Some(0) } else { false } diff --git a/tests/ui/trailing_zero_sized_array_without_repr_c.rs b/tests/ui/trailing_zero_sized_array_without_repr.rs similarity index 98% rename from tests/ui/trailing_zero_sized_array_without_repr_c.rs rename to tests/ui/trailing_zero_sized_array_without_repr.rs index c6ee36e9685..6ac124c8b34 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr_c.rs +++ b/tests/ui/trailing_zero_sized_array_without_repr.rs @@ -1,4 +1,4 @@ -#![warn(clippy::trailing_zero_sized_array_without_repr_c)] +#![warn(clippy::trailing_zero_sized_array_without_repr)] #![feature(const_generics_defaults)] // Do lint: From a6aa9864a339783e2e8400053408a197b607301d Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 03:52:37 -0700 Subject: [PATCH 20/37] Rename stderr --- ...iling_zero_sized_array_without_repr.stderr | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) create mode 100644 tests/ui/trailing_zero_sized_array_without_repr.stderr diff --git a/tests/ui/trailing_zero_sized_array_without_repr.stderr b/tests/ui/trailing_zero_sized_array_without_repr.stderr new file mode 100644 index 00000000000..54b36ec6cec --- /dev/null +++ b/tests/ui/trailing_zero_sized_array_without_repr.stderr @@ -0,0 +1,110 @@ +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_zero_sized_array_without_repr.rs:6:1 + | +LL | / struct RarelyUseful { +LL | | field: i32, +LL | | last: [usize; 0], +LL | | } + | |_^ + | + = note: `-D clippy::trailing-zero-sized-array-without-repr` implied by `-D warnings` + = 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 with a `repr` attribute + --> $DIR/trailing_zero_sized_array_without_repr.rs:11:1 + | +LL | / struct OnlyField { +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 with a `repr` attribute + --> $DIR/trailing_zero_sized_array_without_repr.rs:15:1 + | +LL | / struct GenericArrayType { +LL | | field: i32, +LL | | last: [T; 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 with a `repr` attribute + --> $DIR/trailing_zero_sized_array_without_repr.rs:20:1 + | +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 with a `repr` attribute + --> $DIR/trailing_zero_sized_array_without_repr.rs:29:1 + | +LL | / struct OnlyAnotherAttributeDerive { +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 with a `repr` attribute + --> $DIR/trailing_zero_sized_array_without_repr.rs:35:1 + | +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 with a `repr` attribute + --> $DIR/trailing_zero_sized_array_without_repr.rs:44:1 + | +LL | / struct ZeroSizedWithConstFunction { +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 with a `repr` attribute + --> $DIR/trailing_zero_sized_array_without_repr.rs:49:1 + | +LL | struct ZeroSizedArrayWrapper([usize; 0]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = 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 with a `repr` attribute + --> $DIR/trailing_zero_sized_array_without_repr.rs:51:1 + | +LL | struct TupleStruct(i32, [usize; 0]); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | + = 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 with a `repr` attribute + --> $DIR/trailing_zero_sized_array_without_repr.rs:53:1 + | +LL | / struct LotsOfFields { +LL | | f1: u32, +LL | | f2: u32, +LL | | f3: u32, +... | +LL | | last: [usize; 0], +LL | | } + | |_^ + | + = help: consider annotating the struct definition with `#[repr(C)]` or another `repr` attribute + +error: aborting due to 10 previous errors + From a54dbf6445f923b6b315a4c9b08f3e45eaac0081 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 03:52:57 -0700 Subject: [PATCH 21/37] Improve doc and span messages --- clippy_lints/src/trailing_zero_sized_array_without_repr.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs index 0267f7bdf0e..98f5073681d 100644 --- a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs +++ b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs @@ -31,7 +31,7 @@ declare_clippy_lint! { /// ``` pub TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, nursery, - "struct with a trailing zero-sized array but without `repr(C)` or another `repr` attribute" + "struct with a trailing zero-sized array but without `#[repr(C)]` or another `repr` attribute" } declare_lint_pass!(TrailingZeroSizedArrayWithoutRepr => [TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR]); @@ -54,9 +54,9 @@ impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutRepr { cx, TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, lint_span, - "trailing zero-sized array in a struct which is not marked `#[repr(C)]`", + "trailing zero-sized array in a struct which is not marked with a `repr` attribute", None, - "consider annotating the struct definition with `#[repr(C)]` (or another `repr` attribute)", + "consider annotating the struct definition with `#[repr(C)]` or another `repr` attribute", ); } } From 5b78907be7b67cfb598301e20a9ebd500a494c41 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 03:56:49 -0700 Subject: [PATCH 22/37] Still renaming lmao --- ...ing_zero_sized_array_without_repr_c.stderr | 110 ------------------ 1 file changed, 110 deletions(-) delete mode 100644 tests/ui/trailing_zero_sized_array_without_repr_c.stderr diff --git a/tests/ui/trailing_zero_sized_array_without_repr_c.stderr b/tests/ui/trailing_zero_sized_array_without_repr_c.stderr deleted file mode 100644 index 5ed91e937d5..00000000000 --- a/tests/ui/trailing_zero_sized_array_without_repr_c.stderr +++ /dev/null @@ -1,110 +0,0 @@ -error: trailing zero-sized array in a struct which is not marked `#[repr(C)]` - --> $DIR/trailing_zero_sized_array_without_repr_c.rs:6:1 - | -LL | / struct RarelyUseful { -LL | | field: i32, -LL | | last: [usize; 0], -LL | | } - | |_^ - | - = note: `-D clippy::trailing-zero-sized-array-without-repr-c` implied by `-D warnings` - = 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:11:1 - | -LL | / struct OnlyField { -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:15:1 - | -LL | / struct GenericArrayType { -LL | | field: i32, -LL | | last: [T; 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:20:1 - | -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:29:1 - | -LL | / struct OnlyAnotherAttributeDerive { -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:35:1 - | -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:44:1 - | -LL | / struct ZeroSizedWithConstFunction { -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:49:1 - | -LL | struct ZeroSizedArrayWrapper([usize; 0]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = 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:51:1 - | -LL | struct TupleStruct(i32, [usize; 0]); - | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - | - = 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:53:1 - | -LL | / struct LotsOfFields { -LL | | f1: u32, -LL | | f2: u32, -LL | | f3: u32, -... | -LL | | last: [usize; 0], -LL | | } - | |_^ - | - = help: consider annotating the struct definition with `#[repr(C)]` (or another `repr` attribute) - -error: aborting due to 10 previous errors - From d25b4eeefbcb6e2755b0843b0a66de0f5a744460 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 04:22:43 -0700 Subject: [PATCH 23/37] One more test --- tests/ui/trailing_zero_sized_array_without_repr.rs | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/ui/trailing_zero_sized_array_without_repr.rs b/tests/ui/trailing_zero_sized_array_without_repr.rs index 6ac124c8b34..5b12a882aa7 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr.rs +++ b/tests/ui/trailing_zero_sized_array_without_repr.rs @@ -167,6 +167,11 @@ struct ConstParamNonZeroDefault { last: [usize; N], } +struct TwoGenericParams { + field: i32, + last: [T; N], +} + type A = ConstParamZeroDefault; type B = ConstParamZeroDefault<0>; type C = ConstParamNoDefault<0>; From ab9fa25e82b3af4691df275f53c73bc439e84d78 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 04:59:03 -0700 Subject: [PATCH 24/37] Better testcase names --- tests/ui/trailing_zero_sized_array_without_repr.rs | 8 ++++---- tests/ui/trailing_zero_sized_array_without_repr.stderr | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/ui/trailing_zero_sized_array_without_repr.rs b/tests/ui/trailing_zero_sized_array_without_repr.rs index 5b12a882aa7..52966c64db7 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr.rs +++ b/tests/ui/trailing_zero_sized_array_without_repr.rs @@ -18,7 +18,7 @@ struct GenericArrayType { } #[must_use] -struct OnlyAnotherAttributeMustUse { +struct OnlyAnotherAttribute { field: i32, last: [usize; 0], } @@ -26,7 +26,7 @@ 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 { +struct OnlyADeriveAttribute { field: i32, last: [usize; 0], } @@ -102,14 +102,14 @@ struct NonZeroSizedWithConst { #[derive(Debug)] #[repr(C)] -struct OtherAttributesDerive { +struct AlsoADeriveAttribute { field: i32, last: [usize; 0], } #[must_use] #[repr(C)] -struct OtherAttributesMustUse { +struct AlsoAnotherAttribute { field: i32, last: [usize; 0], } diff --git a/tests/ui/trailing_zero_sized_array_without_repr.stderr b/tests/ui/trailing_zero_sized_array_without_repr.stderr index 54b36ec6cec..e5714386c8b 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr.stderr +++ b/tests/ui/trailing_zero_sized_array_without_repr.stderr @@ -35,7 +35,7 @@ error: trailing zero-sized array in a struct which is not marked with a `repr` a --> $DIR/trailing_zero_sized_array_without_repr.rs:20:1 | LL | / #[must_use] -LL | | struct OnlyAnotherAttributeMustUse { +LL | | struct OnlyAnotherAttribute { LL | | field: i32, LL | | last: [usize; 0], LL | | } @@ -46,7 +46,7 @@ LL | | } error: trailing zero-sized array in a struct which is not marked with a `repr` attribute --> $DIR/trailing_zero_sized_array_without_repr.rs:29:1 | -LL | / struct OnlyAnotherAttributeDerive { +LL | / struct OnlyADeriveAttribute { LL | | field: i32, LL | | last: [usize; 0], LL | | } From 3a41f226c5c89806f23ef67502ea29bedf7d9ce8 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 07:02:00 -0700 Subject: [PATCH 25/37] Exploring emitting other sorts of `span`s --- .../trailing_zero_sized_array_without_repr.rs | 42 ++++++++++++++----- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs index 98f5073681d..48feb365ed8 100644 --- a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs +++ b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs @@ -1,5 +1,6 @@ -use clippy_utils::diagnostics::span_lint_and_help; +use clippy_utils::{diagnostics::{span_lint_and_help, span_lint_and_then, span_lint_and_sugg}, source::{indent_of, snippet}}; use rustc_ast::Attribute; +use rustc_errors::Applicability; use rustc_hir::{Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::dep_graph::DepContext; @@ -50,14 +51,34 @@ impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutRepr { }; if !has_repr_attr(cx, attrs) { - span_lint_and_help( - cx, - TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, - lint_span, - "trailing zero-sized array in a struct which is not marked with a `repr` attribute", - None, - "consider annotating the struct definition with `#[repr(C)]` or another `repr` attribute", - ); + let suggestion_span = item.span.shrink_to_lo(); + let indent = " ".repeat(indent_of(cx, item.span).unwrap_or(0)); + + span_lint_and_sugg(cx, TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, item.span, "trailing zero-sized array in a struct which is not marked with a `repr` attribute", "consider adding `#[repr(C)]` or another `repr` attribute", format!("#[repr(C)]\n{}", snippet(cx, item.span.shrink_to_lo().to(item.ident.span), "..")), Applicability::MaybeIncorrect); + + // span_lint_and_then( + // cx, + // TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, + // item.span, + // "trailing zero-sized array in a struct which is not marked with a `repr` attribute", + // |diag| { + // let sugg = format!("#[repr(C)]\n{}", indent); + // let sugg2 = format!("#[repr(C)]\n{}", item.ident.span); + // diag.span_suggestion(item.span, + // "consider adding `#[repr(C)]` or another `repr` attribute", + // sugg2, + // Applicability::MaybeIncorrect); + // } + // ); + + // span_lint_and_help( + // cx, + // TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, + // lint_span, + // "trailing zero-sized array in a struct which is not marked with a `repr` attribute", + // None, + // "consider annotating the struct definition with `#[repr(C)]` or another `repr` attribute", + // ); } } } @@ -91,7 +112,8 @@ 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 (when i wrote this comment. I added a few since then).) Happy to use another; // they're in the commit history if you want to look (or I can go find them). + let sess = cx.tcx.sess(); // are captured values in closures evaluated once or every time? attrs .iter() - .any(|attr| !rustc_attr::find_repr_attrs(cx.tcx.sess(), attr).is_empty()) + .any(|attr| !rustc_attr::find_repr_attrs(sess, attr).is_empty()) } From d8bacf078a0c0d1dd3f15e6554338805f2d7256b Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 15:33:11 -0700 Subject: [PATCH 26/37] All five `has_repr_attr` agree + are correct --- .../trailing_zero_sized_array_without_repr.rs | 115 ++++++++++-------- 1 file changed, 65 insertions(+), 50 deletions(-) diff --git a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs index 48feb365ed8..10088ea55a9 100644 --- a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs +++ b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs @@ -1,11 +1,15 @@ -use clippy_utils::{diagnostics::{span_lint_and_help, span_lint_and_then, span_lint_and_sugg}, source::{indent_of, snippet}}; +use clippy_utils::{ + diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then}, + source::{indent_of, snippet}, +}; use rustc_ast::Attribute; use rustc_errors::Applicability; -use rustc_hir::{Item, ItemKind}; +use rustc_hir::{HirId, Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::dep_graph::DepContext; -use rustc_middle::ty::Const; +use rustc_middle::ty::{self as ty_mod, Const, ReprFlags}; use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_span::sym; declare_clippy_lint! { /// ### What it does @@ -38,48 +42,19 @@ declare_lint_pass!(TrailingZeroSizedArrayWithoutRepr => [TRAILING_ZERO_SIZED_ARR impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutRepr { 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. (note: if - // you don't get rid of this, change `has_repr_attr` to `includes_repr_attr`). - let attrs = cx.tcx.get_attrs(item.def_id.to_def_id()); - 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.to(item.span) - } else { - item.span - }; - - if !has_repr_attr(cx, attrs) { - let suggestion_span = item.span.shrink_to_lo(); - let indent = " ".repeat(indent_of(cx, item.span).unwrap_or(0)); - - span_lint_and_sugg(cx, TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, item.span, "trailing zero-sized array in a struct which is not marked with a `repr` attribute", "consider adding `#[repr(C)]` or another `repr` attribute", format!("#[repr(C)]\n{}", snippet(cx, item.span.shrink_to_lo().to(item.ident.span), "..")), Applicability::MaybeIncorrect); - - // span_lint_and_then( - // cx, - // TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, - // item.span, - // "trailing zero-sized array in a struct which is not marked with a `repr` attribute", - // |diag| { - // let sugg = format!("#[repr(C)]\n{}", indent); - // let sugg2 = format!("#[repr(C)]\n{}", item.ident.span); - // diag.span_suggestion(item.span, - // "consider adding `#[repr(C)]` or another `repr` attribute", - // sugg2, - // Applicability::MaybeIncorrect); - // } - // ); - - // span_lint_and_help( - // cx, - // TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, - // lint_span, - // "trailing zero-sized array in a struct which is not marked with a `repr` attribute", - // None, - // "consider annotating the struct definition with `#[repr(C)]` or another `repr` attribute", - // ); - } + dbg!(item.ident); + if is_struct_with_trailing_zero_sized_array(cx, item) && !has_repr_attr(cx, item) { + eprintln!("consider yourself linted 😎"); + // span_lint_and_help( + // cx, + // TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, + // item.span, + // "trailing zero-sized array in a struct which is not marked with a `repr` + // attribute", + // None, + // "consider annotating the struct definition with `#[repr(C)]` or another + // `repr` attribute", + // ); } } } @@ -108,12 +83,52 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx } } -fn has_repr_attr(cx: &LateContext<'tcx>, attrs: &[Attribute]) -> bool { +fn has_repr_attr(cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) -> 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 (when i wrote this comment. I added a few since then).) Happy to use another; // they're in the commit history if you want to look (or I can go find them). - let sess = cx.tcx.sess(); // are captured values in closures evaluated once or every time? - attrs - .iter() - .any(|attr| !rustc_attr::find_repr_attrs(sess, attr).is_empty()) + + let attrs1 = cx.tcx.hir().attrs(item.hir_id()); + let attrs2 = cx.tcx.get_attrs(item.def_id.to_def_id()); + + let res11 = { + let sess = cx.tcx.sess(); // are captured values in closures evaluated once or every time? + attrs1 + .iter() + .any(|attr| !rustc_attr::find_repr_attrs(sess, attr).is_empty()) + }; + let res12 = { attrs1.iter().any(|attr| attr.has_name(sym::repr)) }; + + let res21 = { + let sess = cx.tcx.sess(); // are captured values in closures evaluated once or every time? + attrs2 + .iter() + .any(|attr| !rustc_attr::find_repr_attrs(sess, attr).is_empty()) + }; + let res22 = { attrs2.iter().any(|attr| attr.has_name(sym::repr)) }; + + let res_adt = { + let ty = cx.tcx.type_of(item.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 + } + }; + + let all_same = (res11 && res12 && res21 && res22 && res_adt) || (!res11 && !res12 && !res21 && !res22 && !res_adt); + + + dbg!(( + (res11, res12, res21, res22, res_adt), + all_same, + )); + + res12 } From 18c863dd0ef490d8f069dba6263d7d5d6f8b3e13 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 16:53:05 -0700 Subject: [PATCH 27/37] Improve help message --- .../trailing_zero_sized_array_without_repr.rs | 87 ++++++------------- 1 file changed, 26 insertions(+), 61 deletions(-) diff --git a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs index 10088ea55a9..0c9066fda82 100644 --- a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs +++ b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs @@ -2,12 +2,10 @@ use clippy_utils::{ diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then}, source::{indent_of, snippet}, }; -use rustc_ast::Attribute; use rustc_errors::Applicability; use rustc_hir::{HirId, Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::dep_graph::DepContext; -use rustc_middle::ty::{self as ty_mod, Const, ReprFlags}; +use rustc_middle::ty::{Const, TyS}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; @@ -43,18 +41,28 @@ declare_lint_pass!(TrailingZeroSizedArrayWithoutRepr => [TRAILING_ZERO_SIZED_ARR impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutRepr { 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) { - eprintln!("consider yourself linted 😎"); - // span_lint_and_help( - // cx, - // TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, - // item.span, - // "trailing zero-sized array in a struct which is not marked with a `repr` - // attribute", - // None, - // "consider annotating the struct definition with `#[repr(C)]` or another - // `repr` attribute", - // ); + if is_struct_with_trailing_zero_sized_array(cx, item) && !has_repr_attr(cx, item.hir_id()) { + let help_msg = format!( + "consider annotating {} with `#[repr(C)]` or another `repr` attribute", + cx.tcx + .type_of(item.def_id) + .ty_adt_def() + .map(|adt_def| cx.tcx.def_path_str(adt_def.did)) + .unwrap_or_else( + // I don't think this will ever be the case, since we made it through + // `is_struct_with_trailing_zero_sized_array`, but I don't feel comfortable putting an `unwrap` + || "the struct definition".to_string() + ) + ); + + span_lint_and_help( + cx, + TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, + item.span, + "trailing zero-sized array in a struct which is not marked with a `repr` attribute", + None, + &help_msg, + ); } } } @@ -83,52 +91,9 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx } } -fn has_repr_attr(cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) -> bool { +fn has_repr_attr(cx: &LateContext<'tcx>, hir_id: HirId) -> 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 (when i wrote this comment. I added a few since then).) Happy to use another; + // on all testcases.) Happy to use another; // they're in the commit history if you want to look (or I can go find them). - - let attrs1 = cx.tcx.hir().attrs(item.hir_id()); - let attrs2 = cx.tcx.get_attrs(item.def_id.to_def_id()); - - let res11 = { - let sess = cx.tcx.sess(); // are captured values in closures evaluated once or every time? - attrs1 - .iter() - .any(|attr| !rustc_attr::find_repr_attrs(sess, attr).is_empty()) - }; - let res12 = { attrs1.iter().any(|attr| attr.has_name(sym::repr)) }; - - let res21 = { - let sess = cx.tcx.sess(); // are captured values in closures evaluated once or every time? - attrs2 - .iter() - .any(|attr| !rustc_attr::find_repr_attrs(sess, attr).is_empty()) - }; - let res22 = { attrs2.iter().any(|attr| attr.has_name(sym::repr)) }; - - let res_adt = { - let ty = cx.tcx.type_of(item.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 - } - }; - - let all_same = (res11 && res12 && res21 && res22 && res_adt) || (!res11 && !res12 && !res21 && !res22 && !res_adt); - - - dbg!(( - (res11, res12, res21, res22, res_adt), - all_same, - )); - - res12 + cx.tcx.hir().attrs(hir_id).iter().any(|attr| attr.has_name(sym::repr)) } From 48cf9c284ac858545cdd22ef2efec9096e6c6b06 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 16:53:17 -0700 Subject: [PATCH 28/37] Don't need `rustc_attr` anymore --- clippy_lints/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 1473a2f3095..de2ebcab667 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -21,7 +21,6 @@ // (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; From d85f903c91d909534003ee2ff0e16316b20687dc Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 17:07:51 -0700 Subject: [PATCH 29/37] !: this is the commit that demonstrates the ICE --- .../trailing_zero_sized_array_without_repr.rs | 55 ++++++++----------- .../trailing_zero_sized_array_without_repr.rs | 8 ++- 2 files changed, 29 insertions(+), 34 deletions(-) diff --git a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs index 0c9066fda82..ac4bbded127 100644 --- a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs +++ b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs @@ -1,11 +1,6 @@ -use clippy_utils::{ - diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then}, - source::{indent_of, snippet}, -}; -use rustc_errors::Applicability; +use clippy_utils::{consts::miri_to_const, consts::Constant, diagnostics::span_lint_and_help}; use rustc_hir::{HirId, Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::{Const, TyS}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; @@ -42,27 +37,15 @@ impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutRepr { 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.hir_id()) { - let help_msg = format!( - "consider annotating {} with `#[repr(C)]` or another `repr` attribute", - cx.tcx - .type_of(item.def_id) - .ty_adt_def() - .map(|adt_def| cx.tcx.def_path_str(adt_def.did)) - .unwrap_or_else( - // I don't think this will ever be the case, since we made it through - // `is_struct_with_trailing_zero_sized_array`, but I don't feel comfortable putting an `unwrap` - || "the struct definition".to_string() - ) - ); - - span_lint_and_help( - cx, - TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, - item.span, - "trailing zero-sized array in a struct which is not marked with a `repr` attribute", - None, - &help_msg, - ); + // span_lint_and_help( + // cx, + // TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, + // item.span, + // "trailing zero-sized array in a struct which is not marked with a `repr` attribute", + // None, + // "", + // ); + eprintln!("consider yourself linted 😎"); } } } @@ -75,11 +58,19 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx if let ItemKind::Struct(data, _) = &item.kind { if let Some(last_field) = data.fields().last() { if let rustc_hir::TyKind::Array(_, length) = last_field.ty.kind { - // Then 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); - let length = length.try_eval_usize(cx.tcx, cx.param_env); - length == Some(0) + let length_did = cx.tcx.hir().body_owner_def_id(length.body).to_def_id(); + let ty = cx.tcx.type_of(length_did); + let length = cx + .tcx + // ICE happens in `const_eval_poly` according to my backtrace + .const_eval_poly(length_did) + .ok() + .map(|val| rustc_middle::ty::Const::from_value(cx.tcx, val, ty)); + if let Some(Constant::Int(length)) = length.and_then(miri_to_const){ + length == 0 + } else { + false + } } else { false } diff --git a/tests/ui/trailing_zero_sized_array_without_repr.rs b/tests/ui/trailing_zero_sized_array_without_repr.rs index 52966c64db7..c00a772d2fe 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr.rs +++ b/tests/ui/trailing_zero_sized_array_without_repr.rs @@ -1,6 +1,8 @@ #![warn(clippy::trailing_zero_sized_array_without_repr)] #![feature(const_generics_defaults)] +// ICE note: All of these are fine + // Do lint: struct RarelyUseful { @@ -23,8 +25,6 @@ struct OnlyAnotherAttribute { last: [usize; 0], } -// NOTE: Unfortunately the attribute isn't included in the lint output. I'm not sure how to make it -// show up. #[derive(Debug)] struct OnlyADeriveAttribute { field: i32, @@ -150,12 +150,16 @@ struct TupleStructReprC(i32, [usize; 0]); type NamedTuple = (i32, [usize; 0]); +// ICE note: and then this one crashes + #[rustfmt::skip] // [rustfmt#4995](https://github.com/rust-lang/rustfmt/issues/4995) struct ConstParamZeroDefault { field: i32, last: [usize; N], } +// ICE notes: presumably these as well but I'm not sure + struct ConstParamNoDefault { field: i32, last: [usize; N], From 6303d2d075131641ef325344124c71052d2bd3eb Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 17:18:07 -0700 Subject: [PATCH 30/37] Revert "!: this is the commit that demonstrates the ICE" This reverts commit d85f903c91d909534003ee2ff0e16316b20687dc. --- .../trailing_zero_sized_array_without_repr.rs | 55 +++++++++++-------- .../trailing_zero_sized_array_without_repr.rs | 8 +-- 2 files changed, 34 insertions(+), 29 deletions(-) diff --git a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs index ac4bbded127..0c9066fda82 100644 --- a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs +++ b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs @@ -1,6 +1,11 @@ -use clippy_utils::{consts::miri_to_const, consts::Constant, diagnostics::span_lint_and_help}; +use clippy_utils::{ + diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then}, + source::{indent_of, snippet}, +}; +use rustc_errors::Applicability; use rustc_hir::{HirId, Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; +use rustc_middle::ty::{Const, TyS}; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; @@ -37,15 +42,27 @@ impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutRepr { 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.hir_id()) { - // span_lint_and_help( - // cx, - // TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, - // item.span, - // "trailing zero-sized array in a struct which is not marked with a `repr` attribute", - // None, - // "", - // ); - eprintln!("consider yourself linted 😎"); + let help_msg = format!( + "consider annotating {} with `#[repr(C)]` or another `repr` attribute", + cx.tcx + .type_of(item.def_id) + .ty_adt_def() + .map(|adt_def| cx.tcx.def_path_str(adt_def.did)) + .unwrap_or_else( + // I don't think this will ever be the case, since we made it through + // `is_struct_with_trailing_zero_sized_array`, but I don't feel comfortable putting an `unwrap` + || "the struct definition".to_string() + ) + ); + + span_lint_and_help( + cx, + TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, + item.span, + "trailing zero-sized array in a struct which is not marked with a `repr` attribute", + None, + &help_msg, + ); } } } @@ -58,19 +75,11 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx if let ItemKind::Struct(data, _) = &item.kind { if let Some(last_field) = data.fields().last() { if let rustc_hir::TyKind::Array(_, length) = last_field.ty.kind { - let length_did = cx.tcx.hir().body_owner_def_id(length.body).to_def_id(); - let ty = cx.tcx.type_of(length_did); - let length = cx - .tcx - // ICE happens in `const_eval_poly` according to my backtrace - .const_eval_poly(length_did) - .ok() - .map(|val| rustc_middle::ty::Const::from_value(cx.tcx, val, ty)); - if let Some(Constant::Int(length)) = length.and_then(miri_to_const){ - length == 0 - } else { - false - } + // Then 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); + let length = length.try_eval_usize(cx.tcx, cx.param_env); + length == Some(0) } else { false } diff --git a/tests/ui/trailing_zero_sized_array_without_repr.rs b/tests/ui/trailing_zero_sized_array_without_repr.rs index c00a772d2fe..52966c64db7 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr.rs +++ b/tests/ui/trailing_zero_sized_array_without_repr.rs @@ -1,8 +1,6 @@ #![warn(clippy::trailing_zero_sized_array_without_repr)] #![feature(const_generics_defaults)] -// ICE note: All of these are fine - // Do lint: struct RarelyUseful { @@ -25,6 +23,8 @@ struct OnlyAnotherAttribute { last: [usize; 0], } +// NOTE: Unfortunately the attribute isn't included in the lint output. I'm not sure how to make it +// show up. #[derive(Debug)] struct OnlyADeriveAttribute { field: i32, @@ -150,16 +150,12 @@ struct TupleStructReprC(i32, [usize; 0]); type NamedTuple = (i32, [usize; 0]); -// ICE note: and then this one crashes - #[rustfmt::skip] // [rustfmt#4995](https://github.com/rust-lang/rustfmt/issues/4995) struct ConstParamZeroDefault { field: i32, last: [usize; N], } -// ICE notes: presumably these as well but I'm not sure - struct ConstParamNoDefault { field: i32, last: [usize; N], From c654cc56da81e9bebc37dd6c476b40c07d297cbe Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 17:41:27 -0700 Subject: [PATCH 31/37] One more test + final tidying --- .../trailing_zero_sized_array_without_repr.rs | 55 ++++++------------- .../trailing_zero_sized_array_without_repr.rs | 8 +++ ...iling_zero_sized_array_without_repr.stderr | 44 +++++++++------ 3 files changed, 53 insertions(+), 54 deletions(-) diff --git a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs index 0c9066fda82..88ec471184c 100644 --- a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs +++ b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs @@ -1,11 +1,7 @@ -use clippy_utils::{ - diagnostics::{span_lint_and_help, span_lint_and_sugg, span_lint_and_then}, - source::{indent_of, snippet}, -}; -use rustc_errors::Applicability; +use clippy_utils::diagnostics::span_lint_and_help; use rustc_hir::{HirId, Item, ItemKind}; use rustc_lint::{LateContext, LateLintPass}; -use rustc_middle::ty::{Const, TyS}; +use rustc_middle::ty::Const; use rustc_session::{declare_lint_pass, declare_tool_lint}; use rustc_span::sym; @@ -40,54 +36,39 @@ declare_lint_pass!(TrailingZeroSizedArrayWithoutRepr => [TRAILING_ZERO_SIZED_ARR impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutRepr { 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.hir_id()) { - let help_msg = format!( - "consider annotating {} with `#[repr(C)]` or another `repr` attribute", - cx.tcx - .type_of(item.def_id) - .ty_adt_def() - .map(|adt_def| cx.tcx.def_path_str(adt_def.did)) - .unwrap_or_else( - // I don't think this will ever be the case, since we made it through - // `is_struct_with_trailing_zero_sized_array`, but I don't feel comfortable putting an `unwrap` - || "the struct definition".to_string() - ) - ); - span_lint_and_help( cx, TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, item.span, "trailing zero-sized array in a struct which is not marked with a `repr` attribute", None, - &help_msg, + &format!( + "consider annotating `{}` with `#[repr(C)]` or another `repr` attribute", + cx.tcx.def_path_str(item.def_id.to_def_id()) + ), ); } } } fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) -> bool { - // TODO: when finalized, replace with an `if_chain`. I have it like this because my rust-analyzer - // doesn't work when it's an `if_chain`. + if_chain! { + // First check if last field is an array + if let ItemKind::Struct(data, _) = &item.kind; + if let Some(last_field) = data.fields().last(); + if let rustc_hir::TyKind::Array(_, length) = last_field.ty.kind; - // First check if last field is an array - if let ItemKind::Struct(data, _) = &item.kind { - if let Some(last_field) = data.fields().last() { - if let rustc_hir::TyKind::Array(_, length) = last_field.ty.kind { - // Then 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); - let length = length.try_eval_usize(cx.tcx, cx.param_env); - length == Some(0) - } else { - false - } + // Then 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); + let length = length.try_eval_usize(cx.tcx, cx.param_env); + if let Some(length) = length; + then { + length == 0 } else { false } - } else { - false } } diff --git a/tests/ui/trailing_zero_sized_array_without_repr.rs b/tests/ui/trailing_zero_sized_array_without_repr.rs index 52966c64db7..dee7f811540 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr.rs +++ b/tests/ui/trailing_zero_sized_array_without_repr.rs @@ -46,6 +46,14 @@ struct ZeroSizedWithConstFunction { last: [usize; compute_zero()], } +const fn compute_zero_from_arg(x: usize) -> usize { + x - 1 +} +struct ZeroSizedWithConstFunction2 { + field: i32, + last: [usize; compute_zero_from_arg(1)], +} + struct ZeroSizedArrayWrapper([usize; 0]); struct TupleStruct(i32, [usize; 0]); diff --git a/tests/ui/trailing_zero_sized_array_without_repr.stderr b/tests/ui/trailing_zero_sized_array_without_repr.stderr index e5714386c8b..93c607bbf38 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr.stderr +++ b/tests/ui/trailing_zero_sized_array_without_repr.stderr @@ -8,7 +8,7 @@ LL | | } | |_^ | = note: `-D clippy::trailing-zero-sized-array-without-repr` implied by `-D warnings` - = help: consider annotating the struct definition with `#[repr(C)]` or another `repr` attribute + = help: consider annotating `RarelyUseful` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute --> $DIR/trailing_zero_sized_array_without_repr.rs:11:1 @@ -18,7 +18,7 @@ LL | | first_and_last: [usize; 0], LL | | } | |_^ | - = help: consider annotating the struct definition with `#[repr(C)]` or another `repr` attribute + = help: consider annotating `OnlyField` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute --> $DIR/trailing_zero_sized_array_without_repr.rs:15:1 @@ -29,19 +29,18 @@ LL | | last: [T; 0], LL | | } | |_^ | - = help: consider annotating the struct definition with `#[repr(C)]` or another `repr` attribute + = help: consider annotating `GenericArrayType` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute - --> $DIR/trailing_zero_sized_array_without_repr.rs:20:1 + --> $DIR/trailing_zero_sized_array_without_repr.rs:21:1 | -LL | / #[must_use] -LL | | struct OnlyAnotherAttribute { +LL | / struct OnlyAnotherAttribute { LL | | field: i32, LL | | last: [usize; 0], LL | | } | |_^ | - = help: consider annotating the struct definition with `#[repr(C)]` or another `repr` attribute + = help: consider annotating `OnlyAnotherAttribute` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute --> $DIR/trailing_zero_sized_array_without_repr.rs:29:1 @@ -52,7 +51,7 @@ LL | | last: [usize; 0], LL | | } | |_^ | - = help: consider annotating the struct definition with `#[repr(C)]` or another `repr` attribute + = help: consider annotating `OnlyADeriveAttribute` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute --> $DIR/trailing_zero_sized_array_without_repr.rs:35:1 @@ -63,7 +62,7 @@ LL | | last: [usize; ZERO], LL | | } | |_^ | - = help: consider annotating the struct definition with `#[repr(C)]` or another `repr` attribute + = help: consider annotating `ZeroSizedWithConst` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute --> $DIR/trailing_zero_sized_array_without_repr.rs:44:1 @@ -74,26 +73,37 @@ LL | | last: [usize; compute_zero()], LL | | } | |_^ | - = help: consider annotating the struct definition with `#[repr(C)]` or another `repr` attribute + = help: consider annotating `ZeroSizedWithConstFunction` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute - --> $DIR/trailing_zero_sized_array_without_repr.rs:49:1 + --> $DIR/trailing_zero_sized_array_without_repr.rs:52:1 + | +LL | / struct ZeroSizedWithConstFunction2 { +LL | | field: i32, +LL | | last: [usize; compute_zero_from_arg(1)], +LL | | } + | |_^ + | + = help: consider annotating `ZeroSizedWithConstFunction2` with `#[repr(C)]` or another `repr` attribute + +error: trailing zero-sized array in a struct which is not marked with a `repr` attribute + --> $DIR/trailing_zero_sized_array_without_repr.rs:57:1 | LL | struct ZeroSizedArrayWrapper([usize; 0]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: consider annotating the struct definition with `#[repr(C)]` or another `repr` attribute + = help: consider annotating `ZeroSizedArrayWrapper` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute - --> $DIR/trailing_zero_sized_array_without_repr.rs:51:1 + --> $DIR/trailing_zero_sized_array_without_repr.rs:59:1 | LL | struct TupleStruct(i32, [usize; 0]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | - = help: consider annotating the struct definition with `#[repr(C)]` or another `repr` attribute + = help: consider annotating `TupleStruct` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute - --> $DIR/trailing_zero_sized_array_without_repr.rs:53:1 + --> $DIR/trailing_zero_sized_array_without_repr.rs:61:1 | LL | / struct LotsOfFields { LL | | f1: u32, @@ -104,7 +114,7 @@ LL | | last: [usize; 0], LL | | } | |_^ | - = help: consider annotating the struct definition with `#[repr(C)]` or another `repr` attribute + = help: consider annotating `LotsOfFields` with `#[repr(C)]` or another `repr` attribute -error: aborting due to 10 previous errors +error: aborting due to 11 previous errors From bc32be0fecd9a99c981323699bbd5a8e485220f0 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 18:03:00 -0700 Subject: [PATCH 32/37] Remove comment --- clippy_lints/src/trailing_zero_sized_array_without_repr.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs index 88ec471184c..cc1671af82d 100644 --- a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs +++ b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs @@ -73,8 +73,5 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx } fn has_repr_attr(cx: &LateContext<'tcx>, hir_id: HirId) -> 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 if you want to look (or I can go find them). cx.tcx.hir().attrs(hir_id).iter().any(|attr| attr.has_name(sym::repr)) } From 02b1f266d6a98cd9dc430554b80971705c5de09b Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 18:20:35 -0700 Subject: [PATCH 33/37] Remove explicit lifetime --- clippy_lints/src/trailing_zero_sized_array_without_repr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs index cc1671af82d..5fa13605ae3 100644 --- a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs +++ b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs @@ -72,6 +72,6 @@ fn is_struct_with_trailing_zero_sized_array(cx: &LateContext<'tcx>, item: &'tcx } } -fn has_repr_attr(cx: &LateContext<'tcx>, hir_id: HirId) -> bool { +fn has_repr_attr(cx: &LateContext<'_>, hir_id: HirId) -> bool { cx.tcx.hir().attrs(hir_id).iter().any(|attr| attr.has_name(sym::repr)) } From 4c8e8169720d54db905142a18502ab2a2dad63a0 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 18:32:00 -0700 Subject: [PATCH 34/37] Use real type in doc examples --- .../src/trailing_zero_sized_array_without_repr.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs index 5fa13605ae3..bfe0049dfae 100644 --- a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs +++ b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs @@ -15,17 +15,17 @@ declare_clippy_lint! { /// ### Example /// ```rust /// struct RarelyUseful { - /// some_field: usize, - /// last: [SomeType; 0], + /// some_field: u32, + /// last: [u32; 0], /// } /// ``` /// - /// Use instead: + /// Use instead: /// ```rust /// #[repr(C)] /// struct MoreOftenUseful { /// some_field: usize, - /// last: [SomeType; 0], + /// last: [u32; 0], /// } /// ``` pub TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, From 5283d24b38631941790082b41ae3c99c698fc346 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Mon, 18 Oct 2021 18:42:01 -0700 Subject: [PATCH 35/37] =?UTF-8?q?formatting=20=F0=9F=99=83?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- clippy_lints/src/trailing_zero_sized_array_without_repr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs index bfe0049dfae..2e579aecab0 100644 --- a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs +++ b/clippy_lints/src/trailing_zero_sized_array_without_repr.rs @@ -20,7 +20,7 @@ declare_clippy_lint! { /// } /// ``` /// - /// Use instead: + /// Use instead: /// ```rust /// #[repr(C)] /// struct MoreOftenUseful { From 60da4c9cb683a41eca4707ab7a533c5759b52485 Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Tue, 19 Oct 2021 14:23:55 -0700 Subject: [PATCH 36/37] remove resolved note --- tests/ui/trailing_zero_sized_array_without_repr.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/ui/trailing_zero_sized_array_without_repr.rs b/tests/ui/trailing_zero_sized_array_without_repr.rs index dee7f811540..bfa3af0bbc3 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr.rs +++ b/tests/ui/trailing_zero_sized_array_without_repr.rs @@ -23,8 +23,6 @@ struct OnlyAnotherAttribute { last: [usize; 0], } -// NOTE: Unfortunately the attribute isn't included in the lint output. I'm not sure how to make it -// show up. #[derive(Debug)] struct OnlyADeriveAttribute { field: i32, From 0f9f591e30074bf5e31e4ca279280425685f33ff Mon Sep 17 00:00:00 2001 From: Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> Date: Tue, 19 Oct 2021 14:33:43 -0700 Subject: [PATCH 37/37] Rename lint --- CHANGELOG.md | 2 +- clippy_lints/src/lib.register_lints.rs | 2 +- clippy_lints/src/lib.register_nursery.rs | 2 +- clippy_lints/src/lib.rs | 4 ++-- ...ithout_repr.rs => trailing_empty_array.rs} | 8 +++---- ...ithout_repr.rs => trailing_empty_array.rs} | 2 +- ...epr.stderr => trailing_empty_array.stderr} | 24 +++++++++---------- 7 files changed, 22 insertions(+), 22 deletions(-) rename clippy_lints/src/{trailing_zero_sized_array_without_repr.rs => trailing_empty_array.rs} (90%) rename tests/ui/{trailing_zero_sized_array_without_repr.rs => trailing_empty_array.rs} (98%) rename tests/ui/{trailing_zero_sized_array_without_repr.stderr => trailing_empty_array.stderr} (82%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cf19a6f0f1..49499324ea0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3021,7 +3021,7 @@ Released 2018-09-13 [`too_many_arguments`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_arguments [`too_many_lines`]: https://rust-lang.github.io/rust-clippy/master/index.html#too_many_lines [`toplevel_ref_arg`]: https://rust-lang.github.io/rust-clippy/master/index.html#toplevel_ref_arg -[`trailing_zero_sized_array_without_repr`]: https://rust-lang.github.io/rust-clippy/master/index.html#trailing_zero_sized_array_without_repr +[`trailing_empty_array`]: https://rust-lang.github.io/rust-clippy/master/index.html#trailing_empty_array [`trait_duplication_in_bounds`]: https://rust-lang.github.io/rust-clippy/master/index.html#trait_duplication_in_bounds [`transmute_bytes_to_str`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_bytes_to_str [`transmute_float_to_int`]: https://rust-lang.github.io/rust-clippy/master/index.html#transmute_float_to_int diff --git a/clippy_lints/src/lib.register_lints.rs b/clippy_lints/src/lib.register_lints.rs index af0629beb36..37d8d7f422f 100644 --- a/clippy_lints/src/lib.register_lints.rs +++ b/clippy_lints/src/lib.register_lints.rs @@ -443,7 +443,7 @@ store.register_lints(&[ temporary_assignment::TEMPORARY_ASSIGNMENT, to_digit_is_some::TO_DIGIT_IS_SOME, to_string_in_display::TO_STRING_IN_DISPLAY, - trailing_zero_sized_array_without_repr::TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, + trailing_empty_array::TRAILING_EMPTY_ARRAY, trait_bounds::TRAIT_DUPLICATION_IN_BOUNDS, trait_bounds::TYPE_REPETITION_IN_BOUNDS, transmute::CROSSPOINTER_TRANSMUTE, diff --git a/clippy_lints/src/lib.register_nursery.rs b/clippy_lints/src/lib.register_nursery.rs index eab0e5b90f1..1e54482a8da 100644 --- a/clippy_lints/src/lib.register_nursery.rs +++ b/clippy_lints/src/lib.register_nursery.rs @@ -25,7 +25,7 @@ store.register_group(true, "clippy::nursery", Some("clippy_nursery"), vec![ LintId::of(regex::TRIVIAL_REGEX), LintId::of(strings::STRING_LIT_AS_BYTES), LintId::of(suspicious_operation_groupings::SUSPICIOUS_OPERATION_GROUPINGS), - LintId::of(trailing_zero_sized_array_without_repr::TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR), + LintId::of(trailing_empty_array::TRAILING_EMPTY_ARRAY), LintId::of(transmute::USELESS_TRANSMUTE), LintId::of(use_self::USE_SELF), ]) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index de2ebcab667..58e4c061892 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -355,7 +355,7 @@ mod tabs_in_doc_comments; mod temporary_assignment; mod to_digit_is_some; mod to_string_in_display; -mod trailing_zero_sized_array_without_repr; +mod trailing_empty_array; mod trait_bounds; mod transmute; mod transmuting_null; @@ -778,7 +778,7 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || Box::new(undocumented_unsafe_blocks::UndocumentedUnsafeBlocks::default())); store.register_late_pass(|| Box::new(match_str_case_mismatch::MatchStrCaseMismatch)); store.register_late_pass(move || Box::new(format_args::FormatArgs)); - store.register_late_pass(|| Box::new(trailing_zero_sized_array_without_repr::TrailingZeroSizedArrayWithoutRepr)); + store.register_late_pass(|| Box::new(trailing_empty_array::TrailingEmptyArray)); } diff --git a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs b/clippy_lints/src/trailing_empty_array.rs similarity index 90% rename from clippy_lints/src/trailing_zero_sized_array_without_repr.rs rename to clippy_lints/src/trailing_empty_array.rs index 2e579aecab0..c216a1f81ea 100644 --- a/clippy_lints/src/trailing_zero_sized_array_without_repr.rs +++ b/clippy_lints/src/trailing_empty_array.rs @@ -28,18 +28,18 @@ declare_clippy_lint! { /// last: [u32; 0], /// } /// ``` - pub TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, + pub TRAILING_EMPTY_ARRAY, nursery, "struct with a trailing zero-sized array but without `#[repr(C)]` or another `repr` attribute" } -declare_lint_pass!(TrailingZeroSizedArrayWithoutRepr => [TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR]); +declare_lint_pass!(TrailingEmptyArray => [TRAILING_EMPTY_ARRAY]); -impl<'tcx> LateLintPass<'tcx> for TrailingZeroSizedArrayWithoutRepr { +impl<'tcx> LateLintPass<'tcx> for TrailingEmptyArray { fn check_item(&mut self, cx: &LateContext<'tcx>, item: &'tcx Item<'tcx>) { if is_struct_with_trailing_zero_sized_array(cx, item) && !has_repr_attr(cx, item.hir_id()) { span_lint_and_help( cx, - TRAILING_ZERO_SIZED_ARRAY_WITHOUT_REPR, + TRAILING_EMPTY_ARRAY, item.span, "trailing zero-sized array in a struct which is not marked with a `repr` attribute", None, diff --git a/tests/ui/trailing_zero_sized_array_without_repr.rs b/tests/ui/trailing_empty_array.rs similarity index 98% rename from tests/ui/trailing_zero_sized_array_without_repr.rs rename to tests/ui/trailing_empty_array.rs index bfa3af0bbc3..501c9eb7651 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr.rs +++ b/tests/ui/trailing_empty_array.rs @@ -1,4 +1,4 @@ -#![warn(clippy::trailing_zero_sized_array_without_repr)] +#![warn(clippy::trailing_empty_array)] #![feature(const_generics_defaults)] // Do lint: diff --git a/tests/ui/trailing_zero_sized_array_without_repr.stderr b/tests/ui/trailing_empty_array.stderr similarity index 82% rename from tests/ui/trailing_zero_sized_array_without_repr.stderr rename to tests/ui/trailing_empty_array.stderr index 93c607bbf38..d88aa0504b5 100644 --- a/tests/ui/trailing_zero_sized_array_without_repr.stderr +++ b/tests/ui/trailing_empty_array.stderr @@ -1,5 +1,5 @@ error: trailing zero-sized array in a struct which is not marked with a `repr` attribute - --> $DIR/trailing_zero_sized_array_without_repr.rs:6:1 + --> $DIR/trailing_empty_array.rs:6:1 | LL | / struct RarelyUseful { LL | | field: i32, @@ -7,11 +7,11 @@ LL | | last: [usize; 0], LL | | } | |_^ | - = note: `-D clippy::trailing-zero-sized-array-without-repr` implied by `-D warnings` + = note: `-D clippy::trailing-empty-array` implied by `-D warnings` = help: consider annotating `RarelyUseful` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute - --> $DIR/trailing_zero_sized_array_without_repr.rs:11:1 + --> $DIR/trailing_empty_array.rs:11:1 | LL | / struct OnlyField { LL | | first_and_last: [usize; 0], @@ -21,7 +21,7 @@ LL | | } = help: consider annotating `OnlyField` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute - --> $DIR/trailing_zero_sized_array_without_repr.rs:15:1 + --> $DIR/trailing_empty_array.rs:15:1 | LL | / struct GenericArrayType { LL | | field: i32, @@ -32,7 +32,7 @@ LL | | } = help: consider annotating `GenericArrayType` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute - --> $DIR/trailing_zero_sized_array_without_repr.rs:21:1 + --> $DIR/trailing_empty_array.rs:21:1 | LL | / struct OnlyAnotherAttribute { LL | | field: i32, @@ -43,7 +43,7 @@ LL | | } = help: consider annotating `OnlyAnotherAttribute` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute - --> $DIR/trailing_zero_sized_array_without_repr.rs:29:1 + --> $DIR/trailing_empty_array.rs:27:1 | LL | / struct OnlyADeriveAttribute { LL | | field: i32, @@ -54,7 +54,7 @@ LL | | } = help: consider annotating `OnlyADeriveAttribute` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute - --> $DIR/trailing_zero_sized_array_without_repr.rs:35:1 + --> $DIR/trailing_empty_array.rs:33:1 | LL | / struct ZeroSizedWithConst { LL | | field: i32, @@ -65,7 +65,7 @@ LL | | } = help: consider annotating `ZeroSizedWithConst` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute - --> $DIR/trailing_zero_sized_array_without_repr.rs:44:1 + --> $DIR/trailing_empty_array.rs:42:1 | LL | / struct ZeroSizedWithConstFunction { LL | | field: i32, @@ -76,7 +76,7 @@ LL | | } = help: consider annotating `ZeroSizedWithConstFunction` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute - --> $DIR/trailing_zero_sized_array_without_repr.rs:52:1 + --> $DIR/trailing_empty_array.rs:50:1 | LL | / struct ZeroSizedWithConstFunction2 { LL | | field: i32, @@ -87,7 +87,7 @@ LL | | } = help: consider annotating `ZeroSizedWithConstFunction2` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute - --> $DIR/trailing_zero_sized_array_without_repr.rs:57:1 + --> $DIR/trailing_empty_array.rs:55:1 | LL | struct ZeroSizedArrayWrapper([usize; 0]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -95,7 +95,7 @@ LL | struct ZeroSizedArrayWrapper([usize; 0]); = help: consider annotating `ZeroSizedArrayWrapper` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute - --> $DIR/trailing_zero_sized_array_without_repr.rs:59:1 + --> $DIR/trailing_empty_array.rs:57:1 | LL | struct TupleStruct(i32, [usize; 0]); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ @@ -103,7 +103,7 @@ LL | struct TupleStruct(i32, [usize; 0]); = help: consider annotating `TupleStruct` with `#[repr(C)]` or another `repr` attribute error: trailing zero-sized array in a struct which is not marked with a `repr` attribute - --> $DIR/trailing_zero_sized_array_without_repr.rs:61:1 + --> $DIR/trailing_empty_array.rs:59:1 | LL | / struct LotsOfFields { LL | | f1: u32,