Skip to content

Commit

Permalink
A small diagnostic improvement for dropping_copy_types
Browse files Browse the repository at this point in the history
fixes #125189
  • Loading branch information
surechen committed May 24, 2024
1 parent 9f432d7 commit a84f56a
Show file tree
Hide file tree
Showing 9 changed files with 113 additions and 11 deletions.
1 change: 1 addition & 0 deletions compiler/rustc_lint/messages.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ 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}`
.note = use `let _ = ...` to ignore the expression or result
.suggestion = use `let _ = ...` to ignore the expression or result
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
22 changes: 18 additions & 4 deletions compiler/rustc_lint/src/drop_forget_useless.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use rustc_hir::{Arm, Expr, ExprKind, Node};
use rustc_hir::{Arm, Expr, ExprKind, Node, StmtKind};
use rustc_middle::ty;
use rustc_span::sym;

use crate::{
lints::{
DropCopyDiag, DropRefDiag, ForgetCopyDiag, ForgetRefDiag, UndroppedManuallyDropsDiag,
UndroppedManuallyDropsSuggestion,
DropCopyDiag, DropCopySuggestion, DropRefDiag, ForgetCopyDiag, ForgetRefDiag,
UndroppedManuallyDropsDiag, UndroppedManuallyDropsSuggestion,
},
LateContext, LateLintPass, LintContext,
};
Expand Down Expand Up @@ -163,10 +163,24 @@ impl<'tcx> LateLintPass<'tcx> for DropForgetUseless {
);
}
sym::mem_drop if is_copy && !drop_is_single_call_in_arm => {
let sugg = 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
&& let Ok(value) = cx.sess().source_map().span_to_snippet(arg.span)
{
DropCopySuggestion::Suggestion {
sugg: expr.span,
replace: format!("let _ = {}", value),
}
} else {
DropCopySuggestion::Note
};

cx.emit_span_lint(
DROPPING_COPY_TYPES,
expr.span,
DropCopyDiag { arg_ty, label: arg.span },
DropCopyDiag { arg_ty, label: arg.span, sugg },
);
}
sym::mem_forget if is_copy => {
Expand Down
20 changes: 19 additions & 1 deletion compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -669,11 +669,29 @@ pub struct DropRefDiag<'a> {

#[derive(LintDiagnostic)]
#[diag(lint_dropping_copy_types)]
#[note]
pub struct DropCopyDiag<'a> {
pub arg_ty: Ty<'a>,
#[label]
pub label: Span,
#[subdiagnostic]
pub sugg: DropCopySuggestion,
}

#[derive(Subdiagnostic)]
pub enum DropCopySuggestion {
#[note(lint_note)]
Note,
#[suggestion(
lint_suggestion,
style = "verbose",
code = "{replace}",
applicability = "maybe-incorrect"
)]
Suggestion {
#[primary_span]
sugg: Span,
replace: String,
},
}

#[derive(LintDiagnostic)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@ LL | drop(origin);
| |
| argument has type `<T as UncheckedCopy>::Output`
|
= note: use `let _ = ...` to ignore the expression or result
= note: `#[warn(dropping_copy_types)]` on by default
help: use `let _ = ...` to ignore the expression or result
|
LL | let _ = origin;
| ~~~~~~~~~~~~~~

warning: 1 warning emitted

Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,11 @@ LL | drop(origin);
| |
| argument has type `<T as UncheckedCopy>::Output`
|
= note: use `let _ = ...` to ignore the expression or result
= note: `#[warn(dropping_copy_types)]` on by default
help: use `let _ = ...` to ignore the expression or result
|
LL | let _ = origin;
| ~~~~~~~~~~~~~~

warning: 1 warning emitted

9 changes: 9 additions & 0 deletions tests/ui/lint/dropping_copy_types-issue-125189.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//@ run-rustfix

#![deny(dropping_copy_types)]

fn main() {
let y = 1;
let _ = 3.2; //~ ERROR calls to `std::mem::drop`
let _ = y; //~ ERROR calls to `std::mem::drop`
}
9 changes: 9 additions & 0 deletions tests/ui/lint/dropping_copy_types-issue-125189.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
//@ run-rustfix

#![deny(dropping_copy_types)]

fn main() {
let y = 1;
drop(3.2); //~ ERROR calls to `std::mem::drop`
drop(y); //~ ERROR calls to `std::mem::drop`
}
33 changes: 33 additions & 0 deletions tests/ui/lint/dropping_copy_types-issue-125189.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
error: calls to `std::mem::drop` with a value that implements `Copy` does nothing
--> $DIR/dropping_copy_types-issue-125189.rs:7:5
|
LL | drop(3.2);
| ^^^^^---^
| |
| argument has type `f64`
|
note: the lint level is defined here
--> $DIR/dropping_copy_types-issue-125189.rs:3:9
|
LL | #![deny(dropping_copy_types)]
| ^^^^^^^^^^^^^^^^^^^
help: use `let _ = ...` to ignore the expression or result
|
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:8:5
|
LL | drop(y);
| ^^^^^-^
| |
| argument has type `i32`
|
help: use `let _ = ...` to ignore the expression or result
|
LL | let _ = y;
| ~~~~~~~~~

error: aborting due to 2 previous errors

20 changes: 16 additions & 4 deletions tests/ui/lint/dropping_copy_types.stderr
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,15 @@ LL | drop(s1);
| |
| argument has type `SomeStruct`
|
= note: use `let _ = ...` to ignore the expression or result
note: the lint level is defined here
--> $DIR/dropping_copy_types.rs:3:9
|
LL | #![warn(dropping_copy_types)]
| ^^^^^^^^^^^^^^^^^^^
help: use `let _ = ...` to ignore the expression or result
|
LL | let _ = s1;
| ~~~~~~~~~~

warning: calls to `std::mem::drop` with a value that implements `Copy` does nothing
--> $DIR/dropping_copy_types.rs:35:5
Expand All @@ -21,7 +24,10 @@ LL | drop(s2);
| |
| argument has type `SomeStruct`
|
= note: use `let _ = ...` to ignore the expression or result
help: use `let _ = ...` to ignore the expression or result
|
LL | let _ = s2;
| ~~~~~~~~~~

warning: calls to `std::mem::drop` with a reference instead of an owned value does nothing
--> $DIR/dropping_copy_types.rs:36:5
Expand All @@ -42,7 +48,10 @@ LL | drop(s4);
| |
| argument has type `SomeStruct`
|
= note: use `let _ = ...` to ignore the expression or result
help: use `let _ = ...` to ignore the expression or result
|
LL | let _ = s4;
| ~~~~~~~~~~

warning: calls to `std::mem::drop` with a reference instead of an owned value does nothing
--> $DIR/dropping_copy_types.rs:38:5
Expand Down Expand Up @@ -82,7 +91,10 @@ LL | drop(println_and(13));
| |
| argument has type `i32`
|
= note: use `let _ = ...` to ignore the expression or result
help: use `let _ = ...` to ignore the expression or result
|
LL | let _ = println_and(13);
| ~~~~~~~~~~~~~~~~~~~~~~~

warning: calls to `std::mem::drop` with a value that implements `Copy` does nothing
--> $DIR/dropping_copy_types.rs:74:14
Expand Down

0 comments on commit a84f56a

Please sign in to comment.