about summary refs log tree commit diff
diff options
context:
space:
mode:
authorYechan Bae <yechan@gatech.edu>2021-09-17 14:42:32 -0400
committerYechan Bae <yechan@gatech.edu>2021-10-09 05:38:19 -0400
commit759200b6991b5dac5fdb12bc6c366b16850add00 (patch)
tree42889feed24da30f65b8e21c6d0fde54ecf00e15
parentb8ba7269cd10843e4aebf855c4f72e980a9c0ed6 (diff)
downloadrust-759200b6991b5dac5fdb12bc6c366b16850add00.tar.gz
rust-759200b6991b5dac5fdb12bc6c366b16850add00.zip
Handle PR feedbacks first round
-rw-r--r--clippy_lints/src/methods/uninit_assumed_init.rs4
-rw-r--r--clippy_lints/src/uninit_vec.rs41
-rw-r--r--clippy_utils/src/lib.rs10
-rw-r--r--clippy_utils/src/paths.rs1
-rw-r--r--clippy_utils/src/ty.rs10
-rw-r--r--tests/ui/uninit_vec.rs8
6 files changed, 42 insertions, 32 deletions
diff --git a/clippy_lints/src/methods/uninit_assumed_init.rs b/clippy_lints/src/methods/uninit_assumed_init.rs
index dbd91fb6d9d..ce89189bce9 100644
--- a/clippy_lints/src/methods/uninit_assumed_init.rs
+++ b/clippy_lints/src/methods/uninit_assumed_init.rs
@@ -1,5 +1,5 @@
 use clippy_utils::diagnostics::span_lint;
-use clippy_utils::{is_expr_path_def_path, is_uninit_ty_valid, paths};
+use clippy_utils::{is_expr_path_def_path, paths, ty::is_uninit_value_valid_for_ty};
 use if_chain::if_chain;
 use rustc_hir as hir;
 use rustc_lint::LateContext;
@@ -12,7 +12,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr
         if let hir::ExprKind::Call(callee, args) = recv.kind;
         if args.is_empty();
         if is_expr_path_def_path(cx, callee, &paths::MEM_MAYBEUNINIT_UNINIT);
