Auto merge of #5695 - esamudera:lint_mem_uninitialized, r=phansch,oli…

…-obk New lint: suggest `ptr::read` instead of `mem::replace(..., uninitialized())` resolves: #5575 changelog: improvements to `MEM_REPLACE_WITH_UNINIT`: - add a new test case in `tests/ui/repl_uninit.rs` to cover the case of replacing with `mem::MaybeUninit::uninit().assume_init()`. - modify the existing `MEM_REPLACE_WITH_UNINIT` when replacing with `mem::uninitialized` to suggest using `ptr::read` instead. - lint with `MEM_REPLACE_WITH_UNINIT` when replacing with `mem::MaybeUninit::uninit().assume_init()`
rust-lang · Jun 23, 2020 · c56c7e2 · c56c7e2
2 parents fa0f6a8 + 8c1ee06
commit c56c7e2
Show file tree

Hide file tree

Showing 3 changed files with 71 additions and 36 deletions.
diff --git 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
@@ -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
@@ -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