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.

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

Uplift clippy::undropped_manually_drops lint #111530

Merged
merged 3 commits into from
Jun 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 4 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,10 @@ lint_tykind = usage of `ty::TyKind`
lint_tykind_kind = usage of `ty::TyKind::<kind>`
.suggestion = try using `ty::<kind>` directly

lint_undropped_manually_drops = calls to `std::mem::drop` with `std::mem::ManuallyDrop` instead of the inner value does nothing
.label = argument has type `{$arg_ty}`
.suggestion = use `std::mem::ManuallyDrop::into_inner` to get the inner value

lint_ungated_async_fn_track_caller = `#[track_caller]` on async functions is a no-op
.label = this function will not propagate the caller location

Expand Down
44 changes: 42 additions & 2 deletions compiler/rustc_lint/src/drop_forget_useless.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
use rustc_hir::{Arm, Expr, ExprKind, Node};
use rustc_middle::ty;
use rustc_span::sym;

use crate::{
lints::{DropCopyDiag, DropRefDiag, ForgetCopyDiag, ForgetRefDiag},
lints::{
DropCopyDiag, DropRefDiag, ForgetCopyDiag, ForgetRefDiag, UndroppedManuallyDropsDiag,
UndroppedManuallyDropsSuggestion,
},
LateContext, LateLintPass, LintContext,
};

Expand Down Expand Up @@ -109,7 +113,29 @@ declare_lint! {
"calls to `std::mem::forget` with a value that implements Copy"
}

declare_lint_pass!(DropForgetUseless => [DROPPING_REFERENCES, FORGETTING_REFERENCES, DROPPING_COPY_TYPES, FORGETTING_COPY_TYPES]);
declare_lint! {
/// The `undropped_manually_drops` lint check for calls to `std::mem::drop` with
/// a value of `std::mem::ManuallyDrop` which doesn't drop.
///
/// ### Example
///
/// ```rust,compile_fail
/// struct S;
/// drop(std::mem::ManuallyDrop::new(S));
/// ```
///
/// {{produces}}
///
/// ### Explanation
///
/// `ManuallyDrop` does not drop it's inner value so calling `std::mem::drop` will
/// not drop the inner value of the `ManuallyDrop` either.
pub UNDROPPED_MANUALLY_DROPS,
Deny,
"calls to `std::mem::drop` with `std::mem::ManuallyDrop` instead of it's inner value"
}

declare_lint_pass!(DropForgetUseless => [DROPPING_REFERENCES, FORGETTING_REFERENCES, DROPPING_COPY_TYPES, FORGETTING_COPY_TYPES, UNDROPPED_MANUALLY_DROPS]);

impl<'tcx> LateLintPass<'tcx> for DropForgetUseless {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
Expand All @@ -134,6 +160,20 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetUseless {
sym::mem_forget if is_copy => {
cx.emit_spanned_lint(FORGETTING_COPY_TYPES, expr.span, ForgetCopyDiag { arg_ty, label: arg.span });
}
sym::mem_drop if let ty::Adt(adt, _) = arg_ty.kind() && adt.is_manually_drop() => {
cx.emit_spanned_lint(
UNDROPPED_MANUALLY_DROPS,
expr.span,
UndroppedManuallyDropsDiag {
arg_ty,
label: arg.span,
suggestion: UndroppedManuallyDropsSuggestion {
start_span: arg.span.shrink_to_lo(),
end_span: arg.span.shrink_to_hi()
}
}
);
}
_ => return,
};
}
Expand Down
19 changes: 19 additions & 0 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -699,6 +699,25 @@ pub struct ForgetCopyDiag<'a> {
pub label: Span,
}

#[derive(LintDiagnostic)]
#[diag(lint_undropped_manually_drops)]
pub struct UndroppedManuallyDropsDiag<'a> {
pub arg_ty: Ty<'a>,
#[label]
pub label: Span,
#[subdiagnostic]
pub suggestion: UndroppedManuallyDropsSuggestion,
}

#[derive(Subdiagnostic)]
#[multipart_suggestion(lint_suggestion, applicability = "machine-applicable")]
pub struct UndroppedManuallyDropsSuggestion {
#[suggestion_part(code = "std::mem::ManuallyDrop::into_inner(")]
pub start_span: Span,
#[suggestion_part(code = ")")]
pub end_span: Span,
}

