From 5fa04bc3cdb3071dddbfd9cf9cf33bc099b182f0 Mon Sep 17 00:00:00 2001 From: Guillem Nieto Date: Sun, 18 Nov 2018 23:45:53 +0100 Subject: [PATCH] Lint only the first statment/expression after alloc Instead of searching for all the successive expressions after a vector allocation, check only the first expression. This is done to minimize the amount of false positives of the lint. --- .../src/slow_vector_initialization.rs | 41 ++++++++++++++----- tests/ui/slow_vector_initialization.rs | 17 ++++++-- tests/ui/slow_vector_initialization.stderr | 10 ++--- 3 files changed, 49 insertions(+), 19 deletions(-) diff --git a/clippy_lints/src/slow_vector_initialization.rs b/clippy_lints/src/slow_vector_initialization.rs index 2af657a5a..272047bc7 100644 --- a/clippy_lints/src/slow_vector_initialization.rs +++ b/clippy_lints/src/slow_vector_initialization.rs @@ -7,7 +7,7 @@ // option. This file may not be copied, modified, or distributed // except according to those terms. -use crate::rustc::hir::intravisit::{walk_expr, NestedVisitorMap, Visitor}; +use crate::rustc::hir::intravisit::{walk_expr, walk_stmt, walk_block, NestedVisitorMap, Visitor}; use crate::rustc::lint::{LateContext, LateLintPass, LintArray, LintPass, Lint}; use crate::rustc::{declare_tool_lint, lint_array}; use crate::rustc::hir::*; @@ -353,20 +353,41 @@ impl<'a, 'tcx> VectorInitializationVisitor<'a, 'tcx> { } impl<'a, 'tcx> Visitor<'tcx> for VectorInitializationVisitor<'a, 'tcx> { - fn visit_expr(&mut self, expr: &'tcx Expr) { - // Stop the search if we already found a slow zero-filling initialization - if self.slow_expression.is_some() { - return - } + fn visit_stmt(&mut self, stmt: &'tcx Stmt) { + if self.initialization_found { + match stmt.node { + StmtKind::Expr(ref expr, _) | + StmtKind::Semi(ref expr, _) => { + self.search_slow_extend_filling(expr); + self.search_slow_resize_filling(expr); + self.search_unsafe_set_len(expr); + }, + _ => (), + } + self.initialization_found = false; + } else { + walk_stmt(self, stmt); + } + } + + fn visit_block(&mut self, block: &'tcx Block) { + if self.initialization_found { + if let Some(ref s) = block.stmts.get(0) { + self.visit_stmt( s) + } + + self.initialization_found = false; + } else { + walk_block(self, block); + } + } + + fn visit_expr(&mut self, expr: &'tcx Expr) { // Skip all the expressions previous to the vector initialization if self.vec_alloc.allocation_expr.id == expr.id { self.initialization_found = true; } - - self.search_slow_extend_filling(expr); - self.search_slow_resize_filling(expr); - self.search_unsafe_set_len(expr); walk_expr(self, expr); } diff --git a/tests/ui/slow_vector_initialization.rs b/tests/ui/slow_vector_initialization.rs index 991d85046..daa6b9c13 100644 --- a/tests/ui/slow_vector_initialization.rs +++ b/tests/ui/slow_vector_initialization.rs @@ -34,13 +34,13 @@ fn extend_vector() { fn mixed_extend_resize_vector() { // Mismatching len let mut mismatching_len = Vec::with_capacity(30); + mismatching_len.extend(repeat(0).take(40)); // Slow initialization let mut resized_vec = Vec::with_capacity(30); - let mut extend_vec = Vec::with_capacity(30); - resized_vec.resize(30, 0); - mismatching_len.extend(repeat(0).take(40)); + + let mut extend_vec = Vec::with_capacity(30); extend_vec.extend(repeat(0).take(30)); } @@ -70,3 +70,14 @@ fn unsafe_vector() { unsafe_vec.set_len(200); } } + +fn do_stuff(vec: &mut Vec) { + +} + +fn extend_vector_with_manipulations_between() { + let len = 300; + let mut vec1:Vec = Vec::with_capacity(len); + do_stuff(&mut vec1); + vec1.extend(repeat(0).take(len)); +} diff --git a/tests/ui/slow_vector_initialization.stderr b/tests/ui/slow_vector_initialization.stderr index c1e48d42a..577cc82c6 100644 --- a/tests/ui/slow_vector_initialization.stderr +++ b/tests/ui/slow_vector_initialization.stderr @@ -17,20 +17,18 @@ error: slow zero-filling initialization | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ error: slow zero-filling initialization - --> $DIR/slow_vector_initialization.rs:42:5 + --> $DIR/slow_vector_initialization.rs:41:5 | -39 | let mut resized_vec = Vec::with_capacity(30); +40 | let mut resized_vec = Vec::with_capacity(30); | ---------------------- help: consider replace allocation with: `vec![0; 30]` -... -42 | resized_vec.resize(30, 0); +41 | resized_vec.resize(30, 0); | ^^^^^^^^^^^^^^^^^^^^^^^^^ error: slow zero-filling initialization --> $DIR/slow_vector_initialization.rs:44:5 | -40 | let mut extend_vec = Vec::with_capacity(30); +43 | let mut extend_vec = Vec::with_capacity(30); | ---------------------- help: consider replace allocation with: `vec![0; 30]` -... 44 | extend_vec.extend(repeat(0).take(30)); | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^