Skip to content

Commit d1831bf

Browse files
dingxiangfei2009ehuss
authored andcommitted
new suggestion format with extracted variable name
1 parent a78c1c2 commit d1831bf

File tree

4 files changed

+290
-121
lines changed

4 files changed

+290
-121
lines changed

compiler/rustc_mir_transform/messages.ftl

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,17 @@ mir_transform_tail_expr_drop_order = relative drop order changing in Rust 2024
3535
.drop_location =
3636
temporary will be dropped on exiting the block, before the block's local variables
3737
.note_epilogue = most of the time, changing drop order is harmless; inspect the `impl Drop`s for side effects
38+
.label_local_epilogue = {$is_dropped_first_edition_2024 ->
39+
[true] up until Edition 2021 `{$name}` is dropped last but will be dropped earlier in Edition 2024
40+
*[false] `{$name}` will be dropped later since Edition 2024
41+
}
42+
43+
mir_transform_tail_expr_dtor = `{$name}` invokes this custom destructor
44+
45+
mir_transform_tail_expr_local = {$is_generated_name ->
46+
[true] this calls a custom destructor and let us call it `{$name}`
47+
*[false] `{$name}` calls a custom destructor
48+
}
3849
3950
mir_transform_unaligned_packed_ref = reference to packed field is unaligned
4051
.note = packed structs are only aligned by one byte, and many modern architectures penalize unaligned field accesses

compiler/rustc_mir_transform/src/lint_tail_expr_drop_order.rs

Lines changed: 138 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -4,21 +4,23 @@ use std::rc::Rc;
44

55
use rustc_data_structures::fx::{FxHashMap, FxHashSet};
66
use rustc_data_structures::unord::UnordSet;
7+
use rustc_errors::Subdiagnostic;
78
use rustc_hir::CRATE_HIR_ID;
89
use rustc_hir::def_id::{DefId, LocalDefId};
910
use rustc_index::bit_set::ChunkedBitSet;
1011
use rustc_index::{IndexSlice, IndexVec};
11-
use rustc_macros::LintDiagnostic;
12+
use rustc_macros::{LintDiagnostic, Subdiagnostic};
13+
use rustc_middle::bug;
1214
use rustc_middle::mir::{
13-
BasicBlock, Body, ClearCrossCrate, Local, Location, Place, StatementKind, TerminatorKind,
15+
self, BasicBlock, Body, ClearCrossCrate, Local, Location, Place, StatementKind, TerminatorKind,
1416
dump_mir,
1517
};
1618
use rustc_middle::ty::{self, Ty, TyCtxt};
1719
use rustc_mir_dataflow::impls::MaybeInitializedPlaces;
1820
use rustc_mir_dataflow::move_paths::{LookupResult, MoveData, MovePathIndex};
1921
use rustc_mir_dataflow::{Analysis, MaybeReachable, ResultsCursor};
2022
use rustc_session::lint;
21-
use rustc_span::Span;
23+
use rustc_span::{Span, Symbol};
2224
use rustc_type_ir::data_structures::IndexMap;
2325
use smallvec::{SmallVec, smallvec};
2426
use tracing::{debug, instrument};
@@ -279,6 +281,7 @@ pub(crate) fn run_lint<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId, body: &Body<
279281
}
280282

