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

Enhance suggestions of dropping_* lints #126275

Closed
wants to merge 3 commits into from
Closed
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
10 changes: 10 additions & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,9 @@ lint_drop_trait_constraints =
lint_dropping_copy_types = calls to `std::mem::drop` with a value that implements `Copy` does nothing
.label = argument has type `{$arg_ty}`

lint_dropping_mutable_references = calls to `std::mem::drop` with a mutable reference instead of an owned value only makes the reference inaccessible, it does not drop the underlying value
.label = argument has type `{$arg_ty}`

lint_dropping_references = calls to `std::mem::drop` with a reference instead of an owned value does nothing
.label = argument has type `{$arg_ty}`

Expand Down Expand Up @@ -270,6 +273,9 @@ lint_for_loops_over_fallibles =
lint_forgetting_copy_types = calls to `std::mem::forget` with a value that implements `Copy` does nothing
.label = argument has type `{$arg_ty}`

lint_forgetting_mutable_references = calls to `std::mem::forget` with a mutable reference instead of an owned value only makes the reference inaccessible, it does not drop the underlying value
.label = argument has type `{$arg_ty}`

lint_forgetting_references = calls to `std::mem::forget` with a reference instead of an owned value does nothing
.label = argument has type `{$arg_ty}`

Expand Down Expand Up @@ -683,6 +689,10 @@ lint_redundant_semicolons =
*[false] this semicolon
}

lint_remove_entirely_suggestion = remove the whole expression

lint_remove_function_call_suggestion = remove the function call

lint_remove_mut_from_pattern = remove `mut` from the parameter

lint_removed_lint = lint `{$name}` has been removed: {$reason}
Expand Down
108 changes: 75 additions & 33 deletions compiler/rustc_lint/src/drop_forget_useless.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
use rustc_hir::{Arm, Expr, ExprKind, Node, StmtKind};
use rustc_middle::ty;
use rustc_hir::{Arm, Expr, ExprKind, Mutability, Node, StmtKind};
use rustc_middle::ty::{self, Ty};
use rustc_session::{declare_lint, declare_lint_pass};
use rustc_span::sym;

