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 1 commit
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 @@ -689,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
91 changes: 55 additions & 36 deletions compiler/rustc_lint/src/drop_forget_useless.rs
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
use rustc_hir::{Arm, Expr, ExprKind, Mutability, Node, StmtKind};
use rustc_middle::ty;
use rustc_middle::ty::{self, Ty};
use rustc_session::{declare_lint, declare_lint_pass};
use rustc_span::sym;

use crate::{
lints::{
DropCopyDiag, DropMutRefDiag, DropRefDiag, ForgetCopyDiag, ForgetMutRefDiag, ForgetRefDiag,
UndroppedManuallyDropsDiag, UndroppedManuallyDropsSuggestion,
UseLetUnderscoreIgnoreSuggestion,
IgnoreSuggestion, UndroppedManuallyDropsDiag, UndroppedManuallyDropsSuggestion,
},
LateContext, LateLintPass, LintContext,
};
Expand Down Expand Up @@ -141,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 @@ -149,81 +159,90 @@ 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, 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_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: let_underscore_ignore_sugg(),
},
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: let_underscore_ignore_sugg(),
},
ForgetMutRefDiag { arg_ty, label: arg.span, sugg: sugg() },
);
}
(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 => {
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, _)
Expand Down
42 changes: 33 additions & 9 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,7 @@ pub struct DropRefDiag<'a> {
#[label]
pub label: Span,
#[subdiagnostic]
pub sugg: UseLetUnderscoreIgnoreSuggestion,
pub sugg: IgnoreSuggestion,
}

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

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

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

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

#[derive(LintDiagnostic)]
Expand All @@ -731,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
Loading