Skip to content

Commit 8a1b54c

Browse files
committed
make AuxParamsAttr.last_stmt_span optional as well
Not related to the previous change per se, just noticed that this field follows the same `DUMMY_SP` pattern. And now it doesn't make sense anymore to have the `Default` impl: the only place it was used in--initialization of an `AuxParamAttr`--already initializes the really non-defaultable fields (those with types `Span` and `HirId`), so it's easier to fill in the rest manually
1 parent 4a87fd6 commit 8a1b54c

File tree

1 file changed

+48
-54
lines changed

1 file changed

+48
-54
lines changed

clippy_lints/src/significant_drop_tightening.rs

Lines changed: 48 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -83,40 +83,43 @@ impl<'tcx> LateLintPass<'tcx> for SignificantDropTightening<'tcx> {
8383
let emit_sugg = |diag: &mut Diag<'_, _>| match apa.counter {
8484
0 | 1 => {},
8585
2 => {
86-
if let Some(last_method_span) = apa.last_method_span {
87-
let indent = " ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0));
88-
let init_method = snippet(cx, apa.first_method_span, "..");
89-
let usage_method = snippet(cx, last_method_span, "..");
90-
let stmt = if let Some(last_bind_ident) = apa.last_bind_ident {
91-
format!(
92-
"\n{indent}let {} = {init_method}.{usage_method};",
93-
snippet(cx, last_bind_ident.span, ".."),
94-
)
86+
if let Some(last_stmt_span) = apa.last_stmt_span {
87+
if let Some(last_method_span) = apa.last_method_span {
88+
let indent = " ".repeat(indent_of(cx, last_stmt_span).unwrap_or(0));
89+
let init_method = snippet(cx, apa.first_method_span, "..");
90+
let usage_method = snippet(cx, last_method_span, "..");
91+
let stmt = if let Some(last_bind_ident) = apa.last_bind_ident {
92+
format!(
93+
"\n{indent}let {} = {init_method}.{usage_method};",
94+
snippet(cx, last_bind_ident.span, ".."),
95+
)
96+
} else {
97+
format!("\n{indent}{init_method}.{usage_method};")
98+
};
99+
diag.multipart_suggestion_verbose(
100+
"merge the temporary construction with its single usage",
101+
vec![(apa.first_stmt_span, stmt), (last_stmt_span, String::new())],
102+
Applicability::MaybeIncorrect,
103+
);
95104
} else {
96-
format!("\n{indent}{init_method}.{usage_method};")
97-
};
98-
99-
diag.multipart_suggestion_verbose(
100-
"merge the temporary construction with its single usage",
101-
vec![(apa.first_stmt_span, stmt), (apa.last_stmt_span, String::new())],
102-
Applicability::MaybeIncorrect,
103-
);
104-
} else {
105-
diag.help("merge the temporary construction with its single usage");
106-
diag.span_note(apa.last_stmt_span, "single usage here");
105+
diag.help("merge the temporary construction with its single usage");
106+
diag.span_note(last_stmt_span, "single usage here");
107+
}
107108
}
108109
},
109110
_ => {
110-
diag.span_suggestion(
111-
apa.last_stmt_span.shrink_to_hi(),
112-
"drop the temporary after the end of its last usage",
113-
format!(
114-
"\n{}drop({});",
115-
" ".repeat(indent_of(cx, apa.last_stmt_span).unwrap_or(0)),
116-
first_bind_ident
117-
),
118-
Applicability::MaybeIncorrect,
119-
);
111+
if let Some(last_stmt_span) = apa.last_stmt_span {
112+
diag.span_suggestion(
113+
last_stmt_span.shrink_to_hi(),
114+
"drop the temporary after the end of its last usage",
115+
format!(
116+
"\n{}drop({});",
117+
" ".repeat(indent_of(cx, last_stmt_span).unwrap_or(0)),
118+
first_bind_ident
119+
),
120+
Applicability::MaybeIncorrect,
121+
);
122+
}
120123
},
121124
};
122125
span_lint_and_then(
@@ -235,15 +238,18 @@ impl<'ap, 'lc, 'others, 'stmt, 'tcx> StmtsChecker<'ap, 'lc, 'others, 'stmt, 'tcx
235238
};
236239
if has_expensive_stmt {
237240
for apa in self.ap.apas.values_mut() {
238-
let last_stmt_is_not_dummy = apa.last_stmt_span != DUMMY_SP;
239-
let last_stmt_is_not_curr = self.ap.curr_stmt.span != apa.last_stmt_span;
240241
let block_equals_curr = self.ap.curr_block_hir_id == apa.first_block_hir_id;
241242
let block_is_ancestor = self
242243
.cx
243244
.tcx
244245
.hir_parent_iter(self.ap.curr_block_hir_id)
245246
.any(|(id, _)| id == apa.first_block_hir_id);
246-
if last_stmt_is_not_dummy && last_stmt_is_not_curr && (block_equals_curr || block_is_ancestor) {
247+
// last stmt is not dummy
248+
if let Some(last_stmt_span) = apa.last_stmt_span
249+
// last stmt is not current
250+
&& last_stmt_span != self.ap.curr_stmt.span
251+
&& (block_equals_curr || block_is_ancestor)
252+
{
247253
apa.has_expensive_expr_after_last_attr = true;
248254
}
249255
}
@@ -296,7 +302,12 @@ impl<'tcx> Visitor<'tcx> for StmtsChecker<'_, '_, '_, '_, 'tcx> {
296302
}
297303
},
298304
first_stmt_span: self.ap.curr_stmt.span,
299-
..Default::default()
305+
// default values
306+
counter: 0,
307+
has_expensive_expr_after_last_attr: false,
308+
last_bind_ident: None,
309+
last_method_span: None,
310+
last_stmt_span: None,
300311
};
301312
modify_apa_params(&mut apa);
302313
let _ = self.ap.apas.insert(hir_id, apa);
@@ -321,7 +332,7 @@ impl<'tcx> Visitor<'tcx> for StmtsChecker<'_, '_, '_, '_, 'tcx> {
321332
hir::StmtKind::Semi(semi_expr) => {
322333
if has_drop(semi_expr, apa.first_bind_ident, self.cx) {
323334
apa.has_expensive_expr_after_last_attr = false;
324-
apa.last_stmt_span = DUMMY_SP;
335+
apa.last_stmt_span = None;
325336
return;
326337
}
327338
if let hir::ExprKind::MethodCall(_, _, _, span) = semi_expr.kind {
@@ -330,7 +341,7 @@ impl<'tcx> Visitor<'tcx> for StmtsChecker<'_, '_, '_, '_, 'tcx> {
330341
},
331342
_ => {},
332343
}
333-
apa.last_stmt_span = self.ap.curr_stmt.span;
344+
apa.last_stmt_span = Some(self.ap.curr_stmt.span);
334345
modify_apa_params(apa);
335346
}
336347
}
@@ -388,24 +399,7 @@ struct AuxParamsAttr {
388399
/// Similar to `last_bind_span` but encompasses the right-hand method call.
389400
last_method_span: Option<Span>,
390401
/// Similar to `last_bind_span` but encompasses the whole contained statement.
391-
last_stmt_span: Span,
392-
}
393-
394-
impl Default for AuxParamsAttr {
395-
fn default() -> Self {
396-
Self {
397-
counter: 0,
398-
has_expensive_expr_after_last_attr: false,
399-
first_block_hir_id: HirId::INVALID,
400-
first_block_span: DUMMY_SP,
401-
first_bind_ident: None,
402-
first_method_span: DUMMY_SP,
403-
first_stmt_span: DUMMY_SP,
404-
last_bind_ident: None,
405-
last_method_span: None,
406-
last_stmt_span: DUMMY_SP,
407-
}
408-
}
402+
last_stmt_span: Option<Span>,
409403
}
410404

411405
fn dummy_stmt_expr<'any>(expr: &'any hir::Expr<'any>) -> hir::Stmt<'any> {

0 commit comments

Comments
 (0)