From 8c1ee063bb67b20ac17603d0f0025b48b958cc08 Mon Sep 17 00:00:00 2001
From: Ericko Samudera <ericko.samudera@gmail.com>
Date: Mon, 8 Jun 2020 00:44:14 +0700
Subject: [PATCH] mem_replace_with_uninit: suggest std::ptr::read
---
clippy_lints/src/mem_replace.rs | 82 ++++++++++++++++++++++-----------
tests/ui/repl_uninit.rs | 6 +++
tests/ui/repl_uninit.stderr | 19 ++++----
3 files changed, 71 insertions(+), 36 deletions(-)
diff --git a/clippy_lints/src/mem_replace.rs b/clippy_lints/src/mem_replace.rs
index ab6865bf0f3b..e2672e02b36d 100644
--- a/clippy_lints/src/mem_replace.rs
+++ b/clippy_lints/src/mem_replace.rs
@@ -135,33 +135,59 @@ fn check_replace_option_with_none(cx: &LateContext<'_, '_>, src: &Expr<'_>, dest
}
}
-fn check_replace_with_uninit(cx: &LateContext<'_, '_>, src: &Expr<'_>, expr_span: Span) {
- if let ExprKind::Call(ref repl_func, ref repl_args) = src.kind {
- if_chain! {
- if repl_args.is_empty();
- if let ExprKind::Path(ref repl_func_qpath) = repl_func.kind;
- if let Some(repl_def_id) = cx.tables.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id();
- then {
- if cx.tcx.is_diagnostic_item(sym::mem_uninitialized, repl_def_id) {
- span_lint_and_help(
- cx,
- MEM_REPLACE_WITH_UNINIT,
- expr_span,
- "replacing with `mem::uninitialized()`",
- None,
- "consider using the `take_mut` crate instead",
- );
- } else if cx.tcx.is_diagnostic_item(sym::mem_zeroed, repl_def_id) &&
- !cx.tables.expr_ty(src).is_primitive() {
- span_lint_and_help(
- cx,
- MEM_REPLACE_WITH_UNINIT,
- expr_span,
- "replacing with `mem::zeroed()`",
- None,
- "consider using a default value or the `take_mut` crate instead",
- );
- }
+fn check_replace_with_uninit(cx: &LateContext<'_, '_>, src: &Expr<'_>, dest: &Expr<'_>, expr_span: Span) {
+ if_chain! {
+ // check if replacement is mem::MaybeUninit::uninit().assume_init()
+ if let Some(method_def_id) = cx.tables.type_dependent_def_id(src.hir_id);
+ if cx.tcx.is_diagnostic_item(sym::assume_init, method_def_id);
+ then {
+ let mut applicability = Applicability::MachineApplicable;
+ span_lint_and_sugg(
+ cx,
+ MEM_REPLACE_WITH_UNINIT,
+ expr_span,
+ "replacing with `mem::MaybeUninit::uninit().assume_init()`",
+ "consider using",
+ format!(
+ "std::ptr::read({})",
+ snippet_with_applicability(cx, dest.span, "", &mut applicability)
+ ),
+ applicability,
+ );
+ return;
+ }
+ }
+
+ if_chain! {
+ if let ExprKind::Call(ref repl_func, ref repl_args) = src.kind;
+ if repl_args.is_empty();
+ if let ExprKind::Path(ref repl_func_qpath) = repl_func.kind;
+ if let Some(repl_def_id) = cx.tables.qpath_res(repl_func_qpath, repl_func.hir_id).opt_def_id();
+ then {
+ if cx.tcx.is_diagnostic_item(sym::mem_uninitialized, repl_def_id) {
+ let mut applicability = Applicability::MachineApplicable;
+ span_lint_and_sugg(
+ cx,
+ MEM_REPLACE_WITH_UNINIT,
+ expr_span,
+ "replacing with `mem::uninitialized()`",
+ "consider using",
+ format!(
+ "std::ptr::read({})",
+ snippet_with_applicability(cx, dest.span, "", &mut applicability)
+ ),
+ applicability,
+ );
+ } else if cx.tcx.is_diagnostic_item(sym::mem_zeroed, repl_def_id) &&
+ !cx.tables.expr_ty(src).is_primitive() {
+ span_lint_and_help(
+ cx,
+ MEM_REPLACE_WITH_UNINIT,
+ expr_span,
+ "replacing with `mem::zeroed()`",
+ None,
+ "consider using a default value or the `take_mut` crate instead",
+ );
}
}
}
@@ -209,7 +235,7 @@ impl<'a, 'tcx> LateLintPass<'a, 'tcx> for MemReplace {
if let [dest, src] = &**func_args;
then {
check_replace_option_with_none(cx, src, dest, expr.span);
- check_replace_with_uninit(cx, src, expr.span);
+ check_replace_with_uninit(cx, src, dest, expr.span);
check_replace_with_default(cx, src, dest, expr.span);
}
}
diff --git a/tests/ui/repl_uninit.rs b/tests/ui/repl_uninit.rs
index 346972b7bb4e..ad5b8e4857d1 100644
--- a/tests/ui/repl_uninit.rs
+++ b/tests/ui/repl_uninit.rs
@@ -17,6 +17,12 @@ fn main() {
std::mem::forget(mem::replace(&mut v, new_v));
}
+ unsafe {
+ let taken_v = mem::replace(&mut v, mem::MaybeUninit::uninit().assume_init());
+ let new_v = might_panic(taken_v);
+ std::mem::forget(mem::replace(&mut v, new_v));
+ }
+
unsafe {
let taken_v = mem::replace(&mut v, mem::zeroed());
let new_v = might_panic(taken_v);
diff --git a/tests/ui/repl_uninit.stderr b/tests/ui/repl_uninit.stderr
index c1f55d7601e5..09468eeaea4b 100644
--- a/tests/ui/repl_uninit.stderr
+++ b/tests/ui/repl_uninit.stderr
@@ -2,26 +2,29 @@ error: replacing with `mem::uninitialized()`
--> $DIR/repl_uninit.rs:15:23
|
LL | let taken_v = mem::replace(&mut v, mem::uninitialized());
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::ptr::read(&mut v)`
|
= note: `-D clippy::mem-replace-with-uninit` implied by `-D warnings`
- = help: consider using the `take_mut` crate instead
-error: replacing with `mem::zeroed()`
+error: replacing with `mem::MaybeUninit::uninit().assume_init()`
--> $DIR/repl_uninit.rs:21:23
|
+LL | let taken_v = mem::replace(&mut v, mem::MaybeUninit::uninit().assume_init());
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::ptr::read(&mut v)`
+
+error: replacing with `mem::zeroed()`
+ --> $DIR/repl_uninit.rs:27:23
+ |
LL | let taken_v = mem::replace(&mut v, mem::zeroed());
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= help: consider using a default value or the `take_mut` crate instead
error: replacing with `mem::uninitialized()`
- --> $DIR/repl_uninit.rs:33:28
+ --> $DIR/repl_uninit.rs:39:28
|
LL | let taken_u = unsafe { mem::replace(uref, mem::uninitialized()) };
- | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
- |
- = help: consider using the `take_mut` crate instead
+ | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ help: consider using: `std::ptr::read(uref)`
-error: aborting due to 3 previous errors
+error: aborting due to 4 previous errors