diff options
| author | Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> | 2021-10-18 15:33:11 -0700 |
|---|---|---|
| committer | Nathaniel Hamovitz <18648574+nhamovitz@users.noreply.github.com> | 2021-10-18 15:33:11 -0700 |
| commit | d8bacf078a0c0d1dd3f15e6554338805f2d7256b (patch) | |
| tree | b5fa83160055ce3b139a93027890eb61dc2dd547 | |
| parent | 3a41f226c5c89806f23ef67502ea29bedf7d9ce8 (diff) | |
| download | rust-d8bacf078a0c0d1dd3f15e6554338805f2d7256b.tar.gz rust-d8bacf078a0c0d1dd3f15e6554338805f2d7256b.zip | |
All five `has_repr_attr` agree + are correct
| -rw-r--r-- | clippy_lints/src/trailing_zero_sized_array_without_repr.rs | 115 |
1 files 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 } |
