Skip to content

Commit

Permalink
Rollup merge of rust-lang#125433 - surechen:fix_125189, r=Urgau
Browse files Browse the repository at this point in the history
A small diagnostic improvement for dropping_copy_types

For a value `m`  which implements `Copy` trait, `drop(m);` does nothing.
We now suggest user to ignore it by a abstract and general note: `let _ = ...`.
I think we can give a clearer note here: `let _ = m;`

fixes rust-lang#125189

<!--
If this PR is related to an unstable feature or an otherwise tracked effort,
please link to the relevant tracking issue here. If you don't know of a related
tracking issue or there are none, feel free to ignore this.

This PR will get automatically assigned to a reviewer. In case you would like
a specific user to review your work, you can assign it to them by using

    r​? <reviewer name>
-->
  • Loading branch information
matthiaskrgr authored May 26, 2024
2 parents 2abcd3e + 09c8e39 commit 9747e5b
Show file tree
Hide file tree
Showing 11 changed files with 169 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 @@ -233,6 +233,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
21 changes: 17 additions & 4 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};
use rustc_hir::{Arm, Expr, ExprKind, Node, StmtKind};
use rustc_middle::ty;
use rustc_session::{declare_lint, declare_lint_pass};
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 @@ -164,10 +164,23 @@ 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
{
DropCopySuggestion::Suggestion {
start_span: expr.span.shrink_to_lo().until(arg.span),
end_span: arg.span.shrink_to_hi().until(expr.span.shrink_to_hi()),
}
} 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
16 changes: 15 additions & 1 deletion compiler/rustc_lint/src/lints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -677,11 +677,25 @@ 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,
#[multipart_suggestion(lint_suggestion, style = "verbose", applicability = "maybe-incorrect")]
Suggestion {
#[suggestion_part(code = "let _ = ")]
start_span: Span,
#[suggestion_part(code = "")]
end_span: Span,
},
}

#[derive(LintDiagnostic)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@ 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 - drop(origin);
LL + let _ = origin;
|

warning: 1 warning emitted

Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@ 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 - drop(origin);
LL + let _ = origin;
|

warning: 1 warning emitted

14 changes: 14 additions & 0 deletions tests/ui/lint/dropping_copy_types-issue-125189-can-not-fixed.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//@ check-fail

#![deny(dropping_copy_types)]

fn main() {
let y = 1;
let z = 2;
match y {
0 => drop(y), //~ ERROR calls to `std::mem::drop`
1 => drop(z), //~ ERROR calls to `std::mem::drop`
2 => drop(3), //~ ERROR calls to `std::mem::drop`
_ => {},
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
error: calls to `std::mem::drop` with a value that implements `Copy` does nothing
--> $DIR/dropping_copy_types-issue-125189-can-not-fixed.rs:9:14
|
LL | 0 => drop(y),
| ^^^^^-^
| |
| argument has type `i32`
|
= note: use `let _ = ...` to ignore the expression or result
note: the lint level is defined here
--> $DIR/dropping_copy_types-issue-125189-can-not-fixed.rs:3:9
|
LL | #![deny(dropping_copy_types)]
| ^^^^^^^^^^^^^^^^^^^

error: calls to `std::mem::drop` with a value that implements `Copy` does nothing
--> $DIR/dropping_copy_types-issue-125189-can-not-fixed.rs:10:14
|
LL | 1 => drop(z),
| ^^^^^-^
| |
| argument has type `i32`
|
= note: use `let _ = ...` to ignore the expression or result

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
|
LL | 2 => drop(3),
| ^^^^^-^
| |
| argument has type `i32`
|
= note: use `let _ = ...` to ignore the expression or result

error: aborting due to 3 previous errors

10 changes: 10 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,10 @@
//@ check-fail
//@ 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`
}
10 changes: 10 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,10 @@
//@ check-fail
//@ 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`
}
35 changes: 35 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,35 @@
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(3.2);
| ^^^^^---^
| |
| argument has type `f64`
|
note: the lint level is defined here
--> $DIR/dropping_copy_types-issue-125189.rs:4:9
|
LL | #![deny(dropping_copy_types)]
| ^^^^^^^^^^^^^^^^^^^
help: use `let _ = ...` to ignore the expression or result
|
LL - drop(3.2);
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
|
LL | drop(y);
| ^^^^^-^
| |
| argument has type `i32`
|
help: use `let _ = ...` to ignore the expression or result
|
LL - drop(y);
LL + let _ = y;
|

error: aborting due to 2 previous errors

24 changes: 20 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,16 @@ 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 - drop(s1);
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 +25,11 @@ 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 - drop(s2);
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 +50,11 @@ 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 - drop(s4);
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 +94,11 @@ 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 - drop(println_and(13));
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 9747e5b

Please sign in to comment.