Skip to content

Remove unsafe_vector_initialization lint #3478

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Dec 3, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

A collection of lints to catch common mistakes and improve your [Rust](https://github.com/rust-lang/rust) code.

[There are 290 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)
[There are 289 lints included in this crate!](https://rust-lang.github.io/rust-clippy/master/index.html)

We have a bunch of lint categories to allow you to choose how much Clippy is supposed to ~~annoy~~ help you:

Expand Down
11 changes: 11 additions & 0 deletions clippy_lints/src/deprecated_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,14 @@ declare_deprecated_lint! {
pub IF_LET_REDUNDANT_PATTERN_MATCHING,
"this lint has been changed to redundant_pattern_matching"
}

/// **What it does:** Nothing. This lint has been deprecated.
///
/// **Deprecation reason:** This lint used to suggest replacing `let mut vec =
/// Vec::with_capacity(n); vec.set_len(n);` with `let vec = vec![0; n];`. The
/// replacement has very different performance characteristics so the lint is
/// deprecated.
declare_deprecated_lint! {
pub UNSAFE_VECTOR_INITIALIZATION,
"the replacement suggested by this lint had substantially different behavior"
}
6 changes: 4 additions & 2 deletions clippy_lints/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,10 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
"if_let_redundant_pattern_matching",
"this lint has been changed to redundant_pattern_matching",
);
store.register_removed(
"unsafe_vector_initialization",
"the replacement suggested by this lint had substantially different behavior",
);
// end deprecated lints, do not remove this comment, it’s used in `update_lints`

reg.register_late_lint_pass(box serde_api::Serde);
Expand Down Expand Up @@ -727,7 +731,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
returns::UNUSED_UNIT,
serde_api::SERDE_API_MISUSE,
slow_vector_initialization::SLOW_VECTOR_INITIALIZATION,
slow_vector_initialization::UNSAFE_VECTOR_INITIALIZATION,
strings::STRING_LIT_AS_BYTES,
suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL,
suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL,
Expand Down Expand Up @@ -974,7 +977,6 @@ pub fn register_plugins(reg: &mut rustc_plugin::Registry<'_>, conf: &Conf) {
ranges::ITERATOR_STEP_BY_ZERO,
regex::INVALID_REGEX,
serde_api::SERDE_API_MISUSE,
slow_vector_initialization::UNSAFE_VECTOR_INITIALIZATION,
suspicious_trait_impl::SUSPICIOUS_ARITHMETIC_IMPL,
suspicious_trait_impl::SUSPICIOUS_OP_ASSIGN_IMPL,
swap::ALMOST_SWAPPED,
Expand Down
53 changes: 1 addition & 52 deletions clippy_lints/src/slow_vector_initialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,32 +39,12 @@ declare_clippy_lint! {
"slow vector initialization"
}

/// **What it does:** Checks unsafe vector initialization
///
/// **Why is this bad?** Changing the length of a vector may expose uninitialized memory, which
/// can lead to memory safety issues
///
/// **Known problems:** None.
///
/// **Example:**
/// ```rust
/// let mut vec1 = Vec::with_capacity(len);
/// unsafe {
/// vec1.set_len(len);
/// }
/// ```
declare_clippy_lint! {
pub UNSAFE_VECTOR_INITIALIZATION,
correctness,
"unsafe vector initialization"
}

#[derive(Copy, Clone, Default)]
pub struct Pass;

impl LintPass for Pass {
fn get_lints(&self) -> LintArray {
lint_array!(SLOW_VECTOR_INITIALIZATION, UNSAFE_VECTOR_INITIALIZATION,)
lint_array!(SLOW_VECTOR_INITIALIZATION,)
}
}

Expand All @@ -90,9 +70,6 @@ enum InitializationType<'tcx> {

/// Resize is a slow initialization with the form `vec.resize(.., 0)`
Resize(&'tcx Expr),

/// UnsafeSetLen is a slow initialization with the form `vec.set_len(..)`
UnsafeSetLen(&'tcx Expr),
}

impl<'a, 'tcx> LateLintPass<'a, 'tcx> for Pass {
Expand Down Expand Up @@ -188,14 +165,6 @@ impl Pass {
vec_alloc: &VecAllocation<'_>,
) {
match initialization {
InitializationType::UnsafeSetLen(e) => Self::emit_lint(
cx,
e,
vec_alloc,
"unsafe vector initialization",
UNSAFE_VECTOR_INITIALIZATION,
),

InitializationType::Extend(e) | InitializationType::Resize(e) => Self::emit_lint(
cx,
e,
Expand Down Expand Up @@ -282,25 +251,6 @@ impl<'a, 'tcx> VectorInitializationVisitor<'a, 'tcx> {
}
}

/// Checks if the given expression is using `set_len` to initialize the vector
fn search_unsafe_set_len(&mut self, expr: &'tcx Expr) {
if_chain! {
if self.initialization_found;
if let ExprKind::MethodCall(ref path, _, ref args) = expr.node;
if let ExprKind::Path(ref qpath_subj) = args[0].node;
if match_qpath(&qpath_subj, &[&self.vec_alloc.variable_name.to_string()]);
if path.ident.name == "set_len";
if let Some(ref len_arg) = args.get(1);

// 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::UnsafeSetLen(expr));
}
}
}

/// Returns `true` if give expression is `repeat(0).take(...)`
fn is_repeat_take(&self, expr: &Expr) -> bool {
if_chain! {
Expand Down Expand Up @@ -349,7 +299,6 @@ impl<'a, 'tcx> Visitor<'tcx> for VectorInitializationVisitor<'a, 'tcx> {
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);
},
_ => (),
}
Expand Down
9 changes: 0 additions & 9 deletions tests/ui/slow_vector_initialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ fn main() {
resize_vector();
extend_vector();
mixed_extend_resize_vector();
unsafe_vector();
}

fn extend_vector() {
Expand Down Expand Up @@ -63,14 +62,6 @@ fn resize_vector() {
vec1.resize(10, 0);
}

fn unsafe_vector() {
let mut unsafe_vec: Vec<u8> = Vec::with_capacity(200);

unsafe {
unsafe_vec.set_len(200);
}
}

fn do_stuff(vec: &mut Vec<u8>) {

}
Expand Down
55 changes: 22 additions & 33 deletions tests/ui/slow_vector_initialization.stderr
Original file line number Diff line number Diff line change
@@ -1,71 +1,60 @@
error: slow zero-filling initialization
--> $DIR/slow_vector_initialization.rs:23:5
--> $DIR/slow_vector_initialization.rs:22:5
|
22 | let mut vec1 = Vec::with_capacity(len);
21 | let mut vec1 = Vec::with_capacity(len);
| ----------------------- help: consider replace allocation with: `vec![0; len]`
23 | vec1.extend(repeat(0).take(len));
22 | vec1.extend(repeat(0).take(len));
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: `-D clippy::slow-vector-initialization` implied by `-D warnings`

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

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

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

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

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

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

error: unsafe vector initialization
--> $DIR/slow_vector_initialization.rs:70:9
|
67 | let mut unsafe_vec: Vec<u8> = Vec::with_capacity(200);
| ----------------------- help: consider replace allocation with: `vec![0; 200]`
...
70 | unsafe_vec.set_len(200);
| ^^^^^^^^^^^^^^^^^^^^^^^
|
= note: #[deny(clippy::unsafe_vector_initialization)] on by default

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