Skip to content

Commit

Permalink
Auto merge of #8953 - DevAccentor:slow_vector_initialization, r=Manis…
Browse files Browse the repository at this point in the history
…hearth

add vec.capacity() to [`slow_vec_initialization`] detection

fix #8800

for example
```rust
let mut vec1 = Vec::with_capacity(len);
vec1.resize(vec1.capacity(), 0);

let mut vec2 = Vec::with_capacity(len);
vec2.extend(repeat(0).take(vec2.capacity()));
```
will trigger the lint

---

changelog: add `vec.capacity()` to [`slow_vec_initialization`] detection
  • Loading branch information
bors committed Jun 23, 2022
2 parents 2cc5211 + 5a70d88 commit f718984
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 24 deletions.
39 changes: 21 additions & 18 deletions clippy_lints/src/slow_vector_initialization.rs
Expand Up @@ -26,6 +26,9 @@ declare_clippy_lint! {
/// let mut vec1 = Vec::with_capacity(len);
/// vec1.resize(len, 0);
///
/// let mut vec1 = Vec::with_capacity(len);
/// vec1.resize(vec1.capacity(), 0);
///
/// let mut vec2 = Vec::with_capacity(len);
/// vec2.extend(repeat(0).take(len));
/// ```
Expand Down Expand Up @@ -211,23 +214,20 @@ impl<'a, 'tcx> VectorInitializationVisitor<'a, 'tcx> {

/// Checks if the given expression is resizing a vector with 0
fn search_slow_resize_filling(&mut self, expr: &'tcx Expr<'_>) {
if_chain! {
if self.initialization_found;
if let ExprKind::MethodCall(path, [self_arg, len_arg, fill_arg], _) = expr.kind;
if path_to_local_id(self_arg, self.vec_alloc.local_id);
if path.ident.name == sym!(resize);

if self.initialization_found
&& let ExprKind::MethodCall(path, [self_arg, len_arg, fill_arg], _) = expr.kind
&& path_to_local_id(self_arg, self.vec_alloc.local_id)
&& path.ident.name == sym!(resize)
// Check that is filled with 0
if let ExprKind::Lit(ref lit) = fill_arg.kind;
if let LitKind::Int(0, _) = lit.node;

// Check that len expression is equals to `with_capacity` expression
if SpanlessEq::new(self.cx).eq_expr(len_arg, self.vec_alloc.len_expr);

then {
self.slow_expression = Some(InitializationType::Resize(expr));
&& let ExprKind::Lit(ref lit) = fill_arg.kind
&& let LitKind::Int(0, _) = lit.node {
// Check that len expression is equals to `with_capacity` expression
if SpanlessEq::new(self.cx).eq_expr(len_arg, self.vec_alloc.len_expr) {
self.slow_expression = Some(InitializationType::Resize(expr));
} else if let ExprKind::MethodCall(path, _, _) = len_arg.kind && path.ident.as_str() == "capacity" {
self.slow_expression = Some(InitializationType::Resize(expr));
}
}
}
}

/// Returns `true` if give expression is `repeat(0).take(...)`
Expand All @@ -240,12 +240,15 @@ impl<'a, 'tcx> VectorInitializationVisitor<'a, 'tcx> {
if let Some(repeat_expr) = take_args.get(0);
if self.is_repeat_zero(repeat_expr);

// Check that len expression is equals to `with_capacity` expression
if let Some(len_arg) = take_args.get(1);
if SpanlessEq::new(self.cx).eq_expr(len_arg, self.vec_alloc.len_expr);

then {
return true;
// Check that len expression is equals to `with_capacity` expression
if SpanlessEq::new(self.cx).eq_expr(len_arg, self.vec_alloc.len_expr) {
return true;
} else if let ExprKind::MethodCall(path, _, _) = len_arg.kind && path.ident.as_str() == "capacity" {
return true;
}
}
}

Expand Down
6 changes: 6 additions & 0 deletions tests/ui/slow_vector_initialization.rs
Expand Up @@ -19,6 +19,9 @@ fn extend_vector() {
// Extend with mismatching expression should not be warned
let mut vec3 = Vec::with_capacity(24322);
vec3.extend(repeat(0).take(2));

let mut vec4 = Vec::with_capacity(len);
vec4.extend(repeat(0).take(vec4.capacity()));
}

fn mixed_extend_resize_vector() {
Expand Down Expand Up @@ -48,6 +51,9 @@ fn resize_vector() {
let mut vec3 = Vec::with_capacity(len - 10);
vec3.resize(len - 10, 0);

let mut vec4 = Vec::with_capacity(len);
vec4.resize(vec4.capacity(), 0);

// Reinitialization should be warned
vec1 = Vec::with_capacity(10);
vec1.resize(10, 0);
Expand Down
28 changes: 22 additions & 6 deletions tests/ui/slow_vector_initialization.stderr
Expand Up @@ -17,44 +17,60 @@ LL | vec2.extend(repeat(0).take(len - 10));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: slow zero-filling initialization
--> $DIR/slow_vector_initialization.rs:31:5
--> $DIR/slow_vector_initialization.rs:24:5
|
LL | let mut vec4 = Vec::with_capacity(len);
| ----------------------- help: consider replace allocation with: `vec![0; len]`
LL | vec4.extend(repeat(0).take(vec4.capacity()));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: slow zero-filling initialization
--> $DIR/slow_vector_initialization.rs:34:5
|
LL | let mut resized_vec = Vec::with_capacity(30);
| ---------------------- help: consider replace allocation with: `vec![0; 30]`
LL | resized_vec.resize(30, 0);
| ^^^^^^^^^^^^^^^^^^^^^^^^^

error: slow zero-filling initialization
--> $DIR/slow_vector_initialization.rs:34:5
--> $DIR/slow_vector_initialization.rs:37:5
|
LL | let mut extend_vec = Vec::with_capacity(30);
| ---------------------- help: consider replace allocation with: `vec![0; 30]`
LL | extend_vec.extend(repeat(0).take(30));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: slow zero-filling initialization
--> $DIR/slow_vector_initialization.rs:41:5
--> $DIR/slow_vector_initialization.rs:44:5
|
LL | let mut vec1 = Vec::with_capacity(len);
| ----------------------- help: consider replace allocation with: `vec![0; len]`
LL | vec1.resize(len, 0);
| ^^^^^^^^^^^^^^^^^^^

error: slow zero-filling initialization
--> $DIR/slow_vector_initialization.rs:49:5
--> $DIR/slow_vector_initialization.rs:52:5
|
LL | let mut vec3 = Vec::with_capacity(len - 10);
| ---------------------------- help: consider replace allocation with: `vec![0; len - 10]`
LL | vec3.resize(len - 10, 0);
| ^^^^^^^^^^^^^^^^^^^^^^^^

error: slow zero-filling initialization
--> $DIR/slow_vector_initialization.rs:53:5
--> $DIR/slow_vector_initialization.rs:55:5
|
LL | let mut vec4 = Vec::with_capacity(len);
| ----------------------- help: consider replace allocation with: `vec![0; len]`
LL | vec4.resize(vec4.capacity(), 0);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

error: slow zero-filling initialization
--> $DIR/slow_vector_initialization.rs:59:5
|
LL | vec1 = Vec::with_capacity(10);
| ---------------------- help: consider replace allocation with: `vec![0; 10]`
LL | vec1.resize(10, 0);
| ^^^^^^^^^^^^^^^^^^

error: aborting due to 7 previous errors
error: aborting due to 9 previous errors

0 comments on commit f718984

Please sign in to comment.