Skip to content

Commit

Permalink
Auto merge of #11451 - y21:issue11408, r=xFrednet
Browse files Browse the repository at this point in the history
[`slow_vector_initialization`]: use the source span of vec![] macro and fix another FP

Fixes #11408

<details>
<summary>Also fixes a FP when the vec initializer comes from a macro other than `vec![]`</summary>

```rs
macro_rules! x {
  () => { vec![] }
}
fn f() {
  let mut v = x!();
  v.resize(10, 0);
}
```
This shouldn't warn. The `x!` macro might be doing other things, so just replacing `x!()` with `vec![0; 10]` is not always an option.
</details>

I added some test cases for macro expansions, however I don't think there's a way to write a test for that specific warning that appeared in the linked issue. As far as I understand, that happens when the rust-src rustup component isn't installed (so the stdlib source is unavailable) and the span points to the `vec![]` *expansion*, instead of the `vec![]` that the user wrote.

changelog: [`slow_vector_initialization`]: use the source span of `vec![]` macro
changelog: [`slow_vector_initialization`]: only warn on `vec![]` expansions and allow other macros
  • Loading branch information
bors committed Sep 7, 2023
2 parents 69fcbfd + 5120632 commit 415ba21
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 4 deletions.
12 changes: 11 additions & 1 deletion clippy_lints/src/slow_vector_initialization.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::macros::root_macro_call;
use clippy_utils::sugg::Sugg;
use clippy_utils::{
get_enclosing_block, is_expr_path_def_path, is_integer_literal, is_path_diagnostic_item, path_to_local,
Expand Down Expand Up @@ -148,6 +149,15 @@ impl SlowVectorInit {
/// - `Some(InitializedSize::Uninitialized)` for `Vec::new()`
/// - `None` for other, unrelated kinds of expressions
fn as_vec_initializer<'tcx>(cx: &LateContext<'_>, expr: &'tcx Expr<'tcx>) -> Option<InitializedSize<'tcx>> {
// Generally don't warn if the vec initializer comes from an expansion, except for the vec! macro.
// This lets us still warn on `vec![]`, while ignoring other kinds of macros that may output an
// empty vec
if expr.span.from_expansion()
&& root_macro_call(expr.span).map(|m| m.def_id) != cx.tcx.get_diagnostic_item(sym::vec_macro)
{
return None;
}

if let ExprKind::Call(func, [len_expr]) = expr.kind
&& is_expr_path_def_path(cx, func, &paths::VEC_WITH_CAPACITY)
{
Expand Down Expand Up @@ -205,7 +215,7 @@ impl SlowVectorInit {

span_lint_and_then(cx, SLOW_VECTOR_INITIALIZATION, slow_fill.span, msg, |diag| {
diag.span_suggestion(
vec_alloc.allocation_expr.span,
vec_alloc.allocation_expr.span.source_callsite(),
"consider replacing this with",
format!("vec![0; {len_expr}]"),
Applicability::Unspecified,
Expand Down
16 changes: 15 additions & 1 deletion tests/ui/slow_vector_initialization.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use std::iter::repeat;
//@no-rustfix
use std::iter::repeat;
fn main() {
resize_vector();
extend_vector();
Expand Down Expand Up @@ -86,6 +86,20 @@ fn from_empty_vec() {
vec1 = Vec::new();
vec1.resize(10, 0);
//~^ ERROR: slow zero-filling initialization

vec1 = vec![];
vec1.resize(10, 0);
//~^ ERROR: slow zero-filling initialization

macro_rules! x {
() => {
vec![]
};
}

// `vec![]` comes from another macro, don't warn
vec1 = x!();
vec1.resize(10, 0);
}

fn do_stuff(vec: &mut [u8]) {}
Expand Down
12 changes: 10 additions & 2 deletions tests/ui/slow_vector_initialization.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,21 @@ LL | vec1 = Vec::new();
LL | vec1.resize(10, 0);
| ^^^^^^^^^^^^^^^^^^

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

error: this argument is a mutable reference, but not used mutably
--> $DIR/slow_vector_initialization.rs:91:18
--> $DIR/slow_vector_initialization.rs:105:18
|
LL | fn do_stuff(vec: &mut [u8]) {}
| ^^^^^^^^^ help: consider changing to: `&[u8]`
|
= note: `-D clippy::needless-pass-by-ref-mut` implied by `-D warnings`

error: aborting due to 13 previous errors
error: aborting due to 14 previous errors

0 comments on commit 415ba21

Please sign in to comment.