Skip to content

Commit e63d746

Browse files
authored
Rollup merge of #139666 - lcnr:pre-revealing-use-cleanup, r=compiler-errors
cleanup `mir_borrowck` Cleanup pulled out of #139587. Best reviewed commit by commit. r? compiler-errors
2 parents 40b2f75 + 420390c commit e63d746

File tree

11 files changed

+108
-154
lines changed

11 files changed

+108
-154
lines changed

Diff for: compiler/rustc_borrowck/src/lib.rs

+63-84
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ use std::cell::RefCell;
2222
use std::marker::PhantomData;
2323
use std::ops::{ControlFlow, Deref};
2424

25+
use borrow_set::LocalsStateAtExit;
2526
use root_cx::BorrowCheckRootCtxt;
2627
use rustc_abi::FieldIdx;
2728
use rustc_data_structures::fx::{FxIndexMap, FxIndexSet};
@@ -304,33 +305,13 @@ fn do_mir_borrowck<'tcx>(
304305
root_cx.set_tainted_by_errors(e);
305306
}
306307

307-
let mut local_names = IndexVec::from_elem(None, &input_body.local_decls);
308-
for var_debug_info in &input_body.var_debug_info {
309-
if let VarDebugInfoContents::Place(place) = var_debug_info.value {
310-
if let Some(local) = place.as_local() {
311-
if let Some(prev_name) = local_names[local]
312-
&& var_debug_info.name != prev_name
313-
{
314-
span_bug!(
315-
var_debug_info.source_info.span,
316-
"local {:?} has many names (`{}` vs `{}`)",
317-
local,
318-
prev_name,
319-
var_debug_info.name
320-
);
321-
}
322-
local_names[local] = Some(var_debug_info.name);
323-
}
324-
}
325-
}
326-
327308
// Replace all regions with fresh inference variables. This
328309
// requires first making our own copy of the MIR. This copy will
329310
// be modified (in place) to contain non-lexical lifetimes. It
330311
// will have a lifetime tied to the inference context.
331312
let mut body_owned = input_body.clone();
332313
let mut promoted = input_promoted.to_owned();
333-
let free_regions = nll::replace_regions_in_mir(&infcx, &mut body_owned, &mut promoted);
314+
let universal_regions = nll::replace_regions_in_mir(&infcx, &mut body_owned, &mut promoted);
334315
let body = &body_owned; // no further changes
335316