281283
dump_mir(tcx, false, "lint_tail_expr_drop_order", &0 as _, body, |_, _| Ok(()));
284+
let locals_with_user_names = collect_user_names(body);
282285
let param_env = tcx.param_env(def_id).with_reveal_all_normalized(tcx);
283286
let is_closure_like = tcx.is_closure_like(def_id.to_def_id());
284287
let move_data = MoveData::gather_moves(body, tcx, |_| true);
@@ -287,6 +290,8 @@ pub(crate) fn run_lint<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId, body: &Body<
287290
let mut block_drop_value_info =
288291
IndexVec::from_elem_n(MovePathIndexAtBlock::Unknown, body.basic_blocks.len());
289292
for (&block, candidates) in &bid_per_block {
293+
// We will collect drops on locals on paths between BID points to their actual drop locations
294+
// into `all_locals_dropped`.
290295
let mut all_locals_dropped = ChunkedBitSet::new_empty(move_data.move_paths.len());
291296
let mut drop_span = None;
292297
for &(_, place) in candidates.iter() {
@@ -352,16 +357,26 @@ pub(crate) fn run_lint<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId, body: &Body<
352357
all_locals_dropped.subtract(&to_exclude);
353358
}
354359
if all_locals_dropped.is_empty() {
360+
// No drop effect is observable, so let us move on.
355361
continue;
356362
}
363+
let local_names = assign_observables_names(
364+
all_locals_dropped
365+
.iter()
366+
.map(|path_idx| move_data.move_paths[path_idx].place.local)
367+
.chain(candidates.iter().map(|(_, place)| place.local)),
368+
&locals_with_user_names,
369+
);
357370

358371
let mut lint_root = None;
359-
let mut linted_spans = Vec::with_capacity(candidates.len());
360-
let mut tys = Vec::with_capacity(candidates.len());
372+
let mut local_labels = vec![];
361373
// We now collect the types with custom destructors.
362374
for &(_, place) in candidates {
363375
let linted_local_decl = &body.local_decls[place.local];
364-
linted_spans.push(linted_local_decl.source_info.span);
376+
let Some(&(ref name, is_generated_name)) = local_names.get(&place.local) else {
377+
bug!("a name should have been assigned")
378+
};
379+
let name = name.as_str();
365380

366381
if lint_root.is_none()
367382
&& let ClearCrossCrate::Set(data) =
@@ -370,62 +385,139 @@ pub(crate) fn run_lint<'tcx>(tcx: TyCtxt<'tcx>, def_id: LocalDefId, body: &Body<
370385
lint_root = Some(data.lint_root);
371386
}
372387

373-
tys.extend(extract_component_with_significant_dtor(
374-
tcx,
375-
param_env,
376-
linted_local_decl.ty,
377-
));
388+
// Collect spans of the custom destructors.
389+
let destructors =
390+
extract_component_with_significant_dtor(tcx, param_env, linted_local_decl.ty)
391+
.into_iter()
392+
.filter_map(|ty| ty_dtor_span(tcx, ty))
393+
.map(|span| DestructorLabel { span, name })
394+
.collect();
395+
local_labels.push(LocalLabel {
396+
span: linted_local_decl.source_info.span,
397+
destructors,
398+
name,
399+
is_generated_name,
400+
is_dropped_first_edition_2024: true,
401+
});
378402
}
379-
// Collect spans of the custom destructors.
380-
let linted_dtors = tys.into_iter().filter_map(|ty| ty_dtor_span(tcx, ty)).collect();
381403

382404
// Similarly, custom destructors of the observed drops.
383-
let mut observer_spans = Vec::with_capacity(all_locals_dropped.count());
384-
let mut observer_tys = Vec::with_capacity(all_locals_dropped.count());
385405
for path_idx in all_locals_dropped.iter() {
386-
let move_path = &move_data.move_paths[path_idx];
387-
let observer_ty = move_path.place.ty(body, tcx).ty;
406+
let place = &move_data.move_paths[path_idx].place;
407+
// We are not using the type of the local because the drop may be partial.
408+
let observer_ty = place.ty(body, tcx).ty;
409+
410+
let observer_local_decl = &body.local_decls[place.local];
411+
let Some(&(ref name, is_generated_name)) = local_names.get(&place.local) else {
412+
bug!("a name should have been assigned")
413+
};
414+
let name = name.as_str();
388415

389-
let observer_local_decl = &body.local_decls[move_path.place.local];
390-
observer_spans.push(observer_local_decl.source_info.span);
391-
observer_tys.extend(extract_component_with_significant_dtor(
392-
tcx,
393-
param_env,
394-
observer_ty,
395-
));
416+
let destructors = extract_component_with_significant_dtor(tcx, param_env, observer_ty)
417+
.into_iter()
418+
.filter_map(|ty| ty_dtor_span(tcx, ty))
419+
.map(|span| DestructorLabel { span, name })
420+
.collect();
421+
local_labels.push(LocalLabel {
422+
span: observer_local_decl.source_info.span,
423+
destructors,
424+
name,
425+
is_generated_name,
426+
is_dropped_first_edition_2024: false,
427+
});
396428
}
397-
let observer_dtors =
398-
observer_tys.into_iter().filter_map(|ty| ty_dtor_span(tcx, ty)).collect();
399429

