From 1ef9c163aa37417e7596d884e912ffea1fb40932 Mon Sep 17 00:00:00 2001 From: Urgau Date: Tue, 28 Mar 2023 18:31:21 +0200 Subject: [PATCH] Uplift clippy::forget_ref to rustc --- compiler/rustc_lint/messages.ftl | 3 + .../rustc_lint/src/drop_forget_useless.rs | 30 ++++- compiler/rustc_lint/src/lints.rs | 8 ++ tests/ui/lint/forget_ref.rs | 39 ++++++ tests/ui/lint/forget_ref.stderr | 115 ++++++++++++++++++ 5 files changed, 193 insertions(+), 2 deletions(-) create mode 100644 tests/ui/lint/forget_ref.rs create mode 100644 tests/ui/lint/forget_ref.stderr diff --git a/compiler/rustc_lint/messages.ftl b/compiler/rustc_lint/messages.ftl index d52b7777690..a570f03e28c 100644 --- a/compiler/rustc_lint/messages.ftl +++ b/compiler/rustc_lint/messages.ftl @@ -526,3 +526,6 @@ lint_drop_ref = calls to `std::mem::drop` with a reference instead of an owned v lint_drop_copy = calls to `std::mem::drop` with a value that implements `Copy`. .note = argument has type `{$arg_ty}` + +lint_forget_ref = calls to `std::mem::forget` with a reference instead of an owned value + .note = argument has type `{$arg_ty}` diff --git a/compiler/rustc_lint/src/drop_forget_useless.rs b/compiler/rustc_lint/src/drop_forget_useless.rs index 9b667a50fe1..32594178fe8 100644 --- a/compiler/rustc_lint/src/drop_forget_useless.rs +++ b/compiler/rustc_lint/src/drop_forget_useless.rs @@ -1,7 +1,7 @@ use rustc_hir::{Arm, Expr, ExprKind, Node}; use rustc_span::sym; -use crate::{lints::{DropRefDiag, DropCopyDiag}, LateContext, LateLintPass, LintContext}; +use crate::{lints::{DropRefDiag, DropCopyDiag, ForgetRefDiag}, LateContext, LateLintPass, LintContext}; declare_lint! { /// The `drop_ref` lint checks for calls to `std::mem::drop` with a reference @@ -31,6 +31,29 @@ declare_lint! { "calls to `std::mem::drop` with a reference instead of an owned value" } +declare_lint! { + /// The `forget_ref` lint checks for calls to `std::mem::forget` with a reference + /// instead of an owned value. + /// + /// ### Example + /// + /// ```rust + /// let x = Box::new(1); + /// std::mem::forget(&x); // Should have been forget(x), x will still be dropped + /// ``` + /// + /// {{produces}} + /// + /// ### Explanation + /// + /// Calling `forget` on a reference will only forget the + /// reference itself, which is a no-op. It will not forget the underlying + /// referenced value, which is likely what was intended. + pub FORGET_REF, + Warn, + "calls to `std::mem::forget` with a reference instead of an owned value" +} + declare_lint! { /// The `drop_copy` lint checks for calls to `std::mem::drop` with a value /// that derives the Copy trait. @@ -55,7 +78,7 @@ declare_lint! { "calls to `std::mem::drop` with a value that implements Copy" } -declare_lint_pass!(DropForgetUseless => [DROP_REF, DROP_COPY]); +declare_lint_pass!(DropForgetUseless => [DROP_REF, FORGET_REF, DROP_COPY]); impl<'tcx> LateLintPass<'tcx> for DropForgetUseless { fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) { @@ -71,6 +94,9 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetUseless { sym::mem_drop if arg_ty.is_ref() && !drop_is_single_call_in_arm => { cx.emit_spanned_lint(DROP_REF, expr.span, DropRefDiag { arg_ty, note: arg.span }); }, + sym::mem_forget if arg_ty.is_ref() => { + cx.emit_spanned_lint(FORGET_REF, expr.span, ForgetRefDiag { arg_ty, note: arg.span }); + }, sym::mem_drop if is_copy && !drop_is_single_call_in_arm => { cx.emit_spanned_lint(DROP_COPY, expr.span, DropCopyDiag { arg_ty, note: arg.span }); } diff --git a/compiler/rustc_lint/src/lints.rs b/compiler/rustc_lint/src/lints.rs index a91e20eb218..2431c3707f5 100644 --- a/compiler/rustc_lint/src/lints.rs +++ b/compiler/rustc_lint/src/lints.rs @@ -679,6 +679,14 @@ pub struct DropCopyDiag<'a> { pub note: Span, } +#[derive(LintDiagnostic)] +#[diag(lint_forget_ref)] +pub struct ForgetRefDiag<'a> { + pub arg_ty: Ty<'a>, + #[note] + pub note: Span, +} + // hidden_unicode_codepoints.rs #[derive(LintDiagnostic)] #[diag(lint_hidden_unicode_codepoints)] diff --git a/tests/ui/lint/forget_ref.rs b/tests/ui/lint/forget_ref.rs new file mode 100644 index 00000000000..13f6d4be3d1 --- /dev/null +++ b/tests/ui/lint/forget_ref.rs @@ -0,0 +1,39 @@ +// check-pass + +#![warn(forget_ref)] + +use std::mem::forget; + +struct SomeStruct; + +fn main() { + forget(&SomeStruct); //~ WARN calls to `std::mem::forget` + + let mut owned = SomeStruct; + forget(&owned); //~ WARN calls to `std::mem::forget` + forget(&&owned); //~ WARN calls to `std::mem::forget` + forget(&mut owned); //~ WARN calls to `std::mem::forget` + forget(owned); + + let reference1 = &SomeStruct; + forget(&*reference1); //~ WARN calls to `std::mem::forget` + + let reference2 = &mut SomeStruct; + forget(reference2); //~ WARN calls to `std::mem::forget` + + let ref reference3 = SomeStruct; + forget(reference3); //~ WARN calls to `std::mem::forget` +} + +#[allow(dead_code)] +fn test_generic_fn_forget(val: T) { + forget(&val); //~ WARN calls to `std::mem::forget` + forget(val); +} + +#[allow(dead_code)] +fn test_similarly_named_function() { + fn forget(_val: T) {} + forget(&SomeStruct); //OK; call to unrelated function which happens to have the same name + std::mem::forget(&SomeStruct); //~ WARN calls to `std::mem::forget` +} diff --git a/tests/ui/lint/forget_ref.stderr b/tests/ui/lint/forget_ref.stderr new file mode 100644 index 00000000000..abb45443e4e --- /dev/null +++ b/tests/ui/lint/forget_ref.stderr @@ -0,0 +1,115 @@ +warning: calls to `std::mem::forget` with a reference instead of an owned value + --> $DIR/forget_ref.rs:10:5 + | +LL | forget(&SomeStruct); + | ^^^^^^^^^^^^^^^^^^^ + | +note: argument has type `&SomeStruct` + --> $DIR/forget_ref.rs:10:12 + | +LL | forget(&SomeStruct); + | ^^^^^^^^^^^ +note: the lint level is defined here + --> $DIR/forget_ref.rs:3:9 + | +LL | #![warn(forget_ref)] + | ^^^^^^^^^^ + +warning: calls to `std::mem::forget` with a reference instead of an owned value + --> $DIR/forget_ref.rs:13:5 + | +LL | forget(&owned); + | ^^^^^^^^^^^^^^ + | +note: argument has type `&SomeStruct` + --> $DIR/forget_ref.rs:13:12 + | +LL | forget(&owned); + | ^^^^^^ + +warning: calls to `std::mem::forget` with a reference instead of an owned value + --> $DIR/forget_ref.rs:14:5 + | +LL | forget(&&owned); + | ^^^^^^^^^^^^^^^ + | +note: argument has type `&&SomeStruct` + --> $DIR/forget_ref.rs:14:12 + | +LL | forget(&&owned); + | ^^^^^^^ + +warning: calls to `std::mem::forget` with a reference instead of an owned value + --> $DIR/forget_ref.rs:15:5 + | +LL | forget(&mut owned); + | ^^^^^^^^^^^^^^^^^^ + | +note: argument has type `&mut SomeStruct` + --> $DIR/forget_ref.rs:15:12 + | +LL | forget(&mut owned); + | ^^^^^^^^^^ + +warning: calls to `std::mem::forget` with a reference instead of an owned value + --> $DIR/forget_ref.rs:19:5 + | +LL | forget(&*reference1); + | ^^^^^^^^^^^^^^^^^^^^ + | +note: argument has type `&SomeStruct` + --> $DIR/forget_ref.rs:19:12 + | +LL | forget(&*reference1); + | ^^^^^^^^^^^^ + +warning: calls to `std::mem::forget` with a reference instead of an owned value + --> $DIR/forget_ref.rs:22:5 + | +LL | forget(reference2); + | ^^^^^^^^^^^^^^^^^^ + | +note: argument has type `&mut SomeStruct` + --> $DIR/forget_ref.rs:22:12 + | +LL | forget(reference2); + | ^^^^^^^^^^ + +warning: calls to `std::mem::forget` with a reference instead of an owned value + --> $DIR/forget_ref.rs:25:5 + | +LL | forget(reference3); + | ^^^^^^^^^^^^^^^^^^ + | +note: argument has type `&SomeStruct` + --> $DIR/forget_ref.rs:25:12 + | +LL | forget(reference3); + | ^^^^^^^^^^ + +warning: calls to `std::mem::forget` with a reference instead of an owned value + --> $DIR/forget_ref.rs:30:5 + | +LL | forget(&val); + | ^^^^^^^^^^^^ + | +note: argument has type `&T` + --> $DIR/forget_ref.rs:30:12 + | +LL | forget(&val); + | ^^^^ + +warning: calls to `std::mem::forget` with a reference instead of an owned value + --> $DIR/forget_ref.rs:38:5 + | +LL | std::mem::forget(&SomeStruct); + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + | +note: argument has type `&SomeStruct` + --> $DIR/forget_ref.rs:38:22 + | +LL | std::mem::forget(&SomeStruct); + | ^^^^^^^^^^^ + +warning: 9 warnings emitted +