336317
let location_table = PoloniusLocationTable::new(body);
@@ -355,7 +336,7 @@ fn do_mir_borrowck<'tcx>(
355336
} = nll::compute_regions(
356337
root_cx,
357338
&infcx,
358-
free_regions,
339+
universal_regions,
359340
body,
360341
&promoted,
361342
&location_table,
@@ -368,24 +349,23 @@ fn do_mir_borrowck<'tcx>(
368349
// Dump MIR results into a file, if that is enabled. This lets us
369350
// write unit-tests, as well as helping with debugging.
370351
nll::dump_nll_mir(&infcx, body, &regioncx, &opt_closure_req, &borrow_set);
352+
polonius::dump_polonius_mir(
353+
&infcx,
354+
body,
355+
&regioncx,
356+
&opt_closure_req,
357+
&borrow_set,
358+
polonius_diagnostics.as_ref(),
359+
);
371360

372361
// We also have a `#[rustc_regions]` annotation that causes us to dump
373362
// information.
374-
let diags_buffer = &mut BorrowckDiagnosticsBuffer::default();
375-
nll::dump_annotation(&infcx, body, &regioncx, &opt_closure_req, diags_buffer);
376-
377-
let movable_coroutine =
378-
// The first argument is the coroutine type passed by value
379-
if let Some(local) = body.local_decls.raw.get(1)
380-
// Get the interior types and args which typeck computed
381-
&& let ty::Coroutine(def_id, _) = *local.ty.kind()
382-
&& tcx.coroutine_movability(def_id) == hir::Movability::Movable
383-
{
384-
true
385-
} else {
386-
false
387-
};
363+
nll::dump_annotation(&infcx, body, &regioncx, &opt_closure_req);
388364

365+
let movable_coroutine = body.coroutine.is_some()
366+
&& tcx.coroutine_movability(def.to_def_id()) == hir::Movability::Movable;
367+
368+
let diags_buffer = &mut BorrowckDiagnosticsBuffer::default();
389369
// While promoteds should mostly be correct by construction, we need to check them for
390370
// invalid moves to detect moving out of arrays:`struct S; fn main() { &([S][0]); }`.
391371
for promoted_body in &promoted {
@@ -403,7 +383,6 @@ fn do_mir_borrowck<'tcx>(
403383
location_table: &location_table,
404384
movable_coroutine,
405385
fn_self_span_reported: Default::default(),
406-
locals_are_invalidated_at_exit,
407386
access_place_error_reported: Default::default(),
408387
reservation_error_reported: Default::default(),
409388
uninitialized_error_reported: Default::default(),
@@ -435,14 +414,33 @@ fn do_mir_borrowck<'tcx>(
435414
promoted_mbcx.report_move_errors();
436415
}
437416

417+
let mut local_names = IndexVec::from_elem(None, &body.local_decls);
418+
for var_debug_info in &body.var_debug_info {
419+
if let VarDebugInfoContents::Place(place) = var_debug_info.value {
420+
if let Some(local) = place.as_local() {
421+
if let Some(prev_name) = local_names[local]
422+
&& var_debug_info.name != prev_name
423+
{
424+
span_bug!(
425+
var_debug_info.source_info.span,
426+
"local {:?} has many names (`{}` vs `{}`)",
427+
local,
428+
prev_name,
429+
var_debug_info.name
430+
);
431+
}
432+
local_names[local] = Some(var_debug_info.name);
433+
}
434+
}
435+
}
436+
438437
let mut mbcx = MirBorrowckCtxt {
439438
root_cx,
440439
infcx: &infcx,
441440
body,
442441
move_data: &move_data,
443442
location_table: &location_table,
444443
movable_coroutine,
445-
locals_are_invalidated_at_exit,
446444
fn_self_span_reported: Default::default(),
447445
access_place_error_reported: Default::default(),
448446
reservation_error_reported: Default::default(),
@@ -455,9 +453,9 @@ fn do_mir_borrowck<'tcx>(
455453
local_names,
456454
region_names: RefCell::default(),
457455
next_region_name: RefCell::new(1),
458-
polonius_output,
459456
move_errors: Vec::new(),
460457
diags_buffer,
458+
polonius_output: polonius_output.as_deref(),
461459
polonius_diagnostics: polonius_diagnostics.as_ref(),
462460
};
463461

@@ -474,16 +472,6 @@ fn do_mir_borrowck<'tcx>(
474472

475473
mbcx.report_move_errors();
476474

477-
// If requested, dump polonius MIR.
478-
polonius::dump_polonius_mir(
479-
&infcx,
480-
body,
481-
&regioncx,
482-
&borrow_set,
483-
polonius_diagnostics.as_ref(),
484-
&opt_closure_req,
485-
);
486-
487475
// For each non-user used mutable variable, check if it's been assigned from
488476
// a user-declared local. If so, then put that local into the used_mut set.
489477
// Note that this set is expected to be small - only upvars from closures
@@ -514,15 +502,14 @@ fn do_mir_borrowck<'tcx>(
514502
};
515503

