Skip to content

Commit 3d57040

Browse files
committed
Entirely hide Candidates from outside lower_match_tree
1 parent 7dbc49e commit 3d57040

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
@@ -359,36 +359,21 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
359359
unpack!(block = self.lower_scrutinee(block, scrutinee_id, scrutinee_span));
360360

361361
let arms = arms.iter().map(|arm| &self.thir[*arm]);
362-
// Assemble the initial list of candidates. These top-level candidates are 1:1 with the
363-
// original match arms, but other parts of match lowering also introduce subcandidates (for
364-
// sub-or-patterns). So inside the algorithm, the candidates list may not correspond to
365-
// match arms directly.
366-
let candidates: Vec<_> = arms
362+
let match_start_span = span.shrink_to_lo().to(scrutinee_span);
363+
let patterns = arms
367364
.clone()
368365
.map(|arm| {
369-
let arm_has_guard = arm.guard.is_some();
370-
let arm_candidate =
371-
Candidate::new(scrutinee_place.clone(), &arm.pattern, arm_has_guard, self);
372-
arm_candidate
366+
let has_match_guard =
367+
if arm.guard.is_some() { HasMatchGuard::Yes } else { HasMatchGuard::No };
368+
(&*arm.pattern, has_match_guard)
373369
})
374370
.collect();
375-
376-
// The set of places that we are creating fake borrows of. If there are no match guards then
377-
// we don't need any fake borrows, so don't track them.
378-
let match_has_guard = candidates.iter().any(|candidate| candidate.has_guard);
379-
let fake_borrow_temps: Vec<(Place<'tcx>, Local, FakeBorrowKind)> = if match_has_guard {
380-
util::collect_fake_borrows(self, &candidates, scrutinee_span, scrutinee_place.base())
381-
} else {
382-
Vec::new()
383-
};
384-
385-
let match_start_span = span.shrink_to_lo().to(scrutinee_span);
386371
let built_tree = self.lower_match_tree(
387372
block,
388373
scrutinee_span,
389374
&scrutinee_place,
390375
match_start_span,
391-
candidates,
376+
patterns,
392377
false,
393378
);
394379

@@ -397,9 +382,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
397382
scrutinee_place,
398383
scrutinee_span,
399384
arms,
400-
built_tree.branches,
385+
built_tree,
401386
self.source_info(span),
402-
fake_borrow_temps,
403387
)
404388
}
405389

