Skip to content

Commit

Permalink
Auto merge of #96888 - Aaron1011:fake-borrow-no-sort, r=petrochenkov
Browse files Browse the repository at this point in the history
Use `FxIndexSet` to avoid sorting fake borrows

This fixes #96449, but I haven't yet been able to
make the reproducer work using `#[cfg]` attributes,
so we can't use the 'revision' infra to write a test

The previous implementation relied on sorting by `PlaceRef`.
This requires sorting by a `DefId`, which uses untracked state
(see #93315)
  • Loading branch information
bors committed May 11, 2022
2 parents 532be94 + aa0cc9c commit b862b43
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 15 deletions.
20 changes: 9 additions & 11 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::build::ForGuard::{self, OutsideGuard, RefWithinGuard};
use crate::build::{BlockAnd, BlockAndExtension, Builder};
use crate::build::{GuardFrame, GuardFrameLocal, LocalsForNode};
use rustc_data_structures::{
fx::{FxHashSet, FxIndexMap},
fx::{FxIndexMap, FxIndexSet},
stack::ensure_sufficient_stack,
};
use rustc_hir::HirId;
Expand Down Expand Up @@ -264,7 +264,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// The set of places that we are creating fake borrows of. If there are
// no match guards then we don't need any fake borrows, so don't track
// them.
let mut fake_borrows = match_has_guard.then(FxHashSet::default);
let mut fake_borrows = match_has_guard.then(FxIndexSet::default);

let mut otherwise = None;

Expand Down Expand Up @@ -1053,7 +1053,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
start_block: BasicBlock,
otherwise_block: &mut Option<BasicBlock>,
candidates: &mut [&mut Candidate<'pat, 'tcx>],
fake_borrows: &mut Option<FxHashSet<Place<'tcx>>>,
fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
) {
debug!(
"matched_candidate(span={:?}, candidates={:?}, start_block={:?}, otherwise_block={:?})",
Expand Down Expand Up @@ -1105,7 +1105,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
start_block: BasicBlock,
otherwise_block: &mut Option<BasicBlock>,
candidates: &mut [&mut Candidate<'_, 'tcx>],
fake_borrows: &mut Option<FxHashSet<Place<'tcx>>>,
fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
) {
// The candidates are sorted by priority. Check to see whether the
// higher priority candidates (and hence at the front of the slice)
Expand Down Expand Up @@ -1184,7 +1184,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
&mut self,
matched_candidates: &mut [&mut Candidate<'_, 'tcx>],
start_block: BasicBlock,
fake_borrows: &mut Option<FxHashSet<Place<'tcx>>>,
fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
) -> Option<BasicBlock> {
debug_assert!(
!matched_candidates.is_empty(),
Expand Down Expand Up @@ -1322,7 +1322,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
candidates: &mut [&mut Candidate<'_, 'tcx>],
block: BasicBlock,
otherwise_block: &mut Option<BasicBlock>,
fake_borrows: &mut Option<FxHashSet<Place<'tcx>>>,
fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
) {
let (first_candidate, remaining_candidates) = candidates.split_first_mut().unwrap();

Expand Down Expand Up @@ -1385,7 +1385,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
pats: &'pat [Pat<'tcx>],
or_span: Span,
place: PlaceBuilder<'tcx>,
fake_borrows: &mut Option<FxHashSet<Place<'tcx>>>,
fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
) {
debug!("test_or_pattern:\ncandidate={:#?}\npats={:#?}", candidate, pats);
let mut or_candidates: Vec<_> = pats
Expand Down Expand Up @@ -1572,7 +1572,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
mut candidates: &'b mut [&'c mut Candidate<'pat, 'tcx>],
block: BasicBlock,
otherwise_block: &mut Option<BasicBlock>,
fake_borrows: &mut Option<FxHashSet<Place<'tcx>>>,
fake_borrows: &mut Option<FxIndexSet<Place<'tcx>>>,
) {
// extract the match-pair from the highest priority candidate
let match_pair = &candidates.first().unwrap().match_pairs[0];
Expand Down Expand Up @@ -1715,7 +1715,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// by a MIR pass run after borrow checking.
fn calculate_fake_borrows<'b>(
&mut self,
fake_borrows: &'b FxHashSet<Place<'tcx>>,
fake_borrows: &'b FxIndexSet<Place<'tcx>>,
temp_span: Span,
) -> Vec<(Place<'tcx>, Local)> {
let tcx = self.tcx;
Expand All @@ -1741,8 +1741,6 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
all_fake_borrows.push(place.as_ref());
}

// Deduplicate and ensure a deterministic order.
all_fake_borrows.sort();
all_fake_borrows.dedup();

debug!("add_fake_borrows all_fake_borrows = {:?}", all_fake_borrows);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
let mut _0: i32; // return place in scope 0 at $DIR/remove_fake_borrows.rs:6:46: 6:49
let mut _3: isize; // in scope 0 at $DIR/remove_fake_borrows.rs:8:9: 8:16
let mut _4: &std::option::Option<&&i32>; // in scope 0 at $DIR/remove_fake_borrows.rs:7:11: 7:12
let mut _5: &&&i32; // in scope 0 at $DIR/remove_fake_borrows.rs:7:11: 7:12
let mut _6: &&i32; // in scope 0 at $DIR/remove_fake_borrows.rs:7:11: 7:12
let mut _5: &&i32; // in scope 0 at $DIR/remove_fake_borrows.rs:7:11: 7:12
let mut _6: &&&i32; // in scope 0 at $DIR/remove_fake_borrows.rs:7:11: 7:12
let mut _7: &i32; // in scope 0 at $DIR/remove_fake_borrows.rs:7:11: 7:12
let mut _8: bool; // in scope 0 at $DIR/remove_fake_borrows.rs:8:20: 8:21

Expand All @@ -34,8 +34,8 @@

bb4: {
- _4 = &shallow _1; // scope 0 at $DIR/remove_fake_borrows.rs:7:11: 7:12
- _5 = &shallow ((_1 as Some).0: &&i32); // scope 0 at $DIR/remove_fake_borrows.rs:7:11: 7:12
- _6 = &shallow (*((_1 as Some).0: &&i32)); // scope 0 at $DIR/remove_fake_borrows.rs:7:11: 7:12
- _5 = &shallow (*((_1 as Some).0: &&i32)); // scope 0 at $DIR/remove_fake_borrows.rs:7:11: 7:12
- _6 = &shallow ((_1 as Some).0: &&i32); // scope 0 at $DIR/remove_fake_borrows.rs:7:11: 7:12
- _7 = &shallow (*(*((_1 as Some).0: &&i32))); // scope 0 at $DIR/remove_fake_borrows.rs:7:11: 7:12
+ nop; // scope 0 at $DIR/remove_fake_borrows.rs:7:11: 7:12
+ nop; // scope 0 at $DIR/remove_fake_borrows.rs:7:11: 7:12
Expand Down

0 comments on commit b862b43

Please sign in to comment.