Skip to content

Commit

Permalink
Auto merge of #111530 - Urgau:uplift_undropped_manually_drops, r=comp…
Browse files Browse the repository at this point in the history
…iler-errors

Uplift `clippy::undropped_manually_drops` lint

This PR aims at uplifting the `clippy::undropped_manually_drops` lint.

## `undropped_manually_drops`

(warn-by-default)

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
struct S;
drop(std::mem::ManuallyDrop::new(S));
```

### Explanation

`ManuallyDrop` does not drop it's inner value so calling `std::mem::drop` will not drop the inner value of the `ManuallyDrop` either.

-----

Mostly followed the instructions for uplifting an clippy lint described here: #99696 (review)

`@rustbot` label: +I-lang-nominated
r? compiler

-----

For Clippy:

changelog: Moves: Uplifted `clippy::undropped_manually_drops` into rustc
  • Loading branch information
bors committed Jun 9, 2023
2 parents 343ad6f + d9d1c76 commit d7ad9d9
Show file tree
Hide file tree
Showing 14 changed files with 192 additions and 141 deletions.
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

0 comments on commit d7ad9d9

Please sign in to comment.