Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Jun 23, 2020
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
82 changes: 54 additions & 28 deletions clippy_lints/src/mem_replace.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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",
);
}
}
}
Expand Down Expand Up @@ -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);
}
}
Expand Down
6 changes: 6 additions & 0 deletions tests/ui/repl_uninit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
19 changes: 11 additions & 8 deletions tests/ui/repl_uninit.stderr
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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