-        if !is_uninit_ty_valid(cx, cx.typeck_results().expr_ty_adjusted(expr));
+        if !is_uninit_value_valid_for_ty(cx, cx.typeck_results().expr_ty_adjusted(expr));
         then {
             span_lint(
                 cx,
diff --git a/clippy_lints/src/uninit_vec.rs b/clippy_lints/src/uninit_vec.rs
index 8b1254b60f9..37084f57043 100644
--- a/clippy_lints/src/uninit_vec.rs
+++ b/clippy_lints/src/uninit_vec.rs
@@ -1,11 +1,14 @@
 use clippy_utils::diagnostics::span_lint_and_note;
-use clippy_utils::{is_uninit_ty_valid, match_def_path, path_to_local_id, paths, peel_hir_expr_while, SpanlessEq};
+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,
+};
 use rustc_hir::def::Res;
 use rustc_hir::{Block, Expr, ExprKind, HirId, PatKind, Stmt, StmtKind};
 use rustc_lint::{LateContext, LateLintPass};
 use rustc_middle::ty;
 use rustc_session::{declare_lint_pass, declare_tool_lint};
-use rustc_span::Span;
+use rustc_span::{sym, Span};
 
 declare_clippy_lint! {
     /// ### What it does
@@ -44,32 +47,36 @@ impl<'tcx> LateLintPass<'tcx> for UninitVec {
     fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'_>) {
         for w in block.stmts.windows(2) {
             if let StmtKind::Expr(expr) | StmtKind::Semi(expr) = w[1].kind {
-                handle_pair(cx, &w[0], expr);
+                handle_uninit_vec_pair(cx, &w[0], expr);
             }
         }
 
         if let (Some(stmt), Some(expr)) = (block.stmts.last(), block.expr) {
-            handle_pair(cx, stmt, expr);
+            handle_uninit_vec_pair(cx, stmt, expr);
         }
     }
 }
 
-fn handle_pair(cx: &LateContext<'tcx>, first: &'tcx Stmt<'tcx>, second: &'tcx Expr<'tcx>) {
+fn handle_uninit_vec_pair(
+    cx: &LateContext<'tcx>,
+    maybe_with_capacity_or_reserve: &'tcx Stmt<'tcx>,
+    maybe_set_len: &'tcx Expr<'tcx>,
+) {
     if_chain! {
-        if let Some(vec) = extract_with_capacity_or_reserve_target(cx, first);
-        if let Some((set_len_self, call_span)) = extract_set_len_self(cx, second);
+        if let Some(vec) = extract_with_capacity_or_reserve_target(cx, maybe_with_capacity_or_reserve);
+        if let Some((set_len_self, call_span)) = extract_set_len_self(cx, maybe_set_len);
         if vec.eq_expr(cx, set_len_self);
         if let ty::Ref(_, vec_ty, _) = cx.typeck_results().expr_ty_adjusted(set_len_self).kind();
         if let ty::Adt(_, substs) = vec_ty.kind();
         // Check T of Vec<T>
-        if !is_uninit_ty_valid(cx, substs.type_at(0));
+        if !is_uninit_value_valid_for_ty(cx, substs.type_at(0));
         then {
             span_lint_and_note(
                 cx,
                 UNINIT_VEC,
                 call_span,
                 "calling `set_len()` immediately after reserving a buffer creates uninitialized values",
-                Some(first.span),
+                Some(maybe_with_capacity_or_reserve.span),
                 "the buffer is reserved here"
             );
         }
@@ -113,16 +120,14 @@ fn extract_with_capacity_or_reserve_target(cx: &LateContext<'_>, stmt: &'tcx Stm
                     // self.vec = Vec::with_capacity()
                     Some(LocalOrExpr::Expr(lhs))
                 },
-                ExprKind::MethodCall(_, _, [vec_expr, _], _) => {
+                ExprKind::MethodCall(path, _, [self_expr, _], _) => {
                     // self.vec.reserve()
-                    if_chain! {
-                        if let Some(id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
-                        if match_def_path(cx, id, &paths::VEC_RESERVE);
-                        then {
-                            Some(LocalOrExpr::Expr(vec_expr))
-                        } else {
-                            None
-                        }
+                    if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(self_expr).peel_refs(), sym::vec_type)
+                        && path.ident.name.as_str() == "reserve"
+                    {
+                        Some(LocalOrExpr::Expr(self_expr))
+                    } else {
+                        None
                     }
                 },
                 _ => None,
diff --git a/clippy_utils/src/lib.rs b/clippy_utils/src/lib.rs
index 00d2098a021..09eee78f0d1 100644
--- a/clippy_utils/src/lib.rs
+++ b/clippy_utils/src/lib.rs
@@ -1470,16 +1470,6 @@ pub fn is_self_ty(slf: &hir::Ty<'_>) -> bool {
     false
 }
 
-/// Checks if a given type looks safe to be uninitialized.
-pub fn is_uninit_ty_valid(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
-    match ty.kind() {
-        rustc_ty::Array(component, _) => is_uninit_ty_valid(cx, component),
-        rustc_ty::Tuple(types) => types.types().all(|ty| is_uninit_ty_valid(cx, ty)),
-        rustc_ty::Adt(adt, _) => match_def_path(cx, adt.did, &paths::MEM_MAYBEUNINIT),
-        _ => false,
-    }
-}
-
 pub fn iter_input_pats<'tcx>(decl: &FnDecl<'_>, body: &'tcx Body<'_>) -> impl Iterator<Item = &'tcx Param<'tcx>> {
     (0..decl.inputs.len()).map(move |i| &body.params[i])
 }
diff --git a/clippy_utils/src/paths.rs b/clippy_utils/src/paths.rs
index dffe30910dd..63ec10a3178 100644
--- a/clippy_utils/src/paths.rs
+++ b/clippy_utils/src/paths.rs
@@ -183,7 +183,6 @@ pub const VEC_AS_SLICE: [&str; 4] = ["alloc", "vec", "Vec", "as_slice"];
 pub const VEC_FROM_ELEM: [&str; 3] = ["alloc", "vec", "from_elem"];
 pub const VEC_NEW: [&str; 4] = ["alloc", "vec", "Vec", "new"];
 pub const VEC_RESIZE: [&str; 4] = ["alloc", "vec", "Vec", "resize"];
-pub const VEC_RESERVE: [&str; 4] = ["alloc", "vec", "Vec", "reserve"];
 pub const VEC_WITH_CAPACITY: [&str; 4] = ["alloc", "vec", "Vec", "with_capacity"];
 pub const VEC_SET_LEN: [&str; 4] = ["alloc", "vec", "Vec", "set_len"];
 pub const WEAK_ARC: [&str; 3] = ["alloc", "sync", "Weak"];
diff --git a/clippy_utils/src/ty.rs b/clippy_utils/src/ty.rs
index 6ebe1a0028a..ca64ac7de3e 100644
--- a/clippy_utils/src/ty.rs
+++ b/clippy_utils/src/ty.rs
@@ -367,3 +367,13 @@ pub fn same_type_and_consts(a: Ty<'tcx>, b: Ty<'tcx>) -> bool {
         _ => a == b,
     }
 }
+
+/// Checks if a given type looks safe to be uninitialized.
+pub fn is_uninit_value_valid_for_ty(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
+    match ty.kind() {
+        ty::Array(component, _) => is_uninit_value_valid_for_ty(cx, component),
+        ty::Tuple(types) => types.types().all(|ty| is_uninit_value_valid_for_ty(cx, ty)),
+        ty::Adt(adt, _) => cx.tcx.lang_items().maybe_uninit() == Some(adt.did),
+        _ => false,
+    }
+}
diff --git a/tests/ui/uninit_vec.rs b/tests/ui/uninit_vec.rs
index 5e67288405a..34b9e07ef1d 100644
--- a/tests/ui/uninit_vec.rs
+++ b/tests/ui/uninit_vec.rs
@@ -25,8 +25,14 @@ fn main() {
     }
 
     // MaybeUninit-wrapped types should not be detected
-    let mut vec: Vec<MaybeUninit<u8>> = Vec::with_capacity(1000);
     unsafe {
+        let mut vec: Vec<MaybeUninit<u8>> = Vec::with_capacity(1000);
+        vec.set_len(200);
+
+        let mut vec: Vec<(MaybeUninit<u8>, MaybeUninit<bool>)> = Vec::with_capacity(1000);
+        vec.set_len(200);
+
+        let mut vec: Vec<(MaybeUninit<u8>, [MaybeUninit<bool>; 2])> = Vec::with_capacity(1000);
         vec.set_len(200);
     }
 }