Skip to content

match lowering: build the Place instead of keeping a PlaceBuilder around #122439

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 27, 2024
+42 −36
Merged

match lowering: build the Place instead of keeping a PlaceBuilder around #122439

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3878b37
Rename
Nadrieril Mar 2, 2024
14f186c
Store `Place` instead of `PlaceBuilder` in `MatchPair`
Nadrieril Mar 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions compiler/rustc_mir_build/src/build/matches/mod.rs
View file
Original file line number Diff line number Diff line change
@@ -1106,7 +1106,10 @@ impl<'pat, 'tcx> TestCase<'pat, 'tcx> {
#[derive(Debug, Clone)]
pub(crate) struct MatchPair<'pat, 'tcx> {
/// 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...
// Invariant: after creation and simplification in `Candidate::new()`, this must not be
@@ -1587,11 +1590,12 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
fn pick_test(
&mut self,
candidates: &mut [&mut Candidate<'_, 'tcx>],
) -> (PlaceBuilder<'tcx>, Test<'tcx>) {
) -> (Place<'tcx>, Test<'tcx>) {
// Extract the match-pair from the highest priority candidate
let match_pair = &candidates.first().unwrap().match_pairs[0];
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);

(match_place, test)
@@ -1632,7 +1636,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
/// - candidate 1 becomes `[y @ false]` since we know that `x` was `false`.
fn sort_candidates<'b, 'c, 'pat>(
&mut self,
match_place: &PlaceBuilder<'tcx>,
match_place: Place<'tcx>,
test: &Test<'tcx>,
mut candidates: &'b mut [&'c mut Candidate<'pat, 'tcx>],
) -> (
@@ -1650,7 +1654,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// sorting.
while let Some(candidate) = candidates.first_mut() {
let Some(branch) =
self.sort_candidate(&match_place, test, candidate, &target_candidates)
self.sort_candidate(match_place, test, candidate, &target_candidates)
else {
break;
};
@@ -1778,7 +1782,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
// For each of the N possible test outcomes, build the vector of candidates that applies if
// the test has that particular outcome.
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
// `target_candidates` match.
@@ -1818,7 +1822,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
scrutinee_span,
start_block,
remainder_start,
&match_place,
match_place,
&test,
target_blocks,
);
17 changes: 9 additions & 8 deletions compiler/rustc_mir_build/src/build/matches/test.rs
View file
Original file line number Diff line number Diff line change
@@ -5,7 +5,6 @@
// identify what tests are needed, perform the tests, and then filter
// 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::Builder;
use rustc_data_structures::fx::FxIndexMap;
@@ -55,18 +54,17 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
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(
&mut self,
match_start_span: Span,
scrutinee_span: Span,
block: BasicBlock,
otherwise_block: BasicBlock,
place_builder: &PlaceBuilder<'tcx>,
place: Place<'tcx>,
test: &Test<'tcx>,
target_blocks: FxIndexMap<TestBranch<'tcx>, BasicBlock>,
) {
let place = place_builder.to_place(self);
let place_ty = place.ty(&self.local_decls, self.tcx);
debug!(?place, ?place_ty);
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.
pub(super) fn sort_candidate(
&mut self,
test_place: &PlaceBuilder<'tcx>,
test_place: Place<'tcx>,
test: &Test<'tcx>,
candidate: &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
// both require tests; you'd expect one side to be simplified
// away.)
let (match_pair_index, match_pair) =
candidate.match_pairs.iter().enumerate().find(|&(_, mp)| mp.place == *test_place)?;
let (match_pair_index, match_pair) = candidate
.match_pairs
.iter()
.enumerate()
.find(|&(_, mp)| mp.place == Some(test_place))?;

let fully_matched;
let ret = match (&test.kind, &match_pair.test_case) {
@@ -521,7 +522,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
candidate
.match_pairs
.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
.get(&TestBranch::Failure)
43 changes: 22 additions & 21 deletions compiler/rustc_mir_build/src/build/matches/util.rs
View file
Original file line number Diff line number Diff line change
@@ -95,35 +95,37 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {

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

// Only add the OpaqueCast projection if the given place is an opaque type and the
// 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) => {
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()
}
_ => true,
};
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 mut subpairs = Vec::new();
let test_case = match pattern.kind {
PatKind::Never | PatKind::Wild | PatKind::Error(_) => default_irrefutable(),
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) => {
@@ -142,13 +144,13 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
..
} => {
// 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(),
source,
variance,
});

subpairs.push(MatchPair::new(place.clone(), subpattern, cx));
subpairs.push(MatchPair::new(place_builder, subpattern, cx));
TestCase::Irrefutable { ascription, binding: None }
}

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

if let Some(subpattern) = subpattern.as_ref() {
// 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 }
}

PatKind::InlineConstant { subpattern: ref pattern, def, .. } => {
// 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 parent_id = cx.tcx.typeck_root_def_id(cx.def_id.to_def_id());
let args = ty::InlineConstArgs::new(
@@ -203,16 +205,16 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
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 }
}

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()
}
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() {
default_irrefutable()
@@ -225,7 +227,7 @@ impl<'pat, 'tcx> MatchPair<'pat, 'tcx> {
}

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

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

PatKind::Leaf { ref subpatterns } => {
subpairs = cx.field_match_pairs(place.clone(), subpatterns);
subpairs = cx.field_match_pairs(place_builder, subpatterns);
default_irrefutable()
}

PatKind::Deref { ref subpattern } => {
let place_builder = place.clone().deref();
subpairs.push(MatchPair::new(place_builder, subpattern, cx));
subpairs.push(MatchPair::new(place_builder.deref(), subpattern, cx));
default_irrefutable()
}

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

for subpair in &match_pair.subpairs {