From 8514b8407ac83dc02532c82c9188c49967d9a5d6 Mon Sep 17 00:00:00 2001 From: Christoph Walcher Date: Fri, 14 Aug 2020 14:13:35 +0200 Subject: [PATCH] appreciative too_large_for_stack in useless `vec!` Fixes: #5847 --- clippy_lints/src/lib.rs | 2 +- clippy_lints/src/utils/conf.rs | 2 +- clippy_lints/src/vec.rs | 97 +++++++++++++++++++++------------- tests/ui/vec.fixed | 7 +++ tests/ui/vec.rs | 7 +++ 5 files changed, 76 insertions(+), 39 deletions(-) diff --git a/clippy_lints/src/lib.rs b/clippy_lints/src/lib.rs index 6ad525d7620..4a4445200a1 100644 --- a/clippy_lints/src/lib.rs +++ b/clippy_lints/src/lib.rs @@ -930,11 +930,11 @@ pub fn register_plugins(store: &mut rustc_lint::LintStore, sess: &Session, conf: store.register_late_pass(move || box cognitive_complexity::CognitiveComplexity::new(cognitive_complexity_threshold)); let too_large_for_stack = conf.too_large_for_stack; store.register_late_pass(move || box escape::BoxedLocal{too_large_for_stack}); + store.register_late_pass(move || box vec::UselessVec{too_large_for_stack}); store.register_late_pass(|| box panic_unimplemented::PanicUnimplemented); store.register_late_pass(|| box strings::StringLitAsBytes); store.register_late_pass(|| box derive::Derive); store.register_late_pass(|| box types::CharLitAsU8); - store.register_late_pass(|| box vec::UselessVec); store.register_late_pass(|| box drop_bounds::DropBounds); store.register_late_pass(|| box get_last_with_len::GetLastWithLen); store.register_late_pass(|| box drop_forget_ref::DropForgetRef); diff --git a/clippy_lints/src/utils/conf.rs b/clippy_lints/src/utils/conf.rs index ba3492a6fff..292dbd7ad6b 100644 --- a/clippy_lints/src/utils/conf.rs +++ b/clippy_lints/src/utils/conf.rs @@ -138,7 +138,7 @@ define_Conf! { (type_complexity_threshold, "type_complexity_threshold": u64, 250), /// Lint: MANY_SINGLE_CHAR_NAMES. The maximum number of single char bindings a scope may have (single_char_binding_names_threshold, "single_char_binding_names_threshold": u64, 4), - /// Lint: BOXED_LOCAL. The maximum size of objects (in bytes) that will be linted. Larger objects are ok on the heap + /// Lint: BOXED_LOCAL, USELESS_VEC. The maximum size of objects (in bytes) that will be linted. Larger objects are ok on the heap (too_large_for_stack, "too_large_for_stack": u64, 200), /// Lint: ENUM_VARIANT_NAMES. The minimum number of enum variants for the lints about variant names to trigger (enum_variant_name_threshold, "enum_variant_name_threshold": u64, 3), diff --git a/clippy_lints/src/vec.rs b/clippy_lints/src/vec.rs index f2e76442a19..84e907d7125 100644 --- a/clippy_lints/src/vec.rs +++ b/clippy_lints/src/vec.rs @@ -1,13 +1,20 @@ -use crate::consts::constant; +use crate::consts::{constant, Constant}; +use crate::rustc_target::abi::LayoutOf; use crate::utils::{higher, is_copy, snippet_with_applicability, span_lint_and_sugg}; use if_chain::if_chain; use rustc_errors::Applicability; use rustc_hir::{BorrowKind, Expr, ExprKind}; use rustc_lint::{LateContext, LateLintPass}; use rustc_middle::ty::{self, Ty}; -use rustc_session::{declare_lint_pass, declare_tool_lint}; +use rustc_session::{declare_tool_lint, impl_lint_pass}; use rustc_span::source_map::Span; +#[allow(clippy::module_name_repetitions)] +#[derive(Copy, Clone)] +pub struct UselessVec { + pub too_large_for_stack: u64, +} + declare_clippy_lint! { /// **What it does:** Checks for usage of `&vec![..]` when using `&[..]` would /// be possible. @@ -31,7 +38,7 @@ declare_clippy_lint! { "useless `vec!`" } -declare_lint_pass!(UselessVec => [USELESS_VEC]); +impl_lint_pass!(UselessVec => [USELESS_VEC]); impl<'tcx> LateLintPass<'tcx> for UselessVec { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'_>) { @@ -42,7 +49,7 @@ impl<'tcx> LateLintPass<'tcx> for UselessVec { if let ExprKind::AddrOf(BorrowKind::Ref, _, ref addressee) = expr.kind; if let Some(vec_args) = higher::vec_macro(cx, addressee); then { - check_vec_macro(cx, &vec_args, expr.span); + self.check_vec_macro(cx, &vec_args, expr.span); } } @@ -60,46 +67,62 @@ impl<'tcx> LateLintPass<'tcx> for UselessVec { .ctxt() .outer_expn_data() .call_site; - check_vec_macro(cx, &vec_args, span); + self.check_vec_macro(cx, &vec_args, span); } } } } -fn check_vec_macro<'tcx>(cx: &LateContext<'tcx>, vec_args: &higher::VecArgs<'tcx>, span: Span) { - let mut applicability = Applicability::MachineApplicable; - let snippet = match *vec_args { - higher::VecArgs::Repeat(elem, len) => { - if constant(cx, cx.typeck_results(), len).is_some() { - format!( - "&[{}; {}]", - snippet_with_applicability(cx, elem.span, "elem", &mut applicability), - snippet_with_applicability(cx, len.span, "len", &mut applicability) - ) - } else { - return; - } - }, - higher::VecArgs::Vec(args) => { - if let Some(last) = args.iter().last() { - let span = args[0].span.to(last.span); +impl UselessVec { + fn check_vec_macro<'tcx>(self, cx: &LateContext<'tcx>, vec_args: &higher::VecArgs<'tcx>, span: Span) { + let mut applicability = Applicability::MachineApplicable; + let snippet = match *vec_args { + higher::VecArgs::Repeat(elem, len) => { + if let Some((Constant::Int(len_constant), _)) = constant(cx, cx.typeck_results(), len) { + #[allow(clippy::cast_possible_truncation)] + if len_constant as u64 * size_of(cx, elem) > self.too_large_for_stack { + return; + } - format!("&[{}]", snippet_with_applicability(cx, span, "..", &mut applicability)) - } else { - "&[]".into() - } - }, - }; + format!( + "&[{}; {}]", + snippet_with_applicability(cx, elem.span, "elem", &mut applicability), + snippet_with_applicability(cx, len.span, "len", &mut applicability) + ) + } else { + return; + } + }, + higher::VecArgs::Vec(args) => { + if let Some(last) = args.iter().last() { + #[allow(clippy::cast_possible_truncation)] + if args.len() as u64 * size_of(cx, last) > self.too_large_for_stack { + return; + } + let span = args[0].span.to(last.span); - span_lint_and_sugg( - cx, - USELESS_VEC, - span, - "useless use of `vec!`", - "you can use a slice directly", - snippet, - applicability, - ); + format!("&[{}]", snippet_with_applicability(cx, span, "..", &mut applicability)) + } else { + "&[]".into() + } + }, + }; + + span_lint_and_sugg( + cx, + USELESS_VEC, + span, + "useless use of `vec!`", + "you can use a slice directly", + snippet, + applicability, + ); + } +} + +fn size_of(cx: &LateContext<'_>, expr: &Expr<'_>) -> u64 { + let ty = cx.typeck_results().expr_ty_adjusted(expr); + cx.layout_of(ty).map_or(0, |l| l.size.bytes()) } /// Returns the item type of the vector (i.e., the `T` in `Vec`). diff --git a/tests/ui/vec.fixed b/tests/ui/vec.fixed index e73a791891f..85677159620 100644 --- a/tests/ui/vec.fixed +++ b/tests/ui/vec.fixed @@ -52,4 +52,11 @@ fn main() { for a in vec![NonCopy, NonCopy] { println!("{:?}", a); } + + on_vec(&vec![1; 201]); // Ok, size of `vec` higher than `too_large_for_stack` + + // Ok + for a in vec![1; 201] { + println!("{:?}", a); + } } diff --git a/tests/ui/vec.rs b/tests/ui/vec.rs index 3eb960f53d7..03b8ee81665 100644 --- a/tests/ui/vec.rs +++ b/tests/ui/vec.rs @@ -52,4 +52,11 @@ fn main() { for a in vec![NonCopy, NonCopy] { println!("{:?}", a); } + + on_vec(&vec![1; 201]); // Ok, size of `vec` higher than `too_large_for_stack` + + // Ok + for a in vec![1; 201] { + println!("{:?}", a); + } }