Skip to content

Commit cd91954

Browse files
committed
Entirely hide Candidates from outside lower_match_tree
1 parent e06a010 commit cd91954

File tree

2 files changed

+64
-38
lines changed

2 files changed

+64
-38
lines changed

compiler/rustc_mir_build/src/build/matches/mod.rs

+60-38
Original file line numberDiff line numberDiff line change
@@ -366,36 +366,21 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
366366
unpack!(block = self.lower_scrutinee(block, scrutinee_id, scrutinee_span));
367367

368368
let arms = arms.iter().map(|arm| &self.thir[*arm]);
369-
// Assemble the initial list of candidates. These top-level candidates are 1:1 with the
370-
// original match arms, but other parts of match lowering also introduce subcandidates (for
371-
// sub-or-patterns). So inside the algorithm, the candidates list may not correspond to
372-
// match arms directly.
373-
let candidates: Vec<_> = arms
369+
let match_start_span = span.shrink_to_lo().to(scrutinee_span);
370+
let patterns = arms
374371
.clone()
375372
.map(|arm| {
376-
let arm_has_guard = arm.guard.is_some();
377-
let arm_candidate =
378-
Candidate::new(scrutinee_place.clone(), &arm.pattern, arm_has_guard, self);
379-
arm_candidate
373+
let has_match_guard =
374+
if arm.guard.is_some() { HasMatchGuard::Yes } else { HasMatchGuard::No };
375+
(&*arm.pattern, has_match_guard)
380376
})
381377
.collect();
382-
383-
// The set of places that we are creating fake borrows of. If there are no match guards then
384-
// we don't need any fake borrows, so don't track them.
385-
let match_has_guard = candidates.iter().any(|candidate| candidate.has_guard);
386-
let fake_borrow_temps: Vec<(Place<'tcx>, Local, FakeBorrowKind)> = if match_has_guard {
387-
util::collect_fake_borrows(self, &candidates, scrutinee_span, scrutinee_place.base())
388-
} else {
389-
Vec::new()
390-
};
391-
392-
let match_start_span = span.shrink_to_lo().to(scrutinee_span);
393378
let built_tree = self.lower_match_tree(
394379
block,
395380
scrutinee_span,
396381
&scrutinee_place,
397382
match_start_span,
398-
candidates,
383+
patterns,
399384
false,
400385
);
401386

@@ -404,9 +389,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
404389
scrutinee_place,
405390
scrutinee_span,
406391
arms,
407-
built_tree.branches,
392+
built_tree,
408393
self.source_info(span),
409-
fake_borrow_temps,
410394
)
411395
}
412396