@@ -431,16 +415,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
431415
scrutinee_place_builder: PlaceBuilder<'tcx>,
432416
scrutinee_span: Span,
433417
arms: impl IntoIterator<Item = &'pat Arm<'tcx>>,
434-
lowered_branches: impl IntoIterator<Item = MatchTreeBranch<'tcx>>,
418+
built_match_tree: BuiltMatchTree<'tcx>,
435419
outer_source_info: SourceInfo,
436-
fake_borrow_temps: Vec<(Place<'tcx>, Local, FakeBorrowKind)>,
437420
) -> BlockAnd<()>
438421
where
439422
'tcx: 'pat,
440423
{
441424
let arm_end_blocks: Vec<_> = arms
442425
.into_iter()
443-
.zip(lowered_branches)
426+
.zip(built_match_tree.branches)
444427
.map(|(arm, branch)| {
445428
debug!("lowering arm {:?}\ncorresponding branch = {:?}", arm, branch);
446429

@@ -476,7 +459,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
476459
let arm_block = this.bind_pattern(
477460
outer_source_info,
478461
branch,
479-
&fake_borrow_temps,
462+
&built_match_tree.fake_borrow_temps,
480463
scrutinee_span,
481464
Some((arm, match_scope)),
482465
EmitStorageLive::Yes,
@@ -690,13 +673,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
690673
initializer: PlaceBuilder<'tcx>,
691674
set_match_place: bool,
692675
) -> BlockAnd<()> {
693-
let candidate = Candidate::new(initializer.clone(), irrefutable_pat, false, self);
694676
let built_tree = self.lower_match_tree(
695677
block,
696678
irrefutable_pat.span,
697679
&initializer,
698680
irrefutable_pat.span,
699-
vec![candidate],
681+
vec![(irrefutable_pat, HasMatchGuard::No)],
700682
false,
701683
);
702684
let [branch] = built_tree.branches.try_into().unwrap();
@@ -1072,12 +1054,15 @@ impl<'tcx, 'pat> Candidate<'pat, 'tcx> {
10721054
fn new(
10731055
place: PlaceBuilder<'tcx>,
10741056
pattern: &'pat Pat<'tcx>,
1075-
has_guard: bool,
1057+
has_guard: HasMatchGuard,
10761058
cx: &mut Builder<'_, 'tcx>,
10771059
) -> Self {
10781060
// Use `FlatPat` to build simplified match pairs, then immediately
10791061
// incorporate them into a new candidate.
1080-
Self::from_flat_pat(FlatPat::new(place, pattern, cx), has_guard)
1062+
Self::from_flat_pat(
1063+
FlatPat::new(place, pattern, cx),
1064+
matches!(has_guard, HasMatchGuard::Yes),
1065+
)
10811066
}
10821067

10831068
/// Incorporates an already-simplified [`FlatPat`] into a new candidate.
@@ -1317,6 +1302,10 @@ struct MatchTreeBranch<'tcx> {
13171302
struct BuiltMatchTree<'tcx> {
13181303
branches: Vec<MatchTreeBranch<'tcx>>,
13191304
otherwise_block: BasicBlock,
1305+
/// If any of the branches had a guard, we collect here the places and locals to fakely borrow
1306+
/// to ensure match guards can't modify the values as we match them. For more details, see
1307+
/// [`util::collect_fake_borrows`].
1308+
fake_borrow_temps: Vec<(Place<'tcx>, Local, FakeBorrowKind)>,
13201309
}
13211310

13221311
impl<'tcx> MatchTreeSubBranch<'tcx> {
@@ -1367,12 +1356,18 @@ impl<'tcx> MatchTreeBranch<'tcx> {
13671356
}
13681357
}
13691358

1359+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
1360+
enum HasMatchGuard {
1361+
Yes,
1362+
No,
1363+
}
1364+
13701365
impl<'a, 'tcx> Builder<'a, 'tcx> {
13711366
/// The entrypoint of the matching algorithm. Create the decision tree for the match expression,
13721367
/// starting from `block`.
13731368
///
1374-
/// Modifies `candidates` to store the bindings and type ascriptions for
1375-
/// that candidate.
1369+
/// `patterns` is a list of patterns, one for each arm. The associated boolean indicates whether
1370+
/// the arm has a guard.
13761371
///
13771372
/// `refutable` indicates whether the candidate list is refutable (for `if let` and `let else`)
13781373
/// or not (for `let` and `match`). In the refutable case we return the block to which we branch
@@ -1383,9 +1378,30 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
13831378
scrutinee_span: Span,
13841379
scrutinee_place_builder: &PlaceBuilder<'tcx>,
13851380
match_start_span: Span,
1386-
mut candidates: Vec<Candidate<'pat, 'tcx>>,
1381+
patterns: Vec<(&'pat Pat<'tcx>, HasMatchGuard)>,
13871382
refutable: bool,
1388-
) -> BuiltMatchTree<'tcx> {
1383+
) -> BuiltMatchTree<'tcx>
1384+
where
1385+
'tcx: 'pat,
1386+
{
1387+
// Assemble the initial list of candidates. These top-level candidates are 1:1 with the
1388+
// input patterns, but other parts of match lowering also introduce subcandidates (for
1389+
// sub-or-patterns). So inside the algorithm, the candidates list may not correspond to
1390+
// match arms directly.
1391+
let mut candidates: Vec<_> = patterns
1392+
.into_iter()
1393+
.map(|(pat, has_guard)| {
1394+
Candidate::new(scrutinee_place_builder.clone(), pat, has_guard, self)
1395+
})
1396+
.collect();
1397+
1398+
let fake_borrow_temps = util::collect_fake_borrows(
1399+
self,
1400+
&candidates,
1401+
scrutinee_span,
1402+
scrutinee_place_builder.base(),
1403+
);
1404+
13891405
// See the doc comment on `match_candidates` for why we have an otherwise block.
13901406
let otherwise_block = self.cfg.start_new_block();
13911407

@@ -1471,6 +1487,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
14711487
BuiltMatchTree {
14721488
branches: candidates.into_iter().map(MatchTreeBranch::from_candidate).collect(),
14731489
otherwise_block,
1490+
fake_borrow_temps,
14741491
}
14751492
}
14761493

@@ -2185,9 +2202,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
21852202
) -> BlockAnd<()> {
21862203
let expr_span = self.thir[expr_id].span;
21872204
let scrutinee = unpack!(block = self.lower_scrutinee(block, expr_id, expr_span));
2188-
let candidate = Candidate::new(scrutinee.clone(), pat, false, self);
2189-
let built_tree =
2190-
self.lower_match_tree(block, expr_span, &scrutinee, pat.span, vec![candidate], true);
2205+
let built_tree = self.lower_match_tree(
2206+
block,
2207+
expr_span,
2208+
&scrutinee,
2209+
pat.span,
2210+
vec![(pat, HasMatchGuard::No)],
2211+
true,
2212+
);
21912213
let [branch] = built_tree.branches.try_into().unwrap();
21922214

21932215
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
@@ -309,6 +309,10 @@ pub(super) fn collect_fake_borrows<'tcx>(
309309
temp_span: Span,
310310
scrutinee_base: PlaceBase,
311311
) -> Vec<(Place<'tcx>, Local, FakeBorrowKind)> {
312+
if candidates.iter().all(|candidate| !candidate.has_guard) {
313+
// Fake borrows are only used when there is a guard.
314+
return Vec::new();
315+
}
312316
let mut collector =
313317
FakeBorrowCollector { cx, scrutinee_base, fake_borrows: FxIndexMap::default() };
314318
for candidate in candidates.iter() {

0 commit comments

Comments
 (0)