Skip to content
Open
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
90 changes: 45 additions & 45 deletions clippy_lints/src/significant_drop_tightening.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,39 +87,47 @@ impl<'tcx> LateLintPass<'tcx> for SignificantDropTightening<'tcx> {
first_bind_ident.span,
"temporary with significant `Drop` can be early dropped",
|diag| {
match apa.counter {
0 | 1 => {},
2 => {
let indent = " ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0));
let init_method = snippet(cx, apa.first_method_span, "..");
let usage_method = snippet(cx, apa.last_method_span, "..");
let stmt = if let Some(last_bind_ident) = apa.last_bind_ident {
format!(
"\n{indent}let {} = {init_method}.{usage_method};",
snippet(cx, last_bind_ident.span, ".."),
)
} else {
format!("\n{indent}{init_method}.{usage_method};")
};

diag.multipart_suggestion_verbose(
"merge the temporary construction with its single usage",
vec![(apa.first_stmt_span, stmt), (apa.last_stmt_span, String::new())],
Applicability::MaybeIncorrect,
);
},
_ => {
diag.span_suggestion(
apa.last_stmt_span.shrink_to_hi(),
"drop the temporary after the end of its last usage",
format!(
"\n{}drop({});",
" ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0)),
first_bind_ident
),
Applicability::MaybeIncorrect,
);
},
if !apa.last_stmt_span.is_dummy() {
match apa.counter {
0 | 1 => unreachable!("checked above"),
2 =>
{
#[expect(clippy::if_not_else, reason = "for symmetry with the outer check")]
if !apa.last_method_span.is_dummy() {
let indent = " ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0));
let init_method = snippet(cx, apa.first_method_span, "..");
let usage_method = snippet(cx, apa.last_method_span, "..");
let stmt = if let Some(last_bind_ident) = apa.last_bind_ident {
format!(
"\n{indent}let {} = {init_method}.{usage_method};",
snippet(cx, last_bind_ident.span, ".."),
)
} else {
format!("\n{indent}{init_method}.{usage_method};")
};
diag.multipart_suggestion_verbose(
"merge the temporary construction with its single usage",
vec![(apa.first_stmt_span, stmt), (apa.last_stmt_span, String::new())],
Applicability::MaybeIncorrect,
);
} else {
diag.help("merge the temporary construction with its single usage");
diag.span_note(apa.last_stmt_span, "single usage here");
}
},
_ => {
diag.span_suggestion(
apa.last_stmt_span.shrink_to_hi(),
"drop the temporary after the end of its last usage",
format!(
"\n{}drop({});",
" ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0)),
first_bind_ident
),
Applicability::MaybeIncorrect,
);
},
}
}
diag.note("this might lead to unnecessary resource contention");
diag.span_label(
Expand Down Expand Up @@ -276,13 +284,7 @@ impl<'tcx> Visitor<'tcx> for StmtsChecker<'_, '_, '_, '_, 'tcx> {
if let hir::StmtKind::Let(local) = self.ap.curr_stmt.kind
&& let hir::PatKind::Binding(_, hir_id, ident, _) = local.pat.kind
&& !self.ap.apas.contains_key(&hir_id)
&& {
if let Some(local_hir_id) = expr.res_local_id() {
local_hir_id == hir_id
} else {
true
}
}
&& expr.res_local_id().is_none_or(|local_hir_id| local_hir_id == hir_id)
{
let mut apa = AuxParamsAttr {
first_block_hir_id: self.ap.curr_block_hir_id,
Expand Down Expand Up @@ -418,15 +420,15 @@ fn dummy_stmt_expr<'any>(expr: &'any hir::Expr<'any>) -> hir::Stmt<'any> {
}

fn has_drop(cx: &LateContext<'_>, expr: &hir::Expr<'_>, first_bind_ident: Option<Ident>) -> bool {
if let hir::ExprKind::Call(fun, [first_arg]) = expr.kind
if let Some(first_bind_ident) = first_bind_ident
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since you're changing this the function shouldn't even take an Option here.

Copy link
Contributor Author

@ada4a ada4a Sep 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's appropriate in this case, because apa.first_bind_ident is an Option<Ident> as well, no?

&& let hir::ExprKind::Call(fun, [first_arg]) = expr.kind
&& let hir::ExprKind::Path(hir::QPath::Resolved(_, fun_path)) = &fun.kind
&& let Res::Def(DefKind::Fn, did) = fun_path.res
&& cx.tcx.is_diagnostic_item(sym::mem_drop, did)
{
let has_ident = |local_expr: &hir::Expr<'_>| {
if let hir::ExprKind::Path(hir::QPath::Resolved(_, arg_path)) = &local_expr.kind
&& let [first_arg_ps, ..] = arg_path.segments
&& let Some(first_bind_ident) = first_bind_ident
&& first_arg_ps.ident == first_bind_ident
{
true
Expand All @@ -448,7 +450,5 @@ fn has_drop(cx: &LateContext<'_>, expr: &hir::Expr<'_>, first_bind_ident: Option

fn is_inexpensive_expr(expr: &hir::Expr<'_>) -> bool {
let actual = peel_hir_expr_unary(expr).0;
let is_path = matches!(actual.kind, hir::ExprKind::Path(_));
let is_lit = matches!(actual.kind, hir::ExprKind::Lit(_));
is_path || is_lit
matches!(actual.kind, hir::ExprKind::Path(_) | hir::ExprKind::Lit(_))
}
46 changes: 46 additions & 0 deletions tests/ui/significant_drop_tightening_unfixable.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
//@no-rustfix
#![warn(clippy::significant_drop_tightening)]

mod issue_15574 {
use std::io::{BufRead, Read, stdin};
use std::process;

// NOTE: this requires `no_rustfix` for multiple reasons:
//
// There should be two suggestions, one to merge the line:
// ```
// let stdin = stdin.lock();
// ```
// into:
// ```
// let mut stdin = stdin.take(40);
// ```
// and one to merge the latter into the `if`.
//
// This causes the following problems:
// - the first suggestion lacks the `mut` before `stdin`, which doesn't compile
// - the second suggestion isn't a suggestion but a help message, so the warning isn't gone after
// rustfix
// - when the second help becomes a suggestion, it will overlap with the first one
fn main() {
//Let's read from stdin
println!("Hello, what's your name?");
let stdin = stdin().lock();
//~^ significant_drop_tightening
let mut buffer = String::with_capacity(10);
//Here we lock stdin and block to 10 bytes
// Our string is now then only 10 bytes.
//Even if it overflows like expected, it will reallocate.
let mut stdin = stdin.take(40);
//~^ significant_drop_tightening
if stdin.read_line(&mut buffer).is_err() {
eprintln!("An error has occured while reading.");
return;
} //Now we print the result, our data is safe
println!("Our string has a capacity of {}", buffer.capacity());
println!("Hello {}!", buffer);
//The string is freed automatically.
}
}

fn main() {}
54 changes: 54 additions & 0 deletions tests/ui/significant_drop_tightening_unfixable.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
error: temporary with significant `Drop` can be early dropped
--> tests/ui/significant_drop_tightening_unfixable.rs:28:13
|
LL | fn main() {
| _______________-
LL | | //Let's read from stdin
LL | | println!("Hello, what's your name?");
LL | | let stdin = stdin().lock();
| | ^^^^^
... |
LL | | }
| |_____- temporary `stdin` is currently being dropped at the end of its contained scope
|
= note: this might lead to unnecessary resource contention
= note: `-D clippy::significant-drop-tightening` implied by `-D warnings`
= help: to override `-D warnings` add `#[allow(clippy::significant_drop_tightening)]`
help: merge the temporary construction with its single usage
|
LL ~
LL + let stdin = stdin().lock().take(40);
LL |
...
LL | //Even if it overflows like expected, it will reallocate.
LL ~
|

error: temporary with significant `Drop` can be early dropped
--> tests/ui/significant_drop_tightening_unfixable.rs:34:17
|
LL | fn main() {
| _______________-
LL | | //Let's read from stdin
LL | | println!("Hello, what's your name?");
LL | | let stdin = stdin().lock();
... |
LL | | let mut stdin = stdin.take(40);
| | ^^^^^
... |
LL | | }
| |_____- temporary `stdin` is currently being dropped at the end of its contained scope
|
= help: merge the temporary construction with its single usage
note: single usage here
--> tests/ui/significant_drop_tightening_unfixable.rs:36:9
|
LL | / if stdin.read_line(&mut buffer).is_err() {
LL | | eprintln!("An error has occured while reading.");
LL | | return;
LL | | } //Now we print the result, our data is safe
| |_________^
= note: this might lead to unnecessary resource contention

error: aborting due to 2 previous errors