Skip to content

Commit a2122dc

Browse files
authored
Rollup merge of rust-lang#122439 - Nadrieril:store-built-place, r=compiler-errors
match lowering: build the `Place` instead of keeping a `PlaceBuilder` around Outside of `MatchPair::new` we don't construct new places, so we don't need to keep a `PlaceBuilder` around. A bit annoyingly we have to store an `Option<Place>` even though it's never `None` after simplification, but the alternative would be to re-entangle `MatchPair` construction and simplification and I'd rather not do that.
2 parents 435b525 + 14f186c commit a2122dc

File tree

3 files changed

+42
-36
lines changed

3 files changed

+42
-36
lines changed

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

+11-7
Original file line numberDiff line numberDiff line change
@@ -1111,7 +1111,10 @@ impl<'pat, 'tcx> TestCase<'pat, 'tcx> {
11111111
#[derive(Debug, Clone)]
11121112
pub(crate) struct MatchPair<'pat, 'tcx> {
11131113
/// This place...
1114-
place: PlaceBuilder<'tcx>,
1114+
// This can be `None` if it referred to a non-captured place in a closure.
1115+
// Invariant: place.is_none() => test_case is Irrefutable
1116+
// In other words this must be `Some(_)` after simplification.
1117+
place: Option<Place<'tcx>>,
11151118

11161119
/// ... must pass this test...
11171120
// Invariant: after creation and simplification in `Candidate::new()`, this must not be
@@ -1595,11 +1598,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
15951598
fn pick_test(
15961599
&mut self,
15971600
candidates: &mut [&mut Candidate<'_, 'tcx>],
1598-
) -> (PlaceBuilder<'tcx>, Test<'tcx>) {
1601+
) -> (Place<'tcx>, Test<'tcx>) {
15991602
// Extract the match-pair from the highest priority candidate
16001603
let match_pair = &candidates.first().unwrap().match_pairs[0];
16011604
let test = self.test(match_pair);
1602-
let match_place = match_pair.place.clone();
1605+
// Unwrap is ok after simplification.
1606+
let match_place = match_pair.place.unwrap();
16031607
debug!(?test, ?match_pair);
16041608

16051609
(match_place, test)
@@ -1640,7 +1644,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
16401644
/// - candidate 1 becomes `[y @ false]` since we know that `x` was `false`.
16411645
fn sort_candidates<'b, 'c, 'pat>(
16421646
&mut self,
1643-
match_place: &PlaceBuilder<'tcx>,
1647+
match_place: Place<'tcx>,
16441648
test: &Test<'tcx>,
16451649
mut candidates: &'b mut [&'c mut Candidate<'pat, 'tcx>],
16461650
) -> (
@@ -1658,7 +1662,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
16581662
// sorting.
16591663
while let Some(candidate) = candidates.first_mut() {
16601664
let Some(branch) =
1661-
self.sort_candidate(&match_place, test, candidate, &target_candidates)
1665+
self.sort_candidate(match_place, test, candidate, &target_candidates)
16621666
else {
16631667
break;
16641668
};
@@ -1786,7 +1790,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
17861790
// For each of the N possible test outcomes, build the vector of candidates that applies if
17871791
// the test has that particular outcome.
17881792
let (remaining_candidates, target_candidates) =
1789-
self.sort_candidates(&match_place, &test, candidates);
1793+
self.sort_candidates(match_place, &test, candidates);
17901794

17911795
// The block that we should branch to if none of the
17921796
// `target_candidates` match.
@@ -1826,7 +1830,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
18261830
scrutinee_span,
18271831
start_block,
18281832
remainder_start,
1829-
&match_place,
1833+
match_place,
18301834
&test,
18311835
target_blocks,
18321836
);

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

+9-8
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
// identify what tests are needed, perform the tests, and then filter
66
// the candidates based on the result.
77

8-
use crate::build::expr::as_place::PlaceBuilder;
98
use crate::build::matches::{Candidate, MatchPair, Test, TestBranch, TestCase, TestKind};
109
use crate::build::Builder;
1110
use rustc_data_structures::fx::FxIndexMap;
@@ -55,18 +54,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
5554
Test { span: match_pair.pattern.span, kind }
5655
}
5756

58-
#[instrument(skip(self, target_blocks, place_builder), level = "debug")]
57+
#[instrument(skip(self, target_blocks, place), level = "debug")]
5958
pub(super) fn perform_test(
6059
&mut self,
6160
match_start_span: Span,
6261
scrutinee_span: Span,
6362
block: BasicBlock,
6463
otherwise_block: BasicBlock,
65-
place_builder: &PlaceBuilder<'tcx>,
64+
place: Place<'tcx>,
6665
test: &Test<'tcx>,
6766
target_blocks: FxIndexMap<TestBranch<'tcx>, BasicBlock>,
6867
) {
69-
let place = place_builder.to_place(self);
7068
let place_ty = place.ty(&self.local_decls, self.tcx);
7169
debug!(?place, ?place_ty);
7270
let target_block = |branch| target_blocks.get(&branch).copied().unwrap_or(otherwise_block);
@@ -475,7 +473,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
475473
/// tighter match code if we do something a bit different.
476474
pub(super) fn sort_candidate(
477475
&mut self,
478-
test_place: &PlaceBuilder<'tcx>,
476+
test_place: Place<'tcx>,
479477
test: &Test<'tcx>,
480478
candidate: &mut Candidate<'_, 'tcx>,
481479
sorted_candidates: &FxIndexMap<TestBranch<'tcx>, Vec<&mut Candidate<'_, 'tcx>>>,
@@ -486,8 +484,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
486484
// than one, but it'd be very unusual to have two sides that
487485
// both require tests; you'd expect one side to be simplified
488486
// away.)
489-
let (match_pair_index, match_pair) =
490-
candidate.match_pairs.iter().enumerate().find(|&(_, mp)| mp.place == *test_place)?;
487+
let (match_pair_index, match_pair) = candidate
488+
.match_pairs
489+
.iter()
490+
.enumerate()
491+
.find(|&(_, mp)| mp.place == Some(test_place))?;
491492

492493
let fully_matched;
493494
let ret = match (&test.kind, &match_pair.test_case) {
@@ -521,7 +522,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
521522
candidate
522523
.match_pairs
523524
.iter()
524-
.any(|mp| mp.place == *test_place && is_covering_range(&mp.test_case))
525+
.any(|mp| mp.place == Some(test_place) && is_covering_range(&mp.test_case))
525526
};
526527
if sorted_candidates
527528
.get(&TestBranch::Failure)

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

+22-21
Original file line numberDiff line numberDiff line change
@@ -95,35 +95,37 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
9595

9696
impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
9797
pub(in crate::build) fn new(
98-
mut place: PlaceBuilder<'tcx>,
98+
mut place_builder: PlaceBuilder<'tcx>,
9999
pattern: &'pat Pat<'tcx>,
100100
cx: &mut Builder<'_, 'tcx>,
101101
) -> MatchPair<'pat, 'tcx> {
102102
// Force the place type to the pattern's type.
103103
// FIXME(oli-obk): can we use this to simplify slice/array pattern hacks?
104-
if let Some(resolved) = place.resolve_upvar(cx) {
105-
place = resolved;
104+
if let Some(resolved) = place_builder.resolve_upvar(cx) {
105+
place_builder = resolved;
106106
}
107107

108108
// Only add the OpaqueCast projection if the given place is an opaque type and the
109109
// expected type from the pattern is not.
110-
let may_need_cast = match place.base() {
110+
let may_need_cast = match place_builder.base() {
111111
PlaceBase::Local(local) => {
112-
let ty = Place::ty_from(local, place.projection(), &cx.local_decls, cx.tcx).ty;
112+
let ty =
113+
Place::ty_from(local, place_builder.projection(), &cx.local_decls, cx.tcx).ty;
113114
ty != pattern.ty && ty.has_opaque_types()
114115
}
115116
_ => true,
116117
};
117118
if may_need_cast {
118-
place = place.project(ProjectionElem::OpaqueCast(pattern.ty));
119+
place_builder = place_builder.project(ProjectionElem::OpaqueCast(pattern.ty));
119120
}
120121

122+
let place = place_builder.try_to_place(cx);
121123
let default_irrefutable = || TestCase::Irrefutable { binding: None, ascription: None };
122124
let mut subpairs = Vec::new();
123125
let test_case = match pattern.kind {
124126
PatKind::Never | PatKind::Wild | PatKind::Error(_) => default_irrefutable(),
125127
PatKind::Or { ref pats } => TestCase::Or {
126-
pats: pats.iter().map(|pat| FlatPat::new(place.clone(), pat, cx)).collect(),
128+
pats: pats.iter().map(|pat| FlatPat::new(place_builder.clone(), pat, cx)).collect(),
127129
},
128130

129131
PatKind::Range(ref range) => {
@@ -142,13 +144,13 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
142144
..
143145
} => {
144146
// Apply the type ascription to the value at `match_pair.place`
145-
let ascription = place.try_to_place(cx).map(|source| super::Ascription {
147+
let ascription = place.map(|source| super::Ascription {
146148
annotation: annotation.clone(),
147149
source,
148150
variance,
149151
});
150152

151-
subpairs.push(MatchPair::new(place.clone(), subpattern, cx));
153+
subpairs.push(MatchPair::new(place_builder, subpattern, cx));
152154
TestCase::Irrefutable { ascription, binding: None }
153155
}
154156

@@ -161,7 +163,7 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
161163
ref subpattern,
162164
is_primary: _,
163165
} => {
164-
let binding = place.try_to_place(cx).map(|source| super::Binding {
166+
let binding = place.map(|source| super::Binding {
165167
span: pattern.span,
166168
source,
167169
var_id: var,
@@ -170,14 +172,14 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
170172

171173
if let Some(subpattern) = subpattern.as_ref() {
172174
// this is the `x @ P` case; have to keep matching against `P` now
173-
subpairs.push(MatchPair::new(place.clone(), subpattern, cx));
175+
subpairs.push(MatchPair::new(place_builder, subpattern, cx));
174176
}
175177
TestCase::Irrefutable { ascription: None, binding }
176178
}
177179

178180
PatKind::InlineConstant { subpattern: ref pattern, def, .. } => {
179181
// Apply a type ascription for the inline constant to the value at `match_pair.place`
180-
let ascription = place.try_to_place(cx).map(|source| {
182+
let ascription = place.map(|source| {
181183
let span = pattern.span;
182184
let parent_id = cx.tcx.typeck_root_def_id(cx.def_id.to_def_id());
183185
let args = ty::InlineConstArgs::new(
@@ -203,16 +205,16 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
203205
super::Ascription { annotation, source, variance: ty::Contravariant }
204206
});
205207

206-
subpairs.push(MatchPair::new(place.clone(), pattern, cx));
208+
subpairs.push(MatchPair::new(place_builder, pattern, cx));
207209
TestCase::Irrefutable { ascription, binding: None }
208210
}
209211

210212
PatKind::Array { ref prefix, ref slice, ref suffix } => {
211-
cx.prefix_slice_suffix(&mut subpairs, &place, prefix, slice, suffix);
213+
cx.prefix_slice_suffix(&mut subpairs, &place_builder, prefix, slice, suffix);
212214
default_irrefutable()
213215
}
214216
PatKind::Slice { ref prefix, ref slice, ref suffix } => {
215-
cx.prefix_slice_suffix(&mut subpairs, &place, prefix, slice, suffix);
217+
cx.prefix_slice_suffix(&mut subpairs, &place_builder, prefix, slice, suffix);
216218

217219
if prefix.is_empty() && slice.is_some() && suffix.is_empty() {
218220
default_irrefutable()
@@ -225,7 +227,7 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
225227
}
226228

227229
PatKind::Variant { adt_def, variant_index, args, ref subpatterns } => {
228-
let downcast_place = place.clone().downcast(adt_def, variant_index); // `(x as Variant)`
230+
let downcast_place = place_builder.downcast(adt_def, variant_index); // `(x as Variant)`
229231
subpairs = cx.field_match_pairs(downcast_place, subpatterns);
230232

231233
let irrefutable = adt_def.variants().iter_enumerated().all(|(i, v)| {
@@ -247,13 +249,12 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
247249
}
248250

249251
PatKind::Leaf { ref subpatterns } => {
250-
subpairs = cx.field_match_pairs(place.clone(), subpatterns);
252+
subpairs = cx.field_match_pairs(place_builder, subpatterns);
251253
default_irrefutable()
252254
}
253255

254256
PatKind::Deref { ref subpattern } => {
255-
let place_builder = place.clone().deref();
256-
subpairs.push(MatchPair::new(place_builder, subpattern, cx));
257+
subpairs.push(MatchPair::new(place_builder.deref(), subpattern, cx));
257258
default_irrefutable()
258259
}
259260

@@ -310,8 +311,8 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> {
310311
}
311312
} else {
312313
// Insert a Shallow borrow of any place that is switched on.
313-
if let Some(resolved_place) = match_pair.place.try_to_place(self.cx) {
314-
self.fake_borrows.insert(resolved_place);
314+
if let Some(place) = match_pair.place {
315+
self.fake_borrows.insert(place);
315316
}
316317

317318
for subpair in &match_pair.subpairs {

0 commit comments

Comments
 (0)