430+
let span = local_labels[0].span;
400431
tcx.emit_node_span_lint(
401432
lint::builtin::TAIL_EXPR_DROP_ORDER,
402433
lint_root.unwrap_or(CRATE_HIR_ID),
403-
linted_spans[0],
404-
TailExprDropOrderLint {
405-
linted_spans,
406-
linted_dtors,
407-
observer_spans,
408-
observer_dtors,
409-
drop_span,
410-
_epilogue: (),
411-
},
434+
span,
435+
TailExprDropOrderLint { local_labels, drop_span, _epilogue: () },
412436
);
413437
}
414438
}
415439

440+
fn collect_user_names(body: &Body<'_>) -> IndexMap<Local, Symbol> {
441+
let mut names = IndexMap::default();
442+
for var_debug_info in &body.var_debug_info {
443+
if let mir::VarDebugInfoContents::Place(place) = &var_debug_info.value
444+
&& let Some(local) = place.local_or_deref_local()
445+
{
446+
names.entry(local).or_insert(var_debug_info.name);
447+
}
448+
}
449+
names
450+
}
451+
452+
fn assign_observables_names(
453+
locals: impl IntoIterator<Item = Local>,
454+
user_names: &IndexMap<Local, Symbol>,
455+
) -> IndexMap<Local, (String, bool)> {
456+
let mut names = IndexMap::default();
457+
let mut assigned_names = FxHashSet::default();
458+
let mut idx = 0u64;
459+
let mut fresh_name = || {
460+
idx += 1;
461+
(format!("#{idx}"), true)
462+
};
463+
for local in locals {
464+
let name = if let Some(name) = user_names.get(&local) {
465+
let name = name.as_str();
466+
if assigned_names.contains(name) { fresh_name() } else { (name.to_owned(), false) }
467+
} else {
468+
fresh_name()
469+
};
470+
assigned_names.insert(name.0.clone());
471+
names.insert(local, name);
472+
}
473+
names
474+
}
475+
416476
#[derive(LintDiagnostic)]
417477
#[diag(mir_transform_tail_expr_drop_order)]
418-
struct TailExprDropOrderLint {
419-
#[label(mir_transform_temporaries)]
420-
pub linted_spans: Vec<Span>,
421-
#[note(mir_transform_note_dtors)]
422-
pub linted_dtors: Vec<Span>,
423-
#[label(mir_transform_observers)]
424-
pub observer_spans: Vec<Span>,
425-
#[note(mir_transform_note_observer_dtors)]
426-
pub observer_dtors: Vec<Span>,
478+
struct TailExprDropOrderLint<'a> {
479+
#[subdiagnostic]
480+
local_labels: Vec<LocalLabel<'a>>,
427481
#[label(mir_transform_drop_location)]
428-
pub drop_span: Option<Span>,
482+
drop_span: Option<Span>,
429483
#[note(mir_transform_note_epilogue)]
430-
pub _epilogue: (),
484+
_epilogue: (),
485+
}
486+
487+
struct LocalLabel<'a> {
488+
span: Span,
489+
name: &'a str,
490+
is_generated_name: bool,
491+
is_dropped_first_edition_2024: bool,
492+
destructors: Vec<DestructorLabel<'a>>,
493+
}
494+
495+
impl Subdiagnostic for LocalLabel<'_> {
496+
fn add_to_diag_with<
497+
G: rustc_errors::EmissionGuarantee,
498+
F: rustc_errors::SubdiagMessageOp<G>,
499+
>(
500+
self,
501+
diag: &mut rustc_errors::Diag<'_, G>,
502+
f: &F,
503+
) {
504+
diag.arg("name", self.name);
505+
diag.arg("is_generated_name", self.is_generated_name);
506+
diag.arg("is_dropped_first_edition_2024", self.is_dropped_first_edition_2024);
507+
let msg = f(diag, crate::fluent_generated::mir_transform_tail_expr_local.into());
508+
diag.span_label(self.span, msg);
509+
for dtor in self.destructors {
510+
dtor.add_to_diag_with(diag, f);
511+
}
512+
let msg = f(diag, crate::fluent_generated::mir_transform_label_local_epilogue.into());
513+
diag.span_label(self.span, msg);
514+
}
515+
}
516+
517+
#[derive(Subdiagnostic)]
518+
#[note(mir_transform_tail_expr_dtor)]
519+
struct DestructorLabel<'a> {
520+
#[primary_span]
521+
span: Span,
522+
name: &'a str,
431523
}

0 commit comments

Comments
 (0)