516504
let body_with_facts = if consumer_options.is_some() {
517-
let output_facts = mbcx.polonius_output;
518505
Some(Box::new(BodyWithBorrowckFacts {
519506
body: body_owned,
520507
promoted,
521508
borrow_set,
522509
region_inference_context: regioncx,
523510
location_table: polonius_input.as_ref().map(|_| location_table),
524511
input_facts: polonius_input,
525-
output_facts,
512+
output_facts: polonius_output,
526513
}))
527514
} else {
528515
None
@@ -655,13 +642,6 @@ struct MirBorrowckCtxt<'a, 'infcx, 'tcx> {
655642
location_table: &'a PoloniusLocationTable,
656643

657644
movable_coroutine: bool,
658-
/// This keeps track of whether local variables are free-ed when the function
659-
/// exits even without a `StorageDead`, which appears to be the case for
660-
/// constants.
661-
///
662-
/// I'm not sure this is the right approach - @eddyb could you try and
663-
/// figure this out?
664-
locals_are_invalidated_at_exit: bool,
665645
/// This field keeps track of when borrow errors are reported in the access_place function
666646
/// so that there is no duplicate reporting. This field cannot also be used for the conflicting
667647
/// borrow errors that is handled by the `reservation_error_reported` field as the inclusion
@@ -709,12 +689,11 @@ struct MirBorrowckCtxt<'a, 'infcx, 'tcx> {
709689
/// The counter for generating new region names.
710690
next_region_name: RefCell<usize>,
711691

712-
/// Results of Polonius analysis.
713-
polonius_output: Option<Box<PoloniusOutput>>,
714-
715692
diags_buffer: &'a mut BorrowckDiagnosticsBuffer<'infcx, 'tcx>,
716693
move_errors: Vec<MoveError<'tcx>>,
717694

695+
/// Results of Polonius analysis.
696+
polonius_output: Option<&'a PoloniusOutput>,
718697
/// When using `-Zpolonius=next`: the data used to compute errors and diagnostics.
719698
polonius_diagnostics: Option<&'a PoloniusDiagnosticsContext>,
720699
}
@@ -938,13 +917,20 @@ impl<'a, 'tcx> ResultsVisitor<'a, 'tcx, Borrowck<'a, 'tcx>> for MirBorrowckCtxt<
938917
| TerminatorKind::Return
939918
| TerminatorKind::TailCall { .. }
940919
| TerminatorKind::CoroutineDrop => {
941-
// Returning from the function implicitly kills storage for all locals and statics.
942-
// Often, the storage will already have been killed by an explicit
943-
// StorageDead, but we don't always emit those (notably on unwind paths),
944-
// so this "extra check" serves as a kind of backup.
945-
for i in state.borrows.iter() {
946-
let borrow = &self.borrow_set[i];
947-
self.check_for_invalidation_at_exit(loc, borrow, span);
920+
match self.borrow_set.locals_state_at_exit() {
921+
LocalsStateAtExit::AllAreInvalidated => {
922+
// Returning from the function implicitly kills storage for all locals and statics.
923+
// Often, the storage will already have been killed by an explicit
924+
// StorageDead, but we don't always emit those (notably on unwind paths),
925+
// so this "extra check" serves as a kind of backup.
926+
for i in state.borrows.iter() {
927+
let borrow = &self.borrow_set[i];
928+
self.check_for_invalidation_at_exit(loc, borrow, span);
929+
}
930+
}
931+
// If we do not implicitly invalidate all locals on exit,
932+
// we check for conflicts when dropping or moving this local.
933+
LocalsStateAtExit::SomeAreInvalidated { has_storage_dead_or_moved: _ } => {}
948934
}
949935
}
950936

@@ -1716,22 +1702,15 @@ impl<'a, 'tcx> MirBorrowckCtxt<'a, '_, 'tcx> {
17161702
// we'll have a memory leak) and assume that all statics have a destructor.
17171703
//
17181704
// FIXME: allow thread-locals to borrow other thread locals?
1719-
1720-
let (might_be_alive, will_be_dropped) =
1721-
if self.body.local_decls[root_place.local].is_ref_to_thread_local() {
1722-
// Thread-locals might be dropped after the function exits
1723-
// We have to dereference the outer reference because
1724-
// borrows don't conflict behind shared references.
1725-
root_place.projection = TyCtxtConsts::DEREF_PROJECTION;
1726-
(true, true)
1727-
} else {
1728-
(false, self.locals_are_invalidated_at_exit)
1729-
};
1730-
1731-
if !will_be_dropped {
1732-
debug!("place_is_invalidated_at_exit({:?}) - won't be dropped", place);
1733-
return;
1734-
}
1705+
let might_be_alive = if self.body.local_decls[root_place.local].is_ref_to_thread_local() {
1706+
// Thread-locals might be dropped after the function exits
1707+
// We have to dereference the outer reference because
1708+
// borrows don't conflict behind shared references.
1709+
root_place.projection = TyCtxtConsts::DEREF_PROJECTION;
1710+
true
1711+
} else {
1712+
false
1713+
};
17351714

17361715
let sd = if might_be_alive { Deep } else { Shallow(None) };
17371716

Diff for: compiler/rustc_borrowck/src/nll.rs

+4-17
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ use tracing::{debug, instrument};
2121

2222
use crate::borrow_set::BorrowSet;
2323
use crate::consumers::ConsumerOptions;
24-
use crate::diagnostics::{BorrowckDiagnosticsBuffer, RegionErrors};
24+
use crate::diagnostics::RegionErrors;
2525
use crate::polonius::PoloniusDiagnosticsContext;
2626
use crate::polonius::legacy::{
2727
PoloniusFacts, PoloniusFactsExt, PoloniusLocationTable, PoloniusOutput,
@@ -117,11 +117,6 @@ pub(crate) fn compute_regions<'a, 'tcx>(
117117
Rc::clone(&location_map),
118118
);
119119

120-
// Create the region inference context, taking ownership of the
121-
// region inference data that was contained in `infcx`, and the
122-
// base constraints generated by the type-check.
123-
let var_infos = infcx.get_region_var_infos();
124-
125120
// If requested, emit legacy polonius facts.
126121
polonius::legacy::emit_facts(
127122
&mut polonius_facts,
@@ -134,13 +129,8 @@ pub(crate) fn compute_regions<'a, 'tcx>(
134129
&constraints,
135130
);
136131

137-
let mut regioncx = RegionInferenceContext::new(
138-
infcx,
139-
var_infos,
140-
constraints,
141-
universal_region_relations,
142-
location_map,
143-
);
132+
let mut regioncx =
133+
RegionInferenceContext::new(infcx, constraints, universal_region_relations, location_map);
144134

145135
// If requested for `-Zpolonius=next`, convert NLL constraints to localized outlives constraints
146136
// and use them to compute loan liveness.
@@ -297,7 +287,6 @@ pub(super) fn dump_annotation<'tcx, 'infcx>(
297287
body: &Body<'tcx>,
298288
regioncx: &RegionInferenceContext<'tcx>,
299289
closure_region_requirements: &Option<ClosureRegionRequirements<'tcx>>,
300-
diagnostics_buffer: &mut BorrowckDiagnosticsBuffer<'infcx, 'tcx>,
301290
) {
302291
let tcx = infcx.tcx;
303292
let base_def_id = tcx.typeck_root_def_id(body.source.def_id());
@@ -335,13 +324,11 @@ pub(super) fn dump_annotation<'tcx, 'infcx>(
335324
} else {
336325
let mut err = infcx.dcx().struct_span_note(def_span, "no external requirements");
337326
regioncx.annotate(tcx, &mut err);
338-
339327
err
340328
};
341329

342330
// FIXME(@lcnr): We currently don't dump the inferred hidden types here.
343-
344-
diagnostics_buffer.buffer_non_error(err);
331+
err.emit();
345332
}
346333

347334
fn for_each_region_constraint<'tcx>(

Diff for: compiler/rustc_borrowck/src/polonius/dump.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@ pub(crate) fn dump_polonius_mir<'tcx>(
2424
infcx: &BorrowckInferCtxt<'tcx>,
2525
body: &Body<'tcx>,
2626
regioncx: &RegionInferenceContext<'tcx>,
27+
closure_region_requirements: &Option<ClosureRegionRequirements<'tcx>>,
2728
borrow_set: &BorrowSet<'tcx>,
2829
polonius_diagnostics: Option<&PoloniusDiagnosticsContext>,
29-
closure_region_requirements: &Option<ClosureRegionRequirements<'tcx>>,
3030
) {
3131
let tcx = infcx.tcx;
3232
if !tcx.sess.opts.unstable_opts.polonius.is_next_enabled() {

0 commit comments

Comments
 (0)