@@ -438,16 +422,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
438422
scrutinee_place_builder: PlaceBuilder<'tcx>,
439423
scrutinee_span: Span,
440424
arms: impl IntoIterator<Item = &'pat Arm<'tcx>>,
441-
lowered_branches: impl IntoIterator<Item = MatchTreeBranch<'tcx>>,
425+
built_match_tree: BuiltMatchTree<'tcx>,
442426
outer_source_info: SourceInfo,
443-
fake_borrow_temps: Vec<(Place<'tcx>, Local, FakeBorrowKind)>,
444427
) -> BlockAnd<()>
445428
where
446429
'tcx: 'pat,
447430
{
448431
let arm_end_blocks: Vec<BasicBlock> = arms
449432
.into_iter()
450-
.zip(lowered_branches)
433+
.zip(built_match_tree.branches)
451434
.map(|(arm, branch)| {
452435
debug!("lowering arm {:?}\ncorresponding branch = {:?}", arm, branch);
453436

@@ -483,7 +466,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
483466
let arm_block = this.bind_pattern(
484467
outer_source_info,
485468
branch,
486-
&fake_borrow_temps,
469+
&built_match_tree.fake_borrow_temps,
487470
scrutinee_span,
488471
Some((arm, match_scope)),
489472
EmitStorageLive::Yes,
@@ -700,13 +683,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
700683
initializer: PlaceBuilder<'tcx>,
701684
set_match_place: bool,
702685
) -> BlockAnd<()> {
703-
let candidate = Candidate::new(initializer.clone(), irrefutable_pat, false, self);
704686
let built_tree = self.lower_match_tree(
705687
block,
706688
irrefutable_pat.span,
707689
&initializer,
708690
irrefutable_pat.span,
709-
vec![candidate],
691+
vec![(irrefutable_pat, HasMatchGuard::No)],
710692
false,
711693
);
712694
let [branch] = built_tree.branches.try_into().unwrap();
@@ -1136,12 +1118,15 @@ impl<'tcx, 'pat> Candidate<'pat, 'tcx> {
11361118
fn new(
11371119
place: PlaceBuilder<'tcx>,
11381120
pattern: &'pat Pat<'tcx>,
1139-
has_guard: bool,
1121+
has_guard: HasMatchGuard,
11401122
cx: &mut Builder<'_, 'tcx>,
11411123
) -> Self {
11421124
// Use `FlatPat` to build simplified match pairs, then immediately
11431125
// incorporate them into a new candidate.
1144-
Self::from_flat_pat(FlatPat::new(place, pattern, cx), has_guard)
1126+
Self::from_flat_pat(
1127+
FlatPat::new(place, pattern, cx),
1128+
matches!(has_guard, HasMatchGuard::Yes),
1129+
)
11451130
}
11461131

11471132
/// Incorporates an already-simplified [`FlatPat`] into a new candidate.
@@ -1437,6 +1422,10 @@ struct MatchTreeBranch<'tcx> {
14371422
struct BuiltMatchTree<'tcx> {
14381423
branches: Vec<MatchTreeBranch<'tcx>>,
14391424
otherwise_block: BasicBlock,
1425+
/// If any of the branches had a guard, we collect here the places and locals to fakely borrow
1426+
/// to ensure match guards can't modify the values as we match them. For more details, see
1427+
/// [`util::collect_fake_borrows`].
1428+
fake_borrow_temps: Vec<(Place<'tcx>, Local, FakeBorrowKind)>,
14401429
}
14411430

14421431
impl<'tcx> MatchTreeSubBranch<'tcx> {
@@ -1487,12 +1476,18 @@ impl<'tcx> MatchTreeBranch<'tcx> {
14871476
}
14881477
}
14891478

1479+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
1480+
enum HasMatchGuard {
1481+
Yes,
1482+
No,
1483+
}
1484+
14901485
impl<'a, 'tcx> Builder<'a, 'tcx> {
14911486
/// The entrypoint of the matching algorithm. Create the decision tree for the match expression,
14921487
/// starting from `block`.
14931488
///
1494-
/// Modifies `candidates` to store the bindings and type ascriptions for
1495-
/// that candidate.
1489+
/// `patterns` is a list of patterns, one for each arm. The associated boolean indicates whether
1490+
/// the arm has a guard.
14961491
///
14971492
/// `refutable` indicates whether the candidate list is refutable (for `if let` and `let else`)
14981493
/// or not (for `let` and `match`). In the refutable case we return the block to which we branch
@@ -1503,9 +1498,30 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
15031498
scrutinee_span: Span,
15041499
scrutinee_place_builder: &PlaceBuilder<'tcx>,
15051500
match_start_span: Span,
1506-
mut candidates: Vec<Candidate<'pat, 'tcx>>,
1501+
patterns: Vec<(&'pat Pat<'tcx>, HasMatchGuard)>,
15071502
refutable: bool,
1508-
) -> BuiltMatchTree<'tcx> {
1503+
) -> BuiltMatchTree<'tcx>
1504+
where
1505+
'tcx: 'pat,
1506+
{
1507+
// Assemble the initial list of candidates. These top-level candidates are 1:1 with the
1508+
// input patterns, but other parts of match lowering also introduce subcandidates (for
1509+
// sub-or-patterns). So inside the algorithm, the candidates list may not correspond to
1510+
// match arms directly.
1511+
let mut candidates: Vec<Candidate<'_, '_>> = patterns
1512+
.into_iter()
1513+
.map(|(pat, has_guard)| {
1514+
Candidate::new(scrutinee_place_builder.clone(), pat, has_guard, self)
1515+
})
1516+
.collect();
1517+
1518+
let fake_borrow_temps = util::collect_fake_borrows(
1519+
self,
1520+
&candidates,
1521+
scrutinee_span,
1522+
scrutinee_place_builder.base(),
1523+
);
1524+
15091525
// This will generate code to test scrutinee_place and branch to the appropriate arm block.
15101526
// If none of the arms match, we branch to `otherwise_block`. When lowering a `match`
15111527
// expression, exhaustiveness checking ensures that this block is unreachable.
@@ -1584,6 +1600,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
15841600
BuiltMatchTree {
15851601
branches: candidates.into_iter().map(MatchTreeBranch::from_candidate).collect(),
15861602
otherwise_block,
1603+
fake_borrow_temps,
15871604
}
15881605
}
15891606

@@ -2334,9 +2351,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
23342351
) -> BlockAnd<()> {
23352352
let expr_span = self.thir[expr_id].span;
23362353
let scrutinee = unpack!(block = self.lower_scrutinee(block, expr_id, expr_span));
2337-
let candidate = Candidate::new(scrutinee.clone(), pat, false, self);
2338-
let built_tree =
2339-
self.lower_match_tree(block, expr_span, &scrutinee, pat.span, vec![candidate], true);
2354+
let built_tree = self.lower_match_tree(
2355+
block,
2356+
expr_span,
2357+
&scrutinee,
2358+
pat.span,
2359+
vec![(pat, HasMatchGuard::No)],
2360+
true,
2361+
);
23402362
let [branch] = built_tree.branches.try_into().unwrap();
23412363

23422364
self.break_for_else(built_tree.otherwise_block, self.source_info(expr_span));

compiler/rustc_mir_build/src/build/matches/util.rs

+4
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,10 @@ pub(super) fn collect_fake_borrows<'tcx>(
7474
temp_span: Span,
7575
scrutinee_base: PlaceBase,
7676
) -> Vec<(Place<'tcx>, Local, FakeBorrowKind)> {
77+
if candidates.iter().all(|candidate| !candidate.has_guard) {
78+
// Fake borrows are only used when there is a guard.
79+
return Vec::new();
80+
}
7781
let mut collector =
7882
FakeBorrowCollector { cx, scrutinee_base, fake_borrows: FxIndexMap::default() };
7983
for candidate in candidates.iter() {

0 commit comments

Comments
 (0)