// invalid_from_utf8.rs
#[derive(LintDiagnostic)]
pub enum InvalidFromUtf8Diag {
Expand Down
2 changes: 2 additions & 0 deletions library/core/tests/manually_drop.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![cfg_attr(not(bootstrap), allow(undropped_manually_drops))]

use core::mem::ManuallyDrop;

#[test]
Expand Down
1 change: 0 additions & 1 deletion src/tools/clippy/clippy_lints/src/declared_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ pub(crate) static LINTS: &[&crate::LintInfo] = &[
crate::double_parens::DOUBLE_PARENS_INFO,
crate::drop_forget_ref::DROP_NON_DROP_INFO,
crate::drop_forget_ref::FORGET_NON_DROP_INFO,
crate::drop_forget_ref::UNDROPPED_MANUALLY_DROPS_INFO,
crate::duplicate_mod::DUPLICATE_MOD_INFO,
crate::else_if_without_else::ELSE_IF_WITHOUT_ELSE_INFO,
crate::empty_drop::EMPTY_DROP_INFO,
Expand Down
44 changes: 2 additions & 42 deletions src/tools/clippy/clippy_lints/src/drop_forget_ref.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use clippy_utils::diagnostics::{span_lint_and_help, span_lint_and_note};
use clippy_utils::diagnostics::span_lint_and_note;
use clippy_utils::get_parent_node;
use clippy_utils::is_must_use_func_call;
use clippy_utils::ty::{is_copy, is_must_use_ty, is_type_lang_item};
Expand Down Expand Up @@ -47,35 +47,6 @@ declare_clippy_lint! {
"call to `std::mem::forget` with a value which does not implement `Drop`"
}

declare_clippy_lint! {
/// ### What it does
/// Prevents the safe `std::mem::drop` function from being called on `std::mem::ManuallyDrop`.
///
/// ### Why is this bad?
/// The safe `drop` function does not drop the inner value of a `ManuallyDrop`.
///
/// ### Known problems
/// Does not catch cases if the user binds `std::mem::drop`
/// to a different name and calls it that way.
///
/// ### Example
/// ```rust
/// struct S;
/// drop(std::mem::ManuallyDrop::new(S));
/// ```
/// Use instead:
/// ```rust
/// struct S;
/// unsafe {
/// std::mem::ManuallyDrop::drop(&mut std::mem::ManuallyDrop::new(S));
/// }
/// ```
#[clippy::version = "1.49.0"]
pub UNDROPPED_MANUALLY_DROPS,
correctness,
"use of safe `std::mem::drop` function to drop a std::mem::ManuallyDrop, which will not drop the inner value"
}

const DROP_NON_DROP_SUMMARY: &str = "call to `std::mem::drop` with a value that does not implement `Drop`. \
Dropping such a type only extends its contained lifetimes";
const FORGET_NON_DROP_SUMMARY: &str = "call to `std::mem::forget` with a value that does not implement `Drop`. \
Expand All @@ -84,7 +55,6 @@ const FORGET_NON_DROP_SUMMARY: &str = "call to `std::mem::forget` with a value t
declare_lint_pass!(DropForgetRef => [
DROP_NON_DROP,
FORGET_NON_DROP,
UNDROPPED_MANUALLY_DROPS
]);

impl<'tcx> LateLintPass<'tcx> for DropForgetRef {
Expand All @@ -103,17 +73,7 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetRef {
sym::mem_forget if arg_ty.is_ref() => return,
sym::mem_drop if is_copy && !drop_is_single_call_in_arm => return,
sym::mem_forget if is_copy => return,
sym::mem_drop if is_type_lang_item(cx, arg_ty, LangItem::ManuallyDrop) => {
span_lint_and_help(
cx,
UNDROPPED_MANUALLY_DROPS,
expr.span,
"the inner value of this ManuallyDrop will not be dropped",
None,
"to drop a `ManuallyDrop<T>`, use std::mem::ManuallyDrop::drop",
);
return;
}
sym::mem_drop if is_type_lang_item(cx, arg_ty, LangItem::ManuallyDrop) => return,
sym::mem_drop
if !(arg_ty.needs_drop(cx.tcx, cx.param_env)
|| is_must_use_func_call(cx, arg)
Expand Down
1 change: 1 addition & 0 deletions src/tools/clippy/clippy_lints/src/renamed_lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ pub static RENAMED_LINTS: &[(&str, &str)] = &[
("clippy::panic_params", "non_fmt_panics"),
("clippy::positional_named_format_parameters", "named_arguments_used_positionally"),
("clippy::temporary_cstring_as_ptr", "temporary_cstring_as_ptr"),
("clippy::undropped_manually_drops", "undropped_manually_drops"),
("clippy::unknown_clippy_lints", "unknown_lints"),
("clippy::unused_label", "unused_labels"),
];
2 changes: 2 additions & 0 deletions src/tools/clippy/tests/ui/rename.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#![allow(named_arguments_used_positionally)]
#![allow(suspicious_double_ref_op)]
#![allow(temporary_cstring_as_ptr)]
#![allow(undropped_manually_drops)]
#![allow(unknown_lints)]
#![allow(unused_labels)]
#![warn(clippy::almost_complete_range)]
Expand Down Expand Up @@ -97,6 +98,7 @@
#![warn(non_fmt_panics)]
#![warn(named_arguments_used_positionally)]
#![warn(temporary_cstring_as_ptr)]
#![warn(undropped_manually_drops)]
#![warn(unknown_lints)]
#![warn(unused_labels)]

Expand Down
2 changes: 2 additions & 0 deletions src/tools/clippy/tests/ui/rename.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
#![allow(named_arguments_used_positionally)]
#![allow(suspicious_double_ref_op)]
#![allow(temporary_cstring_as_ptr)]
#![allow(undropped_manually_drops)]
#![allow(unknown_lints)]
#![allow(unused_labels)]
#![warn(clippy::almost_complete_letter_range)]
Expand Down Expand Up @@ -97,6 +98,7 @@
#![warn(clippy::panic_params)]
#![warn(clippy::positional_named_format_parameters)]
#![warn(clippy::temporary_cstring_as_ptr)]
#![warn(clippy::undropped_manually_drops)]
#![warn(clippy::unknown_clippy_lints)]
#![warn(clippy::unused_label)]

Expand Down
Loading