Mirrors/rust-clippy
2
0
Fork 0
mirror of https://github.com/rust-lang/rust-clippy synced 2024-11-24 13:43:17 +00:00
Code Issues Projects Releases Packages Wiki Activity

Extract get_vec_init_kind and share it

Browse source
This commit is contained in:
Yechan Bae 2021-10-03 00:23:57 -04:00
parent fec20bf617
commit dd9c8d32f2
4 changed files with 105 additions and 112 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

116
clippy_lints/src/uninit_vec.rs
View file

@ -1,24 +1,24 @@
use clippy_utils::diagnostics::span_lint_and_then;
use clippy_utils::get_vec_init_kind;
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 clippy_utils::{path_to_local_id, peel_hir_expr_while, ty::is_uninit_value_valid_for_ty, SpanlessEq};
use rustc_hir::{Block, Expr, ExprKind, HirId, PatKind, PathSegment, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass};
use rustc_middle::ty;
use rustc_session::{declare_lint_pass, declare_tool_lint};
use rustc_span::{sym, Span};
// TODO: add `ReadBuf` (RFC 2930) in "How to fix" once it is available in std
declare_clippy_lint! {
/// ### What it does
/// Checks for the creation of uninitialized `Vec<T>` by calling `set_len()`
/// immediately after `with_capacity()` or `reserve()`.
/// Checks for `set_len()` call that creates `Vec` with uninitialized elements.
/// This is commonly caused by calling `set_len()` right after after calling
/// `with_capacity()` or `reserve()`.
///
/// ### Why is this bad?
/// It creates `Vec<T>` that contains uninitialized data, which leads to an
/// It creates a `Vec` with uninitialized data, which leads to an
/// undefined behavior with most safe operations.
/// Notably, using uninitialized `Vec<u8>` with generic `Read` is unsound.
/// Notably, uninitialized `Vec<u8>` must not be used with generic `Read`.
///
/// ### Example
/// ```rust,ignore
@ -26,16 +26,25 @@ declare_clippy_lint! {
/// unsafe { vec.set_len(1000); }
/// reader.read(&mut vec); // undefined behavior!
/// ```
/// Use an initialized buffer:
/// ```rust,ignore
/// let mut vec: Vec<u8> = vec![0; 1000];
/// reader.read(&mut vec);
/// ```
/// Or, wrap the content in `MaybeUninit`:
/// ```rust,ignore
/// let mut vec: Vec<MaybeUninit<T>> = Vec::with_capacity(1000);
/// unsafe { vec.set_len(1000); }
/// ```
///
/// ### How to fix?
/// 1. Use an initialized buffer:
/// ```rust,ignore
/// let mut vec: Vec<u8> = vec![0; 1000];
/// reader.read(&mut vec);
/// ```
/// 2. Wrap the content in `MaybeUninit`:
/// ```rust,ignore
/// let mut vec: Vec<MaybeUninit<T>> = Vec::with_capacity(1000);
/// vec.set_len(1000); // `MaybeUninit` can be uninitialized
/// ```
/// 3. If you are on nightly, `Vec::spare_capacity_mut()` is available:
/// ```rust,ignore
/// let mut vec: Vec<u8> = Vec::with_capacity(1000);
/// let remaining = vec.spare_capacity_mut(); // `&mut [MaybeUninit<u8>]`
/// // perform initialization with `remaining`
/// vec.set_len(...); // Safe to call `set_len()` on initialized part
/// ```
pub UNINIT_VEC,
correctness,
"Vec with uninitialized data"
@ -59,11 +68,11 @@ impl<'tcx> LateLintPass<'tcx> for UninitVec {
fn handle_uninit_vec_pair(
cx: &LateContext<'tcx>,
maybe_with_capacity_or_reserve: &'tcx Stmt<'tcx>,
maybe_init_or_reserve: &'tcx Stmt<'tcx>,
maybe_set_len: &'tcx Expr<'tcx>,
) {
if_chain! {
if let Some(vec) = extract_with_capacity_or_reserve_target(cx, maybe_with_capacity_or_reserve);
if let Some(vec) = extract_init_or_reserve_target(cx, maybe_init_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();
@ -71,12 +80,12 @@ fn handle_uninit_vec_pair(
// Check T of Vec<T>
if !is_uninit_value_valid_for_ty(cx, substs.type_at(0));
then {
// FIXME: false positive #7698
// FIXME: #7698, false positive of the internal lints
#[allow(clippy::collapsible_span_lint_calls)]
span_lint_and_then(
cx,
UNINIT_VEC,
vec![call_span, maybe_with_capacity_or_reserve.span],
vec![call_span, maybe_init_or_reserve.span],
"calling `set_len()` immediately after reserving a buffer creates uninitialized values",
|diag| {
diag.help("initialize the buffer or wrap the content in `MaybeUninit`");
@ -101,15 +110,15 @@ impl<'tcx> LocalOrExpr<'tcx> {
}
}
/// Returns the target vec of `Vec::with_capacity()` or `Vec::reserve()`
fn extract_with_capacity_or_reserve_target(cx: &LateContext<'_>, stmt: &'tcx Stmt<'_>) -> Option<LocalOrExpr<'tcx>> {
/// Finds the target location where the result of `Vec` initialization is stored
/// or `self` expression for `Vec::reserve()`.
fn extract_init_or_reserve_target<'tcx>(cx: &LateContext<'tcx>, stmt: &'tcx Stmt<'tcx>) -> Option<LocalOrExpr<'tcx>> {
match stmt.kind {
StmtKind::Local(local) => {
// let mut x = Vec::with_capacity()
if_chain! {
if let Some(init_expr) = local.init;
if let PatKind::Binding(_, hir_id, _, None) = local.pat.kind;
if is_with_capacity(cx, init_expr);
if get_vec_init_kind(cx, init_expr).is_some();
then {
Some(LocalOrExpr::Local(hir_id))
} else {
@ -117,40 +126,20 @@ fn extract_with_capacity_or_reserve_target(cx: &LateContext<'_>, stmt: &'tcx Stm
}
}
},
StmtKind::Expr(expr) | StmtKind::Semi(expr) => {
match expr.kind {
ExprKind::Assign(lhs, rhs, _span) if is_with_capacity(cx, rhs) => {
// self.vec = Vec::with_capacity()
Some(LocalOrExpr::Expr(lhs))
},
ExprKind::MethodCall(path, _, [self_expr, _], _) => {
// self.vec.reserve()
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,
}
StmtKind::Expr(expr) | StmtKind::Semi(expr) => match expr.kind {
ExprKind::Assign(lhs, rhs, _span) if get_vec_init_kind(cx, rhs).is_some() => Some(LocalOrExpr::Expr(lhs)),
ExprKind::MethodCall(path, _, [self_expr, _], _) if is_reserve(cx, path, self_expr) => {
Some(LocalOrExpr::Expr(self_expr))
},
_ => None,
},
StmtKind::Item(_) => None,
}
}
fn is_with_capacity(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> bool {
if_chain! {
if let ExprKind::Call(path_expr, _) = &expr.kind;
if let ExprKind::Path(qpath) = &path_expr.kind;
if let Res::Def(_, def_id) = cx.qpath_res(qpath, path_expr.hir_id);
then {
match_def_path(cx, def_id, &paths::VEC_WITH_CAPACITY)
} else {
false
}
}
fn is_reserve(cx: &LateContext<'_>, path: &PathSegment<'_>, self_expr: &Expr<'_>) -> bool {
is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(self_expr).peel_refs(), sym::Vec)
&& path.ident.name.as_str() == "reserve"
}
/// Returns self if the expression is `Vec::set_len()`
@ -169,14 +158,13 @@ fn extract_set_len_self(cx: &LateContext<'_>, expr: &'tcx Expr<'_>) -> Option<(&
}
});
match expr.kind {
ExprKind::MethodCall(_, _, [vec_expr, _], _) => {
cx.typeck_results().type_dependent_def_id(expr.hir_id).and_then(|id| {
if match_def_path(cx, id, &paths::VEC_SET_LEN) {
Some((vec_expr, expr.span))
} else {
None
}
})
ExprKind::MethodCall(path, _, [self_expr, _], _) => {
let self_type = cx.typeck_results().expr_ty(self_expr).peel_refs();
if is_type_diagnostic_item(cx, self_type, sym::Vec) && path.ident.name.as_str() == "set_len" {
Some((self_expr, expr.span))
} else {
None
}
},
_ => None,
}

51
clippy_lints/src/vec_init_then_push.rs
View file

@ -1,16 +1,13 @@
use clippy_utils::diagnostics::span_lint_and_sugg;
use clippy_utils::source::snippet;
use clippy_utils::ty::is_type_diagnostic_item;
use clippy_utils::{match_def_path, path_to_local, path_to_local_id, paths};
use clippy_utils::{get_vec_init_kind, path_to_local, path_to_local_id, VecInitKind};
use if_chain::if_chain;
use rustc_ast::ast::LitKind;
use rustc_errors::Applicability;
use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, HirId, Local, PatKind, QPath, Stmt, StmtKind};
use rustc_hir::{BindingAnnotation, Block, Expr, ExprKind, HirId, Local, PatKind, Stmt, StmtKind};
use rustc_lint::{LateContext, LateLintPass, LintContext};
use rustc_middle::lint::in_external_macro;
use rustc_session::{declare_tool_lint, impl_lint_pass};
use rustc_span::{symbol::sym, Span};
use std::convert::TryInto;
use rustc_span::Span;
declare_clippy_lint! {
/// ### What it does
@ -41,11 +38,6 @@ pub struct VecInitThenPush {
searcher: Option<VecPushSearcher>,
}
#[derive(Clone, Copy)]
enum VecInitKind {
New,
WithCapacity(u64),
}
struct VecPushSearcher {
local_id: HirId,
init: VecInitKind,
@ -58,7 +50,8 @@ impl VecPushSearcher {
fn display_err(&self, cx: &LateContext<'_>) {
match self.init {
_ if self.found == 0 => return,
VecInitKind::WithCapacity(x) if x > self.found => return,
VecInitKind::WithLiteralCapacity(x) if x > self.found => return,
VecInitKind::WithExprCapacity(_) => return,
_ => (),
};
@ -152,37 +145,3 @@ impl LateLintPass<'_> for VecInitThenPush {
}
}
}
fn get_vec_init_kind<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<VecInitKind> {
if let ExprKind::Call(func, args) = expr.kind {
match func.kind {
ExprKind::Path(QPath::TypeRelative(ty, name))
if is_type_diagnostic_item(cx, cx.typeck_results().node_type(ty.hir_id), sym::Vec) =>
{
if name.ident.name == sym::new {
return Some(VecInitKind::New);
} else if name.ident.name.as_str() == "with_capacity" {
return args.get(0).and_then(|arg| {
if_chain! {
if let ExprKind::Lit(lit) = &arg.kind;
if let LitKind::Int(num, _) = lit.node;
then {
Some(VecInitKind::WithCapacity(num.try_into().ok()?))
} else {
None
}
}
});
}
}
ExprKind::Path(QPath::Resolved(_, path))
if match_def_path(cx, path.res.opt_def_id()?, &paths::DEFAULT_TRAIT_METHOD)
&& is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::Vec) =>
{
return Some(VecInitKind::New);
}
_ => (),
}
}
None
}

49
clippy_utils/src/lib.rs
View file

@ -93,7 +93,7 @@ use rustc_span::{Span, DUMMY_SP};
use rustc_target::abi::Integer;
use crate::consts::{constant, Constant};
use crate::ty::{can_partially_move_ty, is_copy, is_recursively_primitive_type};
use crate::ty::{can_partially_move_ty, is_copy, is_recursively_primitive_type, is_type_diagnostic_item};
pub fn parse_msrv(msrv: &str, sess: Option<&Session>, span: Option<Span>) -> Option<RustcVersion> {
if let Ok(version) = RustcVersion::parse(msrv) {
@ -1789,6 +1789,53 @@ pub fn is_expr_identity_function(cx: &LateContext<'_>, expr: &Expr<'_>) -> bool
}
}
#[derive(Clone, Copy)]
pub enum VecInitKind {
/// `Vec::new()`
New,
/// `Vec::default()` or `Default::default()`
Default,
/// `Vec::with_capacity(123)`
WithLiteralCapacity(u64),
/// `Vec::with_capacity(slice.len())`
WithExprCapacity(HirId),
}
/// Checks if given expression is an initialization of `Vec` and returns its kind.
pub fn get_vec_init_kind<'tcx>(cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) -> Option<VecInitKind> {
if let ExprKind::Call(func, args) = expr.kind {
match func.kind {
ExprKind::Path(QPath::TypeRelative(ty, name))
if is_type_diagnostic_item(cx, cx.typeck_results().node_type(ty.hir_id), sym::Vec) =>
{
if name.ident.name == sym::new {
return Some(VecInitKind::New);
} else if name.ident.name.as_str() == "with_capacity" {
return args.get(0).and_then(|arg| {
if_chain! {
if let ExprKind::Lit(lit) = &arg.kind;
if let LitKind::Int(num, _) = lit.node;
then {
Some(VecInitKind::WithLiteralCapacity(num.try_into().ok()?))
} else {
Some(VecInitKind::WithExprCapacity(arg.hir_id))
}
}
});
}
}
ExprKind::Path(QPath::Resolved(_, path))
if match_def_path(cx, path.res.opt_def_id()?, &paths::DEFAULT_TRAIT_METHOD)
&& is_type_diagnostic_item(cx, cx.typeck_results().expr_ty(expr), sym::Vec) =>
{
return Some(VecInitKind::Default);
}
_ => (),
}
}
None
}
/// Gets the node where an expression is either used, or it's type is unified with another branch.
pub fn get_expr_use_or_unification_node(tcx: TyCtxt<'tcx>, expr: &Expr<'_>) -> Option<Node<'tcx>> {
let mut child_id = expr.hir_id;

1
clippy_utils/src/paths.rs
View file

@ -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_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"];
pub const WEAK_RC: [&str; 3] = ["alloc", "rc", "Weak"];