Skip to content

Commit e815f8f

Browse files
committed
Entirely hide Candidates from outside lower_match_tree
1 parent 5c67b08 commit e815f8f

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

310310
let arms = arms.iter().map(|arm| &self.thir[*arm]);
311-
// Assemble the initial list of candidates. These top-level candidates are 1:1 with the
312-
// original match arms, but other parts of match lowering also introduce subcandidates (for
313-
// sub-or-patterns). So inside the algorithm, the candidates list may not correspond to
314-
// match arms directly.
315-
let candidates: Vec<_> = arms
311+
let match_start_span = span.shrink_to_lo().to(scrutinee_span);
312+
let patterns = arms
316313
.clone()
317314
.map(|arm| {
318-
let arm_has_guard = arm.guard.is_some();
319-
let arm_candidate =
320-
Candidate::new(scrutinee_place.clone(), &arm.pattern, arm_has_guard, self);
321-
arm_candidate
315+
let has_match_guard =
316+
if arm.guard.is_some() { HasMatchGuard::Yes } else { HasMatchGuard::No };
317+
(&*arm.pattern, has_match_guard)
322318
})
323319
.collect();
324-
325-
// The set of places that we are creating fake borrows of. If there are no match guards then
326-
// we don't need any fake borrows, so don't track them.
327-
let match_has_guard = candidates.iter().any(|candidate| candidate.has_guard);
328-
let fake_borrow_temps: Vec<(Place<'tcx>, Local, FakeBorrowKind)> = if match_has_guard {
329-
util::collect_fake_borrows(self, &candidates, scrutinee_span, scrutinee_place.base())
330-
} else {
331-
Vec::new()
332-
};
333-
334-
let match_start_span = span.shrink_to_lo().to(scrutinee_span);
335320
let built_tree = self.lower_match_tree(
336321
block,
337322
scrutinee_span,
338323
&scrutinee_place,
339324
match_start_span,
340-
candidates,
325+
patterns,
341326
false,
342327
);
343328

@@ -346,9 +331,8 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
346331
scrutinee_place,
347332
scrutinee_span,
348333
arms,
349-
built_tree.branches,
334+
built_tree,
350335
self.source_info(span),
351-
fake_borrow_temps,
352336
)
353337
}
354338

