Skip to content

Commit f8d170b

Browse files
committed
Handle PR feedbacks first round
1 parent 36526b7 commit f8d170b

File tree

6 files changed

+42
-32
lines changed

6 files changed

+42
-32
lines changed

clippy_lints/src/methods/uninit_assumed_init.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
use clippy_utils::diagnostics::span_lint;
2-
use clippy_utils::{is_expr_path_def_path, is_uninit_ty_valid, paths};
2+
use clippy_utils::{is_expr_path_def_path, paths, ty::is_uninit_value_valid_for_ty};
33
use if_chain::if_chain;
44
use rustc_hir as hir;
55
use rustc_lint::LateContext;
@@ -12,7 +12,7 @@ pub(super) fn check(cx: &LateContext<'_>, expr: &hir::Expr<'_>, recv: &hir::Expr
1212
if let hir::ExprKind::Call(callee, args) = recv.kind;
1313
if args.is_empty();
1414
if is_expr_path_def_path(cx, callee, &paths::MEM_MAYBEUNINIT_UNINIT);
15-
if !is_uninit_ty_valid(cx, cx.typeck_results().expr_ty_adjusted(expr));
15+
if !is_uninit_value_valid_for_ty(cx, cx.typeck_results().expr_ty_adjusted(expr));
1616
then {
1717
span_lint(
1818
cx,

clippy_lints/src/uninit_vec.rs

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
use clippy_utils::diagnostics::span_lint_and_note;
2-
use clippy_utils::{is_uninit_ty_valid, match_def_path, path_to_local_id, paths, peel_hir_expr_while, SpanlessEq};
2+
use clippy_utils::ty::is_type_diagnostic_item;
3+
use clippy_utils::{
4+
match_def_path, path_to_local_id, paths, peel_hir_expr_while, ty::is_uninit_value_valid_for_ty, SpanlessEq,
5+
};
36
use rustc_hir::def::Res;
47
use rustc_hir::{Block, Expr, ExprKind, HirId, PatKind, Stmt, StmtKind};
58
use rustc_lint::{LateContext, LateLintPass};
69
use rustc_middle::ty;
710
use rustc_session::{declare_lint_pass, declare_tool_lint};
8-
use rustc_span::Span;
11+
use rustc_span::{sym, Span};
912

1013
declare_clippy_lint! {
1114
/// ### What it does
@@ -44,32 +47,36 @@ impl<'tcx> LateLintPass<'tcx> for UninitVec {
4447
fn check_block(&mut self, cx: &LateContext<'tcx>, block: &'tcx Block<'_>) {
4548
for w in block.stmts.windows(2) {
4649
if let StmtKind::Expr(expr) | StmtKind::Semi(expr) = w[1].kind {
47-
handle_pair(cx, &w[0], expr);
50+
handle_uninit_vec_pair(cx, &w[0], expr);
4851
}
4952
}
5053

5154
if let (Some(stmt), Some(expr)) = (block.stmts.last(), block.expr) {
52-
handle_pair(cx, stmt, expr);
55+
handle_uninit_vec_pair(cx, stmt, expr);
5356
}
5457
}
5558
}
5659

57-
fn handle_pair(cx: &LateContext<'tcx>, first: &'tcx Stmt<'tcx>, second: &'tcx Expr<'tcx>) {
60+
fn handle_uninit_vec_pair(
61+
cx: &LateContext<'tcx>,
62+
maybe_with_capacity_or_reserve: &'tcx Stmt<'tcx>,
63+
maybe_set_len: &'tcx Expr<'tcx>,
64+
) {
5865
if_chain! {
59-
if let Some(vec) = extract_with_capacity_or_reserve_target(cx, first);
60-
if let Some((set_len_self, call_span)) = extract_set_len_self(cx, second);
66+
if let Some(vec) = extract_with_capacity_or_reserve_target(cx, maybe_with_capacity_or_reserve);
67+
if let Some((set_len_self, call_span)) = extract_set_len_self(cx, maybe_set_len);
6168
if vec.eq_expr(cx, set_len_self);
6269
if let ty::Ref(_, vec_ty, _) = cx.typeck_results().expr_ty_adjusted(set_len_self).kind();
6370
if let ty::Adt(_, substs) = vec_ty.kind();
6471
// Check T of Vec<T>
65-
if !is_uninit_ty_valid(cx, substs.type_at(0));
72+
if !is_uninit_value_valid_for_ty(cx, substs.type_at(0));
6673
then {
6774
span_lint_and_note(
6875
cx,
6976
UNINIT_VEC,
7077
call_span,
7178
"calling `set_len()` immediately after reserving a buffer creates uninitialized values",
72-
Some(first.span),
79+
Some(maybe_with_capacity_or_reserve.span),
7380
"the buffer is reserved here"
7481
);
7582
}
@@ -113,16 +120,14 @@ fn extract_with_capacity_or_reserve_target(cx: &LateContext<'_>, stmt: &'tcx Stm
113120
// self.vec = Vec::with_capacity()
114121
Some(LocalOrExpr::Expr(lhs))
115122
},
116-
ExprKind::MethodCall(_, _, [vec_expr, _], _) => {
123+
ExprKind::MethodCall(path, _, [self_expr, _], _) => {
117124
// self.vec.reserve()
118-
if_chain! {
119-
if let Some(id) = cx.typeck_results().type_dependent_def_id(expr.hir_id);
120-
if match_def_path(cx, id, &paths::VEC_RESERVE);
121-
then {
122-
Some(LocalOrExpr::Expr(vec_expr))
123-
} else {
124-
None
125-
}
125+
if is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(self_expr).peel_refs(), sym::vec_type)
126+
&& path.ident.name.as_str() == "reserve"
127+
{
128+
Some(LocalOrExpr::Expr(self_expr))
129+
} else {
130+
None
126131
}
127132
},
128133
_ => None,

clippy_utils/src/lib.rs

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1473,16 +1473,6 @@ pub fn is_self_ty(slf: &hir::Ty<'_>) -> bool {
14731473
false
14741474
}
14751475

1476-
/// Checks if a given type looks safe to be uninitialized.
1477-
pub fn is_uninit_ty_valid(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
1478-
match ty.kind() {
1479-
rustc_ty::Array(component, _) => is_uninit_ty_valid(cx, component),
1480-
rustc_ty::Tuple(types) => types.types().all(|ty| is_uninit_ty_valid(cx, ty)),
1481-
rustc_ty::Adt(adt, _) => match_def_path(cx, adt.did, &paths::MEM_MAYBEUNINIT),
1482-
_ => false,
1483-
}
1484-
}
1485-
14861476
pub fn iter_input_pats<'tcx>(decl: &FnDecl<'_>, body: &'tcx Body<'_>) -> impl Iterator<Item = &'tcx Param<'tcx>> {
14871477
(0..decl.inputs.len()).map(move |i| &body.params[i])
14881478
}

clippy_utils/src/paths.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,6 @@ pub const VEC_AS_SLICE: [&str; 4] = ["alloc", "vec", "Vec", "as_slice"];
176176
pub const VEC_FROM_ELEM: [&str; 3] = ["alloc", "vec", "from_elem"];
177177
pub const VEC_NEW: [&str; 4] = ["alloc", "vec", "Vec", "new"];
178178
pub const VEC_RESIZE: [&str; 4] = ["alloc", "vec", "Vec", "resize"];
179-
pub const VEC_RESERVE: [&str; 4] = ["alloc", "vec", "Vec", "reserve"];
180179
pub const VEC_WITH_CAPACITY: [&str; 4] = ["alloc", "vec", "Vec", "with_capacity"];
181180
pub const VEC_SET_LEN: [&str; 4] = ["alloc", "vec", "Vec", "set_len"];
182181
pub const WEAK_ARC: [&str; 3] = ["alloc", "sync", "Weak"];

clippy_utils/src/ty.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -367,3 +367,13 @@ pub fn same_type_and_consts(a: Ty<'tcx>, b: Ty<'tcx>) -> bool {
367367
_ => a == b,
368368
}
369369
}
370+
371+
/// Checks if a given type looks safe to be uninitialized.
372+
pub fn is_uninit_value_valid_for_ty(cx: &LateContext<'_>, ty: Ty<'_>) -> bool {
373+
match ty.kind() {
374+
ty::Array(component, _) => is_uninit_value_valid_for_ty(cx, component),
375+
ty::Tuple(types) => types.types().all(|ty| is_uninit_value_valid_for_ty(cx, ty)),
376+
ty::Adt(adt, _) => cx.tcx.lang_items().maybe_uninit() == Some(adt.did),
377+
_ => false,
378+
}
379+
}

tests/ui/uninit_vec.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,14 @@ fn main() {
2525
}
2626

2727
// MaybeUninit-wrapped types should not be detected
28-
let mut vec: Vec<MaybeUninit<u8>> = Vec::with_capacity(1000);
2928
unsafe {
29+
let mut vec: Vec<MaybeUninit<u8>> = Vec::with_capacity(1000);
30+
vec.set_len(200);
31+
32+
let mut vec: Vec<(MaybeUninit<u8>, MaybeUninit<bool>)> = Vec::with_capacity(1000);
33+
vec.set_len(200);
34+
35+
let mut vec: Vec<(MaybeUninit<u8>, [MaybeUninit<bool>; 2])> = Vec::with_capacity(1000);
3036
vec.set_len(200);
3137
}
3238
}

0 commit comments

Comments
 (0)