use crate::{
lints::{
DropCopyDiag, DropRefDiag, ForgetCopyDiag, ForgetRefDiag, UndroppedManuallyDropsDiag,
UndroppedManuallyDropsSuggestion, UseLetUnderscoreIgnoreSuggestion,
DropCopyDiag, DropMutRefDiag, DropRefDiag, ForgetCopyDiag, ForgetMutRefDiag, ForgetRefDiag,
IgnoreSuggestion, UndroppedManuallyDropsDiag, UndroppedManuallyDropsSuggestion,
},
LateContext, LateLintPass, LintContext,
};
Expand Down Expand Up @@ -140,6 +140,17 @@ declare_lint_pass!(DropForgetUseless => [DROPPING_REFERENCES, FORGETTING_REFEREN

impl<'tcx> LateLintPass<'tcx> for DropForgetUseless {
fn check_expr(&mut self, cx: &LateContext<'tcx>, expr: &'tcx Expr<'tcx>) {
fn remove_entirely(expr: &Expr<'_>) -> bool {
match expr.kind {
ExprKind::Cast(inner, ..) => remove_entirely(inner),
ExprKind::Type(inner, ..) => remove_entirely(inner),
ExprKind::Field(inner, ..) => remove_entirely(inner),
ExprKind::Path(..) => true,
ExprKind::AddrOf(.., inner) => remove_entirely(inner),
_ => false,
}
}

if let ExprKind::Call(path, [arg]) = expr.kind
&& let ExprKind::Path(ref qpath) = path.kind
&& let Some(def_id) = cx.qpath_res(qpath, path.hir_id).opt_def_id()
Expand All @@ -148,62 +159,93 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetUseless {
let arg_ty = cx.typeck_results().expr_ty(arg);
let is_copy = arg_ty.is_copy_modulo_regions(cx.tcx, cx.param_env);
let drop_is_single_call_in_arm = is_single_call_in_arm(cx, arg, expr);
let let_underscore_ignore_sugg = || {
if let Some((_, node)) = cx.tcx.hir().parent_iter(expr.hir_id).nth(0)
let sugg = || {
let start_end = if let Some((_, node)) =
cx.tcx.hir().parent_iter(expr.hir_id).nth(0)
&& let Node::Stmt(stmt) = node
&& let StmtKind::Semi(e) = stmt.kind
&& e.hir_id == expr.hir_id
{
UseLetUnderscoreIgnoreSuggestion::Suggestion {
start_span: expr.span.shrink_to_lo().until(arg.span),
end_span: arg.span.shrink_to_hi().until(expr.span.shrink_to_hi()),
}
Some((
expr.span.shrink_to_lo().until(arg.span),
arg.span.shrink_to_hi().until(expr.span.shrink_to_hi()),
stmt.span,
))
} else {
UseLetUnderscoreIgnoreSuggestion::Note
None
};
fn is_uninteresting(ty: Ty<'_>) -> bool {
match ty.kind() {
ty::Never => true,
ty::Tuple(list) if list.is_empty() => true,
_ => false,
}
}
match (remove_entirely(arg), is_uninteresting(arg_ty), start_end) {
(true, _, Some((_, _, stmt))) => {
IgnoreSuggestion::RemoveEntirelySuggestion { span: stmt }
}
(true, _, None) => IgnoreSuggestion::RemoveEntirelyNote,
(false, true, Some((start, end, _))) => {
IgnoreSuggestion::RemoveFunctionCallSuggestion {
start_span: start,
end_span: end,
}
}
(false, true, None) => IgnoreSuggestion::RemoveFunctionCallNote,
(false, false, Some((start, end, _))) => {
IgnoreSuggestion::UseLetUnderscoreSuggestion {
start_span: start,
end_span: end,
}
}
(false, false, None) => IgnoreSuggestion::UseLetUnderscoreNote,
}
};
match fn_name {
sym::mem_drop if arg_ty.is_ref() && !drop_is_single_call_in_arm => {
match (fn_name, arg_ty.ref_mutability()) {
(sym::mem_drop, Some(Mutability::Not)) if !drop_is_single_call_in_arm => {
cx.emit_span_lint(
DROPPING_REFERENCES,
expr.span,
DropRefDiag { arg_ty, label: arg.span, sugg: let_underscore_ignore_sugg() },
DropRefDiag { arg_ty, label: arg.span, sugg: sugg() },
);
}
sym::mem_forget if arg_ty.is_ref() => {
(sym::mem_drop, Some(Mutability::Mut)) if !drop_is_single_call_in_arm => {
cx.emit_span_lint(
DROPPING_REFERENCES,
expr.span,
DropMutRefDiag { arg_ty, label: arg.span, sugg: sugg() },
);
}
(sym::mem_forget, Some(Mutability::Not)) => {
cx.emit_span_lint(
FORGETTING_REFERENCES,
expr.span,
ForgetRefDiag {
arg_ty,
label: arg.span,
sugg: let_underscore_ignore_sugg(),
},
ForgetRefDiag { arg_ty, label: arg.span, sugg: sugg() },
);
}
(sym::mem_forget, Some(Mutability::Mut)) => {
cx.emit_span_lint(
FORGETTING_REFERENCES,
expr.span,
ForgetMutRefDiag { arg_ty, label: arg.span, sugg: sugg() },
);
}
sym::mem_drop if is_copy && !drop_is_single_call_in_arm => {
(sym::mem_drop, _) if is_copy && !drop_is_single_call_in_arm => {
cx.emit_span_lint(
DROPPING_COPY_TYPES,
expr.span,
DropCopyDiag {
arg_ty,
label: arg.span,
sugg: let_underscore_ignore_sugg(),
},
DropCopyDiag { arg_ty, label: arg.span, sugg: sugg() },
);
}
sym::mem_forget if is_copy => {
(sym::mem_forget, _) if is_copy => {
cx.emit_span_lint(
FORGETTING_COPY_TYPES,
expr.span,
ForgetCopyDiag {
arg_ty,
label: arg.span,
sugg: let_underscore_ignore_sugg(),
},
ForgetCopyDiag { arg_ty, label: arg.span, sugg: sugg() },
);
}
sym::mem_drop
(sym::mem_drop, _)
if let ty::Adt(adt, _) = arg_ty.kind()
&& adt.is_manually_drop() =>
{
Expand Down
58 changes: 51 additions & 7 deletions compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,15 +657,39 @@ pub struct ForLoopsOverFalliblesSuggestion<'a> {
}

#[derive(Subdiagnostic)]
pub enum UseLetUnderscoreIgnoreSuggestion {
pub enum IgnoreSuggestion {
#[note(lint_remove_entirely_suggestion)]
RemoveEntirelyNote,
#[multipart_suggestion(
lint_remove_entirely_suggestion,
style = "verbose",
applicability = "maybe-incorrect"
)]
RemoveEntirelySuggestion {
#[suggestion_part(code = "")]
span: Span,
},
#[note(lint_remove_function_call_suggestion)]
RemoveFunctionCallNote,
#[multipart_suggestion(
lint_remove_function_call_suggestion,
style = "verbose",
applicability = "maybe-incorrect"
)]
RemoveFunctionCallSuggestion {
#[suggestion_part(code = "")]
start_span: Span,
#[suggestion_part(code = "")]
end_span: Span,
},
#[note(lint_use_let_underscore_ignore_suggestion)]
Note,
UseLetUnderscoreNote,
#[multipart_suggestion(
lint_use_let_underscore_ignore_suggestion,
style = "verbose",
applicability = "maybe-incorrect"
)]
Suggestion {
UseLetUnderscoreSuggestion {
#[suggestion_part(code = "let _ = ")]
start_span: Span,
#[suggestion_part(code = "")]
Expand All @@ -681,7 +705,17 @@ pub struct DropRefDiag<'a> {
#[label]
pub label: Span,
#[subdiagnostic]
pub sugg: UseLetUnderscoreIgnoreSuggestion,
pub sugg: IgnoreSuggestion,
}

#[derive(LintDiagnostic)]
#[diag(lint_dropping_mutable_references)]
pub struct DropMutRefDiag<'a> {
pub arg_ty: Ty<'a>,
#[label]
pub label: Span,
#[subdiagnostic]
pub sugg: IgnoreSuggestion,
}

#[derive(LintDiagnostic)]
Expand All @@ -691,7 +725,7 @@ pub struct DropCopyDiag<'a> {
#[label]
pub label: Span,
#[subdiagnostic]
pub sugg: UseLetUnderscoreIgnoreSuggestion,
pub sugg: IgnoreSuggestion,
}

#[derive(LintDiagnostic)]
Expand All @@ -701,7 +735,17 @@ pub struct ForgetRefDiag<'a> {
#[label]
pub label: Span,
#[subdiagnostic]
pub sugg: UseLetUnderscoreIgnoreSuggestion,
pub sugg: IgnoreSuggestion,
}

#[derive(LintDiagnostic)]
#[diag(lint_forgetting_mutable_references)]
pub struct ForgetMutRefDiag<'a> {
pub arg_ty: Ty<'a>,
#[label]
pub label: Span,
#[subdiagnostic]
pub sugg: IgnoreSuggestion,
}

#[derive(LintDiagnostic)]
Expand All @@ -711,7 +755,7 @@ pub struct ForgetCopyDiag<'a> {
#[label]
pub label: Span,
#[subdiagnostic]
pub sugg: UseLetUnderscoreIgnoreSuggestion,
pub sugg: IgnoreSuggestion,
}

#[derive(LintDiagnostic)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ LL | 0 => drop(y),
| |
| argument has type `i32`
|
= note: use `let _ = ...` to ignore the expression or result
= note: remove the whole expression
note: the lint level is defined here
--> $DIR/dropping_copy_types-issue-125189-can-not-fixed.rs:3:9
|
Expand All @@ -21,7 +21,7 @@ LL | 1 => drop(z),
| |
| argument has type `i32`
|
= note: use `let _ = ...` to ignore the expression or result
= note: remove the whole expression

error: calls to `std::mem::drop` with a value that implements `Copy` does nothing
--> $DIR/dropping_copy_types-issue-125189-can-not-fixed.rs:11:14
Expand Down
3 changes: 2 additions & 1 deletion tests/ui/lint/dropping_copy_types-issue-125189.fixed
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@

#![deny(dropping_copy_types)]

#[allow(unused_variables)]
fn main() {
let y = 1;
let _ = 3.2; //~ ERROR calls to `std::mem::drop`
let _ = y; //~ ERROR calls to `std::mem::drop`
//~ ERROR calls to `std::mem::drop`
}
1 change: 1 addition & 0 deletions tests/ui/lint/dropping_copy_types-issue-125189.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#![deny(dropping_copy_types)]

#[allow(unused_variables)]
fn main() {
let y = 1;
drop(3.2); //~ ERROR calls to `std::mem::drop`
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/lint/dropping_copy_types-issue-125189.stderr
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
error: calls to `std::mem::drop` with a value that implements `Copy` does nothing
--> $DIR/dropping_copy_types-issue-125189.rs:8:5
--> $DIR/dropping_copy_types-issue-125189.rs:9:5
|
LL | drop(3.2);
| ^^^^^---^
Expand All @@ -18,17 +18,17 @@ LL + let _ = 3.2;
|

error: calls to `std::mem::drop` with a value that implements `Copy` does nothing
--> $DIR/dropping_copy_types-issue-125189.rs:9:5
--> $DIR/dropping_copy_types-issue-125189.rs:10:5
|
LL | drop(y);
| ^^^^^-^
| |
| argument has type `i32`
|
help: use `let _ = ...` to ignore the expression or result
help: remove the whole expression
|
LL - drop(y);
LL + let _ = y;
LL +
|

error: aborting due to 2 previous errors
Expand Down
2 changes: 2 additions & 0 deletions tests/ui/lint/dropping_copy_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ fn main() {
drop(a3);
drop(a4); //~ WARN calls to `std::mem::drop`
drop(a5);

drop(drop(String::new())); //~ WARN calls to `std::mem::drop`
}

#[allow(unused)]
Expand Down
Loading
Loading