about summary refs log tree commit diff
diff options
context:
space:
mode:
authorYechan Bae <yechan@gatech.edu>2021-09-20 15:32:53 -0400
committerYechan Bae <yechan@gatech.edu>2021-10-09 05:38:19 -0400
commitfdc06d9b2b3e858878b6fd8f6bd815d947e9950a (patch)
tree34b45be9e936dc1b2577bd3f8aaa1552f1da8211
parent759200b6991b5dac5fdb12bc6c366b16850add00 (diff)
downloadrust-fdc06d9b2b3e858878b6fd8f6bd815d947e9950a.tar.gz
rust-fdc06d9b2b3e858878b6fd8f6bd815d947e9950a.zip
Improve error messages
-rw-r--r--clippy_lints/src/uninit_vec.rs14
-rw-r--r--tests/ui/uninit_vec.rs32
-rw-r--r--tests/ui/uninit_vec.stderr69
3 files changed, 88 insertions, 27 deletions
diff --git a/clippy_lints/src/uninit_vec.rs b/clippy_lints/src/uninit_vec.rs
index 37084f57043..b9b575a452b 100644
--- a/clippy_lints/src/uninit_vec.rs
+++ b/clippy_lints/src/uninit_vec.rs
@@ -1,4 +1,4 @@
-use clippy_utils::diagnostics::span_lint_and_note;
+use clippy_utils::diagnostics::span_lint_and_then;
 use clippy_utils::ty::is_type_diagnostic_item;
 use clippy_utils::{
     match_def_path, path_to_local_id, paths, peel_hir_expr_while, ty::is_uninit_value_valid_for_ty, SpanlessEq,
@@ -71,13 +71,14 @@ fn handle_uninit_vec_pair(
         // Check T of Vec<T>
         if !is_uninit_value_valid_for_ty(cx, substs.type_at(0));
         then {
-            span_lint_and_note(
+            span_lint_and_then(
                 cx,
                 UNINIT_VEC,
-                call_span,
+                vec![call_span, maybe_with_capacity_or_reserve.span],
                 "calling `set_len()` immediately after reserving a buffer creates uninitialized values",
-                Some(maybe_with_capacity_or_reserve.span),
-                "the buffer is reserved here"
+                |diag| {
+                    diag.help("initialize the buffer or wrap the content in `MaybeUninit`");
+                },
             );
         }
     }
@@ -155,9 +156,10 @@ fn extract_set_len_self(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> Option<(&
     // peel unsafe blocks in `unsafe { vec.set_len() }`
     let expr = peel_hir_expr_while(expr, |e| {
         if let ExprKind::Block(block, _) = e.kind {
+            // Extract the first statement/expression
             match (block.stmts.get(0).map(|stmt| &stmt.kind), block.expr) {
                 (None, Some(expr)) => Some(expr),
-                (Some(StmtKind::Expr(expr) | StmtKind::Semi(expr)), None) => Some(expr),
+                (Some(StmtKind::Expr(expr) | StmtKind::Semi(expr)), _) => Some(expr),
                 _ => None,
             }
         } else {
diff --git a/tests/ui/uninit_vec.rs b/tests/ui/uninit_vec.rs
index 34b9e07ef1d..e60b73a1e30 100644
--- a/tests/ui/uninit_vec.rs
+++ b/tests/ui/uninit_vec.rs
@@ -2,6 +2,11 @@
 
 use std::mem::MaybeUninit;
 
+#[derive(Default)]
+struct MyVec {
+    vec: Vec<u8>,
+}
+
 fn main() {
     // with_capacity() -> set_len() should be detected
     let mut vec: Vec<u8> = Vec::with_capacity(1000);
@@ -24,6 +29,25 @@ fn main() {
         vec.set_len(200);
     }
 
+    let mut vec: Vec<u8> = Vec::with_capacity(1000);
+    unsafe {
+        // test the case where there are other statements in the following unsafe block
+        vec.set_len(200);
+        assert!(vec.len() == 200);
+    }
+
+    // handle vec stored in the field of a struct
+    let mut my_vec = MyVec::default();
+    my_vec.vec.reserve(1000);
+    unsafe {
+        my_vec.vec.set_len(200);
+    }
+
+    my_vec.vec = Vec::with_capacity(1000);
+    unsafe {
+        my_vec.vec.set_len(200);
+    }
+
     // MaybeUninit-wrapped types should not be detected
     unsafe {
         let mut vec: Vec<MaybeUninit<u8>> = Vec::with_capacity(1000);
@@ -35,4 +59,12 @@ fn main() {
         let mut vec: Vec<(MaybeUninit<u8>, [MaybeUninit<bool>; 2])> = Vec::with_capacity(1000);
         vec.set_len(200);
     }
+
+    // known false negative
+    let mut vec1: Vec<u8> = Vec::with_capacity(1000);
+    let mut vec2: Vec<u8> = Vec::with_capacity(1000);
+    unsafe {
+        vec1.reserve(200);
+        vec2.reserve(200);
+    }
 }
diff --git a/tests/ui/uninit_vec.stderr b/tests/ui/uninit_vec.stderr
index 1d7807fcf7d..31f8ae40350 100644
--- a/tests/ui/uninit_vec.stderr
+++ b/tests/ui/uninit_vec.stderr
@@ -1,51 +1,78 @@
 error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
-  --> $DIR/uninit_vec.rs:9:9
+  --> $DIR/uninit_vec.rs:12:5
    |
+LL |     let mut vec: Vec<u8> = Vec::with_capacity(1000);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     unsafe {
 LL |         vec.set_len(200);
    |         ^^^^^^^^^^^^^^^^
    |
    = note: `-D clippy::uninit-vec` implied by `-D warnings`
-note: the buffer is reserved here
-  --> $DIR/uninit_vec.rs:7:5
-   |
-LL |     let mut vec: Vec<u8> = Vec::with_capacity(1000);
-   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = help: initialize the buffer or wrap the content in `MaybeUninit`
 
 error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
-  --> $DIR/uninit_vec.rs:15:9
+  --> $DIR/uninit_vec.rs:18:5
    |
+LL |     vec.reserve(1000);
+   |     ^^^^^^^^^^^^^^^^^^
+LL |     unsafe {
 LL |         vec.set_len(200);
    |         ^^^^^^^^^^^^^^^^
    |
-note: the buffer is reserved here
-  --> $DIR/uninit_vec.rs:13:5
-   |
-LL |     vec.reserve(1000);
-   |     ^^^^^^^^^^^^^^^^^^
+   = help: initialize the buffer or wrap the content in `MaybeUninit`
 
 error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
-  --> $DIR/uninit_vec.rs:21:9
+  --> $DIR/uninit_vec.rs:32:5
    |
+LL |     let mut vec: Vec<u8> = Vec::with_capacity(1000);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+...
 LL |         vec.set_len(200);
    |         ^^^^^^^^^^^^^^^^
    |
-note: the buffer is reserved here
-  --> $DIR/uninit_vec.rs:20:9
+   = help: initialize the buffer or wrap the content in `MaybeUninit`
+
+error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
+  --> $DIR/uninit_vec.rs:41:5
+   |
+LL |     my_vec.vec.reserve(1000);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     unsafe {
+LL |         my_vec.vec.set_len(200);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^
    |
-LL |         let mut vec: Vec<u8> = Vec::with_capacity(1000);
-   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+   = help: initialize the buffer or wrap the content in `MaybeUninit`
+
+error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
+  --> $DIR/uninit_vec.rs:46:5
+   |
+LL |     my_vec.vec = Vec::with_capacity(1000);
+   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+LL |     unsafe {
+LL |         my_vec.vec.set_len(200);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^
+   |
+   = help: initialize the buffer or wrap the content in `MaybeUninit`
 
 error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
-  --> $DIR/uninit_vec.rs:24:9
+  --> $DIR/uninit_vec.rs:25:9
    |
+LL |         let mut vec: Vec<u8> = Vec::with_capacity(1000);
+   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 LL |         vec.set_len(200);
    |         ^^^^^^^^^^^^^^^^
    |
-note: the buffer is reserved here
-  --> $DIR/uninit_vec.rs:23:9
+   = help: initialize the buffer or wrap the content in `MaybeUninit`
+
+error: calling `set_len()` immediately after reserving a buffer creates uninitialized values
+  --> $DIR/uninit_vec.rs:28:9
    |
 LL |         vec.reserve(1000);
    |         ^^^^^^^^^^^^^^^^^^
+LL |         vec.set_len(200);
+   |         ^^^^^^^^^^^^^^^^
+   |
+   = help: initialize the buffer or wrap the content in `MaybeUninit`
 
-error: aborting due to 4 previous errors
+error: aborting due to 7 previous errors