Rollup merge of #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.
This commit is contained in:
Matthias Krüger 2024-03-27 05:21:15 +01:00 committed by GitHub
commit a2122dc1e3
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 42 additions and 36 deletions

View File

@ -1111,7 +1111,10 @@ impl<'pat, 'tcx> TestCase<'pat, 'tcx> {
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub(crate) struct MatchPair<'pat, 'tcx> { pub(crate) struct MatchPair<'pat, 'tcx> {
/// This place... /// This place...
place: PlaceBuilder<'tcx>, // This can be `None` if it referred to a non-captured place in a closure.
// Invariant: place.is_none() => test_case is Irrefutable
// In other words this must be `Some(_)` after simplification.
place: Option<Place<'tcx>>,
/// ... must pass this test... /// ... must pass this test...
// Invariant: after creation and simplification in `Candidate::new()`, this must not be // Invariant: after creation and simplification in `Candidate::new()`, this must not be
@ -1595,11 +1598,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
fn pick_test( fn pick_test(
&mut self, &mut self,
candidates: &mut [&mut Candidate<'_, 'tcx>], candidates: &mut [&mut Candidate<'_, 'tcx>],
) -> (PlaceBuilder<'tcx>, Test<'tcx>) { ) -> (Place<'tcx>, Test<'tcx>) {
// Extract the match-pair from the highest priority candidate // Extract the match-pair from the highest priority candidate
let match_pair = &candidates.first().unwrap().match_pairs[0]; let match_pair = &candidates.first().unwrap().match_pairs[0];
let test = self.test(match_pair); let test = self.test(match_pair);
let match_place = match_pair.place.clone(); // Unwrap is ok after simplification.
let match_place = match_pair.place.unwrap();
debug!(?test, ?match_pair); debug!(?test, ?match_pair);
(match_place, test) (match_place, test)
@ -1640,7 +1644,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// - candidate 1 becomes `[y @ false]` since we know that `x` was `false`. /// - candidate 1 becomes `[y @ false]` since we know that `x` was `false`.
fn sort_candidates<'b, 'c, 'pat>( fn sort_candidates<'b, 'c, 'pat>(
&mut self, &mut self,
match_place: &PlaceBuilder<'tcx>, match_place: Place<'tcx>,
test: &Test<'tcx>, test: &Test<'tcx>,
mut candidates: &'b mut [&'c mut Candidate<'pat, 'tcx>], mut candidates: &'b mut [&'c mut Candidate<'pat, 'tcx>],
) -> ( ) -> (
@ -1658,7 +1662,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// sorting. // sorting.
while let Some(candidate) = candidates.first_mut() { while let Some(candidate) = candidates.first_mut() {
let Some(branch) = let Some(branch) =
self.sort_candidate(&match_place, test, candidate, &target_candidates) self.sort_candidate(match_place, test, candidate, &target_candidates)
else { else {
break; break;
}; };
@ -1786,7 +1790,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// For each of the N possible test outcomes, build the vector of candidates that applies if // For each of the N possible test outcomes, build the vector of candidates that applies if
// the test has that particular outcome. // the test has that particular outcome.
let (remaining_candidates, target_candidates) = let (remaining_candidates, target_candidates) =
self.sort_candidates(&match_place, &test, candidates); self.sort_candidates(match_place, &test, candidates);
// The block that we should branch to if none of the // The block that we should branch to if none of the
// `target_candidates` match. // `target_candidates` match.
@ -1826,7 +1830,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
scrutinee_span, scrutinee_span,
start_block, start_block,
remainder_start, remainder_start,
&match_place, match_place,
&test, &test,
target_blocks, target_blocks,
); );

View File

@ -5,7 +5,6 @@
// identify what tests are needed, perform the tests, and then filter // identify what tests are needed, perform the tests, and then filter
// the candidates based on the result. // the candidates based on the result.
use crate::build::expr::as_place::PlaceBuilder;
use crate::build::matches::{Candidate, MatchPair, Test, TestBranch, TestCase, TestKind}; use crate::build::matches::{Candidate, MatchPair, Test, TestBranch, TestCase, TestKind};
use crate::build::Builder; use crate::build::Builder;
use rustc_data_structures::fx::FxIndexMap; use rustc_data_structures::fx::FxIndexMap;
@ -55,18 +54,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
Test { span: match_pair.pattern.span, kind } Test { span: match_pair.pattern.span, kind }
} }
#[instrument(skip(self, target_blocks, place_builder), level = "debug")] #[instrument(skip(self, target_blocks, place), level = "debug")]
pub(super) fn perform_test( pub(super) fn perform_test(
&mut self, &mut self,
match_start_span: Span, match_start_span: Span,
scrutinee_span: Span, scrutinee_span: Span,
block: BasicBlock, block: BasicBlock,
otherwise_block: BasicBlock, otherwise_block: BasicBlock,
place_builder: &PlaceBuilder<'tcx>, place: Place<'tcx>,
test: &Test<'tcx>, test: &Test<'tcx>,
target_blocks: FxIndexMap<TestBranch<'tcx>, BasicBlock>, target_blocks: FxIndexMap<TestBranch<'tcx>, BasicBlock>,
) { ) {
let place = place_builder.to_place(self);
let place_ty = place.ty(&self.local_decls, self.tcx); let place_ty = place.ty(&self.local_decls, self.tcx);
debug!(?place, ?place_ty); debug!(?place, ?place_ty);
let target_block = |branch| target_blocks.get(&branch).copied().unwrap_or(otherwise_block); let target_block = |branch| target_blocks.get(&branch).copied().unwrap_or(otherwise_block);
@ -475,7 +473,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// tighter match code if we do something a bit different. /// tighter match code if we do something a bit different.
pub(super) fn sort_candidate( pub(super) fn sort_candidate(
&mut self, &mut self,
test_place: &PlaceBuilder<'tcx>, test_place: Place<'tcx>,
test: &Test<'tcx>, test: &Test<'tcx>,
candidate: &mut Candidate<'_, 'tcx>, candidate: &mut Candidate<'_, 'tcx>,
sorted_candidates: &FxIndexMap<TestBranch<'tcx>, Vec<&mut Candidate<'_, 'tcx>>>, sorted_candidates: &FxIndexMap<TestBranch<'tcx>, Vec<&mut Candidate<'_, 'tcx>>>,
@ -486,8 +484,11 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// than one, but it'd be very unusual to have two sides that // than one, but it'd be very unusual to have two sides that
// both require tests; you'd expect one side to be simplified // both require tests; you'd expect one side to be simplified
// away.) // away.)
let (match_pair_index, match_pair) = let (match_pair_index, match_pair) = candidate
candidate.match_pairs.iter().enumerate().find(|&(_, mp)| mp.place == *test_place)?; .match_pairs
.iter()
.enumerate()
.find(|&(_, mp)| mp.place == Some(test_place))?;
let fully_matched; let fully_matched;
let ret = match (&test.kind, &match_pair.test_case) { let ret = match (&test.kind, &match_pair.test_case) {
@ -521,7 +522,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
candidate candidate
.match_pairs .match_pairs
.iter() .iter()
.any(|mp| mp.place == *test_place && is_covering_range(&mp.test_case)) .any(|mp| mp.place == Some(test_place) && is_covering_range(&mp.test_case))
}; };
if sorted_candidates if sorted_candidates
.get(&TestBranch::Failure) .get(&TestBranch::Failure)

View File

@ -95,35 +95,37 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
impl<'pat, 'tcx> MatchPair<'pat, 'tcx> { impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
pub(in crate::build) fn new( pub(in crate::build) fn new(
mut place: PlaceBuilder<'tcx>, mut place_builder: PlaceBuilder<'tcx>,
pattern: &'pat Pat<'tcx>, pattern: &'pat Pat<'tcx>,
cx: &mut Builder<'_, 'tcx>, cx: &mut Builder<'_, 'tcx>,
) -> MatchPair<'pat, 'tcx> { ) -> MatchPair<'pat, 'tcx> {
// Force the place type to the pattern's type. // Force the place type to the pattern's type.
// FIXME(oli-obk): can we use this to simplify slice/array pattern hacks? // FIXME(oli-obk): can we use this to simplify slice/array pattern hacks?
if let Some(resolved) = place.resolve_upvar(cx) { if let Some(resolved) = place_builder.resolve_upvar(cx) {
place = resolved; place_builder = resolved;
} }
// Only add the OpaqueCast projection if the given place is an opaque type and the // Only add the OpaqueCast projection if the given place is an opaque type and the
// expected type from the pattern is not. // expected type from the pattern is not.
let may_need_cast = match place.base() { let may_need_cast = match place_builder.base() {
PlaceBase::Local(local) => { PlaceBase::Local(local) => {
let ty = Place::ty_from(local, place.projection(), &cx.local_decls, cx.tcx).ty; let ty =
Place::ty_from(local, place_builder.projection(), &cx.local_decls, cx.tcx).ty;
ty != pattern.ty && ty.has_opaque_types() ty != pattern.ty && ty.has_opaque_types()
} }
_ => true, _ => true,
}; };
if may_need_cast { if may_need_cast {
place = place.project(ProjectionElem::OpaqueCast(pattern.ty)); place_builder = place_builder.project(ProjectionElem::OpaqueCast(pattern.ty));
} }
let place = place_builder.try_to_place(cx);
let default_irrefutable = || TestCase::Irrefutable { binding: None, ascription: None }; let default_irrefutable = || TestCase::Irrefutable { binding: None, ascription: None };
let mut subpairs = Vec::new(); let mut subpairs = Vec::new();
let test_case = match pattern.kind { let test_case = match pattern.kind {
PatKind::Never | PatKind::Wild | PatKind::Error(_) => default_irrefutable(), PatKind::Never | PatKind::Wild | PatKind::Error(_) => default_irrefutable(),
PatKind::Or { ref pats } => TestCase::Or { PatKind::Or { ref pats } => TestCase::Or {
pats: pats.iter().map(|pat| FlatPat::new(place.clone(), pat, cx)).collect(), pats: pats.iter().map(|pat| FlatPat::new(place_builder.clone(), pat, cx)).collect(),
}, },
PatKind::Range(ref range) => { PatKind::Range(ref range) => {
@ -142,13 +144,13 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
.. ..
} => { } => {
// Apply the type ascription to the value at `match_pair.place` // Apply the type ascription to the value at `match_pair.place`
let ascription = place.try_to_place(cx).map(|source| super::Ascription { let ascription = place.map(|source| super::Ascription {
annotation: annotation.clone(), annotation: annotation.clone(),
source, source,
variance, variance,
}); });
subpairs.push(MatchPair::new(place.clone(), subpattern, cx)); subpairs.push(MatchPair::new(place_builder, subpattern, cx));
TestCase::Irrefutable { ascription, binding: None } TestCase::Irrefutable { ascription, binding: None }
} }
@ -161,7 +163,7 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
ref subpattern, ref subpattern,
is_primary: _, is_primary: _,
} => { } => {
let binding = place.try_to_place(cx).map(|source| super::Binding { let binding = place.map(|source| super::Binding {
span: pattern.span, span: pattern.span,
source, source,
var_id: var, var_id: var,
@ -170,14 +172,14 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
if let Some(subpattern) = subpattern.as_ref() { if let Some(subpattern) = subpattern.as_ref() {
// this is the `x @ P` case; have to keep matching against `P` now // this is the `x @ P` case; have to keep matching against `P` now
subpairs.push(MatchPair::new(place.clone(), subpattern, cx)); subpairs.push(MatchPair::new(place_builder, subpattern, cx));
} }
TestCase::Irrefutable { ascription: None, binding } TestCase::Irrefutable { ascription: None, binding }
} }
PatKind::InlineConstant { subpattern: ref pattern, def, .. } => { PatKind::InlineConstant { subpattern: ref pattern, def, .. } => {
// Apply a type ascription for the inline constant to the value at `match_pair.place` // Apply a type ascription for the inline constant to the value at `match_pair.place`
let ascription = place.try_to_place(cx).map(|source| { let ascription = place.map(|source| {
let span = pattern.span; let span = pattern.span;
let parent_id = cx.tcx.typeck_root_def_id(cx.def_id.to_def_id()); let parent_id = cx.tcx.typeck_root_def_id(cx.def_id.to_def_id());
let args = ty::InlineConstArgs::new( let args = ty::InlineConstArgs::new(
@ -203,16 +205,16 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
super::Ascription { annotation, source, variance: ty::Contravariant } super::Ascription { annotation, source, variance: ty::Contravariant }
}); });
subpairs.push(MatchPair::new(place.clone(), pattern, cx)); subpairs.push(MatchPair::new(place_builder, pattern, cx));
TestCase::Irrefutable { ascription, binding: None } TestCase::Irrefutable { ascription, binding: None }
} }
PatKind::Array { ref prefix, ref slice, ref suffix } => { PatKind::Array { ref prefix, ref slice, ref suffix } => {
cx.prefix_slice_suffix(&mut subpairs, &place, prefix, slice, suffix); cx.prefix_slice_suffix(&mut subpairs, &place_builder, prefix, slice, suffix);
default_irrefutable() default_irrefutable()
} }
PatKind::Slice { ref prefix, ref slice, ref suffix } => { PatKind::Slice { ref prefix, ref slice, ref suffix } => {
cx.prefix_slice_suffix(&mut subpairs, &place, prefix, slice, suffix); cx.prefix_slice_suffix(&mut subpairs, &place_builder, prefix, slice, suffix);
if prefix.is_empty() && slice.is_some() && suffix.is_empty() { if prefix.is_empty() && slice.is_some() && suffix.is_empty() {
default_irrefutable() default_irrefutable()
@ -225,7 +227,7 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
} }
PatKind::Variant { adt_def, variant_index, args, ref subpatterns } => { PatKind::Variant { adt_def, variant_index, args, ref subpatterns } => {
let downcast_place = place.clone().downcast(adt_def, variant_index); // `(x as Variant)` let downcast_place = place_builder.downcast(adt_def, variant_index); // `(x as Variant)`
subpairs = cx.field_match_pairs(downcast_place, subpatterns); subpairs = cx.field_match_pairs(downcast_place, subpatterns);
let irrefutable = adt_def.variants().iter_enumerated().all(|(i, v)| { let irrefutable = adt_def.variants().iter_enumerated().all(|(i, v)| {
@ -247,13 +249,12 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
} }
PatKind::Leaf { ref subpatterns } => { PatKind::Leaf { ref subpatterns } => {
subpairs = cx.field_match_pairs(place.clone(), subpatterns); subpairs = cx.field_match_pairs(place_builder, subpatterns);
default_irrefutable() default_irrefutable()
} }
PatKind::Deref { ref subpattern } => { PatKind::Deref { ref subpattern } => {
let place_builder = place.clone().deref(); subpairs.push(MatchPair::new(place_builder.deref(), subpattern, cx));
subpairs.push(MatchPair::new(place_builder, subpattern, cx));
default_irrefutable() default_irrefutable()
} }
@ -310,8 +311,8 @@ impl<'a, 'b, 'tcx> FakeBorrowCollector<'a, 'b, 'tcx> {
} }
} else { } else {
// Insert a Shallow borrow of any place that is switched on. // Insert a Shallow borrow of any place that is switched on.
if let Some(resolved_place) = match_pair.place.try_to_place(self.cx) { if let Some(place) = match_pair.place {
self.fake_borrows.insert(resolved_place); self.fake_borrows.insert(place);
} }
for subpair in &match_pair.subpairs { for subpair in &match_pair.subpairs {