Auto merge of rust-lang#5695 - esamudera:lint_mem_uninitialized, r=phansch,oli-obk

New lint: suggest `ptr::read` instead of `mem::replace(..., uninitialized())`

resolves: rust-lang#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()`

Author: bors

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);
             }
         }

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);

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