@@ -380,16 +364,15 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
380364
scrutinee_place_builder: PlaceBuilder<'tcx>,
381365
scrutinee_span: Span,
382366
arms: impl IntoIterator<Item = &'pat Arm<'tcx>>,
383-
lowered_branches: impl IntoIterator<Item = MatchTreeBranch<'tcx>>,
367+
built_match_tree: BuiltMatchTree<'tcx>,
384368
outer_source_info: SourceInfo,
385-
fake_borrow_temps: Vec<(Place<'tcx>, Local, FakeBorrowKind)>,
386369
) -> BlockAnd<()>
387370
where
388371
'tcx: 'pat,
389372
{
390373
let arm_end_blocks: Vec<_> = arms
391374
.into_iter()
392-
.zip(lowered_branches)
375+
.zip(built_match_tree.branches)
393376
.map(|(arm, branch)| {
394377
debug!("lowering arm {:?}\ncorresponding branch = {:?}", arm, branch);
395378

@@ -425,7 +408,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
425408
let arm_block = this.bind_pattern(
426409
outer_source_info,
427410
branch,
428-
&fake_borrow_temps,
411+
&built_match_tree.fake_borrow_temps,
429412
scrutinee_span,
430413
Some((arm, match_scope)),
431414
false,
@@ -629,13 +612,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
629612
initializer: PlaceBuilder<'tcx>,
630613
set_match_place: bool,
631614
) -> BlockAnd<()> {
632-
let candidate = Candidate::new(initializer.clone(), irrefutable_pat, false, self);
633615
let built_tree = self.lower_match_tree(
634616
block,
635617
irrefutable_pat.span,
636618
&initializer,
637619
irrefutable_pat.span,
638-
vec![candidate],
620+
vec![(irrefutable_pat, HasMatchGuard::No)],
639621
false,
640622
);
641623
let [branch] = built_tree.branches.try_into().unwrap();
@@ -1009,12 +991,15 @@ impl<'tcx, 'pat> Candidate<'pat, 'tcx> {
1009991
fn new(
1010992
place: PlaceBuilder<'tcx>,
1011993
pattern: &'pat Pat<'tcx>,
1012-
has_guard: bool,
994+
has_guard: HasMatchGuard,
1013995
cx: &mut Builder<'_, 'tcx>,
1014996
) -> Self {
1015997
// Use `FlatPat` to build simplified match pairs, then immediately
1016998
// incorporate them into a new candidate.
1017-
Self::from_flat_pat(FlatPat::new(place, pattern, cx), has_guard)
999+
Self::from_flat_pat(
1000+
FlatPat::new(place, pattern, cx),
1001+
matches!(has_guard, HasMatchGuard::Yes),
1002+
)
10181003
}
10191004

10201005
/// Incorporates an already-simplified [`FlatPat`] into a new candidate.
@@ -1254,6 +1239,10 @@ struct MatchTreeBranch<'tcx> {
12541239
struct BuiltMatchTree<'tcx> {
12551240
branches: Vec<MatchTreeBranch<'tcx>>,
12561241
otherwise_block: BasicBlock,
1242+
/// If any of the branches had a guard, we collect here the places and locals to fakely borrow
1243+
/// to ensure match guards can't modify the values as we match them. For more details, see
1244+
/// [`util::collect_fake_borrows`].
1245+
fake_borrow_temps: Vec<(Place<'tcx>, Local, FakeBorrowKind)>,
12571246
}
12581247

12591248
impl<'tcx> MatchTreeSubBranch<'tcx> {
@@ -1304,12 +1293,18 @@ impl<'tcx> MatchTreeBranch<'tcx> {
13041293
}
13051294
}
13061295

1296+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
1297+
enum HasMatchGuard {
1298+
Yes,
1299+
No,
1300+
}
1301+
13071302
impl<'a, 'tcx> Builder<'a, 'tcx> {
13081303
/// The entrypoint of the matching algorithm. Create the decision tree for the match expression,
13091304
/// starting from `block`.
13101305
///
1311-
/// Modifies `candidates` to store the bindings and type ascriptions for
1312-
/// that candidate.
1306+
/// `patterns` is a list of patterns, one for each arm. The associated boolean indicates whether
1307+
/// the arm has a guard.
13131308
///
13141309
/// `refutable` indicates whether the candidate list is refutable (for `if let` and `let else`)
13151310
/// or not (for `let` and `match`). In the refutable case we return the block to which we branch
@@ -1320,9 +1315,30 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
13201315
scrutinee_span: Span,
13211316
scrutinee_place_builder: &PlaceBuilder<'tcx>,
13221317
match_start_span: Span,
1323-
mut candidates: Vec<Candidate<'pat, 'tcx>>,
1318+
patterns: Vec<(&'pat Pat<'tcx>, HasMatchGuard)>,
13241319
refutable: bool,
1325-
) -> BuiltMatchTree<'tcx> {
1320+
) -> BuiltMatchTree<'tcx>
1321+
where
1322+
'tcx: 'pat,
1323+
{
1324+
// Assemble the initial list of candidates. These top-level candidates are 1:1 with the
1325+
// input patterns, but other parts of match lowering also introduce subcandidates (for
1326+
// sub-or-patterns). So inside the algorithm, the candidates list may not correspond to
1327+
// match arms directly.
1328+
let mut candidates: Vec<_> = patterns
1329+
.into_iter()
1330+
.map(|(pat, has_guard)| {
1331+
Candidate::new(scrutinee_place_builder.clone(), pat, has_guard, self)
1332+
})
1333+
.collect();
1334+
1335+
let fake_borrow_temps = util::collect_fake_borrows(
1336+
self,
1337+
&candidates,
1338+
scrutinee_span,
1339+
scrutinee_place_builder.base(),
1340+
);
1341+
13261342
// See the doc comment on `match_candidates` for why we have an otherwise block.
13271343
let otherwise_block = self.cfg.start_new_block();
13281344

@@ -1408,6 +1424,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
14081424
BuiltMatchTree {
14091425
branches: candidates.into_iter().map(MatchTreeBranch::from_candidate).collect(),
14101426
otherwise_block,
1427+
fake_borrow_temps,
14111428
}
14121429
}
14131430

@@ -2116,9 +2133,14 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
21162133
) -> BlockAnd<()> {
21172134
let expr_span = self.thir[expr_id].span;
21182135
let scrutinee = unpack!(block = self.lower_scrutinee(block, expr_id, expr_span));
2119-
let candidate = Candidate::new(scrutinee.clone(), pat, false, self);
2120-
let built_tree =
2121-
self.lower_match_tree(block, expr_span, &scrutinee, pat.span, vec![candidate], true);
2136+
let built_tree = self.lower_match_tree(
2137+
block,
2138+
expr_span,
2139+
&scrutinee,
2140+
pat.span,
2141+
vec![(pat, HasMatchGuard::No)],
2142+
true,
2143+
);
21222144
let [branch] = built_tree.branches.try_into().unwrap();
21232145

21242146
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)